From ad17fe9c377def269b4d96537f21427e4fddcdd2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 16 Aug 2018 15:51:34 +0100 Subject: [PATCH 1/4] Fix overly strict bounds check in ssl_parse_certificate_request() --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 321d6367a..466608375 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2721,7 +2721,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) * therefore the buffer length at this point must be greater than that * regardless of the actual code path. */ - if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n ) + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, From ad0fe92fb6e63673ad90c8618f096ccf5ba7b6db Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 16 Aug 2018 15:52:22 +0100 Subject: [PATCH 2/4] Adapt ChangeLog --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index abd5e61bb..f505b3886 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,9 @@ Bugfix * Add ecc extensions only if an ecc based ciphersuite is used. This improves compliance to RFC 4492, and as a result, solves interoperability issues with BouncyCastle. Raised by milenamil in #1157. + * Fix overly strict bounds check in ssl_parse_certificate_request() + which could lead to valid CertificateRequest messages being rejected. + Fixes #1954. Changes * Copy headers preserving timestamps when doing a "make install". From eb2b15accd4433cb15b144acff35a6328efa62f2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 17 Aug 2018 09:47:22 +0100 Subject: [PATCH 3/4] Improve ChangeLog wording for the commmit that Fixes #1954. --- ChangeLog | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index f505b3886..8260ad651 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,9 +10,11 @@ Bugfix * Add ecc extensions only if an ecc based ciphersuite is used. This improves compliance to RFC 4492, and as a result, solves interoperability issues with BouncyCastle. Raised by milenamil in #1157. - * Fix overly strict bounds check in ssl_parse_certificate_request() - which could lead to valid CertificateRequest messages being rejected. - Fixes #1954. + * Fix a bug that caused SSL/TLS clients to incorrectly abort the handshake + with TLS versions 1.1 and earlier when the server requested authentication + without providing a list of CAs. This was due to an overly strict bounds + check in parsing the CertificateRequest message, + introduced in Mbed TLS 2.12.0. Fixes #1954. Changes * Copy headers preserving timestamps when doing a "make install". From d26bb2090f86ea7068ce8493748cd7eaf5bbb66c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 17 Aug 2018 09:54:10 +0100 Subject: [PATCH 4/4] Add tests for empty CA list in CertificateRequest, TLS 1.0 & 1.1 --- tests/ssl-opt.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 937a27b76..58defbfcc 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -801,6 +801,22 @@ run_test "RC4: both enabled" \ -S "SSL - None of the common ciphersuites is usable" \ -S "SSL - The server has no ciphersuites in common" +# Test empty CA list in CertificateRequest in TLS 1.1 and earlier + +requires_gnutls +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +run_test "CertificateRequest with empty CA list, TLS 1.1 (GnuTLS server)" \ + "$G_SRV"\ + "$P_CLI force_version=tls1_1" \ + 0 + +requires_gnutls +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1 +run_test "CertificateRequest with empty CA list, TLS 1.0 (GnuTLS server)" \ + "$G_SRV"\ + "$P_CLI force_version=tls1" \ + 0 + # Tests for SHA-1 support requires_config_disabled MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES