From 7fe62cc88b5864de6f9f6d17eac5a53888f5f448 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 7 Jul 2021 18:34:48 +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 79f96739b..b42eb45a3 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -998,9 +998,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 9bc9659cfbd78d9b842749a942bed22d693cc3fa 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 b42eb45a3..098e4f141 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -998,8 +998,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 c183f2005653a25111c047d7f7b56439d56393ea 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 098e4f141..f51ec87f9 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -1080,12 +1080,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 ed33ef1965b8f02671422c882858be86d528b76f 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 cf524f64a..fb70707b2 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1482,6 +1482,18 @@ cipher_setup:PSA_KEY_TYPE_ARC4:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CTR:PS 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 82f17c8d9..c2c94c10e 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -2620,6 +2620,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 7ac412b45c49a5dfa114b4c0a29eb0f954ce7322 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 000000000..4a4b971c8 --- /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. +