Merge pull request #104 from gilles-peskine-arm/psa-storage-test-cases-never-supported-negative-framework

Switch generate_psa_test.py to automatic dependencies for negative test cases

Author: gilles-peskine-arm

scripts/generate_psa_tests.py

@@ -11,7 +11,7 @@
 import enum
 import re
 import sys
-from typing import Callable, Dict, FrozenSet, Iterable, Iterator, List, Optional
+from typing import Callable, Dict, Iterable, Iterator, List, Optional
 
 from mbedtls_framework import crypto_data_tests
 from mbedtls_framework import crypto_knowledge
@@ -26,7 +26,7 @@
 
 def test_case_for_key_type_not_supported(
         verb: str, key_type: str, bits: int,
-        dependencies: List[str],
+        not_supported_mechanism: str,
         *args: str,
         param_descr: str = ''
 ) -> test_case.TestCase:
@@ -35,17 +35,16 @@ def test_case_for_key_type_not_supported(
     """
     tc = psa_test_case.TestCase()
     short_key_type = crypto_knowledge.short_expression(key_type)
-    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))
+    tc.set_description('PSA {} {} {}-bit{} not supported'
+                       .format(verb, short_key_type, bits,
+                               ' ' + param_descr if param_descr else ''))
+    # if tc.description == 'PSA import RSA_KEY_PAIR 1024-bit not supported':
+    #     import pdb; pdb.set_trace()
     tc.set_function(verb + '_not_supported')
     tc.set_key_bits(bits)
-    tc.set_key_pair_usage(verb.upper())
+    tc.set_key_pair_usage([verb.upper()])
+    tc.assumes_not_supported(not_supported_mechanism)
     tc.set_arguments([key_type] + list(args))
-    tc.set_dependencies(dependencies)
-    tc.skip_if_any_not_implemented(dependencies)
     return tc
 
 class KeyTypeNotSupported:
@@ -77,37 +76,27 @@ def test_cases_for_key_type_not_supported(
             # Don't generate test cases for key types that are always supported.
             # They would be skipped in all configurations, which is noise.
             return
-        import_dependencies = [('!' if param is None else '') +
-                               psa_information.psa_want_symbol(kt.name)]
-        if kt.params is not None:
-            import_dependencies += [('!' if param == i else '') +
-                                    psa_information.psa_want_symbol(sym)
-                                    for i, sym in enumerate(kt.params)]
-        if kt.name.endswith('_PUBLIC_KEY'):
-            generate_dependencies = []
+        if param is None:
+            not_supported_mechanism = kt.name
         else:
-            generate_dependencies = \
-                psa_information.fix_key_pair_dependencies(import_dependencies, 'GENERATE')
-            import_dependencies = \
-                psa_information.fix_key_pair_dependencies(import_dependencies, 'BASIC')
+            assert kt.params is not None
+            not_supported_mechanism = kt.params[param]
         for bits in kt.sizes_to_test():
             yield test_case_for_key_type_not_supported(
                 'import', kt.expression, bits,
-                psa_information.finish_family_dependencies(import_dependencies, bits),
+                not_supported_mechanism,
                 test_case.hex_string(kt.key_material(bits)),
                 param_descr=param_descr,
             )
-            if not generate_dependencies and param is not None:
-                # If generation is impossible for this key type, rather than
-                # supported or not depending on implementation capabilities,
-                # only generate the test case once.
-                continue
-                # For public key we expect that key generation fails with
-                # INVALID_ARGUMENT. It is handled by KeyGenerate class.
+            # Don't generate not-supported test cases for key generation of
+            # public keys. Our implementation always returns
+            # PSA_ERROR_INVALID_ARGUMENT when attempting to generate a
+            # public key, so we cover this together with the positive cases
+            # in the KeyGenerate class.
             if not kt.is_public():
                 yield test_case_for_key_type_not_supported(
                     'generate', kt.expression, bits,
-                    psa_information.finish_family_dependencies(generate_dependencies, bits),
+                    not_supported_mechanism,
                     str(bits),
                     param_descr=param_descr,
                 )
@@ -155,7 +144,7 @@ def test_case_for_key_generation(
                        .format(short_key_type, bits))
     tc.set_function('generate_key')
     tc.set_key_bits(bits)
-    tc.set_key_pair_usage('GENERATE')
+    tc.set_key_pair_usage(['GENERATE'])
     tc.set_arguments([key_type] + list(args) + [result])
     return tc
 
@@ -234,16 +223,19 @@ def make_test_case(
             category: crypto_knowledge.AlgorithmCategory,
             reason: 'Reason',
             kt: Optional[crypto_knowledge.KeyType] = None,
-            not_deps: FrozenSet[str] = frozenset(),
+            not_supported: Optional[str] = None,
     ) -> test_case.TestCase:
-        """Construct a failure test case for a one-key or keyless operation."""
+        """Construct a failure test case for a one-key or keyless operation.
+
+        If `reason` is `Reason.NOT_SUPPORTED`, pass the not-supported
+        dependency symbol as the `not_supported` argument.
+        """
         #pylint: disable=too-many-arguments,too-many-locals
         tc = psa_test_case.TestCase()
         pretty_alg = alg.short_expression()
         if reason == self.Reason.NOT_SUPPORTED:
-            short_deps = [re.sub(r'PSA_WANT_ALG_', r'', dep)
-                          for dep in not_deps]
-            pretty_reason = '!' + '&'.join(sorted(short_deps))
+            assert not_supported is not None
+            pretty_reason = '!' + re.sub(r'PSA_WANT_[A-Z]+_', r'', not_supported)
         else:
             pretty_reason = reason.name.lower()
         if kt:
@@ -257,16 +249,12 @@ def make_test_case(
                                    pretty_alg,
                                    pretty_reason,
                                    ' with ' + pretty_type if pretty_type else ''))
-        dependencies = psa_information.automatic_dependencies(alg.base_expression, key_type)
-        dependencies = psa_information.fix_key_pair_dependencies(dependencies, 'BASIC')
-        for i, dep in enumerate(dependencies):
-            if dep in not_deps:
-                dependencies[i] = '!' + dep
         tc.set_function(category.name.lower() + '_fail')
         arguments = [] # type: List[str]
         if kt:
             bits = kt.sizes_to_test()[0]
             tc.set_key_bits(bits)
+            tc.set_key_pair_usage(['IMPORT'])
             key_material = kt.key_material(bits)
             arguments += [key_type, test_case.hex_string(key_material)]
         arguments.append(alg.expression)
@@ -275,8 +263,25 @@ def make_test_case(
         error = ('NOT_SUPPORTED' if reason == self.Reason.NOT_SUPPORTED else
                  'INVALID_ARGUMENT')
         arguments.append('PSA_ERROR_' + error)
+        if reason == self.Reason.NOT_SUPPORTED:
+            assert not_supported is not None
+            tc.assumes_not_supported(not_supported)
+            # Special case: if one of deterministic/randomized
+            # ECDSA is supported but not the other, then the one
+            # that is not supported in the signature direction is
+            # still supported in the verification direction,
+            # because the two verification algorithms are
+            # identical. This property is how Mbed TLS chooses to
+            # behave, the specification would also allow it to
+            # reject the algorithm. In the generated test cases,
+            # we avoid this difficulty by not running the
+            # not-supported test case when exactly one of the
+            # two variants is supported.
+            if not_supported == 'PSA_WANT_ALG_ECDSA':
+                tc.add_dependencies(['!PSA_WANT_ALG_DETERMINISTIC_ECDSA'])
+            if not_supported == 'PSA_WANT_ALG_DETERMINISTIC_ECDSA':
+                tc.add_dependencies(['!PSA_WANT_ALG_ECDSA'])
         tc.set_arguments(arguments)
-        tc.set_dependencies(dependencies)
         return tc
 
     def no_key_test_cases(
@@ -290,7 +295,7 @@ def no_key_test_cases(
             for dep in psa_information.automatic_dependencies(alg.base_expression):
                 yield self.make_test_case(alg, category,
                                           self.Reason.NOT_SUPPORTED,
-                                          not_deps=frozenset([dep]))
+                                          not_supported=dep)
         else:
             # Incompatible operation, supported algorithm
             yield self.make_test_case(alg, category, self.Reason.INVALID)
@@ -308,7 +313,7 @@ def one_key_test_cases(
                 for dep in psa_information.automatic_dependencies(alg.base_expression):
                     yield self.make_test_case(alg, category,
                                               self.Reason.NOT_SUPPORTED,
-                                              kt=kt, not_deps=frozenset([dep]))
+                                              kt=kt, not_supported=dep)
                 # Public key for a private-key operation
                 if category.is_asymmetric() and kt.is_public():
                     yield self.make_test_case(alg, category,
@@ -481,7 +486,7 @@ def make_test_case(self, key: StorageTestData) -> test_case.TestCase:
         tc.add_dependencies(psa_information.generate_deps_from_description(key.description))
         tc.set_function('key_storage_' + verb)
         tc.set_key_bits(key.bits)
-        tc.set_key_pair_usage('BASIC')
+        tc.set_key_pair_usage(['IMPORT'] if self.forward else ['EXPORT'])
         if self.forward:
             extra_arguments = []
         else:

scripts/mbedtls_framework/psa_information.py

@@ -139,29 +139,18 @@ def generate_deps_from_description(
 
     return dep_list
 
-def tweak_key_pair_dependency(dep: str, usage: str):
+def tweak_key_pair_dependency(dep: str, usages: List[str]) -> List[str]:
     """
     This helper function add the proper suffix to PSA_WANT_KEY_TYPE_xxx_KEY_PAIR
     symbols according to the required usage.
     """
-    ret_list = list()
     if dep.endswith('KEY_PAIR'):
-        if usage == "BASIC":
-            # BASIC automatically includes IMPORT and EXPORT for test purposes (see
-            # config_psa.h).
-            ret_list.append(re.sub(r'KEY_PAIR', r'KEY_PAIR_BASIC', dep))
-            ret_list.append(re.sub(r'KEY_PAIR', r'KEY_PAIR_IMPORT', dep))
-            ret_list.append(re.sub(r'KEY_PAIR', r'KEY_PAIR_EXPORT', dep))
-        elif usage == "GENERATE":
-            ret_list.append(re.sub(r'KEY_PAIR', r'KEY_PAIR_GENERATE', dep))
-    else:
-        # No replacement to do in this case
-        ret_list.append(dep)
-    return ret_list
+        return [dep + '_' + usage for usage in usages]
+    return [dep]
 
-def fix_key_pair_dependencies(dep_list: List[str], usage: str):
+def fix_key_pair_dependencies(dep_list: List[str], usages: List[str]) -> List[str]:
     new_list = [new_deps
                 for dep in dep_list
-                for new_deps in tweak_key_pair_dependency(dep, usage)]
+                for new_deps in tweak_key_pair_dependency(dep, usages)]
 
     return new_list

scripts/mbedtls_framework/psa_test_case.py

@@ -14,11 +14,14 @@
 from . import test_case
 
 
-# A temporary hack: at the time of writing, not all dependency symbols
-# are implemented yet. Skip test cases for which the dependency symbols are
-# not available. Once all dependency symbols are available, this hack must
-# be removed so that a bug in the dependency symbols properly leads to a test
-# failure.
+# Skip test cases for which the dependency symbols are not defined.
+# We assume that this means that a required mechanism is not implemented.
+# Note that if we erroneously skip generating test cases for
+# mechanisms that are not implemented, this should be caught
+# by the NOT_SUPPORTED test cases generated by generate_psa_tests.py
+# in test_suite_psa_crypto_not_supported and test_suite_psa_crypto_op_fail:
+# those emit tests with negative dependencies, which will not be skipped here.
+
 def read_implemented_dependencies(acc: Set[str], filename: str) -> None:
     with open(filename) as input_stream:
         for line in input_stream:
@@ -51,8 +54,8 @@ def find_dependencies_not_implemented(dependencies: List[str]) -> List[str]:
         _implemented_dependencies = frozenset(acc)
     return [dep
             for dep in dependencies
-            if (dep.lstrip('!') not in _implemented_dependencies and
-                dep.lstrip('!').startswith('PSA_WANT'))]
+            if (dep not in _implemented_dependencies and
+                dep.startswith('PSA_WANT'))]
 
 
 class TestCase(test_case.TestCase):
@@ -75,8 +78,9 @@ def __init__(self, dependency_prefix: Optional[str] = None) -> None:
         self.manual_dependencies = [] #type: List[str]
         self.automatic_dependencies = set() #type: Set[str]
         self.dependency_prefix = dependency_prefix #type: Optional[str]
+        self.negated_dependencies = set() #type: Set[str]
         self.key_bits = None #type: Optional[int]
-        self.key_pair_usage = None #type: Optional[str]
+        self.key_pair_usage = None #type: Optional[List[str]]
 
     def set_key_bits(self, key_bits: Optional[int]) -> None:
         """Use the given key size for automatic dependency generation.
@@ -88,8 +92,8 @@ def set_key_bits(self, key_bits: Optional[int]) -> None:
         """
         self.key_bits = key_bits
 
-    def set_key_pair_usage(self, key_pair_usage: Optional[str]) -> None:
-        """Use the given suffix for key pair dependencies.
+    def set_key_pair_usage(self, key_pair_usage: Optional[List[str]]) -> None:
+        """Use the given suffixes for key pair dependencies.
 
         Call this function before set_arguments() if relevant.
 
@@ -109,16 +113,62 @@ def infer_dependencies(self, arguments: List[str]) -> List[str]:
             dependencies = psa_information.fix_key_pair_dependencies(dependencies,
                                                                      self.key_pair_usage)
         if 'PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE' in dependencies and \
+           'PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE' not in self.negated_dependencies and \
            self.key_bits is not None:
             size_dependency = ('PSA_VENDOR_RSA_GENERATE_MIN_KEY_BITS <= ' +
                                str(self.key_bits))
             dependencies.append(size_dependency)
         return dependencies
 
+    def assumes_not_supported(self, name: str) -> None:
+        """Negate the given mechanism for automatic dependency generation.
+
+        `name` can be either a dependency symbol (``PSA_WANT_xxx``) or
+        a mechanism name (``PSA_KEY_TYPE_xxx``, etc.).
+
+        Call this function before set_arguments() for a test case that should
+        run if the given mechanism is not supported.
+
+        Call modifiers such as set_key_bits() and set_key_pair_usage() before
+        calling this method, if applicable.
+
+        A mechanism is a PSA_XXX symbol, e.g. PSA_KEY_TYPE_AES, PSA_ALG_HMAC,
+        etc. For mechanisms like ECC curves where the support status includes
+        the key bit-size, this class assumes that only one bit-size is
+        involved in a given test case.
+        """
+        if name.startswith('PSA_WANT_'):
+            self.negated_dependencies.add(name)
+            return
+        if name == 'PSA_KEY_TYPE_RSA_KEY_PAIR' and \
+           self.key_bits is not None and \
+           self.key_pair_usage == ['GENERATE']:
+            # When RSA key pair generation is not supported, it could be
+            # due to the specific key size is out of range, or because
+            # RSA key pair generation itself is not supported. Assume the
+            # latter.
+            dep = psa_information.psa_want_symbol(name, prefix=self.dependency_prefix)
+
+            self.negated_dependencies.add(dep + '_GENERATE')
+            return
+        dependencies = self.infer_dependencies([name])
+        # * If we have more than one dependency to negate, the result would
+        #   say that all of the dependencies are disabled, which is not
+        #   a desirable outcome: the negation of (A and B) is (!A or !B),
+        #   not (!A and !B).
+        # * If we have no dependency to negate, the result wouldn't be a
+        #   not-supported case.
+        # Assert that we don't reach either such case.
+        assert len(dependencies) == 1
+        self.negated_dependencies.add(dependencies[0])
+
     def set_arguments(self, arguments: List[str]) -> None:
         """Set test case arguments and automatically infer dependencies."""
         super().set_arguments(arguments)
         dependencies = self.infer_dependencies(arguments)
+        for i in range(len(dependencies)): #pylint: disable=consider-using-enumerate
+            if dependencies[i] in self.negated_dependencies:
+                dependencies[i] = '!' + dependencies[i]
         self.skip_if_any_not_implemented(dependencies)
         self.automatic_dependencies.update(dependencies)