From fca1eecb2da9e85d55cc459eccea96ba7968a8c1 Mon Sep 17 00:00:00 2001 From: Jeroen Koekkoek Date: Mon, 29 Jul 2024 14:25:25 +0200 Subject: [PATCH] Implement fallback to ASN1_STRING_data Fixes #359. --- configure.ac | 32 +++---- options.c | 247 +++++++++++++++++++++++++++------------------------ options.h | 2 +- 3 files changed, 149 insertions(+), 132 deletions(-) diff --git a/configure.ac b/configure.ac index e623fdf10..88059c113 100644 --- a/configure.ac +++ b/configure.ac @@ -359,11 +359,11 @@ AC_DEFUN([CHECK_SSL], [ ]) if test x_$withval != x_no; then AC_MSG_CHECKING(for SSL) - if test -n "$withval"; then - dnl look for openssl install with different version, eg. - dnl in /usr/include/openssl11/openssl/ssl.h - dnl and /usr/lib64/openssl11/libssl.so - dnl with the --with-ssl=/usr/include/openssl11 + if test -n "$withval"; then + dnl look for openssl install with different version, eg. + dnl in /usr/include/openssl11/openssl/ssl.h + dnl and /usr/lib64/openssl11/libssl.so + dnl with the --with-ssl=/usr/include/openssl11 if test ! -f "$withval/include/openssl/ssl.h" -a -f "$withval/openssl/ssl.h"; then ssldir="$withval" found_ssl="yes" @@ -383,7 +383,7 @@ AC_DEFUN([CHECK_SSL], [ fi fi fi - fi + fi if test x_$withval = x_ -o x_$withval = x_yes; then withval="/usr/local/ssl /usr/lib/ssl /usr/ssl /usr/pkg /usr/sfw /usr/local /usr /usr/local/opt/openssl" fi @@ -394,12 +394,12 @@ AC_DEFUN([CHECK_SSL], [ if test x_$ssldir != x_/usr; then CPPFLAGS="$CPPFLAGS -I$ssldir/include"; fi - ssldir_include="$ssldir/include" - if test ! -d "$ssldir/lib" -a -d "$ssldir/lib64"; then - ssldir_lib="$ssldir/lib64" - else - ssldir_lib="$ssldir/lib" - fi + ssldir_include="$ssldir/include" + if test ! -d "$ssldir/lib" -a -d "$ssldir/lib64"; then + ssldir_lib="$ssldir/lib64" + else + ssldir_lib="$ssldir/lib" + fi break; fi done @@ -412,9 +412,9 @@ AC_DEFUN([CHECK_SSL], [ if test x_$ssldir != x_/usr; then LDFLAGS="$LDFLAGS -L$ssldir_lib"; fi - if test x_$ssldir = x_/usr/sfw; then - LDFLAGS="$LDFLAGS -R$ssldir_lib"; - fi + if test x_$ssldir = x_/usr/sfw; then + LDFLAGS="$LDFLAGS -R$ssldir_lib"; + fi fi AC_SUBST(HAVE_SSL) fi @@ -1048,7 +1048,7 @@ if test x$HAVE_SSL = x"yes"; then SSL_LIBS="-lssl" AC_SUBST(SSL_LIBS) AC_CHECK_HEADERS([openssl/ssl.h openssl/err.h openssl/rand.h openssl/ocsp.h openssl/core_names.h openssl/x509v3.h],,, [AC_INCLUDES_DEFAULT]) - AC_CHECK_FUNCS([HMAC_CTX_reset HMAC_CTX_new EVP_cleanup ERR_load_crypto_strings OPENSSL_init_crypto CRYPTO_memcmp EC_KEY_new_by_curve_name EVP_MAC_CTX_new EVP_MAC_CTX_set_params EVP_MAC_CTX_get_mac_size SHA1_Init]) + AC_CHECK_FUNCS([HMAC_CTX_reset HMAC_CTX_new EVP_cleanup ERR_load_crypto_strings OPENSSL_init_crypto CRYPTO_memcmp EC_KEY_new_by_curve_name EVP_MAC_CTX_new EVP_MAC_CTX_set_params EVP_MAC_CTX_get_mac_size SHA1_Init ASN1_STRING_get0_data]) if test "$ac_cv_func_SHA1_Init" = "yes"; then ACX_FUNC_DEPRECATED([SHA1_Init], [(void)SHA1_Init(NULL);], [ #include diff --git a/options.c b/options.c index 9af1e1eea..9db0d3987 100644 --- a/options.c +++ b/options.c @@ -1983,18 +1983,15 @@ acl_check_incoming(struct acl_options* acl, struct query* q, if (acl->tls_auth_name && q->tls_auth) { /* we have auth_domain_name in tls_auth */ if (acl->tls_auth_options && acl->tls_auth_options->auth_domain_name) { - q->cert_cn = NULL; - if (!acl_tls_hostname_matches(q->tls_auth, acl->tls_auth_options->auth_domain_name, &(q->cert_cn))) { + if (!acl_tls_hostname_matches(q->tls_auth, acl->tls_auth_options->auth_domain_name)) { VERBOSITY(3, (LOG_WARNING, - "client cert %s does not match %s %s", - (q->cert_cn?q->cert_cn:"(null)"), acl->tls_auth_name, acl->tls_auth_options->auth_domain_name)); - free(q->cert_cn); + "client cert does not match %s %s", + acl->tls_auth_name, acl->tls_auth_options->auth_domain_name)); q->cert_cn = NULL; return -1; } VERBOSITY(5, (LOG_INFO, "%s %s verified", acl->tls_auth_name, acl->tls_auth_options->auth_domain_name)); - free(q->cert_cn); q->cert_cn = acl->tls_auth_options->auth_domain_name; } else { /* nsd gives error on start for this, but check just in case */ @@ -2206,36 +2203,144 @@ acl_addr_match_range_v6(uint32_t* minval, uint32_t* x, uint32_t* maxval, size_t #endif /* INET6 */ #ifdef HAVE_SSL -int -acl_tls_hostname_matches(SSL* tls_auth, const char* acl_cert_cn, char** cert_cn) +/* Code in for matches_subject_alternative_name and matches_common_name + * functions is from https://wiki.openssl.org/index.php/Hostname_validation + * with modifications. + * + * Obtained from: https://github.com/iSECPartners/ssl-conservatory + * Copyright (C) 2012, iSEC Partners. + * License: MIT License + * Author: Alban Diquet + */ +static int matches_subject_alternative_name( + const char *acl_cert_cn, size_t acl_cert_cn_len, const X509 *cert) { - int i; + int result = 0; int san_names_nb = -1; STACK_OF(GENERAL_NAME) *san_names = NULL; + /* Try to extract the names within the SAN extension from the certificate */ + san_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL); + if (san_names == NULL) + return 0; + + san_names_nb = sk_GENERAL_NAME_num(san_names); + + /* Check each name within the extension */ + for (int i = 0; i < san_names_nb && !result; i++) { + int len; + const char *str; + const GENERAL_NAME *current_name = sk_GENERAL_NAME_value(san_names, i); + /* Skip non-DNS SAN entries. */ + if (current_name->type != GEN_DNS) + continue; +#if HAVE_ASN1_STRING_GET0_DATA + str = (const char *)ASN1_STRING_get0_data(current_name->d.dNSName); +#else + str = (const char *)ASN1_STRING_data(current_name->d.dNSName); +#endif + if (str == NULL) + continue; + len = ASN1_STRING_length(current_name->d.dNSName); + if (acl_cert_cn_len == (size_t)len && + strncasecmp(str, acl_cert_cn, len) == 0) + { + result = 1; + } else { + /* Make sure there isn't an embedded NUL character in the DNS name */ + /* From the man page: In general it cannot be assumed that the data + * returned by ASN1_STRING_data() is null terminated or does not + * contain embedded nulls. */ + int pos = 0; + while (pos < len && str[pos] != 0) + pos++; + if (pos == len) { + DEBUG(DEBUG_XFRD, 2, (LOG_INFO, + "SAN %*s does not match acl for %s", len, str, acl_cert_cn)); + } else { + DEBUG(DEBUG_XFRD, 2, (LOG_INFO, "Malformed SAN in certificate")); + break; + } + } + } + sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free); + + return result; +} + +static int matches_common_name( + const char *acl_cert_cn, size_t acl_cert_cn_len, const X509 *cert) +{ + int len; int common_name_loc = -1; - char *common_name_str = NULL; + const char *common_name_str = NULL; X509_NAME *subject_name = NULL; X509_NAME_ENTRY *common_name_entry = NULL; ASN1_STRING *common_name_asn1 = NULL; - X509 *client_cert; - *cert_cn = NULL; + if ((subject_name = X509_get_subject_name(cert)) == NULL) + return 0; -# ifdef HAVE_SSL_GET1_PEER_CERTIFICATE - if ((client_cert = SSL_get1_peer_certificate(tls_auth)) == NULL) { -# else - if ((client_cert = SSL_get_peer_certificate(tls_auth)) == NULL) { -# endif - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "CN match fail no peer certificate")); - X509_free(client_cert); + /* Find the position of the CN field in the Subject field of the certificate */ + common_name_loc = X509_NAME_get_index_by_NID(subject_name, NID_commonName, -1); + if (common_name_loc < 0) return 0; - } - if (!acl_cert_cn || !client_cert) { - X509_free(client_cert); + + /* Extract the CN field */ + common_name_entry = X509_NAME_get_entry(subject_name, common_name_loc); + if (common_name_entry == NULL) + return 0; + + /* Convert the CN field to a C string */ + common_name_asn1 = X509_NAME_ENTRY_get_data(common_name_entry); + if (common_name_asn1 == NULL) return 0; + +#if HAVE_ASN1_STRING_GET0_DATA + common_name_str = (const char *)ASN1_STRING_get0_data(common_name_asn1); +#else + common_name_str = (const char *)ASN1_STRING_data(common_name_asn1); +#endif + + len = ASN1_STRING_length(common_name_asn1); + if (acl_cert_cn_len == (size_t)len && + strncasecmp(acl_cert_cn, common_name_str, len) == 0) + { + return 1; + } else { + /* Make sure there isn't an embedded NUL character in the CN */ + int pos = 0; + while (pos < len && common_name_str[pos] != 0) + pos++; + if (pos == len) { + DEBUG(DEBUG_XFRD, 2, (LOG_INFO, + "CN %*s does not match acl for %s", len, common_name_str, acl_cert_cn)); + } else { + DEBUG(DEBUG_XFRD, 2, (LOG_INFO, "Malformed CN in certificate")); + } } + return 0; +} + +int +acl_tls_hostname_matches(SSL* tls_auth, const char *acl_cert_cn) +{ + int result = 0; + size_t acl_cert_cn_len; + X509 *client_cert; + + assert(acl_cert_cn != NULL); + +#ifdef HAVE_SSL_GET1_PEER_CERTIFICATE + client_cert = SSL_get1_peer_certificate(tls_auth); +#else + client_cert = SSL_get_peer_certificate(tls_auth); +#endif + + if (client_cert == NULL) + return 0; + /* OpenSSL provides functions for hostname checking from certificate * Following code should work but it doesn't. * Keep it for future test in order to not use custom code @@ -2248,102 +2353,14 @@ acl_tls_hostname_matches(SSL* tls_auth, const char* acl_cert_cn, char** cert_cn) * const char *peername = X509_VERIFY_PARAM_get0_peername(vpm); // NOT ok */ -/* Code in rest of function is from - * https://wiki.openssl.org/index.php/Hostname_validation - * with modifications - * - * Obtained from: https://github.com/iSECPartners/ssl-conservatory - * Copyright (C) 2012, iSEC Partners. - * License: MIT License - * Author: Alban Diquet - */ - + acl_cert_cn_len = strlen(acl_cert_cn); /* semi follow RFC6125#section-6.4.4 check SAN DNS first */ + if (!(result = matches_subject_alternative_name(acl_cert_cn, acl_cert_cn_len, client_cert))) + result = matches_common_name(acl_cert_cn, acl_cert_cn_len, client_cert); - /* Try to extract the names within the SAN extension from the certificate */ - san_names = X509_get_ext_d2i(client_cert, NID_subject_alt_name, NULL, NULL); - if (san_names != NULL) { - san_names_nb = sk_GENERAL_NAME_num(san_names); - - /* Check each name within the extension */ - for (i = 0; i < san_names_nb; i++) { - char* dns_name; - const GENERAL_NAME *current_name = sk_GENERAL_NAME_value(san_names, i); - /* skip non-DNS SAN entries */ - if (current_name->type != GEN_DNS) - continue; - dns_name = (char *) ASN1_STRING_get0_data(current_name->d.dNSName); - if (dns_name == NULL) - continue; - /* Make sure there isn't an embedded NUL character in the DNS name */ - if ((size_t)ASN1_STRING_length(current_name->d.dNSName) != strlen(dns_name)) { - DEBUG(DEBUG_XFRD,2, (LOG_WARNING, "SAN Malformed certificate")); - break; - } - if (dns_name != NULL) { - /* certificate SAN DNS match */ - if (strcasecmp(dns_name, acl_cert_cn)==0) { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "SAN %s matches acl", dns_name)); - *cert_cn = strdup(dns_name); - X509_free(client_cert); - return 1; - } else { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "SAN %s does not match acl", dns_name)); - /* check with remaining SANs and then continue with normal CN check */ - } - } - } - sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free); - } else { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "No SAN DNS extension in certificate")); - } - - /* no match on SAN, continue with normal CN */ - if ((subject_name = X509_get_subject_name(client_cert)) == NULL) { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "CN get subject failed")); - X509_free(client_cert); - return 0; - } - /* Find the position of the CN field in the Subject field of the certificate */ - common_name_loc = X509_NAME_get_index_by_NID(subject_name, NID_commonName, -1); - if (common_name_loc < 0) { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "CN not found in Subject")); - X509_free(client_cert); - return 0; - } - /* Extract the CN field */ - common_name_entry = X509_NAME_get_entry(subject_name, common_name_loc); - if (common_name_entry == NULL) { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "CN extraction failed")); - X509_free(client_cert); - return 0; - } - /* Convert the CN field to a C string */ - common_name_asn1 = X509_NAME_ENTRY_get_data(common_name_entry); - if (common_name_asn1 == NULL) { - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "CN name to string convertion failed")); - X509_free(client_cert); - return 0; - } - common_name_str = (char *) ASN1_STRING_get0_data(common_name_asn1); - /* Make sure there isn't an embedded NUL character in the CN */ - if ((size_t)ASN1_STRING_length(common_name_asn1) != strlen(common_name_str)) { - DEBUG(DEBUG_XFRD,2, (LOG_WARNING, "CN Malformed certificate")); - X509_free(client_cert); - return 0; - } - if (common_name_str != NULL) { - /* store it for logging */ - *cert_cn = strdup(common_name_str); - /* certificate CN match */ - if (strcasecmp(common_name_str, acl_cert_cn)==0) { - X509_free(client_cert); - return 1; - } - } - DEBUG(DEBUG_XFRD,2, (LOG_INFO, "CN from cert %s does not match acl", (common_name_str?common_name_str:"(null)"))); X509_free(client_cert); - return 0; + + return result; } #endif diff --git a/options.h b/options.h index 80879dd71..02111890a 100644 --- a/options.h +++ b/options.h @@ -565,7 +565,7 @@ int acl_addr_matches_host(struct acl_options* acl, struct acl_options* host); int acl_addr_matches(struct acl_options* acl, struct query* q); int acl_addr_matches_proxy(struct acl_options* acl, struct query* q); #ifdef HAVE_SSL -int acl_tls_hostname_matches(SSL* ssl, const char* acl_cert_cn, char** cert_cn); +int acl_tls_hostname_matches(SSL* ssl, const char* acl_cert_cn); #endif int acl_key_matches(struct acl_options* acl, struct query* q); int acl_addr_match_mask(uint32_t* a, uint32_t* b, uint32_t* mask, size_t sz);