From 96a0fd951f8995b381ba31b104ce971b0c56007a Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 8 Nov 2022 17:09:56 +0000 Subject: [PATCH 1/4] Fix signature algorithms list entry getting overwritten by length. Fix bug whereby the supported signature algorithm list sent by the server in the certificate request would not leave enough space for the length to be written, and thus the first element would get overwritten, leaving two random bytes in the last entry. Signed-off-by: Paul Elliott --- ChangeLog.d/fix-tls12server-sent-sigalgs.txt | 5 +++++ library/ssl_tls12_server.c | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/fix-tls12server-sent-sigalgs.txt diff --git a/ChangeLog.d/fix-tls12server-sent-sigalgs.txt b/ChangeLog.d/fix-tls12server-sent-sigalgs.txt new file mode 100644 index 0000000000..9abde2b521 --- /dev/null +++ b/ChangeLog.d/fix-tls12server-sent-sigalgs.txt @@ -0,0 +1,5 @@ +Bugfix + * Fix a bug whereby the the list of signature algorithms sent as part of the + TLS 1.2 server certificate request would get corrupted, meaning the first + algorithm would not get sent and an entry consisting of two random bytes + would be sent instead. Found by Serban Bejan and Dudek Sebastian. diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 71f703c7ff..3dab2467c6 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2531,10 +2531,15 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) if( ! mbedtls_ssl_sig_alg_is_supported( ssl, *sig_alg ) ) continue; - MBEDTLS_PUT_UINT16_BE( *sig_alg, p, sa_len ); + /* Write elements at offsets starting from 1 (offset 0 is for the + * length). Thus the offset of each element is the length of the + * partial list including that element. */ sa_len += 2; + MBEDTLS_PUT_UINT16_BE( *sig_alg, p, sa_len ); + } + /* Fill in list length. */ MBEDTLS_PUT_UINT16_BE( sa_len, p, 0 ); sa_len += 2; p += sa_len; From ec71b0937f74a84a781f955ecb10ca89ba1577b2 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 15 Nov 2022 10:21:50 -0500 Subject: [PATCH 2/4] Introduce a test for single signature algorithm correctness The value of the first sent signature algorithm is overwritten. This test forces only a single algorithm to be sent and then validates that the client received such algorithm. 04 03 is the expected value for SECP256R1_SHA256. Signed-off-by: Andrzej Kurek --- library/ssl_tls12_client.c | 2 +- tests/ssl-opt.sh | 29 +++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 1c53a09903..21b3ba6216 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -2654,7 +2654,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) for( size_t i = 0; i < sig_alg_len; i += 2 ) { MBEDTLS_SSL_DEBUG_MSG( 3, - ( "Supported Signature Algorithm found: %d,%d", + ( "Supported Signature Algorithm found: %02x %02x", sig_alg[i], sig_alg[i + 1] ) ); } #endif diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index fc892a18bc..c5d0d9ada6 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2371,6 +2371,19 @@ run_test "Unique IV in GCM" \ -u "IV used" \ -U "IV used" +# Test for correctness of sent single supported algorithm +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_hash_alg SHA_256 +run_test "Single supported algorithm sending" \ + "$P_SRV sig_algs=ecdsa_secp256r1_sha256 auth_mode=required" \ + "$P_CLI sig_algs=ecdsa_secp256r1_sha256 debug_level=3" \ + 0 \ + -c "Supported Signature Algorithm found: 04 03" + # Tests for certificate verification callback requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "Configuration-specific CRT verification callback" \ @@ -5274,8 +5287,8 @@ run_test "Authentication: client SHA256, server required" \ key_file=data_files/server6.key \ force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384" \ 0 \ - -c "Supported Signature Algorithm found: 4," \ - -c "Supported Signature Algorithm found: 5," + -c "Supported Signature Algorithm found: 04 " \ + -c "Supported Signature Algorithm found: 05 " requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT @@ -5285,8 +5298,8 @@ run_test "Authentication: client SHA384, server required" \ key_file=data_files/server6.key \ force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256" \ 0 \ - -c "Supported Signature Algorithm found: 4," \ - -c "Supported Signature Algorithm found: 5," + -c "Supported Signature Algorithm found: 04 " \ + -c "Supported Signature Algorithm found: 05 " requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Authentication: client has no cert, server required (TLS)" \ @@ -5687,8 +5700,8 @@ run_test "Authentication, CA callback: client SHA256, server required" \ force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384" \ 0 \ -s "use CA callback for X.509 CRT verification" \ - -c "Supported Signature Algorithm found: 4," \ - -c "Supported Signature Algorithm found: 5," + -c "Supported Signature Algorithm found: 04 " \ + -c "Supported Signature Algorithm found: 05 " requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 @@ -5700,8 +5713,8 @@ run_test "Authentication, CA callback: client SHA384, server required" \ force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256" \ 0 \ -s "use CA callback for X.509 CRT verification" \ - -c "Supported Signature Algorithm found: 4," \ - -c "Supported Signature Algorithm found: 5," + -c "Supported Signature Algorithm found: 04 " \ + -c "Supported Signature Algorithm found: 05 " requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 From 3b4cedaa713427e609bebfa59538a77d5eac74ce Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 17 Nov 2022 12:47:10 +0000 Subject: [PATCH 3/4] Add SSL_SRV requirement to test Signed-off-by: Paul Elliott --- tests/ssl-opt.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c5d0d9ada6..41dd491f70 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2376,6 +2376,7 @@ requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_ECDSA_C requires_hash_alg SHA_256 run_test "Single supported algorithm sending" \ From f6e342cae29c68c4e598962c3f9553aa7e1b92c2 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 17 Nov 2022 12:50:29 +0000 Subject: [PATCH 4/4] Add test for single signature alg with openssl Test supplied by Gilles Peskine. Also rename previous test to fit to naming pattern. Signed-off-by: Paul Elliott --- tests/ssl-opt.sh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 41dd491f70..f2e3f0cc73 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2379,12 +2379,23 @@ requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_ECDSA_C requires_hash_alg SHA_256 -run_test "Single supported algorithm sending" \ +run_test "Single supported algorithm sending: mbedtls client" \ "$P_SRV sig_algs=ecdsa_secp256r1_sha256 auth_mode=required" \ "$P_CLI sig_algs=ecdsa_secp256r1_sha256 debug_level=3" \ 0 \ -c "Supported Signature Algorithm found: 04 03" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED +requires_hash_alg SHA_256 +run_test "Single supported algorithm sending: openssl client" \ + "$P_SRV sig_algs=ecdsa_secp256r1_sha256 auth_mode=required" \ + "$O_CLI -cert data_files/server6.crt \ + -key data_files/server6.key" \ + 0 + # Tests for certificate verification callback requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "Configuration-specific CRT verification callback" \