From d3d16e36cc09c014f1c1fbb3334daee93047efa8 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Fri, 20 Dec 2024 04:26:20 +1100 Subject: [PATCH] Use ERR marks also when verifying server X.509 certs Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell Reviewed-by: Neil Horman (cherry picked from commit 739c4b2e92116952e4baf3e14d219b82f871ec6a) --- ssl/statem/statem_clnt.c | 14 ++++++++------ test/rpktest.c | 39 ++++++++++++++++++--------------------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 8716ed669f3..df2eed7594c 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2082,10 +2082,7 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s, if (s->rwstate == SSL_RETRY_VERIFY) s->rwstate = SSL_NOTHING; - i = ssl_verify_cert_chain(s, s->session->peer_chain); - if (i > 0 && s->rwstate == SSL_RETRY_VERIFY) { - return WORK_MORE_A; - } + /* * The documented interface is that SSL_VERIFY_PEER should be set in order * for client side verification of the server certificate to take place. @@ -2100,12 +2097,17 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s, * (less clean) historic behaviour of performing validation if any flag is * set. The *documented* interface remains the same. */ - if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) { + ERR_set_mark(); + i = ssl_verify_cert_chain(s, s->session->peer_chain); + if (i <= 0 && s->verify_mode != SSL_VERIFY_NONE) { + ERR_clear_last_mark(); SSLfatal(s, ssl_x509err2alert(s->verify_result), SSL_R_CERTIFICATE_VERIFY_FAILED); return WORK_ERROR; } - ERR_clear_error(); /* but we keep s->verify_result */ + ERR_pop_to_mark(); /* but we keep s->verify_result */ + if (i > 0 && s->rwstate == SSL_RETRY_VERIFY) + return WORK_MORE_A; /* * Inconsistency alert: cert_chain does include the peer's certificate, diff --git a/test/rpktest.c b/test/rpktest.c index 0be8461f77b..624d366508f 100644 --- a/test/rpktest.c +++ b/test/rpktest.c @@ -490,24 +490,22 @@ static int test_rpk(int idx) } /* Make sure client gets RPK or certificate as configured */ - if (expected == 1) { - if (idx_server_server_rpk && idx_client_server_rpk) { - if (!TEST_long_eq(SSL_get_verify_result(clientssl), client_verify_result)) - goto end; - if (!TEST_ptr(SSL_get0_peer_rpk(clientssl))) - goto end; - if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_rpk)) - goto end; - if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_rpk)) - goto end; - } else { - if (!TEST_ptr(SSL_get0_peer_certificate(clientssl))) - goto end; - if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_x509)) - goto end; - if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_x509)) - goto end; - } + if (idx_server_server_rpk && idx_client_server_rpk) { + if (!TEST_long_eq(SSL_get_verify_result(clientssl), client_verify_result)) + goto end; + if (!TEST_ptr(SSL_get0_peer_rpk(clientssl))) + goto end; + if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_rpk)) + goto end; + if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_rpk)) + goto end; + } else { + if (!TEST_ptr(SSL_get0_peer_certificate(clientssl))) + goto end; + if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(serverssl), TLSEXT_cert_type_x509)) + goto end; + if (!TEST_int_eq(SSL_get_negotiated_server_cert_type(clientssl), TLSEXT_cert_type_x509)) + goto end; } if (idx == 9) { @@ -534,8 +532,7 @@ static int test_rpk(int idx) if (!TEST_int_eq(SSL_get_negotiated_client_cert_type(clientssl), TLSEXT_cert_type_rpk)) goto end; } else { - /* only if connection is expected to succeed */ - if (expected == 1 && !TEST_ptr(SSL_get0_peer_certificate(serverssl))) + if (!TEST_ptr(SSL_get0_peer_certificate(serverssl))) goto end; if (!TEST_int_eq(SSL_get_negotiated_client_cert_type(serverssl), TLSEXT_cert_type_x509)) goto end; @@ -625,7 +622,7 @@ static int test_rpk(int idx) } ret = create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE); - if (!TEST_int_eq(expected, ret)) + if (!TEST_true(ret)) goto end; verify = SSL_get_verify_result(clientssl); if (!TEST_int_eq(client_expected, verify)) -- 2.47.2