From c0fe820dc9f79050189b1d02fbe0922dd5875b86 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Thu, 7 Oct 2021 11:08:56 +0200 Subject: [PATCH 1/5] psa_generate_key(): return PSA_ERROR_INVALID_ARGUMENT for public key Signed-off-by: Przemyslaw Stekiel --- library/psa_crypto.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ece64b100d..59c267827a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5703,6 +5703,10 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, if( psa_get_key_bits( attributes ) == 0 ) return( PSA_ERROR_INVALID_ARGUMENT ); + /* Reject any attempt to create a public key. */ + if( PSA_KEY_TYPE_IS_PUBLIC_KEY(attributes->core.type) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + status = psa_start_key_creation( PSA_KEY_CREATION_GENERATE, attributes, &slot, &driver ); if( status != PSA_SUCCESS ) From 770153e83665426606a19ed8a4b8755211dfda74 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Thu, 7 Oct 2021 11:12:41 +0200 Subject: [PATCH 2/5] Add change-log entry Signed-off-by: Przemyslaw Stekiel --- ChangeLog.d/fix-psa_gen_key-status.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/fix-psa_gen_key-status.txt diff --git a/ChangeLog.d/fix-psa_gen_key-status.txt b/ChangeLog.d/fix-psa_gen_key-status.txt new file mode 100644 index 0000000000..c46bd6f01f --- /dev/null +++ b/ChangeLog.d/fix-psa_gen_key-status.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix status ret by psa_generate_key() for public key. Fixes #4551. From d9d630cdf301f88b49458f163b7f0b01bcdbc9fb Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Fri, 8 Oct 2021 12:26:21 +0200 Subject: [PATCH 3/5] Addapt psa_generate_key() tests Signed-off-by: Przemyslaw Stekiel --- tests/scripts/generate_psa_tests.py | 17 +++++++++++------ tests/suites/test_suite_psa_crypto.data | 2 +- ...st_suite_psa_crypto_not_supported.function | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index c788ce6d6d..1cdd28f89a 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -133,7 +133,7 @@ class Information: return constructors -def test_case_for_key_type_not_supported( +def test_case_for_key_type_not_supported_invalid_arg( verb: str, key_type: str, bits: int, dependencies: List[str], *args: str, @@ -148,10 +148,15 @@ def test_case_for_key_type_not_supported( adverb = 'not' if dependencies else 'never' if param_descr: adverb = param_descr + ' ' + adverb - tc.set_description('PSA {} {} {}-bit {} supported' - .format(verb, short_key_type, bits, adverb)) + if (verb == "generate") and ("PUBLIC" in short_key_type): + tc.set_description('PSA {} {} {}-bit invalid argument' + .format(verb, short_key_type, bits)) + tc.set_function(verb + '_invalid_arg') + else: + tc.set_description('PSA {} {} {}-bit {} supported' + .format(verb, short_key_type, bits, adverb)) + tc.set_function(verb + '_not_supported') tc.set_dependencies(dependencies) - tc.set_function(verb + '_not_supported') tc.set_arguments([key_type] + list(args)) return tc @@ -192,7 +197,7 @@ class NotSupported: else: generate_dependencies = import_dependencies for bits in kt.sizes_to_test(): - yield test_case_for_key_type_not_supported( + yield test_case_for_key_type_not_supported_invalid_arg( 'import', kt.expression, bits, finish_family_dependencies(import_dependencies, bits), test_case.hex_string(kt.key_material(bits)), @@ -203,7 +208,7 @@ class NotSupported: # supported or not depending on implementation capabilities, # only generate the test case once. continue - yield test_case_for_key_type_not_supported( + yield test_case_for_key_type_not_supported_invalid_arg( 'generate', kt.expression, bits, finish_family_dependencies(generate_dependencies, bits), str(bits), diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 063629e599..350537b051 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -4705,7 +4705,7 @@ generate_random:2 * MBEDTLS_CTR_DRBG_MAX_REQUEST + 1 PSA generate key: bad type (RSA public key) depends_on:PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY -generate_key:PSA_KEY_TYPE_RSA_PUBLIC_KEY:512:PSA_KEY_USAGE_EXPORT:0:PSA_ERROR_NOT_SUPPORTED:0 +generate_key:PSA_KEY_TYPE_RSA_PUBLIC_KEY:512:PSA_KEY_USAGE_EXPORT:0:PSA_ERROR_INVALID_ARGUMENT:0 PSA generate key: raw data, 0 bits: invalid argument # The spec allows either INVALID_ARGUMENT or NOT_SUPPORTED diff --git a/tests/suites/test_suite_psa_crypto_not_supported.function b/tests/suites/test_suite_psa_crypto_not_supported.function index e3253d8405..6b85fd75a7 100644 --- a/tests/suites/test_suite_psa_crypto_not_supported.function +++ b/tests/suites/test_suite_psa_crypto_not_supported.function @@ -50,3 +50,22 @@ exit: PSA_DONE( ); } /* END_CASE */ + +/* BEGIN_CASE */ +void generate_invalid_arg( int key_type, int bits ) +{ + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = INVALID_KEY_ID; + + PSA_ASSERT( psa_crypto_init( ) ); + psa_set_key_type( &attributes, key_type ); + psa_set_key_bits( &attributes, bits ); + TEST_EQUAL( psa_generate_key( &attributes, &key_id ), + PSA_ERROR_INVALID_ARGUMENT ); + TEST_ASSERT( mbedtls_svc_key_id_equal( key_id, MBEDTLS_SVC_KEY_ID_INIT ) ); + +exit: + psa_destroy_key( key_id ); + PSA_DONE( ); +} +/* END_CASE */ From 25f70635330eb295577368a55cb17febf3e881d1 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Fri, 8 Oct 2021 14:45:04 +0200 Subject: [PATCH 4/5] enerate_psa_tests.py fix format Signed-off-by: Przemyslaw Stekiel --- tests/scripts/generate_psa_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 1cdd28f89a..45d940ea97 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -150,11 +150,11 @@ def test_case_for_key_type_not_supported_invalid_arg( adverb = param_descr + ' ' + adverb if (verb == "generate") and ("PUBLIC" in short_key_type): tc.set_description('PSA {} {} {}-bit invalid argument' - .format(verb, short_key_type, bits)) + .format(verb, short_key_type, bits)) tc.set_function(verb + '_invalid_arg') else: tc.set_description('PSA {} {} {}-bit {} supported' - .format(verb, short_key_type, bits, adverb)) + .format(verb, short_key_type, bits, adverb)) tc.set_function(verb + '_not_supported') tc.set_dependencies(dependencies) tc.set_arguments([key_type] + list(args)) From b576c7b779edb2c9f2e45206f13a0ca8b1c93cbf Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Mon, 11 Oct 2021 10:15:25 +0200 Subject: [PATCH 5/5] Address review comments Signed-off-by: Przemyslaw Stekiel --- ChangeLog.d/fix-psa_gen_key-status.txt | 2 +- tests/scripts/generate_psa_tests.py | 66 +++++++++++++------ ...st_suite_psa_crypto_not_supported.function | 2 +- 3 files changed, 48 insertions(+), 22 deletions(-) diff --git a/ChangeLog.d/fix-psa_gen_key-status.txt b/ChangeLog.d/fix-psa_gen_key-status.txt index c46bd6f01f..78609882f9 100644 --- a/ChangeLog.d/fix-psa_gen_key-status.txt +++ b/ChangeLog.d/fix-psa_gen_key-status.txt @@ -1,2 +1,2 @@ Bugfix - * Fix status ret by psa_generate_key() for public key. Fixes #4551. + * Fix the error returned by psa_generate_key() for a public key. Fixes #4551. diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 45d940ea97..4c8143ff09 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -133,7 +133,7 @@ class Information: return constructors -def test_case_for_key_type_not_supported_invalid_arg( +def test_case_for_key_type_not_supported( verb: str, key_type: str, bits: int, dependencies: List[str], *args: str, @@ -148,20 +148,37 @@ def test_case_for_key_type_not_supported_invalid_arg( adverb = 'not' if dependencies else 'never' if param_descr: adverb = param_descr + ' ' + adverb - if (verb == "generate") and ("PUBLIC" in short_key_type): - tc.set_description('PSA {} {} {}-bit invalid argument' - .format(verb, short_key_type, bits)) - tc.set_function(verb + '_invalid_arg') - else: - tc.set_description('PSA {} {} {}-bit {} supported' - .format(verb, short_key_type, bits, adverb)) - tc.set_function(verb + '_not_supported') + tc.set_description('PSA {} {} {}-bit {} supported' + .format(verb, short_key_type, bits, adverb)) + tc.set_dependencies(dependencies) + tc.set_function(verb + '_not_supported') + tc.set_arguments([key_type] + list(args)) + return tc + +def test_case_for_key_type_invalid_argument( + verb: str, key_type: str, bits: int, + dependencies: List[str], + *args: str, + param_descr: str = '' +) -> test_case.TestCase: + """Return one test case exercising a key creation method + for an invalid argument when key is public. + """ + hack_dependencies_not_implemented(dependencies) + tc = test_case.TestCase() + short_key_type = re.sub(r'PSA_(KEY_TYPE|ECC_FAMILY)_', r'', key_type) + adverb = 'not' if dependencies else 'never' + if param_descr: + adverb = param_descr + ' ' + adverb + tc.set_description('PSA {} {} {}-bit invalid argument' + .format(verb, short_key_type, bits)) + tc.set_function(verb + '_invalid_argument') tc.set_dependencies(dependencies) tc.set_arguments([key_type] + list(args)) return tc class NotSupported: - """Generate test cases for when something is not supported.""" + """Generate test cases for when something is not supported or argument is inavlid.""" def __init__(self, info: Information) -> None: self.constructors = info.constructors @@ -176,11 +193,13 @@ class NotSupported: param: Optional[int] = None, param_descr: str = '', ) -> Iterator[test_case.TestCase]: - """Return test cases exercising key creation when the given type is unsupported. + """Return test cases exercising key creation when the given type is unsupported + or argument is invalid. If param is present and not None, emit test cases conditioned on this parameter not being supported. If it is absent or None, emit test cases - conditioned on the base type not being supported. + conditioned on the base type not being supported. If key is public emit test + case for invalid argument. """ if kt.name in self.ALWAYS_SUPPORTED: # Don't generate test cases for key types that are always supported. @@ -197,7 +216,7 @@ class NotSupported: else: generate_dependencies = import_dependencies for bits in kt.sizes_to_test(): - yield test_case_for_key_type_not_supported_invalid_arg( + yield test_case_for_key_type_not_supported( 'import', kt.expression, bits, finish_family_dependencies(import_dependencies, bits), test_case.hex_string(kt.key_material(bits)), @@ -208,12 +227,20 @@ class NotSupported: # supported or not depending on implementation capabilities, # only generate the test case once. continue - yield test_case_for_key_type_not_supported_invalid_arg( - 'generate', kt.expression, bits, - finish_family_dependencies(generate_dependencies, bits), - str(bits), - param_descr=param_descr, - ) + if kt.name.endswith('_PUBLIC_KEY'): + yield test_case_for_key_type_invalid_argument( + 'generate', kt.expression, bits, + finish_family_dependencies(generate_dependencies, bits), + str(bits), + param_descr=param_descr, + ) + else: + yield test_case_for_key_type_not_supported( + 'generate', kt.expression, bits, + finish_family_dependencies(generate_dependencies, bits), + str(bits), + param_descr=param_descr, + ) # To be added: derive ECC_KEY_TYPES = ('PSA_KEY_TYPE_ECC_KEY_PAIR', @@ -234,7 +261,6 @@ class NotSupported: yield from self.test_cases_for_key_type_not_supported( kt, 0, param_descr='curve') - class StorageKey(psa_storage.Key): """Representation of a key for storage format testing.""" diff --git a/tests/suites/test_suite_psa_crypto_not_supported.function b/tests/suites/test_suite_psa_crypto_not_supported.function index 6b85fd75a7..0665230d72 100644 --- a/tests/suites/test_suite_psa_crypto_not_supported.function +++ b/tests/suites/test_suite_psa_crypto_not_supported.function @@ -52,7 +52,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void generate_invalid_arg( int key_type, int bits ) +void generate_invalid_argument( int key_type, int bits ) { psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; mbedtls_svc_key_id_t key_id = INVALID_KEY_ID;