From 72b391fe074b6c2b008dc41e7076cfb16fe63547 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:35:28 +0000 Subject: [PATCH 1/6] Fix psa_key_derivation_input_integer() not detecting bad state Signed-off-by: Waleed Elmelegy --- .../fix-key-derive-bad-state-error.txt | 3 +++ library/psa_crypto.c | 13 +++++++++++++ tests/suites/test_suite_psa_crypto.data | 8 ++++---- tests/suites/test_suite_psa_crypto.function | 19 +++++++++++++++++++ 4 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 ChangeLog.d/fix-key-derive-bad-state-error.txt diff --git a/ChangeLog.d/fix-key-derive-bad-state-error.txt b/ChangeLog.d/fix-key-derive-bad-state-error.txt new file mode 100644 index 0000000000..0bccf77682 --- /dev/null +++ b/ChangeLog.d/fix-key-derive-bad-state-error.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix issue where psa_key_derivation_input_integer() is not detecting + bad state after an operation has been aborted. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b576f95789..63c6ad46e7 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7488,6 +7488,12 @@ static psa_status_t psa_key_derivation_input_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); + if (kdf_alg == 0) { + /* This is a blank or aborted operation. */ + status = PSA_ERROR_BAD_STATE; + goto exit; + } + status = psa_key_derivation_check_input_type(step, key_type); if (status != PSA_SUCCESS) { goto exit; @@ -7546,6 +7552,12 @@ static psa_status_t psa_key_derivation_input_integer_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); + if (kdf_alg == 0) { + /* This is a blank or aborted operation. */ + status = PSA_ERROR_BAD_STATE; + goto exit; + } + #if defined(PSA_HAVE_SOFT_PBKDF2) if (PSA_ALG_IS_PBKDF2(kdf_alg)) { status = psa_pbkdf2_set_input_cost( @@ -7559,6 +7571,7 @@ static psa_status_t psa_key_derivation_input_integer_internal( status = PSA_ERROR_INVALID_ARGUMENT; } +exit: if (status != PSA_SUCCESS) { psa_key_derivation_abort(operation); } diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 8c53fdc0a6..6bafc589e6 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -5567,11 +5567,11 @@ derive_input:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_COST: PSA key derivation: PBKDF2-HMAC-SHA256, salt and password before cost depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256 -derive_input:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_INVALID_ARGUMENT:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +derive_input:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE PSA key derivation: PBKDF2-HMAC-SHA256, password before cost depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256 -derive_input:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_INVALID_ARGUMENT:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +derive_input:PSA_ALG_PBKDF2_HMAC(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE PSA key derivation: PBKDF2-HMAC-SHA256, password bad key type depends_on:PSA_WANT_ALG_PBKDF2_HMAC:PSA_WANT_ALG_SHA_256 @@ -5643,11 +5643,11 @@ derive_input:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_KEY_DERIVATION_INPUT_COST:INPUT PSA key derivation: PBKDF2-AES-CMAC-PRF-128, salt and password before cost depends_on:PSA_WANT_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_WANT_ALG_CMAC:PSA_WANT_KEY_TYPE_AES -derive_input:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_INVALID_ARGUMENT:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +derive_input:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE PSA key derivation: PBKDF2-AES-CMAC-PRF-128, password before cost depends_on:PSA_WANT_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_WANT_ALG_CMAC:PSA_WANT_KEY_TYPE_AES -derive_input:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_INVALID_ARGUMENT:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +derive_input:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_PASSWORD:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE PSA key derivation: PBKDF2-AES-CMAC-PRF-128, password bad key type depends_on:PSA_WANT_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_WANT_ALG_CMAC:PSA_WANT_KEY_TYPE_AES diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 94bf28be4d..951686730c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -8893,6 +8893,25 @@ void derive_input(int alg_arg, } TEST_EQUAL(actual_output_status, expected_output_status); + /* Test calling input functions after operation has been aborted + result in PSA_ERROR_BAD_STATE error. + */ + psa_key_derivation_abort(&operation); + for (i = 0; i < ARRAY_LENGTH(steps); i++) { + if (key_types[i] == INPUT_INTEGER) { + TEST_EQUAL(psa_key_derivation_input_integer( + &operation, steps[i], + mbedtls_test_parse_binary_string(inputs[i])), + PSA_ERROR_BAD_STATE); + break; + } + } + + TEST_EQUAL(psa_key_derivation_input_bytes( + &operation, steps[0], + inputs[0]->x, inputs[0]->len), + PSA_ERROR_BAD_STATE); + exit: psa_key_derivation_abort(&operation); for (i = 0; i < ARRAY_LENGTH(keys); i++) { From b6ed6f72cdb9acccdef391a2bf004ab228831669 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:42:55 +0000 Subject: [PATCH 2/6] Simplify testing psa_key_derivation_input_*() bad state Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_psa_crypto.data | 4 ++++ tests/suites/test_suite_psa_crypto.function | 23 +++------------------ 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 6bafc589e6..5761e0c25b 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -5697,6 +5697,10 @@ PSA key derivation: PBKDF2-AES-CMAC-PRF-128, reject cost greater than PSA_VENDOR depends_on:PSA_WANT_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_WANT_ALG_CMAC:PSA_WANT_KEY_TYPE_AES derive_input_invalid_cost:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_VENDOR_PBKDF2_MAX_ITERATIONS+1ULL +PSA key derivation: reject calling input functions without calling setup +depends_on:PSA_WANT_ALG_SHA_256 +derive_input:0:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_NONE:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE + PSA key derivation over capacity: HKDF depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 derive_over_capacity:PSA_ALG_HKDF(PSA_ALG_SHA_256) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 951686730c..531e82b3df 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -8839,7 +8839,9 @@ void derive_input(int alg_arg, psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_DERIVE); psa_set_key_algorithm(&attributes, alg); - PSA_ASSERT(psa_key_derivation_setup(&operation, alg)); + if (alg != 0) { + PSA_ASSERT(psa_key_derivation_setup(&operation, alg)); + } for (i = 0; i < ARRAY_LENGTH(steps); i++) { mbedtls_test_set_step(i); @@ -8893,25 +8895,6 @@ void derive_input(int alg_arg, } TEST_EQUAL(actual_output_status, expected_output_status); - /* Test calling input functions after operation has been aborted - result in PSA_ERROR_BAD_STATE error. - */ - psa_key_derivation_abort(&operation); - for (i = 0; i < ARRAY_LENGTH(steps); i++) { - if (key_types[i] == INPUT_INTEGER) { - TEST_EQUAL(psa_key_derivation_input_integer( - &operation, steps[i], - mbedtls_test_parse_binary_string(inputs[i])), - PSA_ERROR_BAD_STATE); - break; - } - } - - TEST_EQUAL(psa_key_derivation_input_bytes( - &operation, steps[0], - inputs[0]->x, inputs[0]->len), - PSA_ERROR_BAD_STATE); - exit: psa_key_derivation_abort(&operation); for (i = 0; i < ARRAY_LENGTH(keys); i++) { From 07e573911578e12fabf3d63e832a7ee82516b29b Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:48:16 +0000 Subject: [PATCH 3/6] Replace zero by PSA_ALG_NONE in key derivation testing Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_psa_crypto.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 5761e0c25b..038d912dc0 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -5699,7 +5699,7 @@ derive_input_invalid_cost:PSA_ALG_PBKDF2_AES_CMAC_PRF_128:PSA_VENDOR_PBKDF2_MAX_ PSA key derivation: reject calling input functions without calling setup depends_on:PSA_WANT_ALG_SHA_256 -derive_input:0:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_NONE:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +derive_input:PSA_ALG_NONE:PSA_KEY_DERIVATION_INPUT_COST:INPUT_INTEGER:"01":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"73616c74":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_PASSWORD:PSA_KEY_TYPE_NONE:"706173737764":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE PSA key derivation over capacity: HKDF depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 From cba05ece2bbd429c7756050aa4b6d381b772747c Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:48:40 +0000 Subject: [PATCH 4/6] Replace zero by PSA_ALG_NONE in key derivation test function Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_psa_crypto.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 531e82b3df..da6c1c454c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -8839,7 +8839,7 @@ void derive_input(int alg_arg, psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_DERIVE); psa_set_key_algorithm(&attributes, alg); - if (alg != 0) { + if (alg != PSA_ALG_NONE) { PSA_ASSERT(psa_key_derivation_setup(&operation, alg)); } From 82cd324fd4ae9349ceac9e9ae1dd49eb87d8e6cb Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 15:04:17 +0000 Subject: [PATCH 5/6] Fix code style for key derivation input function Signed-off-by: Waleed Elmelegy --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 63c6ad46e7..57533cc492 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7489,7 +7489,7 @@ static psa_status_t psa_key_derivation_input_internal( psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); if (kdf_alg == 0) { - /* This is a blank or aborted operation. */ + /* This is a blank or aborted operation. */ status = PSA_ERROR_BAD_STATE; goto exit; } From 443908bc5df6622c76a0cf4c9af36d99c840773f Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 10 Mar 2025 14:19:01 +0000 Subject: [PATCH 6/6] Replace zero by PSA_ALG_NONE in key derivation input functions Signed-off-by: Waleed Elmelegy --- library/psa_crypto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57533cc492..3ec92cc061 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7488,7 +7488,7 @@ static psa_status_t psa_key_derivation_input_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); - if (kdf_alg == 0) { + if (kdf_alg == PSA_ALG_NONE) { /* This is a blank or aborted operation. */ status = PSA_ERROR_BAD_STATE; goto exit; @@ -7552,7 +7552,7 @@ static psa_status_t psa_key_derivation_input_integer_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); - if (kdf_alg == 0) { + if (kdf_alg == PSA_ALG_NONE) { /* This is a blank or aborted operation. */ status = PSA_ERROR_BAD_STATE; goto exit;