From 0784703dee01b2ac9a1355873d433145813f049f Mon Sep 17 00:00:00 2001 From: Ralf Paffrath Date: Fri, 24 May 2019 21:54:22 +0200 Subject: [PATCH 1/3] add explicit option for SubjectAltName:DNS check Patch by Ralf Paffrath --- radsecproxy.h | 1 + tlscommon.c | 42 +++++++++++++++++++++++++++--------------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/radsecproxy.h b/radsecproxy.h index 9b32705..06a55eb 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -148,6 +148,7 @@ struct clsrvconf { char *matchcertattr; regex_t *certcnregex; regex_t *certuriregex; + regex_t *certdnsregex; char *confrewritein; char *confrewriteout; char *confrewriteusername; diff --git a/tlscommon.c b/tlscommon.c index 7362309..f9fa7fb 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -734,6 +734,13 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { ok = 0; } } + if (conf->certdnsregex) { + debug(DBG_DBG, "verifyconfcert: matching subjectaltname DNS regex %s", conf->matchcertattr); + if (subjectaltnameregexp(cert, GEN_DNS, NULL, conf->certdnsregex) < 1) { + debug(DBG_WARN, "verifyconfcert: subjectaltname DNS not matching regex for host %s (%s)", conf->name, subject); + ok = 0; + } + } free(subject); return ok; } @@ -815,31 +822,36 @@ int addmatchcertattr(struct clsrvconf *conf) { regex_t **r; if (!strncasecmp(conf->matchcertattr, "CN:/", 4)) { - r = &conf->certcnregex; - v = conf->matchcertattr + 4; + r = &conf->certcnregex; + v = conf->matchcertattr + 4; } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:URI:/", 20)) { - r = &conf->certuriregex; - v = conf->matchcertattr + 20; - } else - return 0; + r = &conf->certuriregex; + v = conf->matchcertattr + 20; + } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:DNS:/", 20)) { + r = &conf->certdnsregex; + v = conf->matchcertattr + 20; + } + else + return 0; + if (!*v) - return 0; + return 0; /* regexp, remove optional trailing / if present */ if (v[strlen(v) - 1] == '/') - v[strlen(v) - 1] = '\0'; + v[strlen(v) - 1] = '\0'; if (!*v) - return 0; + return 0; *r = malloc(sizeof(regex_t)); if (!*r) { - debug(DBG_ERR, "malloc failed"); - return 0; + debug(DBG_ERR, "malloc failed"); + return 0; } if (regcomp(*r, v, REG_EXTENDED | REG_ICASE | REG_NOSUB)) { - free(*r); - *r = NULL; - debug(DBG_ERR, "failed to compile regular expression %s", v); - return 0; + free(*r); + *r = NULL; + debug(DBG_ERR, "failed to compile regular expression %s", v); + return 0; } return 1; } From 49f291a9c28e0364fe1bcb4642bd36af10529df8 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 27 May 2019 06:56:29 +0200 Subject: [PATCH 2/3] update manpage and changelog --- ChangeLog | 1 + radsecproxy.conf.5 | 17 +++++++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index a989481..d4036cc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,7 @@ changes since 1.7.2 New features: - Autodetect status-server capability of servers - Minimalistic status-server + - Explicit SubjectAltName:DNS match on certificates Misc: - No longer require docbook2x tools, but include plain manpages diff --git a/radsecproxy.conf.5 b/radsecproxy.conf.5 index 706e9b6..23d4ab6 100644 --- a/radsecproxy.conf.5 +++ b/radsecproxy.conf.5 @@ -348,10 +348,11 @@ of the configured clients (in the order they are defined), to determine which this might mask clients defined later, which then will never be matched. In the case of TLS/DTLS, the name of the client must match the FQDN or IP -address in the client certificate. Note that this is not required when the -client name is an IP prefix. If overlapping clients are defined (see section -above), they will be searched for matching \fBMatchCertificateAttribute\fR, but -they must reference the same tls block. +address in the client certificate (CN or SubectAltName:DNS or SubjectAltName:IP +respectively). Note that this is not required when the client name is an IP +prefix. If overlapping clients are defined (see section above), they will be +searched for matching \fBMatchCertificateAttribute\fR, but they must reference +the same tls block. The allowed options in a client block are: @@ -410,11 +411,11 @@ For a TLS/DTLS client, disable the default behaviour of matching CN or SubjectAltName against the specified hostname or IP address. .RE -\fBMatchCertificateAttribute ((\fR CN \fB|\fR SubjectAltName:URI \fB) :\fR/\fIregexp\fR/\fB )\fR +\fBMatchCertificateAttribute ((\fR CN \fB|\fR SubjectAltName:URI \fB|\fR SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/\fB )\fR .RS Perform additional validation of certificate attributes. Currently only matching -of CN and SubjectAltName type URI is supported. Note that currently this option -can only be specified once in a client block. +of CN and SubjectAltName type URI and DNS is supported. Note that currently this +option can only be specified once in a client block. .RE .BI "DuplicateInterval " seconds @@ -606,7 +607,7 @@ block. The details are not repeated here. Please refer to the definitions in the .br .BR "CertificateNameCheck (" on | off ) .br -\fBmatchCertificateAttribute ((\fR CN \fB|\fR SubjectAltName:URI \fB) :\fR/\fIregexp\fR/\fB )\fR +\fBmatchCertificateAttribute ((\fR CN \fB|\fR SubjectAltName:URI \fB|\fR SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/\fB )\fR .br .BR "AddTTL " 1-255 .br From 5bab3db707898f6b8432df315747e3d5f8b68d9a Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 18 Jun 2019 18:08:47 +0200 Subject: [PATCH 3/3] add explicit option for SubjectAltName:IP check --- ChangeLog | 2 +- radsecproxy.conf.5 | 12 ++++++++---- radsecproxy.h | 3 +++ tlscommon.c | 19 +++++++++++++++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index d4036cc..9fde94a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,7 +2,7 @@ changes since 1.7.2 New features: - Autodetect status-server capability of servers - Minimalistic status-server - - Explicit SubjectAltName:DNS match on certificates + - Explicit SubjectAltName:DNS and :IP match on certificates Misc: - No longer require docbook2x tools, but include plain manpages diff --git a/radsecproxy.conf.5 b/radsecproxy.conf.5 index 23d4ab6..4d322e0 100644 --- a/radsecproxy.conf.5 +++ b/radsecproxy.conf.5 @@ -411,10 +411,12 @@ For a TLS/DTLS client, disable the default behaviour of matching CN or SubjectAltName against the specified hostname or IP address. .RE -\fBMatchCertificateAttribute ((\fR CN \fB|\fR SubjectAltName:URI \fB|\fR SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/\fB )\fR +\fBmatchCertificateAttribute (\fR CN \fB|\fR SubjectAltName:URI \fB|\fR SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/ +.br +\fBMatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR .RS -Perform additional validation of certificate attributes. Currently only matching -of CN and SubjectAltName type URI and DNS is supported. Note that currently this +Perform additional validation of certificate attributes. Currently matching +of CN and SubjectAltName types URI DNS and IP is supported. Note that currently this option can only be specified once in a client block. .RE @@ -607,7 +609,9 @@ block. The details are not repeated here. Please refer to the definitions in the .br .BR "CertificateNameCheck (" on | off ) .br -\fBmatchCertificateAttribute ((\fR CN \fB|\fR SubjectAltName:URI \fB|\fR SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/\fB )\fR +\fBmatchCertificateAttribute (\fR CN \fB|\fR SubjectAltName:URI \fB|\fR SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/ +.br +\fBMatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR .br .BR "AddTTL " 1-255 .br diff --git a/radsecproxy.h b/radsecproxy.h index 06a55eb..0ee1f20 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -6,6 +6,7 @@ #include #include #include +#include #include "list.h" #include "tlv11.h" #include "radmsg.h" @@ -149,6 +150,8 @@ struct clsrvconf { regex_t *certcnregex; regex_t *certuriregex; regex_t *certdnsregex; + in6_addr_t certipmatch; + int certipmatchaf; char *confrewritein; char *confrewriteout; char *confrewriteusername; diff --git a/tlscommon.c b/tlscommon.c index f9fa7fb..4522942 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -709,6 +709,7 @@ int certnamecheck(X509 *cert, struct list *hostports) { int verifyconfcert(X509 *cert, struct clsrvconf *conf) { char *subject; + char addrbuf[INET6_ADDRSTRLEN]; int ok = 1; subject = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0); @@ -741,6 +742,13 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { ok = 0; } } + if (conf->certipmatchaf) { + debug(DBG_DBG, "verifyconfcert: matching subjectaltname IP %s", inet_ntop(conf->certipmatchaf, &conf->certipmatch, addrbuf, INET6_ADDRSTRLEN)); + if (subjectaltnameaddr(cert, conf->certipmatchaf, &conf->certipmatch) < 1) { + debug(DBG_WARN, "verifyconfcert: subjectaltname IP not matching regex for host %s (%s)", conf->name, subject); + ok = 0; + } + } free(subject); return ok; } @@ -821,6 +829,17 @@ int addmatchcertattr(struct clsrvconf *conf) { char *v; regex_t **r; + if (!strncasecmp(conf->matchcertattr, "SubjectAltName:IP:", 18)) { + if (inet_pton(AF_INET, conf->matchcertattr+18, &conf->certipmatch)) + conf->certipmatchaf = AF_INET; + else if (inet_pton(AF_INET6, conf->matchcertattr+18, &conf->certipmatch)) + conf->certipmatchaf = AF_INET6; + else + return 0; + return 1; + } + + /* the other cases below use a common regex match */ if (!strncasecmp(conf->matchcertattr, "CN:/", 4)) { r = &conf->certcnregex; v = conf->matchcertattr + 4;