From 438ed2329e36abe0ea33ec6f9768ffb879c3c3ee Mon Sep 17 00:00:00 2001 From: Faidon Liambotis Date: Thu, 5 Sep 2019 02:55:10 +0300 Subject: [PATCH 01/38] Fix radsecproxy.conf.5 manpage errors Remove references to 'T<' and 'T>' macros, as well as a a reference to a URL macro. The latter was for a pointer to RFC 6614, which is entirely removed as not overly helpful. Fixes: #49 --- radsecproxy.conf.5 | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/radsecproxy.conf.5 b/radsecproxy.conf.5 index 8d1c7dd..5a30425 100644 --- a/radsecproxy.conf.5 +++ b/radsecproxy.conf.5 @@ -222,7 +222,7 @@ not supported. The FTicksPrefix option is used to set the \fIprefix\fR printed in F-Ticks messages. This allows for use of F-Ticks messages in non-eduroam environments. If no FTicksPrefix option is given, it defaults to the prefix used for eduroam -(\*(T) +(\fIF\-TICKS/eduroam/1.0\fR). .RE @@ -915,5 +915,4 @@ subattributes are removed. .RE .SH "SEE ALSO" -\fBradsecproxy\fR(1), -.URL https://tools.ietf.org/html/rfc6614 " Transport Layer Security (TLS) Encryption for RADIUS " +\fBradsecproxy\fR(1) From 1e12b94c020a241a4ec395667bf857164c65bbc8 Mon Sep 17 00:00:00 2001 From: Faidon Liambotis Date: Thu, 5 Sep 2019 03:40:45 +0300 Subject: [PATCH 02/38] Avoid the hardcoding of /usr/local/etc On Linux systems, sysconfdir is usually /etc, while all the documentation refers to /usr/local/etc. Switch the two manpages that need to refer to the config file location to autoconf-substituted variables, and remove a spurious reference from the example config. --- .gitignore | 2 ++ README | 6 +++--- configure.ac | 10 ++++++++++ radsecproxy.1 => radsecproxy.1.in | 2 +- radsecproxy.conf-example | 3 +-- radsecproxy.conf.5 => radsecproxy.conf.5.in | 4 ++-- 6 files changed, 19 insertions(+), 8 deletions(-) rename radsecproxy.1 => radsecproxy.1.in (98%) rename radsecproxy.conf.5 => radsecproxy.conf.5.in (99%) diff --git a/.gitignore b/.gitignore index fa95497..b7d90ba 100644 --- a/.gitignore +++ b/.gitignore @@ -22,6 +22,8 @@ TAGS radsecproxy radsecproxy-conf radsecproxy-hash +radsecproxy.1 +radsecproxy.conf.5 build-aux/* tests/t_fticks tests/t_rewrite diff --git a/README b/README index 5576b5e..9ce2df2 100644 --- a/README +++ b/README @@ -13,9 +13,9 @@ support. Without any special options to configure, all transports supported by the system will be enabled. See the output from "./configure --help" for how to change this. -To use radsecproxy you need to create a config file which is normally -found in /usr/local/etc/radsecproxy.conf. You can also specify the -location with the "-c" command line option (see below). For further +To use radsecproxy you need to create a config file which is normally found in +/etc/radsecproxy.conf or /usr/local/etc/radsecproxy.conf. You can also specify +the location with the "-c" command line option (see below). For further instructions, please see the enclosed example file and the manpages radsecproxy(1) and radsecproxy.conf(5) diff --git a/configure.ac b/configure.ac index 55d0921..4d53e89 100644 --- a/configure.ac +++ b/configure.ac @@ -92,6 +92,16 @@ if test "x$dtls" = "xyes" ; then TARGET_CFLAGS="$TARGET_CFLAGS -DRADPROT_DTLS" fi +dnl Substitute variables such as sysconfdir +AC_CONFIG_FILES([radsecproxy.1 radsecproxy.conf.5]) + +dnl Expand sysconfdir early to avoid two layers of substitution +test "x$prefix" = xNONE && prefix=$ac_default_prefix +test "x$exec_prefix" = xNONE && exec_prefix='${prefix}' +SYSCONFDIR=`eval echo $sysconfdir` +SYSCONFDIR=`eval echo $SYSCONFDIR` +AC_SUBST(SYSCONFDIR) + AC_SUBST(TARGET_CFLAGS) AC_SUBST(TARGET_LDFLAGS) AX_CHECK_SSL diff --git a/radsecproxy.1 b/radsecproxy.1.in similarity index 98% rename from radsecproxy.1 rename to radsecproxy.1.in index b556ba7..5a5c9e0 100644 --- a/radsecproxy.1 +++ b/radsecproxy.1.in @@ -84,7 +84,7 @@ This signal is ignored. .SH "FILES" .TP -.B /usr/local/etc/radsecproxy.conf +.B @SYSCONFDIR@/radsecproxy.conf .sp The default configuration file. diff --git a/radsecproxy.conf-example b/radsecproxy.conf-example index c9347ee..1730e55 100644 --- a/radsecproxy.conf-example +++ b/radsecproxy.conf-example @@ -1,5 +1,4 @@ -# Master config file, must be in /usr/local/etc/radsecproxy or specified with -c option -# All possible config options are listed below +# Master config file, all possible config options are listed below # First you may define any global options, these are: # diff --git a/radsecproxy.conf.5 b/radsecproxy.conf.5.in similarity index 99% rename from radsecproxy.conf.5 rename to radsecproxy.conf.5.in index 5a30425..5b79e82 100644 --- a/radsecproxy.conf.5 +++ b/radsecproxy.conf.5.in @@ -6,7 +6,7 @@ radsecproxy.conf \- Radsec proxy configuration file .SH DESCRIPTION When the proxy server starts, it will first check the command line arguments, and then read the configuration file. Normally radsecproxy will read the -configuration file \fI/usr/local/etc/radsecproxy.conf\fR. The command line +configuration file \fI@SYSCONFDIR@/radsecproxy.conf\fR. The command line \fB\-c\fR option can be used to instead read an alternate file (see \fBradsecproxy\fR(1) for details). @@ -79,7 +79,7 @@ be included. The value can be a single file, or it can use normal shell globbing to specify multiple files, e.g.: .RS -include /usr/local/etc/radsecproxy.conf.d/*.conf +include @SYSCONFDIR@/radsecproxy.conf.d/*.conf .RE The files are sorted alphabetically. Included files are read in the order they From 2ec81cae3824950d44e53e22e4b505254d2d761e Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 30 Sep 2019 14:27:59 +0200 Subject: [PATCH 03/38] update ChangeLog --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 4b07ae7..4c56953 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ - Fix BSD platform issues - Fix spelling in log messages and manpages - Fix compile issues for unit tests + - Don't hardcode location of config files 2019-07-04 1.8.0 New features: From de8b9434a19d34da77215cc6093700bccb5115d9 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 30 Sep 2019 16:32:08 +0200 Subject: [PATCH 04/38] ready for radsecproxy 1.8.1 --- ChangeLog | 2 +- README | 2 +- configure.ac | 2 +- radsecproxy.conf.5.in | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4c56953..aed6337 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -2019-09-30 changes since 1.8.0 +2019-10-01 1.8.1 Bug fixes: - Handle Tunnel-Password attribute correctly - Fix BSD platform issues diff --git a/README b/README index 9ce2df2..28dfdc2 100644 --- a/README +++ b/README @@ -1,4 +1,4 @@ -This is radsecproxy 1.8.0 +This is radsecproxy 1.8.1 radsecproxy is a generic RADIUS proxy that supports both UDP and TLS (RadSec) RADIUS transports. There is also experimental support for diff --git a/configure.ac b/configure.ac index 4d53e89..a5b8356 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ dnl Copyright (c) 2006-2010, UNINETT AS dnl Copyright (c) 2010-2013,2016, NORDUnet A/S dnl See LICENSE for licensing information. -AC_INIT(radsecproxy, 1.8.0, https://radsecproxy.github.io) +AC_INIT(radsecproxy, 1.8.1, https://radsecproxy.github.io) AC_CONFIG_AUX_DIR([build-aux]) AC_CANONICAL_TARGET AM_INIT_AUTOMAKE diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 5b79e82..10d9a3f 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -1,4 +1,4 @@ -.TH radsecproxy.conf 5 2019-07-04 "radsecproxy 1.8.0" "" +.TH radsecproxy.conf 5 2019-10-01 "radsecproxy 1.8.1" "" .SH NAME radsecproxy.conf \- Radsec proxy configuration file From 93f47610f710a173ccafed8762ab81c2a09552fe Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 16 Oct 2019 18:55:14 +0200 Subject: [PATCH 05/38] Fix wrong config-unhexing if %25 (%) occurs --- ChangeLog | 4 +++ gconfig.c | 63 ++++++++++++++++++++++------------------ gconfig.h | 2 ++ radsecproxy.c | 12 ++++---- tests/t_rewrite_config.c | 6 ++-- 5 files changed, 50 insertions(+), 37 deletions(-) diff --git a/ChangeLog b/ChangeLog index aed6337..03021f9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +chanes since 1.8.1 + Bug fixes: + - Fix wrong config-unhexing if %25 (%) occurs + 2019-10-01 1.8.1 Bug fixes: - Handle Tunnel-Password attribute correctly diff --git a/gconfig.c b/gconfig.c index 81fe63e..db60501 100644 --- a/gconfig.c +++ b/gconfig.c @@ -415,12 +415,14 @@ int getgenericconfig(struct gconffile **cf, char *block, ...) { while ((word = va_arg(ap, char *))) { type = va_arg(ap, int); switch (type) { - case CONF_STR: + case CONF_STR: /*intentional fall-thru, these are identical*/ + case CONF_STR_NOESC: str = va_arg(ap, char **); if (!str) goto errparam; break; - case CONF_MSTR: + case CONF_MSTR: /*intentional fall-thru, these are identical*/ + case CONF_MSTR_NOESC: mstr = va_arg(ap, char ***); if (!mstr) goto errparam; @@ -456,38 +458,43 @@ int getgenericconfig(struct gconffile **cf, char *block, ...) { goto errexit; } - if (((type == CONF_STR || type == CONF_MSTR || type == CONF_BLN || type == CONF_LINT) && conftype != CONF_STR) || - (type == CONF_CBK && conftype != CONF_CBK)) { + if (((type == CONF_STR || type == CONF_STR_NOESC || type == CONF_MSTR || type == CONF_MSTR_NOESC || + type == CONF_BLN || type == CONF_LINT) && conftype != CONF_STR) || + (type == CONF_CBK && conftype != CONF_CBK)) { if (block) debug(DBG_ERR, "configuration error in block %s, wrong syntax for option %s", block, opt); debug(DBG_ERR, "configuration error, wrong syntax for option %s", opt); goto errexit; } - switch (type) { - case CONF_STR: - if (*str) { - debug(DBG_ERR, "configuration error, option %s already set to %s", opt, *str); - goto errexit; - } - unhex(val,0); - *str = val; - break; - case CONF_MSTR: - if (*mstr) - for (n = 0; (*mstr)[n]; n++); - else - n = 0; - newmstr = realloc(*mstr, sizeof(char *) * (n + 2)); - if (!newmstr) { - debug(DBG_ERR, "malloc failed"); - goto errexit; - } - unhex(val,0); - newmstr[n] = val; - newmstr[n + 1] = NULL; - *mstr = newmstr; - break; + switch (type) { + case CONF_STR: /*intentional fall-thru, these are almost identical*/ + case CONF_STR_NOESC: + if (*str) { + debug(DBG_ERR, "configuration error, option %s already set to %s", opt, *str); + goto errexit; + } + if (type == CONF_STR) + unhex(val,0); + *str = val; + break; + case CONF_MSTR: /*intentional fall-thru, these are almost identical*/ + case CONF_MSTR_NOESC: + if (*mstr) + for (n = 0; (*mstr)[n]; n++); + else + n = 0; + newmstr = realloc(*mstr, sizeof(char *) * (n + 2)); + if (!newmstr) { + debug(DBG_ERR, "malloc failed"); + goto errexit; + } + if (type == CONF_MSTR) + unhex(val,0); + newmstr[n] = val; + newmstr[n + 1] = NULL; + *mstr = newmstr; + break; case CONF_BLN: if (!strcasecmp(val, "on")) *bln = 1; diff --git a/gconfig.h b/gconfig.h index a5276f1..6657d96 100644 --- a/gconfig.h +++ b/gconfig.h @@ -6,6 +6,8 @@ #define CONF_MSTR 3 #define CONF_BLN 4 #define CONF_LINT 5 +#define CONF_STR_NOESC 6 +#define CONF_MSTR_NOESC 7 #include diff --git a/radsecproxy.c b/radsecproxy.c index 76b9f90..d5204db 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -2322,7 +2322,7 @@ int confclient_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "host", CONF_MSTR, &conf->hostsrc, "IPv4Only", CONF_BLN, &ipv4only, "IPv6Only", CONF_BLN, &ipv6only, - "secret", CONF_STR, &conf->confsecret, + "secret", CONF_STR_NOESC, &conf->confsecret, #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) "tls", CONF_STR, &conf->tls, "matchcertificateattribute", CONF_STR, &conf->matchcertattr, @@ -2523,7 +2523,7 @@ int confserver_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "IPv4Only", CONF_BLN, &ipv4only, "IPv6Only", CONF_BLN, &ipv6only, "port", CONF_STR, &conf->portsrc, - "secret", CONF_STR, &conf->confsecret, + "secret", CONF_STR_NOESC, &conf->confsecret, #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) "tls", CONF_STR, &conf->tls, "MatchCertificateAttribute", CONF_STR, &conf->matchcertattr, @@ -2688,12 +2688,12 @@ int confrewrite_cb(struct gconffile **cf, void *arg, char *block, char *opt, cha "removeVendorAttribute", CONF_MSTR, &rmvattrs, "whitelistAttribute", CONF_MSTR, &wlattrs, "whitelistVendorAttribute", CONF_MSTR, &wlvattrs, - "addAttribute", CONF_MSTR, &addattrs, - "addVendorAttribute", CONF_MSTR, &addvattrs, + "addAttribute", CONF_MSTR_NOESC, &addattrs, + "addVendorAttribute", CONF_MSTR_NOESC, &addvattrs, "modifyAttribute", CONF_MSTR, &modattrs, "modifyVendorAttribute", CONF_MSTR, &modvattrs, - "supplementAttribute", CONF_MSTR, &supattrs, - "supplementVendorAttribute", CONF_MSTR, &supvattrs, + "supplementAttribute", CONF_MSTR_NOESC, &supattrs, + "supplementVendorAttribute", CONF_MSTR_NOESC, &supvattrs, NULL)) debugx(1, DBG_ERR, "configuration error"); addrewrite(val, whitelist_mode, whitelist_mode? wlattrs : rmattrs, whitelist_mode? wlvattrs : rmvattrs, diff --git a/tests/t_rewrite_config.c b/tests/t_rewrite_config.c index 97ae7fc..2e0ac89 100644 --- a/tests/t_rewrite_config.c +++ b/tests/t_rewrite_config.c @@ -17,16 +17,16 @@ main (int argc, char *argv[]) char **addattrs; int numtests = 1, i; struct tlv *tlv, *expected; - uint8_t expectedvalue[] = {'1',0,0,'1','A'}; + uint8_t expectedvalue[] = {'1',0,0,'1','A','%','4','1'}; printf("1..%d\n", numtests); numtests = 1; addattrs = malloc(2); - addattrs[0] = stringcopy("1:'1%00%001%41", 0); + addattrs[0] = stringcopy("1:'1%00%001%41%2541", 0); addattrs[1] = NULL; - expected = maketlv(1,5,expectedvalue); + expected = maketlv(1,8,expectedvalue); addrewrite(rewritename, 0, NULL, NULL, addattrs, NULL, NULL, NULL, NULL, NULL); From 0098fbc45e5068ef20ba26910e281f3955299176 Mon Sep 17 00:00:00 2001 From: Robert Scheck Date: Sun, 2 Feb 2020 20:37:16 +0100 Subject: [PATCH 06/38] Declare pthread_attr as extern in header (fixes #63) GCC 10 compatibility as per https://gcc.gnu.org/gcc-10/porting_to.html --- radsecproxy.c | 1 + radsecproxy.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/radsecproxy.c b/radsecproxy.c index a4b1211..b281e21 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -84,6 +84,7 @@ extern int optind; extern char *optarg; #endif static const struct protodefs *protodefs[RAD_PROTOCOUNT]; +pthread_attr_t pthread_attr; /* minimum required declarations to avoid reordering code */ struct realm *adddynamicrealmserver(struct realm *realm, char *id); diff --git a/radsecproxy.h b/radsecproxy.h index 3082300..0f20f50 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -263,7 +263,7 @@ int radsrv(struct request *rq); void replyh(struct server *server, unsigned char *buf); struct addrinfo *resolve_hostport_addrinfo(uint8_t type, char *hostport); uint8_t *radattr2ascii(struct tlv *attr); /* TODO: mv this to radmsg? */ -pthread_attr_t pthread_attr; +extern pthread_attr_t pthread_attr; /* Local Variables: */ /* c-file-style: "stroustrup" */ From 87e71803bf8c1b27cd831bb0b2343b0cab785985 Mon Sep 17 00:00:00 2001 From: Sven Hartge Date: Sun, 5 Jul 2020 15:04:37 +0200 Subject: [PATCH 07/38] Fix spelling error detected by lintian --- radsecproxy.conf.5.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index bf1814d..b3a3ea8 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -539,7 +539,7 @@ options above. .BI "DynamicLookupCommand " command .RS -Execude the \fIcommand\fR to dynamically configure a server. The executable file +Execute the \fIcommand\fR to dynamically configure a server. The executable file should be given with full path and will be invoked with the name of the realm as its first and only argument. It should either print a valid \fBserver {...}\fR option on stdout and exit with a code of 0 or print nothing and exit with a From c1daf53d9bcff12619c55dd9538b42aadad1ff6c Mon Sep 17 00:00:00 2001 From: Robert Scheck Date: Sun, 2 Feb 2020 20:37:16 +0100 Subject: [PATCH 08/38] Declare pthread_attr as extern in header (fixes #63) GCC 10 compatibility as per https://gcc.gnu.org/gcc-10/porting_to.html --- radsecproxy.c | 1 + radsecproxy.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/radsecproxy.c b/radsecproxy.c index d5204db..1c575bc 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -84,6 +84,7 @@ extern int optind; extern char *optarg; #endif static const struct protodefs *protodefs[RAD_PROTOCOUNT]; +pthread_attr_t pthread_attr; /* minimum required declarations to avoid reordering code */ struct realm *adddynamicrealmserver(struct realm *realm, char *id); diff --git a/radsecproxy.h b/radsecproxy.h index 18a8702..d375a81 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -262,7 +262,7 @@ int radsrv(struct request *rq); void replyh(struct server *server, unsigned char *buf); struct addrinfo *resolve_hostport_addrinfo(uint8_t type, char *hostport); uint8_t *radattr2ascii(struct tlv *attr); /* TODO: mv this to radmsg? */ -pthread_attr_t pthread_attr; +extern pthread_attr_t pthread_attr; /* Local Variables: */ /* c-file-style: "stroustrup" */ From 91b6559d9747456a7db20c031d526465f47b96b7 Mon Sep 17 00:00:00 2001 From: Sven Hartge Date: Sun, 5 Jul 2020 15:04:37 +0200 Subject: [PATCH 09/38] Fix spelling error detected by lintian --- radsecproxy.conf.5.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 10d9a3f..d2f947e 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -533,7 +533,7 @@ default to 1812 while TLS and DTLS will default to 2083. .BI "DynamicLookupCommand " command .RS -Execude the \fIcommand\fR to dynamically configure a server. The executable file +Execute the \fIcommand\fR to dynamically configure a server. The executable file should be given with full path and will be invoked with the name of the realm as its first and only argument. It should either print a valid \fBserver {...}\fR option on stdout and exit with a code of 0 or print nothing and exit with a From 63405d3006c2c012c0b5e2540cab6aace682bc97 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 22 Jul 2020 15:35:57 +0200 Subject: [PATCH 10/38] update ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 03021f9..fc188a1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,8 @@ chanes since 1.8.1 Bug fixes: - Fix wrong config-unhexing if %25 (%) occurs + - Fix compatibility with GCC 10 (#63) + - Fix spelling in manpage 2019-10-01 1.8.1 Bug fixes: From 219fd23cb860837051f5273506c02298de8b09cd Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Thu, 23 Jul 2020 18:33:25 +0200 Subject: [PATCH 11/38] Fix modifyVendorAttribute not applied (#62) --- ChangeLog | 1 + rewrite.c | 6 ++-- tests/t_rewrite.c | 53 +++++++++++++++++++++++++++-- tests/t_rewrite_config.c | 73 +++++++++++++++++++++++++++++----------- 4 files changed, 107 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index fc188a1..8511116 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,7 @@ chanes since 1.8.1 - Fix wrong config-unhexing if %25 (%) occurs - Fix compatibility with GCC 10 (#63) - Fix spelling in manpage + - Fix modifyVendorAttribute not applied (#62) 2019-10-01 1.8.1 Bug fixes: diff --git a/rewrite.c b/rewrite.c index fa50f95..04dae00 100644 --- a/rewrite.c +++ b/rewrite.c @@ -147,7 +147,7 @@ struct modattr *extractmodvattr(char *nameval) { s = strchr(nameval, ':'); vendor = atoi(nameval); - if (!s || !vendor || !strchr(s,':')) + if (!s || !vendor || !strchr(s+1,':')) return NULL; modvattr = extractmodattr(s+1); if (modvattr) @@ -278,7 +278,7 @@ void addrewrite(char *value, uint8_t whitelist_mode, char **rmattrs, char **rmva freegconfmstr(supvattrs); } - if (rma || rmva || adda || moda || supa) { + if (rma || rmva || adda || moda || modva || supa) { rewrite = malloc(sizeof(struct rewrite)); if (!rewrite) debugx(1, DBG_ERR, "malloc failed"); @@ -499,7 +499,7 @@ int dorewritemodvattr(struct tlv *vendortlv, struct modattr *modvattr) { int dorewritemod(struct radmsg *msg, struct list *modattrs, struct list *modvattrs) { struct list_node *n, *m; uint32_t vendor; - + for (n = list_first(msg->attrs); n; n = list_next(n)) { struct tlv *attr = (struct tlv *)n->data; if (attr->t == RAD_Attr_Vendor_Specific) { diff --git a/tests/t_rewrite.c b/tests/t_rewrite.c index bbbe469..80d6bee 100644 --- a/tests/t_rewrite.c +++ b/tests/t_rewrite.c @@ -4,16 +4,26 @@ #include #include #include +#include #include "../rewrite.h" #include "../radmsg.h" #include "../debug.h" +static void printescape(uint8_t *v, uint8_t l) { + int i; + for(i=0; idata, (struct tlv *)m->data)) { - printf("attribute list not as expected\n"); + struct tlv *tlv_exp = (struct tlv *)n->data, *tlv_act = (struct tlv *)m->data; + if (!eqtlv(tlv_exp, tlv_act)) { + printf("attribute list at %d not as expected!\n", i); + printf(" expected type: %d, actual type: %d\n", tlv_exp->t, tlv_act->t); + printf(" expected length: %d, actual length: %d\n", tlv_exp->l, tlv_act->l); + printf(" expected value: "); + printescape(tlv_exp->v, tlv_exp->l); + printf(" actual value: "); + printescape(tlv_act->v, tlv_act->l); + printf("\n"); return 1; } m=list_next(m); + i++; } return 0; } @@ -65,7 +84,7 @@ void _reset_rewrite(struct rewrite *rewrite) { int main (int argc, char *argv[]) { - int testcount = 25; + int testcount = 26; struct list *origattrs, *expectedattrs; struct rewrite rewrite; char *username = "user@realm"; @@ -643,6 +662,34 @@ main (int argc, char *argv[]) _reset_rewrite(&rewrite); } + /* test issue #62 + rewrite 9:102:/^(h323-credit-time).*$/\1=86400/ + */ + { + char *value = "h323-credit-time=1846422"; + char *expect = "h323-credit-time=86400"; + struct modattr *mod = malloc(sizeof(struct modattr)); + regex_t regex; + + mod->t = 102; + mod->vendor = 9; + mod->regex = ®ex; + mod->replacement = "\\1=86400"; + regcomp(mod->regex, "^(h323-credit-time).*$", REG_ICASE | REG_EXTENDED); + + list_push(rewrite.modvattrs, mod); + list_push(origattrs, makevendortlv(9,maketlv(102,strlen(value), value))); + list_push(expectedattrs, makevendortlv(9,maketlv(102,strlen(expect), expect))); + + if (_check_rewrite(origattrs, &rewrite, expectedattrs, 0)) + printf("not "); + printf("ok %d - issue #62\n", testcount++); + + _tlv_list_clear(origattrs); + _tlv_list_clear(expectedattrs); + _reset_rewrite(&rewrite); + } + list_destroy(origattrs); list_destroy(expectedattrs); list_destroy(rewrite.addattrs); diff --git a/tests/t_rewrite_config.c b/tests/t_rewrite_config.c index 2e0ac89..129bbbd 100644 --- a/tests/t_rewrite_config.c +++ b/tests/t_rewrite_config.c @@ -15,40 +15,73 @@ main (int argc, char *argv[]) struct rewrite *result; char *rewritename = "rewrite"; char **addattrs; - int numtests = 1, i; + int numtests = 2, i; struct tlv *tlv, *expected; uint8_t expectedvalue[] = {'1',0,0,'1','A','%','4','1'}; printf("1..%d\n", numtests); numtests = 1; - addattrs = malloc(2); - addattrs[0] = stringcopy("1:'1%00%001%41%2541", 0); - addattrs[1] = NULL; + { + addattrs = malloc(2); + addattrs[0] = stringcopy("1:'1%00%001%41%2541", 0); + addattrs[1] = NULL; - expected = maketlv(1,8,expectedvalue); + expected = maketlv(1,8,expectedvalue); - addrewrite(rewritename, 0, NULL, NULL, addattrs, - NULL, NULL, NULL, NULL, NULL); + addrewrite(rewritename, 0, NULL, NULL, addattrs, + NULL, NULL, NULL, NULL, NULL); - result = getrewrite(rewritename, NULL); + result = getrewrite(rewritename, NULL); - if (result->addattrs->first) { - tlv = (struct tlv *)result->addattrs->first->data; - if (!eqtlv(tlv, expected)) { - printf ("tlv value was: 0x"); - for (i = 0; i < tlv->l; i++) { - printf ("%x", *((tlv->v)+i)); + if (result->addattrs->first) { + tlv = (struct tlv *)result->addattrs->first->data; + if (!eqtlv(tlv, expected)) { + printf ("tlv value was: 0x"); + for (i = 0; i < tlv->l; i++) { + printf ("%x", *((tlv->v)+i)); + } + printf ("\n"); + printf ("not "); } - printf ("\n"); - printf ("not "); + printf("ok %d - rewrite config\n", numtests++); + } else { + printf("not ok %d - rewrite config\n", numtests++); } - printf("ok %d - rewrite config\n", numtests++); - } else { - printf("not ok %d - rewrite ocnfig\n", numtests++); + + freetlv(expected); } - freetlv(expected); + /* test issue #62 */ + { + char *expectreplace = "\\1=86400"; + char **modvattrs = malloc(2); + rewritename= "issue62"; + + modvattrs[0] = stringcopy("9:102:/^(h323-credit-time).*$/\\1=86400/",0); + modvattrs[1] = NULL; + + addrewrite(rewritename, 0, NULL, NULL, NULL, NULL, NULL, modvattrs, NULL, NULL); + result = getrewrite(rewritename, NULL); + + if (result && result->modvattrs && result->modvattrs->first) { + struct modattr *mod = (struct modattr *)result->modvattrs->first->data; + if (regexec(mod->regex,"h323-credit-time=1846422",0,NULL,0)) { + printf("not "); + } + if (strcmp(mod->replacement, expectreplace)) { + printf("not "); + } + else if (mod->t != 102 || mod->vendor != 9) { + printf("not "); + } + printf("ok %d - rewrite config issue #62\n", numtests++); + } else { + printf("not ok %d - rewrite config issue #62\n", numtests++); + } + + free(modvattrs); + } return 0; } From f91c070829d82e2731835d66806e1630619de0c0 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Thu, 23 Jul 2020 18:33:25 +0200 Subject: [PATCH 12/38] Fix modifyVendorAttribute not applied (#62) --- rewrite.c | 6 ++-- tests/t_rewrite.c | 53 +++++++++++++++++++++++++++-- tests/t_rewrite_config.c | 73 +++++++++++++++++++++++++++++----------- 3 files changed, 106 insertions(+), 26 deletions(-) diff --git a/rewrite.c b/rewrite.c index fa50f95..04dae00 100644 --- a/rewrite.c +++ b/rewrite.c @@ -147,7 +147,7 @@ struct modattr *extractmodvattr(char *nameval) { s = strchr(nameval, ':'); vendor = atoi(nameval); - if (!s || !vendor || !strchr(s,':')) + if (!s || !vendor || !strchr(s+1,':')) return NULL; modvattr = extractmodattr(s+1); if (modvattr) @@ -278,7 +278,7 @@ void addrewrite(char *value, uint8_t whitelist_mode, char **rmattrs, char **rmva freegconfmstr(supvattrs); } - if (rma || rmva || adda || moda || supa) { + if (rma || rmva || adda || moda || modva || supa) { rewrite = malloc(sizeof(struct rewrite)); if (!rewrite) debugx(1, DBG_ERR, "malloc failed"); @@ -499,7 +499,7 @@ int dorewritemodvattr(struct tlv *vendortlv, struct modattr *modvattr) { int dorewritemod(struct radmsg *msg, struct list *modattrs, struct list *modvattrs) { struct list_node *n, *m; uint32_t vendor; - + for (n = list_first(msg->attrs); n; n = list_next(n)) { struct tlv *attr = (struct tlv *)n->data; if (attr->t == RAD_Attr_Vendor_Specific) { diff --git a/tests/t_rewrite.c b/tests/t_rewrite.c index bbbe469..80d6bee 100644 --- a/tests/t_rewrite.c +++ b/tests/t_rewrite.c @@ -4,16 +4,26 @@ #include #include #include +#include #include "../rewrite.h" #include "../radmsg.h" #include "../debug.h" +static void printescape(uint8_t *v, uint8_t l) { + int i; + for(i=0; idata, (struct tlv *)m->data)) { - printf("attribute list not as expected\n"); + struct tlv *tlv_exp = (struct tlv *)n->data, *tlv_act = (struct tlv *)m->data; + if (!eqtlv(tlv_exp, tlv_act)) { + printf("attribute list at %d not as expected!\n", i); + printf(" expected type: %d, actual type: %d\n", tlv_exp->t, tlv_act->t); + printf(" expected length: %d, actual length: %d\n", tlv_exp->l, tlv_act->l); + printf(" expected value: "); + printescape(tlv_exp->v, tlv_exp->l); + printf(" actual value: "); + printescape(tlv_act->v, tlv_act->l); + printf("\n"); return 1; } m=list_next(m); + i++; } return 0; } @@ -65,7 +84,7 @@ void _reset_rewrite(struct rewrite *rewrite) { int main (int argc, char *argv[]) { - int testcount = 25; + int testcount = 26; struct list *origattrs, *expectedattrs; struct rewrite rewrite; char *username = "user@realm"; @@ -643,6 +662,34 @@ main (int argc, char *argv[]) _reset_rewrite(&rewrite); } + /* test issue #62 + rewrite 9:102:/^(h323-credit-time).*$/\1=86400/ + */ + { + char *value = "h323-credit-time=1846422"; + char *expect = "h323-credit-time=86400"; + struct modattr *mod = malloc(sizeof(struct modattr)); + regex_t regex; + + mod->t = 102; + mod->vendor = 9; + mod->regex = ®ex; + mod->replacement = "\\1=86400"; + regcomp(mod->regex, "^(h323-credit-time).*$", REG_ICASE | REG_EXTENDED); + + list_push(rewrite.modvattrs, mod); + list_push(origattrs, makevendortlv(9,maketlv(102,strlen(value), value))); + list_push(expectedattrs, makevendortlv(9,maketlv(102,strlen(expect), expect))); + + if (_check_rewrite(origattrs, &rewrite, expectedattrs, 0)) + printf("not "); + printf("ok %d - issue #62\n", testcount++); + + _tlv_list_clear(origattrs); + _tlv_list_clear(expectedattrs); + _reset_rewrite(&rewrite); + } + list_destroy(origattrs); list_destroy(expectedattrs); list_destroy(rewrite.addattrs); diff --git a/tests/t_rewrite_config.c b/tests/t_rewrite_config.c index 2e0ac89..129bbbd 100644 --- a/tests/t_rewrite_config.c +++ b/tests/t_rewrite_config.c @@ -15,40 +15,73 @@ main (int argc, char *argv[]) struct rewrite *result; char *rewritename = "rewrite"; char **addattrs; - int numtests = 1, i; + int numtests = 2, i; struct tlv *tlv, *expected; uint8_t expectedvalue[] = {'1',0,0,'1','A','%','4','1'}; printf("1..%d\n", numtests); numtests = 1; - addattrs = malloc(2); - addattrs[0] = stringcopy("1:'1%00%001%41%2541", 0); - addattrs[1] = NULL; + { + addattrs = malloc(2); + addattrs[0] = stringcopy("1:'1%00%001%41%2541", 0); + addattrs[1] = NULL; - expected = maketlv(1,8,expectedvalue); + expected = maketlv(1,8,expectedvalue); - addrewrite(rewritename, 0, NULL, NULL, addattrs, - NULL, NULL, NULL, NULL, NULL); + addrewrite(rewritename, 0, NULL, NULL, addattrs, + NULL, NULL, NULL, NULL, NULL); - result = getrewrite(rewritename, NULL); + result = getrewrite(rewritename, NULL); - if (result->addattrs->first) { - tlv = (struct tlv *)result->addattrs->first->data; - if (!eqtlv(tlv, expected)) { - printf ("tlv value was: 0x"); - for (i = 0; i < tlv->l; i++) { - printf ("%x", *((tlv->v)+i)); + if (result->addattrs->first) { + tlv = (struct tlv *)result->addattrs->first->data; + if (!eqtlv(tlv, expected)) { + printf ("tlv value was: 0x"); + for (i = 0; i < tlv->l; i++) { + printf ("%x", *((tlv->v)+i)); + } + printf ("\n"); + printf ("not "); } - printf ("\n"); - printf ("not "); + printf("ok %d - rewrite config\n", numtests++); + } else { + printf("not ok %d - rewrite config\n", numtests++); } - printf("ok %d - rewrite config\n", numtests++); - } else { - printf("not ok %d - rewrite ocnfig\n", numtests++); + + freetlv(expected); } - freetlv(expected); + /* test issue #62 */ + { + char *expectreplace = "\\1=86400"; + char **modvattrs = malloc(2); + rewritename= "issue62"; + + modvattrs[0] = stringcopy("9:102:/^(h323-credit-time).*$/\\1=86400/",0); + modvattrs[1] = NULL; + + addrewrite(rewritename, 0, NULL, NULL, NULL, NULL, NULL, modvattrs, NULL, NULL); + result = getrewrite(rewritename, NULL); + + if (result && result->modvattrs && result->modvattrs->first) { + struct modattr *mod = (struct modattr *)result->modvattrs->first->data; + if (regexec(mod->regex,"h323-credit-time=1846422",0,NULL,0)) { + printf("not "); + } + if (strcmp(mod->replacement, expectreplace)) { + printf("not "); + } + else if (mod->t != 102 || mod->vendor != 9) { + printf("not "); + } + printf("ok %d - rewrite config issue #62\n", numtests++); + } else { + printf("not ok %d - rewrite config issue #62\n", numtests++); + } + + free(modvattrs); + } return 0; } From b00b958a38db82c2c480183113cdd2eea2182c73 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 31 Jul 2020 13:34:15 +0200 Subject: [PATCH 13/38] update Changelog --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 90332d4..949ba7e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,9 @@ chanes since 1.8.1 Bug fixes: - Fix wrong config-unhexing if %25 (%) occurs + - Fix compatibility with GCC 10 (#63) + - Fix spelling in manpage + - Fix modifyVendorAttribute not applied (#62) Misc: - Move radsecproxy manpage to section 8 From 55e22d505292e438cbfd7e3f1c5649efe610fed6 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 31 Jul 2020 15:43:09 +0200 Subject: [PATCH 14/38] fix coverity issues --- radmsg.c | 2 +- radsecproxy.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/radmsg.c b/radmsg.c index 6828f0d..5f49237 100644 --- a/radmsg.c +++ b/radmsg.c @@ -296,7 +296,7 @@ struct radmsg *buf2radmsg(uint8_t *buf, uint8_t *secret, int secret_len, uint8_t while (p - buf + 2 <= len) { t = *p++; l = *p++; - if (l < 2) { + if (l < 2 || l > 255) { debug(DBG_WARN, "buf2radmsg: invalid attribute length %d", l); radmsg_free(msg); return NULL; diff --git a/radsecproxy.c b/radsecproxy.c index 1c575bc..a8bb07a 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -3043,7 +3043,8 @@ int radsecproxy_main(int argc, char **argv) { sigaddset(&sigset, SIGHUP); sigaddset(&sigset, SIGPIPE); pthread_sigmask(SIG_BLOCK, &sigset, NULL); - pthread_create(&sigth, &pthread_attr, sighandler, NULL); + if (pthread_create(&sigth, &pthread_attr, sighandler, NULL)) + debugx(1, DBG_ERR, "pthread_create failed: sighandler"); for (entry = list_first(srvconfs); entry; entry = list_next(entry)) { srvconf = (struct clsrvconf *)entry->data; From da884aaaadc39bdafee7977498fd3c78dfbcaa1d Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 31 Jul 2020 15:43:09 +0200 Subject: [PATCH 15/38] fix coverity issues --- radmsg.c | 2 +- radsecproxy.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/radmsg.c b/radmsg.c index 6828f0d..5f49237 100644 --- a/radmsg.c +++ b/radmsg.c @@ -296,7 +296,7 @@ struct radmsg *buf2radmsg(uint8_t *buf, uint8_t *secret, int secret_len, uint8_t while (p - buf + 2 <= len) { t = *p++; l = *p++; - if (l < 2) { + if (l < 2 || l > 255) { debug(DBG_WARN, "buf2radmsg: invalid attribute length %d", l); radmsg_free(msg); return NULL; diff --git a/radsecproxy.c b/radsecproxy.c index b281e21..8d53d6e 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -3046,7 +3046,8 @@ int radsecproxy_main(int argc, char **argv) { sigaddset(&sigset, SIGHUP); sigaddset(&sigset, SIGPIPE); pthread_sigmask(SIG_BLOCK, &sigset, NULL); - pthread_create(&sigth, &pthread_attr, sighandler, NULL); + if (pthread_create(&sigth, &pthread_attr, sighandler, NULL)) + debugx(1, DBG_ERR, "pthread_create failed: sighandler"); for (entry = list_first(srvconfs); entry; entry = list_next(entry)) { srvconf = (struct clsrvconf *)entry->data; From 05fc57cc88e12d060ac74c94568f10d9ba9d3948 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Thu, 6 Aug 2020 13:39:10 +0200 Subject: [PATCH 16/38] don't send status-server when connection resets --- ChangeLog | 1 + radsecproxy.c | 9 ++++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8511116..f6c8159 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ chanes since 1.8.1 - Fix compatibility with GCC 10 (#63) - Fix spelling in manpage - Fix modifyVendorAttribute not applied (#62) + - Fix unncessary status-server when in minimal mode (#61) 2019-10-01 1.8.1 Bug fixes: diff --git a/radsecproxy.c b/radsecproxy.c index a8bb07a..8f7f2b6 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -1649,8 +1649,11 @@ void *clientwr(void *arg) { #endif pthread_mutex_unlock(&server->newrq_mutex); - for (i = 0; i < MAX_REQUESTS; i++) { - if (server->clientrdgone) { + if (do_resend || server->lastrcv.tv_sec > laststatsrv.tv_sec) + statusserver_requested = 0; + + for (i = 0; i < MAX_REQUESTS; i++) { + if (server->clientrdgone) { server->state = RSP_SERVER_STATE_FAILING; if (conf->pdef->connecter) pthread_join(clientrdth, NULL); @@ -1681,7 +1684,7 @@ void *clientwr(void *arg) { continue; } - if (rqout->tries > 0 && now.tv_sec - server->lastrcv.tv_sec > conf->retryinterval) + if (rqout->tries > 0 && now.tv_sec - server->lastrcv.tv_sec > conf->retryinterval && !do_resend) statusserver_requested = 1; if (rqout->tries == (*rqout->rq->buf == RAD_Status_Server ? 1 : conf->retrycount + 1)) { debug(DBG_DBG, "clientwr: removing expired packet from queue"); From 087f19407a64d353058f2faf7b51479340651cda Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Thu, 6 Aug 2020 13:58:50 +0200 Subject: [PATCH 17/38] ready for radsecproxy 1.8.2 --- ChangeLog | 2 +- README | 2 +- configure.ac | 2 +- radsecproxy.conf.5.in | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index f6c8159..af1b336 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -chanes since 1.8.1 +2020-08-06 1.8.2 Bug fixes: - Fix wrong config-unhexing if %25 (%) occurs - Fix compatibility with GCC 10 (#63) diff --git a/README b/README index 28dfdc2..8e74ab1 100644 --- a/README +++ b/README @@ -1,4 +1,4 @@ -This is radsecproxy 1.8.1 +This is radsecproxy 1.8.2 radsecproxy is a generic RADIUS proxy that supports both UDP and TLS (RadSec) RADIUS transports. There is also experimental support for diff --git a/configure.ac b/configure.ac index a5b8356..00feb06 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ dnl Copyright (c) 2006-2010, UNINETT AS dnl Copyright (c) 2010-2013,2016, NORDUnet A/S dnl See LICENSE for licensing information. -AC_INIT(radsecproxy, 1.8.1, https://radsecproxy.github.io) +AC_INIT(radsecproxy, 1.8.2, https://radsecproxy.github.io) AC_CONFIG_AUX_DIR([build-aux]) AC_CANONICAL_TARGET AM_INIT_AUTOMAKE diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index d2f947e..0fa6f47 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -1,4 +1,4 @@ -.TH radsecproxy.conf 5 2019-10-01 "radsecproxy 1.8.1" "" +.TH radsecproxy.conf 5 2020-08-06 "radsecproxy 1.8.2" "" .SH NAME radsecproxy.conf \- Radsec proxy configuration file From 319571e403f44e1fbe367e02e9e626c4bb25f5f0 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 10 Aug 2020 17:32:17 +0200 Subject: [PATCH 18/38] add user configurable cipher-list and ciphersuites --- ChangeLog | 1 + radsecproxy.conf.5.in | 13 +++++++ tlscommon.c | 84 ++++++++++++++++++++++++++----------------- tlscommon.h | 2 ++ 4 files changed, 67 insertions(+), 33 deletions(-) diff --git a/ChangeLog b/ChangeLog index 382ec28..b0bc1a3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,7 @@ unreleased chanes New features: - Accept multiple source* configs for IPv4/v6 - Specify source per server + - User configurable cipher-list and ciphersuites Misc: - Move radsecproxy manpage to section 8 diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index a2faa8f..1caef01 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -799,6 +799,19 @@ can be triggered by sending a SIGHUP to the radsecproxy process. This option may be set to zero to disable caching. .RE +.BI "CipherList " ciphers +.RS +Specify the list of accepted \fIciphers\fR. See +.BR openssl-ciphers (1). +.RE + +.BI "CipherSuites " ciphersuites +.RS +Specify the \fIciphersuites\fR to be used for TLS1.3. See +.BR openssl-ciphers (1). +.br +Note this requires openssl 1.1.1 + .SH "REWRITE BLOCK" .nf diff --git a/tlscommon.c b/tlscommon.c index 4522942..1cb7b42 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -348,35 +348,35 @@ static SSL_CTX *tlscreatectx(uint8_t type, struct tls *conf) { case RAD_TLS: #if OPENSSL_VERSION_NUMBER >= 0x10100000 /* TLS_method() was introduced in OpenSSL 1.1.0. */ - ctx = SSL_CTX_new(TLS_method()); + ctx = SSL_CTX_new(TLS_method()); #else /* No TLS_method(), use SSLv23_method() and disable SSLv2 and SSLv3. */ ctx = SSL_CTX_new(SSLv23_method()); SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); #endif #ifdef DEBUG - SSL_CTX_set_info_callback(ctx, ssl_info_callback); + SSL_CTX_set_info_callback(ctx, ssl_info_callback); #endif - break; + break; #endif #ifdef RADPROT_DTLS case RAD_DTLS: #if OPENSSL_VERSION_NUMBER >= 0x10002000 /* DTLS_method() seems to have been introduced in OpenSSL 1.0.2. */ - ctx = SSL_CTX_new(DTLS_method()); + ctx = SSL_CTX_new(DTLS_method()); #else - ctx = SSL_CTX_new(DTLSv1_method()); + ctx = SSL_CTX_new(DTLSv1_method()); #endif #ifdef DEBUG - SSL_CTX_set_info_callback(ctx, ssl_info_callback); + SSL_CTX_set_info_callback(ctx, ssl_info_callback); #endif - SSL_CTX_set_read_ahead(ctx, 1); - break; + SSL_CTX_set_read_ahead(ctx, 1); + break; #endif } if (!ctx) { - debug(DBG_ERR, "tlscreatectx: Error initialising SSL/TLS in TLS context %s", conf->name); - return NULL; + debug(DBG_ERR, "tlscreatectx: Error initialising SSL/TLS in TLS context %s", conf->name); + return NULL; } #if OPENSSL_VERSION_NUMBER < 0x10100000L @@ -394,39 +394,55 @@ static SSL_CTX *tlscreatectx(uint8_t type, struct tls *conf) { #endif if (conf->certkeypwd) { - SSL_CTX_set_default_passwd_cb_userdata(ctx, conf->certkeypwd); - SSL_CTX_set_default_passwd_cb(ctx, pem_passwd_cb); + SSL_CTX_set_default_passwd_cb_userdata(ctx, conf->certkeypwd); + SSL_CTX_set_default_passwd_cb(ctx, pem_passwd_cb); } if (!SSL_CTX_use_certificate_chain_file(ctx, conf->certfile) || - !SSL_CTX_use_PrivateKey_file(ctx, conf->certkeyfile, SSL_FILETYPE_PEM) || - !SSL_CTX_check_private_key(ctx)) { - while ((error = ERR_get_error())) - debug(DBG_ERR, "SSL: %s", ERR_error_string(error, NULL)); - debug(DBG_ERR, "tlscreatectx: Error initialising SSL/TLS in TLS context %s", conf->name); - SSL_CTX_free(ctx); - return NULL; + !SSL_CTX_use_PrivateKey_file(ctx, conf->certkeyfile, SSL_FILETYPE_PEM) || + !SSL_CTX_check_private_key(ctx)) { + while ((error = ERR_get_error())) + debug(DBG_ERR, "SSL: %s", ERR_error_string(error, NULL)); + debug(DBG_ERR, "tlscreatectx: Error initialising SSL/TLS in TLS context %s", conf->name); + SSL_CTX_free(ctx); + return NULL; } if (conf->policyoids) { - if (!conf->vpm) { - conf->vpm = createverifyparams(conf->policyoids); - if (!conf->vpm) { - debug(DBG_ERR, "tlscreatectx: Failed to add policyOIDs in TLS context %s", conf->name); - SSL_CTX_free(ctx); - return NULL; - } - } + if (!conf->vpm) { + conf->vpm = createverifyparams(conf->policyoids); + if (!conf->vpm) { + debug(DBG_ERR, "tlscreatectx: Failed to add policyOIDs in TLS context %s", conf->name); + SSL_CTX_free(ctx); + return NULL; + } + } } if (!tlsaddcacrl(ctx, conf)) { - if (conf->vpm) { - X509_VERIFY_PARAM_free(conf->vpm); - conf->vpm = NULL; - } - SSL_CTX_free(ctx); - return NULL; + if (conf->vpm) { + X509_VERIFY_PARAM_free(conf->vpm); + conf->vpm = NULL; + } + SSL_CTX_free(ctx); + return NULL; } + if (conf->cipherlist) { + if (!SSL_CTX_set_cipher_list(ctx, conf->cipherlist)) { + debug(DBG_ERR, "tlscreatectx: Failed to set cipher list in TLS context %s", conf->name); + SSL_CTX_free(ctx); + return NULL; + } + } +#if OPENSSL_VERSION_NUMBER >= 0x10101000 + if (conf->ciphersuites) { + if (!SSL_CTX_set_ciphersuites(ctx, conf->ciphersuites)) { + debug(DBG_ERR, "tlscreatectx: Failed to set ciphersuites in TLS context %s", conf->name); + SSL_CTX_free(ctx); + return NULL; + } + } +#endif debug(DBG_DBG, "tlscreatectx: created TLS context %s", conf->name); return ctx; } @@ -775,6 +791,8 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v "CacheExpiry", CONF_LINT, &expiry, "CRLCheck", CONF_BLN, &conf->crlcheck, "PolicyOID", CONF_MSTR, &conf->policyoids, + "CipherList", CONF_STR, &conf->cipherlist, + "CipherSuites", CONF_STR, &conf->ciphersuites, NULL )) { debug(DBG_ERR, "conftls_cb: configuration error in block %s", val); diff --git a/tlscommon.h b/tlscommon.h index 1670136..721d5be 100644 --- a/tlscommon.h +++ b/tlscommon.h @@ -18,6 +18,8 @@ struct tls { char *certkeypwd; uint8_t crlcheck; char **policyoids; + char *cipherlist; + char *ciphersuites; uint32_t cacheexpiry; uint32_t tlsexpiry; uint32_t dtlsexpiry; From f8b449e5e56db20b6d1ff457483bbaac7613deda Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 11 Aug 2020 11:53:08 +0200 Subject: [PATCH 19/38] add user configurable tls versions --- ChangeLog | 1 + radsecproxy.conf.5.in | 20 ++++++++++++ tlscommon.c | 75 +++++++++++++++++++++++++++++++++++++++++++ tlscommon.h | 4 +++ 4 files changed, 100 insertions(+) diff --git a/ChangeLog b/ChangeLog index b0bc1a3..df04a89 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,7 @@ unreleased chanes - Accept multiple source* configs for IPv4/v6 - Specify source per server - User configurable cipher-list and ciphersuites + - User configurable TLS versions Misc: - Move radsecproxy manpage to section 8 diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 1caef01..a2ce537 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -811,6 +811,26 @@ Specify the \fIciphersuites\fR to be used for TLS1.3. See .BR openssl-ciphers (1). .br Note this requires openssl 1.1.1 +.RE + +.BR "TlsVersion " ( +.IR version " | " minversion : maxversion " )" +.br +.BR "DtlsVersion " ( +.IR version " | " minversion : maxversion " )" +.RS +Specify the TLS/DTLS protocol \fIversion\fR to be used. +.br +Specify the range of allowed protocol versions between \fIminversion\fR and +\fImaxversion\fR (inclusive). If either is left out, any version up to, or +starting from this version is allowed. E.g. "TLS1_2:" will allow TLSv1.2 or later. +.br +Currently supported values are +.BR SSL3 , TLS1 , TLS1_1 , TLS1_2 , TLS1_3 +for TLS and +.BR DTLS1 , DTLS1_1 +for DTLS. + .SH "REWRITE BLOCK" diff --git a/tlscommon.c b/tlscommon.c index 1cb7b42..7bec744 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -349,6 +349,8 @@ static SSL_CTX *tlscreatectx(uint8_t type, struct tls *conf) { #if OPENSSL_VERSION_NUMBER >= 0x10100000 /* TLS_method() was introduced in OpenSSL 1.1.0. */ ctx = SSL_CTX_new(TLS_method()); + SSL_CTX_set_min_proto_version(ctx, conf->tlsminversion); + SSL_CTX_set_max_proto_version(ctx, conf->tlsmaxversion); #else /* No TLS_method(), use SSLv23_method() and disable SSLv2 and SSLv3. */ ctx = SSL_CTX_new(SSLv23_method()); @@ -364,6 +366,10 @@ static SSL_CTX *tlscreatectx(uint8_t type, struct tls *conf) { #if OPENSSL_VERSION_NUMBER >= 0x10002000 /* DTLS_method() seems to have been introduced in OpenSSL 1.0.2. */ ctx = SSL_CTX_new(DTLS_method()); +#if OPENSSL_VERSION_NUMBER >= 0x10100000 + SSL_CTX_set_min_proto_version(ctx, conf->dtlsminversion); + SSL_CTX_set_max_proto_version(ctx, conf->dtlsmaxversion); +#endif #else ctx = SSL_CTX_new(DTLSv1_method()); #endif @@ -769,9 +775,54 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { return ok; } +#if OPENSSL_VERSION_NUMBER >= 0x10100000 +static int parse_tls_version(const char* version) { + if (!strcasecmp("SSL3", version)) { + return SSL3_VERSION; + } else if (!strcasecmp("TLS1", version)) { + return TLS1_VERSION; + } else if (!strcasecmp("TLS1_1", version)) { + return TLS1_1_VERSION; + } else if (!strcasecmp("TLS1_2", version)) { + return TLS1_2_VERSION; +#if OPENSSL_VERSION_NUMBER >= 0x10101000 + } else if (!strcasecmp("TLS1_3", version)) { + return TLS1_3_VERSION; +#endif + } else if (!strcasecmp("DTLS1", version)) { + return DTLS1_VERSION; + } else if (!strcasecmp("DTLS1_2", version)) { + return DTLS1_2_VERSION; + } else if (!strcasecmp("", version)) { + return 0; + } else { + return -1; + } +} + +static int conf_tls_version(const char *version, int *min, int *max) { + char *ver, *s, *smin, *smax; + ver = stringcopy(version, strlen(version)); + s = strchr(ver, ':'); + if (!s) { + smin = smax = ver; + } else { + *s = '\0'; + smin = ver; + smax = s+1; + } + *min = parse_tls_version(smin); + *max = parse_tls_version(smax); + free(ver); + return *min >=0 && *max >=0 && (*max == 0 || *min <= *max); +} +#endif + int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *val) { struct tls *conf; long int expiry = LONG_MIN; + char *tlsversion = NULL; + char *dtlsversion = NULL; debug(DBG_DBG, "conftls_cb called for %s", block); @@ -793,6 +844,8 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v "PolicyOID", CONF_MSTR, &conf->policyoids, "CipherList", CONF_STR, &conf->cipherlist, "CipherSuites", CONF_STR, &conf->ciphersuites, + "TlsVersion", CONF_STR, &tlsversion, + "DtlsVersion", CONF_STR, &dtlsversion, NULL )) { debug(DBG_ERR, "conftls_cb: configuration error in block %s", val); @@ -813,6 +866,26 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v } conf->cacheexpiry = expiry; } +#if OPENSSL_VERSION_NUMBER >= 0x10100000 + conf->tlsminversion = TLS1_1_VERSION; + if (tlsversion) { + if(!conf_tls_version(tlsversion, &conf->tlsminversion, &conf->tlsmaxversion)) { + debug(DBG_ERR, "error in block %s, invalid TlsVersion %s", val, tlsversion); + goto errexit; + } + free (tlsversion); + } + if (dtlsversion) { + if(!conf_tls_version(dtlsversion, &conf->dtlsminversion, &conf->dtlsmaxversion)) { + debug(DBG_ERR, "error in block %s, invalid DtlsVersion %s", val, dtlsversion); + goto errexit; + } + free (dtlsversion); + } +#else + debug(DBG_ERR, "error in block %s, setting tls/dtls version requires openssl 1.1.0 or later", val); + goto errexit; +#endif conf->name = stringcopy(val, 0); if (!conf->name) { @@ -839,6 +912,8 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v free(conf->certkeyfile); free(conf->certkeypwd); freegconfmstr(conf->policyoids); + free(tlsversion); + free(dtlsversion); free(conf); return 0; } diff --git a/tlscommon.h b/tlscommon.h index 721d5be..5e0562d 100644 --- a/tlscommon.h +++ b/tlscommon.h @@ -21,6 +21,10 @@ struct tls { char *cipherlist; char *ciphersuites; uint32_t cacheexpiry; + int tlsminversion; + int tlsmaxversion; + int dtlsminversion; + int dtlsmaxversion; uint32_t tlsexpiry; uint32_t dtlsexpiry; X509_VERIFY_PARAM *vpm; From 440e3b8b0ea14bda696ff29b1775a2c55ca41177 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 11 Aug 2020 16:28:55 +0200 Subject: [PATCH 20/38] fix memory cleanup in conftls_cb --- tlscommon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tlscommon.c b/tlscommon.c index 7bec744..618b6a2 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -874,6 +874,7 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v goto errexit; } free (tlsversion); + tlsversion = NULL; } if (dtlsversion) { if(!conf_tls_version(dtlsversion, &conf->dtlsminversion, &conf->dtlsmaxversion)) { @@ -881,6 +882,7 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v goto errexit; } free (dtlsversion); + dtlsversion = NULL; } #else debug(DBG_ERR, "error in block %s, setting tls/dtls version requires openssl 1.1.0 or later", val); From a26f42ea7b20efcb7135a21e3ca2bca18f704549 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 11 Aug 2020 16:38:15 +0200 Subject: [PATCH 21/38] add config for DH-file --- ChangeLog | 1 + radsecproxy.conf.5.in | 4 ++++ tlscommon.c | 32 ++++++++++++++++++++++++++++++++ tlscommon.h | 1 + 4 files changed, 38 insertions(+) diff --git a/ChangeLog b/ChangeLog index df04a89..620b6b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ unreleased chanes - Specify source per server - User configurable cipher-list and ciphersuites - User configurable TLS versions + - Config option for DH-file Misc: - Move radsecproxy manpage to section 8 diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index a2ce537..febc21d 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -830,7 +830,11 @@ Currently supported values are for TLS and .BR DTLS1 , DTLS1_1 for DTLS. +.RE +.BI "DhFile " file +.RS +DH parameter \fIfile\fR to use. See \fBopenssl-dhparam\fR(1) .SH "REWRITE BLOCK" diff --git a/tlscommon.c b/tlscommon.c index 618b6a2..cbfe12c 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -449,6 +449,14 @@ static SSL_CTX *tlscreatectx(uint8_t type, struct tls *conf) { } } #endif + + if (conf->dhparam) { + if (!SSL_CTX_set_tmp_dh(ctx, conf->dhparam)) { + while ((error = ERR_get_error())) + debug(DBG_WARN, "tlscreatectx: SSL: %s", ERR_error_string(error, NULL)); + debug(DBG_WARN, "tlscreatectx: Failed to set dh params. Can continue, but some ciphers might not be available."); + } + } debug(DBG_DBG, "tlscreatectx: created TLS context %s", conf->name); return ctx; } @@ -823,6 +831,8 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v long int expiry = LONG_MIN; char *tlsversion = NULL; char *dtlsversion = NULL; + char *dhfile = NULL; + unsigned long error; debug(DBG_DBG, "conftls_cb called for %s", block); @@ -846,6 +856,7 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v "CipherSuites", CONF_STR, &conf->ciphersuites, "TlsVersion", CONF_STR, &tlsversion, "DtlsVersion", CONF_STR, &dtlsversion, + "DhFile", CONF_STR, &dhfile, NULL )) { debug(DBG_ERR, "conftls_cb: configuration error in block %s", val); @@ -889,6 +900,25 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v goto errexit; #endif + if (dhfile) { + FILE *dhfp = fopen(dhfile, "r"); + if (dhfp) { + conf->dhparam = PEM_read_DHparams(dhfp, NULL, NULL, NULL); + fclose(dhfp); + if (!conf->dhparam) { + while ((error = ERR_get_error())) + debug(DBG_ERR, "SSL: %s", ERR_error_string(error, NULL)); + debug(DBG_ERR, "error in block %s: Failed to load DhFile %s.", val, dhfile); + goto errexit; + } + } else { + debug(DBG_ERR, "error in block %s, DhFile: can't open file %s", val, dhfile); + goto errexit; + } + free(dhfile); + dhfile = NULL; + } + conf->name = stringcopy(val, 0); if (!conf->name) { debug(DBG_ERR, "conftls_cb: malloc failed"); @@ -916,6 +946,8 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v freegconfmstr(conf->policyoids); free(tlsversion); free(dtlsversion); + free(dhfile); + DH_free(conf->dhparam); free(conf); return 0; } diff --git a/tlscommon.h b/tlscommon.h index 5e0562d..3cccf65 100644 --- a/tlscommon.h +++ b/tlscommon.h @@ -25,6 +25,7 @@ struct tls { int tlsmaxversion; int dtlsminversion; int dtlsmaxversion; + DH *dhparam; uint32_t tlsexpiry; uint32_t dtlsexpiry; X509_VERIFY_PARAM *vpm; From 1bcc63ac24b90fbaf8b59f00cf05eaa74076735a Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 4 Sep 2020 20:44:34 +0200 Subject: [PATCH 22/38] add subjetaltname rID and otherName --- radsecproxy.h | 4 ++ tlscommon.c | 110 +++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 103 insertions(+), 11 deletions(-) diff --git a/radsecproxy.h b/radsecproxy.h index 0f20f50..e4abea5 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -153,7 +153,11 @@ struct clsrvconf { regex_t *certcnregex; regex_t *certuriregex; regex_t *certdnsregex; + regex_t *certotherregex; + char *certothertype; struct in6_addr certipmatch; + //ASN1_OBJECT certridmatch; + char *certridmatch; int certipmatchaf; char *confrewritein; char *confrewriteout; diff --git a/tlscommon.c b/tlscommon.c index cbfe12c..630c1f9 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -557,6 +557,52 @@ X509 *verifytlscert(SSL *ssl) { return cert; } +static STACK_OF(GENERAL_NAME) *getaltname(X509 *cert) { + STACK_OF(GENERAL_NAME) *altname; + int loc; + + loc = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); + if (loc < 0) + return NULL; + + altname = X509V3_EXT_d2i(X509_get_ext(cert, loc)); + if (!altname) + return NULL; + + return altname; +} + +static int subjectaltnamerid(X509 *cert, const char *oid) { + ASN1_OBJECT *asn1oid; + STACK_OF(GENERAL_NAME) *alt = NULL; + GENERAL_NAME *gn; + int n, i, r=0; + + asn1oid = OBJ_txt2obj(oid, 0); + if (!asn1oid) + return r; + + alt = getaltname(cert); + if (!alt) + goto exit; + + n = sk_GENERAL_NAME_num(alt); + for (i = 0; i < n; i++) { + gn = sk_GENERAL_NAME_value(alt, i); + if (gn->type != GEN_RID) + continue; + if (OBJ_cmp(gn->d.registeredID, asn1oid) == 0){ + r = 1; + break; + } + } + +exit: + GENERAL_NAMES_free(alt); + ASN1_OBJECT_free(asn1oid); + return r; +} + static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { int loc, i, l, n, r = 0; char *v; @@ -581,8 +627,8 @@ static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { if (gn->type != GEN_IPADD) continue; r = -1; - v = (char *)ASN1_STRING_get0_data(gn->d.ia5); - l = ASN1_STRING_length(gn->d.ia5); + v = (char *)ASN1_STRING_get0_data(gn->d.iPAddress); + l = ASN1_STRING_length(gn->d.iPAddress); if (((family == AF_INET && l == sizeof(struct in_addr)) || (family == AF_INET6 && l == sizeof(struct in6_addr))) && !memcmp(v, addr, l)) { r = 1; @@ -593,12 +639,13 @@ static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { return r; } -static int subjectaltnameregexp(X509 *cert, int type, char *exact, regex_t *regex) { +static int subjectaltnameregexp(X509 *cert, int type, char *othernametype, char *exact, regex_t *regex) { int loc, i, l, n, r = 0; char *s, *v, *fail = NULL, *tmp; X509_EXTENSION *ex; STACK_OF(GENERAL_NAME) *alt; GENERAL_NAME *gn; + ASN1_OBJECT *othernameoid; debug(DBG_DBG, "subjectaltnameregexp"); @@ -611,16 +658,30 @@ static int subjectaltnameregexp(X509 *cert, int type, char *exact, regex_t *reg if (!alt) return r; + if (othernametype) { + othernameoid = OBJ_txt2obj(othernametype, 0); + if (!othernameoid) { + goto exit; + } + } + n = sk_GENERAL_NAME_num(alt); for (i = 0; i < n; i++) { gn = sk_GENERAL_NAME_value(alt, i); if (gn->type != type) continue; r = -1; - v = (char *)ASN1_STRING_get0_data(gn->d.ia5); - l = ASN1_STRING_length(gn->d.ia5); - if (l <= 0) - continue; + if (gn->type == GEN_OTHERNAME) { + if (OBJ_cmp(gn->d.otherName->type_id, othernameoid) != 0) + continue; + v = (char *)ASN1_STRING_get0_data(gn->d.otherName->value->value.octet_string); + l = ASN1_STRING_length(gn->d.otherName->value->value.octet_string); + } else { + v = (char *)ASN1_STRING_get0_data(gn->d.ia5); + l = ASN1_STRING_length(gn->d.ia5); + } + if (l <= 0) + continue; #ifdef DEBUG printfchars(NULL, gn->type == GEN_DNS ? "dns" : "uri", NULL, (uint8_t *) v, l); #endif @@ -650,7 +711,10 @@ static int subjectaltnameregexp(X509 *cert, int type, char *exact, regex_t *reg } if (r!=1) debug(DBG_WARN, "subjectaltnameregex: no matching Subject Alt Name %s found! (%s)", - type == GEN_DNS ? "DNS" : "URI", fail); + type == GEN_DNS ? "DNS" : type == GEN_URI ? "URI" : type == GEN_OTHERNAME ? "OtherName" : "", fail); + +exit: + ASN1_OBJECT_free(othernameoid); GENERAL_NAMES_free(alt); free(fail); return r; @@ -719,7 +783,7 @@ int certnamecheck(X509 *cert, struct list *hostports) { if (type) r = subjectaltnameaddr(cert, type, &addr); if (!r) - r = subjectaltnameregexp(cert, GEN_DNS, hp->host, NULL); + r = subjectaltnameregexp(cert, GEN_DNS, NULL, hp->host, NULL); if (r) { if (r > 0) { debug(DBG_DBG, "certnamecheck: Found subjectaltname matching %s %s", type ? "address" : "host", hp->host); @@ -760,14 +824,21 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { } if (conf->certuriregex) { debug(DBG_DBG, "verifyconfcert: matching subjectaltname URI regex %s", conf->matchcertattr); - if (subjectaltnameregexp(cert, GEN_URI, NULL, conf->certuriregex) < 1) { + if (subjectaltnameregexp(cert, GEN_URI, NULL, NULL, conf->certuriregex) < 1) { debug(DBG_WARN, "verifyconfcert: subjectaltname URI not matching regex for host %s (%s)", conf->name, subject); 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) { + if (subjectaltnameregexp(cert, GEN_DNS, NULL, NULL, conf->certdnsregex) < 1) { + debug(DBG_WARN, "verifyconfcert: subjectaltname DNS not matching regex for host %s (%s)", conf->name, subject); + ok = 0; + } + } + if (conf->certotherregex && conf->certothertype) { + debug(DBG_DBG, "verifyconfcert: matching subjectaltname OtherName regex %s", conf->matchcertattr); + if (subjectaltnameregexp(cert, GEN_OTHERNAME, conf->certothertype, NULL, conf->certotherregex) < 1) { debug(DBG_WARN, "verifyconfcert: subjectaltname DNS not matching regex for host %s (%s)", conf->name, subject); ok = 0; } @@ -779,6 +850,13 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { ok = 0; } } + if (conf->certridmatch) { + debug(DBG_DBG, "verfiyconfcert: matching subjectlatname rID %s", conf->certridmatch); + if (subjectaltnamerid(cert, conf->certridmatch) < 1) { + debug(DBG_WARN, "verifyconfcert: subjectaltname rID not matching for host %s (%s)", conf->name, subject); + ok = 0; + } + } free(subject); return ok; } @@ -966,6 +1044,10 @@ int addmatchcertattr(struct clsrvconf *conf) { return 1; } + if (!strncasecmp(conf->matchcertattr, "SubjectAltName:rID:", 19)) { + conf->certridmatch = conf->matchcertattr+19; + } + /* the other cases below use a common regex match */ if (!strncasecmp(conf->matchcertattr, "CN:/", 4)) { r = &conf->certcnregex; @@ -976,6 +1058,12 @@ int addmatchcertattr(struct clsrvconf *conf) { } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:DNS:/", 20)) { r = &conf->certdnsregex; v = conf->matchcertattr + 20; + } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:OtherName:", 25)) { + v = strchr(conf->matchcertattr + 25, ':'); + if (!v || +1 != '/') return 0; + conf->certothertype = stringcopy(conf->matchcertattr+25, conf->matchcertattr+25 - v - 1); + v = v+1; + r = &conf->certotherregex; } else return 0; From 946cef330edaa0b9ecc86734885afe8fca2e28c3 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 4 Sep 2020 20:51:22 +0200 Subject: [PATCH 23/38] extract getaltname --- tlscommon.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/tlscommon.c b/tlscommon.c index 630c1f9..46a6037 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -604,20 +604,14 @@ static int subjectaltnamerid(X509 *cert, const char *oid) { } static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { - int loc, i, l, n, r = 0; + int i, l, n, r = 0; char *v; - X509_EXTENSION *ex; STACK_OF(GENERAL_NAME) *alt; GENERAL_NAME *gn; debug(DBG_DBG, "subjectaltnameaddr"); - loc = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); - if (loc < 0) - return r; - - ex = X509_get_ext(cert, loc); - alt = X509V3_EXT_d2i(ex); + alt = getaltname(cert); if (!alt) return r; @@ -640,21 +634,15 @@ static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { } static int subjectaltnameregexp(X509 *cert, int type, char *othernametype, char *exact, regex_t *regex) { - int loc, i, l, n, r = 0; + int i, l, n, r = 0; char *s, *v, *fail = NULL, *tmp; - X509_EXTENSION *ex; STACK_OF(GENERAL_NAME) *alt; GENERAL_NAME *gn; ASN1_OBJECT *othernameoid; debug(DBG_DBG, "subjectaltnameregexp"); - loc = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); - if (loc < 0) - return r; - - ex = X509_get_ext(cert, loc); - alt = X509V3_EXT_d2i(ex); + alt = getaltname(cert); if (!alt) return r; From f9a0366dcc248ce094c07c106c1bf86af701c2b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rn=20=C3=85ne=20de=20Jong?= Date: Mon, 14 Sep 2020 16:30:04 +0200 Subject: [PATCH 24/38] s/WhitelistAttrbiute/WhitelistAttribute/ --- radsecproxy.conf.5.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index febc21d..88e9178 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -936,7 +936,7 @@ the given vendor id are removed. .BR "WhitelistMode (" on | off ) .RS Enable whitelist mode. All attributes except those configured with -\fBWhitelistAttrbiute\fR or \fBWhitelistVendorAttribute\fR will be removed. +\fBWhitelistAttribute\fR or \fBWhitelistVendorAttribute\fR will be removed. While whitelist mode is active, \fBRemoveAttribute\fR and \fBRemoveVendorAttribute\fR statements are ignored. .RE From e05418f29d642989c873cf5518c28eeb71dd112f Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 15 Sep 2020 07:31:54 +0200 Subject: [PATCH 25/38] implement matchsan as closure --- radsecproxy.h | 8 ++++-- tlscommon.c | 80 ++++++++++++++++++++++++++++++++++----------------- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/radsecproxy.h b/radsecproxy.h index e4abea5..276d925 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -12,6 +12,8 @@ #include "gconfig.h" #include "rewrite.h" +#include + #define DEBUG_LEVEL 2 #define CONFIG_MAIN SYSCONFDIR"/radsecproxy.conf" @@ -154,10 +156,10 @@ struct clsrvconf { regex_t *certuriregex; regex_t *certdnsregex; regex_t *certotherregex; - char *certothertype; + ASN1_OBJECT *certothertype; struct in6_addr certipmatch; - //ASN1_OBJECT certridmatch; - char *certridmatch; + ASN1_OBJECT *certridmatch; + //char *certridmatch; int certipmatchaf; char *confrewritein; char *confrewriteout; diff --git a/tlscommon.c b/tlscommon.c index 46a6037..efd2f3d 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -572,36 +572,66 @@ static STACK_OF(GENERAL_NAME) *getaltname(X509 *cert) { return altname; } -static int subjectaltnamerid(X509 *cert, const char *oid) { - ASN1_OBJECT *asn1oid; +static int matchsubjaltname(X509 *cert, int type, int (*matchfn)(GENERAL_NAME *, void *), void *arg ) { + GENERAL_NAME *gn; + int n,i,r = 0; + STACK_OF(GENERAL_NAME) *alt; + + alt = getaltname(cert); + + n = sk_GENERAL_NAME_num(alt); + for (i = 0; i < n; i++) { + gn = sk_GENERAL_NAME_value(alt, i); + if (gn->type == type) { + r = matchfn(gn, arg); + if (r) + break; + } + } + + GENERAL_NAMES_free(alt); + return r; +} + +static int matchsanrid(GENERAL_NAME *gn, void *rid) { + return OBJ_cmp(gn->d.registeredID, (ASN1_OBJECT *)rid) == 0 ? 1 : 0; +} + +struct matchsanip_arg { + int family; + struct in6_addr *addr; +}; +static int matchsanip(GENERAL_NAME *gn, void *arg){ + struct matchsanip_arg *addr = (struct matchsanip_arg *)arg; + int l = ASN1_STRING_length(gn->d.iPAddress); + return (((addr->family == AF_INET && l == sizeof(struct in_addr)) || (addr->family == AF_INET6 && l == sizeof(struct in6_addr))) + && !memcmp(ASN1_STRING_get0_data(gn->d.iPAddress), addr->addr, l)) ? 1 : 0 ; +} + +/* +static int subjectaltnamerid(X509 *cert, ASN1_OBJECT *rid) { STACK_OF(GENERAL_NAME) *alt = NULL; GENERAL_NAME *gn; int n, i, r=0; - asn1oid = OBJ_txt2obj(oid, 0); - if (!asn1oid) - return r; - alt = getaltname(cert); if (!alt) - goto exit; + return r; n = sk_GENERAL_NAME_num(alt); for (i = 0; i < n; i++) { gn = sk_GENERAL_NAME_value(alt, i); if (gn->type != GEN_RID) continue; - if (OBJ_cmp(gn->d.registeredID, asn1oid) == 0){ + if (OBJ_cmp(gn->d.registeredID, rid) == 0){ r = 1; break; } } -exit: GENERAL_NAMES_free(alt); - ASN1_OBJECT_free(asn1oid); return r; -} +}*/ static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { int i, l, n, r = 0; @@ -633,12 +663,11 @@ static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { return r; } -static int subjectaltnameregexp(X509 *cert, int type, char *othernametype, char *exact, regex_t *regex) { +static int subjectaltnameregexp(X509 *cert, int type, ASN1_OBJECT *othernametype, char *exact, regex_t *regex) { int i, l, n, r = 0; char *s, *v, *fail = NULL, *tmp; STACK_OF(GENERAL_NAME) *alt; GENERAL_NAME *gn; - ASN1_OBJECT *othernameoid; debug(DBG_DBG, "subjectaltnameregexp"); @@ -646,13 +675,6 @@ static int subjectaltnameregexp(X509 *cert, int type, char *othernametype, char if (!alt) return r; - if (othernametype) { - othernameoid = OBJ_txt2obj(othernametype, 0); - if (!othernameoid) { - goto exit; - } - } - n = sk_GENERAL_NAME_num(alt); for (i = 0; i < n; i++) { gn = sk_GENERAL_NAME_value(alt, i); @@ -660,7 +682,7 @@ static int subjectaltnameregexp(X509 *cert, int type, char *othernametype, char continue; r = -1; if (gn->type == GEN_OTHERNAME) { - if (OBJ_cmp(gn->d.otherName->type_id, othernameoid) != 0) + if (OBJ_cmp(gn->d.otherName->type_id, othernametype) != 0) continue; v = (char *)ASN1_STRING_get0_data(gn->d.otherName->value->value.octet_string); l = ASN1_STRING_length(gn->d.otherName->value->value.octet_string); @@ -701,8 +723,6 @@ static int subjectaltnameregexp(X509 *cert, int type, char *othernametype, char debug(DBG_WARN, "subjectaltnameregex: no matching Subject Alt Name %s found! (%s)", type == GEN_DNS ? "DNS" : type == GEN_URI ? "URI" : type == GEN_OTHERNAME ? "OtherName" : "", fail); -exit: - ASN1_OBJECT_free(othernameoid); GENERAL_NAMES_free(alt); free(fail); return r; @@ -832,15 +852,16 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { } } if (conf->certipmatchaf) { + struct matchsanip_arg arg = {conf->certipmatchaf, &conf->certipmatch}; 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) { + if (matchsubjaltname(cert, GEN_IPADD, &matchsanip, &arg)) { debug(DBG_WARN, "verifyconfcert: subjectaltname IP not matching regex for host %s (%s)", conf->name, subject); ok = 0; } } if (conf->certridmatch) { debug(DBG_DBG, "verfiyconfcert: matching subjectlatname rID %s", conf->certridmatch); - if (subjectaltnamerid(cert, conf->certridmatch) < 1) { + if (matchsubjaltname(cert, GEN_RID, &matchsanrid, conf->certridmatch)) { debug(DBG_WARN, "verifyconfcert: subjectaltname rID not matching for host %s (%s)", conf->name, subject); ok = 0; } @@ -1033,7 +1054,9 @@ int addmatchcertattr(struct clsrvconf *conf) { } if (!strncasecmp(conf->matchcertattr, "SubjectAltName:rID:", 19)) { - conf->certridmatch = conf->matchcertattr+19; + conf->certridmatch = OBJ_txt2obj(conf->matchcertattr+19, 0); + if (!conf->certridmatch) + return 0; } /* the other cases below use a common regex match */ @@ -1049,7 +1072,10 @@ int addmatchcertattr(struct clsrvconf *conf) { } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:OtherName:", 25)) { v = strchr(conf->matchcertattr + 25, ':'); if (!v || +1 != '/') return 0; - conf->certothertype = stringcopy(conf->matchcertattr+25, conf->matchcertattr+25 - v - 1); + *v = '\0'; + conf->certothertype = OBJ_txt2obj(conf->matchcertattr+25, 0); + if (!conf->certothertype) + return 0; v = v+1; r = &conf->certotherregex; } From 80475bb8fca15c3c1058d7a644cd8af2a0189ea0 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 28 Sep 2020 13:25:33 +0200 Subject: [PATCH 26/38] implement match certificate attribute as closure --- radsecproxy.c | 56 +++--- radsecproxy.h | 12 +- tlscommon.c | 496 +++++++++++++++++++++----------------------------- tlscommon.h | 3 +- 4 files changed, 242 insertions(+), 325 deletions(-) diff --git a/radsecproxy.c b/radsecproxy.c index 4b59230..c907c73 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -2174,11 +2174,8 @@ void freeclsrvconf(struct clsrvconf *conf) { free(conf->confsecret); free(conf->secret); free(conf->tls); - free(conf->matchcertattr); - if (conf->certcnregex) - regfree(conf->certcnregex); - if (conf->certuriregex) - regfree(conf->certuriregex); + freegconfmstr(conf->confmatchcertattrs); + freematchcertattr(conf); free(conf->confrewritein); free(conf->confrewriteout); if (conf->rewriteusername) { @@ -2268,7 +2265,7 @@ int mergesrvconf(struct clsrvconf *dst, struct clsrvconf *src) { !mergeconfmstring(&dst->source, &src->source) || !mergeconfstring(&dst->confsecret, &src->confsecret) || !mergeconfstring(&dst->tls, &src->tls) || - !mergeconfstring(&dst->matchcertattr, &src->matchcertattr) || + !mergeconfmstring(&dst->confmatchcertattrs, &src->confmatchcertattrs) || !mergeconfstring(&dst->confrewritein, &src->confrewritein) || !mergeconfstring(&dst->confrewriteout, &src->confrewriteout) || !mergeconfstring(&dst->confrewriteusername, &src->confrewriteusername) || @@ -2309,10 +2306,11 @@ int config_hostaf(const char *desc, int ipv4only, int ipv6only, int *af) { int confclient_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *val) { struct clsrvconf *conf, *existing; - char *conftype = NULL, *rewriteinalias = NULL; + char *conftype = NULL, *rewriteinalias = NULL, **matchcertattrs = NULL; long int dupinterval = LONG_MIN, addttl = LONG_MIN; uint8_t ipv4only = 0, ipv6only = 0; struct list_node *entry; + int i; debug(DBG_DBG, "confclient_cb called for %s", block); @@ -2331,7 +2329,7 @@ int confclient_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "secret", CONF_STR_NOESC, &conf->confsecret, #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) "tls", CONF_STR, &conf->tls, - "matchcertificateattribute", CONF_STR, &conf->matchcertattr, + "matchcertificateattribute", CONF_MSTR, &matchcertattrs, "CertificateNameCheck", CONF_BLN, &conf->certnamecheck, #endif "DuplicateInterval", CONF_LINT, &dupinterval, @@ -2368,13 +2366,19 @@ int confclient_cb(struct gconffile **cf, void *arg, char *block, char *opt, char #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) if (conf->type == RAD_TLS || conf->type == RAD_DTLS) { - conf->tlsconf = conf->tls - ? tlsgettls(conf->tls, NULL) - : tlsgettls("defaultClient", "default"); - if (!conf->tlsconf) - debugx(1, DBG_ERR, "error in block %s, no tls context defined", block); - if (conf->matchcertattr && !addmatchcertattr(conf)) - debugx(1, DBG_ERR, "error in block %s, invalid MatchCertificateAttributeValue", block); + conf->tlsconf = conf->tls + ? tlsgettls(conf->tls, NULL) + : tlsgettls("defaultClient", "default"); + if (!conf->tlsconf) + debugx(1, DBG_ERR, "error in block %s, no tls context defined", block); + if (matchcertattrs) { + for (i=0; matchcertattrs[i]; i++){ + if (!addmatchcertattr(conf, matchcertattrs[i])) { + debugx(1, DBG_ERR, "error in block %s, invalid MatchCertificateAttributeValue", block); + } + } + freegconfmstr(matchcertattrs); + } } #endif @@ -2451,19 +2455,23 @@ int confclient_cb(struct gconffile **cf, void *arg, char *block, char *opt, char } int compileserverconfig(struct clsrvconf *conf, const char *block) { + int i; #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) if (conf->type == RAD_TLS || conf->type == RAD_DTLS) { conf->tlsconf = conf->tls ? tlsgettls(conf->tls, NULL) : tlsgettls("defaultServer", "default"); - if (!conf->tlsconf) { - debug(DBG_ERR, "error in block %s, no tls context defined", block); - return 0; - } - if (conf->matchcertattr && !addmatchcertattr(conf)) { - debug(DBG_ERR, "error in block %s, invalid MatchCertificateAttributeValue", block); - return 0; - } + if (!conf->tlsconf) { + debug(DBG_ERR, "error in block %s, no tls context defined", block); + return 0; + } + if (conf->matchcertattrs) { + for (i=0; conf->confmatchcertattrs[i]; i++){ + if (!addmatchcertattr(conf, conf->confmatchcertattrs[i])) { + debugx(1, DBG_ERR, "error in block %s, invalid MatchCertificateAttributeValue", block); + } + } + } } #endif @@ -2533,7 +2541,7 @@ int confserver_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "secret", CONF_STR_NOESC, &conf->confsecret, #if defined(RADPROT_TLS) || defined(RADPROT_DTLS) "tls", CONF_STR, &conf->tls, - "MatchCertificateAttribute", CONF_STR, &conf->matchcertattr, + "MatchCertificateAttribute", CONF_STR, &conf->confmatchcertattrs, "CertificateNameCheck", CONF_BLN, &conf->certnamecheck, #endif "addTTL", CONF_LINT, &addttl, diff --git a/radsecproxy.h b/radsecproxy.h index 276d925..5917faa 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -151,16 +151,8 @@ struct clsrvconf { uint8_t *secret; int secret_len; char *tls; - char *matchcertattr; - regex_t *certcnregex; - regex_t *certuriregex; - regex_t *certdnsregex; - regex_t *certotherregex; - ASN1_OBJECT *certothertype; - struct in6_addr certipmatch; - ASN1_OBJECT *certridmatch; - //char *certridmatch; - int certipmatchaf; + struct list *matchcertattrs; + char **confmatchcertattrs; char *confrewritein; char *confrewriteout; char *confrewriteusername; diff --git a/tlscommon.c b/tlscommon.c index efd2f3d..b2b555b 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -39,6 +39,16 @@ static struct hash *tlsconfs = NULL; static unsigned char cookie_secret[COOKIE_SECRET_LENGTH]; static uint8_t cookie_secret_initialized = 0; +struct certattrmatch { + int (*matchfn)(GENERAL_NAME *, struct certattrmatch *); + int type; + char * exact; + regex_t *regex; + ASN1_OBJECT *oid; + struct in6_addr ipaddr; + int af; + char * debugname; +}; /* callbacks for making OpenSSL < 1.1 thread safe */ #if OPENSSL_VERSION_NUMBER < 0x10100000 @@ -557,223 +567,110 @@ X509 *verifytlscert(SSL *ssl) { return cert; } -static STACK_OF(GENERAL_NAME) *getaltname(X509 *cert) { - STACK_OF(GENERAL_NAME) *altname; - int loc; - - loc = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); - if (loc < 0) - return NULL; - - altname = X509V3_EXT_d2i(X509_get_ext(cert, loc)); - if (!altname) - return NULL; - - return altname; +static int certattr_matchrid(GENERAL_NAME *gn, struct certattrmatch *match) { + return OBJ_cmp(gn->d.registeredID, match->oid) == 0 ? 1 : 0; } -static int matchsubjaltname(X509 *cert, int type, int (*matchfn)(GENERAL_NAME *, void *), void *arg ) { - GENERAL_NAME *gn; - int n,i,r = 0; - STACK_OF(GENERAL_NAME) *alt; +static int certattr_matchip(GENERAL_NAME *gn, struct certattrmatch *match){ + int l = ASN1_STRING_length(gn->d.iPAddress); + return (((match->af == AF_INET && l == sizeof(struct in_addr)) || (match->af == AF_INET6 && l == sizeof(struct in6_addr))) + && !memcmp(ASN1_STRING_get0_data(gn->d.iPAddress), &match->ipaddr, l)) ? 1 : 0 ; +} - alt = getaltname(cert); +static int _general_name_regex_match(char *v, int l, struct certattrmatch *match) { + char *s; + if (l <= 0 ) + return 0; + if (match->exact && memcmp(v, match->exact, l) == 0) + return 1; - n = sk_GENERAL_NAME_num(alt); - for (i = 0; i < n; i++) { - gn = sk_GENERAL_NAME_value(alt, i); - if (gn->type == type) { - r = matchfn(gn, arg); - if (r) - break; - } + s = stringcopy((char *)v, l); + if (!s) { + debug(DBG_ERR, "malloc failed"); + return 0; } - - GENERAL_NAMES_free(alt); - return r; + debug(DBG_DBG, "matchtregex: matching %s", s); + if (regexec(match->regex, s, 0, NULL, 0) == 0) { + free(s); + return 1; + } + free(s); + return 0; } -static int matchsanrid(GENERAL_NAME *gn, void *rid) { - return OBJ_cmp(gn->d.registeredID, (ASN1_OBJECT *)rid) == 0 ? 1 : 0; +static int certattr_matchregex(GENERAL_NAME *gn, struct certattrmatch *match) { + return _general_name_regex_match((char *)ASN1_STRING_get0_data(gn->d.ia5), ASN1_STRING_length(gn->d.ia5), match); } -struct matchsanip_arg { - int family; - struct in6_addr *addr; -}; -static int matchsanip(GENERAL_NAME *gn, void *arg){ - struct matchsanip_arg *addr = (struct matchsanip_arg *)arg; - int l = ASN1_STRING_length(gn->d.iPAddress); - return (((addr->family == AF_INET && l == sizeof(struct in_addr)) || (addr->family == AF_INET6 && l == sizeof(struct in6_addr))) - && !memcmp(ASN1_STRING_get0_data(gn->d.iPAddress), addr->addr, l)) ? 1 : 0 ; +static int certattr_matchothername(GENERAL_NAME *gn, struct certattrmatch *match) { + if (OBJ_cmp(gn->d.otherName->type_id, match->oid) != 0) + return 0; + return _general_name_regex_match((char *)ASN1_STRING_get0_data(gn->d.otherName->value->value.octet_string), + ASN1_STRING_length(gn->d.otherName->value->value.octet_string), + match); + } -/* -static int subjectaltnamerid(X509 *cert, ASN1_OBJECT *rid) { - STACK_OF(GENERAL_NAME) *alt = NULL; - GENERAL_NAME *gn; - int n, i, r=0; - - alt = getaltname(cert); - if (!alt) - return r; +static int certattr_matchcn(X509 *cert, struct certattrmatch *match){ + int loc; + X509_NAME *nm; + X509_NAME_ENTRY *e; + ASN1_STRING *t; - n = sk_GENERAL_NAME_num(alt); - for (i = 0; i < n; i++) { - gn = sk_GENERAL_NAME_value(alt, i); - if (gn->type != GEN_RID) - continue; - if (OBJ_cmp(gn->d.registeredID, rid) == 0){ - r = 1; + nm = X509_get_subject_name(cert); + loc = -1; + for (;;) { + loc = X509_NAME_get_index_by_NID(nm, NID_commonName, loc); + if (loc == -1) break; - } - } - - GENERAL_NAMES_free(alt); - return r; -}*/ - -static int subjectaltnameaddr(X509 *cert, int family, struct in6_addr *addr) { - int i, l, n, r = 0; - char *v; - STACK_OF(GENERAL_NAME) *alt; - GENERAL_NAME *gn; - - debug(DBG_DBG, "subjectaltnameaddr"); - - alt = getaltname(cert); - if (!alt) - return r; - n = sk_GENERAL_NAME_num(alt); - for (i = 0; i < n; i++) { - gn = sk_GENERAL_NAME_value(alt, i); - if (gn->type != GEN_IPADD) - continue; - r = -1; - v = (char *)ASN1_STRING_get0_data(gn->d.iPAddress); - l = ASN1_STRING_length(gn->d.iPAddress); - if (((family == AF_INET && l == sizeof(struct in_addr)) || (family == AF_INET6 && l == sizeof(struct in6_addr))) - && !memcmp(v, addr, l)) { - r = 1; - break; - } + e = X509_NAME_get_entry(nm, loc); + t = X509_NAME_ENTRY_get_data(e); + if (_general_name_regex_match((char *) ASN1_STRING_get0_data(t), ASN1_STRING_length(t), match)) + return 1; } - GENERAL_NAMES_free(alt); - return r; + return 0; } -static int subjectaltnameregexp(X509 *cert, int type, ASN1_OBJECT *othernametype, char *exact, regex_t *regex) { - int i, l, n, r = 0; - char *s, *v, *fail = NULL, *tmp; - STACK_OF(GENERAL_NAME) *alt; +static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { GENERAL_NAME *gn; + int loc, n,i,r = 0; + STACK_OF(GENERAL_NAME) *alt; - debug(DBG_DBG, "subjectaltnameregexp"); + /*special case: don't search in SAN, but CN field in subject */ + if (match->type == -1) + return certattr_matchcn(cert, match); - alt = getaltname(cert); + loc = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); + if (loc < 0) + return 0; + + alt = X509V3_EXT_d2i(X509_get_ext(cert, loc)); if (!alt) - return r; + return 0; n = sk_GENERAL_NAME_num(alt); for (i = 0; i < n; i++) { - gn = sk_GENERAL_NAME_value(alt, i); - if (gn->type != type) - continue; - r = -1; - if (gn->type == GEN_OTHERNAME) { - if (OBJ_cmp(gn->d.otherName->type_id, othernametype) != 0) - continue; - v = (char *)ASN1_STRING_get0_data(gn->d.otherName->value->value.octet_string); - l = ASN1_STRING_length(gn->d.otherName->value->value.octet_string); - } else { - v = (char *)ASN1_STRING_get0_data(gn->d.ia5); - l = ASN1_STRING_length(gn->d.ia5); - } - if (l <= 0) - continue; -#ifdef DEBUG - printfchars(NULL, gn->type == GEN_DNS ? "dns" : "uri", NULL, (uint8_t *) v, l); -#endif - if (exact) { - if (memcmp(v, exact, l)) - continue; - } else { - s = stringcopy((char *)v, l); - if (!s) { - debug(DBG_ERR, "malloc failed"); - continue; - } - debug(DBG_DBG, "subjectaltnameregex: matching %s", s); - if (regexec(regex, s, 0, NULL, 0)) { - tmp = fail; - if (asprintf(&fail, "%s%s%s", tmp ? tmp : "", tmp ? ", " : "", s) >= 0) - free(tmp); - else - fail = tmp; - free(s); - continue; - } - free(s); - } - r = 1; - break; + gn = sk_GENERAL_NAME_value(alt, i); + if (gn->type == match->type) { + r = match->matchfn(gn, match); + if (r) + break; + } } - if (r!=1) - debug(DBG_WARN, "subjectaltnameregex: no matching Subject Alt Name %s found! (%s)", - type == GEN_DNS ? "DNS" : type == GEN_URI ? "URI" : type == GEN_OTHERNAME ? "OtherName" : "", fail); + + //TODO old code prints non-matching elements. GENERAL_NAMES_free(alt); - free(fail); return r; } -static int cnregexp(X509 *cert, char *exact, regex_t *regex) { - int loc, l; - char *v, *s; - X509_NAME *nm; - X509_NAME_ENTRY *e; - ASN1_STRING *t; - - nm = X509_get_subject_name(cert); - loc = -1; - for (;;) { - loc = X509_NAME_get_index_by_NID(nm, NID_commonName, loc); - if (loc == -1) - break; - e = X509_NAME_get_entry(nm, loc); - t = X509_NAME_ENTRY_get_data(e); - v = (char *) ASN1_STRING_get0_data(t); - l = ASN1_STRING_length(t); - if (l < 0) - continue; - if (exact) { - if (l == strlen(exact) && !strncasecmp(exact, v, l)) - return 1; - } else { - s = stringcopy((char *)v, l); - if (!s) { - debug(DBG_ERR, "malloc failed"); - continue; - } - if (regexec(regex, s, 0, NULL, 0)) { - free(s); - continue; - } - free(s); - return 1; - } - } - return 0; -} - /* this is a bit sloppy, should not always accept match to any */ int certnamecheck(X509 *cert, struct list *hostports) { struct list_node *entry; struct hostportres *hp; int r = 0; - uint8_t type = 0; /* 0 for DNS, AF_INET for IPv4, AF_INET6 for IPv6 */ - struct in6_addr addr; + struct certattrmatch match; for (entry = list_first(hostports); entry; entry = list_next(entry)) { hp = (struct hostportres *)entry->data; @@ -781,25 +678,30 @@ int certnamecheck(X509 *cert, struct list *hostports) { /* we disable the check for prefixes */ return 1; } - if (inet_pton(AF_INET, hp->host, &addr)) - type = AF_INET; - else if (inet_pton(AF_INET6, hp->host, &addr)) - type = AF_INET6; + if (inet_pton(AF_INET, hp->host, &match.ipaddr)) + match.af = AF_INET; + else if (inet_pton(AF_INET6, hp->host, &match.ipaddr)) + match.af = AF_INET6; else - type = 0; + match.af = 0; + match.exact = hp->host; - if (type) - r = subjectaltnameaddr(cert, type, &addr); + if (match.af) + match.matchfn = &certattr_matchip; + match.type = GEN_IPADD; + r = matchsubjaltname(cert, &match); if (!r) - r = subjectaltnameregexp(cert, GEN_DNS, NULL, hp->host, NULL); + match.matchfn = &certattr_matchregex; + match.type = GEN_DNS; + r = matchsubjaltname(cert, &match); if (r) { if (r > 0) { - debug(DBG_DBG, "certnamecheck: Found subjectaltname matching %s %s", type ? "address" : "host", hp->host); + debug(DBG_DBG, "certnamecheck: Found subjectaltname matching %s %s", match.af ? "address" : "host", hp->host); return 1; } - debug(DBG_WARN, "certnamecheck: No subjectaltname matching %s %s", type ? "address" : "host", hp->host); + debug(DBG_WARN, "certnamecheck: No subjectaltname matching %s %s", match.af ? "address" : "host", hp->host); } else { - if (cnregexp(cert, hp->host, NULL)) { + if (certattr_matchcn(cert, &match)) { debug(DBG_DBG, "certnamecheck: Found cn matching host %s", hp->host); return 1; } @@ -811,8 +713,8 @@ int certnamecheck(X509 *cert, struct list *hostports) { int verifyconfcert(X509 *cert, struct clsrvconf *conf) { char *subject; - char addrbuf[INET6_ADDRSTRLEN]; int ok = 1; + struct list_node *entry; subject = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0); debug(DBG_DBG, "verifyconfcert: verify certificate for host %s, subject %s", conf->name, subject); @@ -823,49 +725,16 @@ int verifyconfcert(X509 *cert, struct clsrvconf *conf) { ok = 0; } } - if (conf->certcnregex) { - debug(DBG_DBG, "verifyconfcert: matching CN regex %s", conf->matchcertattr); - if (cnregexp(cert, NULL, conf->certcnregex) < 1) { - debug(DBG_WARN, "verifyconfcert: CN not matching regex for host %s (%s)", conf->name, subject); - ok = 0; - } - } - if (conf->certuriregex) { - debug(DBG_DBG, "verifyconfcert: matching subjectaltname URI regex %s", conf->matchcertattr); - if (subjectaltnameregexp(cert, GEN_URI, NULL, NULL, conf->certuriregex) < 1) { - debug(DBG_WARN, "verifyconfcert: subjectaltname URI not matching regex for host %s (%s)", conf->name, subject); - ok = 0; - } - } - if (conf->certdnsregex) { - debug(DBG_DBG, "verifyconfcert: matching subjectaltname DNS regex %s", conf->matchcertattr); - if (subjectaltnameregexp(cert, GEN_DNS, NULL, NULL, conf->certdnsregex) < 1) { - debug(DBG_WARN, "verifyconfcert: subjectaltname DNS not matching regex for host %s (%s)", conf->name, subject); - ok = 0; - } - } - if (conf->certotherregex && conf->certothertype) { - debug(DBG_DBG, "verifyconfcert: matching subjectaltname OtherName regex %s", conf->matchcertattr); - if (subjectaltnameregexp(cert, GEN_OTHERNAME, conf->certothertype, NULL, conf->certotherregex) < 1) { - debug(DBG_WARN, "verifyconfcert: subjectaltname DNS not matching regex for host %s (%s)", conf->name, subject); - ok = 0; - } - } - if (conf->certipmatchaf) { - struct matchsanip_arg arg = {conf->certipmatchaf, &conf->certipmatch}; - debug(DBG_DBG, "verifyconfcert: matching subjectaltname IP %s", inet_ntop(conf->certipmatchaf, &conf->certipmatch, addrbuf, INET6_ADDRSTRLEN)); - if (matchsubjaltname(cert, GEN_IPADD, &matchsanip, &arg)) { - debug(DBG_WARN, "verifyconfcert: subjectaltname IP not matching regex for host %s (%s)", conf->name, subject); - ok = 0; - } - } - if (conf->certridmatch) { - debug(DBG_DBG, "verfiyconfcert: matching subjectlatname rID %s", conf->certridmatch); - if (matchsubjaltname(cert, GEN_RID, &matchsanrid, conf->certridmatch)) { - debug(DBG_WARN, "verifyconfcert: subjectaltname rID not matching for host %s (%s)", conf->name, subject); + + for (entry = list_first(conf->matchcertattrs); entry; entry = list_next(entry)) { + if (matchsubjaltname(cert, (struct certattrmatch *)entry->data) < 1) { + debug(DBG_WARN, "verifyconfcert: %s not matching for host %s (%s)", ((struct certattrmatch *)entry->data)->debugname, conf->name, subject); ok = 0; + } else { + debug(DBG_DBG, "verifyconfcert: %s matching for host %s (%s)", ((struct certattrmatch *)entry->data)->debugname, conf->name, subject); } } + free(subject); return ok; } @@ -1039,69 +908,116 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v return 0; } -int addmatchcertattr(struct clsrvconf *conf) { - char *v; - regex_t **r; +static regex_t *compileregex(char *regstr) { + regex_t *result; - 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; - } + if (regstr[strlen(regstr) - 1] == '/') + regstr[strlen(regstr) - 1] = '\0'; + if (!*regstr) + return NULL; - if (!strncasecmp(conf->matchcertattr, "SubjectAltName:rID:", 19)) { - conf->certridmatch = OBJ_txt2obj(conf->matchcertattr+19, 0); - if (!conf->certridmatch) - return 0; + result = malloc(sizeof(regex_t)); + if (!result) { + debug(DBG_ERR, "malloc failed"); + return NULL; } - - /* the other cases below use a common regex match */ - if (!strncasecmp(conf->matchcertattr, "CN:/", 4)) { - r = &conf->certcnregex; - v = conf->matchcertattr + 4; - } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:URI:/", 20)) { - r = &conf->certuriregex; - v = conf->matchcertattr + 20; - } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:DNS:/", 20)) { - r = &conf->certdnsregex; - v = conf->matchcertattr + 20; - } else if (!strncasecmp(conf->matchcertattr, "SubjectAltName:OtherName:", 25)) { - v = strchr(conf->matchcertattr + 25, ':'); - if (!v || +1 != '/') return 0; - *v = '\0'; - conf->certothertype = OBJ_txt2obj(conf->matchcertattr+25, 0); - if (!conf->certothertype) - return 0; - v = v+1; - r = &conf->certotherregex; + if (regcomp(result, regstr, REG_EXTENDED | REG_ICASE | REG_NOSUB)) { + free(result); + debug(DBG_ERR, "failed to compile regular expression %s", regstr); + return NULL; } - else - return 0; + return result; +} - if (!*v) - return 0; - /* regexp, remove optional trailing / if present */ - if (v[strlen(v) - 1] == '/') - v[strlen(v) - 1] = '\0'; - if (!*v) - return 0; +int addmatchcertattr(struct clsrvconf *conf, char *match) { + struct certattrmatch *certattrmatch; + char *pos, *colon; + + if (!conf->matchcertattrs) { + conf->matchcertattrs = list_create(); + } + + certattrmatch = malloc(sizeof(struct certattrmatch)); + + pos = match; + colon = strchr(pos, ':'); + if (!colon) goto errexit; + + if (strncasecmp(pos, "CN", colon - pos) == 0) { + certattrmatch->regex = compileregex(colon+1); + certattrmatch->type = -1; + certattrmatch->matchfn = NULL; /*special case: don't search in SAN, but CN field in subject */ + } + else if (strncasecmp(pos, "SubjectAltName", colon - pos) == 0) { + pos = colon+1; + colon = strchr(pos, ':'); + if (!colon) goto errexit; + + if (strncasecmp(pos, "IP", colon - pos) == 0) { + pos = colon+1; + if (inet_pton(AF_INET, pos, &certattrmatch->ipaddr)) + certattrmatch->af = AF_INET; + else if (inet_pton(AF_INET6, pos, &certattrmatch->ipaddr)) + certattrmatch->af = AF_INET6; + else + goto errexit; + certattrmatch->type = GEN_IPADD; + certattrmatch->matchfn = &certattr_matchip; + } + else if(strncasecmp(pos, "URI", colon - pos) == 0) { + certattrmatch->regex = compileregex(colon+1); + certattrmatch->type = GEN_URI; + certattrmatch->matchfn = &certattr_matchregex; + } + else if(strncasecmp(pos, "DNS", colon - pos) == 0) { + certattrmatch->regex = compileregex(colon+1); + certattrmatch->type = GEN_DNS; + certattrmatch->matchfn = &certattr_matchregex; + } + else if(strncasecmp(pos, "rID", colon - pos) == 0) { + certattrmatch->oid = OBJ_txt2obj(colon+1, 0); + if (!certattrmatch->oid) goto errexit; + certattrmatch->type = GEN_RID; + certattrmatch->matchfn = &certattr_matchrid; + } + else if(strncasecmp(pos, "otherNAme", colon - pos) == 0){ + pos = colon+1; + colon = strchr(pos, ':'); + if(!colon) goto errexit; + *colon = '\0'; + if(!(certattrmatch->oid = OBJ_txt2obj(pos,0))) goto errexit; + if(!(certattrmatch->regex = compileregex(colon+1))) goto errexit; + certattrmatch->type = GEN_OTHERNAME; + certattrmatch->matchfn = &certattr_matchothername; + *colon = ';'; + } + else goto errexit; + } + else goto errexit; - *r = malloc(sizeof(regex_t)); - if (!*r) { - 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; - } + certattrmatch->debugname = stringcopy(match, 0); + if(!list_push(conf->matchcertattrs, certattrmatch)) goto errexit; return 1; + +errexit: + free(certattrmatch); + return 0; +} + +void freematchcertattr(struct clsrvconf *conf) { + struct list_node *entry; + struct certattrmatch *match; + + if (conf->matchcertattrs) { + for (entry = list_first(conf->matchcertattrs); entry; entry=list_next(entry)) { + match = ((struct certattrmatch*)entry->data); + free(match->debugname); + free(match->exact); + ASN1_OBJECT_free(match->oid); + regfree(match->regex); + } + list_destroy(conf->matchcertattrs); + } } int sslaccepttimeout (SSL *ssl, int timeout) { diff --git a/tlscommon.h b/tlscommon.h index 3cccf65..8644b0e 100644 --- a/tlscommon.h +++ b/tlscommon.h @@ -42,7 +42,8 @@ SSL_CTX *tlsgetctx(uint8_t type, struct tls *t); X509 *verifytlscert(SSL *ssl); int verifyconfcert(X509 *cert, struct clsrvconf *conf); int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *val); -int addmatchcertattr(struct clsrvconf *conf); +int addmatchcertattr(struct clsrvconf *conf, char *match); +void freematchcertattr(struct clsrvconf *conf); void tlsreloadcrls(); int sslconnecttimeout(SSL *ssl, int timeout); int sslaccepttimeout (SSL *ssl, int timeout); From c4aad6d08f730818e100c00305fa5a7a76f24907 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 28 Sep 2020 14:21:36 +0200 Subject: [PATCH 27/38] add legacy non-matching SAN log --- tlscommon.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tlscommon.c b/tlscommon.c index b2b555b..4ad9d69 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -635,6 +635,7 @@ static int certattr_matchcn(X509 *cert, struct certattrmatch *match){ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { GENERAL_NAME *gn; int loc, n,i,r = 0; + char *fail, *tmp, *s; STACK_OF(GENERAL_NAME) *alt; /*special case: don't search in SAN, but CN field in subject */ @@ -657,9 +658,20 @@ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { if (r) break; } + /*legacy print non-matching SAN*/ + if (gn->type == GEN_DNS || gn->type == GEN_URI) { + s = stringcopy((char *)ASN1_STRING_get0_data(gn->d.ia5), ASN1_STRING_length(gn->d.ia5)); + tmp = fail; + if (asprintf(&fail, "%s%s%s", tmp ? tmp : "", tmp ? ", " : "", s) >= 0) + free(tmp); + else + fail = tmp; + free(s); + } } - //TODO old code prints non-matching elements. + if (!r) + debug(DBG_WARN, "matchsubjaltname: no matching Subject Alt Name found! (%s)", fail); GENERAL_NAMES_free(alt); return r; From 3188e9862806b2e4eae614cae5847860bd989f29 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 29 Sep 2020 07:00:48 +0200 Subject: [PATCH 28/38] fix if statements --- tlscommon.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tlscommon.c b/tlscommon.c index 4ad9d69..464ac1b 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -696,16 +696,18 @@ int certnamecheck(X509 *cert, struct list *hostports) { match.af = AF_INET6; else match.af = 0; - match.exact = hp->host; + match.exact = hp->host; - if (match.af) + if (match.af) { match.matchfn = &certattr_matchip; match.type = GEN_IPADD; r = matchsubjaltname(cert, &match); - if (!r) + } + if (!r) { match.matchfn = &certattr_matchregex; match.type = GEN_DNS; r = matchsubjaltname(cert, &match); + } if (r) { if (r > 0) { debug(DBG_DBG, "certnamecheck: Found subjectaltname matching %s %s", match.af ? "address" : "host", hp->host); From 59e488f70b9ab46b9c5fe1e05817449048356b18 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 29 Sep 2020 07:04:53 +0200 Subject: [PATCH 29/38] first unit test for verifycert --- .gitignore | 1 + tests/Makefile.am | 2 +- tests/t_verify_cert.c | 68 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 tests/t_verify_cert.c diff --git a/.gitignore b/.gitignore index 8e166fc..de9c6af 100644 --- a/.gitignore +++ b/.gitignore @@ -29,5 +29,6 @@ tests/t_fticks tests/t_rewrite tests/t_rewrite_config tests/t_resizeattr +tests/t_verify_cert tests/*.log tests/*.trs diff --git a/tests/Makefile.am b/tests/Makefile.am index af5e1c0..728d734 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -4,7 +4,7 @@ AUTOMAKE_OPTIONS = foreign LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh -check_PROGRAMS = t_fticks t_rewrite t_resizeattr t_rewrite_config +check_PROGRAMS = t_fticks t_rewrite t_resizeattr t_rewrite_config t_verify_cert AM_CFLAGS = -g -Wall -Werror @SSL_CFLAGS@ @TARGET_CFLAGS@ LDADD = $(top_builddir)/librsp.a @SSL_LIBS@ LDFLAGS = @SSL_LDFLAGS@ @TARGET_LDFLAGS@ @LDFLAGS@ diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c new file mode 100644 index 0000000..0bb511e --- /dev/null +++ b/tests/t_verify_cert.c @@ -0,0 +1,68 @@ +/* Copyright (C) 2020, SWITCH */ +/* See LICENSE for licensing information. */ + +#include +#include +#include +#include "../radsecproxy.h" +#include "../debug.h" +#include "../hostport.h" + +/* /CN=test */ +char *simplecert = "-----BEGIN CERTIFICATE-----\n\ +MIHAMIGMAgkAx2VNeC1d5FswCQYHKoZIzj0EATAPMQ0wCwYDVQQDDAR0ZXN0MB4X\n\ +DTIwMDkyODE0MTEzMloXDTIwMTAwODE0MTEzMlowDzENMAsGA1UEAwwEdGVzdDAy\n\ +MBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQMNcK0IZozUpupFkD/dWBC37qI\n\ +QW4wCQYHKoZIzj0EAQMkADAhAg8Ajl0dHSkadggaqZiD72ACDjWHqYhaIAWTstBv\n\ +g/Q5\n\ +-----END CERTIFICATE-----"; + +X509 *getcert(char *pem) { + X509* certX509; + BIO* certBio; + + certBio = BIO_new(BIO_s_mem()); + BIO_write(certBio, pem , strlen(pem)); + certX509 = PEM_read_bio_X509(certBio, NULL, NULL, NULL); + + BIO_free(certBio); + + return certX509; +} + +int +main (int argc, char *argv[]) +{ + int numtests = 1; + + struct clsrvconf conf; + X509 *cert; + + debug_init("t_verify_cert"); + debug_set_level(5); + + printf("1..%d\n", numtests); + + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattrs = NULL; + conf.hostports = list_create(); + hp.host = "test"; + hp.prefixlen = 0; + list_push(conf.hostports, &hp); + + cert = getcert(simplecert); + + if (verifyconfcert(cert, &conf)) { + printf("ok %d - simple cert cn\n", numtests++); + } else { + printf("not ok %d - simple cert cn\n", numtests++); + } + X509_free(cert); + } + + return 0; +} From fea734fd922cc182cff92a89d65f6da5461171b9 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 6 Oct 2020 08:16:19 +0200 Subject: [PATCH 30/38] add unit tests for verifyconfcert --- tests/Makefile.am | 2 +- tests/t_verify_cert.c | 334 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 tests/t_verify_cert.c diff --git a/tests/Makefile.am b/tests/Makefile.am index af5e1c0..728d734 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -4,7 +4,7 @@ AUTOMAKE_OPTIONS = foreign LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh -check_PROGRAMS = t_fticks t_rewrite t_resizeattr t_rewrite_config +check_PROGRAMS = t_fticks t_rewrite t_resizeattr t_rewrite_config t_verify_cert AM_CFLAGS = -g -Wall -Werror @SSL_CFLAGS@ @TARGET_CFLAGS@ LDADD = $(top_builddir)/librsp.a @SSL_LIBS@ LDFLAGS = @SSL_LDFLAGS@ @TARGET_LDFLAGS@ @LDFLAGS@ diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c new file mode 100644 index 0000000..3b976f4 --- /dev/null +++ b/tests/t_verify_cert.c @@ -0,0 +1,334 @@ +/* Copyright (C) 2020, SWITCH */ +/* See LICENSE for licensing information. */ + +#include +#include +#include +#include +#include "../radsecproxy.h" +#include "../debug.h" +#include "../hostport.h" + +/* /CN=test */ +char *certsimplepem = "-----BEGIN CERTIFICATE-----\n\ +MIHAMIGMAgkAx2VNeC1d5FswCQYHKoZIzj0EATAPMQ0wCwYDVQQDDAR0ZXN0MB4X\n\ +DTIwMDkyODE0MTEzMloXDTIwMTAwODE0MTEzMlowDzENMAsGA1UEAwwEdGVzdDAy\n\ +MBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQMNcK0IZozUpupFkD/dWBC37qI\n\ +QW4wCQYHKoZIzj0EAQMkADAhAg8Ajl0dHSkadggaqZiD72ACDjWHqYhaIAWTstBv\n\ +g/Q5\n\ +-----END CERTIFICATE-----"; + +/* /CN=other */ +char *certsimpleotherpem = "-----BEGIN CERTIFICATE-----\n\ +MIHDMIGOAgkAwf1w/+YshIwwCQYHKoZIzj0EATAQMQ4wDAYDVQQDDAVvdGhlcjAe\n\ +Fw0yMDA5MjkwNTE1MjlaFw0yMDEwMDkwNTE1MjlaMBAxDjAMBgNVBAMMBW90aGVy\n\ +MDIwEAYHKoZIzj0CAQYFK4EEAAYDHgAEnGezNfbihAw1wrQhmjNSm6kWQP91YELf\n\ +uohBbjAJBgcqhkjOPQQBAyUAMCICDwDD9T+qjNHU461al3c11gIPAMZbk5wkhd6C\n\ +ybOsj/PY\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, SAN DNS:test.local */ +char *certsandnspem = "-----BEGIN CERTIFICATE-----\n\ +MIHrMIG3oAMCAQICFGNCMLUfhveEcLQmEnX2DqjwFZpGMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNjA4NTRaFw0yMDEwMDkxNjA4NTRaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoxkwFzAVBgNVHREEDjAMggp0ZXN0LmxvY2FsMAkG\n\ +ByqGSM49BAEDJAAwIQIPAId8FJW00y8XSFmd2lBvAg5K6WAMIFgjhtwcRFcfQg==\n\ +-----END CERTIFICATE-----"; + +/* /CN=other, SAN DNS:other.local */ +char *certsandnsotherpem = "-----BEGIN CERTIFICATE-----\n\ +MIHuMIG6oAMCAQICFAiFPNqpXcSIwxS0bfJZs8KDDafVMAkGByqGSM49BAEwEDEO\n\ +MAwGA1UEAwwFb3RoZXIwHhcNMjAwOTI5MTYxMTM2WhcNMjAxMDA5MTYxMTM2WjAQ\n\ +MQ4wDAYDVQQDDAVvdGhlcjAyMBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQM\n\ +NcK0IZozUpupFkD/dWBC37qIQW6jGjAYMBYGA1UdEQQPMA2CC290aGVyLmxvY2Fs\n\ +MAkGByqGSM49BAEDJAAwIQIOTrQCgOkGcknZEchJFDgCDwCY84F0R2BnNEba95o9\n\ +NA==\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, SAN IP Address:192.0.2.1 */ +char *certsanippem = "-----BEGIN CERTIFICATE-----\n\ +MIHlMIGxoAMCAQICFEukd75rE75+qB95Bo7fcb9wXlA9MAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkwNTQ5MjZaFw0yMDEwMDkwNTQ5MjZaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoxMwETAPBgNVHREECDAGhwTAAAIBMAkGByqGSM49\n\ +BAEDJAAwIQIPALa7jf16ypPNHJSLiotwAg4DnSeToNmbqlRsvM80Aw==\n\ +-----END CERTIFICATE-----"; + +/* /CN=other, SAN IP Address:192.0.2.2 */ +char *certsanipotherpem = "-----BEGIN CERTIFICATE-----\n\ +MIHmMIGzoAMCAQICFCYvcOo1Lqc+9JbYqfby1S9rJWufMAkGByqGSM49BAEwEDEO\n\ +MAwGA1UEAwwFb3RoZXIwHhcNMjAwOTI5MTU0OTM2WhcNMjAxMDA5MTU0OTM2WjAQ\n\ +MQ4wDAYDVQQDDAVvdGhlcjAyMBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQM\n\ +NcK0IZozUpupFkD/dWBC37qIQW6jEzARMA8GA1UdEQQIMAaHBMAAAgIwCQYHKoZI\n\ +zj0EAQMjADAgAg5trehJeRpM04SJJZ6XnAIOFfzRRnQtm5rnsP+QBe8=\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, SAN IP Address:2001:DB8:0:0:0:0:0:1 */ +char *certsanipv6pem = "-----BEGIN CERTIFICATE-----\n\ +MIHxMIG9oAMCAQICFGhkABYXfolor1EF6Li3hDQeEVU+MAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNTU1NTZaFw0yMDEwMDkxNTU1NTZaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuox8wHTAbBgNVHREEFDAShxAgAQ24AAAAAAAAAAAA\n\ +AAABMAkGByqGSM49BAEDJAAwIQIPAKsn++FWaDIcpnNBOFTuAg5C7gs7DxaNWgEu\n\ +OrBTXA==\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, SAN DNS:192.0.2.1 */ +char *certsanipindnspem = "-----BEGIN CERTIFICATE-----\n\ +MIHqMIG2oAMCAQICFFUjZGG96kpFI2fu90+jAhWsTr8YMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNTU4NDBaFw0yMDEwMDkxNTU4NDBaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoxgwFjAUBgNVHREEDTALggkxOTIuMC4yLjEwCQYH\n\ +KoZIzj0EAQMkADAhAg5BngyplTbRlQ8o/oWWwQIPAL9SfgIaXi/gD6YlQCOU\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, SAN DNS:2001:DB8::1 */ +char *certsanipv6indnspem = "-----BEGIN CERTIFICATE-----\n\ +MIHsMIG4oAMCAQICFFgnXltbOEGcWsS0vCv6Lsj4FhO3MAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNjAyMDRaFw0yMDEwMDkxNjAyMDRaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoxowGDAWBgNVHREEDzANggsyMDAxOmRiODo6MTAJ\n\ +BgcqhkjOPQQBAyQAMCECDlWFhJxpHRgt93ZzN9k7Ag8Ag0YN+dL3MEIo2sqgRWg=\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, DNS:somethinglese, DNS:test.local, IP Address:192.0.2.1, IP Address:2001:DB8:0:0:0:0:0:1 */ +char *certcomplexpem = "-----BEGIN CERTIFICATE-----\n\ +MIIBEjCB3qADAgECAhRgxyW7klgZvTf9isALCvwlVwvRtDAJBgcqhkjOPQQBMA8x\n\ +DTALBgNVBAMMBHRlc3QwHhcNMjAwOTMwMDU1MjIyWhcNMjAxMDEwMDU1MjIyWjAP\n\ +MQ0wCwYDVQQDDAR0ZXN0MDIwEAYHKoZIzj0CAQYFK4EEAAYDHgAEnGezNfbihAw1\n\ +wrQhmjNSm6kWQP91YELfuohBbqNAMD4wPAYDVR0RBDUwM4INc29tZXRoaW5nbGVz\n\ +ZYIKdGVzdC5sb2NhbIcEwAACAYcQIAENuAAAAAAAAAAAAAAAATAJBgcqhkjOPQQB\n\ +AyQAMCECDlTfJfMJElZZgvUdkatdAg8ApiXkPXLXXrV6OMRG9us=\n\ +-----END CERTIFICATE-----"; + +/* /CN=test, DNS:somethinglese, DNS:other.local, IP Address:192.0.2.2, IP Address:2001:DB8:0:0:0:0:0:2 */ +char *certcomplexotherpem = "-----BEGIN CERTIFICATE-----\n\ +MIIBFTCB4aADAgECAhR0GSgeV7pqQnbHRgv5y5plz/6+NjAJBgcqhkjOPQQBMBAx\n\ +DjAMBgNVBAMMBW90aGVyMB4XDTIwMDkzMDA1NTI1NVoXDTIwMTAxMDA1NTI1NVow\n\ +EDEOMAwGA1UEAwwFb3RoZXIwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKE\n\ +DDXCtCGaM1KbqRZA/3VgQt+6iEFuo0EwPzA9BgNVHREENjA0gg1zb21ldGhpbmds\n\ +ZXNlggtvdGhlci5sb2NhbIcEwAACAocQIAENuAAAAAAAAAAAAAAAAjAJBgcqhkjO\n\ +PQQBAyQAMCECDwCEaHL6oHT4zwH6jD91YwIOYO3L8cHIzmnGCOJYIQ4=\n\ +-----END CERTIFICATE-----"; + +X509 *getcert(char *pem) { + X509* certX509; + BIO* certBio; + + certBio = BIO_new(BIO_s_mem()); + BIO_write(certBio, pem , strlen(pem)); + certX509 = PEM_read_bio_X509(certBio, NULL, NULL, NULL); + + BIO_free(certBio); + + return certX509; +} + +int numtests = 0; +void ok(int expect, int result, char *descr) { + if (result == expect) { + printf("ok %d - %s\n", ++numtests, descr); + } else { + printf("not ok %d - %s\n", ++numtests, descr); + } +} + +int +main (int argc, char *argv[]) +{ + struct clsrvconf conf; + X509 *certsimple, *certsimpleother, *certsandns, *certsandnsother, *certsanip, *certsanipother, *certsanipindns, + *certsanipv6, *certsanipv6indns, *certcomplex, *certcomplexother; + + certsimple = getcert(certsimplepem); + certsimpleother = getcert(certsimpleotherpem); + certsandns = getcert(certsandnspem); + certsandnsother = getcert(certsandnsotherpem); + certsanip = getcert(certsanippem); + certsanipother = getcert(certsanipotherpem); + certsanipindns = getcert(certsanipindnspem); + certsanipv6 = getcert(certsanipv6pem); + certsanipv6indns = getcert(certsanipv6indnspem); + certcomplex = getcert(certcomplexpem); + certcomplexother = getcert(certcomplexotherpem); + + memset(&conf, 0, sizeof(conf)); + conf.hostports = list_create(); + + debug_init("t_verify_cert"); + debug_set_level(5); + + /* test check disabled*/ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = NULL; + hp.host = "test"; + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + ok(1, verifyconfcert(certsimple, &conf), "check disabled"); + + while(list_shift(conf.hostports)); + } + + /* test no check if prefixlen != 255 (CIDR) */ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp.host = "0.0.0.0/0"; + hp.prefixlen = 0; + list_push(conf.hostports, &hp); + + ok(1,verifyconfcert(certsimple, &conf),"cidr prefix"); + + while(list_shift(conf.hostports)); + } + + /* test simple match for CN=test */ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp.host = "test"; + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + ok(1,verifyconfcert(certsimple, &conf), "simple cert cn"); + ok(0,verifyconfcert(certsimpleother, &conf), "negative simple cert cn"); + + /* as per RFC 6125 6.4.4: CN MUST NOT be matched if SAN is present */ + ok(0,verifyconfcert(certsandns, &conf), "simple cert cn vs san dns, RFC6125"); + + while(list_shift(conf.hostports)); + } + + /* test literal ip match to SAN IP */ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp.host = "192.0.2.1"; + getaddrinfo(hp.host, NULL, NULL, &hp.addrinfo); + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + ok(1,verifyconfcert(certsanip, &conf),"san ip"); + ok(0,verifyconfcert(certsanipother, &conf),"wrong san ip"); + ok(0,verifyconfcert(certsimple, &conf), "negative san ip"); + ok(1,verifyconfcert(certsanipindns, &conf),"san ip in dns"); + ok(1,verifyconfcert(certcomplex,&conf),"san ip in complex cert"); + + freeaddrinfo(hp.addrinfo); + while(list_shift(conf.hostports)); + } + + /* test literal ipv6 match to SAN IP */ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp.host = "2001:db8::1"; + getaddrinfo(hp.host, NULL, NULL, &hp.addrinfo); + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + ok(1,verifyconfcert(certsanipv6, &conf),"san ipv6"); + ok(0,verifyconfcert(certsanipother, &conf),"wrong san ipv6"); + ok(0,verifyconfcert(certsimple, &conf),"negative san ipv6"); + ok(1,verifyconfcert(certsanipv6indns, &conf),"san ipv6 in dns"); + ok(1,verifyconfcert(certcomplex,&conf),"san ipv6 in complex cert"); + + freeaddrinfo(hp.addrinfo); + while(list_shift(conf.hostports)); + } + + /* test simple match for SAN DNS:test.local */ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp.host = "test.local"; + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + ok(1,verifyconfcert(certsandns, &conf),"san dns"); + ok(0,verifyconfcert(certsandnsother, &conf),"negative san dns"); + ok(1,verifyconfcert(certcomplex,&conf),"san dns in complex cert"); + + while(list_shift(conf.hostports)); + } + + /* test multiple hostports san dns(match in second) */ + { + struct hostportres hp1, hp2; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp1.host = "test.none"; + hp1.prefixlen = 255; + list_push(conf.hostports, &hp1); + hp2.host = "test"; + hp2.prefixlen = 255; + list_push(conf.hostports, &hp2); + + ok(1,verifyconfcert(certsimple, &conf),"multi hostport cn"); + ok(0,verifyconfcert(certsimpleother, &conf),"negative multi hostport cn"); + + while(list_shift(conf.hostports)); + } + + /* test multiple hostports san dns(match in second) */ + { + struct hostportres hp1, hp2; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = NULL; + hp1.host = "test.none"; + hp1.prefixlen = 255; + list_push(conf.hostports, &hp1); + hp2.host = "test.local"; + hp2.prefixlen = 255; + list_push(conf.hostports, &hp2); + + ok(1,verifyconfcert(certsandns, &conf),"multi hostport san dns # TODO fix in refactoring"); + ok(0,verifyconfcert(certsandnsother, &conf),"negative multi hostport san dns"); + ok(1,verifyconfcert(certcomplex,&conf),"multi hostport san dns in complex cert # TODO fix in refactoring"); + + while(list_shift(conf.hostports)); + } + + //TODO explicit CN/SAN checks + //TODO explicit & implicit combined + + printf("1..%d\n", numtests); + X509_free(certsimple); + X509_free(certsimpleother); + X509_free(certsandns); + X509_free(certsandnsother); + X509_free(certsanip); + X509_free(certsanipother); + X509_free(certsanipindns); + X509_free(certsanipv6); + X509_free(certsanipv6indns); + X509_free(certcomplex); + X509_free(certcomplexother); + + return 0; +} From 066fdd66a0df0e4c93dd2d0cfb0a651e111bfb5d Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 6 Oct 2020 13:24:39 +0200 Subject: [PATCH 31/38] add tests for explicit matchcertificateattribute --- tests/t_verify_cert.c | 298 ++++++++++++++++++++++++++++++++---------- 1 file changed, 231 insertions(+), 67 deletions(-) diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c index 3b976f4..72bf0ca 100644 --- a/tests/t_verify_cert.c +++ b/tests/t_verify_cert.c @@ -8,150 +8,176 @@ #include "../radsecproxy.h" #include "../debug.h" #include "../hostport.h" +#include "../util.h" -/* /CN=test */ -char *certsimplepem = "-----BEGIN CERTIFICATE-----\n\ +X509 *getcert(char *pem) { + X509* certX509; + BIO* certBio; + + certBio = BIO_new(BIO_s_mem()); + BIO_write(certBio, pem , strlen(pem)); + certX509 = PEM_read_bio_X509(certBio, NULL, NULL, NULL); + + BIO_free(certBio); + + return certX509; +} + +int numtests = 0; +void ok(int expect, int result, char *descr) { + if (result == expect) { + printf("ok %d - %s\n", ++numtests, descr); + } else { + printf("not ok %d - %s\n", ++numtests, descr); + } +} + +void free_match_srvconf(struct clsrvconf *conf) { + free(conf->matchcertattr); + conf->matchcertattr = NULL; + if (conf->certcnregex) + regfree(conf->certcnregex); + free(conf->certcnregex); + conf->certcnregex = NULL; + if (conf->certuriregex) + regfree(conf->certuriregex); + free(conf->certuriregex); + conf->certuriregex = NULL; + if (conf->certdnsregex) + regfree(conf->certdnsregex); + free(conf->certdnsregex); + conf->certdnsregex = NULL; + memset(&conf->certipmatch, 0, sizeof(struct in6_addr)); + conf->certipmatchaf=0; +} + +int +main (int argc, char *argv[]) +{ + struct clsrvconf conf; + X509 + /* /CN=test */ + *certsimple = getcert("-----BEGIN CERTIFICATE-----\n\ MIHAMIGMAgkAx2VNeC1d5FswCQYHKoZIzj0EATAPMQ0wCwYDVQQDDAR0ZXN0MB4X\n\ DTIwMDkyODE0MTEzMloXDTIwMTAwODE0MTEzMlowDzENMAsGA1UEAwwEdGVzdDAy\n\ MBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQMNcK0IZozUpupFkD/dWBC37qI\n\ QW4wCQYHKoZIzj0EAQMkADAhAg8Ajl0dHSkadggaqZiD72ACDjWHqYhaIAWTstBv\n\ g/Q5\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), -/* /CN=other */ -char *certsimpleotherpem = "-----BEGIN CERTIFICATE-----\n\ + /* /CN=other */ + *certsimpleother = getcert("-----BEGIN CERTIFICATE-----\n\ MIHDMIGOAgkAwf1w/+YshIwwCQYHKoZIzj0EATAQMQ4wDAYDVQQDDAVvdGhlcjAe\n\ Fw0yMDA5MjkwNTE1MjlaFw0yMDEwMDkwNTE1MjlaMBAxDjAMBgNVBAMMBW90aGVy\n\ MDIwEAYHKoZIzj0CAQYFK4EEAAYDHgAEnGezNfbihAw1wrQhmjNSm6kWQP91YELf\n\ uohBbjAJBgcqhkjOPQQBAyUAMCICDwDD9T+qjNHU461al3c11gIPAMZbk5wkhd6C\n\ ybOsj/PY\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, SAN DNS:test.local */ -char *certsandnspem = "-----BEGIN CERTIFICATE-----\n\ +*certsandns = getcert("-----BEGIN CERTIFICATE-----\n\ MIHrMIG3oAMCAQICFGNCMLUfhveEcLQmEnX2DqjwFZpGMAkGByqGSM49BAEwDzEN\n\ MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNjA4NTRaFw0yMDEwMDkxNjA4NTRaMA8x\n\ DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ tCGaM1KbqRZA/3VgQt+6iEFuoxkwFzAVBgNVHREEDjAMggp0ZXN0LmxvY2FsMAkG\n\ ByqGSM49BAEDJAAwIQIPAId8FJW00y8XSFmd2lBvAg5K6WAMIFgjhtwcRFcfQg==\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=other, SAN DNS:other.local */ -char *certsandnsotherpem = "-----BEGIN CERTIFICATE-----\n\ +*certsandnsother = getcert("-----BEGIN CERTIFICATE-----\n\ MIHuMIG6oAMCAQICFAiFPNqpXcSIwxS0bfJZs8KDDafVMAkGByqGSM49BAEwEDEO\n\ MAwGA1UEAwwFb3RoZXIwHhcNMjAwOTI5MTYxMTM2WhcNMjAxMDA5MTYxMTM2WjAQ\n\ MQ4wDAYDVQQDDAVvdGhlcjAyMBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQM\n\ NcK0IZozUpupFkD/dWBC37qIQW6jGjAYMBYGA1UdEQQPMA2CC290aGVyLmxvY2Fs\n\ MAkGByqGSM49BAEDJAAwIQIOTrQCgOkGcknZEchJFDgCDwCY84F0R2BnNEba95o9\n\ NA==\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, SAN IP Address:192.0.2.1 */ -char *certsanippem = "-----BEGIN CERTIFICATE-----\n\ +*certsanip = getcert("-----BEGIN CERTIFICATE-----\n\ MIHlMIGxoAMCAQICFEukd75rE75+qB95Bo7fcb9wXlA9MAkGByqGSM49BAEwDzEN\n\ MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkwNTQ5MjZaFw0yMDEwMDkwNTQ5MjZaMA8x\n\ DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ tCGaM1KbqRZA/3VgQt+6iEFuoxMwETAPBgNVHREECDAGhwTAAAIBMAkGByqGSM49\n\ BAEDJAAwIQIPALa7jf16ypPNHJSLiotwAg4DnSeToNmbqlRsvM80Aw==\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=other, SAN IP Address:192.0.2.2 */ -char *certsanipotherpem = "-----BEGIN CERTIFICATE-----\n\ +*certsanipother = getcert("-----BEGIN CERTIFICATE-----\n\ MIHmMIGzoAMCAQICFCYvcOo1Lqc+9JbYqfby1S9rJWufMAkGByqGSM49BAEwEDEO\n\ MAwGA1UEAwwFb3RoZXIwHhcNMjAwOTI5MTU0OTM2WhcNMjAxMDA5MTU0OTM2WjAQ\n\ MQ4wDAYDVQQDDAVvdGhlcjAyMBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQM\n\ NcK0IZozUpupFkD/dWBC37qIQW6jEzARMA8GA1UdEQQIMAaHBMAAAgIwCQYHKoZI\n\ zj0EAQMjADAgAg5trehJeRpM04SJJZ6XnAIOFfzRRnQtm5rnsP+QBe8=\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, SAN IP Address:2001:DB8:0:0:0:0:0:1 */ -char *certsanipv6pem = "-----BEGIN CERTIFICATE-----\n\ +*certsanipv6 = getcert("-----BEGIN CERTIFICATE-----\n\ MIHxMIG9oAMCAQICFGhkABYXfolor1EF6Li3hDQeEVU+MAkGByqGSM49BAEwDzEN\n\ MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNTU1NTZaFw0yMDEwMDkxNTU1NTZaMA8x\n\ DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ tCGaM1KbqRZA/3VgQt+6iEFuox8wHTAbBgNVHREEFDAShxAgAQ24AAAAAAAAAAAA\n\ AAABMAkGByqGSM49BAEDJAAwIQIPAKsn++FWaDIcpnNBOFTuAg5C7gs7DxaNWgEu\n\ OrBTXA==\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, SAN DNS:192.0.2.1 */ -char *certsanipindnspem = "-----BEGIN CERTIFICATE-----\n\ +*certsanipindns = getcert("-----BEGIN CERTIFICATE-----\n\ MIHqMIG2oAMCAQICFFUjZGG96kpFI2fu90+jAhWsTr8YMAkGByqGSM49BAEwDzEN\n\ MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNTU4NDBaFw0yMDEwMDkxNTU4NDBaMA8x\n\ DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ tCGaM1KbqRZA/3VgQt+6iEFuoxgwFjAUBgNVHREEDTALggkxOTIuMC4yLjEwCQYH\n\ KoZIzj0EAQMkADAhAg5BngyplTbRlQ8o/oWWwQIPAL9SfgIaXi/gD6YlQCOU\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, SAN DNS:2001:DB8::1 */ -char *certsanipv6indnspem = "-----BEGIN CERTIFICATE-----\n\ +*certsanipv6indns = getcert("-----BEGIN CERTIFICATE-----\n\ MIHsMIG4oAMCAQICFFgnXltbOEGcWsS0vCv6Lsj4FhO3MAkGByqGSM49BAEwDzEN\n\ MAsGA1UEAwwEdGVzdDAeFw0yMDA5MjkxNjAyMDRaFw0yMDEwMDkxNjAyMDRaMA8x\n\ DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ tCGaM1KbqRZA/3VgQt+6iEFuoxowGDAWBgNVHREEDzANggsyMDAxOmRiODo6MTAJ\n\ BgcqhkjOPQQBAyQAMCECDlWFhJxpHRgt93ZzN9k7Ag8Ag0YN+dL3MEIo2sqgRWg=\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, DNS:somethinglese, DNS:test.local, IP Address:192.0.2.1, IP Address:2001:DB8:0:0:0:0:0:1 */ -char *certcomplexpem = "-----BEGIN CERTIFICATE-----\n\ +*certcomplex = getcert("-----BEGIN CERTIFICATE-----\n\ MIIBEjCB3qADAgECAhRgxyW7klgZvTf9isALCvwlVwvRtDAJBgcqhkjOPQQBMA8x\n\ DTALBgNVBAMMBHRlc3QwHhcNMjAwOTMwMDU1MjIyWhcNMjAxMDEwMDU1MjIyWjAP\n\ MQ0wCwYDVQQDDAR0ZXN0MDIwEAYHKoZIzj0CAQYFK4EEAAYDHgAEnGezNfbihAw1\n\ wrQhmjNSm6kWQP91YELfuohBbqNAMD4wPAYDVR0RBDUwM4INc29tZXRoaW5nbGVz\n\ ZYIKdGVzdC5sb2NhbIcEwAACAYcQIAENuAAAAAAAAAAAAAAAATAJBgcqhkjOPQQB\n\ AyQAMCECDlTfJfMJElZZgvUdkatdAg8ApiXkPXLXXrV6OMRG9us=\n\ ------END CERTIFICATE-----"; +-----END CERTIFICATE-----"), /* /CN=test, DNS:somethinglese, DNS:other.local, IP Address:192.0.2.2, IP Address:2001:DB8:0:0:0:0:0:2 */ -char *certcomplexotherpem = "-----BEGIN CERTIFICATE-----\n\ +*certcomplexother = getcert("-----BEGIN CERTIFICATE-----\n\ MIIBFTCB4aADAgECAhR0GSgeV7pqQnbHRgv5y5plz/6+NjAJBgcqhkjOPQQBMBAx\n\ DjAMBgNVBAMMBW90aGVyMB4XDTIwMDkzMDA1NTI1NVoXDTIwMTAxMDA1NTI1NVow\n\ EDEOMAwGA1UEAwwFb3RoZXIwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKE\n\ DDXCtCGaM1KbqRZA/3VgQt+6iEFuo0EwPzA9BgNVHREENjA0gg1zb21ldGhpbmds\n\ ZXNlggtvdGhlci5sb2NhbIcEwAACAocQIAENuAAAAAAAAAAAAAAAAjAJBgcqhkjO\n\ PQQBAyQAMCECDwCEaHL6oHT4zwH6jD91YwIOYO3L8cHIzmnGCOJYIQ4=\n\ ------END CERTIFICATE-----"; - -X509 *getcert(char *pem) { - X509* certX509; - BIO* certBio; - - certBio = BIO_new(BIO_s_mem()); - BIO_write(certBio, pem , strlen(pem)); - certX509 = PEM_read_bio_X509(certBio, NULL, NULL, NULL); - - BIO_free(certBio); - - return certX509; -} - -int numtests = 0; -void ok(int expect, int result, char *descr) { - if (result == expect) { - printf("ok %d - %s\n", ++numtests, descr); - } else { - printf("not ok %d - %s\n", ++numtests, descr); - } -} - -int -main (int argc, char *argv[]) -{ - struct clsrvconf conf; - X509 *certsimple, *certsimpleother, *certsandns, *certsandnsother, *certsanip, *certsanipother, *certsanipindns, - *certsanipv6, *certsanipv6indns, *certcomplex, *certcomplexother; - - certsimple = getcert(certsimplepem); - certsimpleother = getcert(certsimpleotherpem); - certsandns = getcert(certsandnspem); - certsandnsother = getcert(certsandnsotherpem); - certsanip = getcert(certsanippem); - certsanipother = getcert(certsanipotherpem); - certsanipindns = getcert(certsanipindnspem); - certsanipv6 = getcert(certsanipv6pem); - certsanipv6indns = getcert(certsanipv6indnspem); - certcomplex = getcert(certcomplexpem); - certcomplexother = getcert(certcomplexotherpem); +-----END CERTIFICATE-----"), + + /* /CN=test, URI:https://test.local/profile#me */ + *certsanuri = getcert("-----BEGIN CERTIFICATE-----\n\ +MIH9MIHKoAMCAQICFHsSOjcYexRKQpNlH1oHV1cxvdgHMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDEwMDYwODU5MzRaFw0yMDEwMTYwODU5MzRaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoywwKjAoBgNVHREEITAfhh1odHRwczovL3Rlc3Qu\n\ +bG9jYWwvcHJvZmlsZSNtZTAJBgcqhkjOPQQBAyMAMCACDniwUmV285CoguiJ6WmW\n\ +Ag5ZWNTJtmNNdKxh0Mahsw==\n\ +-----END CERTIFICATE-----"), + + /* /CN=other, URI:https://other.local/profile#me */ + *certsanuriother = getcert("-----BEGIN CERTIFICATE-----\n\ +MIIBATCBzaADAgECAhQLG7rYpl+8YbPNEtUgw6HRZYIc1DAJBgcqhkjOPQQBMBAx\n\ +DjAMBgNVBAMMBW90aGVyMB4XDTIwMTAwNjA5MDU0OVoXDTIwMTAxNjA5MDU0OVow\n\ +EDEOMAwGA1UEAwwFb3RoZXIwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKE\n\ +DDXCtCGaM1KbqRZA/3VgQt+6iEFuoy0wKzApBgNVHREEIjAghh5odHRwczovL290\n\ +aGVyLmxvY2FsL3Byb2ZpbGUjbWUwCQYHKoZIzj0EAQMkADAhAg8AoOJVnRcp3gyY\n\ +Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ +-----END CERTIFICATE-----"); memset(&conf, 0, sizeof(conf)); conf.hostports = list_create(); @@ -236,6 +262,7 @@ main (int argc, char *argv[]) /* test literal ipv6 match to SAN IP */ { struct hostportres hp; + memset(&hp, 0, sizeof(struct hostportres)); conf.name = "test"; conf.certnamecheck = 1; @@ -314,10 +341,145 @@ main (int argc, char *argv[]) while(list_shift(conf.hostports)); } - //TODO explicit CN/SAN checks - //TODO explicit & implicit combined + /* test explicit CN regex */ + { + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = stringcopy("CN:/t..t/",0); + + ok(1,addmatchcertattr(&conf),"explicit cn regex config"); + + ok(1,verifyconfcert(certsimple, &conf),"explicit cn regex"); + ok(0,verifyconfcert(certsimpleother, &conf),"negative explicit cn regex"); + ok(1,verifyconfcert(certsandns, &conf), "explicit cn regex with SAN DNS"); + + free_match_srvconf(&conf); + } + + /* test explicit ip match to SAN IP */ + { + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = stringcopy("SubjectAltName:IP:192.0.2.1",0); + + ok(1,addmatchcertattr(&conf),"explicit san ip config"); + + + ok(1,verifyconfcert(certsanip, &conf),"explicit san ip"); + ok(0,verifyconfcert(certsanipother, &conf),"wrong explicit san ip"); + ok(0,verifyconfcert(certsimple, &conf), "missing explicit san ip"); + ok(1,verifyconfcert(certcomplex,&conf),"explicit san ip in complex cert"); + + free_match_srvconf(&conf); + } + + /* test explicit ipv6 match to SAN IP */ + { + + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = stringcopy("SubjectAltName:IP:2001:db8::1",0); + + ok(1,addmatchcertattr(&conf),"explicit san ipv6 config"); + + + ok(1,verifyconfcert(certsanipv6, &conf),"explicit san ipv6"); + ok(0,verifyconfcert(certsanipother, &conf),"wrong explicit san ipv6"); + ok(0,verifyconfcert(certsimple, &conf),"missing explicitsan ipv6"); + ok(1,verifyconfcert(certcomplex,&conf),"explicit san ipv6 in complex cert"); + + free_match_srvconf(&conf); + } + + /* test explicit SAN DNS regex */ + { + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = stringcopy("SubjectAltName:DNS:/t..t\\.local/",0); + + ok(1,addmatchcertattr(&conf),"explicit san dns regex config"); + + ok(1,verifyconfcert(certsandns, &conf),"explicit san dns"); + ok(0,verifyconfcert(certsandnsother, &conf),"negative explicit san dns"); + ok(0,verifyconfcert(certsimple,&conf),"missing explicit san dns"); + ok(1,verifyconfcert(certcomplex,&conf),"explicit san dns in complex cert"); + + free_match_srvconf(&conf); + } + /* test explicit SAN URI regex */ + { + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = stringcopy("SubjectAltName:URI:/https:\\/\\/test.local\\/profile#me/",0); + + ok(1,addmatchcertattr(&conf),"explicit cn regex config"); + + ok(1,verifyconfcert(certsanuri, &conf),"explicit san uri regex"); + ok(0,verifyconfcert(certsanuriother, &conf),"negative explicit san uri"); + ok(0,verifyconfcert(certsimple, &conf), "missing explicit san uri"); + + free_match_srvconf(&conf); + } + + /* test valid config syntax */ + { + conf.name = "test"; + conf.certnamecheck = 0; + conf.matchcertattr = stringcopy("CN:/t..t",0); + + ok(1,addmatchcertattr(&conf),"test regex config syntax"); + ok(1,verifyconfcert(certsimple, &conf),"test regex config syntax execution"); + + free_match_srvconf(&conf); + } + + /* test invalid config syntax */ + { + conf.name = "test"; + conf.certnamecheck = 0; + + conf.matchcertattr = stringcopy("CN:t..t",0); + ok(0,addmatchcertattr(&conf),"test invalid syntax regex"); + free_match_srvconf(&conf); + + conf.matchcertattr = stringcopy("SAN:/t..t/",0); + ok(0,addmatchcertattr(&conf),"test invalid syntax attribute"); + free_match_srvconf(&conf); + + conf.matchcertattr = stringcopy("SubjectAltName:IP:1.2.3",0); + ok(0,addmatchcertattr(&conf),"test invalid syntax ip"); + free_match_srvconf(&conf); + + conf.matchcertattr = stringcopy("SubjectAltName:IP:2001:db8:1",0); + ok(0,addmatchcertattr(&conf),"test invalid syntax ipv6"); + free_match_srvconf(&conf); + } + + /* test explicit & implicit combined */ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 1; + conf.matchcertattr = stringcopy("CN:/t..t",0); + hp.host = "test.local"; + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + ok(1,addmatchcertattr(&conf),"combined config"); + + ok(1,verifyconfcert(certsandns, &conf),"combined san dns"); + ok(0,verifyconfcert(certsandnsother, &conf),"negative combined san dns"); + ok(1,verifyconfcert(certcomplex,&conf),"combined san dns in complex cert"); + ok(0,verifyconfcert(certsimple, &conf),"combined missing san dns"); + + while(list_shift(conf.hostports)); + free_match_srvconf(&conf); + } + printf("1..%d\n", numtests); + list_free(conf.hostports); X509_free(certsimple); X509_free(certsimpleother); X509_free(certsandns); @@ -329,6 +491,8 @@ main (int argc, char *argv[]) X509_free(certsanipv6indns); X509_free(certcomplex); X509_free(certcomplexother); + X509_free(certsanuri); + X509_free(certsanuriother); return 0; } From 7edae075462113b1f6f4f48b4eeb43cfa3b90774 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 6 Oct 2020 14:56:17 +0200 Subject: [PATCH 32/38] fix memory handling --- tlscommon.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tlscommon.c b/tlscommon.c index 464ac1b..d0cfdf3 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -581,8 +581,11 @@ static int _general_name_regex_match(char *v, int l, struct certattrmatch *match char *s; if (l <= 0 ) return 0; - if (match->exact && memcmp(v, match->exact, l) == 0) - return 1; + if (match->exact) { + if (memcmp(v, match->exact, l) == 0) + return 1; + return 0; + } s = stringcopy((char *)v, l); if (!s) { @@ -635,7 +638,7 @@ static int certattr_matchcn(X509 *cert, struct certattrmatch *match){ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { GENERAL_NAME *gn; int loc, n,i,r = 0; - char *fail, *tmp, *s; + char *fail = NULL, *tmp, *s; STACK_OF(GENERAL_NAME) *alt; /*special case: don't search in SAN, but CN field in subject */ @@ -661,7 +664,8 @@ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { /*legacy print non-matching SAN*/ if (gn->type == GEN_DNS || gn->type == GEN_URI) { s = stringcopy((char *)ASN1_STRING_get0_data(gn->d.ia5), ASN1_STRING_length(gn->d.ia5)); - tmp = fail; + if (!s) continue; + tmp = fail; if (asprintf(&fail, "%s%s%s", tmp ? tmp : "", tmp ? ", " : "", s) >= 0) free(tmp); else @@ -672,6 +676,7 @@ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { if (!r) debug(DBG_WARN, "matchsubjaltname: no matching Subject Alt Name found! (%s)", fail); + free(fail); GENERAL_NAMES_free(alt); return r; @@ -684,6 +689,8 @@ int certnamecheck(X509 *cert, struct list *hostports) { int r = 0; struct certattrmatch match; + memset(&match, 0, sizeof(struct certattrmatch)); + for (entry = list_first(hostports); entry; entry = list_next(entry)) { hp = (struct hostportres *)entry->data; if (hp->prefixlen != 255) { @@ -952,6 +959,7 @@ int addmatchcertattr(struct clsrvconf *conf, char *match) { } certattrmatch = malloc(sizeof(struct certattrmatch)); + memset(certattrmatch, 0, sizeof(struct certattrmatch)); pos = match; colon = strchr(pos, ':'); @@ -1028,9 +1036,12 @@ void freematchcertattr(struct clsrvconf *conf) { free(match->debugname); free(match->exact); ASN1_OBJECT_free(match->oid); - regfree(match->regex); + if(match->regex) + regfree(match->regex); + free(match->regex); } list_destroy(conf->matchcertattrs); + conf->matchcertattrs = NULL; } } From ea9747b4a92aebfd3d7bf479347bed2ab2c9e817 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Tue, 6 Oct 2020 16:59:07 +0200 Subject: [PATCH 33/38] fix failing testcases --- tests/t_verify_cert.c | 5 +++-- tlscommon.c | 20 ++++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c index 9a02c14..2ecc2f9 100644 --- a/tests/t_verify_cert.c +++ b/tests/t_verify_cert.c @@ -272,6 +272,7 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ ok(1,verifyconfcert(certsandns, &conf),"san dns"); ok(0,verifyconfcert(certsandnsother, &conf),"negative san dns"); ok(1,verifyconfcert(certcomplex,&conf),"san dns in complex cert"); + ok(0,verifyconfcert(certsimple, &conf),"missing san dns"); while(list_shift(conf.hostports)); } @@ -308,9 +309,9 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ hp2.prefixlen = 255; list_push(conf.hostports, &hp2); - ok(1,verifyconfcert(certsandns, &conf),"multi hostport san dns # TODO fix in refactoring"); + ok(1,verifyconfcert(certsandns, &conf),"multi hostport san dns"); ok(0,verifyconfcert(certsandnsother, &conf),"negative multi hostport san dns"); - ok(1,verifyconfcert(certcomplex,&conf),"multi hostport san dns in complex cert # TODO fix in refactoring"); + ok(1,verifyconfcert(certcomplex,&conf),"multi hostport san dns in complex cert"); while(list_shift(conf.hostports)); } diff --git a/tlscommon.c b/tlscommon.c index d0cfdf3..aadefa2 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -582,7 +582,7 @@ static int _general_name_regex_match(char *v, int l, struct certattrmatch *match if (l <= 0 ) return 0; if (match->exact) { - if (memcmp(v, match->exact, l) == 0) + if (l == strlen(match->exact) && memcmp(v, match->exact, l) == 0) return 1; return 0; } @@ -635,6 +635,10 @@ static int certattr_matchcn(X509 *cert, struct certattrmatch *match){ return 0; } +/* returns + 1 if expected type is present and matches + 0 if expected type is not present + -1 if expected type is present but does not match */ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { GENERAL_NAME *gn; int loc, n,i,r = 0; @@ -660,6 +664,7 @@ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { r = match->matchfn(gn, match); if (r) break; + r = -1; } /*legacy print non-matching SAN*/ if (gn->type == GEN_DNS || gn->type == GEN_URI) { @@ -674,7 +679,7 @@ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { } } - if (!r) + if (r<1) debug(DBG_WARN, "matchsubjaltname: no matching Subject Alt Name found! (%s)", fail); free(fail); @@ -682,7 +687,6 @@ static int matchsubjaltname(X509 *cert, struct certattrmatch* match) { return r; } -/* this is a bit sloppy, should not always accept match to any */ int certnamecheck(X509 *cert, struct list *hostports) { struct list_node *entry; struct hostportres *hp; @@ -692,6 +696,7 @@ int certnamecheck(X509 *cert, struct list *hostports) { memset(&match, 0, sizeof(struct certattrmatch)); for (entry = list_first(hostports); entry; entry = list_next(entry)) { + r = 0; hp = (struct hostportres *)entry->data; if (hp->prefixlen != 255) { /* we disable the check for prefixes */ @@ -931,6 +936,9 @@ int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *v static regex_t *compileregex(char *regstr) { regex_t *result; + if (regstr[0] != '/') + return NULL; + regstr++; if (regstr[strlen(regstr) - 1] == '/') regstr[strlen(regstr) - 1] = '\0'; @@ -966,7 +974,7 @@ int addmatchcertattr(struct clsrvconf *conf, char *match) { if (!colon) goto errexit; if (strncasecmp(pos, "CN", colon - pos) == 0) { - certattrmatch->regex = compileregex(colon+1); + if(!(certattrmatch->regex = compileregex(colon+1))) goto errexit; certattrmatch->type = -1; certattrmatch->matchfn = NULL; /*special case: don't search in SAN, but CN field in subject */ } @@ -987,12 +995,12 @@ int addmatchcertattr(struct clsrvconf *conf, char *match) { certattrmatch->matchfn = &certattr_matchip; } else if(strncasecmp(pos, "URI", colon - pos) == 0) { - certattrmatch->regex = compileregex(colon+1); + if(!(certattrmatch->regex = compileregex(colon+1))) goto errexit; certattrmatch->type = GEN_URI; certattrmatch->matchfn = &certattr_matchregex; } else if(strncasecmp(pos, "DNS", colon - pos) == 0) { - certattrmatch->regex = compileregex(colon+1); + if(!(certattrmatch->regex = compileregex(colon+1))) goto errexit; certattrmatch->type = GEN_DNS; certattrmatch->matchfn = &certattr_matchregex; } From 3030935252918b35ff7c18fb6a279ba5ef841186 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Sun, 11 Oct 2020 22:00:55 +0200 Subject: [PATCH 34/38] add tests for new altnames --- tests/t_verify_cert.c | 84 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 2 deletions(-) diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c index 2ecc2f9..4a2e96f 100644 --- a/tests/t_verify_cert.c +++ b/tests/t_verify_cert.c @@ -151,14 +151,52 @@ bG9jYWwvcHJvZmlsZSNtZTAJBgcqhkjOPQQBAyMAMCACDniwUmV285CoguiJ6WmW\n\ Ag5ZWNTJtmNNdKxh0Mahsw==\n\ -----END CERTIFICATE-----"), - /* /CN=other, URI:https://other.local/profile#me */ - *certsanuriother = getcert("-----BEGIN CERTIFICATE-----\n\ + /* /CN=other, URI:https://other.local/profile#me */ + *certsanuriother = getcert("-----BEGIN CERTIFICATE-----\n\ MIIBATCBzaADAgECAhQLG7rYpl+8YbPNEtUgw6HRZYIc1DAJBgcqhkjOPQQBMBAx\n\ DjAMBgNVBAMMBW90aGVyMB4XDTIwMTAwNjA5MDU0OVoXDTIwMTAxNjA5MDU0OVow\n\ EDEOMAwGA1UEAwwFb3RoZXIwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKE\n\ DDXCtCGaM1KbqRZA/3VgQt+6iEFuoy0wKzApBgNVHREEIjAghh5odHRwczovL290\n\ aGVyLmxvY2FsL3Byb2ZpbGUjbWUwCQYHKoZIzj0EAQMkADAhAg8AoOJVnRcp3gyY\n\ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ +-----END CERTIFICATE-----"), + + /* /CN=test, Registered ID:1.2.3.4 */ + *certsanrid = getcert("-----BEGIN CERTIFICATE-----\n\ +MIHjMIGwoAMCAQICFBKq59XodNaMiLZDZbE7BMFn+GnAMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDEwMDYxNTA1NTBaFw0yMDEwMTYxNTA1NTBaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoxIwEDAOBgNVHREEBzAFiAMqAwQwCQYHKoZIzj0E\n\ +AQMjADAgAg4QFOirxwoC5OYpFArE8gIORG+zCoikzhvY95kBGvg=\n\ +-----END CERTIFICATE-----"), + + /* /CN=other, Registered ID:1.2.3.9 */ + *certsanridother = getcert("-----BEGIN CERTIFICATE-----\n\ +MIHmMIGyoAMCAQICFEvhI4VZvPr7cITrckvz6J576uy3MAkGByqGSM49BAEwEDEO\n\ +MAwGA1UEAwwFb3RoZXIwHhcNMjAxMDA2MTUwNzQzWhcNMjAxMDE2MTUwNzQzWjAQ\n\ +MQ4wDAYDVQQDDAVvdGhlcjAyMBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQM\n\ +NcK0IZozUpupFkD/dWBC37qIQW6jEjAQMA4GA1UdEQQHMAWIAyoDCTAJBgcqhkjO\n\ +PQQBAyQAMCECDwCJMMBtTsOZNwvy43TlLgIOKtssl/hBDN/JcPbBQgI=\n\ +-----END CERTIFICATE-----"), + + /* /CN=test, otherNAME 1.3.6.1.5.5.7.8.8;UTF8:test.local */ + *certsanothername = getcert("-----BEGIN CERTIFICATE-----\n\ +MIH4MIHFoAMCAQICFHfn1oV2cr4BkkWImdYCJXkSmiKrMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDEwMDYxNTE4NTNaFw0yMDEwMTYxNTE4NTNaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoycwJTAjBgNVHREEHDAaoBgGCCsGAQUFBwgIoAwM\n\ +CnRlc3QubG9jYWwwCQYHKoZIzj0EAQMjADAgAg5picQbJfIM1Ljn7H/26QIOCLcA\n\ +UXfI8XA07aHTgzE=\n\ +-----END CERTIFICATE-----"), + + /* /CN=other, otherNAME 1.3.6.1.5.5.7.8.8;UTF8:other.local */ + *certsanothernameother = getcert("-----BEGIN CERTIFICATE-----\n\ +MIH6MIHGoAMCAQICFEa/hIvgCkqCF6ulCq3Jy3iw6XkwMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDEwMDYxNTIwMDhaFw0yMDEwMTYxNTIwMDhaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuoygwJjAkBgNVHREEHTAboBkGCCsGAQUFBwgIoA0M\n\ +C290aGVyLmxvY2FsMAkGByqGSM49BAEDJAAwIQIOSOJ5OK2xzjrCweD/ImECDwDL\n\ +COiok62ckBQsaUG8AA==\n\ -----END CERTIFICATE-----"); memset(&conf, 0, sizeof(conf)); @@ -385,6 +423,7 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ freematchcertattr(&conf); free(match); } + /* test explicit SAN URI regex */ { conf.name = "test"; @@ -401,6 +440,38 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ free(match); } + /* test explicit SAN rID */ + { + conf.name = "test"; + conf.certnamecheck = 0; + match = stringcopy("SubjectAltName:rID:1.2.3.4",0); + + ok(1,addmatchcertattr(&conf, match),"explicit san rid config"); + + ok(1,verifyconfcert(certsanrid, &conf),"explicit san rid"); + ok(0,verifyconfcert(certsanridother, &conf),"negative explicit san rid"); + ok(0,verifyconfcert(certsimple, &conf), "missing explicit san rid"); + + freematchcertattr(&conf); + free(match); + } + + /* test explicit SAN otherNAME */ + { + conf.name = "test"; + conf.certnamecheck = 0; + match = stringcopy("SubjectAltName:otherName:1.3.6.1.5.5.7.8.8:/test.local/",0); + + ok(1,addmatchcertattr(&conf, match),"explicit san otherName config"); + + ok(1,verifyconfcert(certsanothername, &conf),"explicit san otherName"); + ok(0,verifyconfcert(certsanothernameother, &conf),"negative explicit san otherName"); + ok(0,verifyconfcert(certsimple, &conf), "missing explicit san otherName"); + + freematchcertattr(&conf); + free(match); + } + /* test valid config syntax */ { conf.name = "test"; @@ -438,6 +509,11 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ ok(0,addmatchcertattr(&conf, match),"test invalid syntax ipv6"); freematchcertattr(&conf); free(match); + + match = stringcopy("SubjectAltName:rID:1:2",0); + ok(0,addmatchcertattr(&conf, match),"test invalid syntax rID"); + freematchcertattr(&conf); + free(match); } /* test explicit & implicit combined */ @@ -463,6 +539,8 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ free(match); } + //TODO test new features + // - multiple attribute checks printf("1..%d\n", numtests); list_free(conf.hostports); @@ -479,6 +557,8 @@ Qe0Vy/UCDijCHK6Y5GkzWD7H008l\n\ X509_free(certcomplexother); X509_free(certsanuri); X509_free(certsanuriother); + X509_free(certsanrid); + X509_free(certsanridother); return 0; } From 834c1cf481349e2392ffb1005987665b843cc9c4 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 18 Dec 2020 11:27:02 +0100 Subject: [PATCH 35/38] add test for multiple attribute checks --- tests/t_verify_cert.c | 53 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c index 4a2e96f..e90ee55 100644 --- a/tests/t_verify_cert.c +++ b/tests/t_verify_cert.c @@ -197,6 +197,26 @@ DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ tCGaM1KbqRZA/3VgQt+6iEFuoygwJjAkBgNVHREEHTAboBkGCCsGAQUFBwgIoA0M\n\ C290aGVyLmxvY2FsMAkGByqGSM49BAEDJAAwIQIOSOJ5OK2xzjrCweD/ImECDwDL\n\ COiok62ckBQsaUG8AA==\n\ +-----END CERTIFICATE-----"), + + /* /CN=test, DNS:test.local, Registered ID:1.2.3.4 */ + *certmulti = getcert("-----BEGIN CERTIFICATE-----\n\ +MIHxMIG8oAMCAQICFFrDaNQffsxLTERNbN7sXupYziWAMAkGByqGSM49BAEwDzEN\n\ +MAsGA1UEAwwEdGVzdDAeFw0yMDEyMTgwOTQwMDFaFw0yMTAxMTcwOTQwMDFaMA8x\n\ +DTALBgNVBAMMBHRlc3QwMjAQBgcqhkjOPQIBBgUrgQQABgMeAAScZ7M19uKEDDXC\n\ +tCGaM1KbqRZA/3VgQt+6iEFuox4wHDAaBgNVHREEEzARggp0ZXN0LmxvY2FsiAMq\n\ +AwQwCQYHKoZIzj0EAQMlADAiAg8AnsiRL2CH3u0bAX/FOt4CDwC9wGzr0l/PCnxK\n\ +mKlpkQ==\n\ +-----END CERTIFICATE-----"), + + /* /CN=other, DNS:other.local, Registered ID:1.2.3.4 */ + *certmultiother = getcert("-----BEGIN CERTIFICATE-----\n\ +MIHyMIG/oAMCAQICFAke6IO1yAeuwOewT/QfAF9afFo7MAkGByqGSM49BAEwEDEO\n\ +MAwGA1UEAwwFb3RoZXIwHhcNMjAxMjE4MDk0NTI1WhcNMjEwMTE3MDk0NTI1WjAQ\n\ +MQ4wDAYDVQQDDAVvdGhlcjAyMBAGByqGSM49AgEGBSuBBAAGAx4ABJxnszX24oQM\n\ +NcK0IZozUpupFkD/dWBC37qIQW6jHzAdMBsGA1UdEQQUMBKCC290aGVyLmxvY2Fs\n\ +iAMqAwQwCQYHKoZIzj0EAQMjADAgAg521Y8BtyeKAMIY8lcLbwIORNNmcwVIJjGj\n\ +vY/uPjA=\n\ -----END CERTIFICATE-----"); memset(&conf, 0, sizeof(conf)); @@ -539,8 +559,33 @@ COiok62ckBQsaUG8AA==\n\ free(match); } - //TODO test new features - // - multiple attribute checks + /* test multiple explicit checks*/ + { + struct hostportres hp; + + conf.name = "test"; + conf.certnamecheck = 0; + hp.host = "test.local"; + hp.prefixlen = 255; + list_push(conf.hostports, &hp); + + match = stringcopy("SubjectAltName:DNS:/test\\.local/",0); + ok(1,addmatchcertattr(&conf, match),"multiple check 1"); + free(match); + match = stringcopy("SubjectAltName:rID:1.2.3.4",0); + ok(1,addmatchcertattr(&conf, match),"multiple check 2"); + free(match); + + ok(0,verifyconfcert(certsandns, &conf),"multiple missing rID"); + ok(0,verifyconfcert(certsanrid, &conf), "multiple missing DNS"); + ok(1,verifyconfcert(certmulti, &conf),"multiple SANs"); + ok(0,verifyconfcert(certmultiother, &conf),"multiple negative match"); + ok(0,verifyconfcert(certcomplex, &conf),"multiple missing rID in complex cert"); + ok(0,verifyconfcert(certsimple, &conf),"multiple missing everything"); + + while(list_shift(conf.hostports)); + freematchcertattr(&conf); + } printf("1..%d\n", numtests); list_free(conf.hostports); @@ -559,6 +604,10 @@ COiok62ckBQsaUG8AA==\n\ X509_free(certsanuriother); X509_free(certsanrid); X509_free(certsanridother); + X509_free(certsanothername); + X509_free(certsanothernameother); + X509_free(certmulti); + X509_free(certmultiother); return 0; } From 1f67a681cff4e8dbedb3cc0cd344993834fd3ed0 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 18 Dec 2020 14:45:45 +0100 Subject: [PATCH 36/38] dont mess with passed string in addmatchcertattr --- tests/t_verify_cert.c | 68 ++++++++++--------------------------------- tlscommon.c | 10 ++++--- tlscommon.h | 2 +- 3 files changed, 23 insertions(+), 57 deletions(-) diff --git a/tests/t_verify_cert.c b/tests/t_verify_cert.c index e90ee55..ca0d47e 100644 --- a/tests/t_verify_cert.c +++ b/tests/t_verify_cert.c @@ -36,7 +36,6 @@ int main (int argc, char *argv[]) { struct clsrvconf conf; - char *match; X509 /* /CN=test */ *certsimple = getcert("-----BEGIN CERTIFICATE-----\n\ @@ -379,25 +378,21 @@ vY/uPjA=\n\ conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("CN:/t..t/",0); - ok(1,addmatchcertattr(&conf, match),"explicit cn regex config"); + ok(1,addmatchcertattr(&conf, "CN:/t..t/"),"explicit cn regex config"); ok(1,verifyconfcert(certsimple, &conf),"explicit cn regex"); ok(0,verifyconfcert(certsimpleother, &conf),"negative explicit cn regex"); ok(1,verifyconfcert(certsandns, &conf), "explicit cn regex with SAN DNS"); freematchcertattr(&conf); - free(match); } /* test explicit ip match to SAN IP */ { conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("SubjectAltName:IP:192.0.2.1",0); - - ok(1,addmatchcertattr(&conf, match),"explicit san ip config"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:IP:192.0.2.1"),"explicit san ip config"); ok(1,verifyconfcert(certsanip, &conf),"explicit san ip"); ok(0,verifyconfcert(certsanipother, &conf),"wrong explicit san ip"); @@ -405,18 +400,14 @@ vY/uPjA=\n\ ok(1,verifyconfcert(certcomplex,&conf),"explicit san ip in complex cert"); freematchcertattr(&conf); - free(match); } /* test explicit ipv6 match to SAN IP */ { - conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("SubjectAltName:IP:2001:db8::1",0); - - ok(1,addmatchcertattr(&conf, match),"explicit san ipv6 config"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:IP:2001:db8::1"),"explicit san ipv6 config"); ok(1,verifyconfcert(certsanipv6, &conf),"explicit san ipv6"); ok(0,verifyconfcert(certsanipother, &conf),"wrong explicit san ipv6"); @@ -424,16 +415,14 @@ vY/uPjA=\n\ ok(1,verifyconfcert(certcomplex,&conf),"explicit san ipv6 in complex cert"); freematchcertattr(&conf); - free(match); } /* test explicit SAN DNS regex */ { conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("SubjectAltName:DNS:/t..t\\.local/",0); - ok(1,addmatchcertattr(&conf, match),"explicit san dns regex config"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:DNS:/t..t\\.local/"),"explicit san dns regex config"); ok(1,verifyconfcert(certsandns, &conf),"explicit san dns"); ok(0,verifyconfcert(certsandnsother, &conf),"negative explicit san dns"); @@ -441,68 +430,59 @@ vY/uPjA=\n\ ok(1,verifyconfcert(certcomplex,&conf),"explicit san dns in complex cert"); freematchcertattr(&conf); - free(match); } /* test explicit SAN URI regex */ { conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("SubjectAltName:URI:/https:\\/\\/test.local\\/profile#me/",0); - ok(1,addmatchcertattr(&conf, match),"explicit cn regex config"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:URI:/https:\\/\\/test.local\\/profile#me/"),"explicit cn regex config"); ok(1,verifyconfcert(certsanuri, &conf),"explicit san uri regex"); ok(0,verifyconfcert(certsanuriother, &conf),"negative explicit san uri"); ok(0,verifyconfcert(certsimple, &conf), "missing explicit san uri"); freematchcertattr(&conf); - free(match); } /* test explicit SAN rID */ { conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("SubjectAltName:rID:1.2.3.4",0); - ok(1,addmatchcertattr(&conf, match),"explicit san rid config"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:rID:1.2.3.4"),"explicit san rid config"); ok(1,verifyconfcert(certsanrid, &conf),"explicit san rid"); ok(0,verifyconfcert(certsanridother, &conf),"negative explicit san rid"); ok(0,verifyconfcert(certsimple, &conf), "missing explicit san rid"); freematchcertattr(&conf); - free(match); } /* test explicit SAN otherNAME */ { conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("SubjectAltName:otherName:1.3.6.1.5.5.7.8.8:/test.local/",0); - ok(1,addmatchcertattr(&conf, match),"explicit san otherName config"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:otherName:1.3.6.1.5.5.7.8.8:/test.local/"),"explicit san otherName config"); ok(1,verifyconfcert(certsanothername, &conf),"explicit san otherName"); ok(0,verifyconfcert(certsanothernameother, &conf),"negative explicit san otherName"); ok(0,verifyconfcert(certsimple, &conf), "missing explicit san otherName"); freematchcertattr(&conf); - free(match); } /* test valid config syntax */ { conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("CN:/t..t",0); - ok(1,addmatchcertattr(&conf, match),"test regex config syntax"); + ok(1,addmatchcertattr(&conf, "CN:/t..t"),"test regex config syntax"); ok(1,verifyconfcert(certsimple, &conf),"test regex config syntax execution"); freematchcertattr(&conf); - free(match); } /* test invalid config syntax */ @@ -510,30 +490,20 @@ vY/uPjA=\n\ conf.name = "test"; conf.certnamecheck = 0; - match = stringcopy("CN:t..t",0); - ok(0,addmatchcertattr(&conf, match),"test invalid syntax regex"); + ok(0,addmatchcertattr(&conf, "CN:t..t"),"test invalid syntax regex"); freematchcertattr(&conf); - free(match); - match = stringcopy("SAN:/t..t/",0); - ok(0,addmatchcertattr(&conf, match),"test invalid syntax attribute"); + ok(0,addmatchcertattr(&conf, "SAN:/t..t/"),"test invalid syntax attribute"); freematchcertattr(&conf); - free(match); - match = stringcopy("SubjectAltName:IP:1.2.3",0); - ok(0,addmatchcertattr(&conf, match),"test invalid syntax ip"); + ok(0,addmatchcertattr(&conf, "SubjectAltName:IP:1.2.3"),"test invalid syntax ip"); freematchcertattr(&conf); - free(match); - match = stringcopy("SubjectAltName:IP:2001:db8:1",0); - ok(0,addmatchcertattr(&conf, match),"test invalid syntax ipv6"); + ok(0,addmatchcertattr(&conf, "SubjectAltName:IP:2001:db8:1"),"test invalid syntax ipv6"); freematchcertattr(&conf); - free(match); - match = stringcopy("SubjectAltName:rID:1:2",0); - ok(0,addmatchcertattr(&conf, match),"test invalid syntax rID"); + ok(0,addmatchcertattr(&conf, "SubjectAltName:rID:1:2"),"test invalid syntax rID"); freematchcertattr(&conf); - free(match); } /* test explicit & implicit combined */ @@ -542,12 +512,11 @@ vY/uPjA=\n\ conf.name = "test"; conf.certnamecheck = 1; - match = stringcopy("CN:/t..t",0); hp.host = "test.local"; hp.prefixlen = 255; list_push(conf.hostports, &hp); - ok(1,addmatchcertattr(&conf, match),"combined config"); + ok(1,addmatchcertattr(&conf, "CN:/t..t"),"combined config"); ok(1,verifyconfcert(certsandns, &conf),"combined san dns"); ok(0,verifyconfcert(certsandnsother, &conf),"negative combined san dns"); @@ -556,7 +525,6 @@ vY/uPjA=\n\ while(list_shift(conf.hostports)); freematchcertattr(&conf); - free(match); } /* test multiple explicit checks*/ @@ -569,12 +537,8 @@ vY/uPjA=\n\ hp.prefixlen = 255; list_push(conf.hostports, &hp); - match = stringcopy("SubjectAltName:DNS:/test\\.local/",0); - ok(1,addmatchcertattr(&conf, match),"multiple check 1"); - free(match); - match = stringcopy("SubjectAltName:rID:1.2.3.4",0); - ok(1,addmatchcertattr(&conf, match),"multiple check 2"); - free(match); + ok(1,addmatchcertattr(&conf, "SubjectAltName:DNS:/test\\.local/"),"multiple check 1"); + ok(1,addmatchcertattr(&conf, "SubjectAltName:rID:1.2.3.4"),"multiple check 2"); ok(0,verifyconfcert(certsandns, &conf),"multiple missing rID"); ok(0,verifyconfcert(certsanrid, &conf), "multiple missing DNS"); diff --git a/tlscommon.c b/tlscommon.c index aadefa2..48c8db4 100644 --- a/tlscommon.c +++ b/tlscommon.c @@ -958,9 +958,9 @@ static regex_t *compileregex(char *regstr) { return result; } -int addmatchcertattr(struct clsrvconf *conf, char *match) { +int addmatchcertattr(struct clsrvconf *conf, const char *match) { struct certattrmatch *certattrmatch; - char *pos, *colon; + char *pos, *colon, *matchcopy; if (!conf->matchcertattrs) { conf->matchcertattrs = list_create(); @@ -969,7 +969,8 @@ int addmatchcertattr(struct clsrvconf *conf, char *match) { certattrmatch = malloc(sizeof(struct certattrmatch)); memset(certattrmatch, 0, sizeof(struct certattrmatch)); - pos = match; + matchcopy = stringcopy(match,0); + pos = matchcopy; colon = strchr(pos, ':'); if (!colon) goto errexit; @@ -1019,7 +1020,6 @@ int addmatchcertattr(struct clsrvconf *conf, char *match) { if(!(certattrmatch->regex = compileregex(colon+1))) goto errexit; certattrmatch->type = GEN_OTHERNAME; certattrmatch->matchfn = &certattr_matchothername; - *colon = ';'; } else goto errexit; } @@ -1027,10 +1027,12 @@ int addmatchcertattr(struct clsrvconf *conf, char *match) { certattrmatch->debugname = stringcopy(match, 0); if(!list_push(conf->matchcertattrs, certattrmatch)) goto errexit; + free(matchcopy); return 1; errexit: free(certattrmatch); + free(matchcopy); return 0; } diff --git a/tlscommon.h b/tlscommon.h index 8644b0e..c376675 100644 --- a/tlscommon.h +++ b/tlscommon.h @@ -42,7 +42,7 @@ SSL_CTX *tlsgetctx(uint8_t type, struct tls *t); X509 *verifytlscert(SSL *ssl); int verifyconfcert(X509 *cert, struct clsrvconf *conf); int conftls_cb(struct gconffile **cf, void *arg, char *block, char *opt, char *val); -int addmatchcertattr(struct clsrvconf *conf, char *match); +int addmatchcertattr(struct clsrvconf *conf, const char *match); void freematchcertattr(struct clsrvconf *conf); void tlsreloadcrls(); int sslconnecttimeout(SSL *ssl, int timeout); From 6260cc453b29f133398c6266e4771d090d5cf94e Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 18 Dec 2020 16:51:37 +0100 Subject: [PATCH 37/38] update manpage and changelog --- ChangeLog | 2 ++ radsecproxy.conf.5.in | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 620b6b2..68dc918 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,8 @@ unreleased chanes - User configurable cipher-list and ciphersuites - User configurable TLS versions - Config option for DH-file + - Add rID and otherName options to certifcateAttributeCheck + - Allow multiple matchCertificateAttribute Misc: - Move radsecproxy manpage to section 8 diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 88e9178..299a991 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -413,13 +413,21 @@ 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/ +\fBmatchCertificateAttribute \fRCN:/\fIregexp\fR/ .br -\fBMatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR +\fBmatchCertificateAttribute \fRSubjectAltName:DNS:/\fIregexp\fR/ +.br +\fBmatchCertificateAttribute \fRSubjectAltName:URI:/\fIregexp\fR/ +.br +\fBmatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR +.br +\fBmatchCertificateAttribute \fRSubjectAltName:rID:\fIoid\fR +.br +\fBmatchCertificateAttribute \fRSubjectAltName:otherName:\fIoid\fR:/\fIregexp\fR/ .RS 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. +of CN and SubjectAltName types URI, DNS, IP, rID, and otherName is supported. If specified +multiple times, all terms must match for the certificate to be considered valid. .RE .BI "DuplicateInterval " seconds From 5f772842c37e96de00408e48563deb365630ff2d Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 8 Jan 2021 07:52:14 +0100 Subject: [PATCH 38/38] fix matchCertificateAttribute not applied in server config minor manpage fixes --- radsecproxy.c | 2 +- radsecproxy.conf.5.in | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/radsecproxy.c b/radsecproxy.c index c907c73..6a2acdb 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -2465,7 +2465,7 @@ int compileserverconfig(struct clsrvconf *conf, const char *block) { debug(DBG_ERR, "error in block %s, no tls context defined", block); return 0; } - if (conf->matchcertattrs) { + if (conf->confmatchcertattrs) { for (i=0; conf->confmatchcertattrs[i]; i++){ if (!addmatchcertattr(conf, conf->confmatchcertattrs[i])) { debugx(1, DBG_ERR, "error in block %s, invalid MatchCertificateAttributeValue", block); diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 299a991..f346e9c 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -413,17 +413,17 @@ For a TLS/DTLS client, disable the default behaviour of matching CN or SubjectAltName against the specified hostname or IP address. .RE -\fBmatchCertificateAttribute \fRCN:/\fIregexp\fR/ +\fBMatchCertificateAttribute \fRCN:/\fIregexp\fR/ .br -\fBmatchCertificateAttribute \fRSubjectAltName:DNS:/\fIregexp\fR/ +\fBMatchCertificateAttribute \fRSubjectAltName:DNS:/\fIregexp\fR/ .br -\fBmatchCertificateAttribute \fRSubjectAltName:URI:/\fIregexp\fR/ +\fBMatchCertificateAttribute \fRSubjectAltName:URI:/\fIregexp\fR/ .br -\fBmatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR +\fBMatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR .br -\fBmatchCertificateAttribute \fRSubjectAltName:rID:\fIoid\fR +\fBMatchCertificateAttribute \fRSubjectAltName:rID:\fIoid\fR .br -\fBmatchCertificateAttribute \fRSubjectAltName:otherName:\fIoid\fR:/\fIregexp\fR/ +\fBMatchCertificateAttribute \fRSubjectAltName:otherName:\fIoid\fR:/\fIregexp\fR/ .RS Perform additional validation of certificate attributes. Currently matching of CN and SubjectAltName types URI, DNS, IP, rID, and otherName is supported. If specified @@ -625,9 +625,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 SubjectAltName:DNS \fB) :\fR/\fIregexp\fR/ -.br -\fBMatchCertificateAttribute \fRSubjectAltName:IP:\fIaddress\fR +\fBMatchCertificateAttribute \fR... .br .BR "AddTTL " 1-255 .br