From: Arran Cudbard-Bell Date: Thu, 7 Oct 2021 18:38:02 +0000 (-0500) Subject: Re-arrange certificate verification code so we check the chain certs too. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8aafb5ddad2f96fc76906e5341ab4dcdd9d49a84;p=thirdparty%2Ffreeradius-server.git Re-arrange certificate verification code so we check the chain certs too. --- diff --git a/src/lib/tls/ctx.c b/src/lib/tls/ctx.c index 63c4bf4c400..778dff7239d 100644 --- a/src/lib/tls/ctx.c +++ b/src/lib/tls/ctx.c @@ -125,6 +125,121 @@ static int ctx_dh_params_load(SSL_CTX *ctx, char *file) return 0; } +static int tls_ctx_verify_chain_member(fr_unix_time_t *expires_first, X509 **self_signed, + SSL_CTX *ctx, X509 *to_verify, + fr_tls_chain_verify_mode_t verify_mode) +{ + fr_unix_time_t not_after; + + STACK_OF(X509) *chain; + X509 *leaf; + + leaf = SSL_CTX_get0_certificate(ctx); + if (!leaf) { + ERROR("Chain does not contain a valid leaf certificate"); + return -1; + } + + if (!SSL_CTX_get0_chain_certs(ctx, &chain)) { + fr_tls_log_error(NULL, "Failed retrieving chain certificates"); + return -1; + } + + switch (fr_tls_cert_is_valid(NULL, ¬_after, to_verify)) { + case -1: + fr_tls_log_certificate_chain_marker(NULL, L_ERR, chain, leaf, to_verify); + PERROR("Malformed certificate"); + return -1; + + case -2: + case -3: + switch (verify_mode) { + case FR_TLS_CHAIN_VERIFY_SOFT: + fr_tls_log_certificate_chain_marker(NULL, L_WARN, chain, leaf, to_verify); + PWARN("Certificate validation failed"); + break; + + case FR_TLS_CHAIN_VERIFY_HARD: + fr_tls_log_certificate_chain_marker(NULL, L_ERR, chain, leaf, to_verify); + PERROR("Certificate validation failed"); + return -1; + + default: + break; + } + + } + + /* + * Check for self-signed certs + */ + switch (verify_mode) { + case FR_TLS_CHAIN_VERIFY_SOFT: + case FR_TLS_CHAIN_VERIFY_HARD: + /* + * There can be only one... self signed + * cert in a chain. + * + * We have to do this check manually + * because the OpenSSL functions will + * only check to see if it can build + * a chain, not that all certificates + * in the chain are used. + * + * Having multiple self-signed certificates + * usually indicates someone has copied + * the wrong certificates into the + * server.pem file. + */ + if (X509_name_cmp(X509_get_subject_name(to_verify), + X509_get_issuer_name(to_verify)) == 0) { + if (*self_signed) { + switch (verify_mode) { + case FR_TLS_CHAIN_VERIFY_SOFT: + WARN("Found multiple self-signed certificates in chain"); + WARN("First certificate was:"); + fr_tls_log_certificate_chain_marker(NULL, L_WARN, + chain, leaf, *self_signed); + + WARN("Second certificate was:"); + fr_tls_log_certificate_chain_marker(NULL, L_WARN, + chain, leaf, to_verify); + break; + + case FR_TLS_CHAIN_VERIFY_HARD: + ERROR("Found multiple self-signed certificates in chain"); + ERROR("First certificate was:"); + fr_tls_log_certificate_chain_marker(NULL, L_ERR, + chain, leaf, *self_signed); + + ERROR("Second certificate was:"); + fr_tls_log_certificate_chain_marker(NULL, L_WARN, + chain, leaf, to_verify); + return -1; + + default: + break; + } + } + *self_signed = to_verify; + } + break; + + default: + break; + } + + /* + * Record the time the first certificate in + * the chain expires so we can use it for + * runtime checks. + */ + if (!fr_unix_time_ispos(*expires_first) || + (fr_unix_time_gt(*expires_first, not_after))) *expires_first = not_after; + + return 0; +} + static int tls_ctx_load_cert_chain(SSL_CTX *ctx, fr_tls_chain_conf_t *chain) { char *password; @@ -243,114 +358,31 @@ static int tls_ctx_load_cert_chain(SSL_CTX *ctx, fr_tls_chain_conf_t *chain) */ { fr_unix_time_t expires_first = fr_unix_time_wrap(0); - int ret; X509 *self_signed = NULL; + STACK_OF(X509) *our_chain; + int i; - for (ret = SSL_CTX_set_current_cert(ctx, SSL_CERT_SET_FIRST); - ret == 1; - ret = SSL_CTX_set_current_cert(ctx, SSL_CERT_SET_NEXT)) { - fr_unix_time_t not_after; - STACK_OF(X509) *our_chain; - X509 *our_cert; - - our_cert = SSL_CTX_get0_certificate(ctx); - if (!SSL_CTX_get0_chain_certs(ctx, &our_chain)) { - fr_tls_log_error(NULL, "Failed retrieving chain certificates"); - return -1; - } - - switch (fr_tls_cert_is_valid(NULL, ¬_after, our_cert)) { - case -1: - fr_tls_log_certificate_chain(NULL, L_ERR, our_chain, our_cert); - PERROR("Malformed certificate"); - return -1; - - case -2: - case -3: - switch (chain->verify_mode) { - case FR_TLS_CHAIN_VERIFY_SOFT: - fr_tls_log_certificate_chain(NULL, L_WARN, our_chain, our_cert); - PWARN("Certificate validation failed"); - break; - - case FR_TLS_CHAIN_VERIFY_HARD: - fr_tls_log_certificate_chain(NULL, L_ERR, our_chain, our_cert); - PERROR("Certificate validation failed"); - return -1; - - default: - break; - } - - } - - /* - * Check for self-signed certs - */ - switch (chain->verify_mode) { - case FR_TLS_CHAIN_VERIFY_SOFT: - case FR_TLS_CHAIN_VERIFY_HARD: - /* - * There can be only one... self signed - * cert in a chain. - * - * We have to do this check manually - * because the OpenSSL functions will - * only check to see if it can build - * a chain, not that all certificates - * in the chain are used. - * - * Having multiple self-signed certificates - * usually indicates someone has copied - * the wrong certificates into the - * server.pem file. - */ - if (X509_name_cmp(X509_get_subject_name(our_cert), - X509_get_issuer_name(our_cert)) == 0) { - if (self_signed) { - switch (chain->verify_mode) { - case FR_TLS_CHAIN_VERIFY_SOFT: - WARN("Found multiple self-signed certificates in chain"); - WARN("First certificate was:"); - fr_tls_log_certificate_chain(NULL, L_WARN, - our_chain, self_signed); - - WARN("Second certificate was:"); - fr_tls_log_certificate_chain(NULL, L_WARN, - our_chain, our_cert); - break; - - case FR_TLS_CHAIN_VERIFY_HARD: - ERROR("Found multiple self-signed certificates in chain"); - ERROR("First certificate was:"); - fr_tls_log_certificate_chain(NULL, L_ERR, - our_chain, self_signed); - - ERROR("Second certificate was:"); - fr_tls_log_certificate_chain(NULL, L_ERR, - our_chain, our_cert); - return -1; - - default: - break; - } - } - self_signed = our_cert; - } - break; + if (tls_ctx_verify_chain_member(&expires_first, &self_signed, + ctx, SSL_CTX_get0_certificate(ctx), + chain->verify_mode) < 0) return -1; - default: - break; - } + if (!SSL_CTX_get0_chain_certs(ctx, &our_chain)) { + fr_tls_log_error(NULL, "Failed retrieving chain certificates"); + return -1; + } +DIAG_OFF(used-but-marked-unused) /* fix spurious warnings for sk macros */ + for (i = sk_X509_num(our_chain); i > 0 ; i--) { /* - * Record the time the first certificate in - * the chain expires so we can use it for - * runtime checks. + * SSL_CTX_use_certificate_chain_file set the + * current cert to be the one loaded from + * that pem file. */ - if (!fr_unix_time_ispos(expires_first) || - (fr_unix_time_gt(expires_first, not_after))) expires_first = not_after; + if (tls_ctx_verify_chain_member(&expires_first, &self_signed, + ctx, sk_X509_value(our_chain, i - 1), + chain->verify_mode) < 0) return -1; } +DIAG_ON(used-but-marked-unused) /* fix spurious warnings for sk macros */ /* * Record this as a unix timestamp as diff --git a/src/lib/tls/log.c b/src/lib/tls/log.c index 049ece4d17e..4b82ae2eb94 100644 --- a/src/lib/tls/log.c +++ b/src/lib/tls/log.c @@ -92,6 +92,27 @@ static void _tls_ctx_print_cert_line(char const *file, int line, } } + +static void _tls_ctx_print_cert_line_marker(char const *file, int line, + request_t *request, fr_log_type_t log_type, int idx, + X509 *cert, bool marker) +{ + char subject[1024]; + + X509_NAME_oneline(X509_get_subject_name(cert), subject, sizeof(subject)); + subject[sizeof(subject) - 1] = '\0'; + + if (request) { + log_request(log_type, fr_debug_lvl, request, file, line, + "%s [%i] %s %s", marker ? ">" : " ", + idx, fr_tls_utils_x509_pkey_type(cert), subject); + } else { + fr_log(LOG_DST, log_type, file, line, + "%s [%i] %s %s", marker ? ">" : " ", + idx, fr_tls_utils_x509_pkey_type(cert), subject); + } +} + DIAG_OFF(DIAG_UNKNOWN_PRAGMAS) DIAG_OFF(used-but-marked-unused) /* fix spurious warnings for sk macros */ /** Print out the current stack of certs @@ -113,6 +134,29 @@ void _fr_tls_log_certificate_chain(char const *file, int line, } _tls_ctx_print_cert_line(file, line, request, log_type, i, cert); } + +/** Print out the current stack of certs + * + * @param[in] file File where this function is being called. + * @param[in] line Line where this function is being called. + * @param[in] request Current request, may be NULL. + * @param[in] log_type The type of log message to produce L_INFO, L_ERR, L_DBG etc... + * @param[in] chain The certificate chain. + * @param[in] cert The leaf certificate. + * @param[in] marker The certificate we want to mark. + */ +void _fr_tls_log_certificate_chain_marker(char const *file, int line, + request_t *request, fr_log_type_t log_type, + STACK_OF(X509) *chain, X509 *cert, X509 *marker) +{ + int i; + + for (i = sk_X509_num(chain); i > 0 ; i--) { + X509 *selected = sk_X509_value(chain, i - 1); + _tls_ctx_print_cert_line_marker(file, line, request, log_type, i, selected, (selected == marker)); + } + _tls_ctx_print_cert_line_marker(file, line, request, log_type, i, cert, (cert == marker)); +} DIAG_ON(used-but-marked-unused) DIAG_ON(DIAG_UNKNOWN_PRAGMAS) diff --git a/src/lib/tls/log.h b/src/lib/tls/log.h index 95aefe7ebfb..8a0f540ee50 100644 --- a/src/lib/tls/log.h +++ b/src/lib/tls/log.h @@ -34,10 +34,17 @@ RCSIDH(tls_log_h, "$Id$") #include "base.h" -#define fr_tls_log_certificate_chain(_request, _log_type, _chain, _cert) \ - _fr_tls_log_certificate_chain( __FILE__, __LINE__, _request, _log_type, _chain, _cert) +#define fr_tls_log_certificate_chain(_request, _log_type, _chain, _leaf) \ + _fr_tls_log_certificate_chain( __FILE__, __LINE__, _request, _log_type, _chain, _leaf) void _fr_tls_log_certificate_chain(char const *file, int line, - request_t *request, fr_log_type_t log_type, STACK_OF(X509) *chain, X509 *cert); + request_t *request, fr_log_type_t log_type, STACK_OF(X509) *chain, X509 *leaf); + +void _fr_tls_log_certificate_chain_marker(char const *file, int line, + request_t *request, fr_log_type_t log_type, STACK_OF(X509) *chain, + X509 *leaf, X509 *marker); + +#define fr_tls_log_certificate_chain_marker(_request, _log_type, _chain, _leaf, _marker) \ + _fr_tls_log_certificate_chain_marker( __FILE__, __LINE__, _request, _log_type, _chain, _leaf, _marker) int fr_tls_log_io_error(request_t *request, int err, char const *msg, ...) CC_HINT(format (printf, 3, 4));