From 75b1fe7199e42fb4ad3ed5b9e8fc01dc56e1966a Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 14:32:30 +0100 Subject: [PATCH 01/20] Refactor ARIA_SELF_TEST_IF_FAIL macro Change the ARIA_SELF_TEST_IF_FAIL macro to be more code-style friendly. Currently it expands to the body of an if statement, which causes problems for automatic brace-addition for if statements. Convert the macro to a function-like macro that takes the condition as an argument and expands to a full if statement inside a do {} while (0) idiom. Signed-off-by: David Horstmann --- library/aria.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/library/aria.c b/library/aria.c index f78d289a4..6c4f30d94 100644 --- a/library/aria.c +++ b/library/aria.c @@ -888,15 +888,17 @@ static const uint8_t aria_test2_ctr_ct[3][48] = // CTR ciphertext }; #endif /* MBEDTLS_CIPHER_MODE_CFB */ -#define ARIA_SELF_TEST_IF_FAIL \ - { \ - if( verbose ) \ - mbedtls_printf( "failed\n" ); \ - goto exit; \ - } else { \ - if( verbose ) \ - mbedtls_printf( "passed\n" ); \ - } +#define ARIA_SELF_TEST_IF_FAIL( cond ) \ + do { \ + if( cond ) { \ + if( verbose ) \ + mbedtls_printf( "failed\n" ); \ + goto exit; \ + } else { \ + if( verbose ) \ + mbedtls_printf( "passed\n" ); \ + } \ + } while( 0 ) /* * Checkup routine @@ -930,16 +932,18 @@ int mbedtls_aria_self_test( int verbose ) mbedtls_printf( " ARIA-ECB-%d (enc): ", 128 + 64 * i ); mbedtls_aria_setkey_enc( &ctx, aria_test1_ecb_key, 128 + 64 * i ); mbedtls_aria_crypt_ecb( &ctx, aria_test1_ecb_pt, blk ); - if( memcmp( blk, aria_test1_ecb_ct[i], MBEDTLS_ARIA_BLOCKSIZE ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( + memcmp( blk, aria_test1_ecb_ct[i], MBEDTLS_ARIA_BLOCKSIZE ) + != 0 ); /* test ECB decryption */ if( verbose ) mbedtls_printf( " ARIA-ECB-%d (dec): ", 128 + 64 * i ); mbedtls_aria_setkey_dec( &ctx, aria_test1_ecb_key, 128 + 64 * i ); mbedtls_aria_crypt_ecb( &ctx, aria_test1_ecb_ct[i], blk ); - if( memcmp( blk, aria_test1_ecb_pt, MBEDTLS_ARIA_BLOCKSIZE ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( + memcmp( blk, aria_test1_ecb_pt, MBEDTLS_ARIA_BLOCKSIZE ) + != 0 ); } if( verbose ) mbedtls_printf( "\n" ); @@ -958,8 +962,8 @@ int mbedtls_aria_self_test( int verbose ) memset( buf, 0x55, sizeof( buf ) ); mbedtls_aria_crypt_cbc( &ctx, MBEDTLS_ARIA_ENCRYPT, 48, iv, aria_test2_pt, buf ); - if( memcmp( buf, aria_test2_cbc_ct[i], 48 ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_cbc_ct[i], 48 ) + != 0 ); /* Test CBC decryption */ if( verbose ) @@ -969,8 +973,7 @@ int mbedtls_aria_self_test( int verbose ) memset( buf, 0xAA, sizeof( buf ) ); mbedtls_aria_crypt_cbc( &ctx, MBEDTLS_ARIA_DECRYPT, 48, iv, aria_test2_cbc_ct[i], buf ); - if( memcmp( buf, aria_test2_pt, 48 ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_pt, 48 ) != 0 ); } if( verbose ) mbedtls_printf( "\n" ); @@ -989,8 +992,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_cfb128( &ctx, MBEDTLS_ARIA_ENCRYPT, 48, &j, iv, aria_test2_pt, buf ); - if( memcmp( buf, aria_test2_cfb_ct[i], 48 ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_cfb_ct[i], 48 ) != 0 ); /* Test CFB decryption */ if( verbose ) @@ -1001,8 +1003,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_cfb128( &ctx, MBEDTLS_ARIA_DECRYPT, 48, &j, iv, aria_test2_cfb_ct[i], buf ); - if( memcmp( buf, aria_test2_pt, 48 ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_pt, 48 ) != 0 ); } if( verbose ) mbedtls_printf( "\n" ); @@ -1020,8 +1021,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_ctr( &ctx, 48, &j, iv, blk, aria_test2_pt, buf ); - if( memcmp( buf, aria_test2_ctr_ct[i], 48 ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_ctr_ct[i], 48 ) != 0 ); /* Test CTR decryption */ if( verbose ) @@ -1032,8 +1032,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_ctr( &ctx, 48, &j, iv, blk, aria_test2_ctr_ct[i], buf ); - if( memcmp( buf, aria_test2_pt, 48 ) != 0 ) - ARIA_SELF_TEST_IF_FAIL; + ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_pt, 48 ) != 0 ); } if( verbose ) mbedtls_printf( "\n" ); From 0bb72434255db30660f9043015f4695648c7f14d Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 17:49:31 +0100 Subject: [PATCH 02/20] Refactor macro-spanning if in ssl_tls12_client.c Signed-off-by: David Horstmann --- library/ssl_cli.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 0267af7f6..46ef43f06 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3451,9 +3451,11 @@ start_processing: if( ( ret = mbedtls_pk_verify_restartable( peer_pk, md_alg, hash, hashlen, p, sig_len, rs_ctx ) ) != 0 ) { + int send_alert_msg = 1; #if defined(MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED) - if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ) + send_alert_msg = ( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ); #endif + if( send_alert_msg ) mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, From 5846c9de19ba4457107151cd7ec97825a4408ca1 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 18:31:25 +0100 Subject: [PATCH 03/20] Refactor macro-spanning if in ssl_msg.c Signed-off-by: David Horstmann --- library/ssl_msg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 7e2757018..e8f47dea4 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3944,8 +3944,8 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, if( ssl_record_is_in_progress( ssl ) == 0 ) { + int dtls_have_buffered = 0; #if defined(MBEDTLS_SSL_PROTO_DTLS) - int have_buffered = 0; /* We only check for buffered messages if the * current datagram is fully consumed. */ @@ -3953,11 +3953,11 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, ssl_next_record_is_in_datagram( ssl ) == 0 ) { if( ssl_load_buffered_message( ssl ) == 0 ) - have_buffered = 1; + dtls_have_buffered = 1; } - if( have_buffered == 0 ) #endif /* MBEDTLS_SSL_PROTO_DTLS */ + if( dtls_have_buffered == 0 ) { ret = ssl_get_next_record( ssl ); if( ret == MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ) From f3b1eaf95dde1f7b0e8fe18cf89fd15cd7bff122 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 18:45:09 +0100 Subject: [PATCH 04/20] Refactor macro-spanning if in sha512.c Signed-off-by: David Horstmann --- library/sha512.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/sha512.c b/library/sha512.c index 2a0cbe610..1a6872c8a 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -418,9 +418,11 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, sha512_put_uint64_be( ctx->state[4], output, 32 ); sha512_put_uint64_be( ctx->state[5], output, 40 ); + int truncated = 0; #if !defined(MBEDTLS_SHA512_NO_SHA384) - if( ctx->is384 == 0 ) + truncated = ctx->is384; #endif + if( !truncated ) { sha512_put_uint64_be( ctx->state[6], output, 48 ); sha512_put_uint64_be( ctx->state[7], output, 56 ); From 863b17d0cc7db3ebcd90a9aa1a9fde56d7c6ec7c Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 18:57:57 +0100 Subject: [PATCH 05/20] Refactor macro-spanning if in asn1write.c Signed-off-by: David Horstmann --- library/asn1write.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/library/asn1write.c b/library/asn1write.c index 6477e3d58..89a937b29 100644 --- a/library/asn1write.c +++ b/library/asn1write.c @@ -72,9 +72,11 @@ int mbedtls_asn1_write_len( unsigned char **p, unsigned char *start, size_t len return( 4 ); } + int len_valid = 1; #if SIZE_MAX > 0xFFFFFFFF - if( len <= 0xFFFFFFFF ) + len_valid = ( len <= 0xFFFFFFFF ); #endif + if( len_valid ) { if( *p - start < 5 ) return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); @@ -86,10 +88,10 @@ int mbedtls_asn1_write_len( unsigned char **p, unsigned char *start, size_t len *--(*p) = 0x84; return( 5 ); } - -#if SIZE_MAX > 0xFFFFFFFF - return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); -#endif + else + { + return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); + } } int mbedtls_asn1_write_tag( unsigned char **p, unsigned char *start, unsigned char tag ) From b95ee002441683e7ed64f173fe21d7e2231b9303 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 19:11:04 +0100 Subject: [PATCH 06/20] Refactor macro-spanning ifs in ecp.c Signed-off-by: David Horstmann --- library/ecp.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index dc7363817..48c2403d4 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2395,12 +2395,14 @@ cleanup: mbedtls_free( T ); } - /* don't free R while in progress in case R == P */ -#if defined(MBEDTLS_ECP_RESTARTABLE) - if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ) -#endif + int should_free_R = 0; /* prevent caller from using invalid value */ - if( ret != 0 ) + should_free_R = ( ret != 0 ); +#if defined(MBEDTLS_ECP_RESTARTABLE) + /* don't free R while in progress in case R == P */ + should_free_R = should_free_R && ( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ); +#endif + if( should_free_R ) mbedtls_ecp_point_free( R ); ECP_RS_LEAVE( rsm ); @@ -2672,10 +2674,12 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MBEDTLS_MPI_CHK( mbedtls_internal_ecp_init( grp ) ); #endif /* MBEDTLS_ECP_INTERNAL_ALT */ + int restarting = 0; #if defined(MBEDTLS_ECP_RESTARTABLE) - /* skip argument check when restarting */ - if( rs_ctx == NULL || rs_ctx->rsm == NULL ) + restarting = ( rs_ctx != NULL && rs_ctx->rsm != NULL ); #endif + /* skip argument check when restarting */ + if( !restarting ) { /* check_privkey is free */ MBEDTLS_ECP_BUDGET( MBEDTLS_ECP_OPS_CHK ); From d209197f37eb00c071609d1b8c326a2ff2dff026 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 6 Oct 2022 19:11:28 +0100 Subject: [PATCH 07/20] Refactor macro-spanning ifs in ecdh.c Signed-off-by: David Horstmann --- library/ecdh.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 60c6e429d..724c938a7 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -77,10 +77,12 @@ static int ecdh_gen_public_restartable( mbedtls_ecp_group *grp, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - /* If multiplication is in progress, we already generated a privkey */ + int restarting = 0; #if defined(MBEDTLS_ECP_RESTARTABLE) - if( rs_ctx == NULL || rs_ctx->rsm == NULL ) + restarting = ( rs_ctx != NULL && rs_ctx->rsm != NULL ); #endif + /* If multiplication is in progress, we already generated a privkey */ + if( !restarting ) MBEDTLS_MPI_CHK( mbedtls_ecp_gen_privkey( grp, d, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( mbedtls_ecp_mul_restartable( grp, Q, d, &grp->G, From 9e722ad97d92c31ae062d9385e3ee6fd27df0765 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 24 Oct 2022 13:11:38 +0100 Subject: [PATCH 08/20] Refactor macro-spanning if in ssl_client2.c Signed-off-by: David Horstmann --- programs/ssl/ssl_client2.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 4f076602a..30ecf68d9 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1573,15 +1573,17 @@ int main( int argc, char *argv[] ) if( ret != 0 ) break; } - if( ret == 0 ) #endif /* MBEDTLS_PEM_PARSE_C */ - for( i = 0; mbedtls_test_cas_der[i] != NULL; i++ ) + if( ret == 0 ) { - ret = mbedtls_x509_crt_parse_der( &cacert, - (const unsigned char *) mbedtls_test_cas_der[i], - mbedtls_test_cas_der_len[i] ); - if( ret != 0 ) - break; + for( i = 0; mbedtls_test_cas_der[i] != NULL; i++ ) + { + ret = mbedtls_x509_crt_parse_der( &cacert, + (const unsigned char *) mbedtls_test_cas_der[i], + mbedtls_test_cas_der_len[i] ); + if( ret != 0 ) + break; + } } } #else From 068a00baf1cba54677f45ef6b2dd013ad96f5d41 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 24 Oct 2022 13:12:19 +0100 Subject: [PATCH 09/20] Refactor macro-spanning if in ssl_server2.c Signed-off-by: David Horstmann --- programs/ssl/ssl_server2.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 48f50c568..283216b8b 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2377,15 +2377,17 @@ int main( int argc, char *argv[] ) if( ret != 0 ) break; } - if( ret == 0 ) #endif /* MBEDTLS_PEM_PARSE_C */ - for( i = 0; mbedtls_test_cas_der[i] != NULL; i++ ) + if( ret == 0 ) { - ret = mbedtls_x509_crt_parse_der( &cacert, - (const unsigned char *) mbedtls_test_cas_der[i], - mbedtls_test_cas_der_len[i] ); - if( ret != 0 ) - break; + for( i = 0; mbedtls_test_cas_der[i] != NULL; i++ ) + { + ret = mbedtls_x509_crt_parse_der( &cacert, + (const unsigned char *) mbedtls_test_cas_der[i], + mbedtls_test_cas_der_len[i] ); + if( ret != 0 ) + break; + } } } #else From 864cc8dba2b520df7e3620d7fc76139c3896c780 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 25 Oct 2022 10:16:45 +0100 Subject: [PATCH 10/20] Minor changes to asn1write.c Signed-off-by: David Horstmann --- library/asn1write.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/library/asn1write.c b/library/asn1write.c index 89a937b29..4b59927cb 100644 --- a/library/asn1write.c +++ b/library/asn1write.c @@ -72,11 +72,11 @@ int mbedtls_asn1_write_len( unsigned char **p, unsigned char *start, size_t len return( 4 ); } - int len_valid = 1; + int len_is_valid = 1; #if SIZE_MAX > 0xFFFFFFFF - len_valid = ( len <= 0xFFFFFFFF ); + len_is_valid = ( len <= 0xFFFFFFFF ); #endif - if( len_valid ) + if( len_is_valid ) { if( *p - start < 5 ) return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); @@ -88,10 +88,8 @@ int mbedtls_asn1_write_len( unsigned char **p, unsigned char *start, size_t len *--(*p) = 0x84; return( 5 ); } - else - { - return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); - } + + return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); } int mbedtls_asn1_write_tag( unsigned char **p, unsigned char *start, unsigned char tag ) From 9430330d2f238ce29da3258b5de4df9a795191b5 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 25 Oct 2022 10:23:34 +0100 Subject: [PATCH 11/20] Rename ARIA_SELF_TEST_IF_FAIL Change to ARIA_SELF_TEST_ASSERT Signed-off-by: David Horstmann --- library/aria.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/aria.c b/library/aria.c index 6c4f30d94..924f95283 100644 --- a/library/aria.c +++ b/library/aria.c @@ -888,7 +888,7 @@ static const uint8_t aria_test2_ctr_ct[3][48] = // CTR ciphertext }; #endif /* MBEDTLS_CIPHER_MODE_CFB */ -#define ARIA_SELF_TEST_IF_FAIL( cond ) \ +#define ARIA_SELF_TEST_ASSERT( cond ) \ do { \ if( cond ) { \ if( verbose ) \ @@ -932,7 +932,7 @@ int mbedtls_aria_self_test( int verbose ) mbedtls_printf( " ARIA-ECB-%d (enc): ", 128 + 64 * i ); mbedtls_aria_setkey_enc( &ctx, aria_test1_ecb_key, 128 + 64 * i ); mbedtls_aria_crypt_ecb( &ctx, aria_test1_ecb_pt, blk ); - ARIA_SELF_TEST_IF_FAIL( + ARIA_SELF_TEST_ASSERT( memcmp( blk, aria_test1_ecb_ct[i], MBEDTLS_ARIA_BLOCKSIZE ) != 0 ); @@ -941,7 +941,7 @@ int mbedtls_aria_self_test( int verbose ) mbedtls_printf( " ARIA-ECB-%d (dec): ", 128 + 64 * i ); mbedtls_aria_setkey_dec( &ctx, aria_test1_ecb_key, 128 + 64 * i ); mbedtls_aria_crypt_ecb( &ctx, aria_test1_ecb_ct[i], blk ); - ARIA_SELF_TEST_IF_FAIL( + ARIA_SELF_TEST_ASSERT( memcmp( blk, aria_test1_ecb_pt, MBEDTLS_ARIA_BLOCKSIZE ) != 0 ); } @@ -962,7 +962,7 @@ int mbedtls_aria_self_test( int verbose ) memset( buf, 0x55, sizeof( buf ) ); mbedtls_aria_crypt_cbc( &ctx, MBEDTLS_ARIA_ENCRYPT, 48, iv, aria_test2_pt, buf ); - ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_cbc_ct[i], 48 ) + ARIA_SELF_TEST_ASSERT( memcmp( buf, aria_test2_cbc_ct[i], 48 ) != 0 ); /* Test CBC decryption */ @@ -973,7 +973,7 @@ int mbedtls_aria_self_test( int verbose ) memset( buf, 0xAA, sizeof( buf ) ); mbedtls_aria_crypt_cbc( &ctx, MBEDTLS_ARIA_DECRYPT, 48, iv, aria_test2_cbc_ct[i], buf ); - ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_pt, 48 ) != 0 ); + ARIA_SELF_TEST_ASSERT( memcmp( buf, aria_test2_pt, 48 ) != 0 ); } if( verbose ) mbedtls_printf( "\n" ); @@ -992,7 +992,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_cfb128( &ctx, MBEDTLS_ARIA_ENCRYPT, 48, &j, iv, aria_test2_pt, buf ); - ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_cfb_ct[i], 48 ) != 0 ); + ARIA_SELF_TEST_ASSERT( memcmp( buf, aria_test2_cfb_ct[i], 48 ) != 0 ); /* Test CFB decryption */ if( verbose ) @@ -1003,7 +1003,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_cfb128( &ctx, MBEDTLS_ARIA_DECRYPT, 48, &j, iv, aria_test2_cfb_ct[i], buf ); - ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_pt, 48 ) != 0 ); + ARIA_SELF_TEST_ASSERT( memcmp( buf, aria_test2_pt, 48 ) != 0 ); } if( verbose ) mbedtls_printf( "\n" ); @@ -1021,7 +1021,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_ctr( &ctx, 48, &j, iv, blk, aria_test2_pt, buf ); - ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_ctr_ct[i], 48 ) != 0 ); + ARIA_SELF_TEST_ASSERT( memcmp( buf, aria_test2_ctr_ct[i], 48 ) != 0 ); /* Test CTR decryption */ if( verbose ) @@ -1032,7 +1032,7 @@ int mbedtls_aria_self_test( int verbose ) j = 0; mbedtls_aria_crypt_ctr( &ctx, 48, &j, iv, blk, aria_test2_ctr_ct[i], buf ); - ARIA_SELF_TEST_IF_FAIL( memcmp( buf, aria_test2_pt, 48 ) != 0 ); + ARIA_SELF_TEST_ASSERT( memcmp( buf, aria_test2_pt, 48 ) != 0 ); } if( verbose ) mbedtls_printf( "\n" ); From e9af9e3e120a4361f13212d8d89a7abf21f53ea7 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 25 Oct 2022 10:32:08 +0100 Subject: [PATCH 12/20] Minor improvements to ecp.c changes Signed-off-by: David Horstmann --- library/ecp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 48c2403d4..83df03762 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2395,12 +2395,12 @@ cleanup: mbedtls_free( T ); } - int should_free_R = 0; /* prevent caller from using invalid value */ - should_free_R = ( ret != 0 ); + int should_free_R = ( ret != 0 ); #if defined(MBEDTLS_ECP_RESTARTABLE) /* don't free R while in progress in case R == P */ - should_free_R = should_free_R && ( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ); + if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) + should_free_R = 0; #endif if( should_free_R ) mbedtls_ecp_point_free( R ); From ee0a0e75c8e0e1e0ac1cc14b4f3024642ede8d27 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 25 Oct 2022 17:20:00 +0100 Subject: [PATCH 13/20] Fix macro-spanning ifs in ssl_cli.c Signed-off-by: David Horstmann --- library/ssl_cli.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 46ef43f06..5df2758da 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -998,9 +998,12 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_NO_RNG ); } + int renegotiating = 0; #if defined(MBEDTLS_SSL_RENEGOTIATION) - if( ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE ) + if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE ) + renegotiating = 1; #endif + if( !renegotiating ) { ssl->major_ver = ssl->conf->min_major_ver; ssl->minor_ver = ssl->conf->min_minor_ver; @@ -1086,9 +1089,12 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) * RFC 5077 section 3.4: "When presenting a ticket, the client MAY * generate and include a Session ID in the TLS ClientHello." */ + renegotiating = 0; #if defined(MBEDTLS_SSL_RENEGOTIATION) - if( ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE ) + if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE ) + renegotiating = 1; #endif + if( !renegotiating ) { if( ssl->session_negotiate->ticket != NULL && ssl->session_negotiate->ticket_len != 0 ) @@ -1203,9 +1209,12 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) /* * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV */ + renegotiating = 0; #if defined(MBEDTLS_SSL_RENEGOTIATION) - if( ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE ) + if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE ) + renegotiating = 1; #endif + if( !renegotiating ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding EMPTY_RENEGOTIATION_INFO_SCSV" ) ); MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); @@ -2235,20 +2244,23 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) */ comp = buf[37 + n]; + int bad_comp = 0; #if defined(MBEDTLS_ZLIB_SUPPORT) /* See comments in ssl_write_client_hello() */ + accept_comp = 1; #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) accept_comp = 0; - else #endif - accept_comp = 1; if( comp != MBEDTLS_SSL_COMPRESS_NULL && ( comp != MBEDTLS_SSL_COMPRESS_DEFLATE || accept_comp == 0 ) ) + bad_comp = 1; #else /* MBEDTLS_ZLIB_SUPPORT */ if( comp != MBEDTLS_SSL_COMPRESS_NULL ) + bad_comp = 1; #endif/* MBEDTLS_ZLIB_SUPPORT */ + if( bad_comp ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "server hello, bad compression: %d", comp ) ); @@ -2692,12 +2704,16 @@ static int ssl_check_server_ecdh_params( const mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); + int bad_params = 0; #if defined(MBEDTLS_ECP_C) if( mbedtls_ssl_check_curve( ssl, grp_id ) != 0 ) + bad_params = 1; #else if( ssl->handshake->ecdh_ctx.grp.nbits < 163 || ssl->handshake->ecdh_ctx.grp.nbits > 521 ) + bad_params = 1; #endif + if( bad_params ) return( -1 ); MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, From 74ace59dc62907f1d4062d5ae1b1de1ba750d007 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 25 Oct 2022 17:26:58 +0100 Subject: [PATCH 14/20] Fix macro-spanning ifs in ssl_srv.c Signed-off-by: David Horstmann --- library/ssl_srv.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 2dda6bed0..0563c0b59 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1454,6 +1454,7 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); + int renegotiating = 0; #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) read_record_header: #endif @@ -1463,8 +1464,10 @@ read_record_header: * ClientHello, which doesn't use the same record layer format. */ #if defined(MBEDTLS_SSL_RENEGOTIATION) - if( ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE ) + if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE ) + renegotiating = 1; #endif + if( !renegotiating ) { if( ( ret = mbedtls_ssl_fetch_input( ssl, 5 ) ) != 0 ) { @@ -1477,9 +1480,12 @@ read_record_header: buf = ssl->in_hdr; #if defined(MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO) + int is_dtls = 0; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_STREAM ) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + is_dtls = 1; #endif + if( !is_dtls ) if( ( buf[0] & 0x80 ) != 0 ) return( ssl_parse_client_hello_v2( ssl ) ); #endif From ef661c531f13595fe4fc31b4dec78b6108b0d238 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 26 Oct 2022 17:55:53 +0100 Subject: [PATCH 15/20] Fix macro-spanning ifs in ecp.c Signed-off-by: David Horstmann --- library/ecp.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 83df03762..1ba2143a7 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2048,9 +2048,13 @@ static int ecp_mul_comb_core( const mbedtls_ecp_group *grp, mbedtls_ecp_point *R i = d; MBEDTLS_MPI_CHK( ecp_select_comb( grp, R, T, T_size, x[i] ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &R->Z, 1 ) ); + + int have_rng = 1; #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng != 0 ) + if( f_rng == 0 ) + have_rng = 0; #endif + if( have_rng ) MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, R, f_rng, p_rng ) ); } @@ -2184,9 +2188,12 @@ final_norm: * * Avoid the leak by randomizing coordinates before we normalize them. */ + int have_rng = 1; #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng != 0 ) + if( f_rng == 0 ) + have_rng = 0; #endif + if( have_rng ) MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, RR, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( ecp_normalize_jac( grp, RR ) ); @@ -2590,9 +2597,12 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MOD_ADD( RP.X ); /* Randomize coordinates of the starting point */ + int have_rng = 1; #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng != NULL ) + if( f_rng == NULL ) + have_rng = 0; #endif + if( have_rng ) MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, &RP, f_rng, p_rng ) ); /* Loop invariant: R = result so far, RP = R + P */ @@ -2625,9 +2635,12 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, * * Avoid the leak by randomizing coordinates before we normalize them. */ + have_rng = 1; #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng != NULL ) + if( f_rng == NULL ) + have_rng = 0; #endif + if( have_rng ) MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, R, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( ecp_normalize_mxz( grp, R ) ); From 197b240089d3d090537284548267654d0ea08706 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 26 Oct 2022 18:06:31 +0100 Subject: [PATCH 16/20] Fix macro-spanning if in ssl_msg.c Signed-off-by: David Horstmann --- library/ssl_msg.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index e8f47dea4..815af7b76 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -435,9 +435,12 @@ static void ssl_extract_add_data_from_record( unsigned char* add_data, unsigned char *cur = add_data; + int is_tls13 = 0; #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) - if( minor_ver != MBEDTLS_SSL_MINOR_VERSION_4 ) + if( minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + is_tls13 = 1; #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + if( !is_tls13 ) { ((void) minor_ver); memcpy( cur, rec->ctr, sizeof( rec->ctr ) ); From d4f22083ba25c821635cfe34abf2c1afe255b6e2 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 26 Oct 2022 18:25:14 +0100 Subject: [PATCH 17/20] Fix macro-spanning ifs in ssl_tls.c Signed-off-by: David Horstmann --- library/ssl_tls.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7a4d43791..bf023189c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -979,6 +979,7 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, #if defined(MBEDTLS_USE_PSA_CRYPTO) int psa_fallthrough; #endif /* MBEDTLS_USE_PSA_CRYPTO */ + int do_mbedtls_cipher_setup; unsigned char keyblk[256]; unsigned char *key1; unsigned char *key2; @@ -1357,6 +1358,7 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, } #endif + do_mbedtls_cipher_setup = 1; #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Only use PSA-based ciphers for TLS-1.2. @@ -1392,15 +1394,18 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, psa_fallthrough = 1; #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - if( psa_fallthrough == 1 ) + if( psa_fallthrough == 0 ) + do_mbedtls_cipher_setup = 0; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - if( ( ret = mbedtls_cipher_setup( &transform->cipher_ctx_enc, - cipher_info ) ) != 0 ) + if( do_mbedtls_cipher_setup && + ( ret = mbedtls_cipher_setup( &transform->cipher_ctx_enc, + cipher_info ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setup", ret ); goto end; } + do_mbedtls_cipher_setup = 1; #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Only use PSA-based ciphers for TLS-1.2. * That's relevant at least for TLS-1.0, where @@ -1435,10 +1440,12 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, psa_fallthrough = 1; #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - if( psa_fallthrough == 1 ) + if( psa_fallthrough == 0 ) + do_mbedtls_cipher_setup = 0; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - if( ( ret = mbedtls_cipher_setup( &transform->cipher_ctx_dec, - cipher_info ) ) != 0 ) + if( do_mbedtls_cipher_setup && + ( ret = mbedtls_cipher_setup( &transform->cipher_ctx_dec, + cipher_info ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setup", ret ); goto end; @@ -4083,9 +4090,12 @@ int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ) memset( ssl->out_buf, 0, out_buf_len ); + int clear_in_buf = 1; #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C) - if( partial == 0 ) + if( partial != 0 ) + clear_in_buf = 0; #endif /* MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE && MBEDTLS_SSL_SRV_C */ + if( clear_in_buf ) { ssl->in_left = 0; memset( ssl->in_buf, 0, in_buf_len ); @@ -4121,10 +4131,13 @@ int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ) ssl->alpn_chosen = NULL; #endif + int free_cli_id = 1; #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) - if( partial == 0 ) + if( partial != 0 ) + free_cli_id = 0; #endif + if( free_cli_id ) { mbedtls_free( ssl->cli_id ); ssl->cli_id = NULL; From 04020abfaefd1b28b872d0e370094bf90d5279b0 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 27 Oct 2022 11:30:09 +0100 Subject: [PATCH 18/20] Fix macro-spanning ifs in ssl_ticket.c Signed-off-by: David Horstmann --- library/ssl_ticket.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 3bc95709c..e6abe850d 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -146,7 +146,10 @@ int mbedtls_ssl_ticket_setup( mbedtls_ssl_ticket_context *ctx, if( cipher_info->key_bitlen > 8 * MAX_KEY_BYTES ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + int do_mbedtls_cipher_setup = 1; #if defined(MBEDTLS_USE_PSA_CRYPTO) + do_mbedtls_cipher_setup = 0; + ret = mbedtls_cipher_setup_psa( &ctx->keys[0].ctx, cipher_info, TICKET_AUTH_TAG_BYTES ); if( ret != 0 && ret != MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) @@ -154,19 +157,28 @@ int mbedtls_ssl_ticket_setup( mbedtls_ssl_ticket_context *ctx, /* We don't yet expect to support all ciphers through PSA, * so allow fallback to ordinary mbedtls_cipher_setup(). */ if( ret == MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) + do_mbedtls_cipher_setup = 1; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - if( ( ret = mbedtls_cipher_setup( &ctx->keys[0].ctx, cipher_info ) ) != 0 ) - return( ret ); + if( do_mbedtls_cipher_setup ) + if( ( ret = mbedtls_cipher_setup( &ctx->keys[0].ctx, cipher_info ) ) + != 0 ) + return( ret ); + do_mbedtls_cipher_setup = 1; #if defined(MBEDTLS_USE_PSA_CRYPTO) + do_mbedtls_cipher_setup = 0; + ret = mbedtls_cipher_setup_psa( &ctx->keys[1].ctx, cipher_info, TICKET_AUTH_TAG_BYTES ); if( ret != 0 && ret != MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) return( ret ); if( ret == MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) + do_mbedtls_cipher_setup = 1; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - if( ( ret = mbedtls_cipher_setup( &ctx->keys[1].ctx, cipher_info ) ) != 0 ) - return( ret ); + if( do_mbedtls_cipher_setup ) + if( ( ret = mbedtls_cipher_setup( &ctx->keys[1].ctx, cipher_info ) ) + != 0 ) + return( ret ); if( ( ret = ssl_ticket_gen_key( ctx, 0 ) ) != 0 || ( ret = ssl_ticket_gen_key( ctx, 1 ) ) != 0 ) From ab6175130b67e050725b918d55c0493a9da6663a Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 27 Oct 2022 11:45:01 +0100 Subject: [PATCH 19/20] Fix macro-spanning if in x509_crt.c Signed-off-by: David Horstmann --- library/x509_crt.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index d97c867c0..c7ee5c181 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1258,9 +1258,12 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, } } + int extensions_allowed = 1; #if !defined(MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3) - if( crt->version == 3 ) + if( crt->version != 3 ) + extensions_allowed = 0; #endif + if( extensions_allowed ) { ret = x509_get_crt_ext( &p, end, crt, cb, p_ctx ); if( ret != 0 ) From b5b1ed2969233c77c8dee46cd190d7758e28931e Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 27 Oct 2022 13:21:49 +0100 Subject: [PATCH 20/20] Fix unused warning in ssl_tls.c Signed-off-by: David Horstmann --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bf023189c..d56bc5bc2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4131,8 +4131,8 @@ int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ) ssl->alpn_chosen = NULL; #endif - int free_cli_id = 1; #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) + int free_cli_id = 1; #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) if( partial != 0 ) free_cli_id = 0;