From: Arran Cudbard-Bell Date: Fri, 8 Oct 2021 12:44:01 +0000 (-0500) Subject: Do a better job of syncing up when we need to call the verification server X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7ce6202ef08dd2a7f9038ee3e29c0ced792ace3e;p=thirdparty%2Ffreeradius-server.git Do a better job of syncing up when we need to call the verification server Previously if chain validation failed we _wouldn't_ generate attributes, but we _would_ call the virtual server. --- diff --git a/src/lib/tls/cache.c b/src/lib/tls/cache.c index 2c5c998e8b1..f465a96b237 100644 --- a/src/lib/tls/cache.c +++ b/src/lib/tls/cache.c @@ -763,7 +763,7 @@ again: * the code there knows what job it needs to push onto * the unlang stack. */ - fr_tls_verify_client_cert_request(tls_session, true); + fr_tls_verify_cert_request(tls_session, true); ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */ @@ -775,7 +775,7 @@ again: */ if (unlang_request_is_cancelled(request)) { tls_cache_load_state_reset(tls_cache); /* Clears any loaded session data */ - fr_tls_verify_client_cert_reset(tls_session); + fr_tls_verify_cert_reset(tls_session); return NULL; } @@ -784,7 +784,7 @@ again: * If we couldn't validate the client certificate * then validation overall fails. */ - if (!fr_tls_verify_client_cert_result(tls_session)) { + if (!fr_tls_verify_cert_result(tls_session)) { RDEBUG2("Certificate re-validation failed, denying session resumption via session-id"); goto verify_error; } @@ -1113,7 +1113,7 @@ static SSL_TICKET_RETURN tls_cache_session_ticket_app_data_get(SSL *ssl, SSL_SES * the code there knows what job it needs to push onto * the unlang stack. */ - fr_tls_verify_client_cert_request(tls_session, true); + fr_tls_verify_cert_request(tls_session, true); ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */ @@ -1122,7 +1122,7 @@ static SSL_TICKET_RETURN tls_cache_session_ticket_app_data_get(SSL *ssl, SSL_SES * a known state. */ if (unlang_request_is_cancelled(request)) { - fr_tls_verify_client_cert_reset(tls_session); + fr_tls_verify_cert_reset(tls_session); return SSL_TICKET_RETURN_ABORT; } @@ -1131,7 +1131,7 @@ static SSL_TICKET_RETURN tls_cache_session_ticket_app_data_get(SSL *ssl, SSL_SES * give the client the opportunity to send a new * one, but _don't_ allow session resumption. */ - if (!fr_tls_verify_client_cert_result(tls_session)) { + if (!fr_tls_verify_cert_result(tls_session)) { RDEBUG2("Certificate re-validation failed, denying session resumption via session-ticket"); return SSL_TICKET_RETURN_IGNORE_RENEW; } diff --git a/src/lib/tls/session.c b/src/lib/tls/session.c index 8d9a8359593..cea83f39684 100644 --- a/src/lib/tls/session.c +++ b/src/lib/tls/session.c @@ -1378,7 +1378,7 @@ DIAG_ON(used-but-marked-unused) * Next service any pending certificate * validation actions. */ - ua = fr_tls_verify_client_cert_pending_push(request, tls_session); + ua = fr_tls_verify_cert_pending_push(request, tls_session); switch (ua) { case UNLANG_ACTION_FAIL: if (unlang_function_clear(request) < 0) goto error; diff --git a/src/lib/tls/verify.c b/src/lib/tls/verify.c index d494cdb745a..81272784531 100644 --- a/src/lib/tls/verify.c +++ b/src/lib/tls/verify.c @@ -83,7 +83,7 @@ DIAG_OFF(used-but-marked-unused) /* fix spurious warnings for sk macros */ * certificate chain. * * @note As a byproduct of validation, various OIDs will be extracted from the - * certificates, and inserted into the session-state. list as fr_pair_t. + * certificates, and inserted into the session-state list as fr_pair_t. * * @param ok preverify ok. 1 if true, 0 if false. * @param x509_ctx containing certs to verify. @@ -215,10 +215,11 @@ int fr_tls_verify_cert_cb(int ok, X509_STORE_CTX *x509_ctx) } done: /* - * This is a client cert, call our - * virtual server here. + * If verification hasn't already failed + * and we're meant to verify this cert + * then call the virtual server. */ - if (depth == 0) { + if (my_ok && verify_applies(conf->verify.pair_mode, depth, untrusted)) { if (conf->virtual_server && tls_session->verify_client_cert) { RDEBUG2("Requesting certificate validation"); @@ -230,7 +231,7 @@ done: * the code there knows what job it needs to push onto * the unlang stack. */ - fr_tls_verify_client_cert_request(tls_session, SSL_session_reused(tls_session->ssl)); + fr_tls_verify_cert_request(tls_session, SSL_session_reused(tls_session->ssl)); ASYNC_pause_job(); /* Jumps back to SSL_read() in session.c */ @@ -239,26 +240,26 @@ done: */ if (unlang_request_is_cancelled(request)) { X509_STORE_CTX_set_error(x509_ctx, 0); - fr_tls_verify_client_cert_reset(tls_session); + fr_tls_verify_cert_reset(tls_session); return 1; } /* - * If we couldn't validate the client certificate + * If we couldn't validate the certificate * then validation overall fails. */ - if (!fr_tls_verify_client_cert_result(tls_session)) { + if (!fr_tls_verify_cert_result(tls_session)) { REDEBUG("Certificate validation failed"); my_ok = 0; X509_STORE_CTX_set_error(x509_ctx, X509_V_ERR_APPLICATION_VERIFICATION); } } - - tls_session->client_cert_ok = (my_ok > 0); - RDEBUG2("[verify] = %s", my_ok ? "ok" : "invalid"); } + tls_session->client_cert_ok = (my_ok > 0); + RDEBUG2("[verify] = %s", my_ok ? "ok" : "invalid"); + return my_ok; } DIAG_ON(used-but-marked-unused) @@ -276,7 +277,7 @@ DIAG_ON(DIAG_UNKNOWN_PRAGMAS) * - 1 if the chain could be validated. * - 0 if the chain failed validation. */ -int fr_tls_verify_client_cert_chain(request_t *request, SSL *ssl) +int fr_tls_verify_cert_chain(request_t *request, SSL *ssl) { int err; int verify; @@ -405,7 +406,7 @@ static unlang_action_t tls_verify_client_cert_push(request_t *request, fr_tls_se * - true if the certificate chain was validated. * - false if the certificate chain failed validation. */ -bool fr_tls_verify_client_cert_result(fr_tls_session_t *tls_session) +bool fr_tls_verify_cert_result(fr_tls_session_t *tls_session) { bool result; @@ -422,7 +423,7 @@ bool fr_tls_verify_client_cert_result(fr_tls_session_t *tls_session) /** Reset the verification state * */ -void fr_tls_verify_client_cert_reset(fr_tls_session_t *tls_session) +void fr_tls_verify_cert_reset(fr_tls_session_t *tls_session) { tls_session->validate.state = FR_TLS_VALIDATION_INIT; tls_session->validate.resumed = false; @@ -431,7 +432,7 @@ void fr_tls_verify_client_cert_reset(fr_tls_session_t *tls_session) /** Setup a verification request * */ -void fr_tls_verify_client_cert_request(fr_tls_session_t *tls_session, bool session_resumed) +void fr_tls_verify_cert_request(fr_tls_session_t *tls_session, bool session_resumed) { fr_assert(tls_session->validate.state == FR_TLS_VALIDATION_INIT); @@ -447,7 +448,7 @@ void fr_tls_verify_client_cert_request(fr_tls_session_t *tls_session, bool sessi * - UNLANG_ACTION_CALCULATE_RESULT - No pending actions * - UNLANG_ACTION_PUSHED_CHILD - Pending operations to evaluate. */ -unlang_action_t fr_tls_verify_client_cert_pending_push(request_t *request, fr_tls_session_t *tls_session) +unlang_action_t fr_tls_verify_cert_pending_push(request_t *request, fr_tls_session_t *tls_session) { if (tls_session->validate.state == FR_TLS_VALIDATION_REQUESTED) { return tls_verify_client_cert_push(request, tls_session); diff --git a/src/lib/tls/verify.h b/src/lib/tls/verify.h index a4da02556c0..4300967ee23 100644 --- a/src/lib/tls/verify.h +++ b/src/lib/tls/verify.h @@ -76,15 +76,15 @@ extern "C" { int fr_tls_verify_cert_cb(int ok, X509_STORE_CTX *ctx); -int fr_tls_verify_client_cert_chain(request_t *request, SSL *ssl); +int fr_tls_verify_cert_chain(request_t *request, SSL *ssl); -bool fr_tls_verify_client_cert_result(fr_tls_session_t *tls_session); +bool fr_tls_verify_cert_result(fr_tls_session_t *tls_session); -void fr_tls_verify_client_cert_reset(fr_tls_session_t *tls_session); +void fr_tls_verify_cert_reset(fr_tls_session_t *tls_session); -void fr_tls_verify_client_cert_request(fr_tls_session_t *tls_session, bool resumed); +void fr_tls_verify_cert_request(fr_tls_session_t *tls_session, bool resumed); -unlang_action_t fr_tls_verify_client_cert_pending_push(request_t *request, fr_tls_session_t *tls_session); +unlang_action_t fr_tls_verify_cert_pending_push(request_t *request, fr_tls_session_t *tls_session); #ifdef __cplusplus }