Merge pull request #6491 from davidhorstmann-arm/2.28-fix-unusual-mac…

…ros-0

[Backport-ish 2.28] Fix unusual macros

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_ASSERT( 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_ASSERT(
+                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_ASSERT(
+                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_ASSERT( 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_ASSERT( 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_ASSERT( 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_ASSERT( 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_ASSERT( 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_ASSERT( memcmp( buf, aria_test2_pt, 48 ) != 0 );
     }
     if( verbose )
         mbedtls_printf( "\n" );

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_is_valid = 1;
 #if SIZE_MAX > 0xFFFFFFFF
-    if( len <= 0xFFFFFFFF )
+    len_is_valid = ( len <= 0xFFFFFFFF );
 #endif
+    if( len_is_valid )
     {
         if( *p - start < 5 )
             return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL );
@@ -87,9 +89,7 @@ int mbedtls_asn1_write_len( unsigned char **p, unsigned char *start, size_t len
         return( 5 );
     }
 
-#if SIZE_MAX > 0xFFFFFFFF
     return( MBEDTLS_ERR_ASN1_INVALID_LENGTH );
-#endif
 }
 
 int mbedtls_asn1_write_tag( unsigned char **p, unsigned char *start, unsigned char tag )

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,

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 @@ static int ecp_mul_comb_after_precomp( const mbedtls_ecp_group *grp,
      *
      * 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 ) );
@@ -2395,12 +2402,14 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
         mbedtls_free( T );
     }
 
-    /* don't free R while in progress in case R == P */
+    /* prevent caller from using invalid value */
+    int should_free_R = ( ret != 0 );
 #if defined(MBEDTLS_ECP_RESTARTABLE)
-    if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS )
+    /* don't free R while in progress in case R == P */
+    if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS )
+        should_free_R = 0;
 #endif
-    /* prevent caller from using invalid value */
-    if( ret != 0 )
+    if( should_free_R )
         mbedtls_ecp_point_free( R );
 
     ECP_RS_LEAVE( rsm );
@@ -2588,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 */
@@ -2623,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 ) );
@@ -2672,10 +2687,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 );

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 );

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,
@@ -3451,9 +3467,11 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl )
         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,

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 ) );
@@ -3944,20 +3947,20 @@ 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. */
                 if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
                     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 )