Merge pull request #5072 from mprse/issue_5065

Use switch statement instead if-else in psa_aead_check_nonce_length() and psa_aead_set_lengths(). Fixes #5065
Mbed-TLS · Oct 18, 2021 · bf21c07 · bf21c07
2 parents 7637ab0 + 316c4fa
commit bf21c07
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 48 deletions.
diff --git a/ChangeLog.d/issue5065.txt b/ChangeLog.d/issue5065.txt
@@ -0,0 +1,3 @@
+Bugfix
+   * Fix compile-time or run-time errors in PSA
+     AEAD functions when ChachaPoly is disabled. Fixes #5065.
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
@@ -3621,34 +3621,35 @@ static psa_status_t psa_aead_check_nonce_length( psa_algorithm_t alg,
 {
     psa_algorithm_t base_alg = psa_aead_get_base_algorithm( alg );
 
+    switch(base_alg)
+    {
 #if defined(PSA_WANT_ALG_GCM)
-    if( base_alg == PSA_ALG_GCM )
-    {
-        /* Not checking max nonce size here as GCM spec allows almost
-         * arbitrarily large nonces. Please note that we do not generally
-         * recommend the usage of nonces of greater length than
-         * PSA_AEAD_NONCE_MAX_SIZE, as large nonces are hashed to a shorter
-         * size, which can then lead to collisions if you encrypt a very
-         * large number of messages.*/
-        if( nonce_length != 0 )
-            return( PSA_SUCCESS );
-    }
+        case PSA_ALG_GCM:
+            /* Not checking max nonce size here as GCM spec allows almost
+            * arbitrarily large nonces. Please note that we do not generally
+            * recommend the usage of nonces of greater length than
+            * PSA_AEAD_NONCE_MAX_SIZE, as large nonces are hashed to a shorter
+            * size, which can then lead to collisions if you encrypt a very
+            * large number of messages.*/
+            if( nonce_length != 0 )
+                return( PSA_SUCCESS );
+            break;
 #endif /* PSA_WANT_ALG_GCM */
 #if defined(PSA_WANT_ALG_CCM)
-    if( base_alg == PSA_ALG_CCM )
-    {
-        if( nonce_length >= 7 && nonce_length <= 13 )
-            return( PSA_SUCCESS );
-    }
-    else
+        case PSA_ALG_CCM:
+            if( nonce_length >= 7 && nonce_length <= 13 )
+                return( PSA_SUCCESS );
+            break;
 #endif /* PSA_WANT_ALG_CCM */
 #if defined(PSA_WANT_ALG_CHACHA20_POLY1305)
-    if( base_alg == PSA_ALG_CHACHA20_POLY1305 )
-    {
-        if( nonce_length == 12 )
-            return( PSA_SUCCESS );
-    }
+        case PSA_ALG_CHACHA20_POLY1305:
+            if( nonce_length == 12 )
+                return( PSA_SUCCESS );
+            break;
 #endif /* PSA_WANT_ALG_CHACHA20_POLY1305 */
+        default:
+            break;
+    }
 
     return( PSA_ERROR_NOT_SUPPORTED );
 }
@@ -3950,40 +3951,40 @@ psa_status_t psa_aead_set_lengths( psa_aead_operation_t *operation,
         goto exit;
     }
 
-#if defined(PSA_WANT_ALG_GCM)
-    if( operation->alg == PSA_ALG_GCM )
+    switch(operation->alg)
     {
-        /* Lengths can only be too large for GCM if size_t is bigger than 32
-         * bits. Without the guard this code will generate warnings on 32bit
-         * builds. */
+#if defined(PSA_WANT_ALG_GCM)
+        case PSA_ALG_GCM:
+            /* Lengths can only be too large for GCM if size_t is bigger than 32
+            * bits. Without the guard this code will generate warnings on 32bit
+            * builds. */
 #if SIZE_MAX > UINT32_MAX
-        if( (( uint64_t ) ad_length ) >> 61 != 0 ||
-            (( uint64_t ) plaintext_length ) > 0xFFFFFFFE0ull )
-        {
-            status = PSA_ERROR_INVALID_ARGUMENT;
-            goto exit;
-        }
+            if( (( uint64_t ) ad_length ) >> 61 != 0 ||
+                (( uint64_t ) plaintext_length ) > 0xFFFFFFFE0ull )
+            {
+                status = PSA_ERROR_INVALID_ARGUMENT;
+                goto exit;
+            }
 #endif
-    }
-    else
+            break;
 #endif /* PSA_WANT_ALG_GCM */
 #if defined(PSA_WANT_ALG_CCM)
-    if( operation->alg == PSA_ALG_CCM )
-    {
-        if( ad_length > 0xFF00 )
-        {
-            status = PSA_ERROR_INVALID_ARGUMENT;
-            goto exit;
-        }
-    }
-    else
+        case PSA_ALG_CCM:
+            if( ad_length > 0xFF00 )
+            {
+                status = PSA_ERROR_INVALID_ARGUMENT;
+                goto exit;
+            }
+            break;
 #endif /* PSA_WANT_ALG_CCM */
 #if defined(PSA_WANT_ALG_CHACHA20_POLY1305)
-    if( operation->alg == PSA_ALG_CHACHA20_POLY1305 )
-    {
-        /* No length restrictions for ChaChaPoly. */
-    }
+        case PSA_ALG_CHACHA20_POLY1305:
+            /* No length restrictions for ChaChaPoly. */
+            break;
 #endif /* PSA_WANT_ALG_CHACHA20_POLY1305 */
+        default:
+            break;
+    }
 
     status = psa_driver_wrapper_aead_set_lengths( operation, ad_length,
                                                   plaintext_length );

diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
@@ -1596,6 +1596,19 @@ component_test_psa_crypto_config_no_driver() {
     make test
 }
 
+component_test_psa_crypto_config_chachapoly_disabled() {
+    # full minus MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305
+    msg "build: full minus MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305"
+    scripts/config.py full
+    scripts/config.py unset MBEDTLS_CHACHAPOLY_C
+    scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_GCM
+    scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_CHACHA20_POLY1305
+    make CC=gcc CFLAGS="$ASAN_CFLAGS -O2" LDFLAGS="$ASAN_CFLAGS"
+
+    msg "test: full minus MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305"
+    make test
+}
+
 # This should be renamed to test and updated once the accelerator ECDSA code is in place and ready to test.
 component_build_psa_accel_alg_ecdsa() {
     # full plus MBEDTLS_PSA_CRYPTO_CONFIG with PSA_WANT_ALG_ECDSA