diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 436b397346..df2eed7594 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1910,6 +1910,7 @@ static WORK_STATE tls_post_process_server_rpk(SSL_CONNECTION *sc, { size_t certidx; const SSL_CERT_LOOKUP *clu; + int v_ok; if (sc->session->peer_rpk == NULL) { SSLfatal(sc, SSL_AD_ILLEGAL_PARAMETER, @@ -1919,9 +1920,19 @@ static WORK_STATE tls_post_process_server_rpk(SSL_CONNECTION *sc, if (sc->rwstate == SSL_RETRY_VERIFY) sc->rwstate = SSL_NOTHING; - if (ssl_verify_rpk(sc, sc->session->peer_rpk) > 0 - && sc->rwstate == SSL_RETRY_VERIFY) + + ERR_set_mark(); + v_ok = ssl_verify_rpk(sc, sc->session->peer_rpk); + if (v_ok <= 0 && sc->verify_mode != SSL_VERIFY_NONE) { + ERR_clear_last_mark(); + SSLfatal(sc, ssl_x509err2alert(sc->verify_result), + SSL_R_CERTIFICATE_VERIFY_FAILED); + return WORK_ERROR; + } + ERR_pop_to_mark(); /* but we keep s->verify_result */ + if (v_ok > 0 && sc->rwstate == SSL_RETRY_VERIFY) { return WORK_MORE_A; + } if ((clu = ssl_cert_lookup_by_pkey(sc->session->peer_rpk, &certidx, SSL_CONNECTION_GET_CTX(sc))) == NULL) { @@ -2071,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. @@ -2089,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 ac824798f1..624d366508 100644 --- a/test/rpktest.c +++ b/test/rpktest.c @@ -89,12 +89,14 @@ static int rpk_verify_server_cb(int ok, X509_STORE_CTX *ctx) * idx = 13 - resumption with client authentication * idx = 14 - resumption with client authentication, no ticket * idx = 15 - like 0, but use non-default libctx + * idx = 16 - like 7, but with SSL_VERIFY_PEER connection should fail + * idx = 17 - like 8, but with SSL_VERIFY_PEER connection should fail * - * 16 * 2 * 4 * 2 * 2 * 2 * 2 = 2048 tests + * 18 * 2 * 4 * 2 * 2 * 2 * 2 = 2048 tests */ static int test_rpk(int idx) { -# define RPK_TESTS 16 +# define RPK_TESTS 18 # define RPK_DIMS (2 * 4 * 2 * 2 * 2 * 2) SSL_CTX *cctx = NULL, *sctx = NULL; SSL *clientssl = NULL, *serverssl = NULL; @@ -114,6 +116,7 @@ static int test_rpk(int idx) int idx_cert, idx_prot; int client_auth = 0; int resumption = 0; + int want_error = SSL_ERROR_NONE; long server_verify_result = 0; long client_verify_result = 0; OSSL_LIB_CTX *test_libctx = NULL; @@ -188,7 +191,7 @@ static int test_rpk(int idx) #ifdef OPENSSL_NO_ECDSA /* Can't get other_key if it's ECDSA */ if (other_pkey == NULL && idx_cert == 0 - && (idx == 4 || idx == 6 || idx == 7)) { + && (idx == 4 || idx == 6 || idx == 7 || idx == 16)) { testresult = TEST_skip("EDCSA disabled"); goto end; } @@ -266,8 +269,10 @@ static int test_rpk(int idx) goto end; /* Only a private key */ if (idx == 1) { - if (idx_server_server_rpk == 0 || idx_client_server_rpk == 0) + if (idx_server_server_rpk == 0 || idx_client_server_rpk == 0) { expected = 0; + want_error = SSL_ERROR_SSL; + } } else { /* Add certificate */ if (!TEST_int_eq(SSL_use_certificate_file(serverssl, cert_file, SSL_FILETYPE_PEM), 1)) @@ -333,12 +338,14 @@ static int test_rpk(int idx) client_expected = -1; if (!TEST_true(SSL_add_expected_rpk(clientssl, other_pkey))) goto end; + SSL_set_verify(clientssl, SSL_VERIFY_NONE, rpk_verify_client_cb); client_verify_result = X509_V_ERR_DANE_NO_MATCH; break; case 8: if (idx_server_server_rpk == 1 && idx_client_server_rpk == 1) client_expected = -1; /* no peer keys */ + SSL_set_verify(clientssl, SSL_VERIFY_NONE, rpk_verify_client_cb); client_verify_result = X509_V_ERR_RPK_UNTRUSTED; break; case 9: @@ -370,9 +377,13 @@ static int test_rpk(int idx) if (!TEST_int_eq(SSL_use_PrivateKey_file(clientssl, privkey_file, SSL_FILETYPE_PEM), 1)) goto end; /* Since there's no cert, this is expected to fail without RPK support */ - if (!idx_server_client_rpk || !idx_client_client_rpk) + if (!idx_server_client_rpk || !idx_client_client_rpk) { expected = 0; - SSL_set_verify(serverssl, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, rpk_verify_server_cb); + want_error = SSL_ERROR_SSL; + SSL_set_verify(serverssl, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL); + } else { + SSL_set_verify(serverssl, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, rpk_verify_server_cb); + } client_auth = 1; break; case 11: @@ -449,31 +460,52 @@ static int test_rpk(int idx) if (!TEST_true(SSL_add_expected_rpk(clientssl, pkey))) goto end; break; + case 16: + if (idx_server_server_rpk == 1 && idx_client_server_rpk == 1) { + /* wrong expected server key */ + expected = 0; + want_error = SSL_ERROR_SSL; + SSL_set_verify(serverssl, SSL_VERIFY_PEER, NULL); + } + if (!TEST_true(SSL_add_expected_rpk(clientssl, other_pkey))) + goto end; + break; + case 17: + if (idx_server_server_rpk == 1 && idx_client_server_rpk == 1) { + /* no expected server keys */ + expected = 0; + want_error = SSL_ERROR_SSL; + SSL_set_verify(serverssl, SSL_VERIFY_PEER, NULL); + } + break; } - ret = create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE); + ret = create_ssl_connection(serverssl, clientssl, want_error); if (!TEST_int_eq(expected, ret)) goto end; + if (expected <= 0) { + testresult = 1; + goto end; + } + /* 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) { @@ -500,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; @@ -591,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))