diff --git a/CMakeLists.txt b/CMakeLists.txt index 890521853..499ccff90 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -111,7 +111,7 @@ if(ENABLE_TESTING) ADD_CUSTOM_TARGET(covtest COMMAND make test COMMAND programs/test/selftest - COMMAND tests/compat.sh + COMMAND tests/compat.sh -m 'tls1 tls1_1 tls1_2 dtls1 dtls1_2' COMMAND tests/ssl-opt.sh ) diff --git a/ChangeLog b/ChangeLog index 19547689c..d1a60c262 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,8 @@ Bugfix arguments where the same (in-place doubling). Found and fixed by Janos Follath. #309 * Fix issue in Makefile that prevented building using armar. #386 + * Fix bug in mbedtls_x509_crt_parse that caused trailing extra data in the + buffer after DER certificates to be included in the raw representation. Changes * On ARM platforms, when compiling with -O0 with GCC, Clang or armcc5, diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 4bcf94953..5147ec6c4 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1039,7 +1039,7 @@ * * Comment this macro to disable support for SSL 3.0 */ -#define MBEDTLS_SSL_PROTO_SSL3 +//#define MBEDTLS_SSL_PROTO_SSL3 /** * \def MBEDTLS_SSL_PROTO_TLS1 diff --git a/library/x509_crt.c b/library/x509_crt.c index 6dc5ad34f..a1ce2544e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -680,14 +680,9 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * if( crt == NULL || buf == NULL ) return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); - p = mbedtls_calloc( 1, len = buflen ); - if( p == NULL ) - return( MBEDTLS_ERR_X509_ALLOC_FAILED ); - - memcpy( p, buf, buflen ); - - crt->raw.p = p; - crt->raw.len = len; + // Use the original buffer until we figure out actual length + p = (unsigned char*) buf; + len = buflen; end = p + len; /* @@ -711,6 +706,18 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * } crt_end = p + len; + // Create and populate a new buffer for the raw field + crt->raw.len = crt_end - buf; + crt->raw.p = p = mbedtls_calloc( 1, crt->raw.len ); + if( p == NULL ) + return( MBEDTLS_ERR_X509_ALLOC_FAILED ); + + memcpy( p, buf, crt->raw.len ); + + // Direct pointers to the new buffer + p += crt->raw.len - len; + end = crt_end = p + len; + /* * TBSCertificate ::= SEQUENCE { */ diff --git a/tests/compat.sh b/tests/compat.sh index 4b43e33a5..a333a1916 100755 --- a/tests/compat.sh +++ b/tests/compat.sh @@ -45,7 +45,7 @@ else fi # default values for options -MODES="ssl3 tls1 tls1_1 tls1_2 dtls1 dtls1_2" +MODES="tls1 tls1_1 tls1_2 dtls1 dtls1_2" VERIFIES="NO YES" TYPES="ECDSA RSA PSK" FILTER="" diff --git a/tests/data_files/server5-der0.crt b/tests/data_files/server5-der0.crt new file mode 100644 index 000000000..08d8dd311 Binary files /dev/null and b/tests/data_files/server5-der0.crt differ diff --git a/tests/data_files/server5-der1a.crt b/tests/data_files/server5-der1a.crt new file mode 100644 index 000000000..015017b17 Binary files /dev/null and b/tests/data_files/server5-der1a.crt differ diff --git a/tests/data_files/server5-der1b.crt b/tests/data_files/server5-der1b.crt new file mode 100644 index 000000000..6340d9e2e Binary files /dev/null and b/tests/data_files/server5-der1b.crt differ diff --git a/tests/data_files/server5-der2.crt b/tests/data_files/server5-der2.crt new file mode 100644 index 000000000..c6e320a36 Binary files /dev/null and b/tests/data_files/server5-der2.crt differ diff --git a/tests/data_files/server5-der4.crt b/tests/data_files/server5-der4.crt new file mode 100644 index 000000000..4af05cce1 Binary files /dev/null and b/tests/data_files/server5-der4.crt differ diff --git a/tests/data_files/server5-der8.crt b/tests/data_files/server5-der8.crt new file mode 100644 index 000000000..65be7dcae Binary files /dev/null and b/tests/data_files/server5-der8.crt differ diff --git a/tests/data_files/server5-der9.crt b/tests/data_files/server5-der9.crt new file mode 100644 index 000000000..4947f1f83 Binary files /dev/null and b/tests/data_files/server5-der9.crt differ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c3369ea7d..5ef74cd79 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -111,6 +111,23 @@ make msg "test: compat.sh (ASan build)" # ~ 6 min tests/compat.sh +msg "build: Default + SSLv3 (ASan build)" # ~ 6 min +cleanup +cp "$CONFIG_H" "$CONFIG_BAK" +scripts/config.pl set MBEDTLS_SSL_PROTO_SSL3 +CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . +make + +msg "test: SSLv3 - main suites and selftest (ASan build)" # ~ 50s +make test +programs/test/selftest + +msg "build: SSLv3 - compat.sh (ASan build)" # ~ 6 min +tests/compat.sh -m 'ssl3 tls1 tls1_1 tls1_2 dtls1 dtls1_2' + +msg "build: SSLv3 - ssl-opt.sh (ASan build)" # ~ 6 min +tests/ssl-opt.sh + msg "build: cmake, full config, clang" # ~ 50s cleanup cp "$CONFIG_H" "$CONFIG_BAK" diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e49441301..8f49e9661 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -695,6 +695,7 @@ run_test "Encrypt then MAC: client disabled, server enabled" \ -C "using encrypt then mac" \ -S "using encrypt then mac" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Encrypt then MAC: client SSLv3, server enabled" \ "$P_SRV debug_level=3 min_version=ssl3 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ @@ -707,6 +708,7 @@ run_test "Encrypt then MAC: client SSLv3, server enabled" \ -C "using encrypt then mac" \ -S "using encrypt then mac" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Encrypt then MAC: client enabled, server SSLv3" \ "$P_SRV debug_level=3 force_version=ssl3 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ @@ -754,6 +756,7 @@ run_test "Extended Master Secret: client disabled, server enabled" \ -C "using extended master secret" \ -S "using extended master secret" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Extended Master Secret: client SSLv3, server enabled" \ "$P_SRV debug_level=3 min_version=ssl3" \ "$P_CLI debug_level=3 force_version=ssl3" \ @@ -765,6 +768,7 @@ run_test "Extended Master Secret: client SSLv3, server enabled" \ -C "using extended master secret" \ -S "using extended master secret" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Extended Master Secret: client enabled, server SSLv3" \ "$P_SRV debug_level=3 force_version=ssl3" \ "$P_CLI debug_level=3 min_version=ssl3" \ @@ -883,6 +887,7 @@ run_test "CBC Record splitting: TLS 1.0, splitting" \ -s "Read from client: 1 bytes read" \ -s "122 bytes read" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "CBC Record splitting: SSLv3, splitting" \ "$P_SRV min_version=ssl3" \ "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \ @@ -1554,6 +1559,64 @@ run_test "Renego ext: gnutls client unsafe, server break legacy" \ -S "received TLS_EMPTY_RENEGOTIATION_INFO\|found renegotiation extension" \ -S "server hello, secure renegotiation extension" +# Tests for silently dropping trailing extra bytes in .der certificates + +requires_gnutls +run_test "DER format: no trailing bytes" \ + "$P_SRV crt_file=data_files/server5-der0.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + +requires_gnutls +run_test "DER format: with a trailing zero byte" \ + "$P_SRV crt_file=data_files/server5-der1a.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + +requires_gnutls +run_test "DER format: with a trailing random byte" \ + "$P_SRV crt_file=data_files/server5-der1b.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + +requires_gnutls +run_test "DER format: with 2 trailing random bytes" \ + "$P_SRV crt_file=data_files/server5-der2.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + +requires_gnutls +run_test "DER format: with 4 trailing random bytes" \ + "$P_SRV crt_file=data_files/server5-der4.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + +requires_gnutls +run_test "DER format: with 8 trailing random bytes" \ + "$P_SRV crt_file=data_files/server5-der8.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + +requires_gnutls +run_test "DER format: with 9 trailing random bytes" \ + "$P_SRV crt_file=data_files/server5-der9.crt \ + key_file=data_files/server5.key" \ + "$G_CLI " \ + 0 \ + -c "Handshake was completed" \ + # Tests for auth_mode run_test "Authentication: server badcert, client required" \ @@ -1674,6 +1737,7 @@ run_test "Authentication: client no cert, openssl server optional" \ -c "skip write certificate verify" \ -C "! mbedtls_ssl_handshake returned" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Authentication: client no cert, ssl3" \ "$P_SRV debug_level=3 auth_mode=optional force_version=ssl3" \ "$P_CLI debug_level=3 crt_file=none key_file=none min_version=ssl3" \ @@ -2501,6 +2565,7 @@ run_test "PSK callback: wrong key" \ # Tests for ciphersuites per version +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Per-version suites: SSL3" \ "$P_SRV min_version=ssl3 version_suites=TLS-RSA-WITH-3DES-EDE-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \ "$P_CLI force_version=ssl3" \ @@ -2550,6 +2615,7 @@ run_test "mbedtls_ssl_get_bytes_avail: extra data" \ # Tests for small packets +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Small packet SSLv3 BlockCipher" \ "$P_SRV min_version=ssl3" \ "$P_CLI request_size=1 force_version=ssl3 \ @@ -2557,6 +2623,7 @@ run_test "Small packet SSLv3 BlockCipher" \ 0 \ -s "Read from client: 1 bytes read" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Small packet SSLv3 StreamCipher" \ "$P_SRV min_version=ssl3 arc4=1 force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \ "$P_CLI request_size=1 force_version=ssl3 \ @@ -2691,6 +2758,7 @@ run_test "Small packet TLS 1.2 AEAD shorter tag" \ # Test for large packets +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Large packet SSLv3 BlockCipher" \ "$P_SRV min_version=ssl3" \ "$P_CLI request_size=16384 force_version=ssl3 recsplit=0 \ @@ -2698,6 +2766,7 @@ run_test "Large packet SSLv3 BlockCipher" \ 0 \ -s "Read from client: 16384 bytes read" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Large packet SSLv3 StreamCipher" \ "$P_SRV min_version=ssl3 arc4=1 force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \ "$P_CLI request_size=16384 force_version=ssl3 \ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 2f2137f54..6b04ae37e 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -755,7 +755,7 @@ X509 Certificate ASN1 (Incorrect first tag) x509parse_crt:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT X509 Certificate ASN1 (Correct first tag, data length does not match) -x509parse_crt:"300000":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG +x509parse_crt:"300000":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_OUT_OF_DATA X509 Certificate ASN1 (Correct first tag, no more data) x509parse_crt:"3000":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_OUT_OF_DATA