From 6a3fa2159cec18939ce3523b8854566aa83deaff Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 19 Sep 2016 16:58:45 +0100 Subject: [PATCH 1/5] Fix sig->tag update in mbedtls_x509_get_sig() --- ChangeLog | 3 +++ library/x509.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index d28d2c7b1..97cdfc3be 100644 --- a/ChangeLog +++ b/ChangeLog @@ -22,6 +22,9 @@ Bugfix * Fix documentation and implementation missmatch for function arguments of mbedtls_gcm_finish(). Found by cmiatpaar. #602 * Guarantee that P>Q at RSA key generation. Found by inestlerode. #558 + * Fix mbedtls_x509_get_sig() to update the ASN1 type in the mbedtls_x509_buf + data structure until after error checks are successful. Found by + subramanyam-c. = mbed TLS 2.1.5 branch released 2016-06-28 diff --git a/library/x509.c b/library/x509.c index ffc3d6c94..8696a7e8a 100644 --- a/library/x509.c +++ b/library/x509.c @@ -554,16 +554,18 @@ int mbedtls_x509_get_sig( unsigned char **p, const unsigned char *end, mbedtls_x { int ret; size_t len; + int tag_type; if( ( end - *p ) < 1 ) return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + MBEDTLS_ERR_ASN1_OUT_OF_DATA ); - sig->tag = **p; + tag_type = **p; if( ( ret = mbedtls_asn1_get_bitstring_null( p, end, &len ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + ret ); + sig->tag = tag_type; sig->len = len; sig->p = *p; From 8390e0a97c18da6284f2888b61ddca6cac5ac192 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 13 Oct 2016 15:27:09 +0100 Subject: [PATCH 2/5] Update and clean up Changelog for #622 --- ChangeLog | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 97cdfc3be..1e882efee 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,12 +19,12 @@ Bugfix * Fixed the sample applications gen_key.c, cert_req.c and cert_write.c for builds where the configuration MBEDTLS_PEM_WRITE_C is not defined. Found by inestlerode. #559. + * Fix mbedtls_x509_get_sig() to update the ASN1 type in the mbedtls_x509_buf + data structure until after error checks are successful. Found by + subramanyam-c. #622 * Fix documentation and implementation missmatch for function arguments of mbedtls_gcm_finish(). Found by cmiatpaar. #602 * Guarantee that P>Q at RSA key generation. Found by inestlerode. #558 - * Fix mbedtls_x509_get_sig() to update the ASN1 type in the mbedtls_x509_buf - data structure until after error checks are successful. Found by - subramanyam-c. = mbed TLS 2.1.5 branch released 2016-06-28 From 6220ecbc485ffb935225bab298d7f707c33bacfa Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 26 Sep 2016 14:53:05 +0100 Subject: [PATCH 3/5] Fix overread when verifying SERVER_HELLO in DTLS --- ChangeLog | 2 ++ library/ssl_cli.c | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/ChangeLog b/ChangeLog index 1e882efee..def7e6d0f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,8 @@ Bugfix * Fix documentation and implementation missmatch for function arguments of mbedtls_gcm_finish(). Found by cmiatpaar. #602 * Guarantee that P>Q at RSA key generation. Found by inestlerode. #558 + * Fix potential byte overread when verifying malformed SERVER_HELLO in + ssl_parse_hello_verify_request() for DTLS. Found by Guido Vranken. = mbed TLS 2.1.5 branch released 2016-06-28 diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 16fd43b71..3d7283c3d 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1226,6 +1226,15 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) cookie_len = *p++; MBEDTLS_SSL_DEBUG_BUF( 3, "cookie", p, cookie_len ); + if( ( ssl->in_msg + ssl->in_msglen ) - p < cookie_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "cookie length does not match incoming message size" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); + } + mbedtls_free( ssl->handshake->verify_cookie ); ssl->handshake->verify_cookie = mbedtls_calloc( 1, cookie_len ); From 53d77130fc9021effbe21fa36f080a348f77f310 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Fri, 23 Sep 2016 13:16:02 +0100 Subject: [PATCH 4/5] Add check for validity of date in x509_get_time() --- ChangeLog | 2 ++ library/x509.c | 32 +++++++++++++++++++ tests/suites/test_suite_x509parse.data | 36 ++++++++++++++++++++++ tests/suites/test_suite_x509parse.function | 34 ++++++++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/ChangeLog b/ChangeLog index def7e6d0f..68cf97d0b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,8 @@ Bugfix * Guarantee that P>Q at RSA key generation. Found by inestlerode. #558 * Fix potential byte overread when verifying malformed SERVER_HELLO in ssl_parse_hello_verify_request() for DTLS. Found by Guido Vranken. + * Fix check for validity of date when parsing in mbedtls_x509_get_time(). + Found by subramanyam-c. = mbed TLS 2.1.5 branch released 2016-06-28 diff --git a/library/x509.c b/library/x509.c index 8696a7e8a..33bcb9ef0 100644 --- a/library/x509.c +++ b/library/x509.c @@ -75,6 +75,7 @@ #endif #define CHECK(code) if( ( ret = code ) != 0 ){ return( ret ); } +#define CHECK_RANGE(min, max, val) if( val < min || val > max ){ return( ret ); } /* * CertificateSerialNumber ::= INTEGER @@ -484,6 +485,33 @@ static int x509_parse_int(unsigned char **p, unsigned n, int *res){ return 0; } +static int x509_date_is_valid(const mbedtls_x509_time *time) +{ + int ret = MBEDTLS_ERR_X509_INVALID_DATE; + + CHECK_RANGE( 0, 9999, time->year ); + CHECK_RANGE( 0, 23, time->hour ); + CHECK_RANGE( 0, 59, time->min ); + CHECK_RANGE( 0, 59, time->sec ); + + switch( time->mon ) + { + case 1: case 3: case 5: case 7: case 8: case 10: case 12: + CHECK_RANGE( 1, 31, time->day ); + break; + case 4: case 6: case 9: case 11: + CHECK_RANGE( 1, 30, time->day ); + break; + case 2: + CHECK_RANGE( 1, 28 + (time->year % 4 == 0), time->day ); + break; + default: + return( ret ); + } + + return( 0 ); +} + /* * Time ::= CHOICE { * utcTime UTCTime, @@ -523,6 +551,8 @@ int mbedtls_x509_get_time( unsigned char **p, const unsigned char *end, time->year += 100 * ( time->year < 50 ); time->year += 1900; + CHECK( x509_date_is_valid( time ) ); + return( 0 ); } else if( tag == MBEDTLS_ASN1_GENERALIZED_TIME ) @@ -543,6 +573,8 @@ int mbedtls_x509_get_time( unsigned char **p, const unsigned char *end, if( len > 14 && *(*p)++ != 'Z' ) return( MBEDTLS_ERR_X509_INVALID_DATE ); + CHECK( x509_date_is_valid( time ) ); + return( 0 ); } else diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 5c68872c0..6511cef9f 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1476,3 +1476,39 @@ x509parse_crt_file:"data_files/server7_all_space.crt":MBEDTLS_ERR_PEM_INVALID_DA X509 File parse (trailing spaces, OK) depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED x509parse_crt_file:"data_files/server7_trailing_space.crt":0 + +X509 Get time (UTC no issues) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"500101000000Z":0:1950:1:1:0:0:0 + +X509 Get time (Generalized Time no issues) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_GENERALIZED_TIME:"99991231235959Z":0:9999:12:31:23:59:59 + +X509 Get time (UTC year without leap day) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"490229121212Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 + +X509 Get time (UTC year with leap day) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"000229121212Z":0:2000:2:29:12:12:12 + +X509 Get time (UTC invalid day of month #1) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"000132121212Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 + +X509 Get time (UTC invalid day of month #2) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"001131121212Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 + +X509 Get time (UTC invalid hour) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"001130241212Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 + +X509 Get time (UTC invalid min) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"001130236012Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 + +X509 Get time (UTC invalid sec) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_UTC_TIME:"001130235960Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index c476ec507..40e653d1d 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -1,4 +1,5 @@ /* BEGIN_HEADER */ +#include "mbedtls/x509.h" #include "mbedtls/x509_crt.h" #include "mbedtls/x509_crl.h" #include "mbedtls/x509_csr.h" @@ -590,6 +591,39 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_USE_C */ +void x509_get_time( int tag, char *time_str, int ret, + int year, int mon, int day, + int hour, int min, int sec ) +{ + mbedtls_x509_time time; + unsigned char buf[17]; + unsigned char* start = buf; + unsigned char* end = buf; + + memset( &time, 0x00, sizeof( time ) ); + *end = (unsigned char)tag; end++; + if( tag == MBEDTLS_ASN1_UTC_TIME ) + *end = 13; + else + *end = 15; + end++; + memcpy( end, time_str, (size_t)*(end - 1) ); + end += *(end - 1); + + TEST_ASSERT( mbedtls_x509_get_time( &start, end, &time ) == ret ); + if( ret == 0 ) + { + TEST_ASSERT( year == time.year ); + TEST_ASSERT( mon == time.mon ); + TEST_ASSERT( day == time.day ); + TEST_ASSERT( hour == time.hour ); + TEST_ASSERT( min == time.min ); + TEST_ASSERT( sec == time.sec ); + } +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT */ void x509_parse_rsassa_pss_params( char *hex_params, int params_tag, int ref_msg_md, int ref_mgf_md, From 8ee9d7658c4ab892bf24743e467a0af79ac02b01 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 13 Oct 2016 16:30:19 +0100 Subject: [PATCH 5/5] Update to Changelog for #626 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 68cf97d0b..374c6aa5c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,7 +28,7 @@ Bugfix * Fix potential byte overread when verifying malformed SERVER_HELLO in ssl_parse_hello_verify_request() for DTLS. Found by Guido Vranken. * Fix check for validity of date when parsing in mbedtls_x509_get_time(). - Found by subramanyam-c. + Found by subramanyam-c. #626 = mbed TLS 2.1.5 branch released 2016-06-28