From f522114db33309987aa558ada46c0395aa045d1c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Jan 2025 20:09:44 +0100 Subject: [PATCH 1/4] raw_key_agreement_with_self: check status consistency In `mbedtls_test_psa_raw_key_agreement_with_self()`, we may use up to three key agreement methods: `psa_raw_key_agreement()`, `psa_key_agreement()`, and the interruptible interface. Check that all three have the same status. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index b92c1f750..254759dde 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -732,9 +732,9 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( } PSA_ASSERT(status); - status = psa_raw_key_agreement(alg, key, - public_key, public_key_length, - output, sizeof(output), &output_length); + status = psa_raw_key_agreement( + alg, key, public_key, public_key_length, + output, sizeof(output), &output_length); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; @@ -749,6 +749,8 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( } #if MBEDTLS_VERSION_MAJOR >= 4 + psa_status_t raw_status = status; + psa_set_key_type(&shared_secret_attributes, PSA_KEY_TYPE_DERIVE); psa_set_key_usage_flags(&shared_secret_attributes, PSA_KEY_USAGE_DERIVE | PSA_KEY_USAGE_EXPORT); @@ -759,8 +761,15 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( /* The key has been destroyed. */ status = PSA_SUCCESS; goto exit; - } else if (status == PSA_SUCCESS) { + } + /* In this function, we expect either success or a validation failure, + * which should be identical for raw output and key output. So flag any + * discrepancy between the two (in particular a key creation failure) + * as a test failure. */ + TEST_EQUAL(raw_status, status); + + if (status == PSA_SUCCESS) { status = psa_get_key_attributes(shared_secret_id, &export_attributes); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ @@ -798,7 +807,16 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( /* The key has been destroyed. */ status = PSA_SUCCESS; goto exit; - } else if (status == PSA_SUCCESS) { + } + + /* In this function, we expect either success or a validation failure, + * which should be identical for one-shot and interruptible. For an + * interruptible operation, we insist on detecting error conditions + * early, in setup() rather than complete(). So flag any discrepancy + * between one-shot and interruptible-setup as a test failure. */ + TEST_EQUAL(raw_status, status); + + if (status == PSA_SUCCESS) { do { status = psa_key_agreement_iop_complete(&iop_operation, &shared_secret_id); From 82b5b92bf8a845c32d752c450907c74bd00c802c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Jan 2025 20:11:35 +0100 Subject: [PATCH 2/4] raw_key_agreement_with_self: check output consistency In `mbedtls_test_psa_raw_key_agreement_with_self()`, we may use up to three key agreement methods: `psa_raw_key_agreement()`, `psa_key_agreement()`, and the interruptible interface. Check that all three have the same output. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 254759dde..3798d1a45 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -782,13 +782,14 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( TEST_CALLOC(exported, exported_size); status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); - if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; + } else { + PSA_ASSERT(status); + TEST_MEMORY_COMPARE(exported, exported_length, + output, output_length); } - - PSA_ASSERT(status); } #if defined(MBEDTLS_ECP_RESTARTABLE) && defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH) @@ -825,9 +826,18 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; + } else { + PSA_ASSERT(status); + status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); + if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { + /* The key has been destroyed. */ + status = PSA_SUCCESS; + } else { + PSA_ASSERT(status); + TEST_MEMORY_COMPARE(exported, exported_length, + output, output_length); + } } - - PSA_ASSERT(status); } } else { TEST_EQUAL(psa_key_agreement_iop_setup(&iop_operation, key, public_key, From 9ae1988c074143337179a40d9c0c5077af388916 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Jan 2025 20:11:46 +0100 Subject: [PATCH 3/4] mbedtls_test_psa_raw_key_agreement_with_self: update documentation Fix various obsolete or copy-pasted things, and document what test assertions this function makes. Signed-off-by: Gilles Peskine --- tests/include/test/psa_exercise_key.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/include/test/psa_exercise_key.h b/tests/include/test/psa_exercise_key.h index f44608b4b..b0e318144 100644 --- a/tests/include/test/psa_exercise_key.h +++ b/tests/include/test/psa_exercise_key.h @@ -138,11 +138,22 @@ int mbedtls_test_psa_setup_key_derivation_wrap( size_t capacity, int key_destroyable); /** Perform a key agreement using the given key pair against its public key - * using psa_raw_key_agreement() and psa_key_agreement(). + * (not combined with a key derivation). * - * The result is discarded. The purpose of this function is to smoke-test a key. + * The result is discarded. Thus this function can be used for smoke-testing + * a key, and to validate input validation, but not to validate results. * - * In case of failure, mark the current test case as failed. + * Depending on the library version, there can be multiple interfaces for key + * agreement. This test function performs the ones that are available amongst: + * - psa_raw_key_agreement() + * - psa_key_agreement() + * - psa_key_agreement_iop_setup() and psa_key_agreement_iop_complete() + * + * Mark the current test case as failed in the following cases: + * - Operational errors such as failure to allocate memory for an intermediate + * buffer. + * - Results are not consistent between the methods that are performed: + * different statuses, or inconsistent metadata, or different shared secret. * * \param alg A key agreement algorithm compatible with \p key. * \param key A key that allows key agreement with \p alg. @@ -150,7 +161,7 @@ int mbedtls_test_psa_setup_key_derivation_wrap( * or the key being destroyed mid-operation will only * be reported if the error code is unexpected. * - * \return \c 1 on success, \c 0 on failure. + * \return The status from psa_raw_key_agreement(). */ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( psa_algorithm_t alg, From 3f8b367e5feb5fc16718f68f9850a3d3cda2b6a2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Jan 2025 12:25:22 +0100 Subject: [PATCH 4/4] Shorten lines Pacify code_style.py and shorten a few more long lines. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 3798d1a45..01fc143e7 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -752,7 +752,8 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( psa_status_t raw_status = status; psa_set_key_type(&shared_secret_attributes, PSA_KEY_TYPE_DERIVE); - psa_set_key_usage_flags(&shared_secret_attributes, PSA_KEY_USAGE_DERIVE | PSA_KEY_USAGE_EXPORT); + psa_set_key_usage_flags(&shared_secret_attributes, + PSA_KEY_USAGE_DERIVE | PSA_KEY_USAGE_EXPORT); status = psa_key_agreement(key, public_key, public_key_length, alg, &shared_secret_attributes, &shared_secret_id); @@ -777,11 +778,13 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( goto exit; } - exported_size = PSA_EXPORT_KEY_OUTPUT_SIZE(psa_get_key_type(&export_attributes), - psa_get_key_bits(&export_attributes)); + exported_size = + PSA_EXPORT_KEY_OUTPUT_SIZE(psa_get_key_type(&export_attributes), + psa_get_key_bits(&export_attributes)); TEST_CALLOC(exported, exported_size); - status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); + status = psa_export_key(shared_secret_id, + exported, exported_size, &exported_length); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; @@ -820,7 +823,8 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( if (status == PSA_SUCCESS) { do { - status = psa_key_agreement_iop_complete(&iop_operation, &shared_secret_id); + status = psa_key_agreement_iop_complete(&iop_operation, + &shared_secret_id); } while (status == PSA_OPERATION_INCOMPLETE); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { @@ -828,7 +832,9 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( status = PSA_SUCCESS; } else { PSA_ASSERT(status); - status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); + status = psa_export_key(shared_secret_id, + exported, exported_size, + &exported_length); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS;