From 364fd8bb717686367d11d38c55957251e31eb630 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Feb 2022 23:53:36 +0100 Subject: [PATCH 01/43] More SSL debug messages for ClientHello parsing In particular, be verbose when checking the ClientHello cookie in a possible DTLS reconnection. Signed-off-by: Gilles Peskine --- library/ssl_msg.c | 62 ++++++++++++++++++++++++++------------ library/ssl_tls12_server.c | 10 +++++- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 083c8d2e6..a77a1a821 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3139,8 +3139,8 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C) /* - * Without any SSL context, check if a datagram looks like a ClientHello with - * a valid cookie, and if it doesn't, generate a HelloVerifyRequest message. + * Check if a datagram looks like a ClientHello with a valid cookie, + * and if it doesn't, generate a HelloVerifyRequest message. * Both input and output include full DTLS headers. * * - if cookie is valid, return 0 @@ -3150,9 +3150,7 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ) * - otherwise return a specific error code */ static int ssl_check_dtls_clihlo_cookie( - mbedtls_ssl_cookie_write_t *f_cookie_write, - mbedtls_ssl_cookie_check_t *f_cookie_check, - void *p_cookie, + mbedtls_ssl_context *ssl, const unsigned char *cli_id, size_t cli_id_len, const unsigned char *in, size_t in_len, unsigned char *obuf, size_t buf_len, size_t *olen ) @@ -3186,26 +3184,52 @@ static int ssl_check_dtls_clihlo_cookie( * * Minimum length is 61 bytes. */ - if( in_len < 61 || - in[0] != MBEDTLS_SSL_MSG_HANDSHAKE || + MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: in_len=%u", + (unsigned) in_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 4, "cli_id", cli_id, cli_id_len ); + if( in_len < 61 ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: record too short" ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + if( in[0] != MBEDTLS_SSL_MSG_HANDSHAKE || in[3] != 0 || in[4] != 0 || in[19] != 0 || in[20] != 0 || in[21] != 0 ) { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: not a good ClientHello" ) ); + MBEDTLS_SSL_DEBUG_MSG( 4, ( " type=%u epoch=%u fragment_offset=%u", + in[0], + (unsigned) in[3] << 8 | in[4], + (unsigned) in[19] << 16 | in[20] << 8 | in[21] ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } sid_len = in[59]; if( sid_len > in_len - 61 ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: sid_len=%u > %u", + (unsigned) sid_len, + (unsigned) in_len - 61 ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + MBEDTLS_SSL_DEBUG_BUF( 4, "sid received from network", + in + 60, sid_len ); cookie_len = in[60 + sid_len]; - if( cookie_len > in_len - 60 ) + if( cookie_len > in_len - 60 ) { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: cookie_len=%u > %u", + (unsigned) cookie_len, + (unsigned) in_len - 60 ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } - if( f_cookie_check( p_cookie, in + sid_len + 61, cookie_len, - cli_id, cli_id_len ) == 0 ) + MBEDTLS_SSL_DEBUG_BUF( 4, "cookie received from network", + in + sid_len + 61, cookie_len ); + if( ssl->conf->f_cookie_check( ssl->conf->p_cookie, + in + sid_len + 61, cookie_len, + cli_id, cli_id_len ) == 0 ) { - /* Valid cookie */ + MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: valid" ) ); return( 0 ); } @@ -3240,8 +3264,9 @@ static int ssl_check_dtls_clihlo_cookie( /* Generate and write actual cookie */ p = obuf + 28; - if( f_cookie_write( p_cookie, - &p, obuf + buf_len, cli_id, cli_id_len ) != 0 ) + if( ssl->conf->f_cookie_write( ssl->conf->p_cookie, + &p, obuf + buf_len, + cli_id, cli_id_len ) != 0 ) { return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } @@ -3296,9 +3321,7 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) } ret = ssl_check_dtls_clihlo_cookie( - ssl->conf->f_cookie_write, - ssl->conf->f_cookie_check, - ssl->conf->p_cookie, + ssl, ssl->cli_id, ssl->cli_id_len, ssl->in_buf, ssl->in_left, ssl->out_buf, MBEDTLS_SSL_OUT_CONTENT_LEN, &len ); @@ -3481,7 +3504,6 @@ static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, /* * Parse and validate record version */ - rec->ver[0] = buf[ rec_hdr_version_offset + 0 ]; rec->ver[1] = buf[ rec_hdr_version_offset + 1 ]; tls_version = mbedtls_ssl_read_version( buf + rec_hdr_version_offset, @@ -3489,10 +3511,12 @@ static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, if( tls_version > ssl->conf->max_tls_version ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS version mismatch" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS version mismatch: got %u, expected max %u", + (unsigned) tls_version, + (unsigned) ssl->conf->max_tls_version) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } - /* * Parse/Copy record sequence number. */ diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index a1505d16a..58d609335 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1286,7 +1286,10 @@ read_record_header: if( buf[1] != 0 || msg_len != mbedtls_ssl_hs_hdr_len( ssl ) + ( ( buf[2] << 8 ) | buf[3] ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != %u + %u", + (unsigned) msg_len, + (unsigned) mbedtls_ssl_hs_hdr_len( ssl ), + (unsigned) ( buf[2] << 8 ) | buf[3] ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } @@ -1327,6 +1330,11 @@ read_record_header: * For now we don't support fragmentation, so make sure * fragment_offset == 0 and fragment_length == length */ + MBEDTLS_SSL_DEBUG_MSG( + 4, ( "fragment_offset=%u fragment_length=%u length=%u", + (unsigned) ( ssl->in_msg[6] << 16 | ssl->in_msg[7] << 8 | ssl->in_msg[8] ), + (unsigned) ( ssl->in_msg[9] << 16 | ssl->in_msg[10] << 8 | ssl->in_msg[11] ), + (unsigned) ( ssl->in_msg[1] << 16 | ssl->in_msg[2] << 8 | ssl->in_msg[3] ) ) ); if( ssl->in_msg[6] != 0 || ssl->in_msg[7] != 0 || ssl->in_msg[8] != 0 || memcmp( ssl->in_msg + 1, ssl->in_msg + 9, 3 ) != 0 ) { From c8183cc4924e25a205db6f42150fd075cdeb83c8 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 6 Jun 2022 14:42:41 -0400 Subject: [PATCH 02/43] Add missing sid_len in calculations of cookie sizes This could lead to a potential buffer overread with small MBEDTLS_SSL_IN_CONTENT_LEN. Change the bound calculations so that it is apparent what lengths and sizes are used. Signed-off-by: Andrzej Kurek --- library/ssl_msg.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index a77a1a821..da4dbc721 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3205,7 +3205,7 @@ static int ssl_check_dtls_clihlo_cookie( } sid_len = in[59]; - if( sid_len > in_len - 61 ) + if( 59 + 1 + sid_len + 1 > in_len ) { MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: sid_len=%u > %u", (unsigned) sid_len, @@ -3216,10 +3216,11 @@ static int ssl_check_dtls_clihlo_cookie( in + 60, sid_len ); cookie_len = in[60 + sid_len]; - if( cookie_len > in_len - 60 ) { + if( 59 + 1 + sid_len + 1 + cookie_len > in_len ) + { MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: cookie_len=%u > %u", (unsigned) cookie_len, - (unsigned) in_len - 60 ) ); + (unsigned) ( in_len - sid_len - 61 ) ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } From cfb01948c86e5a13ca164dd69a576264ff71583e Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 6 Jun 2022 13:08:23 -0400 Subject: [PATCH 03/43] Add cookie parsing tests to test_suite_ssl Signed-off-by: Andrzej Kurek --- library/ssl_misc.h | 8 ++++++++ library/ssl_msg.c | 5 ++++- tests/suites/test_suite_ssl.data | 18 ++++++++++++++++++ tests/suites/test_suite_ssl.function | 27 +++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index c2c3cd210..1a3217c6c 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2296,4 +2296,12 @@ int mbedtls_ssl_validate_ciphersuite( int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); +#if defined(MBEDTLS_TEST_HOOKS) +int ssl_check_dtls_clihlo_cookie( + mbedtls_ssl_context *ssl, + const unsigned char *cli_id, size_t cli_id_len, + const unsigned char *in, size_t in_len, + unsigned char *obuf, size_t buf_len, size_t *olen ); +#endif + #endif /* ssl_misc.h */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index da4dbc721..ee6a01be5 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3149,7 +3149,10 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ) * return MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED * - otherwise return a specific error code */ -static int ssl_check_dtls_clihlo_cookie( +#if !defined(MBEDTLS_TEST_HOOKS) +static +#endif +int ssl_check_dtls_clihlo_cookie( mbedtls_ssl_context *ssl, const unsigned char *cli_id, size_t cli_id_len, const unsigned char *in, size_t in_len, diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 848a497cb..b6a46b2a0 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3366,3 +3366,21 @@ raw_key_agreement_fail:0 Raw key agreement: bad server key raw_key_agreement_fail:1 + +Cookie parsing: nominal run +cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d00200000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_SSL_INTERNAL_ERROR + +Cookie parsing: cookie_len overflow +cookie_parsing:"16fefd000000000000000000ea010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727db97b7373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737373737db963":MBEDTLS_ERR_SSL_DECODE_ERROR + +Cookie parsing: non-zero fragment offset +cookie_parsing:"16fefd00000000000000000032010000de000072000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d01730143":MBEDTLS_ERR_SSL_DECODE_ERROR + +Cookie parsing: sid_len overflow +cookie_parsing:"16fefd00000000000000000032010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727dFF730143":MBEDTLS_ERR_SSL_DECODE_ERROR + +Cookie parsing: record too short +cookie_parsing:"16fefd0000000000000000002f010000de000072000000011efefd7b7272727272727272727272727272727272727272727272727272727272727dFF":MBEDTLS_ERR_SSL_DECODE_ERROR + +Cookie parsing: one byte overread +cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d0001":MBEDTLS_ERR_SSL_DECODE_ERROR diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 35f1638cb..51d57a057 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5506,6 +5506,33 @@ void conf_group() } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS */ +void cookie_parsing( data_t *cookie, int exp_ret ) +{ + mbedtls_ssl_context ssl; + mbedtls_ssl_config conf; + size_t len; + + mbedtls_ssl_init( &ssl ); + mbedtls_ssl_config_init( &conf ); + TEST_EQUAL( mbedtls_ssl_config_defaults( &conf, MBEDTLS_SSL_IS_SERVER, + MBEDTLS_SSL_TRANSPORT_DATAGRAM, + MBEDTLS_SSL_PRESET_DEFAULT ), + 0 ); + + TEST_EQUAL( mbedtls_ssl_setup( &ssl, &conf ), 0 ); + TEST_EQUAL( ssl_check_dtls_clihlo_cookie( &ssl, ssl.cli_id, ssl.cli_id_len, + cookie->x, cookie->len, + ssl.out_buf, + MBEDTLS_SSL_OUT_CONTENT_LEN, + &len ), + exp_ret ); + + mbedtls_ssl_free( &ssl ); + mbedtls_ssl_config_free( &conf ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_TIMING_C:MBEDTLS_HAVE_TIME */ void timing_final_delay_accessor( ) { From e6487ab490edbc28618270164ca0503bb02018ea Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 6 Jun 2022 14:54:58 -0400 Subject: [PATCH 04/43] Add a changelog entry for the cookie parsing bounds bug Co-authored-by: Gilles Peskine Signed-off-by: Andrzej Kurek --- ChangeLog.d/cookie_parsing_bug.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 ChangeLog.d/cookie_parsing_bug.txt diff --git a/ChangeLog.d/cookie_parsing_bug.txt b/ChangeLog.d/cookie_parsing_bug.txt new file mode 100644 index 000000000..a5f5875d3 --- /dev/null +++ b/ChangeLog.d/cookie_parsing_bug.txt @@ -0,0 +1,11 @@ +Security + * Fix a buffer overread in DTLS ClientHello parsing in servers with + MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE enabled. An unauthenticated client + or a man-in-the-middle could cause a DTLS server to read up to 255 bytes + after the end of the SSL input buffer. The buffer overread only happens + when MBEDTLS_SSL_IN_CONTENT_LEN is less than a threshold that depends on + the exact configuration: 258 bytes if using mbedtls_ssl_cookie_check(), + and possibly up to 571 bytes with a custom cookie check function. + If the function provider deliberately omits these size checks, he/she + is responsible for the negative impact on his/her code. + Reported by the Cybeats PSI Team. From 078e9bcda6a0ccc355dcff9b0adf87edd1fdfdff Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 8 Jun 2022 11:47:33 -0400 Subject: [PATCH 05/43] Add the mbedtls prefix to ssl_check_dtls_clihlo_cookie Signed-off-by: Andrzej Kurek --- library/ssl_misc.h | 2 +- library/ssl_msg.c | 10 ++++------ tests/suites/test_suite_ssl.function | 11 ++++++----- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 1a3217c6c..f07f9d452 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2297,7 +2297,7 @@ int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); #if defined(MBEDTLS_TEST_HOOKS) -int ssl_check_dtls_clihlo_cookie( +int mbedtls_ssl_check_dtls_clihlo_cookie( mbedtls_ssl_context *ssl, const unsigned char *cli_id, size_t cli_id_len, const unsigned char *in, size_t in_len, diff --git a/library/ssl_msg.c b/library/ssl_msg.c index ee6a01be5..681e431e6 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3149,10 +3149,8 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ) * return MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED * - otherwise return a specific error code */ -#if !defined(MBEDTLS_TEST_HOOKS) -static -#endif -int ssl_check_dtls_clihlo_cookie( +MBEDTLS_STATIC_TESTABLE +int mbedtls_ssl_check_dtls_clihlo_cookie( mbedtls_ssl_context *ssl, const unsigned char *cli_id, size_t cli_id_len, const unsigned char *in, size_t in_len, @@ -3324,13 +3322,13 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) return( 0 ); } - ret = ssl_check_dtls_clihlo_cookie( + ret = mbedtls_ssl_check_dtls_clihlo_cookie( ssl, ssl->cli_id, ssl->cli_id_len, ssl->in_buf, ssl->in_left, ssl->out_buf, MBEDTLS_SSL_OUT_CONTENT_LEN, &len ); - MBEDTLS_SSL_DEBUG_RET( 2, "ssl_check_dtls_clihlo_cookie", ret ); + MBEDTLS_SSL_DEBUG_RET( 2, "mbedtls_ssl_check_dtls_clihlo_cookie", ret ); if( ret == MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED ) { diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 51d57a057..67880da3a 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5521,11 +5521,12 @@ void cookie_parsing( data_t *cookie, int exp_ret ) 0 ); TEST_EQUAL( mbedtls_ssl_setup( &ssl, &conf ), 0 ); - TEST_EQUAL( ssl_check_dtls_clihlo_cookie( &ssl, ssl.cli_id, ssl.cli_id_len, - cookie->x, cookie->len, - ssl.out_buf, - MBEDTLS_SSL_OUT_CONTENT_LEN, - &len ), + TEST_EQUAL( mbedtls_ssl_check_dtls_clihlo_cookie( &ssl, ssl.cli_id, + ssl.cli_id_len, + cookie->x, cookie->len, + ssl.out_buf, + MBEDTLS_SSL_OUT_CONTENT_LEN, + &len ), exp_ret ); mbedtls_ssl_free( &ssl ); From b58cf0d172cf3d6a4a27d16ab29ecf43be7546db Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 8 Jun 2022 11:53:59 -0400 Subject: [PATCH 06/43] Split a debug message into two - for clarity Signed-off-by: Andrzej Kurek --- library/ssl_tls12_server.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 58d609335..4e0756988 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1282,9 +1282,14 @@ read_record_header: MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, handshake len.: %d", ( buf[1] << 16 ) | ( buf[2] << 8 ) | buf[3] ) ); + if( buf[1] != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != 0", + (unsigned) buf[1] ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } /* We don't support fragmentation of ClientHello (yet?) */ - if( buf[1] != 0 || - msg_len != mbedtls_ssl_hs_hdr_len( ssl ) + ( ( buf[2] << 8 ) | buf[3] ) ) + if( msg_len != mbedtls_ssl_hs_hdr_len( ssl ) + ( ( buf[2] << 8 ) | buf[3] ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != %u + %u", (unsigned) msg_len, From ed4d217874f3c2d31eeb38f0f586907cfdd85b7a Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 8 Jun 2022 11:57:57 -0400 Subject: [PATCH 07/43] Add missing test dependencies for cookie parsing Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 67880da3a..2c7ecd55b 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5506,7 +5506,7 @@ void conf_group() } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE:MBEDTLS_TEST_HOOKS */ void cookie_parsing( data_t *cookie, int exp_ret ) { mbedtls_ssl_context ssl; From ae25bb043c140ccfc6b6c2c0e523a062a966a275 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 9 Jun 2022 19:32:46 +0200 Subject: [PATCH 08/43] Fix null pointer dereference in mpi_mod_int(0, 2) Fix a null pointer dereference when performing some operations on zero represented with 0 limbs: mbedtls_mpi_mod_int() dividing by 2, or mbedtls_mpi_write_string() in base 2. Signed-off-by: Gilles Peskine --- ChangeLog.d/bignum-0-mod-2.txt | 4 ++++ library/bignum.c | 2 +- tests/suites/test_suite_mpi.data | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/bignum-0-mod-2.txt diff --git a/ChangeLog.d/bignum-0-mod-2.txt b/ChangeLog.d/bignum-0-mod-2.txt new file mode 100644 index 000000000..55e53e5ff --- /dev/null +++ b/ChangeLog.d/bignum-0-mod-2.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix a null pointer dereference when performing some operations on zero + represented with 0 limbs: mbedtls_mpi_mod_int() dividing by 2, or + mbedtls_mpi_write_string() in base 2. diff --git a/library/bignum.c b/library/bignum.c index f06eff09b..7c0033ebd 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1785,7 +1785,7 @@ int mbedtls_mpi_mod_int( mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_ /* * handle trivial cases */ - if( b == 1 ) + if( b == 1 || A->n == 0 ) { *r = 0; return( 0 ); diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index 02a11c894..056310ad7 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -67,12 +67,18 @@ mpi_read_write_string:16:"":16:"":4:0:0 Test mpi_read_write_string #9 (Empty MPI hex -> dec) mpi_read_write_string:16:"":10:"0":4:0:0 +Test mpi_read_write_string #9 (Empty MPI hex -> base 2) +mpi_read_write_string:16:"":2:"0":4:0:0 + Test mpi_read_write_string #8 (Empty MPI dec -> hex) mpi_read_write_string:10:"":16:"":4:0:0 Test mpi_read_write_string #9 (Empty MPI dec -> dec) mpi_read_write_string:10:"":10:"0":4:0:0 +Test mpi_read_write_string #9 (Empty MPI dec -> base 2) +mpi_read_write_string:16:"":2:"0":4:0:0 + Test mpi_write_string #10 (Negative hex with odd number of digits) mpi_read_write_string:16:"-1":16:"":3:0:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL @@ -1216,9 +1222,15 @@ mbedtls_mpi_mod_int:10:"1000":2:0:0 Test mbedtls_mpi_mod_int: 0 (null) % 1 mbedtls_mpi_mod_int:16:"":1:0:0 +Test mbedtls_mpi_mod_int: 0 (null) % 2 +mbedtls_mpi_mod_int:16:"":2:0:0 + Test mbedtls_mpi_mod_int: 0 (null) % -1 mbedtls_mpi_mod_int:16:"":-1:0:MBEDTLS_ERR_MPI_NEGATIVE_VALUE +Test mbedtls_mpi_mod_int: 0 (null) % -2 +mbedtls_mpi_mod_int:16:"":-2:0:MBEDTLS_ERR_MPI_NEGATIVE_VALUE + Base test mbedtls_mpi_exp_mod #1 mbedtls_mpi_exp_mod:10:"23":10:"13":10:"29":10:"24":0 From 96d5439da5dc69419604a44a021bb77f55c3820e Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 8 Jun 2022 12:00:52 -0400 Subject: [PATCH 09/43] Fix incorrect changelog entry Signed-off-by: Andrzej Kurek --- ChangeLog.d/cookie_parsing_bug.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/ChangeLog.d/cookie_parsing_bug.txt b/ChangeLog.d/cookie_parsing_bug.txt index a5f5875d3..1c25f3952 100644 --- a/ChangeLog.d/cookie_parsing_bug.txt +++ b/ChangeLog.d/cookie_parsing_bug.txt @@ -6,6 +6,4 @@ Security when MBEDTLS_SSL_IN_CONTENT_LEN is less than a threshold that depends on the exact configuration: 258 bytes if using mbedtls_ssl_cookie_check(), and possibly up to 571 bytes with a custom cookie check function. - If the function provider deliberately omits these size checks, he/she - is responsible for the negative impact on his/her code. Reported by the Cybeats PSI Team. From cbe14ec96755121037c48c39d0bbe3e347ac63ab Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 15 Jun 2022 07:17:28 -0400 Subject: [PATCH 10/43] Improve variable extracting operations by using MBEDTLS_GET macros Signed-off-by: Andrzej Kurek --- library/ssl_msg.c | 16 +++++---- library/ssl_tls12_server.c | 66 +++++++++++++++++++++----------------- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 681e431e6..5aa098ec5 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3156,7 +3156,7 @@ int mbedtls_ssl_check_dtls_clihlo_cookie( const unsigned char *in, size_t in_len, unsigned char *obuf, size_t buf_len, size_t *olen ) { - size_t sid_len, cookie_len; + size_t sid_len, cookie_len, epoch, fragment_offset; unsigned char *p; /* @@ -3193,15 +3193,17 @@ int mbedtls_ssl_check_dtls_clihlo_cookie( MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: record too short" ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - if( in[0] != MBEDTLS_SSL_MSG_HANDSHAKE || - in[3] != 0 || in[4] != 0 || - in[19] != 0 || in[20] != 0 || in[21] != 0 ) + + epoch = MBEDTLS_GET_UINT16_BE( in, 3 ); + fragment_offset = MBEDTLS_GET_UINT24_BE( in, 19 ); + + if( in[0] != MBEDTLS_SSL_MSG_HANDSHAKE || epoch != 0 || + fragment_offset != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 4, ( "check cookie: not a good ClientHello" ) ); MBEDTLS_SSL_DEBUG_MSG( 4, ( " type=%u epoch=%u fragment_offset=%u", - in[0], - (unsigned) in[3] << 8 | in[4], - (unsigned) in[19] << 16 | in[20] << 8 | in[21] ) ); + in[0], (unsigned) epoch, + (unsigned) fragment_offset ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 4e0756988..7f8047beb 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1278,24 +1278,29 @@ read_record_header: MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, handshake len.: %d", - ( buf[1] << 16 ) | ( buf[2] << 8 ) | buf[3] ) ); - - if( buf[1] != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != 0", - (unsigned) buf[1] ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - /* We don't support fragmentation of ClientHello (yet?) */ - if( msg_len != mbedtls_ssl_hs_hdr_len( ssl ) + ( ( buf[2] << 8 ) | buf[3] ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != %u + %u", + size_t handshake_len = MBEDTLS_GET_UINT24_BE( buf, 1 ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, handshake len.: %d", + ( unsigned ) handshake_len ) ); + + /* The record layer has a record size limit of 2^14 - 1 and + * fragmentation is not supported, so buf[1] should be zero. */ + if( buf[1] != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != 0", + (unsigned) buf[1] ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + /* We don't support fragmentation of ClientHello (yet?) */ + if( msg_len != mbedtls_ssl_hs_hdr_len( ssl ) + handshake_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message: %u != %u + %u", (unsigned) msg_len, (unsigned) mbedtls_ssl_hs_hdr_len( ssl ), - (unsigned) ( buf[2] << 8 ) | buf[3] ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + (unsigned) handshake_len ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } } #if defined(MBEDTLS_SSL_PROTO_DTLS) @@ -1330,21 +1335,24 @@ read_record_header: ssl->handshake->out_msg_seq = cli_msg_seq; ssl->handshake->in_msg_seq = cli_msg_seq + 1; } - - /* - * For now we don't support fragmentation, so make sure - * fragment_offset == 0 and fragment_length == length - */ - MBEDTLS_SSL_DEBUG_MSG( - 4, ( "fragment_offset=%u fragment_length=%u length=%u", - (unsigned) ( ssl->in_msg[6] << 16 | ssl->in_msg[7] << 8 | ssl->in_msg[8] ), - (unsigned) ( ssl->in_msg[9] << 16 | ssl->in_msg[10] << 8 | ssl->in_msg[11] ), - (unsigned) ( ssl->in_msg[1] << 16 | ssl->in_msg[2] << 8 | ssl->in_msg[3] ) ) ); - if( ssl->in_msg[6] != 0 || ssl->in_msg[7] != 0 || ssl->in_msg[8] != 0 || - memcmp( ssl->in_msg + 1, ssl->in_msg + 9, 3 ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "ClientHello fragmentation not supported" ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + /* + * For now we don't support fragmentation, so make sure + * fragment_offset == 0 and fragment_length == length + */ + size_t fragment_offset, fragment_length, length; + fragment_offset = MBEDTLS_GET_UINT24_BE( ssl->in_msg, 6 ); + fragment_length = MBEDTLS_GET_UINT24_BE( ssl->in_msg, 9 ); + length = MBEDTLS_GET_UINT24_BE( ssl->in_msg, 1 ); + MBEDTLS_SSL_DEBUG_MSG( + 4, ( "fragment_offset=%u fragment_length=%u length=%u", + (unsigned) fragment_offset, (unsigned) fragment_length, + (unsigned) length ) ); + if( fragment_offset != 0 || length != fragment_length ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ClientHello fragmentation not supported" ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } } } #endif /* MBEDTLS_SSL_PROTO_DTLS */ From ca35f5bed0e7b56c88fe918ceb9fba7ea912011f Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 15 Jun 2022 07:19:40 -0400 Subject: [PATCH 11/43] test_suite_ssl: Use a zero fragment offset in a test with a too short record Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index b6a46b2a0..4db96037f 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3380,7 +3380,7 @@ Cookie parsing: sid_len overflow cookie_parsing:"16fefd00000000000000000032010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727dFF730143":MBEDTLS_ERR_SSL_DECODE_ERROR Cookie parsing: record too short -cookie_parsing:"16fefd0000000000000000002f010000de000072000000011efefd7b7272727272727272727272727272727272727272727272727272727272727dFF":MBEDTLS_ERR_SSL_DECODE_ERROR +cookie_parsing:"16fefd0000000000000000002f010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727dFF":MBEDTLS_ERR_SSL_DECODE_ERROR Cookie parsing: one byte overread cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d0001":MBEDTLS_ERR_SSL_DECODE_ERROR From 755ddff25c68a97cd4e8ac0c5b23761d4ac6a154 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 15 Jun 2022 07:31:40 -0400 Subject: [PATCH 12/43] Fix print format in a debug message Signed-off-by: Andrzej Kurek --- library/ssl_tls12_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 7f8047beb..ac36f04d2 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1280,7 +1280,7 @@ read_record_header: } { size_t handshake_len = MBEDTLS_GET_UINT24_BE( buf, 1 ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, handshake len.: %d", + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, handshake len.: %u", ( unsigned ) handshake_len ) ); /* The record layer has a record size limit of 2^14 - 1 and From 903c97937608d1dfd0481408c89b57cdf430b72b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Jun 2022 16:55:31 +0200 Subject: [PATCH 13/43] programs: ssl: Add one RSA PSS signature algorithm Add one RSA PSS signature algorithm to the test list of signature algorithms. This allows certificate chains exposing an RSA key with signatures using SHA-1 to be used in tests where an TLS 1.3 handshake is performed. Signed-off-by: Ronald Cron --- programs/ssl/ssl_test_common_source.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index 0e66895db..c6a9a70f4 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -285,6 +285,9 @@ uint16_t ssl_sig_algs_for_test[] = { #if defined(MBEDTLS_SHA224_C) MBEDTLS_SSL_SIG_ALG( MBEDTLS_SSL_HASH_SHA224 ) #endif +#if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_SHA256_C) + MBEDTLS_TLS1_3_SIG_RSA_PSS_RSAE_SHA256, +#endif /* MBEDTLS_RSASSA_C && MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA1_C) /* Allow SHA-1 as we use it extensively in tests. */ MBEDTLS_SSL_SIG_ALG( MBEDTLS_SSL_HASH_SHA1 ) From d28f5a98f158fd5cdac3ac42cc58cc60c2b2f5a2 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Jun 2022 19:27:25 +0200 Subject: [PATCH 14/43] ssl-opt.sh: Add certificate key usage tests for TLS 1.3 Those are adaptations of the already existing TLS 1.2 tests. It is not really possible to just remove the TLS 1.2 dependency of the existing tests because of the following: . in TLS 1.3 the ciphersuite selection on server side is not related to the server certificate . for tests involving OpenSSL the OpenSSL command line as to be adapted to TLS 1.3 . server authentication is mandatory in TLS 1.3 . a key with KeyEncipherment and not DigitalSignature usage is never acceptable Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 209 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 208 insertions(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 9ec2ffa30..c0d8b49ad 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6016,7 +6016,6 @@ run_test "keyUsage srv: RSA, digitalSignature -> (EC)DHE-RSA" \ 0 \ -c "Ciphersuite is TLS-[EC]*DHE-RSA-WITH-" - requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "keyUsage srv: RSA, keyEncipherment -> RSA" \ "$P_SRV key_file=data_files/server2.key \ @@ -6151,6 +6150,78 @@ run_test "keyUsage cli: DigitalSignature, RSA: fail, soft" \ -c "Ciphersuite is TLS-" \ -c "! Usage does not match the keyUsage extension" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli 1.3: DigitalSignature+KeyEncipherment, RSA: OK" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server2.key \ + -cert data_files/server2.ku-ds_ke.crt" \ + "$P_CLI debug_level=3" \ + 0 \ + -C "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli 1.3: KeyEncipherment, RSA: KO" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + "$P_CLI debug_level=1" \ + 1 \ + -c "bad certificate (usage extensions)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli 1.3: KeyAgreement, RSA: KO" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server2.key \ + -cert data_files/server2.ku-ka.crt" \ + "$P_CLI debug_level=1" \ + 1 \ + -c "bad certificate (usage extensions)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli 1.3: DigitalSignature, ECDSA: OK" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.ku-ds.crt" \ + "$P_CLI debug_level=3" \ + 0 \ + -C "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: KO" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.ku-ke.crt" \ + "$P_CLI debug_level=1" \ + 1 \ + -c "bad certificate (usage extensions)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: KO" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.ku-ka.crt" \ + "$P_CLI debug_level=1" \ + 1 \ + -c "bad certificate (usage extensions)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is" + # Tests for keyUsage in leaf certificates, part 3: # server-side checking of client cert @@ -6199,6 +6270,50 @@ run_test "keyUsage cli-auth: ECDSA, KeyAgreement: fail (soft)" \ -s "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature: OK" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server2.key \ + -cert data_files/server2.ku-ds.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (soft)" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli-auth 1.3: ECDSA, DigitalSignature: OK" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ + -cert data_files/server5.ku-ds.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (soft)" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ + -cert data_files/server5.ku-ka.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + # Tests for extendedKeyUsage, part 1: server-side certificate/suite selection requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 @@ -6271,6 +6386,54 @@ run_test "extKeyUsage cli: codeSign -> fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli 1.3: serverAuth -> OK" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.eku-srv.crt" \ + "$P_CLI debug_level=1" \ + 0 \ + -C "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli 1.3: serverAuth,clientAuth -> OK" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.eku-srv_cli.crt" \ + "$P_CLI debug_level=1" \ + 0 \ + -C "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli 1.3: codeSign,anyEKU -> OK" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.eku-cs_any.crt" \ + "$P_CLI debug_level=1" \ + 0 \ + -C "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli 1.3: codeSign -> fail" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ + -cert data_files/server5.eku-cs.crt" \ + "$P_CLI debug_level=1" \ + 1 \ + -c "bad certificate (usage extensions)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is" + # Tests for extendedKeyUsage, part 3: server-side checking of client cert requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 @@ -6318,6 +6481,50 @@ run_test "extKeyUsage cli-auth: codeSign -> fail (hard)" \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli-auth 1.3: clientAuth -> OK" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ + -cert data_files/server5.eku-cli.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli-auth 1.3: serverAuth,clientAuth -> OK" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ + -cert data_files/server5.eku-srv_cli.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli-auth 1.3: codeSign,anyEKU -> OK" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ + -cert data_files/server5.eku-cs_any.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_openssl_tls1_3 +requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli-auth 1.3: codeSign -> fail (soft)" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ + -cert data_files/server5.eku-cs.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + # Tests for DHM parameters loading requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 From 30c5a2520eb558e8b40e745ae98c4a5aa91087cb Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 16 Jun 2022 19:31:06 +0200 Subject: [PATCH 15/43] tls13: Fix certificate key usage checks Signed-off-by: Ronald Cron --- library/ssl_tls13_generic.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 3b49ec5f7..7a7713ae4 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -542,6 +542,8 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) int authmode = MBEDTLS_SSL_VERIFY_REQUIRED; mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; + const char *ext_oid; + size_t ext_len; uint32_t verify_result = 0; /* If SNI was used, overwrite authentication mode @@ -623,12 +625,25 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) /* * Secondary checks: always done, but change 'ret' only if it was 0 */ - if( mbedtls_ssl_check_cert_usage( ssl->session_negotiate->peer_cert, - ssl->handshake->ciphersuite_info, - !ssl->conf->endpoint, - &verify_result ) != 0 ) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate ( usage extensions )" ) ); + ext_oid = MBEDTLS_OID_SERVER_AUTH; + ext_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_SERVER_AUTH ); + } + else + { + ext_oid = MBEDTLS_OID_CLIENT_AUTH; + ext_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_CLIENT_AUTH ); + } + + if( ( mbedtls_x509_crt_check_key_usage( + ssl->session_negotiate->peer_cert, + MBEDTLS_X509_KU_DIGITAL_SIGNATURE ) != 0 ) || + ( mbedtls_x509_crt_check_extended_key_usage( + ssl->session_negotiate->peer_cert, + ext_oid, ext_len ) != 0 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (usage extensions)" ) ); if( ret == 0 ) ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; } From c3e9abedfffc954b9dd68bece97158dd5bfaddda Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Jun 2022 17:23:06 +0200 Subject: [PATCH 16/43] Add change log Signed-off-by: Ronald Cron --- ChangeLog.d/tls13-fix-key-usage-checks.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 ChangeLog.d/tls13-fix-key-usage-checks.txt diff --git a/ChangeLog.d/tls13-fix-key-usage-checks.txt b/ChangeLog.d/tls13-fix-key-usage-checks.txt new file mode 100644 index 000000000..f19bf523e --- /dev/null +++ b/ChangeLog.d/tls13-fix-key-usage-checks.txt @@ -0,0 +1,7 @@ +Security + * Fix check of certificate key usage in TLS 1.3. The usage of the public key + provided by a client or server certificate for authentication was not + checked properly when validating the certificate. This could cause a + client or server to be able to authenticate itself through a certificate + to an Mbed TLS TLS 1.3 server or client while it does not own a proper + certificate to do so. From 32a38dfec586f5b721592b46d4f4a21da99c082e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 15 Jun 2022 10:50:18 +0200 Subject: [PATCH 17/43] Add ChangeLog for potential overread with USE_PSA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The issue was fixed while adding support for static ECDH with Opaque keys: https://github.com/Mbed-TLS/mbedtls/pull/5624 This is just adding the ChangeLog entry for that fix. Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/buf-overread-use-psa-static-ecdh.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 ChangeLog.d/buf-overread-use-psa-static-ecdh.txt diff --git a/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt new file mode 100644 index 000000000..7eff12a30 --- /dev/null +++ b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt @@ -0,0 +1,7 @@ +Security + * Fix a potential heap buffer overread in TLS 1.2 server-side when + MBEDTLS_USE_PSA_CRYPTO is enabled, an opaque key (created with + mbedtls_pk_setup_opaque()) is provisioned, and a static ECDH ciphersuite + is selected. This may result in an application crash. No path to + information leak has been identified. + From b64fb62eadfee68fb5ff6462645eae51fa01d506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:34:20 +0200 Subject: [PATCH 18/43] Fix unchecked return value from internal function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls12_server.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 04121c1bd..955aac523 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -3350,7 +3350,12 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDH_ENABLED) if( mbedtls_ssl_ciphersuite_uses_ecdh( ciphersuite_info ) ) { - ssl_get_ecdh_params_from_cert( ssl ); + ret = ssl_get_ecdh_params_from_cert( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); + return( ret ); + } } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDH_ENABLED */ From 1c91b0c43463c27c4930618ffc3700218be12687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:40:58 +0200 Subject: [PATCH 19/43] Clarify warning about mbedtls_pk_ec/rsa() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous wording "ensure it holds an XXX" context did not mean anything without looking at the source. Looking at the source, the criterion is: - for mbedtls_pk_rsa(), that the info structure uses rsa_alloc_wrap; - for mbedtls_pk_ec(), that it uses eckey_alloc_wrap or ecdsa_alloc_wrap, since mbedtls_ecdsa_context is a typedef for mbedtls_ecp_keypair. (Note that our test code uses mbedtls_pk_ec() on contexts of type MBEDTLS_PK_ECDSA.) Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 207aed044..126a38967 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -220,8 +220,9 @@ typedef void mbedtls_pk_restart_ctx; /** * Quick access to an RSA context inside a PK context. * - * \warning You must make sure the PK context actually holds an RSA context - * before using this function! + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_RSA. + * Ensuring that is the caller's responsibility. */ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) { @@ -233,8 +234,10 @@ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) /** * Quick access to an EC context inside a PK context. * - * \warning You must make sure the PK context actually holds an EC context - * before using this function! + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_ECKEY, + * #MBEDTLS_PK_ECKEY_DH, or #MBEDTLS_PK_ECDSA. + * Ensuring that is the caller's responsibility. */ static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) { From 22e84de97145e22d2229e1cf9769e4dce1e172c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:48:38 +0200 Subject: [PATCH 20/43] Improve contract of mbedtls_pk_ec/rsa() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trusting the caller to perform the appropriate check is both risky, and a bit user-unfriendly. Returning NULL on error seems both safer (dereferencing a NULL pointer is more likely to result in a clean crash, while mis-casting a pointer might have deeper, less predictable consequences) and friendlier (the caller can just check the return value for NULL, which is a common idiom). Only add that as an additional way of using the function, for the sake of backwards compatibility. Calls where we know the type of the context for sure (for example because we just set it up) were legal and safe, so they should remain legal without checking the result for NULL, which would be redundant. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 70 ++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 126a38967..30f0492e9 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -216,35 +216,6 @@ typedef struct typedef void mbedtls_pk_restart_ctx; #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ -#if defined(MBEDTLS_RSA_C) -/** - * Quick access to an RSA context inside a PK context. - * - * \warning This function can only be used when the type of the context, as - * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_RSA. - * Ensuring that is the caller's responsibility. - */ -static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) -{ - return( (mbedtls_rsa_context *) (pk).MBEDTLS_PRIVATE(pk_ctx) ); -} -#endif /* MBEDTLS_RSA_C */ - -#if defined(MBEDTLS_ECP_C) -/** - * Quick access to an EC context inside a PK context. - * - * \warning This function can only be used when the type of the context, as - * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_ECKEY, - * #MBEDTLS_PK_ECKEY_DH, or #MBEDTLS_PK_ECDSA. - * Ensuring that is the caller's responsibility. - */ -static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) -{ - return( (mbedtls_ecp_keypair *) (pk).MBEDTLS_PRIVATE(pk_ctx) ); -} -#endif /* MBEDTLS_ECP_C */ - #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) /** * \brief Types for RSA-alt abstraction @@ -738,6 +709,47 @@ const char * mbedtls_pk_get_name( const mbedtls_pk_context *ctx ); */ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); +#if defined(MBEDTLS_RSA_C) +/** + * Quick access to an RSA context inside a PK context. + * + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_RSA. + * Ensuring that is the caller's responsibility. + * Alternatively, you can check whether this function returns NULL. + * + * \return The internal RSA context held by the PK context, or NULL. + */ +static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) +{ + return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_RSA ? + (mbedtls_rsa_context *) (pk).MBEDTLS_PRIVATE(pk_ctx) : + NULL ); +} +#endif /* MBEDTLS_RSA_C */ + +#if defined(MBEDTLS_ECP_C) +/** + * Quick access to an EC context inside a PK context. + * + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_ECKEY, + * #MBEDTLS_PK_ECKEY_DH, or #MBEDTLS_PK_ECDSA. + * Ensuring that is the caller's responsibility. + * Alternatively, you can check whether this function returns NULL. + * + * \return The internal EC context held by the PK context, or NULL. + */ +static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) +{ + return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY || + mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY_DH || + mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECDSA ? + (mbedtls_ecp_keypair *) (pk).MBEDTLS_PRIVATE(pk_ctx) : + NULL ); +} +#endif /* MBEDTLS_ECP_C */ + #if defined(MBEDTLS_PK_PARSE_C) /** \ingroup pk_module */ /** From 19a567ba431a9b4c39b091583d8f8e46f4d67328 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Jun 2022 09:50:04 +0200 Subject: [PATCH 21/43] Fix impact evaluation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/buf-overread-use-psa-static-ecdh.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt index 7eff12a30..84b9f790d 100644 --- a/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt +++ b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt @@ -2,6 +2,5 @@ Security * Fix a potential heap buffer overread in TLS 1.2 server-side when MBEDTLS_USE_PSA_CRYPTO is enabled, an opaque key (created with mbedtls_pk_setup_opaque()) is provisioned, and a static ECDH ciphersuite - is selected. This may result in an application crash. No path to - information leak has been identified. - + is selected. This may result in an application crash or potentially an + information leak. From 66b0d617184104c491525b36518b90c0f3c67bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Jun 2022 10:49:29 +0200 Subject: [PATCH 22/43] Add comments when can_do() is safe to use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 4 +++- library/ssl_tls12_client.c | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 53318650c..06c30f62c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6280,7 +6280,9 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, { const mbedtls_pk_context *pk = &chain->pk; - /* If certificate uses an EC key, make sure the curve is OK */ + /* If certificate uses an EC key, make sure the curve is OK. + * This is a public key, so it can't be opaque, so can_do() is a good + * enough check to ensure pk_ec() is safe to use here. */ if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) && mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 ) { diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index f516efab1..3d0e3a759 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -2089,6 +2089,8 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) peer_pk = &ssl->session_negotiate->peer_cert->pk; #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ + /* This is a public key, so it can't be opaque, so can_do() is a good + * enough check to ensure pk_ec() is safe to use below. */ if( ! mbedtls_pk_can_do( peer_pk, MBEDTLS_PK_ECKEY ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key not ECDH capable" ) ); From a3115dc0e6b3185812efbf73cc01e4da238257e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Jun 2022 10:52:54 +0200 Subject: [PATCH 23/43] Mark static int SSL functions CHECK_RETURN_CRITICAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_cache.c | 2 ++ library/ssl_ciphersuites.c | 1 + library/ssl_client.c | 7 +++++++ library/ssl_cookie.c | 1 + library/ssl_msg.c | 39 +++++++++++++++++++++++++++++++++++++ library/ssl_ticket.c | 2 ++ library/ssl_tls.c | 38 ++++++++++++++++++++++++++++++++++++ library/ssl_tls12_client.c | 37 +++++++++++++++++++++++++++++++++++ library/ssl_tls12_server.c | 36 ++++++++++++++++++++++++++++++++++ library/ssl_tls13_client.c | 36 ++++++++++++++++++++++++++++++++++ library/ssl_tls13_generic.c | 13 +++++++++++++ library/ssl_tls13_keys.c | 2 ++ library/ssl_tls13_server.c | 29 +++++++++++++++++++++++++++ 13 files changed, 243 insertions(+) diff --git a/library/ssl_cache.c b/library/ssl_cache.c index fe4f30cf8..0288479ea 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -50,6 +50,7 @@ void mbedtls_ssl_cache_init( mbedtls_ssl_cache_context *cache ) #endif } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cache_find_entry( mbedtls_ssl_cache_context *cache, unsigned char const *session_id, size_t session_id_len, @@ -124,6 +125,7 @@ exit: return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache, unsigned char const *session_id, size_t session_id_len, diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 7deb57a1d..ec2edfa7d 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -1778,6 +1778,7 @@ const int *mbedtls_ssl_list_ciphersuites( void ) static int supported_ciphersuites[MAX_CIPHERSUITES]; static int supported_init = 0; +MBEDTLS_CHECK_RETURN_CRITICAL static int ciphersuite_is_removed( const mbedtls_ssl_ciphersuite_t *cs_info ) { (void)cs_info; diff --git a/library/ssl_client.c b/library/ssl_client.c index 22ca57cab..20f1aff4b 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -46,6 +46,7 @@ #include "ssl_debug_helpers.h" #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hostname_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -129,6 +130,7 @@ static int ssl_write_hostname_ext( mbedtls_ssl_context *ssl, * } ProtocolNameList; * */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -226,6 +228,7 @@ static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, * * DHE groups are not supported yet. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -308,6 +311,7 @@ static int ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C || MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_client_hello_cipher_suites( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -424,6 +428,7 @@ static int ssl_write_client_hello_cipher_suites( * }; * } ClientHello; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_client_hello_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -654,6 +659,7 @@ static int ssl_write_client_hello_body( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_generate_random( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -691,6 +697,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_client_hello( mbedtls_ssl_context *ssl ) { int ret; diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 0f2bc60a3..4b2d2d2bf 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -160,6 +160,7 @@ int mbedtls_ssl_cookie_setup( mbedtls_ssl_cookie_ctx *ctx, /* * Generate the HMAC part of a cookie */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cookie_hmac( mbedtls_md_context_t *hmac_ctx, const unsigned char time[4], unsigned char **p, unsigned char *end, diff --git a/library/ssl_msg.c b/library/ssl_msg.c index ac79b2f03..580a1fbc9 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -86,6 +86,7 @@ int mbedtls_ssl_check_timer( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, unsigned char *buf, size_t len, @@ -157,11 +158,16 @@ exit: static void ssl_buffering_free_slot( mbedtls_ssl_context *ssl, uint8_t slot ); static void ssl_free_buffered_record( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_future_record( mbedtls_ssl_context *ssl, mbedtls_record const *rec ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ); static size_t ssl_get_maximum_datagram_size( mbedtls_ssl_context const *ssl ) @@ -179,6 +185,7 @@ static size_t ssl_get_maximum_datagram_size( mbedtls_ssl_context const *ssl ) return( out_buf_len ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_remaining_space_in_datagram( mbedtls_ssl_context const *ssl ) { size_t const bytes_written = ssl->out_left; @@ -195,6 +202,7 @@ static int ssl_get_remaining_space_in_datagram( mbedtls_ssl_context const *ssl ) return( (int) ( mtu - bytes_written ) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_remaining_payload_in_datagram( mbedtls_ssl_context const *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -246,6 +254,7 @@ static int ssl_get_remaining_payload_in_datagram( mbedtls_ssl_context const *ssl * Double the retransmit timeout value, within the allowed range, * returning -1 if the maximum value has already been reached. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_double_retransmit_timeout( mbedtls_ssl_context *ssl ) { uint32_t new_timeout; @@ -329,6 +338,7 @@ static size_t ssl_compute_padding_length( size_t len, * - A negative error code if `max_len` didn't offer enough space * for the expansion. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_build_inner_plaintext( unsigned char *content, size_t *content_size, size_t remaining, @@ -356,6 +366,7 @@ static int ssl_build_inner_plaintext( unsigned char *content, /* This function parses a (D)TLSInnerPlaintext structure. * See ssl_build_inner_plaintext() for details. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_inner_plaintext( unsigned char const *content, size_t *content_size, uint8_t *rec_type ) @@ -469,6 +480,7 @@ static void ssl_extract_add_data_from_record( unsigned char* add_data, #if defined(MBEDTLS_GCM_C) || \ defined(MBEDTLS_CCM_C) || \ defined(MBEDTLS_CHACHAPOLY_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_transform_aead_dynamic_iv_is_explicit( mbedtls_ssl_transform const *transform ) { @@ -2066,6 +2078,7 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) /* * Append current handshake message to current outgoing flight */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_flight_append( mbedtls_ssl_context *ssl ) { mbedtls_ssl_flight_item *msg; @@ -2132,6 +2145,7 @@ void mbedtls_ssl_flight_free( mbedtls_ssl_flight_item *flight ) /* * Swap transform_out and out_ctr with the alternative ones */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_swap_epochs( mbedtls_ssl_context *ssl ) { mbedtls_ssl_transform *tmp_transform; @@ -2767,6 +2781,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, int force_flush ) #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_hs_is_proper_fragment( mbedtls_ssl_context *ssl ) { if( ssl->in_msglen < ssl->in_hslen || @@ -2792,6 +2807,7 @@ static uint32_t ssl_get_hs_frag_off( mbedtls_ssl_context const *ssl ) ssl->in_msg[8] ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_hs_header( mbedtls_ssl_context const *ssl ) { uint32_t msg_len, frag_off, frag_len; @@ -2858,6 +2874,7 @@ static void ssl_bitmask_set( unsigned char *mask, size_t offset, size_t len ) /* * Check that bitmask is full */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_bitmask_check( unsigned char *mask, size_t len ) { size_t i; @@ -3057,6 +3074,7 @@ static inline uint64_t ssl_load_six_bytes( unsigned char *buf ) ( (uint64_t) buf[5] ) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int mbedtls_ssl_dtls_record_replay_check( mbedtls_ssl_context *ssl, uint8_t *record_in_ctr ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3149,6 +3167,7 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ) * return MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED * - otherwise return a specific error code */ +MBEDTLS_CHECK_RETURN_CRITICAL MBEDTLS_STATIC_TESTABLE int mbedtls_ssl_check_dtls_clihlo_cookie( mbedtls_ssl_context *ssl, @@ -3309,6 +3328,7 @@ int mbedtls_ssl_check_dtls_clihlo_cookie( * includes the case of MBEDTLS_ERR_SSL_CLIENT_RECONNECT and of unexpected * errors, and is the right thing to do in both cases). */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3364,6 +3384,7 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE && MBEDTLS_SSL_SRV_C */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_record_type( uint8_t record_type ) { if( record_type != MBEDTLS_SSL_MSG_HANDSHAKE && @@ -3396,6 +3417,7 @@ static int ssl_check_record_type( uint8_t record_type ) * Point 2 is needed when the peer is resending, and we have already received * the first record from a datagram but are still waiting for the others. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, unsigned char *buf, size_t len, @@ -3622,6 +3644,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_client_reconnect( mbedtls_ssl_context *ssl ) { unsigned int rec_epoch = ( ssl->in_ctr[0] << 8 ) | ssl->in_ctr[1]; @@ -3651,6 +3674,7 @@ static int ssl_check_client_reconnect( mbedtls_ssl_context *ssl ) /* * If applicable, decrypt record content */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_record_content( mbedtls_ssl_context *ssl, mbedtls_record *rec ) { @@ -3803,8 +3827,11 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl, */ /* Helper functions for mbedtls_ssl_read_record(). */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_consume_current_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_next_record( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_record_is_in_progress( mbedtls_ssl_context *ssl ); int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, @@ -3892,6 +3919,7 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ) { if( ssl->in_left > ssl->next_record_offset ) @@ -3900,6 +3928,7 @@ static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_message( mbedtls_ssl_context *ssl ) { mbedtls_ssl_handshake_params * const hs = ssl->handshake; @@ -3997,6 +4026,7 @@ exit: return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_make_space( mbedtls_ssl_context *ssl, size_t desired ) { @@ -4039,6 +4069,7 @@ static int ssl_buffer_make_space( mbedtls_ssl_context *ssl, return( -1 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_message( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -4243,6 +4274,7 @@ exit: } #endif /* MBEDTLS_SSL_PROTO_DTLS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_consume_current_message( mbedtls_ssl_context *ssl ) { /* @@ -4330,6 +4362,7 @@ static int ssl_consume_current_message( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_record_is_in_progress( mbedtls_ssl_context *ssl ) { if( ssl->in_msglen > 0 ) @@ -4356,6 +4389,7 @@ static void ssl_free_buffered_record( mbedtls_ssl_context *ssl ) } } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ) { mbedtls_ssl_handshake_params * const hs = ssl->handshake; @@ -4413,6 +4447,7 @@ exit: return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_future_record( mbedtls_ssl_context *ssl, mbedtls_record const *rec ) { @@ -4471,6 +4506,7 @@ static int ssl_buffer_future_record( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_DTLS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_next_record( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -5221,6 +5257,7 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) /* * Check record counters and renegotiate if they're above the limit. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) { size_t ep_len = mbedtls_ssl_ep_len( ssl ); @@ -5260,6 +5297,7 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) * and having a helper function allows to distinguish between TLS <= 1.2 and * TLS 1.3 in the future without bloating the logic of mbedtls_ssl_read(). */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handle_hs_message_post_handshake( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -5577,6 +5615,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) * Therefore, it is possible that the input message length is 0 and the * corresponding return code is 0 on success. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_real( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index a924a2a14..28c4d3e55 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -66,6 +66,7 @@ void mbedtls_ssl_ticket_init( mbedtls_ssl_ticket_context *ctx ) /* * Generate/update a key */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_ticket_gen_key( mbedtls_ssl_ticket_context *ctx, unsigned char index ) { @@ -113,6 +114,7 @@ static int ssl_ticket_gen_key( mbedtls_ssl_ticket_context *ctx, /* * Rotate/generate keys if necessary */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_ticket_update_keys( mbedtls_ssl_ticket_context *ctx ) { #if !defined(MBEDTLS_HAVE_TIME) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 06c30f62c..668b5ecae 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -275,6 +275,7 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, } #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int resize_buffer( unsigned char **buffer, size_t len_new, size_t *len_old ) { unsigned char* resized_buffer = mbedtls_calloc( 1, len_new ); @@ -380,6 +381,7 @@ typedef int ssl_tls_prf_t(const unsigned char *, size_t, const char *, const unsigned char *, size_t, unsigned char *, size_t); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, int ciphersuite, const unsigned char master[48], @@ -393,6 +395,7 @@ static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, const mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SHA256_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_sha256( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -403,6 +406,7 @@ static void ssl_calc_finished_tls_sha256( mbedtls_ssl_context *,unsigned char *, #endif /* MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA384_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_sha384( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -415,6 +419,7 @@ static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char * static size_t ssl_session_save_tls12( const mbedtls_ssl_session *session, unsigned char *buf, size_t buf_len ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_session_load_tls12( mbedtls_ssl_session *session, const unsigned char *buf, size_t len ); @@ -672,6 +677,7 @@ void mbedtls_ssl_session_init( mbedtls_ssl_session *session ) memset( session, 0, sizeof(mbedtls_ssl_session) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handshake_init( mbedtls_ssl_context *ssl ) { /* Clear old handshake information if present */ @@ -863,6 +869,7 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) /* Dummy cookie callbacks for defaults */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cookie_write_dummy( void *ctx, unsigned char **p, unsigned char *end, const unsigned char *cli_id, size_t cli_id_len ) @@ -876,6 +883,7 @@ static int ssl_cookie_write_dummy( void *ctx, return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cookie_check_dummy( void *ctx, const unsigned char *cookie, size_t cookie_len, const unsigned char *cli_id, size_t cli_id_len ) @@ -898,6 +906,7 @@ void mbedtls_ssl_init( mbedtls_ssl_context *ssl ) memset( ssl, 0, sizeof( mbedtls_ssl_context ) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) { const mbedtls_ssl_config *conf = ssl->conf; @@ -949,6 +958,7 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_CONFIG ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_check(const mbedtls_ssl_context *ssl) { int ret; @@ -1372,6 +1382,7 @@ static void ssl_key_cert_free( mbedtls_ssl_key_cert *key_cert ) } /* Append a new keycert entry to a (possibly empty) list */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_append_key_cert( mbedtls_ssl_key_cert **head, mbedtls_x509_crt *cert, mbedtls_pk_context *key ) @@ -1517,6 +1528,7 @@ int mbedtls_ssl_set_hs_ecjpake_password( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_psk_is_configured( mbedtls_ssl_config const *conf ) { #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -1562,6 +1574,7 @@ static void ssl_conf_remove_psk( mbedtls_ssl_config *conf ) * It checks that the provided identity is well-formed and attempts * to make a copy of it in the SSL config. * On failure, the PSK identity in the config remains unset. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_set_psk_identity( mbedtls_ssl_config *conf, unsigned char const *psk_identity, size_t psk_identity_len ) @@ -2756,6 +2769,7 @@ static unsigned char ssl_serialized_session_header[] = { * */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_session_save( const mbedtls_ssl_session *session, unsigned char omit_header, unsigned char *buf, @@ -2830,6 +2844,7 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, * This internal version is wrapped by a public function that cleans up in * case of error, and has an extra option omit_header. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_session_load( mbedtls_ssl_session *session, unsigned char omit_header, const unsigned char *buf, @@ -2896,6 +2911,7 @@ int mbedtls_ssl_session_load( mbedtls_ssl_session *session, /* * Perform a single step of the SSL handshake */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_handshake_step( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3054,6 +3070,7 @@ int mbedtls_ssl_handshake( mbedtls_ssl_context *ssl ) /* * Write HelloRequest to request renegotiation on server */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hello_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3652,6 +3669,7 @@ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl, * This internal version is wrapped by a public function that cleans up in * case of error. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_context_load( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -4235,6 +4253,7 @@ static uint16_t ssl_preset_suiteb_groups[] = { #if defined(MBEDTLS_DEBUG_C) && defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* Function for checking `ssl_preset_*_sig_algs` and `ssl_tls12_preset_*_sig_algs` * to make sure there are no duplicated signature algorithm entries. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_no_sig_alg_duplication( uint16_t * sig_algs ) { size_t i, j; @@ -4786,6 +4805,7 @@ exit: #else /* MBEDTLS_USE_PSA_CRYPTO */ #if defined(MBEDTLS_SHA384_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_handshake_transcript_sha384( mbedtls_ssl_context *ssl, unsigned char *dst, size_t dst_len, @@ -4816,6 +4836,7 @@ exit: #endif /* MBEDTLS_SHA384_C */ #if defined(MBEDTLS_SHA256_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_handshake_transcript_sha256( mbedtls_ssl_context *ssl, unsigned char *dst, size_t dst_len, @@ -5044,6 +5065,7 @@ static psa_status_t setup_psa_key_derivation( psa_key_derivation_operation_t* de return( PSA_SUCCESS ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_generic( mbedtls_md_type_t md_type, const unsigned char *secret, size_t slen, const char *label, @@ -5120,6 +5142,7 @@ static int tls_prf_generic( mbedtls_md_type_t md_type, #else /* MBEDTLS_USE_PSA_CRYPTO */ +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_generic( mbedtls_md_type_t md_type, const unsigned char *secret, size_t slen, const char *label, @@ -5212,6 +5235,7 @@ exit: #endif /* MBEDTLS_USE_PSA_CRYPTO */ #if defined(MBEDTLS_SHA256_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_sha256( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -5223,6 +5247,7 @@ static int tls_prf_sha256( const unsigned char *secret, size_t slen, #endif /* MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA384_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_sha384( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -5242,6 +5267,7 @@ static int tls_prf_sha384( const unsigned char *secret, size_t slen, * Outputs: * - the tls_prf, calc_verify and calc_finished members of handshake structure */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_set_handshake_prfs( mbedtls_ssl_handshake_params *handshake, mbedtls_md_type_t hash ) { @@ -5286,6 +5312,7 @@ static int ssl_set_handshake_prfs( mbedtls_ssl_handshake_params *handshake, * EMS: passed to calc_verify (debug + session_negotiate) * PSA-PSA: conf */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, unsigned char *master, const mbedtls_ssl_context *ssl ) @@ -5754,6 +5781,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch #endif /* !MBEDTLS_USE_PSA_CRYPTO && MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_RENEGOTIATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hello_request( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SSL_PROTO_DTLS) @@ -5925,6 +5953,7 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_RENEGOTIATION) && defined(MBEDTLS_SSL_CLI_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, unsigned char *crt_buf, size_t crt_buf_len ) @@ -5940,6 +5969,7 @@ static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, return( memcmp( peer_crt->raw.p, crt_buf, peer_crt->raw.len ) ); } #else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, unsigned char *crt_buf, size_t crt_buf_len ) @@ -5974,6 +6004,7 @@ static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, * Once the certificate message is read, parse it into a cert chain and * perform basic checks, but leave actual verification to the caller */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_chain( mbedtls_ssl_context *ssl, mbedtls_x509_crt *chain ) { @@ -6129,6 +6160,7 @@ static int ssl_parse_certificate_chain( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_SRV_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_srv_check_client_no_crt_notification( mbedtls_ssl_context *ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) @@ -6154,6 +6186,7 @@ static int ssl_srv_check_client_no_crt_notification( mbedtls_ssl_context *ssl ) */ #define SSL_CERTIFICATE_EXPECTED 0 #define SSL_CERTIFICATE_SKIP 1 +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_coordinate( mbedtls_ssl_context *ssl, int authmode ) { @@ -6183,6 +6216,7 @@ static int ssl_parse_certificate_coordinate( mbedtls_ssl_context *ssl, return( SSL_CERTIFICATE_EXPECTED ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, int authmode, mbedtls_x509_crt *chain, @@ -6373,6 +6407,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, } #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_remember_peer_crt_digest( mbedtls_ssl_context *ssl, unsigned char *start, size_t len ) { @@ -6404,6 +6439,7 @@ static int ssl_remember_peer_crt_digest( mbedtls_ssl_context *ssl, return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_remember_peer_pubkey( mbedtls_ssl_context *ssl, unsigned char *start, size_t len ) { @@ -7072,6 +7108,7 @@ static mbedtls_tls_prf_types tls_prf_get_type( mbedtls_ssl_tls_prf_cb *tls_prf ) * [in] optionally used for: * - MBEDTLS_DEBUG_C: ssl->conf->{f,p}_dbg */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls12_populate_transform( mbedtls_ssl_transform *transform, int ciphersuite, const unsigned char master[48], @@ -7873,6 +7910,7 @@ static size_t ssl_session_save_tls12( const mbedtls_ssl_session *session, return( used ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_session_load_tls12( mbedtls_ssl_session *session, const unsigned char *buf, size_t len ) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 3d0e3a759..a0b0bfc9d 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -75,6 +75,7 @@ int mbedtls_ssl_conf_has_static_psk( mbedtls_ssl_config const *conf ) #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #if defined(MBEDTLS_SSL_RENEGOTIATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -116,6 +117,7 @@ static int ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -147,6 +149,7 @@ static int ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -220,6 +223,7 @@ static int ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_cid_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -266,6 +270,7 @@ static int ssl_write_cid_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -298,6 +303,7 @@ static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -328,6 +334,7 @@ static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -358,6 +365,7 @@ static int ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -400,6 +408,7 @@ static int ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_DTLS_SRTP) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_use_srtp_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -629,6 +638,7 @@ int mbedtls_ssl_tls12_write_client_hello_exts( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -673,6 +683,7 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -699,6 +710,7 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -755,6 +767,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -780,6 +793,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -805,6 +819,7 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -831,6 +846,7 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_supported_point_formats_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -879,6 +895,7 @@ static int ssl_parse_supported_point_formats_ext( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -913,6 +930,7 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -983,6 +1001,7 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ALPN */ #if defined(MBEDTLS_SSL_DTLS_SRTP) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1103,6 +1122,7 @@ static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, * Parse HelloVerifyRequest. Only called after verifying the HS type. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) { const unsigned char *p = ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ); @@ -1184,6 +1204,7 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_PROTO_DTLS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) { int ret, i; @@ -1708,6 +1729,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -1753,6 +1775,7 @@ static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) #if defined(MBEDTLS_USE_PSA_CRYPTO) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -1817,6 +1840,7 @@ static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, return( 0 ); } #else +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_server_ecdh_params( const mbedtls_ssl_context *ssl ) { const mbedtls_ecp_curve_info *curve_info; @@ -1845,6 +1869,7 @@ static int ssl_check_server_ecdh_params( const mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -1885,6 +1910,7 @@ static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -1931,6 +1957,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, /* * Generate a pre-master secret and encrypt it with the server's RSA key */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, size_t offset, size_t *olen, size_t pms_offset ) @@ -2013,6 +2040,7 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end, @@ -2071,6 +2099,7 @@ static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2160,6 +2189,7 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2507,6 +2537,7 @@ exit: } #if ! defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -2525,6 +2556,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2690,6 +2722,7 @@ exit: } #endif /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2729,6 +2762,7 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3251,6 +3285,7 @@ ecdh_calc_secret: } #if !defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -3276,6 +3311,7 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* !MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -3415,6 +3451,7 @@ sign: #endif /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_new_session_ticket( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 955aac523..7e1e77229 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -78,6 +78,7 @@ void mbedtls_ssl_conf_dtls_cookies( mbedtls_ssl_config *conf, #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -152,6 +153,7 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_has_psk_or_cb( mbedtls_ssl_config const *conf ) { if( conf->f_psk != NULL ) @@ -173,6 +175,7 @@ static int ssl_conf_has_psk_or_cb( mbedtls_ssl_config const *conf ) } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -250,6 +253,7 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, * * DHE groups are not supported yet. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -316,6 +320,7 @@ static int ssl_parse_supported_groups_ext( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_supported_point_formats( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -361,6 +366,7 @@ static int ssl_parse_supported_point_formats( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -390,6 +396,7 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -409,6 +416,7 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -481,6 +489,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -505,6 +514,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -529,6 +539,7 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) @@ -603,6 +614,7 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -691,6 +703,7 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ALPN */ #if defined(MBEDTLS_SSL_DTLS_SRTP) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -819,6 +832,7 @@ static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, * Return 0 if the given key uses one of the acceptable curves, -1 otherwise */ #if defined(MBEDTLS_ECDSA_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_key_curve( mbedtls_pk_context *pk, const mbedtls_ecp_curve_info **curves ) { @@ -840,6 +854,7 @@ static int ssl_check_key_curve( mbedtls_pk_context *pk, * Try picking a certificate for this ciphersuite, * return 0 on success and -1 on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_pick_cert( mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t * ciphersuite_info ) { @@ -924,6 +939,7 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, * Check if a given ciphersuite is suitable for use with our config/keys/etc * Sets ciphersuite_info only if the suite matches. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, const mbedtls_ssl_ciphersuite_t **ciphersuite_info ) { @@ -1021,6 +1037,7 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, /* This function doesn't alert on errors that happen early during ClientHello parsing because they might indicate that the client is not talking SSL/TLS at all and would not understand our alert. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) { int ret, got_common_suite; @@ -2237,6 +2254,7 @@ static void ssl_write_use_srtp_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_SRTP */ #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2355,6 +2373,7 @@ exit: mbedtls_ssl_session_free( &session_tmp ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) { #if defined(MBEDTLS_HAVE_TIME) @@ -2570,6 +2589,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) } #if !defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -2588,6 +2608,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* !MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -2749,6 +2770,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_USE_PSA_CRYPTO) && \ ( defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) ) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2843,6 +2865,7 @@ cleanup: } #elif defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2875,6 +2898,7 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) && \ defined(MBEDTLS_SSL_ASYNC_PRIVATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_resume_server_key_exchange( mbedtls_ssl_context *ssl, size_t *signature_len ) { @@ -2902,6 +2926,7 @@ static int ssl_resume_server_key_exchange( mbedtls_ssl_context *ssl, /* Prepare the ServerKeyExchange message, up to and including * calculating the signature if any, but excluding formatting the * signature and sending the message. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, size_t *signature_len ) { @@ -3329,6 +3354,7 @@ curve_matching_done: * that do not include a ServerKeyExchange message, do nothing. Either * way, if successful, move on to the next step in the SSL state * machine. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3431,6 +3457,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3470,6 +3497,7 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_dh_public( mbedtls_ssl_context *ssl, unsigned char **p, const unsigned char *end ) { @@ -3513,6 +3541,7 @@ static int ssl_parse_client_dh_public( mbedtls_ssl_context *ssl, unsigned char * defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_resume_decrypt_pms( mbedtls_ssl_context *ssl, unsigned char *peer_pms, size_t *peer_pmslen, @@ -3530,6 +3559,7 @@ static int ssl_resume_decrypt_pms( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_decrypt_encrypted_pms( mbedtls_ssl_context *ssl, const unsigned char *p, const unsigned char *end, @@ -3615,6 +3645,7 @@ static int ssl_decrypt_encrypted_pms( mbedtls_ssl_context *ssl, return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, const unsigned char *p, const unsigned char *end, @@ -3703,6 +3734,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned char **p, const unsigned char *end ) { @@ -3763,6 +3795,7 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -4200,6 +4233,7 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) } #if !defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -4218,6 +4252,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* !MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -4381,6 +4416,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_new_session_ticket( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index e9250fcd3..b29818426 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -42,6 +42,7 @@ * ProtocolVersion versions<2..254>; * } SupportedVersions; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -91,6 +92,7 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -120,6 +122,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_alpn_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -166,6 +169,7 @@ static int ssl_tls13_parse_alpn_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_ALPN */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_reset_key_share( mbedtls_ssl_context *ssl ) { uint16_t group_id = ssl->handshake->offered_group_id; @@ -204,6 +208,7 @@ static int ssl_tls13_reset_key_share( mbedtls_ssl_context *ssl ) /* * Functions for writing key_share extension. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_get_default_group_id( mbedtls_ssl_context *ssl, uint16_t *group_id ) { @@ -253,6 +258,7 @@ static int ssl_tls13_get_default_group_id( mbedtls_ssl_context *ssl, * KeyShareEntry client_shares<0..2^16-1>; * } KeyShareClientHello; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -366,6 +372,7 @@ cleanup: * NamedGroup selected_group; * } KeyShareHelloRetryRequest; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_hrr_key_share_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -440,6 +447,7 @@ static int ssl_tls13_parse_hrr_key_share_ext( mbedtls_ssl_context *ssl, * opaque key_exchange<1..2^16-1>; * } KeyShareEntry; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -514,6 +522,7 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, * a "cookie" extension in the new ClientHello. Clients MUST NOT use * cookies in their initial ClientHello in subsequent connections. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -547,6 +556,7 @@ static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_cookie_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -640,6 +650,7 @@ int mbedtls_ssl_tls13_write_client_hello_exts( mbedtls_ssl_context *ssl, * \return 1 if the ServerHello contains a supported_versions extension * \return A negative value if an error occurred while parsing the ServerHello. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_is_supported_versions_ext_present( mbedtls_ssl_context *ssl, const unsigned char *buf, @@ -714,6 +725,7 @@ static int ssl_tls13_is_supported_versions_ext_present( * the server is TLS 1.3 capable but negotiating TLS 1.2 or below. * - 0 otherwise */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_is_downgrade_negotiation( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -746,6 +758,7 @@ static int ssl_tls13_is_downgrade_negotiation( mbedtls_ssl_context *ssl, */ #define SSL_SERVER_HELLO_COORDINATE_HELLO 0 #define SSL_SERVER_HELLO_COORDINATE_HRR 1 +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -785,6 +798,7 @@ static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, * - SSL_SERVER_HELLO_COORDINATE_TLS1_2 */ #define SSL_SERVER_HELLO_COORDINATE_TLS1_2 2 +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, unsigned char **buf, size_t *buf_len ) @@ -878,6 +892,7 @@ cleanup: return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ssl, const unsigned char **buf, const unsigned char *end ) @@ -925,6 +940,7 @@ static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ss * Extension extensions<6..2^16-1>; * } ServerHello; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end, @@ -1183,6 +1199,7 @@ cleanup: return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_server_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1263,6 +1280,7 @@ cleanup: return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_hrr( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1297,6 +1315,7 @@ static int ssl_tls13_postprocess_hrr( mbedtls_ssl_context *ssl ) * Wait and parse ServerHello handshake message. * Handler for MBEDTLS_SSL_SERVER_HELLO */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_server_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1359,16 +1378,20 @@ cleanup: */ /* Main entry point; orchestrates the other functions */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_encrypted_extensions( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl ); /* * Handler for MBEDTLS_SSL_ENCRYPTED_EXTENSIONS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_encrypted_extensions( mbedtls_ssl_context *ssl ) { int ret; @@ -1402,6 +1425,7 @@ cleanup: * Extension extensions<0..2^16-1>; * } EncryptedExtensions; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -1483,6 +1507,7 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl ) { #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) @@ -1512,6 +1537,7 @@ static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl * - SSL_CERTIFICATE_REQUEST_SKIP * indicating if a Certificate Request is expected or not. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1546,6 +1572,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) * Extension extensions<2..2^16-1>; * } CertificateRequest; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -1662,6 +1689,7 @@ decode_error: /* * Handler for MBEDTLS_SSL_CERTIFICATE_REQUEST */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1711,6 +1739,7 @@ cleanup: /* * Handler for MBEDTLS_SSL_SERVER_CERTIFICATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_server_certificate( mbedtls_ssl_context *ssl ) { int ret; @@ -1726,6 +1755,7 @@ static int ssl_tls13_process_server_certificate( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CERTIFICATE_VERIFY */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl ) { int ret; @@ -1742,6 +1772,7 @@ static int ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_SERVER_FINISHED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_server_finished( mbedtls_ssl_context *ssl ) { int ret; @@ -1773,6 +1804,7 @@ static int ssl_tls13_process_server_finished( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) { int non_empty_certificate_msg = 0; @@ -1812,6 +1844,7 @@ static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_client_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = mbedtls_ssl_tls13_write_certificate_verify( ssl ); @@ -1826,6 +1859,7 @@ static int ssl_tls13_write_client_certificate_verify( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CLIENT_FINISHED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { int ret; @@ -1849,6 +1883,7 @@ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_FLUSH_BUFFERS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_flush_buffers( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "handshake: done" ) ); @@ -1859,6 +1894,7 @@ static int ssl_tls13_flush_buffers( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_HANDSHAKE_WRAPUP */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) { diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 3b49ec5f7..c48922fbf 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -156,6 +156,7 @@ static void ssl_tls13_create_verify_structure( const unsigned char *transcript_h *verify_buffer_len = idx; } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_certificate_verify( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end, @@ -389,6 +390,7 @@ cleanup: */ /* Parse certificate chain send by the server. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -521,6 +523,7 @@ exit: return( ret ); } #else +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -536,6 +539,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) /* Validate certificate chain sent by the server. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -693,6 +697,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) return( ret ); } #else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) { ((void) ssl); @@ -754,6 +759,7 @@ cleanup: * CertificateEntry certificate_list<0..2^24-1>; * } Certificate; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_certificate_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -847,6 +853,7 @@ cleanup: /* * STATE HANDLING: Output Certificate Verify */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_get_sig_alg_from_pk( mbedtls_ssl_context *ssl, mbedtls_pk_context *own_key, uint16_t *algorithm ) @@ -964,6 +971,7 @@ static int ssl_tls13_get_sig_alg_from_pk( mbedtls_ssl_context *ssl, return( -1 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_certificate_verify_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1109,6 +1117,7 @@ cleanup: * Implementation */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_preprocess_finished_message( mbedtls_ssl_context *ssl ) { int ret; @@ -1128,6 +1137,7 @@ static int ssl_tls13_preprocess_finished_message( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -1206,6 +1216,7 @@ cleanup: * Implement */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ) { int ret; @@ -1226,6 +1237,7 @@ static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1305,6 +1317,7 @@ void mbedtls_ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) * */ #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_change_cipher_spec_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 91cc4d9fc..8303cda63 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -761,6 +761,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_calc_finished_core( psa_algorithm_t hash_alg, unsigned char const *base_key, unsigned char const *transcript, @@ -1188,6 +1189,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_early( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int mbedtls_ssl_tls13_get_cipher_key_info( const mbedtls_ssl_ciphersuite_t *ciphersuite_info, size_t *key_len, size_t *iv_len ) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 65023075c..4ac822f25 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -50,6 +50,7 @@ * ProtocolVersion versions<2..254>; * } SupportedVersions; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -106,6 +107,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, * NamedGroup named_group_list<2..2^16-1>; * } NamedGroupList; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -168,6 +170,7 @@ static int ssl_tls13_parse_supported_groups_ext( mbedtls_ssl_context *ssl, * be needed. * - A negative value for fatal errors. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -306,6 +309,7 @@ static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_DEBUG_C */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, int exts_mask ) { @@ -313,6 +317,7 @@ static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, return( masked == exts_mask ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_client_hello_has_exts_for_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) { @@ -322,6 +327,7 @@ static int ssl_tls13_client_hello_has_exts_for_ephemeral_key_exchange( MBEDTLS_SSL_EXT_SIG_ALG ) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) { if( !mbedtls_ssl_conf_tls13_ephemeral_enabled( ssl ) ) @@ -379,6 +385,7 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) #define SSL_CLIENT_HELLO_OK 0 #define SSL_CLIENT_HELLO_HRR_REQUIRED 1 +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -704,6 +711,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* Update the handshake state machine */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -724,6 +732,7 @@ static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) * Main entry point from the state machine; orchestrates the otherfunctions. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) { @@ -762,6 +771,7 @@ cleanup: /* * Handler for MBEDTLS_SSL_SERVER_HELLO */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -797,6 +807,7 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) * ProtocolVersion selected_version; * } SupportedVersions; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_server_hello_supported_versions_ext( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -834,6 +845,7 @@ static int ssl_tls13_write_server_hello_supported_versions_ext( /* Generate and export a single key share. For hybrid KEMs, this can * be called multiple times with the different components of the hybrid. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_generate_and_write_key_share( mbedtls_ssl_context *ssl, uint16_t named_group, unsigned char *buf, @@ -887,6 +899,7 @@ static int ssl_tls13_generate_and_write_key_share( mbedtls_ssl_context *ssl, * KeyShareEntry server_share; * } KeyShareServerHello; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -930,6 +943,7 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1005,6 +1019,7 @@ static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, * Extension extensions<6..2^16-1>; * } ServerHello; */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1131,6 +1146,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_finalize_write_server_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1151,6 +1167,7 @@ static int ssl_tls13_finalize_write_server_hello( mbedtls_ssl_context *ssl ) return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1189,6 +1206,7 @@ cleanup: /* * Handler for MBEDTLS_SSL_HELLO_RETRY_REQUEST */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_hello_retry_request_coordinate( mbedtls_ssl_context *ssl ) { @@ -1216,6 +1234,7 @@ static int ssl_tls13_write_hello_retry_request_coordinate( return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1260,6 +1279,7 @@ cleanup: * } EncryptedExtensions; * */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_encrypted_extensions_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1287,6 +1307,7 @@ static int ssl_tls13_write_encrypted_extensions_body( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_encrypted_extensions( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1333,6 +1354,7 @@ cleanup: * indicating if the writing of the CertificateRequest * should be skipped or not. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int authmode; @@ -1354,6 +1376,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) * } CertificateRequest; * */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -1400,6 +1423,7 @@ static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1447,6 +1471,7 @@ cleanup: /* * Handler for MBEDTLS_SSL_SERVER_CERTIFICATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_server_certificate( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1468,6 +1493,7 @@ static int ssl_tls13_write_server_certificate( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CERTIFICATE_VERIFY */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = mbedtls_ssl_tls13_write_certificate_verify( ssl ); @@ -1481,6 +1507,7 @@ static int ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_SERVER_FINISHED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_write_server_finished( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1513,6 +1540,7 @@ static int ssl_tls13_write_server_finished( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CLIENT_FINISHED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_process_client_finished( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1541,6 +1569,7 @@ static int ssl_tls13_process_client_finished( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_HANDSHAKE_WRAPUP */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "handshake: done" ) ); From a82a8b9f4b886b355f0d90c9459010d175af471c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Jun 2022 10:53:58 +0200 Subject: [PATCH 24/43] Mark internal int SSL functions CHECK_RETURN_CRITICAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_client.h | 1 + library/ssl_misc.h | 56 ++++++++++++++++++++++++++++++++++++++++ library/ssl_tls13_keys.h | 19 ++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/library/ssl_client.h b/library/ssl_client.h index 8e0c21634..be4d0677d 100644 --- a/library/ssl_client.h +++ b/library/ssl_client.h @@ -28,6 +28,7 @@ #include +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_client_hello( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_CLIENT_H */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 003434553..1280241dc 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1099,6 +1099,7 @@ struct mbedtls_ssl_flight_item * (<> 0) or not ( 0 ). * \param[out] out_len Length of the data written into the buffer \p buf */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls12_write_client_hello_exts( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -1150,7 +1151,9 @@ void mbedtls_ssl_set_inbound_transform( mbedtls_ssl_context *ssl, void mbedtls_ssl_set_outbound_transform( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ); void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ); static inline void mbedtls_ssl_handshake_set_state( mbedtls_ssl_context *ssl, @@ -1159,15 +1162,19 @@ static inline void mbedtls_ssl_handshake_set_state( mbedtls_ssl_context *ssl, ssl->state = ( int ) state; } +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_send_fatal_handshake_failure( mbedtls_ssl_context *ssl ); void mbedtls_ssl_reset_checksum( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ); void mbedtls_ssl_update_handshake_status( mbedtls_ssl_context *ssl ); @@ -1247,16 +1254,20 @@ void mbedtls_ssl_update_handshake_status( mbedtls_ssl_context *ssl ); * following the above definition. * */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, unsigned update_hs_digest ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ); /* * Write handshake message header */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_start_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, unsigned char **buf, size_t *buf_len ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, int update_checksum, int force_flush ); @@ -1268,19 +1279,28 @@ static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) /* * Write handshake message tail */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_finish_handshake_msg( mbedtls_ssl_context *ssl, size_t buf_len, size_t msg_len ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, int force_flush ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_change_cipher_spec( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ); void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, @@ -1296,10 +1316,12 @@ void mbedtls_ssl_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) #if !defined(MBEDTLS_USE_PSA_CRYPTO) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exchange_type_t key_ex ); #endif /* !MBEDTLS_USE_PSA_CRYPTO */ #if defined(MBEDTLS_SSL_CLI_C) && defined(MBEDTLS_SSL_PROTO_TLS1_2) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_conf_has_static_psk( mbedtls_ssl_config const *conf ); #endif @@ -1367,11 +1389,14 @@ mbedtls_md_type_t mbedtls_ssl_md_alg_from_hash( unsigned char hash ); unsigned char mbedtls_ssl_hash_from_md_alg( int md ); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ); #endif +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id ); #if defined(MBEDTLS_ECP_C) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id ); #endif @@ -1426,6 +1451,7 @@ static inline mbedtls_x509_crt *mbedtls_ssl_own_cert( mbedtls_ssl_context *ssl ) * * Return 0 if everything is OK, -1 if not. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, const mbedtls_ssl_ciphersuite_t *ciphersuite, int cert_endpoint, @@ -1474,21 +1500,26 @@ static inline size_t mbedtls_ssl_hs_hdr_len( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_DTLS) void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ); void mbedtls_ssl_recv_flight_completed( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ); #endif /* Visible for testing purposes only */ #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_dtls_replay_check( mbedtls_ssl_context const *ssl ); void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ); #endif +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, const mbedtls_ssl_session *src ); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) /* The hash buffer must have at least MBEDTLS_MD_MAX_SIZE bytes of length. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_get_key_exchange_md_tls1_2( mbedtls_ssl_context *ssl, unsigned char *hash, size_t *hashlen, unsigned char *data, size_t data_len, @@ -1500,11 +1531,13 @@ int mbedtls_ssl_get_key_exchange_md_tls1_2( mbedtls_ssl_context *ssl, #endif void mbedtls_ssl_transform_init( mbedtls_ssl_transform *transform ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec ); @@ -1522,10 +1555,12 @@ static inline size_t mbedtls_ssl_ep_len( const mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_resend_hello_request( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ void mbedtls_ssl_set_timer( mbedtls_ssl_context *ssl, uint32_t millisecs ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_timer( mbedtls_ssl_context *ssl ); void mbedtls_ssl_reset_in_out_pointers( mbedtls_ssl_context *ssl ); @@ -1533,6 +1568,7 @@ void mbedtls_ssl_update_out_pointers( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ); void mbedtls_ssl_update_in_pointers( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ); void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl, int partial ); @@ -1540,6 +1576,7 @@ void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl, /* * Send pending alert */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handle_pending_alert( mbedtls_ssl_context *ssl ); /* @@ -1560,6 +1597,7 @@ void mbedtls_ssl_dtls_replay_reset( mbedtls_ssl_context *ssl ); void mbedtls_ssl_handshake_wrapup_free_hs_transform( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SSL_RENEGOTIATION) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_start_renegotiation( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_RENEGOTIATION */ @@ -1624,7 +1662,9 @@ static inline int mbedtls_ssl_conf_is_hybrid_tls12_tls13( const mbedtls_ssl_conf #if defined(MBEDTLS_SSL_PROTO_TLS1_3) extern const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[ MBEDTLS_SERVER_HELLO_RANDOM_LEN ]; +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ); void mbedtls_ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ); @@ -1637,6 +1677,7 @@ void mbedtls_ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ); * \param[in] end End address of the buffer where to write the extensions * \param[out] out_len Length of the data written into the buffer \p buf */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_write_client_hello_exts( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1647,6 +1688,7 @@ int mbedtls_ssl_tls13_write_client_hello_exts( mbedtls_ssl_context *ssl, * * \param ssl SSL context */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ); /** @@ -1654,6 +1696,7 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ); * * \param ssl SSL context */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ); @@ -1746,6 +1789,7 @@ static inline int mbedtls_ssl_tls13_some_psk_enabled( mbedtls_ssl_context *ssl ) /* * Fetch TLS 1.3 handshake message header */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_fetch_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, unsigned char **buf, @@ -1754,17 +1798,20 @@ int mbedtls_ssl_tls13_fetch_handshake_msg( mbedtls_ssl_context *ssl, /* * Handler of TLS 1.3 server certificate message */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Handler of TLS 1.3 write Certificate message */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_write_certificate( mbedtls_ssl_context *ssl ); /* * Handler of TLS 1.3 write Certificate Verify message */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ @@ -1772,16 +1819,20 @@ int mbedtls_ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ); /* * Generic handler of Certificate Verify */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl ); /* * Write of dummy-CCS's for middlebox compatibility */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_ECDH_C) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( mbedtls_ssl_context *ssl, uint16_t named_group, @@ -1797,12 +1848,14 @@ int mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( /* * Parse TLS Signature Algorithm extension */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_sig_alg_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* Get handshake transcript */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, const mbedtls_md_type_t md, unsigned char *dst, @@ -2229,6 +2282,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite( #if defined(MBEDTLS_ECDH_C) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t buf_len ); @@ -2261,12 +2315,14 @@ static inline int mbedtls_ssl_tls13_cipher_suite_is_offered( * * \return 0 if valid, negative value otherwise. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_validate_ciphersuite( const mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t *suite_info, mbedtls_ssl_protocol_version min_tls_version, mbedtls_ssl_protocol_version max_tls_version ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 693b6c4df..76c1e93d8 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -121,6 +121,7 @@ extern const struct mbedtls_ssl_tls13_labels_struct mbedtls_ssl_tls13_labels; * \return A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_hkdf_expand_label( psa_algorithm_t hash_alg, const unsigned char *secret, size_t secret_len, @@ -159,6 +160,7 @@ int mbedtls_ssl_tls13_hkdf_expand_label( * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_make_traffic_keys( psa_algorithm_t hash_alg, const unsigned char *client_secret, @@ -205,6 +207,7 @@ int mbedtls_ssl_tls13_make_traffic_keys( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_derive_secret( psa_algorithm_t hash_alg, const unsigned char *secret, size_t secret_len, @@ -255,6 +258,7 @@ int mbedtls_ssl_tls13_derive_secret( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_derive_early_secrets( psa_algorithm_t hash_alg, unsigned char const *early_secret, @@ -300,6 +304,7 @@ int mbedtls_ssl_tls13_derive_early_secrets( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_derive_handshake_secrets( psa_algorithm_t hash_alg, unsigned char const *handshake_secret, @@ -350,6 +355,7 @@ int mbedtls_ssl_tls13_derive_handshake_secrets( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_derive_application_secrets( psa_algorithm_t hash_alg, unsigned char const *master_secret, @@ -380,6 +386,7 @@ int mbedtls_ssl_tls13_derive_application_secrets( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_derive_resumption_master_secret( psa_algorithm_t hash_alg, unsigned char const *application_secret, @@ -453,6 +460,7 @@ int mbedtls_ssl_tls13_derive_resumption_master_secret( * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_evolve_secret( psa_algorithm_t hash_alg, const unsigned char *secret_old, @@ -482,6 +490,7 @@ int mbedtls_ssl_tls13_evolve_secret( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_create_psk_binder( mbedtls_ssl_context *ssl, const psa_algorithm_t hash_alg, unsigned char const *psk, size_t psk_len, @@ -516,6 +525,7 @@ int mbedtls_ssl_tls13_create_psk_binder( mbedtls_ssl_context *ssl, * mbedtls_ssl_transform_encrypt(). * \return A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_populate_transform( mbedtls_ssl_transform *transform, int endpoint, int ciphersuite, @@ -542,6 +552,7 @@ int mbedtls_ssl_tls13_populate_transform( mbedtls_ssl_transform *transform, * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_key_schedule_stage_early( mbedtls_ssl_context *ssl ); /** @@ -560,6 +571,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_early( mbedtls_ssl_context *ssl ); * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); /** @@ -574,6 +586,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, mbedtls_ssl_key_set *traffic_keys ); @@ -593,6 +606,7 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ); /** @@ -607,6 +621,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_generate_application_keys( mbedtls_ssl_context* ssl, mbedtls_ssl_key_set *traffic_keys ); @@ -620,6 +635,7 @@ int mbedtls_ssl_tls13_generate_application_keys( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_generate_resumption_master_secret( mbedtls_ssl_context *ssl ); @@ -645,6 +661,7 @@ int mbedtls_ssl_tls13_generate_resumption_master_secret( * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context *ssl, unsigned char *dst, size_t dst_len, @@ -660,6 +677,7 @@ int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context *ssl, * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_compute_handshake_transform( mbedtls_ssl_context *ssl ); /** @@ -671,6 +689,7 @@ int mbedtls_ssl_tls13_compute_handshake_transform( mbedtls_ssl_context *ssl ); * \returns \c 0 on success. * \returns A negative error code on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_tls13_compute_application_transform( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ From ba65fbbe30aeb274352c03de59554b9665c47607 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Jun 2022 14:35:05 +0200 Subject: [PATCH 25/43] Fix comments Signed-off-by: Ronald Cron --- programs/ssl/ssl_test_common_source.c | 2 +- tests/ssl-opt.sh | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index c6a9a70f4..511ede9cb 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -287,7 +287,7 @@ uint16_t ssl_sig_algs_for_test[] = { #endif #if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_SHA256_C) MBEDTLS_TLS1_3_SIG_RSA_PSS_RSAE_SHA256, -#endif /* MBEDTLS_RSASSA_C && MBEDTLS_SHA256_C */ +#endif /* MBEDTLS_RSA_C && MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA1_C) /* Allow SHA-1 as we use it extensively in tests. */ MBEDTLS_SSL_SIG_ALG( MBEDTLS_SSL_HASH_SHA1 ) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c0d8b49ad..e9a809019 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6165,7 +6165,7 @@ run_test "keyUsage cli 1.3: DigitalSignature+KeyEncipherment, RSA: OK" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_openssl_tls1_3 requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli 1.3: KeyEncipherment, RSA: KO" \ +run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server2.key \ -cert data_files/server2.ku-ke.crt" \ "$P_CLI debug_level=1" \ @@ -6177,7 +6177,7 @@ run_test "keyUsage cli 1.3: KeyEncipherment, RSA: KO" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_openssl_tls1_3 requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli 1.3: KeyAgreement, RSA: KO" \ +run_test "keyUsage cli 1.3: KeyAgreement, RSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server2.key \ -cert data_files/server2.ku-ka.crt" \ "$P_CLI debug_level=1" \ @@ -6201,7 +6201,7 @@ run_test "keyUsage cli 1.3: DigitalSignature, ECDSA: OK" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_openssl_tls1_3 requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: KO" \ +run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ -cert data_files/server5.ku-ke.crt" \ "$P_CLI debug_level=1" \ @@ -6213,7 +6213,7 @@ run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: KO" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_openssl_tls1_3 requires_config_disabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: KO" \ +run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key data_files/server5.key \ -cert data_files/server5.ku-ka.crt" \ "$P_CLI debug_level=1" \ From f9c13fe69f7cabfb5046ce243057da51b0ba18b3 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 22 Jun 2022 14:35:17 +0200 Subject: [PATCH 26/43] ssl-opt.sh: Add positive check in successful "keyUsage client-auth" tests Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e9a809019..41d69a36e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6231,6 +6231,7 @@ run_test "keyUsage cli-auth: RSA, DigitalSignature: OK" \ "$O_CLI -key data_files/server2.key \ -cert data_files/server2.ku-ds.crt" \ 0 \ + -s "Verifying peer X.509 certificate... ok" \ -S "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" @@ -6258,6 +6259,7 @@ run_test "keyUsage cli-auth: ECDSA, DigitalSignature: OK" \ "$O_CLI -key data_files/server5.key \ -cert data_files/server5.ku-ds.crt" \ 0 \ + -s "Verifying peer X.509 certificate... ok" \ -S "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" @@ -6278,6 +6280,7 @@ run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature: OK" \ "$O_NEXT_CLI_NO_CERT -key data_files/server2.key \ -cert data_files/server2.ku-ds.crt" \ 0 \ + -s "Verifying peer X.509 certificate... ok" \ -S "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" @@ -6300,6 +6303,7 @@ run_test "keyUsage cli-auth 1.3: ECDSA, DigitalSignature: OK" \ "$O_NEXT_CLI_NO_CERT -key data_files/server5.key \ -cert data_files/server5.ku-ds.crt" \ 0 \ + -s "Verifying peer X.509 certificate... ok" \ -S "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" From 4cfaae5b6b98c8f05071650a33f7b0098bf35b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 23 Jun 2022 09:43:39 +0200 Subject: [PATCH 27/43] Save code size by calling get_type only once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an external function, so in the absence of link-time optimisation (LTO) the compiler can't know anything about it and has to call it the number of times it's called in the source code. This only matters for pk_ec, but change pk_rsa as well for the sake of uniformity. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 30f0492e9..867961d32 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -722,9 +722,13 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); */ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) { - return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_RSA ? - (mbedtls_rsa_context *) (pk).MBEDTLS_PRIVATE(pk_ctx) : - NULL ); + switch( mbedtls_pk_get_type( &pk ) ) + { + case MBEDTLS_PK_RSA: + return( (mbedtls_rsa_context *) (pk).MBEDTLS_PRIVATE(pk_ctx) ); + default: + return( NULL ); + } } #endif /* MBEDTLS_RSA_C */ @@ -742,11 +746,15 @@ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) */ static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) { - return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY || - mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY_DH || - mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECDSA ? - (mbedtls_ecp_keypair *) (pk).MBEDTLS_PRIVATE(pk_ctx) : - NULL ); + switch( mbedtls_pk_get_type( &pk ) ) + { + case MBEDTLS_PK_ECKEY: + case MBEDTLS_PK_ECKEY_DH: + case MBEDTLS_PK_ECDSA: + return( (mbedtls_ecp_keypair *) (pk).MBEDTLS_PRIVATE(pk_ctx) ); + default: + return( NULL ); + } } #endif /* MBEDTLS_ECP_C */ From 668b31f210356ef26b1ca26bced775b3d36f63f9 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 10 Jun 2022 14:11:31 +0100 Subject: [PATCH 28/43] Fix the wrong variable being used for TLS record size checks Fix an issue whereby a variable was used to check the size of incoming TLS records against the configured maximum prior to it being set to the right value. Signed-off-by: Paul Elliott --- ChangeLog.d/fix_tls_record_size_check.txt | 4 ++++ library/ssl_msg.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/fix_tls_record_size_check.txt diff --git a/ChangeLog.d/fix_tls_record_size_check.txt b/ChangeLog.d/fix_tls_record_size_check.txt new file mode 100644 index 000000000..13d452d61 --- /dev/null +++ b/ChangeLog.d/fix_tls_record_size_check.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix record sizes larger than 16384 being sometimes accepted despite being + non-compliant. This could not lead to a buffer overflow. In particular, + application data size was already checked correctly. diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 083c8d2e6..0b31a3bff 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3757,7 +3757,7 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl, /* Check actual (decrypted) record content length against * configured maximum. */ - if( ssl->in_msglen > MBEDTLS_SSL_IN_CONTENT_LEN ) + if( rec->data_len > MBEDTLS_SSL_IN_CONTENT_LEN ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); return( MBEDTLS_ERR_SSL_INVALID_RECORD ); From e3dac4aaa15c19bc56fc2513cea7f8289931e24b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:21:51 +0200 Subject: [PATCH 29/43] tls13: Add Certificate msg parsing tests with invalid vector lengths Signed-off-by: Ronald Cron --- library/ssl_tls13_generic.c | 22 ++-- library/ssl_tls13_invasive.h | 4 + tests/scripts/all.sh | 12 ++ tests/suites/test_suite_ssl.data | 3 + tests/suites/test_suite_ssl.function | 175 +++++++++++++++++++++++++++ 5 files changed, 207 insertions(+), 9 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index c7e00a98a..2ddefc3d4 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -31,6 +31,7 @@ #include #include "ssl_misc.h" +#include "ssl_tls13_invasive.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" @@ -391,9 +392,10 @@ cleanup: /* Parse certificate chain send by the server. */ MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +MBEDTLS_STATIC_TESTABLE +int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t certificate_request_context_len = 0; @@ -524,9 +526,10 @@ exit: } #else MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +MBEDTLS_STATIC_TESTABLE +int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { ((void) ssl); ((void) buf); @@ -657,7 +660,8 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) * with details encoded in the verification flags. All other kinds * of error codes, including those from the user provided f_vrfy * functions, are treated as fatal and lead to a failure of - * ssl_tls13_parse_certificate even if verification was optional. */ + * mbedtls_ssl_tls13_parse_certificate even if verification was optional. + */ if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && ( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE ) ) @@ -735,8 +739,8 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) &buf, &buf_len ) ); /* Parse the certificate chain sent by the peer. */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, - buf + buf_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_parse_certificate( ssl, buf, + buf + buf_len ) ); /* Validate the certificate chain and set the verification results. */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); diff --git a/library/ssl_tls13_invasive.h b/library/ssl_tls13_invasive.h index 4e39f90cf..55fc95836 100644 --- a/library/ssl_tls13_invasive.h +++ b/library/ssl_tls13_invasive.h @@ -80,6 +80,10 @@ psa_status_t mbedtls_psa_hkdf_expand( psa_algorithm_t hash_alg, const unsigned char *info, size_t info_len, unsigned char *okm, size_t okm_len ); +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); #endif /* MBEDTLS_TEST_HOOKS */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6144c2fad..27c211fbf 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2904,6 +2904,18 @@ component_test_tls13_only () { if_build_succeeded tests/ssl-opt.sh } +component_test_tls13_only_with_hooks () { + msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3 and MBEDTLS_TEST_HOOKS, without MBEDTLS_SSL_PROTO_TLS1_2" + scripts/config.py set MBEDTLS_TEST_HOOKS + make CFLAGS="'-DMBEDTLS_USER_CONFIG_FILE=\"../tests/configs/tls13-only.h\"'" + + msg "test: default config with MBEDTLS_SSL_PROTO_TLS1_3 enabled, without MBEDTLS_SSL_PROTO_TLS1_2" + if_build_succeeded make test + + msg "ssl-opt.sh (TLS 1.3)" + if_build_succeeded tests/ssl-opt.sh +} + component_test_tls13 () { msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3 enabled, without padding" scripts/config.py set MBEDTLS_SSL_PROTO_TLS1_3 diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index e82c66ed4..8ce81d0a1 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3381,3 +3381,6 @@ cookie_parsing:"16fefd0000000000000000002f010000de000000000000011efefd7b72727272 Cookie parsing: one byte overread cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d0001":MBEDTLS_ERR_SSL_DECODE_ERROR + +TLS 1.3 srv Certificate msg - wrong vector lengths +tls13_server_certificate_msg_invalid_vector_len diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 2ab2aaa7c..29a065b26 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -105,6 +105,7 @@ void init_handshake_options( handshake_test_options *opts ) opts->cli_log_fun = NULL; opts->resize_buffers = 1; } + /* * Buffer structure for custom I/O callbacks. */ @@ -2307,6 +2308,103 @@ exit: } #endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */ +/* + * Tweak vector lengths in a TLS 1.3 Certificate message + * + * /param[in] buf Buffer containing the Certificate message to tweak + * /param[in]]out] end End of the buffer to parse + * /param tweak Tweak identifier (from 1 to the number of tweaks). + * /param[out] expected_result Error code expected from the parsing function + * /param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that + * is expected to fail. All zeroes if no + * MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected. + */ +int tweak_tls13_certificate_msg_vector_len( + unsigned char *buf, unsigned char **end, int tweak, int *expected_result ) +{ +/* + * The definition of the tweaks assume that the certificate list contains only + * one certificate. + */ + +/* + * struct { + * opaque cert_data<1..2^24-1>; + * Extension extensions<0..2^16-1>; + * } CertificateEntry; + * + * struct { + * opaque certificate_request_context<0..2^8-1>; + * CertificateEntry certificate_list<0..2^24-1>; + * } Certificate; + */ + unsigned char *p_certificate_request_context_len = buf; + size_t certificate_request_context_len = buf[0]; + + unsigned char *p_certificate_list_len = buf + 1 + certificate_request_context_len; + unsigned char *certificate_list = p_certificate_list_len + 3; + size_t certificate_list_len = MBEDTLS_GET_UINT24_BE( p_certificate_list_len, 0 ); + + unsigned char *p_cert_data_len = certificate_list; + unsigned char *cert_data = p_cert_data_len + 3; + size_t cert_data_len = MBEDTLS_GET_UINT24_BE( p_cert_data_len, 0 ); + + unsigned char *p_extensions_len = cert_data + cert_data_len; + unsigned char *extensions = p_extensions_len + 2; + size_t extensions_len = MBEDTLS_GET_UINT16_BE( p_extensions_len, 0 ); + + *expected_result = MBEDTLS_ERR_SSL_DECODE_ERROR; + + switch( tweak ) + { + case 1: + /* Failure when checking if the certificate request context length and + * certificate list length can be read + */ + *end = buf + 3; + break; + + case 2: + /* Invalid certificate request context length. + */ + *p_certificate_request_context_len = + certificate_request_context_len + 1; + break; + + case 3: + /* Failure when checking if certificate_list data can be read. */ + MBEDTLS_PUT_UINT24_BE( certificate_list_len + 1, + p_certificate_list_len, 0 ); + break; + + case 4: + /* Failure when checking if the cert_data length can be read. */ + MBEDTLS_PUT_UINT24_BE( 2, p_certificate_list_len, 0 ); + break; + + case 5: + /* Failure when checking if cert_data data can be read. */ + MBEDTLS_PUT_UINT24_BE( certificate_list_len - 3 + 1, + p_cert_data_len, 0 ); + break; + + case 6: + /* Failure when checking if the extensions length can be read. */ + MBEDTLS_PUT_UINT24_BE( certificate_list_len - extensions_len - 1, + p_certificate_list_len, 0 ); + break; + + case 7: + /* Failure when checking if extensions data can be read. */ + MBEDTLS_PUT_UINT16_BE( extensions_len + 1, p_extensions_len, 0 ); + break; + + default: + return( -1 ); + } + + return( 0 ); +} /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -5708,3 +5806,80 @@ exit: USE_PSA_DONE( ); } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED */ +void tls13_server_certificate_msg_invalid_vector_len( ) +{ + int ret = -1; + mbedtls_endpoint client_ep, server_ep; + unsigned char *buf, *end; + size_t buf_len; + int step = 0; + int expected_result; + + /* + * Test set-up + */ + USE_PSA_INIT( ); + + ret = mbedtls_endpoint_init( &client_ep, MBEDTLS_SSL_IS_CLIENT, + MBEDTLS_PK_ECDSA, NULL, NULL, NULL, NULL ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_endpoint_init( &server_ep, MBEDTLS_SSL_IS_SERVER, + MBEDTLS_PK_ECDSA, NULL, NULL, NULL, NULL ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_mock_socket_connect( &(client_ep.socket), + &(server_ep.socket), 1024 ); + TEST_EQUAL( ret, 0 ); + + while( 1 ) + { + mbedtls_test_set_step( ++step ); + + ret = mbedtls_move_handshake_to_state( &(server_ep.ssl), + &(client_ep.ssl), + MBEDTLS_SSL_CERTIFICATE_VERIFY ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_ssl_flush_output( &(server_ep.ssl) ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_move_handshake_to_state( &(client_ep.ssl), + &(server_ep.ssl), + MBEDTLS_SSL_SERVER_CERTIFICATE ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_ssl_tls13_fetch_handshake_msg( &(client_ep.ssl), + MBEDTLS_SSL_HS_CERTIFICATE, + &buf, &buf_len ); + TEST_EQUAL( ret, 0 ); + + end = buf + buf_len; + + /* + * Tweak server Certificate message and parse it. + */ + + ret = tweak_tls13_certificate_msg_vector_len( + buf, &end, step, &expected_result ); + + if( ret != 0 ) + break; + + ret = mbedtls_ssl_tls13_parse_certificate( &(client_ep.ssl), buf, end ); + TEST_EQUAL( ret, expected_result ); + + ret = mbedtls_ssl_session_reset( &(client_ep.ssl) ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_ssl_session_reset( &(server_ep.ssl) ); + TEST_EQUAL( ret, 0 ); + } + +exit: + mbedtls_endpoint_free( &client_ep, NULL ); + mbedtls_endpoint_free( &server_ep, NULL ); + USE_PSA_DONE( ); +} +/* END_CASE */ From ad8c17b9c6214a076b48ecdb2d27bf55cafb7490 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:18:09 +0200 Subject: [PATCH 30/43] tls: Add overread/overwrite check failure tracking Signed-off-by: Ronald Cron --- library/ssl_misc.h | 25 +++++++++++++++++++++++++ library/ssl_tls.c | 26 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 1280241dc..816beea31 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -381,11 +381,36 @@ static inline size_t mbedtls_ssl_get_input_buflen( const mbedtls_ssl_context *ct * \return Zero if the needed space is available in the buffer, non-zero * otherwise. */ +#if ! defined(MBEDTLS_TEST_HOOKS) static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, const uint8_t *end, size_t need ) { return( ( cur > end ) || ( need > (size_t)( end - cur ) ) ); } +#else +typedef struct +{ + const uint8_t *cur; + const uint8_t *end; + size_t need; +} mbedtls_ssl_chk_buf_ptr_args; + +void mbedtls_ssl_set_chk_buf_ptr_fail_args( + const uint8_t *cur, const uint8_t *end, size_t need ); +void mbedtls_ssl_reset_chk_buf_ptr_fail_args( void ); +int mbedtls_ssl_cmp_chk_buf_ptr_fail_args( mbedtls_ssl_chk_buf_ptr_args *args ); + +static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, + const uint8_t *end, size_t need ) +{ + if( ( cur > end ) || ( need > (size_t)( end - cur ) ) ) + { + mbedtls_ssl_set_chk_buf_ptr_fail_args( cur, end, need ); + return( 1 ); + } + return( 0 ); +} +#endif /** * \brief This macro checks if the remaining size in a buffer is diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 668b5ecae..55b7f85ce 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -58,6 +58,30 @@ #include "mbedtls/oid.h" #endif +#if defined(MBEDTLS_TEST_HOOKS) +static mbedtls_ssl_chk_buf_ptr_args chk_buf_ptr_fail_args; + +void mbedtls_ssl_set_chk_buf_ptr_fail_args( + const uint8_t *cur, const uint8_t *end, size_t need ) +{ + chk_buf_ptr_fail_args.cur = cur; + chk_buf_ptr_fail_args.end = end; + chk_buf_ptr_fail_args.need = need; +} + +void mbedtls_ssl_reset_chk_buf_ptr_fail_args( void ) +{ + memset( &chk_buf_ptr_fail_args, 0, sizeof( chk_buf_ptr_fail_args ) ); +} + +int mbedtls_ssl_cmp_chk_buf_ptr_fail_args( mbedtls_ssl_chk_buf_ptr_args *args ) +{ + return( ( chk_buf_ptr_fail_args.cur != args->cur ) || + ( chk_buf_ptr_fail_args.end != args->end ) || + ( chk_buf_ptr_fail_args.need != args->need ) ); +} +#endif /* MBEDTLS_TEST_HOOKS */ + #if defined(MBEDTLS_SSL_PROTO_DTLS) #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) @@ -1103,6 +1127,8 @@ void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl, memset( ssl->in_buf, 0, in_buf_len ); } + ssl->send_alert = 0; + /* Reset outgoing message writing */ ssl->out_msgtype = 0; ssl->out_msglen = 0; From e7b9b6b3805a1217b7c8922891f29d48dd9ec041 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:24:31 +0200 Subject: [PATCH 31/43] tls13: Add checks of overread check failures In Certificate message parsing tests with invalid vector lengths, add checks that the parsing failed on the expected overread check. Signed-off-by: Ronald Cron --- tests/suites/test_suite_ssl.function | 43 ++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 29a065b26..97eccbd24 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -106,6 +106,22 @@ void init_handshake_options( handshake_test_options *opts ) opts->resize_buffers = 1; } +#if defined(MBEDTLS_TEST_HOOKS) +static void set_chk_buf_ptr_args( + mbedtls_ssl_chk_buf_ptr_args *args, + unsigned char *cur, unsigned char *end, size_t need ) +{ + args->cur = cur; + args->end = end; + args->need = need; +} + +static void reset_chk_buf_ptr_args( mbedtls_ssl_chk_buf_ptr_args *args ) +{ + memset( args, 0, sizeof( *args ) ); +} +#endif /* MBEDTLS_TEST_HOOKS */ + /* * Buffer structure for custom I/O callbacks. */ @@ -2308,6 +2324,7 @@ exit: } #endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */ +#if defined(MBEDTLS_TEST_HOOKS) /* * Tweak vector lengths in a TLS 1.3 Certificate message * @@ -2320,7 +2337,8 @@ exit: * MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected. */ int tweak_tls13_certificate_msg_vector_len( - unsigned char *buf, unsigned char **end, int tweak, int *expected_result ) + unsigned char *buf, unsigned char **end, int tweak, + int *expected_result, mbedtls_ssl_chk_buf_ptr_args *args ) { /* * The definition of the tweaks assume that the certificate list contains only @@ -2362,6 +2380,7 @@ int tweak_tls13_certificate_msg_vector_len( * certificate list length can be read */ *end = buf + 3; + set_chk_buf_ptr_args( args, buf, *end, 4 ); break; case 2: @@ -2369,34 +2388,46 @@ int tweak_tls13_certificate_msg_vector_len( */ *p_certificate_request_context_len = certificate_request_context_len + 1; + reset_chk_buf_ptr_args( args ); break; case 3: /* Failure when checking if certificate_list data can be read. */ MBEDTLS_PUT_UINT24_BE( certificate_list_len + 1, p_certificate_list_len, 0 ); + set_chk_buf_ptr_args( args, certificate_list, *end, + certificate_list_len + 1 ); break; case 4: /* Failure when checking if the cert_data length can be read. */ MBEDTLS_PUT_UINT24_BE( 2, p_certificate_list_len, 0 ); + set_chk_buf_ptr_args( args, p_cert_data_len, certificate_list + 2, 3 ); break; case 5: /* Failure when checking if cert_data data can be read. */ MBEDTLS_PUT_UINT24_BE( certificate_list_len - 3 + 1, p_cert_data_len, 0 ); + set_chk_buf_ptr_args( args, cert_data, + certificate_list + certificate_list_len, + certificate_list_len - 3 + 1 ); break; case 6: /* Failure when checking if the extensions length can be read. */ MBEDTLS_PUT_UINT24_BE( certificate_list_len - extensions_len - 1, p_certificate_list_len, 0 ); + set_chk_buf_ptr_args( args, p_extensions_len, + certificate_list + certificate_list_len - extensions_len - 1, 2 ); break; case 7: /* Failure when checking if extensions data can be read. */ MBEDTLS_PUT_UINT16_BE( extensions_len + 1, p_extensions_len, 0 ); + + set_chk_buf_ptr_args( args, extensions, + certificate_list + certificate_list_len, extensions_len + 1 ); break; default: @@ -2405,6 +2436,7 @@ int tweak_tls13_certificate_msg_vector_len( return( 0 ); } +#endif /* MBEDTLS_TEST_HOOKS */ /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -5815,6 +5847,7 @@ void tls13_server_certificate_msg_invalid_vector_len( ) size_t buf_len; int step = 0; int expected_result; + mbedtls_ssl_chk_buf_ptr_args expected_chk_buf_ptr_args; /* * Test set-up @@ -5862,7 +5895,7 @@ void tls13_server_certificate_msg_invalid_vector_len( ) */ ret = tweak_tls13_certificate_msg_vector_len( - buf, &end, step, &expected_result ); + buf, &end, step, &expected_result, &expected_chk_buf_ptr_args ); if( ret != 0 ) break; @@ -5870,6 +5903,11 @@ void tls13_server_certificate_msg_invalid_vector_len( ) ret = mbedtls_ssl_tls13_parse_certificate( &(client_ep.ssl), buf, end ); TEST_EQUAL( ret, expected_result ); + TEST_ASSERT( mbedtls_ssl_cmp_chk_buf_ptr_fail_args( + &expected_chk_buf_ptr_args ) == 0 ); + + mbedtls_ssl_reset_chk_buf_ptr_fail_args( ); + ret = mbedtls_ssl_session_reset( &(client_ep.ssl) ); TEST_EQUAL( ret, 0 ); @@ -5878,6 +5916,7 @@ void tls13_server_certificate_msg_invalid_vector_len( ) } exit: + mbedtls_ssl_reset_chk_buf_ptr_fail_args( ); mbedtls_endpoint_free( &client_ep, NULL ); mbedtls_endpoint_free( &server_ep, NULL ); USE_PSA_DONE( ); From 2b1a43c1011a03ae0050273b390cccd6d78853fc Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:03:54 +0200 Subject: [PATCH 32/43] tls13: Add missing overread check in Certificate msg parsing. Signed-off-by: Ronald Cron --- library/ssl_tls13_generic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 2ddefc3d4..7d0559bc2 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -446,6 +446,7 @@ int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, mbedtls_x509_crt_init( ssl->session_negotiate->peer_cert ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_list_len ); certificate_list_end = p + certificate_list_len; while( p < certificate_list_end ) { From e0d7367a9ed3e6096914686c19f8175d00de44a5 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Jun 2022 15:38:26 +0200 Subject: [PATCH 33/43] Add change log Signed-off-by: Ronald Cron --- ChangeLog.d/tls13-add-missing-overread-check.txt | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 ChangeLog.d/tls13-add-missing-overread-check.txt diff --git a/ChangeLog.d/tls13-add-missing-overread-check.txt b/ChangeLog.d/tls13-add-missing-overread-check.txt new file mode 100644 index 000000000..4552cd735 --- /dev/null +++ b/ChangeLog.d/tls13-add-missing-overread-check.txt @@ -0,0 +1,8 @@ +Security + * Fix a buffer overread in TLS 1.3 Certificate parsing. An unauthenticated + client or server could cause an MbedTLS server or client to overread up + to 64 kBytes of data and potentially overread the input buffer by that + amount minus the size of the input buffer. As overread data undergoes + various checks, the likelihood of reaching the boundary of the input + buffer is rather small but increases as its size + MBEDTLS_SSL_IN_CONTENT_LEN decreases. From cf600bc07c19bbe104636baee09fe73612d615d0 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Jun 2022 15:54:16 +0200 Subject: [PATCH 34/43] Comment fixes Signed-off-by: Ronald Cron --- library/ssl_misc.h | 2 +- tests/suites/test_suite_ssl.function | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 816beea31..c17fa20f3 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -410,7 +410,7 @@ static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, } return( 0 ); } -#endif +#endif /* MBEDTLS_TEST_HOOKS */ /** * \brief This macro checks if the remaining size in a buffer is diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 97eccbd24..59c69c6af 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2328,11 +2328,11 @@ exit: /* * Tweak vector lengths in a TLS 1.3 Certificate message * - * /param[in] buf Buffer containing the Certificate message to tweak - * /param[in]]out] end End of the buffer to parse - * /param tweak Tweak identifier (from 1 to the number of tweaks). - * /param[out] expected_result Error code expected from the parsing function - * /param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that + * \param[in] buf Buffer containing the Certificate message to tweak + * \param[in]]out] end End of the buffer to parse + * \param tweak Tweak identifier (from 1 to the number of tweaks). + * \param[out] expected_result Error code expected from the parsing function + * \param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that * is expected to fail. All zeroes if no * MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected. */ From 251ca25d94d16001b0a20f56667e373af30bd0d3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 27 Jun 2022 14:47:15 +0200 Subject: [PATCH 35/43] Clarify potential ambiguity in changelog entry Signed-off-by: Gilles Peskine --- ChangeLog.d/bignum-0-mod-2.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/bignum-0-mod-2.txt b/ChangeLog.d/bignum-0-mod-2.txt index 55e53e5ff..4a1ab161d 100644 --- a/ChangeLog.d/bignum-0-mod-2.txt +++ b/ChangeLog.d/bignum-0-mod-2.txt @@ -1,4 +1,4 @@ Bugfix * Fix a null pointer dereference when performing some operations on zero - represented with 0 limbs: mbedtls_mpi_mod_int() dividing by 2, or - mbedtls_mpi_write_string() in base 2. + represented with 0 limbs (specifically mbedtls_mpi_mod_int() dividing + by 2, and mbedtls_mpi_write_string() in base 2). From 6497b5a1d10e59caa45c77bdcfacfeada1337ac0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 17:01:40 +0200 Subject: [PATCH 36/43] Add setbuf platform function Add a platform function mbedtls_setbuf(), defaulting to setbuf(). The intent is to allow disabling stdio buffering when reading or writing files with sensitive data, because this exposes the sensitive data to a subsequent memory disclosure vulnerability. Signed-off-by: Gilles Peskine --- include/mbedtls/check_config.h | 14 ++++++++ include/mbedtls/mbedtls_config.h | 3 ++ include/mbedtls/platform.h | 55 ++++++++++++++++++++++++++++++++ library/platform.c | 28 ++++++++++++++++ tests/scripts/all.sh | 1 + 5 files changed, 101 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 1ced6e578..0ac9ccac4 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -368,6 +368,20 @@ #error "MBEDTLS_PLATFORM_EXIT_MACRO and MBEDTLS_PLATFORM_STD_EXIT/MBEDTLS_PLATFORM_EXIT_ALT cannot be defined simultaneously" #endif +#if defined(MBEDTLS_PLATFORM_SETBUF_ALT) && !defined(MBEDTLS_PLATFORM_C) +#error "MBEDTLS_PLATFORM_SETBUF_ALT defined, but not all prerequisites" +#endif + +#if defined(MBEDTLS_PLATFORM_SETBUF_MACRO) && !defined(MBEDTLS_PLATFORM_C) +#error "MBEDTLS_PLATFORM_SETBUF_MACRO defined, but not all prerequisites" +#endif + +#if defined(MBEDTLS_PLATFORM_SETBUF_MACRO) &&\ + ( defined(MBEDTLS_PLATFORM_STD_SETBUF) ||\ + defined(MBEDTLS_PLATFORM_SETBUF_ALT) ) +#error "MBEDTLS_PLATFORM_SETBUF_MACRO and MBEDTLS_PLATFORM_STD_SETBUF/MBEDTLS_PLATFORM_SETBUF_ALT cannot be defined simultaneously" +#endif + #if defined(MBEDTLS_PLATFORM_TIME_ALT) &&\ ( !defined(MBEDTLS_PLATFORM_C) ||\ !defined(MBEDTLS_HAVE_TIME) ) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 2d32f67cc..ad9696135 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -225,6 +225,7 @@ * Uncomment a macro to enable alternate implementation of specific base * platform function */ +//#define MBEDTLS_PLATFORM_SETBUF_ALT //#define MBEDTLS_PLATFORM_EXIT_ALT //#define MBEDTLS_PLATFORM_TIME_ALT //#define MBEDTLS_PLATFORM_FPRINTF_ALT @@ -3288,6 +3289,7 @@ //#define MBEDTLS_PLATFORM_STD_MEM_HDR /**< Header to include if MBEDTLS_PLATFORM_NO_STD_FUNCTIONS is defined. Don't define if no header is needed. */ //#define MBEDTLS_PLATFORM_STD_CALLOC calloc /**< Default allocator to use, can be undefined */ //#define MBEDTLS_PLATFORM_STD_FREE free /**< Default free to use, can be undefined */ +//#define MBEDTLS_PLATFORM_STD_SETBUF setbuf /**< Default setbuf to use, can be undefined */ //#define MBEDTLS_PLATFORM_STD_EXIT exit /**< Default exit to use, can be undefined */ //#define MBEDTLS_PLATFORM_STD_TIME time /**< Default time to use, can be undefined. MBEDTLS_HAVE_TIME must be enabled */ //#define MBEDTLS_PLATFORM_STD_FPRINTF fprintf /**< Default fprintf to use, can be undefined */ @@ -3305,6 +3307,7 @@ //#define MBEDTLS_PLATFORM_CALLOC_MACRO calloc /**< Default allocator macro to use, can be undefined */ //#define MBEDTLS_PLATFORM_FREE_MACRO free /**< Default free macro to use, can be undefined */ //#define MBEDTLS_PLATFORM_EXIT_MACRO exit /**< Default exit macro to use, can be undefined */ +//#define MBEDTLS_PLATFORM_SETBUF_MACRO setbuf /**< Default setbuf macro to use, can be undefined */ //#define MBEDTLS_PLATFORM_TIME_MACRO time /**< Default time macro to use, can be undefined. MBEDTLS_HAVE_TIME must be enabled */ //#define MBEDTLS_PLATFORM_TIME_TYPE_MACRO time_t /**< Default time macro to use, can be undefined. MBEDTLS_HAVE_TIME must be enabled */ //#define MBEDTLS_PLATFORM_FPRINTF_MACRO fprintf /**< Default fprintf macro to use, can be undefined */ diff --git a/include/mbedtls/platform.h b/include/mbedtls/platform.h index a5984345b..bad442e72 100644 --- a/include/mbedtls/platform.h +++ b/include/mbedtls/platform.h @@ -91,6 +91,9 @@ extern "C" { #if !defined(MBEDTLS_PLATFORM_STD_FREE) #define MBEDTLS_PLATFORM_STD_FREE free /**< The default \c free function to use. */ #endif +#if !defined(MBEDTLS_PLATFORM_STD_SETBUF) +#define MBEDTLS_PLATFORM_STD_SETBUF setbuf /**< The default \c setbuf function to use. */ +#endif #if !defined(MBEDTLS_PLATFORM_STD_EXIT) #define MBEDTLS_PLATFORM_STD_EXIT exit /**< The default \c exit function to use. */ #endif @@ -276,6 +279,58 @@ int mbedtls_platform_set_vsnprintf( int (*vsnprintf_func)( char * s, size_t n, #endif /* MBEDTLS_PLATFORM_VSNPRINTF_MACRO */ #endif /* MBEDTLS_PLATFORM_VSNPRINTF_ALT */ +/* + * The function pointers for setbuf + */ +#if defined(MBEDTLS_PLATFORM_SETBUF_ALT) +#include +/** + * \brief Function pointer to call for `setbuf()` functionality + * (changing the internal buffering on stdio calls). + * + * \note The library calls this function to disable + * buffering when reading or writing sensitive data, + * to avoid having extra copies of sensitive data + * remaining in stdio buffers after the file is + * closed. If this is not a concern, for example if + * your platform's stdio doesn't have any buffering, + * you can set mbedtls_setbuf to a function that + * does nothing. + * + * \param setbuf_func The \c setbuf function implementation. + * It is always called with `buf` equal to `NULL`. + * + * \return \c 0 on success, negative on error. + */ +extern void (*mbedtls_setbuf)( FILE *stream, char *buf ); + +/** + * \brief Dynamically configure the function that is called + * when the mbedtls_setbuf() function is called by the + * library. + * + * \param setbuf_func The \c setbuf function implementation + * + * \return \c 0 + */ +int mbedtls_platform_set_setbuf( void (*setbuf_func)( + FILE *stream, char *buf ) ); +#elif defined(MBEDTLS_PLATFORM_SETBUF_MACRO) +/** + * \brief Macro defining the function for the library to + * call for `setbuf` functionality (changing the + * internal buffering on stdio calls). + * + * \note See extra comments on the mbedtls_setbuf() function + * pointer above. + * + * \return \c 0 on success, negative on error. + */ +#define mbedtls_setbuf MBEDTLS_PLATFORM_SETBUF_MACRO +#else +#define mbedtls_setbuf setbuf +#endif /* MBEDTLS_PLATFORM_SETBUF_ALT / MBEDTLS_PLATFORM_SETBUF_MACRO */ + /* * The function pointers for exit */ diff --git a/library/platform.c b/library/platform.c index e742fde7c..f3a1f9847 100644 --- a/library/platform.c +++ b/library/platform.c @@ -226,6 +226,28 @@ int mbedtls_platform_set_fprintf( int (*fprintf_func)( FILE *, const char *, ... } #endif /* MBEDTLS_PLATFORM_FPRINTF_ALT */ +#if defined(MBEDTLS_PLATFORM_SETBUF_ALT) +#if !defined(MBEDTLS_PLATFORM_STD_SETBUF) +/* + * Make dummy function to prevent NULL pointer dereferences + */ +static void platform_setbuf_uninit( FILE *stream, char *buf ) +{ + (( void ) stream); + (( void ) buf); +} + +#define MBEDTLS_PLATFORM_STD_SETBUF platform_setbuf_uninit +#endif /* !MBEDTLS_PLATFORM_STD_SETBUF */ +void (*mbedtls_setbuf)( FILE *stream, char *buf ) = MBEDTLS_PLATFORM_STD_SETBUF; + +int mbedtls_platform_set_setbuf( void (*setbuf_func)( FILE *stream, char *buf ) ) +{ + mbedtls_setbuf = setbuf_func; + return( 0 ); +} +#endif /* MBEDTLS_PLATFORM_SETBUF_ALT */ + #if defined(MBEDTLS_PLATFORM_EXIT_ALT) #if !defined(MBEDTLS_PLATFORM_STD_EXIT) /* @@ -288,6 +310,9 @@ int mbedtls_platform_std_nv_seed_read( unsigned char *buf, size_t buf_len ) if( ( file = fopen( MBEDTLS_PLATFORM_STD_NV_SEED_FILE, "rb" ) ) == NULL ) return( -1 ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + if( ( n = fread( buf, 1, buf_len, file ) ) != buf_len ) { fclose( file ); @@ -307,6 +332,9 @@ int mbedtls_platform_std_nv_seed_write( unsigned char *buf, size_t buf_len ) if( ( file = fopen( MBEDTLS_PLATFORM_STD_NV_SEED_FILE, "w" ) ) == NULL ) return -1; + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + if( ( n = fwrite( buf, 1, buf_len, file ) ) != buf_len ) { fclose( file ); diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1e5cd65ee..1a3a04b82 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2178,6 +2178,7 @@ component_test_no_platform () { scripts/config.py unset MBEDTLS_PLATFORM_SNPRINTF_ALT scripts/config.py unset MBEDTLS_PLATFORM_TIME_ALT scripts/config.py unset MBEDTLS_PLATFORM_EXIT_ALT + scripts/config.py unset MBEDTLS_PLATFORM_SETBUF_ALT scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED scripts/config.py unset MBEDTLS_FS_IO From da0913ba6b3ddd5708189df8115d85bbd0ff2be3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 17:03:40 +0200 Subject: [PATCH 37/43] Call setbuf when reading or writing files: library After opening a file containing sensitive data, call mbedtls_setbuf() to disable buffering. This way, we don't expose sensitive data to a memory disclosure vulnerability in a buffer outside our control. This commit adds a call to mbedtls_setbuf() after each call to fopen(), except: * In ctr_drbg.c, in load_file(), because this is only used for DH parameters and they are not confidential data. * In psa_its_file.c, in psa_its_remove(), because the file is only opened to check its existence, we don't read data from it. Signed-off-by: Gilles Peskine --- library/ctr_drbg.c | 6 ++++++ library/dhm.c | 1 + library/entropy.c | 6 ++++++ library/entropy_poll.c | 5 ++++- library/hmac_drbg.c | 6 ++++++ library/md.c | 3 +++ library/pkparse.c | 3 +++ library/psa_its_file.c | 7 +++++++ 8 files changed, 36 insertions(+), 1 deletion(-) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 93a7cdcd1..a2f55cc59 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -607,6 +607,9 @@ int mbedtls_ctr_drbg_write_seed_file( mbedtls_ctr_drbg_context *ctx, if( ( f = fopen( path, "wb" ) ) == NULL ) return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( ( ret = mbedtls_ctr_drbg_random( ctx, buf, MBEDTLS_CTR_DRBG_MAX_INPUT ) ) != 0 ) goto exit; @@ -640,6 +643,9 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + n = fread( buf, 1, sizeof( buf ), f ); if( fread( &c, 1, 1, f ) != 0 ) { diff --git a/library/dhm.c b/library/dhm.c index 2ce0ed4fd..1e95bdab0 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -620,6 +620,7 @@ static int load_file( const char *path, unsigned char **buf, size_t *n ) if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_DHM_FILE_IO_ERROR ); + /* The data loaded here is public, so don't bother disabling buffering. */ fseek( f, 0, SEEK_END ); if( ( size = ftell( f ) ) == -1 ) diff --git a/library/entropy.c b/library/entropy.c index 9e31f8491..08c5bd7d1 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -457,6 +457,9 @@ int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *p goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) { ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; @@ -484,6 +487,9 @@ int mbedtls_entropy_update_seed_file( mbedtls_entropy_context *ctx, const char * if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + fseek( f, 0, SEEK_END ); n = (size_t) ftell( f ); fseek( f, 0, SEEK_SET ); diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 058c307df..2ae57fdc0 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -35,7 +35,7 @@ #if defined(MBEDTLS_TIMING_C) #include "mbedtls/timing.h" #endif -#if defined(MBEDTLS_ENTROPY_NV_SEED) +#if defined(MBEDTLS_ENTROPY_NV_SEED) || !defined(HAVE_SYSCTL_ARND) #include "mbedtls/platform.h" #endif @@ -195,6 +195,9 @@ int mbedtls_platform_entropy_poll( void *data, if( file == NULL ) return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + read_len = fread( output, 1, len, file ); if( read_len != len ) { diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index ab353bfd5..8b13a860f 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -436,6 +436,9 @@ int mbedtls_hmac_drbg_write_seed_file( mbedtls_hmac_drbg_context *ctx, const cha if( ( f = fopen( path, "wb" ) ) == NULL ) return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( ( ret = mbedtls_hmac_drbg_random( ctx, buf, sizeof( buf ) ) ) != 0 ) goto exit; @@ -465,6 +468,9 @@ int mbedtls_hmac_drbg_update_seed_file( mbedtls_hmac_drbg_context *ctx, const ch if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + n = fread( buf, 1, sizeof( buf ), f ); if( fread( &c, 1, 1, f ) != 0 ) { diff --git a/library/md.c b/library/md.c index f2c1a90f8..a387da50a 100644 --- a/library/md.c +++ b/library/md.c @@ -605,6 +605,9 @@ int mbedtls_md_file( const mbedtls_md_info_t *md_info, const char *path, unsigne if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_MD_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + mbedtls_md_init( &ctx ); if( ( ret = mbedtls_md_setup( &ctx, md_info, 0 ) ) != 0 ) diff --git a/library/pkparse.c b/library/pkparse.c index 22dab3ad7..0e5dd5546 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -82,6 +82,9 @@ int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n ) if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_PK_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + fseek( f, 0, SEEK_END ); if( ( size = ftell( f ) ) == -1 ) { diff --git a/library/psa_its_file.c b/library/psa_its_file.c index f05872095..b7c2e6b04 100644 --- a/library/psa_its_file.c +++ b/library/psa_its_file.c @@ -102,6 +102,9 @@ static psa_status_t psa_its_read_file( psa_storage_uid_t uid, if( *p_stream == NULL ) return( PSA_ERROR_DOES_NOT_EXIST ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( *p_stream, NULL ); + n = fread( &header, 1, sizeof( header ), *p_stream ); if( n != sizeof( header ) ) return( PSA_ERROR_DATA_CORRUPT ); @@ -201,9 +204,13 @@ psa_status_t psa_its_set( psa_storage_uid_t uid, psa_its_fill_filename( uid, filename ); stream = fopen( PSA_ITS_STORAGE_TEMP, "wb" ); + if( stream == NULL ) goto exit; + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( stream, NULL ); + status = PSA_ERROR_INSUFFICIENT_STORAGE; n = fwrite( &header, 1, sizeof( header ), stream ); if( n != sizeof( header ) ) From 6d576c9646fec898d938232d03aaad7c048e6aa1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 17:06:11 +0200 Subject: [PATCH 38/43] Call setbuf when reading or writing files: programs After opening a file containing sensitive data, call mbedtls_setbuf() to disable buffering. This way, we don't expose sensitive data to a memory disclosure vulnerability in a buffer outside our control. This commit adds a call to mbedtls_setbuf() after each call to fopen(), but only in sample programs that were calling mbedtls_platform_zeroize(). Don't bother protecting stdio buffers in programs where application buffers weren't protected. Signed-off-by: Gilles Peskine --- programs/aes/crypt_and_hash.c | 4 ++++ programs/psa/key_ladder_demo.c | 13 +++++++++++++ programs/ssl/ssl_test_common_source.c | 4 ++++ programs/ssl/ssl_test_lib.h | 1 + 4 files changed, 22 insertions(+) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index 74ea88c3c..fa874b240 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -171,6 +171,10 @@ int main( int argc, char *argv[] ) goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( fin, NULL ); + mbedtls_setbuf( fout, NULL ); + /* * Read the Cipher and MD from the command line */ diff --git a/programs/psa/key_ladder_demo.c b/programs/psa/key_ladder_demo.c index cad875e01..13037190d 100644 --- a/programs/psa/key_ladder_demo.c +++ b/programs/psa/key_ladder_demo.c @@ -56,6 +56,7 @@ #include #include +#include "mbedtls/platform.h" // for mbedtls_setbuf #include "mbedtls/platform_util.h" // for mbedtls_platform_zeroize #include @@ -177,6 +178,8 @@ static psa_status_t save_key( psa_key_id_t key, key_data, sizeof( key_data ), &key_size ) ); SYS_CHECK( ( key_file = fopen( output_file_name, "wb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( key_file, NULL ); SYS_CHECK( fwrite( key_data, 1, key_size, key_file ) == key_size ); SYS_CHECK( fclose( key_file ) == 0 ); key_file = NULL; @@ -231,6 +234,8 @@ static psa_status_t import_key_from_file( psa_key_usage_t usage, unsigned char extra_byte; SYS_CHECK( ( key_file = fopen( key_file_name, "rb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( key_file, NULL ); SYS_CHECK( ( key_size = fread( key_data, 1, sizeof( key_data ), key_file ) ) != 0 ); if( fread( &extra_byte, 1, 1, key_file ) != 0 ) @@ -372,6 +377,8 @@ static psa_status_t wrap_data( const char *input_file_name, /* Find the size of the data to wrap. */ SYS_CHECK( ( input_file = fopen( input_file_name, "rb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( input_file, NULL ); SYS_CHECK( fseek( input_file, 0, SEEK_END ) == 0 ); SYS_CHECK( ( input_position = ftell( input_file ) ) != -1 ); #if LONG_MAX > SIZE_MAX @@ -418,6 +425,8 @@ static psa_status_t wrap_data( const char *input_file_name, /* Write the output. */ SYS_CHECK( ( output_file = fopen( output_file_name, "wb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( output_file, NULL ); SYS_CHECK( fwrite( &header, 1, sizeof( header ), output_file ) == sizeof( header ) ); SYS_CHECK( fwrite( buffer, 1, ciphertext_size, @@ -453,6 +462,8 @@ static psa_status_t unwrap_data( const char *input_file_name, /* Load and validate the header. */ SYS_CHECK( ( input_file = fopen( input_file_name, "rb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( input_file, NULL ); SYS_CHECK( fread( &header, 1, sizeof( header ), input_file ) == sizeof( header ) ); if( memcmp( &header.magic, WRAPPED_DATA_MAGIC, @@ -509,6 +520,8 @@ static psa_status_t unwrap_data( const char *input_file_name, /* Write the output. */ SYS_CHECK( ( output_file = fopen( output_file_name, "wb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( output_file, NULL ); SYS_CHECK( fwrite( buffer, 1, plaintext_size, output_file ) == plaintext_size ); SYS_CHECK( fclose( output_file ) == 0 ); diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index 0e66895db..23237d1a2 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -101,6 +101,10 @@ void nss_keylog_export( void *p_expkey, goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be + * wiped. */ + mbedtls_setbuf( f, NULL ); + if( fwrite( nss_keylog_line, 1, len, f ) != len ) { fclose( f ); diff --git a/programs/ssl/ssl_test_lib.h b/programs/ssl/ssl_test_lib.h index f0d0c3b89..d2354bbd4 100644 --- a/programs/ssl/ssl_test_lib.h +++ b/programs/ssl/ssl_test_lib.h @@ -35,6 +35,7 @@ #define mbedtls_fprintf fprintf #define mbedtls_snprintf snprintf #define mbedtls_exit exit +#define mbedtls_setbuf setbuf #define MBEDTLS_EXIT_SUCCESS EXIT_SUCCESS #define MBEDTLS_EXIT_FAILURE EXIT_FAILURE #endif From cf4d9f98c79b45d7b5f38880d0bb7535d5b408b0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 17:07:47 +0200 Subject: [PATCH 39/43] Changelog entry for mbedtls_setbuf() * Security: we're improving a countermeasure. * Requirement change: the library will no longer compile on a platform without setbuf(). Signed-off-by: Gilles Peskine --- ChangeLog.d/add_mbedtls_setbuf.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 ChangeLog.d/add_mbedtls_setbuf.txt diff --git a/ChangeLog.d/add_mbedtls_setbuf.txt b/ChangeLog.d/add_mbedtls_setbuf.txt new file mode 100644 index 000000000..6152d60df --- /dev/null +++ b/ChangeLog.d/add_mbedtls_setbuf.txt @@ -0,0 +1,10 @@ +Security + * Add the platform function mbedtls_setbuf() to allow buffering to be + disabled on stdio files, to stop secrets loaded from said files being + potentially left in memory after file operations. Reported by + Glenn Strauss. +Requirement changes + * The library will no longer compile out of the box on a platform without + setbuf() if MBEDTLS_FS_IO is enabled. If your platform does not have + setbuf(), you can configure an alternative function by enabling + MBEDTLS_PLATFORM_SETBUF_ALT or MBEDTLS_PLATFORM_SETBUF_MACRO. From 0bd76ee2eda38c4813477eea05a86e8c76283488 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 19:32:02 +0200 Subject: [PATCH 40/43] Fix Doxygen documentation attached to non-existent elements Signed-off-by: Gilles Peskine --- include/mbedtls/platform.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/platform.h b/include/mbedtls/platform.h index bad442e72..a5a43ac6d 100644 --- a/include/mbedtls/platform.h +++ b/include/mbedtls/platform.h @@ -297,10 +297,8 @@ int mbedtls_platform_set_vsnprintf( int (*vsnprintf_func)( char * s, size_t n, * you can set mbedtls_setbuf to a function that * does nothing. * - * \param setbuf_func The \c setbuf function implementation. - * It is always called with `buf` equal to `NULL`. - * - * \return \c 0 on success, negative on error. + * The library always calls this function with + * `buf` equal to `NULL`. */ extern void (*mbedtls_setbuf)( FILE *stream, char *buf ); From ff15dbab4cfb308a8c6bf2036d1b192c8f185f7a Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 1 Jul 2022 16:30:08 +0100 Subject: [PATCH 41/43] Make definition order a bit neater Signed-off-by: Paul Elliott --- programs/ssl/ssl_test_lib.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/ssl_test_lib.h b/programs/ssl/ssl_test_lib.h index d2354bbd4..e37a9d8f3 100644 --- a/programs/ssl/ssl_test_lib.h +++ b/programs/ssl/ssl_test_lib.h @@ -34,8 +34,8 @@ #define mbedtls_printf printf #define mbedtls_fprintf fprintf #define mbedtls_snprintf snprintf -#define mbedtls_exit exit #define mbedtls_setbuf setbuf +#define mbedtls_exit exit #define MBEDTLS_EXIT_SUCCESS EXIT_SUCCESS #define MBEDTLS_EXIT_FAILURE EXIT_FAILURE #endif From dfb5da2a99f8c3118914f86a1b295bf622ca73af Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 1 Jul 2022 16:32:14 +0100 Subject: [PATCH 42/43] Fix changelog requirements section. Setbuf() is currently not guarded by MBEDTLS_FS_IO, nor can it easily be so. Raised a seperate issue to cover this, and removed the mention of MBEDTLS_FS_IO from the Changelog. Signed-off-by: Paul Elliott --- ChangeLog.d/add_mbedtls_setbuf.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/add_mbedtls_setbuf.txt b/ChangeLog.d/add_mbedtls_setbuf.txt index 6152d60df..d14cd18aa 100644 --- a/ChangeLog.d/add_mbedtls_setbuf.txt +++ b/ChangeLog.d/add_mbedtls_setbuf.txt @@ -5,6 +5,7 @@ Security Glenn Strauss. Requirement changes * The library will no longer compile out of the box on a platform without - setbuf() if MBEDTLS_FS_IO is enabled. If your platform does not have - setbuf(), you can configure an alternative function by enabling - MBEDTLS_PLATFORM_SETBUF_ALT or MBEDTLS_PLATFORM_SETBUF_MACRO. + setbuf(). If your platform does not have setbuf(), you can configure an + alternative function by enabling MBEDTLS_PLATFORM_SETBUF_ALT or + MBEDTLS_PLATFORM_SETBUF_MACRO. + From c466ec2e73a8ce808f99fbee0411b8fb98a66e9c Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 1 Jul 2022 16:43:25 +0100 Subject: [PATCH 43/43] Fix code formatting Signed-off-by: Paul Elliott --- library/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/platform.c b/library/platform.c index f3a1f9847..6151e6c49 100644 --- a/library/platform.c +++ b/library/platform.c @@ -233,8 +233,8 @@ int mbedtls_platform_set_fprintf( int (*fprintf_func)( FILE *, const char *, ... */ static void platform_setbuf_uninit( FILE *stream, char *buf ) { - (( void ) stream); - (( void ) buf); + ((void) stream); + ((void) buf); } #define MBEDTLS_PLATFORM_STD_SETBUF platform_setbuf_uninit