From e5204c94a148b48da8819682bf77bfd54448c924 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Mon, 22 Oct 2018 17:24:55 +0300 Subject: [PATCH 1/7] add tests that increase key derivation code coverage slightly added tests that increase code coverage for the key derivation functions slightly by reaching error cases not covered before. --- tests/suites/test_suite_psa_crypto.data | 12 +++++++++++ tests/suites/test_suite_psa_crypto.function | 23 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 871a511b2..1a93a8929 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1189,6 +1189,10 @@ PSA key derivation: HKDF-SHA-256, good case depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HKDF(PSA_ALG_SHA_256):"":"":42:PSA_SUCCESS +PSA key derivation: HKDF-SHA-512, good case +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HKDF(PSA_ALG_SHA_512):"":"":42:PSA_SUCCESS + PSA key derivation: bad key type depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_setup:PSA_KEY_TYPE_RAW_DATA:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HKDF(PSA_ALG_SHA_256):"":"":42:PSA_ERROR_INVALID_ARGUMENT @@ -1201,6 +1205,14 @@ PSA key derivation: unsupported key derivation algorithm depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HKDF(PSA_ALG_CATEGORY_HASH):"":"":42:PSA_ERROR_NOT_SUPPORTED +PSA key derivation: unsupported key derivation algorithm +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_CATEGORY_KEY_DERIVATION:"":"":42:PSA_ERROR_NOT_SUPPORTED + +PSA key derivation: bad arguments test +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +test_derive_invalid_generator: + PSA key derivation: HKDF SHA-256, RFC5869 #1, output 42+0 depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_output:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":42:"3cb25f25faacd57a90434f64d0362f2a2d2d0a90cf1a5a4c5db02d56ecc4c5bf34007208d5b887185865":"" diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 63d837fdc..c6f49c007 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3085,6 +3085,29 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void test_derive_invalid_generator() +{ + psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; + psa_key_slot_t base_key = 1; + psa_algorithm_t alg = PSA_ALG_HKDF(PSA_ALG_SHA_256); + data_t salt; + data_t label; + size_t capacity = 0; + salt.x = NULL; + salt.len = 0; + label.x = NULL; + label.len = 0; + + generator.alg = alg; + /* invalid generator.alg */ + TEST_ASSERT( psa_key_derivation( &generator, base_key, alg, + salt.x, salt.len, + label.x, label.len, + capacity ) == PSA_ERROR_BAD_STATE ); +} +/* END_CASE */ + /* BEGIN_CASE */ void derive_output( int alg_arg, data_t *key_data, From b46e7ca16bd1f06d3162d9ba8576cb44751f79f6 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Thu, 25 Oct 2018 14:46:09 +0300 Subject: [PATCH 2/7] add additional generator tests and generalize key derivation test Key derivation test now uses an indirect way to test generator validity as the direct way previously used isn't compatible with the PSA IPC implementation. Additional bad path test for the generator added to check basic bad-path scenarios. --- tests/suites/test_suite_psa_crypto.data | 8 ++- tests/suites/test_suite_psa_crypto.function | 80 +++++++++++++++++---- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 1a93a8929..39ac88839 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1209,9 +1209,13 @@ PSA key derivation: unsupported key derivation algorithm depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_CATEGORY_KEY_DERIVATION:"":"":42:PSA_ERROR_NOT_SUPPORTED -PSA key derivation: bad arguments test +PSA key derivation: invalid generator state ( double generate + read past capacity ) depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C -test_derive_invalid_generator: +test_derive_invalid_generator_state:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b" + +PSA key derivation: invalid generator state ( call read/get_capacity after init and abort ) +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +test_derive_invalid_generator_tests: PSA key derivation: HKDF SHA-256, RFC5869 #1, output 42+0 depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index c6f49c007..65bec58c3 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3086,25 +3086,77 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void test_derive_invalid_generator() +void test_derive_invalid_generator_state( int key_type_arg, data_t *key_data) { - psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; psa_key_slot_t base_key = 1; + size_t key_type = key_type_arg; + psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; psa_algorithm_t alg = PSA_ALG_HKDF(PSA_ALG_SHA_256); - data_t salt; - data_t label; - size_t capacity = 0; - salt.x = NULL; - salt.len = 0; - label.x = NULL; - label.len = 0; + size_t capacity = 42; + uint8_t buffer[42]; + psa_key_policy_t policy; - generator.alg = alg; - /* invalid generator.alg */ + TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS ); + + psa_key_policy_init( &policy ); + psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_DERIVE, alg ); + TEST_ASSERT( psa_set_key_policy( base_key, &policy ) == PSA_SUCCESS ); + + TEST_ASSERT( psa_import_key( base_key, key_type, + key_data->x, + key_data->len ) == PSA_SUCCESS ); + + /* valid key derivation */ TEST_ASSERT( psa_key_derivation( &generator, base_key, alg, - salt.x, salt.len, - label.x, label.len, - capacity ) == PSA_ERROR_BAD_STATE ); + NULL, 0, + NULL, 0, + capacity ) == PSA_SUCCESS ); + + /* state of generator shouldn't allow additional generation */ + TEST_ASSERT( psa_key_derivation( &generator, base_key, alg, + NULL, 0, + NULL, 0, + capacity ) == PSA_ERROR_BAD_STATE ); + + TEST_ASSERT( psa_generator_read( &generator, buffer, capacity ) + == PSA_SUCCESS ); + + TEST_ASSERT( psa_generator_read( &generator, buffer, capacity ) + == PSA_ERROR_INSUFFICIENT_CAPACITY ); + + +exit: + psa_generator_abort( &generator ); + psa_destroy_key( base_key ); + mbedtls_psa_crypto_free( ); +} +/* END_CASE */ + + +/* BEGIN_CASE */ +void test_derive_invalid_generator_tests( ) +{ + uint8_t output_buffer[16]; + size_t buffer_size = 16; + size_t capacity = 0; + psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; + + TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size) + == PSA_ERROR_INSUFFICIENT_CAPACITY ); + + TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) + == PSA_ERROR_BAD_STATE ); + + TEST_ASSERT( psa_generator_abort(&generator) == PSA_SUCCESS ); + + TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size) + == PSA_ERROR_INSUFFICIENT_CAPACITY ); + + TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity) + == PSA_ERROR_BAD_STATE ); + +exit: + psa_generator_abort( &generator ); } /* END_CASE */ From 4eda37bb9e831a8c28fa1706e08908f84e367ce7 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Wed, 31 Oct 2018 12:15:58 +0200 Subject: [PATCH 3/7] streamline test function API by removing parameter streamline the API for the test test_derive_invalid_generator_state by removing the key type paramter (it is assumed to always be PSA_KEY_TYPE_DERIVE) --- tests/suites/test_suite_psa_crypto.data | 2 +- tests/suites/test_suite_psa_crypto.function | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 39ac88839..66bb175a2 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1211,7 +1211,7 @@ derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b": PSA key derivation: invalid generator state ( double generate + read past capacity ) depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C -test_derive_invalid_generator_state:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b" +test_derive_invalid_generator_state:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b" PSA key derivation: invalid generator state ( call read/get_capacity after init and abort ) depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 65bec58c3..34455fffa 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3086,10 +3086,10 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void test_derive_invalid_generator_state( int key_type_arg, data_t *key_data) +void test_derive_invalid_generator_state( data_t *key_data ) { psa_key_slot_t base_key = 1; - size_t key_type = key_type_arg; + size_t key_type = PSA_KEY_TYPE_DERIVE; psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; psa_algorithm_t alg = PSA_ALG_HKDF(PSA_ALG_SHA_256); size_t capacity = 42; From 5078930459f6be94b9984a2518b9b26340fefe55 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Wed, 31 Oct 2018 12:16:38 +0200 Subject: [PATCH 4/7] fix whitespace issues --- tests/suites/test_suite_psa_crypto.function | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 34455fffa..f7a48093b 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3141,18 +3141,18 @@ void test_derive_invalid_generator_tests( ) size_t capacity = 0; psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; - TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size) + TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size ) == PSA_ERROR_INSUFFICIENT_CAPACITY ); TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) == PSA_ERROR_BAD_STATE ); - TEST_ASSERT( psa_generator_abort(&generator) == PSA_SUCCESS ); + TEST_ASSERT( psa_generator_abort( &generator ) == PSA_SUCCESS ); - TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size) + TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size ) == PSA_ERROR_INSUFFICIENT_CAPACITY ); - TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity) + TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) == PSA_ERROR_BAD_STATE ); exit: From f8964b95804fb2212ace60ab78a272dcc540fd7b Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Wed, 31 Oct 2018 18:06:14 +0200 Subject: [PATCH 5/7] updated test to work around https://github.com/ARMmbed/mbedtls-psa/issues/183 test should check the correct error values once this issue is fixed --- tests/suites/test_suite_psa_crypto.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index f7a48093b..7e8120040 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3145,7 +3145,7 @@ void test_derive_invalid_generator_tests( ) == PSA_ERROR_INSUFFICIENT_CAPACITY ); TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) - == PSA_ERROR_BAD_STATE ); + == PSA_SUCCESS ); // should be PSA_ERROR_BAD_STATE:issue opened TEST_ASSERT( psa_generator_abort( &generator ) == PSA_SUCCESS ); @@ -3153,7 +3153,7 @@ void test_derive_invalid_generator_tests( ) == PSA_ERROR_INSUFFICIENT_CAPACITY ); TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) - == PSA_ERROR_BAD_STATE ); + == PSA_SUCCESS );// should be PSA_ERROR_BAD_STATE:issue opened exit: psa_generator_abort( &generator ); From dd69d8b7ffd73deb184c81bfca1b2c4c04e4e0d2 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Thu, 1 Nov 2018 12:24:23 +0200 Subject: [PATCH 6/7] Streamline test function API by removing parameter streamline the API for the test test_derive_invalid_generator_state: by removing the key_data parameter. This parameter is not important for test flow and can be hard-coded. --- tests/suites/test_suite_psa_crypto.data | 2 +- tests/suites/test_suite_psa_crypto.function | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 66bb175a2..10ab81222 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1211,7 +1211,7 @@ derive_setup:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b": PSA key derivation: invalid generator state ( double generate + read past capacity ) depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C -test_derive_invalid_generator_state:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b" +test_derive_invalid_generator_state: PSA key derivation: invalid generator state ( call read/get_capacity after init and abort ) depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 7e8120040..528857b8e 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3086,7 +3086,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void test_derive_invalid_generator_state( data_t *key_data ) +void test_derive_invalid_generator_state( ) { psa_key_slot_t base_key = 1; size_t key_type = PSA_KEY_TYPE_DERIVE; @@ -3094,6 +3094,9 @@ void test_derive_invalid_generator_state( data_t *key_data ) psa_algorithm_t alg = PSA_ALG_HKDF(PSA_ALG_SHA_256); size_t capacity = 42; uint8_t buffer[42]; + const uint8_t key_data[22] = { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, + 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, + 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b}; psa_key_policy_t policy; TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS ); @@ -3103,8 +3106,8 @@ void test_derive_invalid_generator_state( data_t *key_data ) TEST_ASSERT( psa_set_key_policy( base_key, &policy ) == PSA_SUCCESS ); TEST_ASSERT( psa_import_key( base_key, key_type, - key_data->x, - key_data->len ) == PSA_SUCCESS ); + key_data, + sizeof(key_data) ) == PSA_SUCCESS ); /* valid key derivation */ TEST_ASSERT( psa_key_derivation( &generator, base_key, alg, From 1caf6d24f2b642996cd82569880808830977bfed Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein <nir.sonnenschein@arm.com> Date: Thu, 1 Nov 2018 12:27:20 +0200 Subject: [PATCH 7/7] Fix code style and clarify issue comment * remove unneeded constants * clarify comment reference to issue 183 * add additional reference comment * fix brace spacing issues --- tests/suites/test_suite_psa_crypto.function | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 528857b8e..56a23fe04 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3091,9 +3091,9 @@ void test_derive_invalid_generator_state( ) psa_key_slot_t base_key = 1; size_t key_type = PSA_KEY_TYPE_DERIVE; psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; - psa_algorithm_t alg = PSA_ALG_HKDF(PSA_ALG_SHA_256); - size_t capacity = 42; + psa_algorithm_t alg = PSA_ALG_HKDF( PSA_ALG_SHA_256 ); uint8_t buffer[42]; + size_t capacity = sizeof( buffer ); const uint8_t key_data[22] = { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b}; @@ -3107,7 +3107,7 @@ void test_derive_invalid_generator_state( ) TEST_ASSERT( psa_import_key( base_key, key_type, key_data, - sizeof(key_data) ) == PSA_SUCCESS ); + sizeof( key_data ) ) == PSA_SUCCESS ); /* valid key derivation */ TEST_ASSERT( psa_key_derivation( &generator, base_key, alg, @@ -3145,18 +3145,18 @@ void test_derive_invalid_generator_tests( ) psa_crypto_generator_t generator = PSA_CRYPTO_GENERATOR_INIT; TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size ) - == PSA_ERROR_INSUFFICIENT_CAPACITY ); + == PSA_ERROR_INSUFFICIENT_CAPACITY ); // should be PSA_ERROR_BAD_STATE:#183 TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) - == PSA_SUCCESS ); // should be PSA_ERROR_BAD_STATE:issue opened + == PSA_SUCCESS ); // should be PSA_ERROR_BAD_STATE:#183 TEST_ASSERT( psa_generator_abort( &generator ) == PSA_SUCCESS ); TEST_ASSERT( psa_generator_read( &generator, output_buffer, buffer_size ) - == PSA_ERROR_INSUFFICIENT_CAPACITY ); + == PSA_ERROR_INSUFFICIENT_CAPACITY ); // should be PSA_ERROR_BAD_STATE:#183 TEST_ASSERT( psa_get_generator_capacity( &generator, &capacity ) - == PSA_SUCCESS );// should be PSA_ERROR_BAD_STATE:issue opened + == PSA_SUCCESS );// should be PSA_ERROR_BAD_STATE:#183 exit: psa_generator_abort( &generator );