mirror of https://github.com/openssl/openssl.git
				
				
				
			With SSL_VERIFY_PEER client RPK should abort on X509 error
While RPK performs X.509 checks correctly, at the SSL layer the SSL_VERIFY_PEER flag was not honoured and connections were allowed to complete even when the server was not verified. The client can of course determine this by calling SSL_get_verify_result(), but some may not know to do this. Added tests to make sure this does not regress. Fixes CVE-2024-12797 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org>
This commit is contained in:
		
							parent
							
								
									d69c014608
								
							
						
					
					
						commit
						6ae8e947d8
					
				|  | @ -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) { | ||||
|  |  | |||
|  | @ -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,12 +460,35 @@ 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) { | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue