From a02003babe4e5145d37f47f42a2565b33e8534ea Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 7 Jul 2021 17:20:06 +0100 Subject: [PATCH 1/5] Fix divide by zero if macro used with wrong key If PSA_CIPHER_ENCRYPT_OUTPUT_SIZE was called on a non symmetric key, then a divide by zero could happen, as PSA_CIPHER_BLOCK_LENGTH will return 0 for such a key, and PSA_ROUND_UP_TO_MULTIPLE will divide by the block length. Signed-off-by: Paul Elliott --- include/psa/crypto_sizes.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index 2811c237af..836f9ee147 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -992,9 +992,11 @@ */ #define PSA_CIPHER_ENCRYPT_OUTPUT_SIZE(key_type, alg, input_length) \ (alg == PSA_ALG_CBC_PKCS7 ? \ + (((key_type) & PSA_KEY_TYPE_CATEGORY_MASK) \ + == PSA_KEY_TYPE_CATEGORY_SYMMETRIC ? \ PSA_ROUND_UP_TO_MULTIPLE(PSA_BLOCK_CIPHER_BLOCK_LENGTH(key_type), \ (input_length) + 1) + \ - PSA_CIPHER_IV_LENGTH((key_type), (alg)) : \ + PSA_CIPHER_IV_LENGTH((key_type), (alg)) : 0) : \ (PSA_ALG_IS_CIPHER(alg) ? \ (input_length) + PSA_CIPHER_IV_LENGTH((key_type), (alg)) : \ 0)) From c22950c9d057be71938f7509280e72ce19c57cee Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 8 Jul 2021 16:50:01 +0100 Subject: [PATCH 2/5] Change PSA Cipher macro safety to use block length Although checking if the key was symmetric was correct, its easier to read if we just check the block length is not zero before we use it in a division. Signed-off-by: Paul Elliott --- include/psa/crypto_sizes.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index 836f9ee147..1f8b6c305c 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -992,8 +992,7 @@ */ #define PSA_CIPHER_ENCRYPT_OUTPUT_SIZE(key_type, alg, input_length) \ (alg == PSA_ALG_CBC_PKCS7 ? \ - (((key_type) & PSA_KEY_TYPE_CATEGORY_MASK) \ - == PSA_KEY_TYPE_CATEGORY_SYMMETRIC ? \ + (PSA_BLOCK_CIPHER_BLOCK_LENGTH(key_type) != 0 ? \ PSA_ROUND_UP_TO_MULTIPLE(PSA_BLOCK_CIPHER_BLOCK_LENGTH(key_type), \ (input_length) + 1) + \ PSA_CIPHER_IV_LENGTH((key_type), (alg)) : 0) : \ From 6603e2b81cb789aa1018acae78714ce49f189010 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 8 Jul 2021 16:53:42 +0100 Subject: [PATCH 3/5] Add fix to update output size macro as well. Same issue with zero block length applies here. Signed-off-by: Paul Elliott --- include/psa/crypto_sizes.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index 1f8b6c305c..4c67f10afa 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -1074,12 +1074,13 @@ */ #define PSA_CIPHER_UPDATE_OUTPUT_SIZE(key_type, alg, input_length) \ (PSA_ALG_IS_CIPHER(alg) ? \ + (PSA_BLOCK_CIPHER_BLOCK_LENGTH(key_type) != 0 ? \ (((alg) == PSA_ALG_CBC_PKCS7 || \ (alg) == PSA_ALG_CBC_NO_PADDING || \ (alg) == PSA_ALG_ECB_NO_PADDING) ? \ PSA_ROUND_UP_TO_MULTIPLE(PSA_BLOCK_CIPHER_BLOCK_LENGTH(key_type), \ input_length) : \ - (input_length)) : \ + (input_length)) : 0) : \ 0) /** A sufficient output buffer size for psa_cipher_update(), for any of the From a417f56d28ec1a74276b6053c8aa53cb30a54fe1 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 14 Jul 2021 12:31:21 +0100 Subject: [PATCH 4/5] Add non regression test for cipher output size Call the output size macros specifically with asymmetric keys, which would cause a crash (and thus test fail) should this fix get regressed. Signed-off-by: Paul Elliott --- tests/suites/test_suite_psa_crypto.data | 12 +++++++ tests/suites/test_suite_psa_crypto.function | 40 +++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 8671e3718d..56295ee5b3 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1466,6 +1466,18 @@ cipher_setup:PSA_KEY_TYPE_CHACHA20:"000102030405060708090a0b0c0d0e0f101112131415 PSA cipher: bad order function calls cipher_bad_order: +PSA cipher: incorrect key type (HMAC) +depends_on:PSA_WANT_ALG_HMAC:PSA_WANT_ALG_SHA_256 +cipher_bad_key:PSA_ALG_HMAC(PSA_ALG_SHA_256):PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f" + +PSA cipher: incorrect key type (RSA) +depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR +cipher_bad_key:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24" + +PSA cipher: incorrect key type (ECC Family Sep R1) +depends_on:PSA_WANT_ALG_ECDSA:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:PSA_WANT_ECC_SECP_R1_256 +cipher_bad_key:PSA_ALG_ECDSA_ANY:PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_FAMILY_SECP_R1):"04dea5e45d0ea37fc566232a508f4ad20ea13d47e4bf5fa4d54a57a0ba012042087097496efc583fed8b24a5b9be9a51de063f5a00a8b698a16fd7f29b5485f320" + PSA cipher encrypt: without initialization depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_AES cipher_encrypt_fail:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"":"":PSA_ERROR_BAD_STATE diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 9136cb91da..150a3f43e3 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -2559,6 +2559,46 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void cipher_bad_key( int alg_arg, int key_type_arg, data_t *key_data ) +{ + mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; + psa_algorithm_t alg = alg_arg; + psa_key_type_t key_type = key_type_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; + psa_status_t status; + + PSA_ASSERT( psa_crypto_init( ) ); + + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_ENCRYPT ); + psa_set_key_algorithm( &attributes, alg ); + psa_set_key_type( &attributes, key_type ); + + /* Usage of either of these two size macros would cause divide by zero + * with incorrect key types previously. Input length should be irrelevant + * here. */ + TEST_EQUAL( PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, 16 ), + 0 ); + TEST_EQUAL( PSA_CIPHER_UPDATE_OUTPUT_SIZE( key_type, alg, 16 ), 0 ); + + + PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len, + &key ) ); + + /* Should fail due to invalid alg type (to support invalid key type). + * Encrypt or decrypt will end up in the same place. */ + status = psa_cipher_encrypt_setup( &operation, key, alg ); + + TEST_EQUAL( status, PSA_ERROR_INVALID_ARGUMENT ); + +exit: + psa_cipher_abort( &operation ); + psa_destroy_key( key ); + PSA_DONE( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void cipher_encrypt_validation( int alg_arg, int key_type_arg, From 64df5f88c5d4d177bae047c56d3cdbd49974bfdd Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 14 Jul 2021 12:37:00 +0100 Subject: [PATCH 5/5] Add Changelog entry Signed-off-by: Paul Elliott --- ChangeLog.d/fix-cipher-output-size-macros.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/fix-cipher-output-size-macros.txt diff --git a/ChangeLog.d/fix-cipher-output-size-macros.txt b/ChangeLog.d/fix-cipher-output-size-macros.txt new file mode 100644 index 0000000000..4a4b971c83 --- /dev/null +++ b/ChangeLog.d/fix-cipher-output-size-macros.txt @@ -0,0 +1,4 @@ +Bugfix + * Prevent divide by zero if either of PSA_CIPHER_ENCRYPT_OUTPUT_SIZE() or + PSA_CIPHER_UPDATE_OUTPUT_SIZE() were called using an asymmetric key type. +