From 1ecfdea002aee1ba9e4ef805a8ddd194e92fe58f Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Wed, 13 Oct 2021 11:09:44 +0200 Subject: [PATCH 1/4] all.sh: add full - MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305 Signed-off-by: Przemyslaw Stekiel --- tests/scripts/all.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1bcc2e4a5..ed5faf5db 100755 --- 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 - MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305 + msg "build: full - 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 - 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 From 4cad4fc8a94e1a42bae3f705a0d59f1022c1ea41 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Wed, 13 Oct 2021 11:12:08 +0200 Subject: [PATCH 2/4] psa_crypto.c: use switch instead if-else in psa_aead_check_nonce_length and psa_aead_set_lengths (fixes #5065) Signed-off-by: Przemyslaw Stekiel --- library/psa_crypto.c | 97 ++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ece64b100..2299da3a5 100644 --- 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 ); -#if defined(PSA_WANT_ALG_GCM) - if( base_alg == PSA_ALG_GCM ) + switch(base_alg) { - /* 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 ); - } +#if defined(PSA_WANT_ALG_GCM) + 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 ); From ed61c5e8b000898f99a8f02995ac4bef605700a2 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Wed, 13 Oct 2021 15:24:25 +0200 Subject: [PATCH 3/4] Add change-log file (issue #5065) Signed-off-by: Przemyslaw Stekiel --- ChangeLog.d/issue5065.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/issue5065.txt diff --git a/ChangeLog.d/issue5065.txt b/ChangeLog.d/issue5065.txt new file mode 100644 index 000000000..f468c63ff --- /dev/null +++ b/ChangeLog.d/issue5065.txt @@ -0,0 +1,4 @@ +Bugfix + * Use switch statement instead if-else in + psa_aead_check_nonce_length() + and psa_aead_set_lengths(). Fixes #5065. From 316c4fa3ce1e3bb7d615d268e26ea4ccc44b6d96 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Fri, 15 Oct 2021 08:04:53 +0200 Subject: [PATCH 4/4] Address review comments Signed-off-by: Przemyslaw Stekiel --- ChangeLog.d/issue5065.txt | 5 ++--- tests/scripts/all.sh | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ChangeLog.d/issue5065.txt b/ChangeLog.d/issue5065.txt index f468c63ff..943ee47d9 100644 --- a/ChangeLog.d/issue5065.txt +++ b/ChangeLog.d/issue5065.txt @@ -1,4 +1,3 @@ Bugfix - * Use switch statement instead if-else in - psa_aead_check_nonce_length() - and psa_aead_set_lengths(). Fixes #5065. + * Fix compile-time or run-time errors in PSA + AEAD functions when ChachaPoly is disabled. Fixes #5065. diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index ed5faf5db..28387f443 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1597,15 +1597,15 @@ component_test_psa_crypto_config_no_driver() { } component_test_psa_crypto_config_chachapoly_disabled() { - # full - MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305 - msg "build: full - MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305" + # 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 - MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305" + msg "test: full minus MBEDTLS_CHACHAPOLY_C without PSA_WANT_ALG_GCM and PSA_WANT_ALG_CHACHA20_POLY1305" make test }