Address review comments

Signed-off-by: Przemyslaw Stekiel <przemyslaw.stekiel@mobica.com>

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.

tests/scripts/generate_psa_tests.py

@@ -127,7 +127,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,
@@ -142,20 +142,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 {} supported'
-        tc.set_description('PSA {} {} {}-bit invalid argument'
+                       .format(verb, short_key_type, bits, adverb))
-                           .format(verb, short_key_type, bits))
+    tc.set_dependencies(dependencies)
-        tc.set_function(verb + '_invalid_arg')
+    tc.set_function(verb + '_not_supported')
-    else:
+    tc.set_arguments([key_type] + list(args))
-        tc.set_description('PSA {} {} {}-bit {} supported'
+    return tc
-                           .format(verb, short_key_type, bits, adverb))
+
-        tc.set_function(verb + '_not_supported')
+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
@@ -170,11 +187,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.
@@ -191,7 +210,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)),
@@ -202,12 +221,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(
+            if kt.name.endswith('_PUBLIC_KEY'):
-                'generate', kt.expression, bits,
+                yield test_case_for_key_type_invalid_argument(
-                finish_family_dependencies(generate_dependencies, bits),
+                    'generate', kt.expression, bits,
-                str(bits),
+                    finish_family_dependencies(generate_dependencies, bits),
-                param_descr=param_descr,
+                    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',
@@ -228,7 +255,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."""
 

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;