From fa70ced195f367053bc140296688670e1e149c18 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Mar 2022 12:52:24 +0100 Subject: [PATCH 01/29] Remove ad hoc is_valid_for_signature method Use the new generic is_public method. Impact on generated cases: there are new HMAC test cases for SIGN_HASH. It was a bug that these test cases were previously not generated. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 17 +---------------- tests/scripts/generate_psa_tests.py | 8 +++++--- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 1bd011fe21..82487d97fa 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -20,7 +20,7 @@ This module is entirely based on the PSA API. import enum import re -from typing import Dict, Iterable, Optional, Pattern, Tuple +from typing import Iterable, Optional, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA @@ -178,21 +178,6 @@ class KeyType: return b''.join([self.DATA_BLOCK] * (length // len(self.DATA_BLOCK)) + [self.DATA_BLOCK[:length % len(self.DATA_BLOCK)]]) - KEY_TYPE_FOR_SIGNATURE = { - 'PSA_KEY_USAGE_SIGN_HASH': re.compile('.*KEY_PAIR'), - 'PSA_KEY_USAGE_VERIFY_HASH': re.compile('.*KEY.*') - } #type: Dict[str, Pattern] - """Use a regexp to determine key types for which signature is possible - when using the actual usage flag. - """ - def is_valid_for_signature(self, usage: str) -> bool: - """Determine if the key type is compatible with the specified - signitute type. - - """ - # This is just temporaly solution for the implicit usage flags. - return re.match(self.KEY_TYPE_FOR_SIGNATURE[usage], self.name) is not None - def can_do(self, alg: 'Algorithm') -> bool: """Whether this key type can be used for operations with the given algorithm. diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index ca94d7d324..d25af22b27 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -206,7 +206,7 @@ class NotSupported: continue # For public key we expect that key generation fails with # INVALID_ARGUMENT. It is handled by KeyGenerate class. - if not kt.name.endswith('_PUBLIC_KEY'): + if not kt.is_public(): yield test_case_for_key_type_not_supported( 'generate', kt.expression, bits, finish_family_dependencies(generate_dependencies, bits), @@ -822,8 +822,10 @@ class StorageFormatV0(StorageFormat): for key_type in sorted(alg_with_keys[alg]): # The key types must be filtered to fit the specific usage flag. kt = crypto_knowledge.KeyType(key_type) - if kt.is_valid_for_signature(usage): - yield self.keys_for_implicit_usage(usage, alg, kt) + if kt.is_public() and '_SIGN_' in usage: + # Can't sign with a public key + continue + yield self.keys_for_implicit_usage(usage, alg, kt) def generate_all_keys(self) -> Iterator[StorageTestData]: yield from super().generate_all_keys() From 7de7c1020c6db6ed94450b995e53f40f085af90a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 22:28:07 +0200 Subject: [PATCH 02/29] Storage format tests: cover algorithms for each key type In the generated storage format test cases, cover all supported algorithms for each key type. This is a step towards exercising the key with all the algorithms it supports; a subsequent commit will generate a policy that permits the specified algorithms. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 78 ++++++++++++++++++----------- 1 file changed, 50 insertions(+), 28 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index d25af22b27..82008ce211 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -623,46 +623,68 @@ class StorageFormat: yield from self.generate_keys_for_usage_flags() yield from self.generate_key_for_all_usage_flags() + def key_for_type_and_alg( + self, + kt: crypto_knowledge.KeyType, + bits: int, + alg: Optional[crypto_knowledge.Algorithm] = None, + ) -> StorageTestData: + """Construct a test key of the given type. + + If alg is not None, this key allows it. + """ + usage_flags = 'PSA_KEY_USAGE_EXPORT' + alg1 = 0 if alg is None else alg.expression #type: psa_storage.Exprable + alg2 = 0 + key_material = kt.key_material(bits) + short_expression = re.sub(r'\bPSA_(?:KEY_TYPE|ECC_FAMILY)_', + r'', + kt.expression) + description = 'type: {} {}-bit'.format(short_expression, bits) + if alg is not None: + description += ', ' + re.sub(r'PSA_ALG_', r'', alg.expression) + key = StorageTestData(version=self.version, + id=1, lifetime=0x00000001, + type=kt.expression, bits=bits, + usage=usage_flags, alg=alg1, alg2=alg2, + material=key_material, + description=description) + return key + def keys_for_type( self, key_type: str, - params: Optional[Iterable[str]] = None + all_algorithms: List[crypto_knowledge.Algorithm], ) -> Iterator[StorageTestData]: - """Generate test keys for the given key type. - - For key types that depend on a parameter (e.g. elliptic curve family), - `param` is the parameter to pass to the constructor. Only a single - parameter is supported. - """ - kt = crypto_knowledge.KeyType(key_type, params) + """Generate test keys for the given key type.""" + kt = crypto_knowledge.KeyType(key_type) for bits in kt.sizes_to_test(): - usage_flags = 'PSA_KEY_USAGE_EXPORT' - alg = 0 - alg2 = 0 - key_material = kt.key_material(bits) - short_expression = re.sub(r'\bPSA_(?:KEY_TYPE|ECC_FAMILY)_', - r'', - kt.expression) - description = 'type: {} {}-bit'.format(short_expression, bits) - key = StorageTestData(version=self.version, - id=1, lifetime=0x00000001, - type=kt.expression, bits=bits, - usage=usage_flags, alg=alg, alg2=alg2, - material=key_material, - description=description) - yield key + # Test a non-exercisable key, as well as exercisable keys for + # each compatible algorithm. + # To do: test reading a key from storage with an incompatible + # or unsupported algorithm. + yield self.key_for_type_and_alg(kt, bits) + compatible_algorithms = [alg for alg in all_algorithms + if kt.can_do(alg)] + for alg in compatible_algorithms: + yield self.key_for_type_and_alg(kt, bits, alg) def all_keys_for_types(self) -> Iterator[StorageTestData]: """Generate test keys covering key types and their representations.""" key_types = sorted(self.constructors.key_types) + all_algorithms = [crypto_knowledge.Algorithm(alg) + for alg in self.constructors.generate_expressions( + sorted(self.constructors.algorithms) + )] for key_type in self.constructors.generate_expressions(key_types): - yield from self.keys_for_type(key_type) + yield from self.keys_for_type(key_type, all_algorithms) def keys_for_algorithm(self, alg: str) -> Iterator[StorageTestData]: - """Generate test keys for the specified algorithm.""" - # For now, we don't have information on the compatibility of key - # types and algorithms. So we just test the encoding of algorithms, - # and not that operations can be performed with them. + """Generate test keys for the encoding of the specified algorithm.""" + # These test cases only validate the encoding of algorithms, not + # whether the key read from storage is suitable for an operation. + # `keys_for_types` generate read tests with an algorithm and a + # compatible key. descr = re.sub(r'PSA_ALG_', r'', alg) descr = re.sub(r',', r', ', re.sub(r' +', r'', descr)) usage = 'PSA_KEY_USAGE_EXPORT' From 564fae8328c3897c65410a840170f040d353c8b6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Mar 2022 22:32:59 +0100 Subject: [PATCH 03/29] Refactor usage flag formatting and implication When generating storage format tests, pass usage flags around as a list, and format them as the last thing. In Storagekey(), simplify the addition of implicit usage flags: this no longer requires parsing. The output is unchanged. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 57 +++++++++++++++++------------ 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 82008ce211..4875cf9e5c 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -456,7 +456,7 @@ class StorageKey(psa_storage.Key): def __init__( self, - usage: str, + usage: Iterable[str], without_implicit_usage: Optional[bool] = False, **kwargs ) -> None: @@ -465,13 +465,16 @@ class StorageKey(psa_storage.Key): * `usage` : The usage flags used for the key. * `without_implicit_usage`: Flag to defide to apply the usage extension """ - super().__init__(usage=usage, **kwargs) - + usage_flags = set(usage) if not without_implicit_usage: - for flag, implicit in self.IMPLICIT_USAGE_FLAGS.items(): - if self.usage.value() & psa_storage.Expr(flag).value() and \ - self.usage.value() & psa_storage.Expr(implicit).value() == 0: - self.usage = psa_storage.Expr(self.usage.string + ' | ' + implicit) + for flag in sorted(usage_flags): + if flag in self.IMPLICIT_USAGE_FLAGS: + usage_flags.add(self.IMPLICIT_USAGE_FLAGS[flag]) + if usage_flags: + usage_expression = ' | '.join(sorted(usage_flags)) + else: + usage_expression = '0' + super().__init__(usage=usage_expression, **kwargs) class StorageTestData(StorageKey): """Representation of test case data for storage format testing.""" @@ -479,7 +482,7 @@ class StorageTestData(StorageKey): def __init__( self, description: str, - expected_usage: Optional[str] = None, + expected_usage: Optional[List[str]] = None, **kwargs ) -> None: """Prepare to generate test data @@ -491,7 +494,12 @@ class StorageTestData(StorageKey): """ super().__init__(**kwargs) self.description = description #type: str - self.expected_usage = expected_usage if expected_usage else self.usage.string #type: str + if expected_usage is None: + self.expected_usage = self.usage #type: psa_storage.Expr + elif expected_usage: + self.expected_usage = psa_storage.Expr(' | '.join(expected_usage)) + else: + self.expected_usage = psa_storage.Expr(0) class StorageFormat: """Storage format stability test cases.""" @@ -524,7 +532,7 @@ class StorageFormat: tc.set_description('PSA storage {}: {}'.format(verb, key.description)) dependencies = automatic_dependencies( key.lifetime.string, key.type.string, - key.expected_usage, key.alg.string, key.alg2.string, + key.alg.string, key.alg2.string, ) dependencies = finish_family_dependencies(dependencies, key.bits) tc.set_dependencies(dependencies) @@ -545,7 +553,8 @@ class StorageFormat: extra_arguments = [' | '.join(flags) if flags else '0'] tc.set_arguments([key.lifetime.string, key.type.string, str(key.bits), - key.expected_usage, key.alg.string, key.alg2.string, + key.expected_usage.string, + key.alg.string, key.alg2.string, '"' + key.material.hex() + '"', '"' + key.hex() + '"', *extra_arguments]) @@ -564,7 +573,7 @@ class StorageFormat: key = StorageTestData(version=self.version, id=1, lifetime=lifetime, type='PSA_KEY_TYPE_RAW_DATA', bits=8, - usage='PSA_KEY_USAGE_EXPORT', alg=0, alg2=0, + usage=['PSA_KEY_USAGE_EXPORT'], alg=0, alg2=0, material=b'L', description=description) return key @@ -590,19 +599,21 @@ class StorageFormat: test_implicit_usage: Optional[bool] = True ) -> StorageTestData: """Construct a test key for the given key usage.""" - usage = ' | '.join(usage_flags) if usage_flags else '0' - if short is None: - short = re.sub(r'\bPSA_KEY_USAGE_', r'', usage) extra_desc = ' without implication' if test_implicit_usage else '' - description = 'usage' + extra_desc + ': ' + short + description = 'usage' + extra_desc + ': ' key1 = StorageTestData(version=self.version, id=1, lifetime=0x00000001, type='PSA_KEY_TYPE_RAW_DATA', bits=8, - expected_usage=usage, + expected_usage=usage_flags, without_implicit_usage=not test_implicit_usage, - usage=usage, alg=0, alg2=0, + usage=usage_flags, alg=0, alg2=0, material=b'K', description=description) + if short is None: + key1.description += re.sub(r'\bPSA_KEY_USAGE_', r'', + key1.expected_usage.string) + else: + key1.description += short return key1 def generate_keys_for_usage_flags(self, **kwargs) -> Iterator[StorageTestData]: @@ -633,7 +644,7 @@ class StorageFormat: If alg is not None, this key allows it. """ - usage_flags = 'PSA_KEY_USAGE_EXPORT' + usage_flags = ['PSA_KEY_USAGE_EXPORT'] alg1 = 0 if alg is None else alg.expression #type: psa_storage.Exprable alg2 = 0 key_material = kt.key_material(bits) @@ -687,7 +698,7 @@ class StorageFormat: # compatible key. descr = re.sub(r'PSA_ALG_', r'', alg) descr = re.sub(r',', r', ', re.sub(r' +', r'', descr)) - usage = 'PSA_KEY_USAGE_EXPORT' + usage = ['PSA_KEY_USAGE_EXPORT'] key1 = StorageTestData(version=self.version, id=1, lifetime=0x00000001, type='PSA_KEY_TYPE_RAW_DATA', bits=8, @@ -760,9 +771,9 @@ class StorageFormatV0(StorageFormat): """ bits = key_type.sizes_to_test()[0] implicit_usage = StorageKey.IMPLICIT_USAGE_FLAGS[implyer_usage] - usage_flags = 'PSA_KEY_USAGE_EXPORT' - material_usage_flags = usage_flags + ' | ' + implyer_usage - expected_usage_flags = material_usage_flags + ' | ' + implicit_usage + usage_flags = ['PSA_KEY_USAGE_EXPORT'] + material_usage_flags = usage_flags + [implyer_usage] + expected_usage_flags = material_usage_flags + [implicit_usage] alg2 = 0 key_material = key_type.key_material(bits) usage_expression = re.sub(r'PSA_KEY_USAGE_', r'', implyer_usage) From e8e058c9d4fa5d4aacf90d8a4cff8d7232a5a206 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Mar 2022 23:42:25 +0100 Subject: [PATCH 04/29] Unify the code to shorten expressions The output of generate_psa_tests.py is almost unchanged: the differences are only spaces after commas (now consistently omitted). Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 29 ++++++++++++++++++++++ tests/scripts/generate_psa_tests.py | 33 ++++++++++--------------- 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 82487d97fa..f2775265cc 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -25,6 +25,20 @@ from typing import Iterable, Optional, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA +def short_expression(original: str) -> str: + """Abbreviate the expression, keeping it human-readable. + + If `level` is 0, just remove parts that are implicit from context, + such as a leading ``PSA_KEY_TYPE_``. + For larger values of `level`, also abbreviate some names in an + unambiguous, but ad hoc way. + """ + short = original + short = re.sub(r'\bPSA_(?:ALG|ECC_FAMILY|KEY_[A-Z]+)_', r'', short) + short = re.sub(r' +', r'', short) + return short + + BLOCK_CIPHERS = frozenset(['AES', 'ARIA', 'CAMELLIA', 'DES']) BLOCK_MAC_MODES = frozenset(['CBC_MAC', 'CMAC']) BLOCK_CIPHER_MODES = frozenset([ @@ -104,6 +118,13 @@ class KeyType: `self.name`. """ + def short_expression(self) -> str: + """Abbreviate the expression, keeping it human-readable. + + See `crypto_knowledge.short_expression`. + """ + return short_expression(self.expression) + def is_public(self) -> bool: """Whether the key type is for public keys.""" return self.name.endswith('_PUBLIC_KEY') @@ -376,6 +397,14 @@ class Algorithm: # Assume kdf_alg is either a valid KDF or 0. return not re.match(r'(?:0[Xx])?0+\s*\Z', kdf_alg) + + def short_expression(self) -> str: + """Abbreviate the expression, keeping it human-readable. + + See `crypto_knowledge.short_expression`. + """ + return short_expression(self.expression) + def can_do(self, category: AlgorithmCategory) -> bool: """Whether this algorithm fits the specified operation category.""" if category == self.category: diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 4875cf9e5c..68cd714f56 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -145,7 +145,7 @@ def test_case_for_key_type_not_supported( """ hack_dependencies_not_implemented(dependencies) tc = test_case.TestCase() - short_key_type = re.sub(r'PSA_(KEY_TYPE|ECC_FAMILY)_', r'', key_type) + short_key_type = crypto_knowledge.short_expression(key_type) adverb = 'not' if dependencies else 'never' if param_descr: adverb = param_descr + ' ' + adverb @@ -243,7 +243,7 @@ def test_case_for_key_generation( """ hack_dependencies_not_implemented(dependencies) tc = test_case.TestCase() - short_key_type = re.sub(r'PSA_(KEY_TYPE|ECC_FAMILY)_', r'', key_type) + short_key_type = crypto_knowledge.short_expression(key_type) tc.set_description('PSA {} {}-bit' .format(short_key_type, bits)) tc.set_dependencies(dependencies) @@ -335,7 +335,7 @@ class OpFail: """Construct a failure test case for a one-key or keyless operation.""" #pylint: disable=too-many-arguments,too-many-locals tc = test_case.TestCase() - pretty_alg = re.sub(r'PSA_ALG_', r'', alg.expression) + 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] @@ -344,7 +344,7 @@ class OpFail: pretty_reason = reason.name.lower() if kt: key_type = kt.expression - pretty_type = re.sub(r'PSA_KEY_TYPE_', r'', key_type) + pretty_type = kt.short_expression() else: key_type = '' pretty_type = '' @@ -568,7 +568,7 @@ class StorageFormat: short = lifetime short = re.sub(r'PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION', r'', short) - short = re.sub(r'PSA_KEY_[A-Z]+_', r'', short) + short = crypto_knowledge.short_expression(short) description = 'lifetime: ' + short key = StorageTestData(version=self.version, id=1, lifetime=lifetime, @@ -610,8 +610,8 @@ class StorageFormat: material=b'K', description=description) if short is None: - key1.description += re.sub(r'\bPSA_KEY_USAGE_', r'', - key1.expected_usage.string) + usage_expr = key1.expected_usage.string + key1.description += crypto_knowledge.short_expression(usage_expr) else: key1.description += short return key1 @@ -648,12 +648,9 @@ class StorageFormat: alg1 = 0 if alg is None else alg.expression #type: psa_storage.Exprable alg2 = 0 key_material = kt.key_material(bits) - short_expression = re.sub(r'\bPSA_(?:KEY_TYPE|ECC_FAMILY)_', - r'', - kt.expression) - description = 'type: {} {}-bit'.format(short_expression, bits) + description = 'type: {} {}-bit'.format(kt.short_expression(), bits) if alg is not None: - description += ', ' + re.sub(r'PSA_ALG_', r'', alg.expression) + description += ', ' + alg.short_expression() key = StorageTestData(version=self.version, id=1, lifetime=0x00000001, type=kt.expression, bits=bits, @@ -696,8 +693,7 @@ class StorageFormat: # whether the key read from storage is suitable for an operation. # `keys_for_types` generate read tests with an algorithm and a # compatible key. - descr = re.sub(r'PSA_ALG_', r'', alg) - descr = re.sub(r',', r', ', re.sub(r' +', r'', descr)) + descr = crypto_knowledge.short_expression(alg) usage = ['PSA_KEY_USAGE_EXPORT'] key1 = StorageTestData(version=self.version, id=1, lifetime=0x00000001, @@ -776,12 +772,9 @@ class StorageFormatV0(StorageFormat): expected_usage_flags = material_usage_flags + [implicit_usage] alg2 = 0 key_material = key_type.key_material(bits) - usage_expression = re.sub(r'PSA_KEY_USAGE_', r'', implyer_usage) - alg_expression = re.sub(r'PSA_ALG_', r'', alg) - alg_expression = re.sub(r',', r', ', re.sub(r' +', r'', alg_expression)) - key_type_expression = re.sub(r'\bPSA_(?:KEY_TYPE|ECC_FAMILY)_', - r'', - key_type.expression) + usage_expression = crypto_knowledge.short_expression(implyer_usage) + alg_expression = crypto_knowledge.short_expression(alg) + key_type_expression = key_type.short_expression() description = 'implied by {}: {} {} {}-bit'.format( usage_expression, alg_expression, key_type_expression, bits) key = StorageTestData(version=self.version, From 16b2506e3de25fb1972bf515408f59a0d65db056 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Mar 2022 00:02:15 +0100 Subject: [PATCH 05/29] Abbreviate descriptions of generated PSA storage tests This currently makes all the descriptions unambiguous even when truncated at 66 characters, as the unit test framework does. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 19 ++++++++++++++----- tests/scripts/generate_psa_tests.py | 14 +++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index f2775265cc..434ecf33e8 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -25,7 +25,7 @@ from typing import Iterable, Optional, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA -def short_expression(original: str) -> str: +def short_expression(original: str, level: int = 0) -> str: """Abbreviate the expression, keeping it human-readable. If `level` is 0, just remove parts that are implicit from context, @@ -36,6 +36,15 @@ def short_expression(original: str) -> str: short = original short = re.sub(r'\bPSA_(?:ALG|ECC_FAMILY|KEY_[A-Z]+)_', r'', short) short = re.sub(r' +', r'', short) + if level >= 1: + short = re.sub(r'PUBLIC_KEY\b', r'PUB', short) + short = re.sub(r'KEY_PAIR\b', r'PAIR', short) + short = re.sub(r'\bBRAINPOOL_P', r'BP', short) + short = re.sub(r'\bMONTGOMERY\b', r'MGM', short) + short = re.sub(r'AEAD_WITH_SHORTENED_TAG\b', r'AEAD_SHORT', short) + short = re.sub(r'\bDETERMINISTIC_', r'DET_', short) + short = re.sub(r'\bKEY_AGREEMENT\b', r'KA', short) + short = re.sub(r'_PSK_TO_MS\b', r'_PSK2MS', short) return short @@ -118,12 +127,12 @@ class KeyType: `self.name`. """ - def short_expression(self) -> str: + def short_expression(self, level: int = 0) -> str: """Abbreviate the expression, keeping it human-readable. See `crypto_knowledge.short_expression`. """ - return short_expression(self.expression) + return short_expression(self.expression, level=level) def is_public(self) -> bool: """Whether the key type is for public keys.""" @@ -398,12 +407,12 @@ class Algorithm: return not re.match(r'(?:0[Xx])?0+\s*\Z', kdf_alg) - def short_expression(self) -> str: + def short_expression(self, level: int = 0) -> str: """Abbreviate the expression, keeping it human-readable. See `crypto_knowledge.short_expression`. """ - return short_expression(self.expression) + return short_expression(self.expression, level=level) def can_do(self, category: AlgorithmCategory) -> bool: """Whether this algorithm fits the specified operation category.""" diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 68cd714f56..2a70147055 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -529,7 +529,7 @@ class StorageFormat: """ verb = 'save' if self.forward else 'read' tc = test_case.TestCase() - tc.set_description('PSA storage {}: {}'.format(verb, key.description)) + tc.set_description(verb + ' ' + key.description) dependencies = automatic_dependencies( key.lifetime.string, key.type.string, key.alg.string, key.alg2.string, @@ -648,9 +648,9 @@ class StorageFormat: alg1 = 0 if alg is None else alg.expression #type: psa_storage.Exprable alg2 = 0 key_material = kt.key_material(bits) - description = 'type: {} {}-bit'.format(kt.short_expression(), bits) + description = 'type: {} {}-bit'.format(kt.short_expression(1), bits) if alg is not None: - description += ', ' + alg.short_expression() + description += ', ' + alg.short_expression(1) key = StorageTestData(version=self.version, id=1, lifetime=0x00000001, type=kt.expression, bits=bits, @@ -693,7 +693,7 @@ class StorageFormat: # whether the key read from storage is suitable for an operation. # `keys_for_types` generate read tests with an algorithm and a # compatible key. - descr = crypto_knowledge.short_expression(alg) + descr = crypto_knowledge.short_expression(alg, 1) usage = ['PSA_KEY_USAGE_EXPORT'] key1 = StorageTestData(version=self.version, id=1, lifetime=0x00000001, @@ -772,9 +772,9 @@ class StorageFormatV0(StorageFormat): expected_usage_flags = material_usage_flags + [implicit_usage] alg2 = 0 key_material = key_type.key_material(bits) - usage_expression = crypto_knowledge.short_expression(implyer_usage) - alg_expression = crypto_knowledge.short_expression(alg) - key_type_expression = key_type.short_expression() + usage_expression = crypto_knowledge.short_expression(implyer_usage, 1) + alg_expression = crypto_knowledge.short_expression(alg, 1) + key_type_expression = key_type.short_expression(1) description = 'implied by {}: {} {} {}-bit'.format( usage_expression, alg_expression, key_type_expression, bits) key = StorageTestData(version=self.version, From e6b85b4d427f3605e676d18d81297920bef96cff Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Mar 2022 09:58:09 +0100 Subject: [PATCH 06/29] Storage format tests: exercise operations with keys In key read tests, add usage flags that are suitable for the key type and algorithm. This way, the call to exercise_key() in the test not only checks that exporting the key is possible, but also that operations on the key are possible. This triggers a number of failures in edge cases where the generator generates combinations that are not valid, which will be fixed in subsequent commits. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 30 ++++++++++++++++++++++++- tests/scripts/generate_psa_tests.py | 5 ++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 434ecf33e8..6f499403a0 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -20,7 +20,7 @@ This module is entirely based on the PSA API. import enum import re -from typing import Iterable, Optional, Tuple +from typing import Iterable, List, Optional, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA @@ -422,3 +422,31 @@ class Algorithm: self.is_key_agreement_with_derivation(): return True return False + + def usage_flags(self, public: bool = False) -> List[str]: + """The list of usage flags describing operations that can perform this algorithm. + + If public is true, only return public-key operations, not private-key operations. + """ + if self.category == AlgorithmCategory.HASH: + flags = [] + elif self.category == AlgorithmCategory.MAC: + flags = ['SIGN_HASH', 'SIGN_MESSAGE', + 'VERIFY_HASH', 'VERIFY_MESSAGE'] + elif self.category == AlgorithmCategory.CIPHER or \ + self.category == AlgorithmCategory.AEAD: + flags = ['DECRYPT', 'ENCRYPT'] + elif self.category == AlgorithmCategory.SIGN: + flags = ['VERIFY_HASH', 'VERIFY_MESSAGE'] + if not public: + flags += ['SIGN_HASH', 'SIGN_MESSAGE'] + elif self.category == AlgorithmCategory.ASYMMETRIC_ENCRYPTION: + flags = ['ENCRYPT'] + if not public: + flags += ['DECRYPT'] + elif self.category == AlgorithmCategory.KEY_DERIVATION or \ + self.category == AlgorithmCategory.KEY_AGREEMENT: + flags = ['DERIVE'] + else: + raise AlgorithmNotRecognized(self.expression) + return ['PSA_KEY_USAGE_' + flag for flag in flags] diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 2a70147055..27ee52d6bb 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -645,8 +645,11 @@ class StorageFormat: If alg is not None, this key allows it. """ usage_flags = ['PSA_KEY_USAGE_EXPORT'] - alg1 = 0 if alg is None else alg.expression #type: psa_storage.Exprable + alg1 = 0 #type: psa_storage.Exprable alg2 = 0 + if alg is not None: + alg1 = alg.expression + usage_flags += alg.usage_flags(public=kt.is_public()) key_material = kt.key_material(bits) description = 'type: {} {}-bit'.format(kt.short_expression(1), bits) if alg is not None: From c47d3a42866b0e150c4dd6c7c4d0868c0f1bda03 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Mar 2022 10:18:58 +0100 Subject: [PATCH 07/29] 64-bit block ciphers are incompatible with some modes Only allow selected modes with 64-bit block ciphers (i.e. DES). This removes some storage tests and creates corresponding op_fail tests. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 6f499403a0..bd4d94ee82 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -218,6 +218,12 @@ class KeyType: return False if self.head == 'HMAC' and alg.head == 'HMAC': return True + if self.head == 'DES': + # 64-bit block ciphers only allow a reduced set of modes. + return alg.head in [ + 'CBC_NO_PADDING', 'CBC_PKCS7', + 'ECB_NO_PADDING', + ] if self.head in BLOCK_CIPHERS and \ alg.head in frozenset.union(BLOCK_MAC_MODES, BLOCK_CIPHER_MODES, From bbf452c689ceb544017ca5d0b41ae8d9fddb40d6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Mar 2022 18:40:47 +0100 Subject: [PATCH 08/29] exercise_key: support modes where IV length is not 16 Support ECB, which has no IV. The code also now supports arbitrary IV lengths based on the algorithm and key type. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 71a1b78740..b6126477a9 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -166,18 +166,27 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; unsigned char iv[16] = {0}; size_t iv_length = sizeof( iv ); + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_type_t key_type; const unsigned char plaintext[16] = "Hello, world..."; unsigned char ciphertext[32] = "(wabblewebblewibblewobblewubble)"; size_t ciphertext_length = sizeof( ciphertext ); unsigned char decrypted[sizeof( ciphertext )]; size_t part_length; + PSA_ASSERT( psa_get_key_attributes( key, &attributes ) ); + key_type = psa_get_key_type( &attributes ); + iv_length = PSA_CIPHER_IV_LENGTH( key_type, alg ); + if( usage & PSA_KEY_USAGE_ENCRYPT ) { PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); - PSA_ASSERT( psa_cipher_generate_iv( &operation, - iv, sizeof( iv ), - &iv_length ) ); + if( PSA_CIPHER_IV_LENGTH( key_type, alg ) != 0 ) + { + PSA_ASSERT( psa_cipher_generate_iv( &operation, + iv, sizeof( iv ), + &iv_length ) ); + } PSA_ASSERT( psa_cipher_update( &operation, plaintext, sizeof( plaintext ), ciphertext, sizeof( ciphertext ), @@ -195,18 +204,14 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, int maybe_invalid_padding = 0; if( ! ( usage & PSA_KEY_USAGE_ENCRYPT ) ) { - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - PSA_ASSERT( psa_get_key_attributes( key, &attributes ) ); - /* This should be PSA_CIPHER_GET_IV_SIZE but the API doesn't - * have this macro yet. */ - iv_length = PSA_BLOCK_CIPHER_BLOCK_LENGTH( - psa_get_key_type( &attributes ) ); maybe_invalid_padding = ! PSA_ALG_IS_STREAM_CIPHER( alg ); - psa_reset_key_attributes( &attributes ); } PSA_ASSERT( psa_cipher_decrypt_setup( &operation, key, alg ) ); - PSA_ASSERT( psa_cipher_set_iv( &operation, - iv, iv_length ) ); + if( iv_length != 0 ) + { + PSA_ASSERT( psa_cipher_set_iv( &operation, + iv, iv_length ) ); + } PSA_ASSERT( psa_cipher_update( &operation, ciphertext, ciphertext_length, decrypted, sizeof( decrypted ), @@ -229,6 +234,7 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, exit: psa_cipher_abort( &operation ); + psa_reset_key_attributes( &attributes ); return( 0 ); } From d78c59c0de95e77d6b747989ea6cd426d98cc78d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Mar 2022 18:46:00 +0100 Subject: [PATCH 09/29] Test more truncated MAC and short AEAD tag lengths The current macro collector only tried the minimum and maximum expressible lengths for PSA_ALG_TRUNCATED_MAC and PSA_ALG_AEAD_WITH_DEFAULT_LENGTH_TAG. This was good enough for psa_constant_names, but it's weak for exercising keys, in particular because it doesn't include any valid AEAD tag length. So cover more lengths. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/macro_collector.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/scripts/mbedtls_dev/macro_collector.py b/scripts/mbedtls_dev/macro_collector.py index 987779d0e1..218a718f31 100644 --- a/scripts/mbedtls_dev/macro_collector.py +++ b/scripts/mbedtls_dev/macro_collector.py @@ -400,10 +400,26 @@ enumerate 'other_algorithm': [], 'lifetime': [self.lifetimes], } #type: Dict[str, List[Set[str]]] - self.arguments_for['mac_length'] += ['1', '63'] - self.arguments_for['min_mac_length'] += ['1', '63'] - self.arguments_for['tag_length'] += ['1', '63'] - self.arguments_for['min_tag_length'] += ['1', '63'] + mac_lengths = [str(n) for n in [ + 1, # minimum expressible + 4, # minimum allowed by policy + 13, # an odd size in a plausible range + 14, # an even non-power-of-two size in a plausible range + 16, # same as full size for at least one algorithm + 63, # maximum expressible + ]] + self.arguments_for['mac_length'] += mac_lengths + self.arguments_for['min_mac_length'] += mac_lengths + aead_lengths = [str(n) for n in [ + 1, # minimum expressible + 4, # minimum allowed by policy + 13, # an odd size in a plausible range + 14, # an even non-power-of-two size in a plausible range + 16, # same as full size for at least one algorithm + 63, # maximum expressible + ]] + self.arguments_for['tag_length'] += aead_lengths + self.arguments_for['min_tag_length'] += aead_lengths def add_numerical_values(self) -> None: """Add numerical values that are not supported to the known identifiers.""" From 2fa829c7ddfec3c249945018ff840db95b30608b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 10:36:07 +0100 Subject: [PATCH 10/29] Fix invalid argument enumeration when there are >=3 arguments This bug had no impact since currently no macro has more than 2 arguments. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/macro_collector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/mbedtls_dev/macro_collector.py b/scripts/mbedtls_dev/macro_collector.py index 218a718f31..3cad2a3f62 100644 --- a/scripts/mbedtls_dev/macro_collector.py +++ b/scripts/mbedtls_dev/macro_collector.py @@ -186,7 +186,7 @@ class PSAMacroEnumerator: for value in argument_lists[i][1:]: arguments[i] = value yield self._format_arguments(name, arguments) - arguments[i] = argument_lists[0][0] + arguments[i] = argument_lists[i][0] except BaseException as e: raise Exception('distribute_arguments({})'.format(name)) from e From e3a0890e4f5b1122ee68063035c827e340d715e2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 10:37:33 +0100 Subject: [PATCH 11/29] Reject invalid MAC and AEAD truncations Reject algorithms of the form PSA_ALG_TRUNCATED_MAC(...) or PSA_ALG_AEAD_WITH_SHORTENED_TAG(...) when the truncation length is invalid or not accepted by policy in Mbed TLS. This is done in KeyType.can_do, so in generate_psa_tests.py, keys will be tested for operation failure with this algorithm if the algorithm is rejected, and for storage if the algorithm is accepted. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 53 ++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index bd4d94ee82..7d2e755f63 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -20,7 +20,7 @@ This module is entirely based on the PSA API. import enum import re -from typing import Iterable, List, Optional, Tuple +from typing import FrozenSet, Iterable, List, Optional, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA @@ -216,6 +216,8 @@ class KeyType: #pylint: disable=too-many-return-statements if alg.is_wildcard: return False + if alg.is_invalid_truncation(): + return False if self.head == 'HMAC' and alg.head == 'HMAC': return True if self.head == 'DES': @@ -420,8 +422,55 @@ class Algorithm: """ return short_expression(self.expression, level=level) + PERMITTED_TAG_LENGTHS = { + 'PSA_ALG_CCM': frozenset([4, 6, 8, 10, 12, 14, 16]), + 'PSA_ALG_CHACHA20_POLY1305': frozenset([16]), + 'PSA_ALG_GCM': frozenset([4, 8, 12, 13, 14, 15, 16]), + } + MAC_LENGTH = { + 'PSA_ALG_CBC_MAC': 16, + 'PSA_ALG_CMAC': 16, + 'PSA_ALG_HMAC(PSA_ALG_MD5)': 16, + 'PSA_ALG_HMAC(PSA_ALG_SHA_1)': 20, + } + HMAC_WITH_NOMINAL_LENGTH_RE = re.compile(r'PSA_ALG_HMAC\(\w+([0-9])+\)\Z') + @classmethod + def mac_or_tag_length(cls, base: str) -> FrozenSet[int]: + """Return the set of permitted lengths for the given MAC or AEAD tag.""" + if base in cls.PERMITTED_TAG_LENGTHS: + return cls.PERMITTED_TAG_LENGTHS[base] + max_length = cls.MAC_LENGTH.get(base, None) + if max_length is None: + m = cls.HMAC_WITH_NOMINAL_LENGTH_RE.match(base) + if m: + max_length = int(m.group(1)) // 8 + if max_length is None: + raise ValueError('Unknown permitted lengths for ' + base) + return frozenset(range(4, max_length + 1)) + + TRUNCATED_ALG_RE = re.compile( + r'(?PPSA_ALG_(?:AEAD_WITH_SHORTENED_TAG|TRUNCATED_MAC))' + r'\((?P.*),' + r'(?P0[Xx][0-9A-Fa-f]+|[1-9][0-9]*|0[0-9]*)[LUlu]*\)\Z') + def is_invalid_truncation(self) -> bool: + """False for a MAC or AEAD algorithm truncated to an invalid length. + + True for a MAC or AEAD algorithm truncated to a valid length or to + a length that cannot be determined. True for anything other than + a truncated MAC or AEAD. + """ + m = self.TRUNCATED_ALG_RE.match(self.expression) + if m: + base = m.group('base') + to_length = int(m.group('length'), 0) + permitted_lengths = self.mac_or_tag_length(base) + if to_length not in permitted_lengths: + return True + return False + def can_do(self, category: AlgorithmCategory) -> bool: - """Whether this algorithm fits the specified operation category.""" + """Whether this algorithm can perform operations in the given category. + """ if category == self.category: return True if category == AlgorithmCategory.KEY_DERIVATION and \ From 7095d47749cc043a9ac11966449fd72a2ae465e0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 10:49:43 +0100 Subject: [PATCH 12/29] Reject block cipher modes that are not implemented in Mbed TLS Mbed TLS doesn't support certain block cipher mode combinations. This limitation should probably be lifted, but for now, test them as unsupported. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 7d2e755f63..55eb54d596 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -230,6 +230,9 @@ class KeyType: alg.head in frozenset.union(BLOCK_MAC_MODES, BLOCK_CIPHER_MODES, BLOCK_AEAD_MODES): + if alg.head in ['CMAC', 'OFB'] and \ + self.head in ['ARIA', 'CAMELLIA']: + return False # not implemented in Mbed TLS return True if self.head == 'CHACHA20' and alg.head == 'CHACHA20_POLY1305': return True From 6d187afd8de8bcb79e29898b5a2950ade7bd650e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 10:56:13 +0100 Subject: [PATCH 13/29] psa_crypto does not support XTS The cipher module implements XTS, and the PSA API specifies XTS, but the PSA implementation does not support XTS. It requires double-size keys, which psa_crypto does not currently support. Signed-off-by: Gilles Peskine --- include/mbedtls/config_psa.h | 14 -------------- include/psa/crypto_builtin_primitives.h | 1 - include/psa/crypto_config.h | 4 +++- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/config_psa.h b/include/mbedtls/config_psa.h index f9171330d7..edf62827ea 100644 --- a/include/mbedtls/config_psa.h +++ b/include/mbedtls/config_psa.h @@ -260,7 +260,6 @@ extern "C" { #if (defined(PSA_WANT_ALG_CTR) && !defined(MBEDTLS_PSA_ACCEL_ALG_CTR)) || \ (defined(PSA_WANT_ALG_CFB) && !defined(MBEDTLS_PSA_ACCEL_ALG_CFB)) || \ (defined(PSA_WANT_ALG_OFB) && !defined(MBEDTLS_PSA_ACCEL_ALG_OFB)) || \ - (defined(PSA_WANT_ALG_XTS) && !defined(MBEDTLS_PSA_ACCEL_ALG_XTS)) || \ defined(PSA_WANT_ALG_ECB_NO_PADDING) || \ (defined(PSA_WANT_ALG_CBC_NO_PADDING) && \ !defined(MBEDTLS_PSA_ACCEL_ALG_CBC_NO_PADDING)) || \ @@ -382,14 +381,6 @@ extern "C" { #endif #endif /* PSA_WANT_ALG_OFB */ -#if defined(PSA_WANT_ALG_XTS) -#if !defined(MBEDTLS_PSA_ACCEL_ALG_XTS) || \ - defined(PSA_HAVE_SOFT_BLOCK_CIPHER) -#define MBEDTLS_PSA_BUILTIN_ALG_XTS 1 -#define MBEDTLS_CIPHER_MODE_XTS -#endif -#endif /* PSA_WANT_ALG_XTS */ - #if defined(PSA_WANT_ALG_ECB_NO_PADDING) && \ !defined(MBEDTLS_PSA_ACCEL_ALG_ECB_NO_PADDING) #define MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING 1 @@ -726,11 +717,6 @@ extern "C" { #define PSA_WANT_ALG_OFB 1 #endif -#if defined(MBEDTLS_CIPHER_MODE_XTS) -#define MBEDTLS_PSA_BUILTIN_ALG_XTS 1 -#define PSA_WANT_ALG_XTS 1 -#endif - #if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) #define MBEDTLS_PSA_BUILTIN_ECC_BRAINPOOL_P_R1_256 1 #define PSA_WANT_ECC_BRAINPOOL_P_R1_256 diff --git a/include/psa/crypto_builtin_primitives.h b/include/psa/crypto_builtin_primitives.h index d3cf33a9dd..35f3a8b30b 100644 --- a/include/psa/crypto_builtin_primitives.h +++ b/include/psa/crypto_builtin_primitives.h @@ -94,7 +94,6 @@ typedef struct defined(MBEDTLS_PSA_BUILTIN_ALG_CTR) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_CFB) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_OFB) || \ - defined(MBEDTLS_PSA_BUILTIN_ALG_XTS) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_CBC_NO_PADDING) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_CBC_PKCS7) diff --git a/include/psa/crypto_config.h b/include/psa/crypto_config.h index d290971f89..46cf1265f7 100644 --- a/include/psa/crypto_config.h +++ b/include/psa/crypto_config.h @@ -87,7 +87,9 @@ #define PSA_WANT_ALG_STREAM_CIPHER 1 #define PSA_WANT_ALG_TLS12_PRF 1 #define PSA_WANT_ALG_TLS12_PSK_TO_MS 1 -#define PSA_WANT_ALG_XTS 1 +/* PBKDF2-HMAC is not yet supported via the PSA API in Mbed TLS. + * Note: when adding support, also adjust include/mbedtls/config_psa.h */ +//#define PSA_WANT_ALG_XTS 1 #define PSA_WANT_ECC_BRAINPOOL_P_R1_256 1 #define PSA_WANT_ECC_BRAINPOOL_P_R1_384 1 From 7acb1980eea8f3d2e780a866004b1bc53d402ed2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 11:03:32 +0100 Subject: [PATCH 14/29] Use PSA_AEAD_NONCE_LENGTH when exercising AEAD keys Don't re-code the logic to determine a valid nonce length. This fixes exercise_key() for PSA_ALG_CHACHA20_POLY1305, which was trying to use a 16-byte nonce. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index b6126477a9..8a2207c017 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -243,7 +243,9 @@ static int exercise_aead_key( mbedtls_svc_key_id_t key, psa_algorithm_t alg ) { unsigned char nonce[16] = {0}; - size_t nonce_length = sizeof( nonce ); + size_t nonce_length; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_type_t key_type; unsigned char plaintext[16] = "Hello, world..."; unsigned char ciphertext[48] = "(wabblewebblewibblewobblewubble)"; size_t ciphertext_length = sizeof( ciphertext ); @@ -255,19 +257,9 @@ static int exercise_aead_key( mbedtls_svc_key_id_t key, alg = PSA_ALG_AEAD_WITH_SHORTENED_TAG( alg, PSA_ALG_AEAD_GET_TAG_LENGTH( alg ) ); } - /* Default IV length for AES-GCM is 12 bytes */ - if( PSA_ALG_AEAD_WITH_SHORTENED_TAG( alg, 0 ) == - PSA_ALG_AEAD_WITH_SHORTENED_TAG( PSA_ALG_GCM, 0 ) ) - { - nonce_length = 12; - } - - /* IV length for CCM needs to be between 7 and 13 bytes */ - if( PSA_ALG_AEAD_WITH_SHORTENED_TAG( alg, 0 ) == - PSA_ALG_AEAD_WITH_SHORTENED_TAG( PSA_ALG_CCM, 0 ) ) - { - nonce_length = 12; - } + PSA_ASSERT( psa_get_key_attributes( key, &attributes ) ); + key_type = psa_get_key_type( &attributes ); + nonce_length = PSA_AEAD_NONCE_LENGTH( key_type, alg ); if( usage & PSA_KEY_USAGE_ENCRYPT ) { @@ -297,6 +289,7 @@ static int exercise_aead_key( mbedtls_svc_key_id_t key, return( 1 ); exit: + psa_reset_key_attributes( &attributes ); return( 0 ); } From d586b82e127843dbf5e269c1e57255daab723150 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 11:15:41 +0100 Subject: [PATCH 15/29] exercise_key: signature: detect function/algorithm incompatibility Don't try to use {sign,verify}_message on algorithms that only support {sign_verify}_hash. Normally exercise_key() tries all usage that is supported by policy, however PSA_KEY_USAGE_{SIGN,VERIFY}_MESSAGE is implied by PSA_KEY_USAGE_{SIGN,VERIFY}_HASH so it's impossible for the test data to omit the _MESSAGE policies with hash-only algorithms. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 8a2207c017..db3651d917 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -293,6 +293,17 @@ exit: return( 0 ); } +static int can_sign_or_verify_message( psa_key_usage_t usage, + psa_algorithm_t alg ) +{ + /* Sign-the-unspecified-hash algorithms can only be used with + * {sign,verify}_hash, not with {sign,verify}_message. */ + if( alg == PSA_ALG_ECDSA_ANY || alg == PSA_ALG_RSA_PKCS1V15_SIGN_RAW ) + return( 0 ); + return( usage & ( PSA_KEY_USAGE_SIGN_MESSAGE | + PSA_KEY_USAGE_VERIFY_MESSAGE ) ); +} + static int exercise_signature_key( mbedtls_svc_key_id_t key, psa_key_usage_t usage, psa_algorithm_t alg ) @@ -343,7 +354,7 @@ static int exercise_signature_key( mbedtls_svc_key_id_t key, } } - if( usage & ( PSA_KEY_USAGE_SIGN_MESSAGE | PSA_KEY_USAGE_VERIFY_MESSAGE ) ) + if( can_sign_or_verify_message( usage, alg ) ) { unsigned char message[256] = "Hello, world..."; unsigned char signature[PSA_SIGNATURE_MAX_SIZE] = {0}; From 4bd90dc6b101ec185dbd19c04f743574ee6a1bfc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 12:09:13 +0100 Subject: [PATCH 16/29] Don't exercise OAEP with small key and large hash RSA-OAEP requires the key to be larger than a function of the hash size. Ideally such combinations would be detected as a key/algorithm incompatibility. However key/algorithm compatibility is currently tested between the key type and the algorithm without considering the key size, and this is inconvenient to change. So as a workaround, dispense OAEP-with-too-small-hash from exercising, without including it in the automatic operation-failure test generation. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 33 +++++++++++++++++-------- tests/scripts/generate_psa_tests.py | 29 +++++++++++++++++++++- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 55eb54d596..1491a844ad 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -213,7 +213,7 @@ class KeyType: This function does not currently handle key derivation or PAKE. """ - #pylint: disable=too-many-return-statements + #pylint: disable=too-many-branches,too-many-return-statements if alg.is_wildcard: return False if alg.is_invalid_truncation(): @@ -425,28 +425,41 @@ class Algorithm: """ return short_expression(self.expression, level=level) + HASH_LENGTH = { + 'PSA_ALG_MD5': 16, + 'PSA_ALG_SHA_1': 20, + } + HASH_LENGTH_BITS_RE = re.compile(r'([0-9]+)\Z') + @classmethod + def hash_length(cls, alg: str) -> int: + """The length of the given hash algorithm, in bytes.""" + if alg in cls.HASH_LENGTH: + return cls.HASH_LENGTH[alg] + m = cls.HASH_LENGTH_BITS_RE.search(alg) + if m: + return int(m.group(1)) // 8 + raise ValueError('Unknown hash length for ' + alg) + PERMITTED_TAG_LENGTHS = { 'PSA_ALG_CCM': frozenset([4, 6, 8, 10, 12, 14, 16]), 'PSA_ALG_CHACHA20_POLY1305': frozenset([16]), 'PSA_ALG_GCM': frozenset([4, 8, 12, 13, 14, 15, 16]), } MAC_LENGTH = { - 'PSA_ALG_CBC_MAC': 16, - 'PSA_ALG_CMAC': 16, - 'PSA_ALG_HMAC(PSA_ALG_MD5)': 16, - 'PSA_ALG_HMAC(PSA_ALG_SHA_1)': 20, + 'PSA_ALG_CBC_MAC': 16, # actually the block cipher length + 'PSA_ALG_CMAC': 16, # actually the block cipher length } - HMAC_WITH_NOMINAL_LENGTH_RE = re.compile(r'PSA_ALG_HMAC\(\w+([0-9])+\)\Z') + HMAC_RE = re.compile(r'PSA_ALG_HMAC\((.*)\)\Z') @classmethod - def mac_or_tag_length(cls, base: str) -> FrozenSet[int]: + def mac_or_tag_lengths(cls, base: str) -> FrozenSet[int]: """Return the set of permitted lengths for the given MAC or AEAD tag.""" if base in cls.PERMITTED_TAG_LENGTHS: return cls.PERMITTED_TAG_LENGTHS[base] max_length = cls.MAC_LENGTH.get(base, None) if max_length is None: - m = cls.HMAC_WITH_NOMINAL_LENGTH_RE.match(base) + m = cls.HMAC_RE.match(base) if m: - max_length = int(m.group(1)) // 8 + max_length = cls.hash_length(m.group(1)) if max_length is None: raise ValueError('Unknown permitted lengths for ' + base) return frozenset(range(4, max_length + 1)) @@ -466,7 +479,7 @@ class Algorithm: if m: base = m.group('base') to_length = int(m.group('length'), 0) - permitted_lengths = self.mac_or_tag_length(base) + permitted_lengths = self.mac_or_tag_lengths(base) if to_length not in permitted_lengths: return True return False diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 27ee52d6bb..6fd311e632 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -518,6 +518,32 @@ class StorageFormat: self.version = version #type: int self.forward = forward #type: bool + RSA_OAEP_RE = re.compile(r'PSA_ALG_RSA_OAEP\((.*)\)\Z') + @classmethod + def valid_key_size_for_algorithm( + cls, + key_type: psa_storage.Expr, bits: int, + alg: psa_storage.Expr + ) -> bool: + """Whether the given key type and size are valid for the algorithm. + + Normally only the type and algorithm matter for compatibility, and + this is handled in crypto_knowledge.KeyType.can_do(). This function + exists to detect exceptional cases. Exceptional cases detected here + are not tested in OpFail and should therefore have manually written + test cases. + """ + #pylint: disable=unused-argument + # OAEP requires room for two hashes plus wrapping + m = cls.RSA_OAEP_RE.match(alg.string) + if m: + hash_alg = m.group(1) + hash_length = crypto_knowledge.Algorithm.hash_length(hash_alg) + key_length = (bits + 7) // 8 + # Leave enough room for at least one byte of plaintext + return key_length > 2 * hash_length + 2 + return True + def make_test_case(self, key: StorageTestData) -> test_case.TestCase: """Construct a storage format test case for the given key. @@ -546,7 +572,8 @@ class StorageFormat: # encodings of the attributes. # Raw data keys have no useful exercise anyway so there is no # loss of test coverage. - if key.type.string != 'PSA_KEY_TYPE_RAW_DATA': + if key.type.string != 'PSA_KEY_TYPE_RAW_DATA' and \ + self.valid_key_size_for_algorithm(key.type, key.bits, key.alg): flags.append('TEST_FLAG_EXERCISE') if 'READ_ONLY' in key.lifetime.string: flags.append('TEST_FLAG_READ_ONLY') From ac17ec438858cbeae9c2a3f5f3a45772fc519954 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 12:16:45 +0100 Subject: [PATCH 17/29] Public keys can't be used as private-key inputs to key agreement The PSA API does not use public key objects in key agreement operations: it imports the public key as a formatted byte string. So a public key object with a key agreement algorithm is not a valid combination. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 1491a844ad..54de0def67 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -241,6 +241,13 @@ class KeyType: return True if self.head == 'RSA' and alg.head.startswith('RSA_'): return True + if alg.category == AlgorithmCategory.KEY_AGREEMENT and \ + self.is_public(): + # The PSA API does not use public key objects in key agreement + # operations: it imports the public key as a formatted byte string. + # So a public key object with a key agreement algorithm is not + # a valid combination. + return False if self.head == 'ECC': assert self.params is not None eccc = EllipticCurveCategory.from_family(self.params[0]) From 61548d10359d35edd6a052d44f4fd0003afd8f0c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 15:36:09 +0100 Subject: [PATCH 18/29] Only exercise Brainpool curve keys on one algorithm There's nothing wrong with ECC keys on Brainpool curves, but operations with them are very slow. So we only exercise them with a single algorithm, not with all possible hashes. We do exercise other curves with all algorithms so test coverage is perfectly adequate like this. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 6fd311e632..492810bf01 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -519,13 +519,14 @@ class StorageFormat: self.forward = forward #type: bool RSA_OAEP_RE = re.compile(r'PSA_ALG_RSA_OAEP\((.*)\)\Z') + BRAINPOOL_RE = re.compile(r'PSA_KEY_TYPE_\w+\(PSA_ECC_FAMILY_BRAINPOOL_\w+\)\Z') @classmethod - def valid_key_size_for_algorithm( + def exercise_key_with_algorithm( cls, key_type: psa_storage.Expr, bits: int, alg: psa_storage.Expr ) -> bool: - """Whether the given key type and size are valid for the algorithm. + """Whether to the given key with the given algorithm. Normally only the type and algorithm matter for compatibility, and this is handled in crypto_knowledge.KeyType.can_do(). This function @@ -533,7 +534,13 @@ class StorageFormat: are not tested in OpFail and should therefore have manually written test cases. """ - #pylint: disable=unused-argument + # Some test keys have the RAW_DATA type and attributes that don't + # necessarily make sense. We do this to validate numerical + # encodings of the attributes. + # Raw data keys have no useful exercise anyway so there is no + # loss of test coverage. + if key_type.string == 'PSA_KEY_TYPE_RAW_DATA': + return False # OAEP requires room for two hashes plus wrapping m = cls.RSA_OAEP_RE.match(alg.string) if m: @@ -542,6 +549,14 @@ class StorageFormat: key_length = (bits + 7) // 8 # Leave enough room for at least one byte of plaintext return key_length > 2 * hash_length + 2 + # There's nothing wrong with ECC keys on Brainpool curves, + # but operations with them are very slow. So we only exercise them + # with a single algorithm, not with all possible hashes. We do + # exercise other curves with all algorithms so test coverage is + # perfectly adequate like this. + m = cls.BRAINPOOL_RE.match(key_type.string) + if m and alg.string != 'PSA_ALG_ECDSA_ANY': + return False return True def make_test_case(self, key: StorageTestData) -> test_case.TestCase: @@ -567,13 +582,7 @@ class StorageFormat: extra_arguments = [] else: flags = [] - # Some test keys have the RAW_DATA type and attributes that don't - # necessarily make sense. We do this to validate numerical - # encodings of the attributes. - # Raw data keys have no useful exercise anyway so there is no - # loss of test coverage. - if key.type.string != 'PSA_KEY_TYPE_RAW_DATA' and \ - self.valid_key_size_for_algorithm(key.type, key.bits, key.alg): + if self.exercise_key_with_algorithm(key.type, key.bits, key.alg): flags.append('TEST_FLAG_EXERCISE') if 'READ_ONLY' in key.lifetime.string: flags.append('TEST_FLAG_READ_ONLY') From aa3449dd2241ca6dc654b1576c77043b231d4a54 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 16:04:30 +0100 Subject: [PATCH 19/29] exercise_key: support combined key agreement+derivation algorithms Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index db3651d917..3d2dfc69a7 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -623,15 +623,39 @@ static int exercise_key_agreement_key( mbedtls_svc_key_id_t key, psa_algorithm_t alg ) { psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; + unsigned char input[1]; unsigned char output[1]; int ok = 0; + psa_algorithm_t kdf_alg = PSA_ALG_KEY_AGREEMENT_GET_KDF( alg ); if( usage & PSA_KEY_USAGE_DERIVE ) { /* We need two keys to exercise key agreement. Exercise the * private key against its own public key. */ PSA_ASSERT( psa_key_derivation_setup( &operation, alg ) ); + if( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || + PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) + { + PSA_ASSERT( psa_key_derivation_input_bytes( + &operation, PSA_KEY_DERIVATION_INPUT_SEED, + input, sizeof( input ) ) ); + } + PSA_ASSERT( mbedtls_test_psa_key_agreement_with_self( &operation, key ) ); + + if( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || + PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) + { + PSA_ASSERT( psa_key_derivation_input_bytes( + &operation, PSA_KEY_DERIVATION_INPUT_LABEL, + input, sizeof( input ) ) ); + } + else if( PSA_ALG_IS_HKDF( kdf_alg ) ) + { + PSA_ASSERT( psa_key_derivation_input_bytes( + &operation, PSA_KEY_DERIVATION_INPUT_INFO, + input, sizeof( input ) ) ); + } PSA_ASSERT( psa_key_derivation_output_bytes( &operation, output, sizeof( output ) ) ); From 17e350b12ab1da43402494187b349462b7aa8248 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 19 Mar 2022 18:06:52 +0100 Subject: [PATCH 20/29] Short-tag AEAD with the nominal length are encoded as nominal AEAD `PSA_ALG_AEAD_WITH_SHORTENED_TAG(aead_alg, len) == aead_alg` when `len == PSA_AEAD_TAG_LENGTH(aead_alg)`. So skip this case when testing the printing of constants. This fixes one test case due to the way arguments of `PSA_ALG_AEAD_WITH_SHORTENED_TAG` are enumerated (all algorithms are tested for a value of `len` which isn't problematic, and all values of `len` are tested for one algorithm). Signed-off-by: Gilles Peskine --- tests/scripts/test_psa_constant_names.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 07c8ab2e97..e43a0baef2 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -77,6 +77,22 @@ def normalize(expr: str) -> str: """ return re.sub(NORMALIZE_STRIP_RE, '', expr) +ALG_TRUNCATED_TO_SELF_RE = \ + re.compile(r'PSA_ALG_AEAD_WITH_SHORTENED_TAG\(' + r'PSA_ALG_(?:CCM|CHACHA20_POLY1305|GCM)' + r', *16\)\Z') + +def is_simplifiable(expr: str) -> bool: + """Determine whether an expression is simplifiable. + + Simplifiable expressions can't be output in their input form, since + the output will be the simple form. Therefore they must be excluded + from testing. + """ + if ALG_TRUNCATED_TO_SELF_RE.match(expr): + return True + return False + def collect_values(inputs: InputsForTest, type_word: str, include_path: Optional[str] = None, @@ -87,7 +103,9 @@ def collect_values(inputs: InputsForTest, value is a string representation of its integer value. """ names = inputs.get_names(type_word) - expressions = sorted(inputs.generate_expressions(names)) + expressions = sorted(expr + for expr in inputs.generate_expressions(names) + if not is_simplifiable(expr)) values = run_c(type_word, expressions, include_path=include_path, keep_c=keep_c) return expressions, values From 6e0f80ab94ab3d09a3669e463243763b47f9270b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 20 Mar 2022 20:44:22 +0100 Subject: [PATCH 21/29] Don't try to perform operations when driver support is lacking We test some configurations using drivers where the driver doesn't support certain hash algorithms, but declares that it supports compound algorithms that use those hashes. Until this is fixed, in those configurations, don't try to actually perform operations. The built-in implementation of asymmetric algorithms that use a hash internally only dispatch to the internal md module, not to PSA. Until this is supported, don't try to actually perform operations when the operation is built-in and the hash isn't. Signed-off-by: Gilles Peskine --- ...t_suite_psa_crypto_storage_format.function | 151 +++++++++++++++++- 1 file changed, 150 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_storage_format.function b/tests/suites/test_suite_psa_crypto_storage_format.function index b90ef6efcb..c52dae1882 100644 --- a/tests/suites/test_suite_psa_crypto_storage_format.function +++ b/tests/suites/test_suite_psa_crypto_storage_format.function @@ -60,6 +60,155 @@ static int can_export( const psa_key_attributes_t *attributes ) return( 0 ); } +#if defined(MBEDTLS_TEST_LIBTESTDRIVER1) +static int is_accelerated_rsa( psa_algorithm_t alg ) +{ +#if defined(MBEDTLS_PSA_ACCEL_ALG_RSA_PKCS1V15_SIGN) + if ( PSA_ALG_IS_RSA_PKCS1V15_SIGN( alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_ACCEL_ALG_RSA_PSS) + if( PSA_ALG_IS_RSA_PSS( alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_ACCEL_ALG_RSA_OAEP) + if( PSA_ALG_IS_RSA_OAEP( alg ) ) + return( 1 ); +#endif + (void) alg; + return( 0 ); +} + +/* Whether the algorithm is implemented as a builtin, i.e. not accelerated, + * and calls mbedtls_md() functions that require the hash algorithm to + * also be built-in. */ +static int is_builtin_calling_md( psa_algorithm_t alg ) +{ +#if defined(MBEDTLS_PSA_BUILTIN_ALG_RSA_PKCS1V15_SIGN) + if( PSA_ALG_IS_RSA_PKCS1V15_SIGN( alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_RSA_PSS) + if( PSA_ALG_IS_RSA_PSS( alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_RSA_OAEP) + if( PSA_ALG_IS_RSA_OAEP( alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA) + if( PSA_ALG_IS_DETERMINISTIC_ECDSA( alg ) ) + return( 1 ); +#endif + (void) alg; + return( 0 ); +} + +static int has_builtin_hash( psa_algorithm_t alg ) +{ +#if !defined(MBEDTLS_MD5_C) + if( alg == PSA_ALG_MD5 ) + return( 0 ); +#endif +#if !defined(MBEDTLS_RIPEMD160_C) + if( alg == PSA_ALG_RIPEMD160 ) + return( 0 ); +#endif +#if !defined(MBEDTLS_SHA1_C) + if( alg == PSA_ALG_SHA_1 ) + return( 0 ); +#endif +#if !defined(MBEDTLS_SHA224_C) + if( alg == PSA_ALG_SHA_224 ) + return( 0 ); +#endif +#if !defined(MBEDTLS_SHA256_C) + if( alg == PSA_ALG_SHA_256 ) + return( 0 ); +#endif +#if !defined(MBEDTLS_SHA384_C) + if( alg == PSA_ALG_SHA_384 ) + return( 0 ); +#endif +#if !defined(MBEDTLS_SHA512_C) + if( alg == PSA_ALG_SHA_512 ) + return( 0 ); +#endif + (void) alg; + return( 1 ); +} +#endif + +/* Mbed TLS doesn't support certain combinations of key type and algorithm + * in certain configurations. */ +static int can_exercise( const psa_key_attributes_t *attributes ) +{ + psa_key_type_t key_type = psa_get_key_type( attributes ); + psa_algorithm_t alg = psa_get_key_algorithm( attributes ); + psa_algorithm_t hash_alg = + PSA_ALG_IS_HASH_AND_SIGN( alg ) ? PSA_ALG_SIGN_GET_HASH( alg ) : + PSA_ALG_IS_RSA_OAEP( alg ) ? PSA_ALG_RSA_OAEP_GET_HASH( alg ) : + PSA_ALG_NONE; + psa_key_usage_t usage = psa_get_key_usage_flags( attributes ); + +#if defined(MBEDTLS_TEST_LIBTESTDRIVER1) + /* We test some configurations using drivers where the driver doesn't + * support certain hash algorithms, but declares that it supports + * compound algorithms that use those hashes. Until this is fixed, + * in those configurations, don't try to actually perform operations. + * + * Hash-and-sign algorithms where the asymmetric part doesn't use + * a hash operation are ok. So randomized ECDSA signature is fine, + * ECDSA verification is fine, but deterministic ECDSA signature is + * affected. All RSA signatures are affected except raw PKCS#1v1.5. + * OAEP is also affected. + */ + if( PSA_ALG_IS_DETERMINISTIC_ECDSA( alg ) && + ! ( usage & ( PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_SIGN_MESSAGE ) ) ) + { + /* Verification only. Verification doesn't use the hash algorithm. */ + return( 1 ); + } + +#if defined(MBEDTLS_PSA_ACCEL_ALG_DETERMINISTIC_ECDSA) + if( PSA_ALG_IS_DETERMINISTIC_ECDSA( alg ) && + ( hash_alg == PSA_ALG_MD5 || + hash_alg == PSA_ALG_RIPEMD160 || + hash_alg == PSA_ALG_SHA_1 ) ) + { + return( 0 ); + } +#endif + if( is_accelerated_rsa( alg ) && + ( hash_alg == PSA_ALG_RIPEMD160 || hash_alg == PSA_ALG_SHA_384 ) ) + { + return( 0 ); + } +#if defined(MBEDTLS_PSA_ACCEL_ALG_RSA_OAEP) + if( PSA_ALG_IS_RSA_OAEP( alg ) && + ( hash_alg == PSA_ALG_RIPEMD160 || hash_alg == PSA_ALG_SHA_384 ) ) + { + return( 0 ); + } +#endif + + /* The built-in implementation of asymmetric algorithms that use a + * hash internally only dispatch to the internal md module, not to + * PSA. Until this is supported, don't try to actually perform + * operations when the operation is built-in and the hash isn't. */ + if( is_builtin_calling_md( alg ) && ! has_builtin_hash( hash_alg ) ) + { + return( 0 ); + } +#endif /* MBEDTLS_TEST_LIBTESTDRIVER1 */ + + (void) key_type; + (void) alg; + (void) hash_alg; + (void) usage; + return( 1 ); +} + /** Write a key with the given representation to storage, then check * that it has the given attributes and (if exportable) key material. * @@ -108,7 +257,7 @@ static int test_read_key( const psa_key_attributes_t *expected_attributes, exported_material, length ); } - if( flags & TEST_FLAG_EXERCISE ) + if( ( flags & TEST_FLAG_EXERCISE ) && can_exercise( &actual_attributes ) ) { TEST_ASSERT( mbedtls_test_psa_exercise_key( key_id, From 2773f26971b220492906bb070b4e9d4b9beb6d67 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Apr 2022 16:31:16 +0200 Subject: [PATCH 22/29] Fix digits in octal constant Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 54de0def67..238a34bb85 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -474,7 +474,7 @@ class Algorithm: TRUNCATED_ALG_RE = re.compile( r'(?PPSA_ALG_(?:AEAD_WITH_SHORTENED_TAG|TRUNCATED_MAC))' r'\((?P.*),' - r'(?P0[Xx][0-9A-Fa-f]+|[1-9][0-9]*|0[0-9]*)[LUlu]*\)\Z') + r'(?P0[Xx][0-9A-Fa-f]+|[1-9][0-9]*|0[0-7]*)[LUlu]*\)\Z') def is_invalid_truncation(self) -> bool: """False for a MAC or AEAD algorithm truncated to an invalid length. From f96e9775843ac37e721365c52d8ed7dec71bd08c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Apr 2022 16:32:07 +0200 Subject: [PATCH 23/29] No need to recalculate iv_length Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 3d2dfc69a7..415f8852c0 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -181,7 +181,7 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, if( usage & PSA_KEY_USAGE_ENCRYPT ) { PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); - if( PSA_CIPHER_IV_LENGTH( key_type, alg ) != 0 ) + if( iv_length != 0 ) { PSA_ASSERT( psa_cipher_generate_iv( &operation, iv, sizeof( iv ), From dce7d8f51e7bedbf6abf30d62001107bc4180e1e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 12 Apr 2022 18:51:01 +0200 Subject: [PATCH 24/29] Rename and document mac_or_tag_lengths -> permitted_truncations No behavior change. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 238a34bb85..592fc0afe2 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -458,8 +458,14 @@ class Algorithm: } HMAC_RE = re.compile(r'PSA_ALG_HMAC\((.*)\)\Z') @classmethod - def mac_or_tag_lengths(cls, base: str) -> FrozenSet[int]: - """Return the set of permitted lengths for the given MAC or AEAD tag.""" + def permitted_truncations(cls, base: str) -> FrozenSet[int]: + """Permitted output lengths for the given MAC or AEAD base algorithm. + + For a MAC algorithm, this is the set of truncation lengths that + Mbed TLS supports. + For an AEAD algorithm, this is the set of truncation lengths that + are permitted by the algorithm specification. + """ if base in cls.PERMITTED_TAG_LENGTHS: return cls.PERMITTED_TAG_LENGTHS[base] max_length = cls.MAC_LENGTH.get(base, None) @@ -486,7 +492,7 @@ class Algorithm: if m: base = m.group('base') to_length = int(m.group('length'), 0) - permitted_lengths = self.mac_or_tag_lengths(base) + permitted_lengths = self.permitted_truncations(base) if to_length not in permitted_lengths: return True return False From 9b9b614a0249d4b615c61d9df35b72d2c260bd1e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 20 Apr 2022 16:55:03 +0200 Subject: [PATCH 25/29] cipher_encrypt_alg_without_iv: validate size macros independently Validate the size macros directly from the output length in the test data, rather than using the value returned by the library. This is equivalent since the value returned by the library is checked to be identical. Enforce that SIZE() <= MAX_SIZE(), in addition to length <= SIZE(). This is stronger than the previous code which merely enforced length <= SIZE() and length <= MAX_SIZE(). Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto.function | 23 +++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 6552ecd8c5..d24d1eb745 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3323,25 +3323,33 @@ void cipher_encrypt_alg_without_iv( int alg_arg, PSA_ASSERT( psa_crypto_init( ) ); + /* Validate size macros */ + TEST_ASSERT( expected_output->len <= + PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ) ); + TEST_ASSERT( PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ) <= + PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE( input->len ) ); + + /* Set up key and output buffer */ psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_ENCRYPT ); psa_set_key_algorithm( &attributes, alg ); psa_set_key_type( &attributes, key_type ); - + PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len, + &key ) ); output_buffer_size = PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ); ASSERT_ALLOC( output, output_buffer_size ); - PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len, - &key ) ); - + /* set_iv() is not allowed */ PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); TEST_EQUAL( psa_cipher_set_iv( &operation, iv, sizeof( iv ) ), PSA_ERROR_BAD_STATE ); + + /* generate_iv() is not allowed */ PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); TEST_EQUAL( psa_cipher_generate_iv( &operation, iv, sizeof( iv ), &iv_length ), PSA_ERROR_BAD_STATE ); - /* Encrypt, one-shot */ + /* One-shot encryption */ PSA_ASSERT( psa_cipher_encrypt( key, alg, input->x, input->len, output, output_buffer_size, &output_length ) ); TEST_ASSERT( output_length <= @@ -3359,10 +3367,6 @@ void cipher_encrypt_alg_without_iv( int alg_arg, PSA_ASSERT( psa_cipher_update( &operation, input->x, input->len, output, output_buffer_size, &output_length) ); - TEST_ASSERT( output_length <= - PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ) ); - TEST_ASSERT( output_length <= - PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE( input->len ) ); ASSERT_COMPARE( expected_output->x, expected_output->len, output, output_length ); @@ -3370,6 +3374,7 @@ void cipher_encrypt_alg_without_iv( int alg_arg, exit: PSA_ASSERT( psa_cipher_abort( &operation ) ); mbedtls_free( output ); + psa_cipher_abort( &operation ); psa_destroy_key( key ); PSA_DONE( ); } From 9e38f2c8fdbcff24798c454c3fb5df141fb41347 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 20 Apr 2022 17:07:52 +0200 Subject: [PATCH 26/29] cipher_alg_without_iv: generalized to also do decryption Test set_iv/generate_iv after decrypt_setup. Test successful decryption. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto.data | 10 ++-- tests/suites/test_suite_psa_crypto.function | 54 +++++++++++++-------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index ae24d63eb2..9368927fa3 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -2109,23 +2109,23 @@ cipher_encrypt_fail:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf PSA symmetric encrypt: AES-ECB, 0 bytes, good depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_AES -cipher_encrypt_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"":"" +cipher_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"":"" PSA symmetric encrypt: AES-ECB, 16 bytes, good depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_AES -cipher_encrypt_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a":"3ad77bb40d7a3660a89ecaf32466ef97" +cipher_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a":"3ad77bb40d7a3660a89ecaf32466ef97" PSA symmetric encrypt: AES-ECB, 32 bytes, good depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_AES -cipher_encrypt_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a3ad77bb40d7a3660a89ecaf32466ef97":"3ad77bb40d7a3660a89ecaf32466ef972249a2638c6f1c755a84f9681a9f08c1" +cipher_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a3ad77bb40d7a3660a89ecaf32466ef97":"3ad77bb40d7a3660a89ecaf32466ef972249a2638c6f1c755a84f9681a9f08c1" PSA symmetric encrypt: 2-key 3DES-ECB, 8 bytes, good depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_DES -cipher_encrypt_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce":"c78e2b38139610e3":"5d0652429c5b0ac7" +cipher_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce":"c78e2b38139610e3":"5d0652429c5b0ac7" PSA symmetric encrypt: 3-key 3DES-ECB, 8 bytes, good depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_DES -cipher_encrypt_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"c78e2b38139610e3":"817ca7d69b80d86a" +cipher_alg_without_iv:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"c78e2b38139610e3":"817ca7d69b80d86a" PSA symmetric encrypt validation: AES-CBC-nopad, 16 bytes, good depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_AES diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index d24d1eb745..f67230d01d 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3304,11 +3304,8 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void cipher_encrypt_alg_without_iv( int alg_arg, - int key_type_arg, - data_t *key_data, - data_t *input, - data_t *expected_output ) +void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data, + data_t *plaintext, data_t *ciphertext ) { mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; psa_key_type_t key_type = key_type_arg; @@ -3318,46 +3315,65 @@ void cipher_encrypt_alg_without_iv( int alg_arg, size_t iv_length; unsigned char *output = NULL; size_t output_buffer_size = 0; - size_t output_length = 0; + size_t output_length; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; PSA_ASSERT( psa_crypto_init( ) ); /* Validate size macros */ - TEST_ASSERT( expected_output->len <= - PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ) ); - TEST_ASSERT( PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ) <= - PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE( input->len ) ); + TEST_ASSERT( ciphertext->len <= + PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, plaintext->len ) ); + TEST_ASSERT( PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, plaintext->len ) <= + PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE( plaintext->len ) ); + TEST_ASSERT( plaintext->len <= + PSA_CIPHER_DECRYPT_OUTPUT_SIZE( key_type, alg, ciphertext->len ) ); + TEST_ASSERT( PSA_CIPHER_DECRYPT_OUTPUT_SIZE( key_type, alg, ciphertext->len ) <= + PSA_CIPHER_DECRYPT_OUTPUT_MAX_SIZE( ciphertext->len ) ); + /* Set up key and output buffer */ - psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_ENCRYPT ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT ); psa_set_key_algorithm( &attributes, alg ); psa_set_key_type( &attributes, key_type ); PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len, &key ) ); - output_buffer_size = PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ); + output_buffer_size = PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, + plaintext->len ); ASSERT_ALLOC( output, output_buffer_size ); /* set_iv() is not allowed */ PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); + TEST_EQUAL( psa_cipher_set_iv( &operation, iv, sizeof( iv ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_decrypt_setup( &operation, key, alg ) ); TEST_EQUAL( psa_cipher_set_iv( &operation, iv, sizeof( iv ) ), PSA_ERROR_BAD_STATE ); /* generate_iv() is not allowed */ PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); + TEST_EQUAL( psa_cipher_generate_iv( &operation, iv, sizeof( iv ), + &iv_length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_decrypt_setup( &operation, key, alg ) ); TEST_EQUAL( psa_cipher_generate_iv( &operation, iv, sizeof( iv ), &iv_length ), PSA_ERROR_BAD_STATE ); /* One-shot encryption */ - PSA_ASSERT( psa_cipher_encrypt( key, alg, input->x, input->len, output, - output_buffer_size, &output_length ) ); - TEST_ASSERT( output_length <= - PSA_CIPHER_ENCRYPT_OUTPUT_SIZE( key_type, alg, input->len ) ); - TEST_ASSERT( output_length <= - PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE( input->len ) ); + output_length = ~0; + PSA_ASSERT( psa_cipher_encrypt( key, alg, plaintext->x, plaintext->len, + output, output_buffer_size, + &output_length ) ); + ASSERT_COMPARE( ciphertext->x, ciphertext->len, + output, output_length ); - ASSERT_COMPARE( expected_output->x, expected_output->len, + /* One-shot decryption */ + output_length = ~0; + PSA_ASSERT( psa_cipher_decrypt( key, alg, ciphertext->x, ciphertext->len, + output, output_buffer_size, + &output_length ) ); + ASSERT_COMPARE( plaintext->x, plaintext->len, output, output_length ); /* Encrypt, multi-part */ From 286c314ae32e551448a8235257b338c8a1789199 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 20 Apr 2022 17:09:38 +0200 Subject: [PATCH 27/29] cipher_alg_without_iv: also test multipart decryption For multipart encrpytion, call psa_cipher_finish(). This is not actually necessary for non-pathological implementations of ECB (the only currently supported IV-less cipher algorithm) because it requires the input to be a whole number of blocks and non-pathological implementations emit the output block from update() as soon as an input block is available. But in principle a driver could delay output and thus require a call to finish(). Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto.function | 54 +++++++++++++++------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index f67230d01d..e2bff0478c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3312,10 +3312,9 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data, psa_algorithm_t alg = alg_arg; psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; uint8_t iv[1] = { 0x5a }; - size_t iv_length; unsigned char *output = NULL; size_t output_buffer_size = 0; - size_t output_length; + size_t output_length, length; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; PSA_ASSERT( psa_crypto_init( ) ); @@ -3353,13 +3352,49 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data, /* generate_iv() is not allowed */ PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); TEST_EQUAL( psa_cipher_generate_iv( &operation, iv, sizeof( iv ), - &iv_length ), + &length ), PSA_ERROR_BAD_STATE ); PSA_ASSERT( psa_cipher_decrypt_setup( &operation, key, alg ) ); TEST_EQUAL( psa_cipher_generate_iv( &operation, iv, sizeof( iv ), - &iv_length ), + &length ), PSA_ERROR_BAD_STATE ); + /* Multipart encryption */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); + output_length = 0; + length = ~0; + PSA_ASSERT( psa_cipher_update( &operation, + plaintext->x, plaintext->len, + output, output_buffer_size, + &length ) ); + TEST_ASSERT( length <= output_buffer_size ); + output_length += length; + PSA_ASSERT( psa_cipher_finish( &operation, + output + output_length, + output_buffer_size - output_length, + &length ) ); + output_length += length; + ASSERT_COMPARE( ciphertext->x, ciphertext->len, + output, output_length ); + + /* Multipart encryption */ + PSA_ASSERT( psa_cipher_decrypt_setup( &operation, key, alg ) ); + output_length = 0; + length = ~0; + PSA_ASSERT( psa_cipher_update( &operation, + ciphertext->x, ciphertext->len, + output, output_buffer_size, + &length ) ); + TEST_ASSERT( length <= output_buffer_size ); + output_length += length; + PSA_ASSERT( psa_cipher_finish( &operation, + output + output_length, + output_buffer_size - output_length, + &length ) ); + output_length += length; + ASSERT_COMPARE( plaintext->x, plaintext->len, + output, output_length ); + /* One-shot encryption */ output_length = ~0; PSA_ASSERT( psa_cipher_encrypt( key, alg, plaintext->x, plaintext->len, @@ -3376,17 +3411,6 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data, ASSERT_COMPARE( plaintext->x, plaintext->len, output, output_length ); - /* Encrypt, multi-part */ - PSA_ASSERT( psa_cipher_abort( &operation ) ); - PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) ); - - PSA_ASSERT( psa_cipher_update( &operation, input->x, input->len, - output, output_buffer_size, - &output_length) ); - - ASSERT_COMPARE( expected_output->x, expected_output->len, - output, output_length ); - exit: PSA_ASSERT( psa_cipher_abort( &operation ) ); mbedtls_free( output ); From 5eef11af2c5cfff412582da336d30a0917d50441 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Apr 2022 11:14:30 +0200 Subject: [PATCH 28/29] Remove redundant initialization of iv_length Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 415f8852c0..53af7b801c 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -165,7 +165,7 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, { psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; unsigned char iv[16] = {0}; - size_t iv_length = sizeof( iv ); + size_t iv_length; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_key_type_t key_type; const unsigned char plaintext[16] = "Hello, world..."; From b29d814169c2d879f2b14a865e2439ac0e0131da Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Apr 2022 11:14:52 +0200 Subject: [PATCH 29/29] Use MAX_SIZE macros instead of hard-coding IV/nonce max size Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 53af7b801c..9576afd0c2 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -164,7 +164,7 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, psa_algorithm_t alg ) { psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; - unsigned char iv[16] = {0}; + unsigned char iv[PSA_CIPHER_IV_MAX_SIZE] = {0}; size_t iv_length; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_key_type_t key_type; @@ -242,7 +242,7 @@ static int exercise_aead_key( mbedtls_svc_key_id_t key, psa_key_usage_t usage, psa_algorithm_t alg ) { - unsigned char nonce[16] = {0}; + unsigned char nonce[PSA_AEAD_NONCE_MAX_SIZE] = {0}; size_t nonce_length; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_key_type_t key_type;