Merge pull request #5037 from mprse/issue_4551

Fix psa_generate_key(): return PSA_ERROR_INVALID_ARGUMENT for public key

ChangeLog.d/fix-psa_gen_key-status.txt

@@ -0,0 +1,2 @@
+Bugfix
+   * Fix the error returned by psa_generate_key() for a public key. Fixes #4551.

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 )

tests/scripts/generate_psa_tests.py

@@ -155,8 +155,30 @@ def test_case_for_key_type_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
@@ -171,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.
@@ -203,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(
-                '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',
@@ -229,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."""
 

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

tests/suites/test_suite_psa_crypto_not_supported.function

@@ -50,3 +50,22 @@ exit:
     PSA_DONE( );
 }
 /* END_CASE */
+
+/* BEGIN_CASE */
+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;
+
+    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 */