diff --git a/ChangeLog.d/verify-result-default-value.txt b/ChangeLog.d/verify-result-default-value.txt new file mode 100644 index 0000000000..2cf3f0c21b --- /dev/null +++ b/ChangeLog.d/verify-result-default-value.txt @@ -0,0 +1,5 @@ +Changes + * Harden mbedtls_ssl_get_verify_result() against misuse. + If the handshake has not yet been attempted, return -1u to indicate + that the result is not available. Previously the result of verification + was zero-initialized so the function would return 0 (indicating success). diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d7773be3e0..1185b0b283 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1056,6 +1056,8 @@ void mbedtls_ssl_transform_init(mbedtls_ssl_transform *transform) void mbedtls_ssl_session_init(mbedtls_ssl_session *session) { memset(session, 0, sizeof(mbedtls_ssl_session)); + /* Set verify_result to -1u to indicate 'result not available'. */ + session->verify_result = 0xFFFFFFFF; } MBEDTLS_CHECK_RETURN_CRITICAL @@ -5004,6 +5006,9 @@ void mbedtls_ssl_session_free(mbedtls_ssl_session *session) #endif mbedtls_platform_zeroize(session, sizeof(mbedtls_ssl_session)); + + /* Set verify_result to -1u to indicate 'result not available'. */ + session->verify_result = 0xFFFFFFFF; } #if defined(MBEDTLS_SSL_CONTEXT_SERIALIZATION) @@ -7930,6 +7935,7 @@ static int ssl_parse_certificate_coordinate(mbedtls_ssl_context *ssl, ssl->handshake->ciphersuite_info; if (!mbedtls_ssl_ciphersuite_uses_srv_cert(ciphersuite_info)) { + ssl->session_negotiate->verify_result = 0; return SSL_CERTIFICATE_SKIP; } @@ -9874,6 +9880,7 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, void *rs_ctx) { if (authmode == MBEDTLS_SSL_VERIFY_NONE) { + ssl->session_negotiate->verify_result = 0; return 0; } diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 78a069f792..752bc033fe 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2270,6 +2270,9 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED) if (mbedtls_ssl_tls13_key_exchange_mode_with_psk(ssl)) { mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_SERVER_FINISHED); + + /* Since we're not using a certificate, set verify_result to success */ + ssl->session_negotiate->verify_result = 0; } else { mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST); } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 5757d20180..e370a6b3f3 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -2637,6 +2637,9 @@ static int ssl_tls13_write_encrypted_extensions(mbedtls_ssl_context *ssl) #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED) if (mbedtls_ssl_tls13_key_exchange_mode_with_psk(ssl)) { mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_SERVER_FINISHED); + + /* Since we're not using a certificate, set verify_result to success */ + ssl->session_negotiate->verify_result = 0; } else { mbedtls_ssl_handshake_set_state(ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST); } diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 92bda3efd1..53cf1a0796 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3524,3 +3524,6 @@ ssl_tls_exporter_rejects_bad_parameters:MBEDTLS_SSL_VERSION_TLS1_3:24:250:10 TLS 1.3 Keying Material Exporter: Handshake not done depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:MBEDTLS_X509_RSASSA_PSS_SUPPORT ssl_tls_exporter_too_early:MBEDTLS_SSL_VERSION_TLS1_3:1:MBEDTLS_SSL_SERVER_CERTIFICATE + +Default verify_result before doing a handshake +verify_result_without_handshake diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 6797a4dd7e..38837f2ead 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5999,3 +5999,47 @@ exit: MD_OR_USE_PSA_DONE(); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED */ +void verify_result_without_handshake(void) +{ + /* Test the result of verification before we perform a handshake. */ + mbedtls_ssl_context ssl; + mbedtls_ssl_config conf; + + PSA_INIT(); + + mbedtls_ssl_init(&ssl); + mbedtls_ssl_config_init(&conf); + + TEST_EQUAL(mbedtls_ssl_config_defaults(&conf, + MBEDTLS_SSL_IS_CLIENT, + MBEDTLS_SSL_TRANSPORT_STREAM, + MBEDTLS_SSL_PRESET_DEFAULT), 0); + + mbedtls_ssl_conf_authmode(&conf, MBEDTLS_SSL_VERIFY_OPTIONAL); + mbedtls_ssl_conf_ca_chain(&conf, NULL, NULL); + mbedtls_ssl_conf_rng(&conf, mbedtls_test_random, NULL); + + TEST_EQUAL(mbedtls_ssl_setup(&ssl, &conf), 0); + + uint32_t verify_result = mbedtls_ssl_get_verify_result(&ssl); + + TEST_EQUAL(verify_result, 0xFFFFFFFF); + + /* Set the verify result manually and check that session_free resets it. */ + + /* Set the verify result to 0. */ + ssl.session_negotiate->verify_result = 0; + + mbedtls_ssl_session_free(ssl.session_negotiate); + + verify_result = mbedtls_ssl_get_verify_result(&ssl); + TEST_EQUAL(verify_result, 0xFFFFFFFF); + +exit: + mbedtls_ssl_config_free(&conf); + mbedtls_ssl_free(&ssl); + PSA_DONE(); +} +/* END_CASE */