From e8894974cbc8dc98b4ddeea0f645ce0e19ac84f5 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 10 Mar 2026 11:51:30 +0000 Subject: [PATCH] Reintroduce ssl_parse_signature_algorithm The logic was easier to follow before 693a47a, which removed the ssl_parse_signature_algorithm function and introduced the bug being fixed in this PR. When validating multiple conditions, it's easier to read, easier to debug and, as we can see, easier to get right if you validate them separately. Signed-off-by: Janos Follath --- library/ssl_tls12_client.c | 74 +++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index b03859a8d6..8d26562c1e 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -1732,6 +1732,71 @@ static int ssl_parse_server_psk_hint(mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL +static int ssl_parse_signature_algorithm(mbedtls_ssl_context *ssl, + unsigned char **p, + unsigned char *end, + mbedtls_md_type_t *md_alg, + mbedtls_pk_sigalg_t *pk_alg) +{ + *md_alg = MBEDTLS_MD_NONE; + *pk_alg = MBEDTLS_PK_SIGALG_NONE; + + MBEDTLS_SSL_CHK_BUF_READ_PTR(*p, end, 2); + uint16_t sig_alg = MBEDTLS_GET_UINT16_BE(*p, 0); + + if (mbedtls_ssl_get_pk_sigalg_and_md_alg_from_sig_alg(sig_alg, pk_alg, md_alg) != 0) { + /* + * Check hash algorithm + */ + if (*md_alg == MBEDTLS_MD_NONE) { + MBEDTLS_SSL_DEBUG_MSG(1, ("Server used unsupported HashAlgorithm %d", (*p)[0])); + return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + } + + /* + * Check signature algorithm + */ + if (*pk_alg == MBEDTLS_PK_SIGALG_NONE) { + MBEDTLS_SSL_DEBUG_MSG(1, ("Server used unsupported SignatureAlgorithm %d", (*p)[1])); + return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + } + + /* + * This shouldn't happen, but be robust. + */ + MBEDTLS_SSL_DEBUG_MSG(1, ("Server used unsupported value in SigAlg extension %d", sig_alg)); + return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + } + + /* + * 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 %d", sig_alg)); + return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + } + + /* + * 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 %d that was not offered", sig_alg)); + return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + } + + MBEDTLS_SSL_DEBUG_MSG(2, ("Server used SignatureAlgorithm %d", (*p)[1])); + MBEDTLS_SSL_DEBUG_MSG(2, ("Server used HashAlgorithm %d", (*p)[0])); + *p += 2; + + return 0; +} +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_key_exchange(mbedtls_ssl_context *ssl) { @@ -1889,7 +1954,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; @@ -1907,12 +1971,7 @@ 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_sigalg_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)) { + if (ssl_parse_signature_algorithm(ssl, &p, end, &md_alg, &pk_alg) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad server key exchange message")); mbedtls_ssl_send_alert_message( @@ -1921,7 +1980,6 @@ start_processing: MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER); return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER; } - p += 2; psa_hash_alg = mbedtls_md_psa_alg_from_type(md_alg); if (!mbedtls_pk_can_do_psa(peer_pk,