From 067a1e735e33c2c9502e3ae689f02f1e80d75885 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 16 Sep 2022 13:44:49 +0200 Subject: [PATCH] tls13: Try reasonable sig alg for CertificateVerify signature Instead of fully validating beforehand signature algorithms with regards to the private key, do minimum validation and then just try to compute the signature. If it fails try another reasonable algorithm if any. Signed-off-by: Ronald Cron --- library/ssl_tls13_generic.c | 191 ++++++++++++------------------------ tests/ssl-opt.sh | 28 +++--- 2 files changed, 79 insertions(+), 140 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 54884e9ff..858fe0316 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -923,83 +923,13 @@ int mbedtls_ssl_tls13_check_sig_alg_cert_key_match( uint16_t sig_alg, return( 0 ); } -#if defined(MBEDTLS_USE_PSA_CRYPTO) -static psa_algorithm_t ssl_tls13_select_sig_alg_to_psa_alg( uint16_t sig_alg ) -{ - switch( sig_alg ) - { - case MBEDTLS_TLS1_3_SIG_ECDSA_SECP256R1_SHA256: - return( PSA_ALG_ECDSA( PSA_ALG_SHA_256 ) ); - case MBEDTLS_TLS1_3_SIG_ECDSA_SECP384R1_SHA384: - return( PSA_ALG_ECDSA( PSA_ALG_SHA_384 ) ); - case MBEDTLS_TLS1_3_SIG_ECDSA_SECP521R1_SHA512: - return( PSA_ALG_ECDSA( PSA_ALG_SHA_512 ) ); - case MBEDTLS_TLS1_3_SIG_RSA_PSS_RSAE_SHA256: - return( PSA_ALG_RSA_PSS( PSA_ALG_SHA_256 ) ); - case MBEDTLS_TLS1_3_SIG_RSA_PSS_RSAE_SHA384: - return( PSA_ALG_RSA_PSS( PSA_ALG_SHA_384 ) ); - case MBEDTLS_TLS1_3_SIG_RSA_PSS_RSAE_SHA512: - return( PSA_ALG_RSA_PSS( PSA_ALG_SHA_512 ) ); - case MBEDTLS_TLS1_3_SIG_RSA_PKCS1_SHA256: - case MBEDTLS_TLS1_3_SIG_RSA_PKCS1_SHA384: - case MBEDTLS_TLS1_3_SIG_RSA_PKCS1_SHA512: - return( PSA_ALG_RSA_PKCS1V15_CRYPT ); - default: - return( 0 ); - } -} -#endif /* MBEDTLS_USE_PSA_CRYPTO */ - -MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_tls13_select_sig_alg_for_certificate_verify( - mbedtls_ssl_context *ssl, - mbedtls_pk_context *own_key, - uint16_t *algorithm ) -{ - uint16_t *sig_alg = ssl->handshake->received_sig_algs; -#if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_algorithm_t psa_alg = 0; -#endif /* MBEDTLS_USE_PSA_CRYPTO */ - - *algorithm = MBEDTLS_TLS1_3_SIG_NONE; - for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE ; sig_alg++ ) - { -#if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_alg = ssl_tls13_select_sig_alg_to_psa_alg( *sig_alg ); -#endif /* MBEDTLS_USE_PSA_CRYPTO */ - - if( mbedtls_ssl_sig_alg_is_offered( ssl, *sig_alg ) && - mbedtls_ssl_tls13_sig_alg_for_cert_verify_is_supported( *sig_alg ) && - mbedtls_ssl_tls13_check_sig_alg_cert_key_match( *sig_alg, own_key ) -#if defined(MBEDTLS_USE_PSA_CRYPTO) - && psa_alg != 0 && - mbedtls_pk_can_do_ext( own_key, psa_alg, - PSA_KEY_USAGE_SIGN_HASH ) == 1 -#endif /* MBEDTLS_USE_PSA_CRYPTO */ - ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "select_sig_alg_for_certificate_verify:" - "selected signature algorithm %s [%04x]", - mbedtls_ssl_sig_alg_to_str( *sig_alg ), - *sig_alg ) ); - *algorithm = *sig_alg; - return( 0 ); - } - } - MBEDTLS_SSL_DEBUG_MSG( 2, - ( "select_sig_alg_for_certificate_verify:" - "no suitable signature algorithm found" ) ); - return( -1 ); -} - MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_certificate_verify_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, size_t *out_len ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; mbedtls_pk_context *own_key; @@ -1007,14 +937,9 @@ static int ssl_tls13_write_certificate_verify_body( mbedtls_ssl_context *ssl, size_t handshake_hash_len; unsigned char verify_buffer[ SSL_VERIFY_STRUCT_MAX_SIZE ]; size_t verify_buffer_len; - mbedtls_pk_type_t pk_type = MBEDTLS_PK_NONE; - mbedtls_md_type_t md_alg = MBEDTLS_MD_NONE; - psa_algorithm_t psa_algorithm = PSA_ALG_NONE; - uint16_t algorithm = MBEDTLS_TLS1_3_SIG_NONE; + + uint16_t *sig_alg = ssl->handshake->received_sig_algs; size_t signature_len = 0; - unsigned char verify_hash[PSA_HASH_MAX_SIZE]; - size_t verify_hash_len; - psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; *out_len = 0; @@ -1047,64 +972,78 @@ static int ssl_tls13_write_certificate_verify_body( mbedtls_ssl_context *ssl, * opaque signature<0..2^16-1>; * } CertificateVerify; */ - ret = ssl_tls13_select_sig_alg_for_certificate_verify( ssl, own_key, - &algorithm ); - if( ret != 0 ) + /* Check there is space for the algorithm identifier (2 bytes) and the + * signature length (2 bytes). + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); + + for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE ; sig_alg++ ) { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "signature algorithm not in received or offered list." ) ); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + mbedtls_pk_type_t pk_type = MBEDTLS_PK_NONE; + mbedtls_md_type_t md_alg = MBEDTLS_MD_NONE; + psa_algorithm_t psa_algorithm = PSA_ALG_NONE; + unsigned char verify_hash[PSA_HASH_MAX_SIZE]; + size_t verify_hash_len; - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Signature algorithm is %s", - mbedtls_ssl_sig_alg_to_str( algorithm ) ) ); + if( !mbedtls_ssl_sig_alg_is_offered( ssl, *sig_alg ) ) + continue; + if( !mbedtls_ssl_tls13_sig_alg_for_cert_verify_is_supported( *sig_alg ) ) + continue; + + if( !mbedtls_ssl_tls13_check_sig_alg_cert_key_match( *sig_alg, own_key ) ) + continue; + + if( mbedtls_ssl_get_pk_type_and_md_alg_from_sig_alg( + *sig_alg, &pk_type, &md_alg ) != 0 ) + { + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + /* Hash verify buffer with indicated hash function */ + psa_algorithm = mbedtls_hash_info_psa_from_md( md_alg ); + status = psa_hash_compute( psa_algorithm, + verify_buffer, + verify_buffer_len, + verify_hash, sizeof( verify_hash ), + &verify_hash_len ); + if( status != PSA_SUCCESS ) + return( psa_ssl_status_to_mbedtls( status ) ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "verify hash", verify_hash, verify_hash_len ); + + if( ( ret = mbedtls_pk_sign_ext( pk_type, own_key, + md_alg, verify_hash, verify_hash_len, + p + 4, (size_t)( end - ( p + 4 ) ), &signature_len, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "CertificateVerify signature failed with %s", + mbedtls_ssl_sig_alg_to_str( *sig_alg ) ) ); + MBEDTLS_SSL_DEBUG_RET( 2, "mbedtls_pk_sign_ext", ret ); + continue; + } + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "CertificateVerify signature with %s", + mbedtls_ssl_sig_alg_to_str( *sig_alg ) ) ); + + break; + } + + if( *sig_alg == MBEDTLS_TLS1_3_SIG_NONE ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "no suitable signature algorithm" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } - MBEDTLS_SSL_DEBUG_MSG( 2, ( "CertificateVerify with %s", - mbedtls_ssl_sig_alg_to_str( algorithm )) ); + MBEDTLS_PUT_UINT16_BE( *sig_alg, p, 0 ); + MBEDTLS_PUT_UINT16_BE( signature_len, p, 2 ); - if( mbedtls_ssl_get_pk_type_and_md_alg_from_sig_alg( - algorithm, &pk_type, &md_alg ) != 0 ) - { - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } + *out_len = 4 + signature_len; - /* Check there is space for the algorithm identifier (2 bytes) and the - * signature length (2 bytes). - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); - MBEDTLS_PUT_UINT16_BE( algorithm, p, 0 ); - p += 2; - - /* Hash verify buffer with indicated hash function */ - psa_algorithm = mbedtls_hash_info_psa_from_md( md_alg ); - status = psa_hash_compute( psa_algorithm, - verify_buffer, - verify_buffer_len, - verify_hash,sizeof( verify_hash ), - &verify_hash_len ); - if( status != PSA_SUCCESS ) - return( psa_ssl_status_to_mbedtls( status ) ); - - MBEDTLS_SSL_DEBUG_BUF( 3, "verify hash", verify_hash, verify_hash_len ); - - if( ( ret = mbedtls_pk_sign_ext( pk_type, own_key, - md_alg, verify_hash, verify_hash_len, - p + 2, (size_t)( end - ( p + 2 ) ), &signature_len, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_sign", ret ); - return( ret ); - } - - MBEDTLS_PUT_UINT16_BE( signature_len, p, 0 ); - p += 2 + signature_len; - - *out_len = (size_t)( p - buf ); - - return( ret ); + return( 0 ); } int mbedtls_ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 1ddd74231..f680f57e8 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2054,7 +2054,7 @@ run_test "TLS1.3 opaque key: no suitable algorithm found" \ -c "key type: Opaque" \ -s "key types: Opaque, Opaque" \ -c "error" \ - -s "select_sig_alg_for_certificate_verify:no suitable signature algorithm found" \ + -s "no suitable signature algorithm" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_USE_PSA_CRYPTO @@ -11562,7 +11562,7 @@ run_test "TLS 1.3: Client authentication, client alg not in server list - ope -c "got a certificate request" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY" \ - -c "signature algorithm not in received or offered list." \ + -c "no suitable signature algorithm" \ -C "unknown pk type" requires_gnutls_tls1_3 @@ -11580,7 +11580,7 @@ run_test "TLS 1.3: Client authentication, client alg not in server list - gnu -c "got a certificate request" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY" \ - -c "signature algorithm not in received or offered list." \ + -c "no suitable signature algorithm" \ -C "unknown pk type" # Test using an opaque private key for client authentication @@ -11834,7 +11834,7 @@ run_test "TLS 1.3: Client authentication - opaque key, client alg not in serv -c "got a certificate request" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY" \ - -c "signature algorithm not in received or offered list." \ + -c "no suitable signature algorithm" \ -C "unkown pk type" requires_gnutls_tls1_3 @@ -11853,7 +11853,7 @@ run_test "TLS 1.3: Client authentication - opaque key, client alg not in serv -c "got a certificate request" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE" \ -c "client state: MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY" \ - -c "signature algorithm not in received or offered list." \ + -c "no suitable signature algorithm" \ -C "unkown pk type" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 @@ -12582,7 +12582,7 @@ run_test "TLS 1.3: Check signature algorithm order, m->O" \ sig_algs=rsa_pkcs1_sha512,rsa_pss_rsae_sha512,rsa_pss_rsae_sha384,ecdsa_secp256r1_sha256" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "select_sig_alg_for_certificate_verify:selected signature algorithm rsa_pss_rsae_sha512" \ + -c "CertificateVerify signature with rsa_pss_rsae_sha512" \ -c "HTTP/1.0 200 [Oo][Kk]" requires_gnutls_tls1_3 @@ -12598,7 +12598,7 @@ run_test "TLS 1.3: Check signature algorithm order, m->G" \ sig_algs=rsa_pkcs1_sha512,rsa_pss_rsae_sha512,rsa_pss_rsae_sha384,ecdsa_secp256r1_sha256" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "select_sig_alg_for_certificate_verify:selected signature algorithm rsa_pss_rsae_sha512" \ + -c "CertificateVerify signature with rsa_pss_rsae_sha512" \ -c "HTTP/1.0 200 [Oo][Kk]" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 @@ -12615,8 +12615,8 @@ run_test "TLS 1.3: Check signature algorithm order, m->m" \ sig_algs=rsa_pkcs1_sha512,rsa_pss_rsae_sha512,rsa_pss_rsae_sha384,ecdsa_secp256r1_sha256" \ 0 \ -c "Protocol is TLSv1.3" \ - -c "select_sig_alg_for_certificate_verify:selected signature algorithm rsa_pss_rsae_sha512" \ - -s "select_sig_alg_for_certificate_verify:selected signature algorithm rsa_pss_rsae_sha512" \ + -c "CertificateVerify signature with rsa_pss_rsae_sha512" \ + -s "CertificateVerify signature with rsa_pss_rsae_sha512" \ -s "ssl_tls13_pick_key_cert:selected signature algorithm rsa_pss_rsae_sha512" \ -c "HTTP/1.0 200 [Oo][Kk]" @@ -12635,7 +12635,7 @@ run_test "TLS 1.3: Check signature algorithm order, O->m" \ -sigalgs rsa_pkcs1_sha512:rsa_pss_rsae_sha512:rsa_pss_rsae_sha384:ecdsa_secp256r1_sha256" \ 0 \ -c "TLSv1.3" \ - -s "select_sig_alg_for_certificate_verify:selected signature algorithm rsa_pss_rsae_sha512" \ + -s "CertificateVerify signature with rsa_pss_rsae_sha512" \ -s "ssl_tls13_pick_key_cert:selected signature algorithm rsa_pss_rsae_sha512" requires_gnutls_tls1_3 @@ -12654,7 +12654,7 @@ run_test "TLS 1.3: Check signature algorithm order, G->m" \ 0 \ -c "Negotiated version: 3.4" \ -c "HTTP/1.0 200 [Oo][Kk]" \ - -s "select_sig_alg_for_certificate_verify:selected signature algorithm rsa_pss_rsae_sha512" \ + -s "CertificateVerify signature with rsa_pss_rsae_sha512" \ -s "ssl_tls13_pick_key_cert:selected signature algorithm rsa_pss_rsae_sha512" requires_gnutls_tls1_3 @@ -12758,7 +12758,7 @@ run_test "TLS 1.3: Check client no signature algorithm, m->O" \ "$P_CLI debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key \ sig_algs=rsa_pkcs1_sha512,rsa_pss_rsae_sha512,rsa_pss_rsae_sha384,ecdsa_secp256r1_sha256" \ 1 \ - -c "select_sig_alg_for_certificate_verify:no suitable signature algorithm found" + -c "no suitable signature algorithm" requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 @@ -12772,7 +12772,7 @@ run_test "TLS 1.3: Check client no signature algorithm, m->G" \ "$P_CLI debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key \ sig_algs=rsa_pkcs1_sha512,rsa_pss_rsae_sha512,rsa_pss_rsae_sha384,ecdsa_secp256r1_sha256" \ 1 \ - -c "select_sig_alg_for_certificate_verify:no suitable signature algorithm found" + -c "no suitable signature algorithm" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE @@ -12787,7 +12787,7 @@ run_test "TLS 1.3: Check client no signature algorithm, m->m" \ "$P_CLI debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key \ sig_algs=rsa_pkcs1_sha512,rsa_pss_rsae_sha512,rsa_pss_rsae_sha384,ecdsa_secp256r1_sha256" \ 1 \ - -c "select_sig_alg_for_certificate_verify:no suitable signature algorithm found" + -c "no suitable signature algorithm" requires_openssl_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3