| Message ID | NaAsPRnb2g-raEjxjv15C_odADru2NIyXCs5wvZO609b_-4IqW2Do2sVbBull-FE9myUOZ7lNV6gdAx7MGugGUOBbdYOFO8AsoFHu35BIs8=@protonmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] openssl: alternative names support for --verify-x509-name CN checks | expand |
Am 10.02.20 um 18:59 schrieb Mateusz Markowicz via Openvpn-devel: > when using "--verify-x509-name [hostname] name" hostname will now be > accepted > also when matched against one of the X509v3 Subject Alternative Name IP > or DNS > entries (instead of just Subject's CN). > > see also: https://github.com/OpenVPN/openvpn/pull/136/ > > Signed-off-by: Mateusz Markowicz <poniekad@protonmail.com > <mailto:poniekad@protonmail.com>> If this should have a chance of being included it needs to cover mbed TLS as well. Also documentation in the man page is missing. > --- > src/openvpn/options.c | 4 +++ > src/openvpn/ssl_verify.c | 18 +++++++++++--- > src/openvpn/ssl_verify.h | 1 + > src/openvpn/ssl_verify_backend.h | 7 ++++++ > src/openvpn/ssl_verify_mbedtls.c | 11 +++++++++ > src/openvpn/ssl_verify_openssl.c | 42 ++++++++++++++++++++++++++++++++ > 6 files changed, 80 insertions(+), 3 deletions(-) > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index 173a1eea..438dfff0 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -8144,6 +8144,10 @@ add_option(struct options *options, > { > type = VERIFY_X509_SUBJECT_RDN_PREFIX; > } > + else if (streq(p[2], "subject-alt-name")) > + { > + type = VERIFY_X509_SAN; > + } > else > { > msg(msglevel, "unknown X.509 name type: %s", p[2]); > diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c > index 65188d23..6480b5eb 100644 > --- a/src/openvpn/ssl_verify.c > +++ b/src/openvpn/ssl_verify.c > @@ -390,15 +390,27 @@ verify_peer_cert(const struct tls_options *opt, > openvpn_x509_cert_t *peer_cert, > /* verify X509 name or username against --verify-x509-[user]name */ > if (opt->verify_x509_type != VERIFY_X509_NONE) > { > - if ( (opt->verify_x509_type == VERIFY_X509_SUBJECT_DN > + bool match; > + if (opt->verify_x509_type == VERIFY_X509_SAN) > + { > + bool have_alt_names; > + match = x509v3_is_host_in_alternative_names(peer_cert, > opt->verify_x509_name, &have_alt_names) > + || (!have_alt_names && > strcmp(opt->verify_x509_name, common_name) == 0); I know that this technically correct C but setting a variable in the first part of via & and then using it in the second part feels like not good style. I would rather like too a bit more verbose and less clever code that is easier to understand. > + } > + else > + { > + match = (opt->verify_x509_type == VERIFY_X509_SUBJECT_DN > && strcmp(opt->verify_x509_name, subject) == 0) > || (opt->verify_x509_type == VERIFY_X509_SUBJECT_RDN > && strcmp(opt->verify_x509_name, common_name) == 0) > || (opt->verify_x509_type == VERIFY_X509_SUBJECT_RDN_PREFIX > && strncmp(opt->verify_x509_name, common_name, > - strlen(opt->verify_x509_name)) == 0) ) > + strlen(opt->verify_x509_name)) == 0); > + } > + > + if (match) > { > - msg(D_HANDSHAKE, "VERIFY X509NAME OK: %s", subject); > + msg(D_HANDSHAKE, "VERIFY X509NAME OK: %s", > opt->verify_x509_name); This changes the log message. If you want to verify that the cert prefix with OVPN-client or something you don't get to see what certificate you accepted anymore. If you want to log opt->verify_x509_name that needs to be in addition. > } > else > { > diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h > index c54b89a6..1295e76b 100644 > --- a/src/openvpn/ssl_verify.h > +++ b/src/openvpn/ssl_verify.h > @@ -64,6 +64,7 @@ struct cert_hash_set { > #define VERIFY_X509_SUBJECT_DN 1 > #define VERIFY_X509_SUBJECT_RDN 2 > #define VERIFY_X509_SUBJECT_RDN_PREFIX 3 > +#define VERIFY_X509_SAN 4 > > #define TLS_AUTHENTICATION_SUCCEEDED 0 > #define TLS_AUTHENTICATION_FAILED 1 > diff --git a/src/openvpn/ssl_verify_backend.h > b/src/openvpn/ssl_verify_backend.h > index d6b31bfa..927a5a29 100644 > --- a/src/openvpn/ssl_verify_backend.h > +++ b/src/openvpn/ssl_verify_backend.h > @@ -268,4 +268,11 @@ result_t x509_write_pem(FILE *peercert_file, > openvpn_x509_cert_t *peercert); > */ > bool tls_verify_crl_missing(const struct tls_options *opt); > > +/** > + * Return true iff {host} was found in {cert} Subject Alternative Names > DNS or IP entries. > + * If {has_alt_names} != NULL it'll return true iff Subject Alternative > Names were defined > + * for {cert}. > + */ > +bool x509v3_is_host_in_alternative_names(openvpn_x509_cert_t *cert, > const char *host, bool *has_alt_names); > + > #endif /* SSL_VERIFY_BACKEND_H_ */ > diff --git a/src/openvpn/ssl_verify_mbedtls.c > b/src/openvpn/ssl_verify_mbedtls.c > index fd31bbbd..2f2e04be 100644 > --- a/src/openvpn/ssl_verify_mbedtls.c > +++ b/src/openvpn/ssl_verify_mbedtls.c > @@ -245,6 +245,17 @@ x509_get_subject(mbedtls_x509_crt *cert, struct > gc_arena *gc) > return subject; > } > > +bool > +x509v3_is_host_in_alternative_names(mbedtls_x509_crt *cert, const char > *host, bool *has_alt_names) > +{ > + msg(M_WARN, "Missing support for subject alternative names in > mbedtls."); > + if (has_alt_names != NULL) > + { > + *has_alt_names = false; > + } > + return false; > +} *has_alt_names = (has_alt_names != NULL);
On 12/02/2020 15:39, Arne Schwabe wrote: >> +bool >> +x509v3_is_host_in_alternative_names(mbedtls_x509_crt *cert, const char >> *host, bool *has_alt_names) >> +{ >> + msg(M_WARN, "Missing support for subject alternative names in >> mbedtls."); I'm not happy about this at all. This should be possible to achieve with mbed TLS as well: <https://tls.mbed.org/api/structmbedtls__x509__crt.html#a1f148e8fb52e03e2604e716386a07df4> One starting point for this can probably found here: <https://tls.mbed.org/api/group__x509__module.html#ga033567483649030f7f859db4f4cb7e14>
diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 173a1eea..438dfff0 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8144,6 +8144,10 @@ add_option(struct options *options, { type = VERIFY_X509_SUBJECT_RDN_PREFIX; } + else if (streq(p[2], "subject-alt-name")) + { + type = VERIFY_X509_SAN; + } else { msg(msglevel, "unknown X.509 name type: %s", p[2]); diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 65188d23..6480b5eb 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -390,15 +390,27 @@ verify_peer_cert(const struct tls_options *opt, openvpn_x509_cert_t *peer_cert, /* verify X509 name or username against --verify-x509-[user]name */ if (opt->verify_x509_type != VERIFY_X509_NONE) { - if ( (opt->verify_x509_type == VERIFY_X509_SUBJECT_DN + bool match; + if (opt->verify_x509_type == VERIFY_X509_SAN) + { + bool have_alt_names; + match = x509v3_is_host_in_alternative_names(peer_cert, opt->verify_x509_name, &have_alt_names) + || (!have_alt_names && strcmp(opt->verify_x509_name, common_name) == 0); + } + else + { + match = (opt->verify_x509_type == VERIFY_X509_SUBJECT_DN && strcmp(opt->verify_x509_name, subject) == 0) || (opt->verify_x509_type == VERIFY_X509_SUBJECT_RDN && strcmp(opt->verify_x509_name, common_name) == 0) || (opt->verify_x509_type == VERIFY_X509_SUBJECT_RDN_PREFIX && strncmp(opt->verify_x509_name, common_name, - strlen(opt->verify_x509_name)) == 0) ) + strlen(opt->verify_x509_name)) == 0); + } + + if (match) { - msg(D_HANDSHAKE, "VERIFY X509NAME OK: %s", subject); + msg(D_HANDSHAKE, "VERIFY X509NAME OK: %s", opt->verify_x509_name); } else { diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h index c54b89a6..1295e76b 100644 --- a/src/openvpn/ssl_verify.h +++ b/src/openvpn/ssl_verify.h @@ -64,6 +64,7 @@ struct cert_hash_set { #define VERIFY_X509_SUBJECT_DN 1 #define VERIFY_X509_SUBJECT_RDN 2 #define VERIFY_X509_SUBJECT_RDN_PREFIX 3 +#define VERIFY_X509_SAN 4 #define TLS_AUTHENTICATION_SUCCEEDED 0 #define TLS_AUTHENTICATION_FAILED 1 diff --git a/src/openvpn/ssl_verify_backend.h b/src/openvpn/ssl_verify_backend.h index d6b31bfa..927a5a29 100644 --- a/src/openvpn/ssl_verify_backend.h +++ b/src/openvpn/ssl_verify_backend.h @@ -268,4 +268,11 @@ result_t x509_write_pem(FILE *peercert_file, openvpn_x509_cert_t *peercert); */ bool tls_verify_crl_missing(const struct tls_options *opt); +/** + * Return true iff {host} was found in {cert} Subject Alternative Names DNS or IP entries. + * If {has_alt_names} != NULL it'll return true iff Subject Alternative Names were defined + * for {cert}. + */ +bool x509v3_is_host_in_alternative_names(openvpn_x509_cert_t *cert, const char *host, bool *has_alt_names); + #endif /* SSL_VERIFY_BACKEND_H_ */ diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index fd31bbbd..2f2e04be 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -245,6 +245,17 @@ x509_get_subject(mbedtls_x509_crt *cert, struct gc_arena *gc) return subject; } +bool +x509v3_is_host_in_alternative_names(mbedtls_x509_crt *cert, const char *host, bool *has_alt_names) +{ + msg(M_WARN, "Missing support for subject alternative names in mbedtls."); + if (has_alt_names != NULL) + { + *has_alt_names = false; + } + return false; +} + static void do_setenv_x509(struct env_set *es, const char *name, char *value, int depth) { diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c index ff14db23..bb639abc 100644 --- a/src/openvpn/ssl_verify_openssl.c +++ b/src/openvpn/ssl_verify_openssl.c @@ -364,6 +364,48 @@ err: return subject; } +bool +x509v3_is_host_in_alternative_names(X509 *cert, const char *host, bool *has_alt_names) +{ + GENERAL_NAMES* altnames = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL); + if (has_alt_names != NULL) + { + *has_alt_names = altnames != NULL; + } + if (altnames == NULL) + { + return false; + } + + int n = sk_GENERAL_NAME_num(altnames); + for (int i = 0; i < n; i++) + { + GENERAL_NAME* altname = sk_GENERAL_NAME_value(altnames, i); + ASN1_STRING *altname_asn1 = NULL; + if (altname->type == GEN_DNS) + { + altname_asn1 = altname->d.dNSName; + } + else if (altname->type == GEN_IPADD) + { + altname_asn1 = altname->d.iPAddress; + } + + if (altname_asn1 != NULL) + { + char* altname_cstr = NULL; + if (ASN1_STRING_to_UTF8((unsigned char **)&altname_cstr, altname_asn1) >= 0) { + bool match = strcmp(host, altname_cstr) == 0; + OPENSSL_free(altname_cstr); + if (match) + { + return true; + } + } + } + } + return false; +} /* * x509-track implementation -- save X509 fields to environment,
when using "--verify-x509-name [hostname] name" hostname will now be accepted also when matched against one of the X509v3 Subject Alternative Name IP or DNS entries (instead of just Subject's CN). see also: https://github.com/OpenVPN/openvpn/pull/136/ Signed-off-by: Mateusz Markowicz <poniekad@protonmail.com> --- src/openvpn/options.c | 4 +++ src/openvpn/ssl_verify.c | 18 +++++++++++--- src/openvpn/ssl_verify.h | 1 + src/openvpn/ssl_verify_backend.h | 7 ++++++ src/openvpn/ssl_verify_mbedtls.c | 11 +++++++++ src/openvpn/ssl_verify_openssl.c | 42 ++++++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+), 3 deletions(-) -- 2.20.1