diff --git a/ChangeLog.d/sig_algs_check.txt b/ChangeLog.d/sig_algs_check.txt new file mode 100644 index 0000000000..b0aed6dfd3 --- /dev/null +++ b/ChangeLog.d/sig_algs_check.txt @@ -0,0 +1,5 @@ +Security + * Fix a bug in the TLS 1.2 client's signature algorithm check, which caused + the client to accept server key exchange messages signed with a signature + algorithm explicitly disallowed by the client. Found and reported by + EFR-GmbH and M. Heuft of Security-Research-Consulting GmbH. CVE-2026-25834 diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index b19c2d7786..0196c0cc53 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -2078,6 +2078,46 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL +static int ssl_parse_signature_algorithm(mbedtls_ssl_context *ssl, + uint16_t sig_alg, + mbedtls_md_type_t *md_alg, + mbedtls_pk_type_t *pk_alg) +{ + if (mbedtls_ssl_get_pk_type_and_md_alg_from_sig_alg(sig_alg, pk_alg, md_alg) != 0) { + MBEDTLS_SSL_DEBUG_MSG(1, + ("Server used unsupported value in SigAlg extension 0x%04x", + sig_alg)); + return MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; + } + + /* + * mbedtls_ssl_get_pk_sigalg_and_md_alg_from_sig_alg() understands sig_alg code points across + * TLS versions. Make sure that the received sig_alg extension is valid in TLS 1.2. + */ + if (!mbedtls_ssl_sig_alg_is_supported(ssl, sig_alg)) { + MBEDTLS_SSL_DEBUG_MSG(1, + ("Server used unsupported value in SigAlg extension 0x%04x", + sig_alg)); + return MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; + } + + /* + * Check if the signature algorithm is acceptable + */ + if (!mbedtls_ssl_sig_alg_is_offered(ssl, sig_alg)) { + MBEDTLS_SSL_DEBUG_MSG(1, ("Server used SigAlg value 0x%04x that was not offered", sig_alg)); + return MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; + } + + MBEDTLS_SSL_DEBUG_MSG(2, ("Server used SignatureAlgorithm %d", sig_alg & 0x00FF)); + MBEDTLS_SSL_DEBUG_MSG(2, ("Server used HashAlgorithm %d", sig_alg >> 8)); + + return 0; +} +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) */ + MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_key_exchange(mbedtls_ssl_context *ssl) { @@ -2300,7 +2340,6 @@ start_processing: unsigned char *params = ssl->in_msg + mbedtls_ssl_hs_hdr_len(ssl); size_t params_len = (size_t) (p - params); void *rs_ctx = NULL; - uint16_t sig_alg; mbedtls_pk_context *peer_pk; @@ -2319,11 +2358,8 @@ start_processing: * Handle the digitally-signed structure */ MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, 2); - sig_alg = MBEDTLS_GET_UINT16_BE(p, 0); - if (mbedtls_ssl_get_pk_type_and_md_alg_from_sig_alg( - sig_alg, &pk_alg, &md_alg) != 0 && - !mbedtls_ssl_sig_alg_is_offered(ssl, sig_alg) && - !mbedtls_ssl_sig_alg_is_supported(ssl, sig_alg)) { + uint16_t sig_alg = MBEDTLS_GET_UINT16_BE(p, 0); + if (ssl_parse_signature_algorithm(ssl, sig_alg, &md_alg, &pk_alg) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad server key exchange message")); mbedtls_ssl_send_alert_message( diff --git a/tests/include/test/ssl_helpers.h b/tests/include/test/ssl_helpers.h index 33ed2a883d..84b847b203 100644 --- a/tests/include/test/ssl_helpers.h +++ b/tests/include/test/ssl_helpers.h @@ -71,6 +71,39 @@ defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) #define MBEDTLS_CAN_HANDLE_RSA_TEST_KEY #endif + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) && \ + defined(MBEDTLS_ECP_HAVE_SECP384R1) && \ + defined(MBEDTLS_MD_CAN_SHA384) +#define MBEDTLS_CAN_HANDLE_ECDSA_TEST_KEY +#endif + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) && \ + defined(MBEDTLS_ECP_HAVE_SECP256R1) && \ + defined(MBEDTLS_MD_CAN_SHA256) +#define MBEDTLS_CAN_HANDLE_ECDSA_CLIENT_TEST_KEY +#endif + +#if defined(MBEDTLS_ECP_HAVE_CURVE25519) || \ + defined(MBEDTLS_ECP_HAVE_SECP256R1) || \ + defined(MBEDTLS_ECP_HAVE_SECP384R1) || \ + defined(MBEDTLS_ECP_HAVE_CURVE448) || \ + defined(MBEDTLS_ECP_HAVE_SECP521R1) || \ + defined(MBEDTLS_ECP_HAVE_BP256R1) || \ + defined(MBEDTLS_ECP_HAVE_BP384R1) || \ + defined(MBEDTLS_ECP_HAVE_BP512R1) +#define MBEDTLS_TEST_HAS_DEFAULT_EC_GROUP +#endif + +/* + * To use the test keys we need PSA_WANT_ALG_SHA_256. Some test cases need an additional hash that + * is configured by default (see mbedtls_ssl_config_defaults()), but it doesn't matter which one. + */ +#if defined(MBEDTLS_MD_CAN_SHA512) || \ + defined(MBEDTLS_MD_CAN_SHA384) +#define MBEDTLS_TEST_HAS_ADDITIONAL_HASH +#endif + enum { #define MBEDTLS_SSL_TLS1_3_LABEL(name, string) \ tls13_label_ ## name, diff --git a/tests/src/certs.c b/tests/src/certs.c index 6a01893f4a..e7f4d3553b 100644 --- a/tests/src/certs.c +++ b/tests/src/certs.c @@ -440,7 +440,7 @@ const unsigned char *mbedtls_test_cas_der[] = { mbedtls_test_ca_crt_rsa_sha1_der, #endif /* MBEDTLS_MD_CAN_SHA1 */ #endif /* MBEDTLS_RSA_C */ -#if defined(MBEDTLS_PK_CAN_ECDSA_SOME) +#if defined(MBEDTLS_PK_CAN_ECDSA_SOME) && defined(MBEDTLS_ECP_HAVE_SECP384R1) mbedtls_test_ca_crt_ec_der, #endif /* MBEDTLS_PK_CAN_ECDSA_SOME */ NULL @@ -455,7 +455,7 @@ const size_t mbedtls_test_cas_der_len[] = { sizeof(mbedtls_test_ca_crt_rsa_sha1_der), #endif /* MBEDTLS_MD_CAN_SHA1 */ #endif /* MBEDTLS_RSA_C */ -#if defined(MBEDTLS_PK_CAN_ECDSA_SOME) +#if defined(MBEDTLS_PK_CAN_ECDSA_SOME) && defined(MBEDTLS_ECP_HAVE_SECP384R1) sizeof(mbedtls_test_ca_crt_ec_der), #endif /* MBEDTLS_PK_CAN_ECDSA_SOME */ 0 diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 7fb23ab829..e3c0aa1cbf 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -97,8 +97,10 @@ void mbedtls_test_free_handshake_options( mbedtls_test_handshake_test_options *opts) { #if defined(MBEDTLS_SSL_CACHE_C) - mbedtls_ssl_cache_free(opts->cache); - mbedtls_free(opts->cache); + if (opts->cache != NULL) { + mbedtls_ssl_cache_free(opts->cache); + mbedtls_free(opts->cache); + } #else (void) opts; #endif diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 1e57420a77..827636df35 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3527,3 +3527,19 @@ ssl_tls_exporter_too_early:MBEDTLS_SSL_VERSION_TLS1_3:1:MBEDTLS_SSL_SERVER_CERTI TLS 1.3 - HRR then TLS 1.2 second ClientHello tls13_hrr_then_tls12_second_client_hello + +Baseline for: Server using sig_alg not offered by the client - RSA with SHA256 +depends_on:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED:MBEDTLS_MD_CAN_SHA256 +send_invalid_sig_alg:MBEDTLS_SSL_SIG_RSA:MBEDTLS_SSL_HASH_SHA256:0 + +Negative Test: Server using sig_alg not offered by the client - RSA with SHA256 +depends_on:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED:MBEDTLS_MD_CAN_SHA256 +send_invalid_sig_alg:MBEDTLS_SSL_SIG_RSA:MBEDTLS_SSL_HASH_SHA256:MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER + +Baseline for: Server using sig_alg not offered by the client - ECDSA with SHA512 +depends_on:MBEDTLS_CAN_HANDLE_ECDSA_TEST_KEY:MBEDTLS_CAN_HANDLE_ECDSA_CLIENT_TEST_KEY:MBEDTLS_MD_CAN_SHA512 +send_invalid_sig_alg:MBEDTLS_SSL_SIG_ECDSA:MBEDTLS_SSL_HASH_SHA512:0 + +Negative Test: Server using sig_alg not offered by the client - ECDSA with SHA512 +depends_on:MBEDTLS_CAN_HANDLE_ECDSA_TEST_KEY:MBEDTLS_CAN_HANDLE_ECDSA_CLIENT_TEST_KEY:MBEDTLS_MD_CAN_SHA512 +send_invalid_sig_alg:MBEDTLS_SSL_SIG_ECDSA:MBEDTLS_SSL_HASH_SHA512:MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 875b48c426..b9b459635f 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5727,6 +5727,140 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_HAVE_CHACHAPOLY:MBEDTLS_MD_CAN_SHA256:MBEDTLS_TEST_HAS_ADDITIONAL_HASH:MBEDTLS_TEST_HAS_DEFAULT_EC_GROUP*/ +void send_invalid_sig_alg(int sig, int hash, int expected_ret) +{ + // This is a test about the client behaviour in case it receives a key exchange signed with a + // sig_alg it didn't specify in the client hello. The input specifies a target_sig_alg, which we + // make sure that the client does not offer but the server does. Then we make the server believe + // that target_sig_alg is the only one the client offered. + + // Remark: We need an additional hash algorithm offered, because if we don't have it, the server + // realises too early that there is no common ground and we don't get the chance to manipulate + // it. This is why we need MBEDTLS_TEST_HAS_ADDITIONAL_HASH in the requirements. + + enum { BUFFSIZE = 16384 }; + uint16_t *client_sig_algs = NULL; + mbedtls_test_ssl_endpoint server, client; + memset(&server, 0, sizeof(server)); + memset(&client, 0, sizeof(client)); + mbedtls_test_handshake_test_options options; + memset(&options, 0, sizeof(options)); + int forced_ciphersuite[2] = { 0, 0 }; + + uint16_t target_sig_alg = ((hash << 8) | sig); + + mbedtls_test_init_handshake_options(&options); + + // Make sure the server has credentials for target_sig_alg + if (sig == MBEDTLS_SSL_SIG_ECDSA) { + options.pk_alg = MBEDTLS_PK_ECDSA; + } else { + options.pk_alg = MBEDTLS_PK_RSA; + } + + // Force a ciphersuite where target_sig_alg is relevant + if (sig == MBEDTLS_SSL_SIG_ECDSA) { + forced_ciphersuite[0] = + mbedtls_ssl_get_ciphersuite_id("TLS-ECDHE-ECDSA-WITH-CHACHA20-POLY1305-SHA256"); + } else { + forced_ciphersuite[0] = + mbedtls_ssl_get_ciphersuite_id("TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256"); + } + TEST_ASSERT(forced_ciphersuite[0] != 0); + + // Force TLS 1.2 as this test is a non-regression test for a bug in TLS 1.2 client and TLS 1.3 + // behaviour in this regard is substantially different. + options.client_max_version = MBEDTLS_SSL_VERSION_TLS1_2; + options.server_max_version = MBEDTLS_SSL_VERSION_TLS1_2; + +#if defined(MBEDTLS_DEBUG_C) + mbedtls_test_ssl_log_pattern cli_pattern; + cli_pattern.pattern = "that was not offered"; + cli_pattern.counter = 0; + options.cli_log_obj = &cli_pattern; + options.cli_log_fun = mbedtls_test_ssl_log_analyzer; + // Add loggers for easier debugging - we are not looking for any patterns in the server logs. + // To turn on debug output, uncomment the threshold line and set the macro in the definition + // of mbedtls_test_ssl_log_analyzer(). + options.srv_log_obj = NULL; + options.srv_log_fun = mbedtls_test_ssl_log_analyzer; + mbedtls_debug_set_threshold(3); +#endif + + int ret = -1; + + PSA_INIT(); + + ret = mbedtls_test_ssl_endpoint_init(&client, MBEDTLS_SSL_IS_CLIENT, + &options, NULL, NULL, NULL); + TEST_EQUAL(ret, 0); + + mbedtls_ssl_conf_ciphersuites(&client.conf, forced_ciphersuite); + + // Remove the target signature algorithm from the client's list + size_t client_sig_algs_len = 0; + while (client.conf.sig_algs[client_sig_algs_len++] != MBEDTLS_TLS1_3_SIG_NONE) { + ; + } + client_sig_algs_len--; + + TEST_CALLOC(client_sig_algs, client_sig_algs_len); + size_t j = 0; + for (size_t i = 0; client.conf.sig_algs[i] != MBEDTLS_TLS1_3_SIG_NONE; i++) { + if (client.conf.sig_algs[i] != target_sig_alg) { + client_sig_algs[j++] = client.conf.sig_algs[i]; + } + } + TEST_ASSERT(j < client_sig_algs_len); + client_sig_algs[j] = MBEDTLS_TLS1_3_SIG_NONE; + mbedtls_ssl_conf_sig_algs(&client.conf, client_sig_algs); + + ret = mbedtls_test_ssl_endpoint_init(&server, MBEDTLS_SSL_IS_SERVER, + &options, NULL, NULL, NULL); + TEST_EQUAL(ret, 0); + + mbedtls_ssl_conf_ciphersuites(&server.conf, forced_ciphersuite); + + ret = mbedtls_test_mock_socket_connect(&server.socket, &client.socket, + BUFFSIZE); + TEST_EQUAL(ret, 0); + + // Move the connection to the point before the server sending the key exchange message + ret = mbedtls_test_move_handshake_to_state(&server.ssl, &client.ssl, + MBEDTLS_SSL_SERVER_KEY_EXCHANGE); + TEST_EQUAL(ret, 0); + + if (expected_ret != 0) { + // Make the server believe that the only sig_alg the client accepts is target_sig_alg + server.ssl.handshake->received_sig_algs[0] = target_sig_alg; + server.ssl.handshake->received_sig_algs[1] = MBEDTLS_TLS1_3_SIG_NONE; + } + + // Move the connection to a state where it is certain that the client has parsed the server key + // exchange + ret = mbedtls_test_move_handshake_to_state(&client.ssl, &server.ssl, + MBEDTLS_SSL_CERTIFICATE_REQUEST); + TEST_EQUAL(ret, expected_ret); + +#if defined(MBEDTLS_DEBUG_C) + if (expected_ret != 0) { + TEST_EQUAL(cli_pattern.counter, 1); + } +#endif + +exit: +#if defined(MBEDTLS_DEBUG_C) + mbedtls_debug_set_threshold(0); +#endif + mbedtls_test_free_handshake_options(&options); + mbedtls_test_ssl_endpoint_free(&server, NULL); + mbedtls_test_ssl_endpoint_free(&client, NULL); + mbedtls_free(client_sig_algs); + PSA_DONE(); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SSL_KEYING_MATERIAL_EXPORT:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256 */ void ssl_tls_exporter_consistent_result(int proto, int exported_key_length, int use_context) {