From f761427fb96e6503d38c5bbe70ab440647c2c708 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 24 Feb 2022 18:58:08 +0100 Subject: [PATCH 01/21] Simplify key_for_usage_flags Generate "with implication" and "without implication" usage test cases separately. The set of generated test cases is unchanged. The order, and the description of "with implication" test cases, changes. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 34 ++++++++++------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 75ef353514..adbf8866b4 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -443,51 +443,41 @@ class StorageFormat: continue yield self.key_for_lifetime(lifetime) - def keys_for_usage_flags( + def key_for_usage_flags( self, usage_flags: List[str], short: Optional[str] = None, - test_implicit_usage: Optional[bool] = False - ) -> Iterator[StorageTestData]: + 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 = ' with implication' if test_implicit_usage else '' + extra_desc = ' without implication' if test_implicit_usage else '' description = 'usage' + extra_desc + ': ' + short key1 = StorageTestData(version=self.version, id=1, lifetime=0x00000001, type='PSA_KEY_TYPE_RAW_DATA', bits=8, expected_usage=usage, + without_implicit_usage=not test_implicit_usage, usage=usage, alg=0, alg2=0, material=b'K', description=description) - yield key1 - - if test_implicit_usage: - description = 'usage without implication' + ': ' + short - key2 = StorageTestData(version=self.version, - id=1, lifetime=0x00000001, - type='PSA_KEY_TYPE_RAW_DATA', bits=8, - without_implicit_usage=True, - usage=usage, alg=0, alg2=0, - material=b'K', - description=description) - yield key2 + return key1 def generate_keys_for_usage_flags(self, **kwargs) -> Iterator[StorageTestData]: """Generate test keys covering usage flags.""" known_flags = sorted(self.constructors.key_usage_flags) - yield from self.keys_for_usage_flags(['0'], **kwargs) + yield self.key_for_usage_flags(['0'], **kwargs) for usage_flag in known_flags: - yield from self.keys_for_usage_flags([usage_flag], **kwargs) + yield self.key_for_usage_flags([usage_flag], **kwargs) for flag1, flag2 in zip(known_flags, known_flags[1:] + [known_flags[0]]): - yield from self.keys_for_usage_flags([flag1, flag2], **kwargs) + yield self.key_for_usage_flags([flag1, flag2], **kwargs) def generate_key_for_all_usage_flags(self) -> Iterator[StorageTestData]: known_flags = sorted(self.constructors.key_usage_flags) - yield from self.keys_for_usage_flags(known_flags, short='all known') + yield self.key_for_usage_flags(known_flags, short='all known') def all_keys_for_usage_flags(self) -> Iterator[StorageTestData]: yield from self.generate_keys_for_usage_flags() @@ -593,8 +583,8 @@ class StorageFormatV0(StorageFormat): def all_keys_for_usage_flags(self) -> Iterator[StorageTestData]: """Generate test keys covering usage flags.""" - yield from self.generate_keys_for_usage_flags(test_implicit_usage=True) - yield from self.generate_key_for_all_usage_flags() + yield from super().all_keys_for_usage_flags() + yield from self.generate_keys_for_usage_flags(test_implicit_usage=False) def keys_for_implicit_usage( self, From b9dbb7fe62f7e51f9a21434f29e7c7c0ead6f0fc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 20:19:57 +0200 Subject: [PATCH 02/21] Add missing type annotation 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 c38955b0ed..6b38aac16e 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -26,7 +26,7 @@ from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA class KeyType: """Knowledge about a PSA key type.""" - def __init__(self, name: str, params: Optional[Iterable[str]] = None): + def __init__(self, name: str, params: Optional[Iterable[str]] = None) -> None: """Analyze a key type. The key type must be specified in PSA syntax. In its simplest form, From 26f9054d8ffdfb0fd328783aee5ca06c7594a1ed Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 16:39:51 +0100 Subject: [PATCH 03/21] Declare modules used by generate_psa_tests.py as dependencies Signed-off-by: Gilles Peskine --- tests/CMakeLists.txt | 4 ++++ tests/Makefile | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2431e40697..9abab0ad5c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -46,6 +46,10 @@ if(GEN_FILES) --directory ${CMAKE_CURRENT_BINARY_DIR}/suites DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_psa_tests.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/crypto_knowledge.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/macro_collector.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/psa_storage.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_case.py ${CMAKE_CURRENT_SOURCE_DIR}/../include/psa/crypto_config.h ${CMAKE_CURRENT_SOURCE_DIR}/../include/psa/crypto_values.h ${CMAKE_CURRENT_SOURCE_DIR}/../include/psa/crypto_extra.h diff --git a/tests/Makefile b/tests/Makefile index 94a834ef85..0d08f845d5 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -84,6 +84,10 @@ generated_files: $(GENERATED_FILES) .SECONDARY: generated_psa_test_data $(GENERATED_DATA_FILES): generated_psa_test_data generated_psa_test_data: scripts/generate_psa_tests.py +generated_psa_test_data: ../scripts/mbedtls_dev/crypto_knowledge.py +generated_psa_test_data: ../scripts/mbedtls_dev/macro_collector.py +generated_psa_test_data: ../scripts/mbedtls_dev/psa_storage.py +generated_psa_test_data: ../scripts/mbedtls_dev/test_case.py ## The generated file only depends on the options that are present in ## crypto_config.h, not on which options are set. To avoid regenerating this ## file all the time when switching between configurations, don't declare From 08622b6dc7e6a10ea8f4f4f2e9c484ce1cb5bdc8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 16:40:59 +0100 Subject: [PATCH 04/21] Declare PSA_WANT_ALG_CCM_STAR_NO_TAG and use it in tests CCM*-no-tag is currently available whenever CCM is, so declare PSA_WANT_ALG_CCM_STAR_NO_TAG whenever PSA_WANT_ALG_CCM is declared and vice versa. Fix dependencies of test cases that use PSA_ALG_CCM_STAR_NO_TAG: some were using PSA_WANT_ALG_CCM and some had altogether wrong dependencies. This commit does not touch library code. There is still no provision for providing CCM support without CCM*-no-tag or vice versa. Signed-off-by: Gilles Peskine --- ChangeLog.d/ccm_star_no_tag.txt | 4 ++++ include/mbedtls/config_psa.h | 9 +++++++++ tests/suites/test_suite_psa_crypto.data | 18 +++++++++--------- 3 files changed, 22 insertions(+), 9 deletions(-) create mode 100644 ChangeLog.d/ccm_star_no_tag.txt diff --git a/ChangeLog.d/ccm_star_no_tag.txt b/ChangeLog.d/ccm_star_no_tag.txt new file mode 100644 index 0000000000..21e829c2a9 --- /dev/null +++ b/ChangeLog.d/ccm_star_no_tag.txt @@ -0,0 +1,4 @@ +Bugfix + * Declare or use PSA_WANT_ALG_CCM_STAR_NO_TAG following the general + pattern for PSA_WANT_xxx symbols. Previously you had to specify + PSA_WANT_ALG_CCM for PSA_ALG_CCM_STAR_NO_TAG. diff --git a/include/mbedtls/config_psa.h b/include/mbedtls/config_psa.h index 68dda0f395..6eccd5a6cf 100644 --- a/include/mbedtls/config_psa.h +++ b/include/mbedtls/config_psa.h @@ -50,6 +50,12 @@ extern "C" { #define PSA_WANT_ALG_ECDSA_ANY PSA_WANT_ALG_ECDSA #endif +#if defined(PSA_WANT_ALG_CCM_STAR_NO_TAG) && !defined(PSA_WANT_ALG_CCM) +#define PSA_WANT_ALG_CCM PSA_WANT_ALG_CCM_STAR_NO_TAG +#elif !defined(PSA_WANT_ALG_CCM_STAR_NO_TAG) && defined(PSA_WANT_ALG_CCM) +#define PSA_WANT_ALG_CCM_STAR_NO_TAG PSA_WANT_ALG_CCM +#endif + #if defined(PSA_WANT_ALG_RSA_PKCS1V15_SIGN_RAW) && !defined(PSA_WANT_ALG_RSA_PKCS1V15_SIGN) #define PSA_WANT_ALG_RSA_PKCS1V15_SIGN PSA_WANT_ALG_RSA_PKCS1V15_SIGN_RAW #elif !defined(PSA_WANT_ALG_RSA_PKCS1V15_SIGN_RAW) && defined(PSA_WANT_ALG_RSA_PKCS1V15_SIGN) @@ -411,6 +417,7 @@ extern "C" { defined(PSA_HAVE_SOFT_KEY_TYPE_ARIA) || \ defined(PSA_HAVE_SOFT_KEY_TYPE_CAMELLIA) #define MBEDTLS_PSA_BUILTIN_ALG_CCM 1 +#define MBEDTLS_PSA_BUILTIN_ALG_CCM_STAR_NO_TAG 1 #define MBEDTLS_CCM_C #endif #endif /* PSA_WANT_ALG_CCM */ @@ -545,7 +552,9 @@ extern "C" { #if defined(MBEDTLS_CCM_C) #define MBEDTLS_PSA_BUILTIN_ALG_CCM 1 +#define MBEDTLS_PSA_BUILTIN_ALG_CCM_STAR_NO_TAG 1 #define PSA_WANT_ALG_CCM 1 +#define PSA_WANT_ALG_CCM_STAR_NO_TAG 1 #endif /* MBEDTLS_CCM_C */ #if defined(MBEDTLS_CMAC_C) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 1a50749c38..ae24d63eb2 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -2160,7 +2160,7 @@ depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_DES cipher_encrypt_validation:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"eda4011239bc3ac9" PSA symmetric encrypt validation: CCM*-no-tag, 15 bytes, good -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_encrypt_validation:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"d24a3d3dde8c84830280cb87abad0bb3":"6bc1bee22e409f96e93d7e11739317" PSA symmetric encrypt multipart: AES-ECB, 0 bytes, good @@ -2224,7 +2224,7 @@ depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_DES cipher_encrypt_multipart:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"":"c78e2b38139610e3":8:8:0:"817ca7d69b80d86a":PSA_SUCCESS PSA symmetric encrypt multipart: CCM*-no-tag, AES, 24 bytes, good -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_encrypt_multipart:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"d24a3d3dde8c84830280cb87abad0bb3":"f1100035bb24a8d26004e0e24b":"7c86135ed9c2a515aaae0e9a208133897269220f30870006":10:10:14:"1faeb0ee2ca2cd52f0aa3966578344f24e69b742c4ab37ab":PSA_SUCCESS PSA cipher decrypt: without initialization @@ -2260,7 +2260,7 @@ depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_AES cipher_decrypt_fail:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"6bc1bee223":PSA_ERROR_INVALID_ARGUMENT PSA symetric decrypt: CCM*-no-tag, input too short (15 bytes) -depends_on:MBEDTLS_AES_C:MBEDTLS_CCM_C +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:MBEDTLS_AES_C cipher_decrypt_fail:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"19ebfde2d5468ba0a3031bde629b11fd":"5a8aa485c316e9":"2a2a2a2a2a2a2a2a":PSA_ERROR_INVALID_ARGUMENT PSA symmetric decrypt: AES-ECB, 0 bytes, good @@ -2312,7 +2312,7 @@ depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_DES cipher_decrypt:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"":"817ca7d69b80d86a":"c78e2b38139610e3" PSA symmetric decrypt: CCM*-no-tag, NIST DVPT AES-128 #15 -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_decrypt:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"90929a4b0ac65b350ad1591611fe4829":"5a8aa485c316e9403aff859fbb":"4bfe4e35784f0a65b545477e5e2f4bae0e1e6fa717eaf2cb":"a16a2e741f1cd9717285b6d882c1fc53655e9773761ad697" PSA symmetric decrypt multipart: AES-ECB, 0 bytes, good @@ -2376,7 +2376,7 @@ depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_DES cipher_decrypt_multipart:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"":"817ca7d69b80d86a":8:8:0:"c78e2b38139610e3":PSA_SUCCESS PSA symmetric decrypt multipart: CCM*-no-tag, 24 bytes, good -depends_on:PSA_WANT_ALG_ECB_NO_PADDING:PSA_WANT_KEY_TYPE_DES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_decrypt_multipart:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"197afb02ffbd8f699dacae87094d5243":"5a8aa485c316e9403aff859fbb":"4a550134f94455979ec4bf89ad2bd80d25a77ae94e456134":10:10:14:"a16a2e741f1cd9717285b6d882c1fc53655e9773761ad697":PSA_SUCCESS PSA symmetric encrypt/decrypt: AES-ECB, 16 bytes, good @@ -2400,19 +2400,19 @@ depends_on:PSA_WANT_ALG_CTR:PSA_WANT_KEY_TYPE_AES cipher_verify_output:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a" PSA symmetric encrypt/decrypt: CCM*-no-tag, AES -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_verify_output:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a" CCM*-no-tag encrypt, iv_length = 14, bad -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_encrypt_validate_iv_length:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"90929a4b0ac65b350ad1591611fe4829":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":14:PSA_ERROR_INVALID_ARGUMENT CCM*-no-tag encrypt, iv_length = 13, good -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_encrypt_validate_iv_length:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"90929a4b0ac65b350ad1591611fe4829":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":13:PSA_SUCCESS CCM*-no-tag encrypt, iv_length = 12, bad -depends_on:PSA_WANT_ALG_CCM:PSA_WANT_KEY_TYPE_AES +depends_on:PSA_WANT_ALG_CCM_STAR_NO_TAG:PSA_WANT_KEY_TYPE_AES cipher_encrypt_validate_iv_length:PSA_ALG_CCM_STAR_NO_TAG:PSA_KEY_TYPE_AES:"90929a4b0ac65b350ad1591611fe4829":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":12:PSA_ERROR_INVALID_ARGUMENT PSA symmetric encryption multipart: AES-ECB, 16+16 bytes From c7e1ea074a726d15b361c1ebbe77e09facc75e65 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 27 Apr 2021 20:40:10 +0200 Subject: [PATCH 05/21] New test suite for systematically generated operation failure tests The new test suite psa_crypto_op_fail is intended for systematically generated test cases that ensure that cryptographic operations with invalid parameters fail as expected. I intend invalid parameters to cover things like an invalid algorithm, an algorithm that is incompatible with the operation, a key type that is incompatible with the algorithm, etc. This commit just creates the infrastructure. Subsequent commits will add data generation and test code. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 16 ++++++++++++++++ .../test_suite_psa_crypto_op_fail.function | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 tests/suites/test_suite_psa_crypto_op_fail.function diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index adbf8866b4..29ac591bd2 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -305,6 +305,20 @@ class KeyGenerate: kt = crypto_knowledge.KeyType(constr, [curve_family]) yield from self.test_cases_for_key_type_key_generation(kt) +class OpFail: + """Generate test cases for operations that must fail.""" + #pylint: disable=too-few-public-methods + + def __init__(self, info: Information) -> None: + self.constructors = info.constructors + + def all_test_cases(self) -> Iterator[test_case.TestCase]: + """Generate all test cases for operations that must fail.""" + #pylint: disable=no-self-use + return # To do + yield #pylint: disable=unreachable + + class StorageKey(psa_storage.Key): """Representation of a key for storage format testing.""" @@ -722,6 +736,8 @@ class TestGenerator: lambda info: KeyGenerate(info).test_cases_for_key_generation(), 'test_suite_psa_crypto_not_supported.generated': lambda info: NotSupported(info).test_cases_for_not_supported(), + 'test_suite_psa_crypto_op_fail.generated': + lambda info: OpFail(info).all_test_cases(), 'test_suite_psa_crypto_storage_format.current': lambda info: StorageFormatForward(info, 0).all_test_cases(), 'test_suite_psa_crypto_storage_format.v0': diff --git a/tests/suites/test_suite_psa_crypto_op_fail.function b/tests/suites/test_suite_psa_crypto_op_fail.function new file mode 100644 index 0000000000..d72f2c3c79 --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_op_fail.function @@ -0,0 +1,18 @@ +/* BEGIN_HEADER */ + +#include "psa/crypto.h" +#include "test/psa_crypto_helpers.h" + + +/* END_HEADER */ + +/* BEGIN_DEPENDENCIES + * depends_on:MBEDTLS_PSA_CRYPTO_C + * END_DEPENDENCIES + */ + +/* BEGIN_CASE depends_on:NEVER_USED */ +void test_suites_must_have_at_least_one_function( ) +{ +} +/* END_CASE */ From 8b4a38176a33b3510a28d00e37321c1c1d8d632d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 27 Apr 2021 21:03:43 +0200 Subject: [PATCH 06/21] Generate test cases for hash operation failure Test that hash operation functions fail when given a hash algorithm that is not supported or an algorithm that is not a hash. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 32 +++++++++++++++++-- .../test_suite_psa_crypto_op_fail.function | 25 +++++++++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 29ac591bd2..e7819b91c5 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -312,11 +312,37 @@ class OpFail: def __init__(self, info: Information) -> None: self.constructors = info.constructors + @staticmethod + def hash_test_cases(alg: str) -> Iterator[test_case.TestCase]: + """Generate hash failure test cases for the specified algorithm.""" + tc = test_case.TestCase() + is_hash = (alg.startswith('PSA_ALG_SHA') or + alg.startswith('PSA_ALG_MD') or + alg in frozenset(['PSA_ALG_RIPEMD160', 'PSA_ALG_ANY_HASH'])) + if is_hash: + descr = 'not supported' + status = 'PSA_ERROR_NOT_SUPPORTED' + dependencies = ['!PSA_WANT_' + alg[4:]] + else: + descr = 'invalid' + status = 'PSA_ERROR_INVALID_ARGUMENT' + dependencies = automatic_dependencies(alg) + tc.set_description('PSA hash {}: {}' + .format(descr, re.sub(r'PSA_ALG_', r'', alg))) + tc.set_dependencies(dependencies) + tc.set_function('hash_fail') + tc.set_arguments([alg, status]) + yield tc + + def test_cases_for_algorithm(self, alg: str) -> Iterator[test_case.TestCase]: + """Generate operation failure test cases for the specified algorithm.""" + yield from self.hash_test_cases(alg) + def all_test_cases(self) -> Iterator[test_case.TestCase]: """Generate all test cases for operations that must fail.""" - #pylint: disable=no-self-use - return # To do - yield #pylint: disable=unreachable + algorithms = sorted(self.constructors.algorithms) + for alg in self.constructors.generate_expressions(algorithms): + yield from self.test_cases_for_algorithm(alg) class StorageKey(psa_storage.Key): diff --git a/tests/suites/test_suite_psa_crypto_op_fail.function b/tests/suites/test_suite_psa_crypto_op_fail.function index d72f2c3c79..21dbafaf97 100644 --- a/tests/suites/test_suite_psa_crypto_op_fail.function +++ b/tests/suites/test_suite_psa_crypto_op_fail.function @@ -11,8 +11,29 @@ * END_DEPENDENCIES */ -/* BEGIN_CASE depends_on:NEVER_USED */ -void test_suites_must_have_at_least_one_function( ) +/* BEGIN_CASE */ +void hash_fail( int alg_arg, int expected_status_arg ) { + psa_status_t expected_status = expected_status_arg; + psa_algorithm_t alg = alg_arg; + psa_hash_operation_t operation = PSA_HASH_OPERATION_INIT; + uint8_t input[1] = {'A'}; + uint8_t output[PSA_HASH_MAX_SIZE] = {0}; + size_t length = SIZE_MAX; + + PSA_INIT( ); + + TEST_EQUAL( expected_status, + psa_hash_setup( &operation, alg ) ); + TEST_EQUAL( expected_status, + psa_hash_compute( alg, input, sizeof( input ), + output, sizeof( output ), &length ) ); + TEST_EQUAL( expected_status, + psa_hash_compare( alg, input, sizeof( input ), + output, sizeof( output ) ) ); + +exit: + psa_hash_abort( &operation ); + PSA_DONE( ); } /* END_CASE */ From ee7554e606a8be1eac00fca05d336dfccafa5ba3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 20:38:01 +0200 Subject: [PATCH 07/21] Add knowledge of algorithms Determine the category of operations supported by an algorithm based on its name. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 151 ++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 6b38aac16e..072d53d933 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -18,11 +18,21 @@ This module is entirely based on the PSA API. # See the License for the specific language governing permissions and # limitations under the License. +import enum import re from typing import Dict, Iterable, Optional, Pattern, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA + +BLOCK_MAC_MODES = frozenset(['CBC_MAC', 'CMAC']) +BLOCK_CIPHER_MODES = frozenset([ + 'CTR', 'CFB', 'OFB', 'XTS', 'CCM_STAR_NO_TAG', + 'ECB_NO_PADDING', 'CBC_NO_PADDING', 'CBC_PKCS7', +]) +BLOCK_AEAD_MODES = frozenset(['CCM', 'GCM']) + + class KeyType: """Knowledge about a PSA key type.""" @@ -153,3 +163,144 @@ class KeyType: """ # This is just temporaly solution for the implicit usage flags. return re.match(self.KEY_TYPE_FOR_SIGNATURE[usage], self.name) is not None + + +class AlgorithmCategory(enum.Enum): + """PSA algorithm categories.""" + # The numbers are aligned with the category bits in numerical values of + # algorithms. + HASH = 2 + MAC = 3 + CIPHER = 4 + AEAD = 5 + SIGN = 6 + ASYMMETRIC_ENCRYPTION = 7 + KEY_DERIVATION = 8 + KEY_AGREEMENT = 9 + PAKE = 10 + + def requires_key(self) -> bool: + return self not in {self.HASH, self.KEY_DERIVATION} + + +class AlgorithmNotRecognized(Exception): + def __init__(self, expr: str) -> None: + super().__init__('Algorithm not recognized: ' + expr) + self.expr = expr + + +class Algorithm: + """Knowledge about a PSA algorithm.""" + + @staticmethod + def determine_base(expr: str) -> str: + """Return an expression for the "base" of the algorithm. + + This strips off variants of algorithms such as MAC truncation. + + This function does not attempt to detect invalid inputs. + """ + m = re.match(r'PSA_ALG_(?:' + r'(?:TRUNCATED|AT_LEAST_THIS_LENGTH)_MAC|' + r'AEAD_WITH_(?:SHORTENED|AT_LEAST_THIS_LENGTH)_TAG' + r')\((.*),[^,]+\)\Z', expr) + if m: + expr = m.group(1) + return expr + + @staticmethod + def determine_head(expr: str) -> str: + """Return the head of an algorithm expression. + + The head is the first (outermost) constructor, without its PSA_ALG_ + prefix, and with some normalization of similar algorithms. + """ + m = re.match(r'PSA_ALG_(?:DETERMINISTIC_)?(\w+)', expr) + if not m: + raise AlgorithmNotRecognized(expr) + head = m.group(1) + if head == 'KEY_AGREEMENT': + m = re.match(r'PSA_ALG_KEY_AGREEMENT\s*\(\s*PSA_ALG_(\w+)', expr) + if not m: + raise AlgorithmNotRecognized(expr) + head = m.group(1) + head = re.sub(r'_ANY\Z', r'', head) + if re.match(r'ED[0-9]+PH\Z', head): + head = 'EDDSA_PREHASH' + return head + + CATEGORY_FROM_HEAD = { + 'SHA': AlgorithmCategory.HASH, + 'SHAKE256_512': AlgorithmCategory.HASH, + 'MD': AlgorithmCategory.HASH, + 'RIPEMD': AlgorithmCategory.HASH, + 'ANY_HASH': AlgorithmCategory.HASH, + 'HMAC': AlgorithmCategory.MAC, + 'STREAM_CIPHER': AlgorithmCategory.CIPHER, + 'CHACHA20_POLY1305': AlgorithmCategory.AEAD, + 'DSA': AlgorithmCategory.SIGN, + 'ECDSA': AlgorithmCategory.SIGN, + 'EDDSA': AlgorithmCategory.SIGN, + 'PURE_EDDSA': AlgorithmCategory.SIGN, + 'RSA_PSS': AlgorithmCategory.SIGN, + 'RSA_PKCS1V15_SIGN': AlgorithmCategory.SIGN, + 'RSA_PKCS1V15_CRYPT': AlgorithmCategory.ASYMMETRIC_ENCRYPTION, + 'RSA_OAEP': AlgorithmCategory.ASYMMETRIC_ENCRYPTION, + 'HKDF': AlgorithmCategory.KEY_DERIVATION, + 'TLS12_PRF': AlgorithmCategory.KEY_DERIVATION, + 'TLS12_PSK_TO_MS': AlgorithmCategory.KEY_DERIVATION, + 'PBKDF': AlgorithmCategory.KEY_DERIVATION, + 'ECDH': AlgorithmCategory.KEY_AGREEMENT, + 'FFDH': AlgorithmCategory.KEY_AGREEMENT, + # KEY_AGREEMENT(...) is a key derivation with a key agreement component + 'KEY_AGREEMENT': AlgorithmCategory.KEY_DERIVATION, + 'JPAKE': AlgorithmCategory.PAKE, + } + for x in BLOCK_MAC_MODES: + CATEGORY_FROM_HEAD[x] = AlgorithmCategory.MAC + for x in BLOCK_CIPHER_MODES: + CATEGORY_FROM_HEAD[x] = AlgorithmCategory.CIPHER + for x in BLOCK_AEAD_MODES: + CATEGORY_FROM_HEAD[x] = AlgorithmCategory.AEAD + + def determine_category(self, expr: str, head: str) -> AlgorithmCategory: + """Return the category of the given algorithm expression. + + This function does not attempt to detect invalid inputs. + """ + prefix = head + while prefix: + if prefix in self.CATEGORY_FROM_HEAD: + return self.CATEGORY_FROM_HEAD[prefix] + if re.match(r'.*[0-9]\Z', prefix): + prefix = re.sub(r'_*[0-9]+\Z', r'', prefix) + else: + prefix = re.sub(r'_*[^_]*\Z', r'', prefix) + raise AlgorithmNotRecognized(expr) + + @staticmethod + def determine_wildcard(expr) -> bool: + """Whether the given algorithm expression is a wildcard. + + This function does not attempt to detect invalid inputs. + """ + if re.search(r'\bPSA_ALG_ANY_HASH\b', expr): + return True + if re.search(r'_AT_LEAST_', expr): + return True + return False + + def __init__(self, expr: str) -> None: + """Analyze an algorithm value. + + The algorithm must be expressed as a C expression containing only + calls to PSA algorithm constructor macros and numeric literals. + + This class is only programmed to handle valid expressions. Invalid + expressions may result in exceptions or in nonsensical results. + """ + self.expression = re.sub(r'\s+', r'', expr) + self.base_expression = self.determine_base(self.expression) + self.head = self.determine_head(self.base_expression) + self.category = self.determine_category(self.base_expression, self.head) + self.is_wildcard = self.determine_wildcard(self.expression) From 8345d6369516031e9623840db0ecbddc029f7053 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 20:38:47 +0200 Subject: [PATCH 08/21] Add knowledge of the compatibility of key types and algorithms Determine key types that are compatible with an algorithm based on their names. Key derivation and PAKE are not yet supported. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 62 +++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 072d53d933..adda02f520 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -25,6 +25,7 @@ from typing import Dict, Iterable, Optional, Pattern, Tuple from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA +BLOCK_CIPHERS = frozenset(['AES', 'ARIA', 'CAMELLIA', 'DES']) BLOCK_MAC_MODES = frozenset(['CBC_MAC', 'CMAC']) BLOCK_CIPHER_MODES = frozenset([ 'CTR', 'CFB', 'OFB', 'XTS', 'CCM_STAR_NO_TAG', @@ -32,6 +33,25 @@ BLOCK_CIPHER_MODES = frozenset([ ]) BLOCK_AEAD_MODES = frozenset(['CCM', 'GCM']) +class EllipticCurveCategory(enum.Enum): + """Categorization of elliptic curve families. + + The category of a curve determines what algorithms are defined over it. + """ + + SHORT_WEIERSTRASS = 0 + MONTGOMERY = 1 + TWISTED_EDWARDS = 2 + + @staticmethod + def from_family(family: str) -> 'EllipticCurveCategory': + if family == 'PSA_ECC_FAMILY_MONTGOMERY': + return EllipticCurveCategory.MONTGOMERY + if family == 'PSA_ECC_FAMILY_TWISTED_EDWARDS': + return EllipticCurveCategory.TWISTED_EDWARDS + # Default to SW, which most curves belong to. + return EllipticCurveCategory.SHORT_WEIERSTRASS + class KeyType: """Knowledge about a PSA key type.""" @@ -72,6 +92,11 @@ class KeyType: if self.params is not None: self.expression += '(' + ', '.join(self.params) + ')' + m = re.match(r'PSA_KEY_TYPE_(\w+)', self.name) + assert m + self.head = re.sub(r'_(?:PUBLIC_KEY|KEY_PAIR)\Z', r'', m.group(1)) + """The key type macro name, with common prefixes and suffixes stripped.""" + self.private_type = re.sub(r'_PUBLIC_KEY\Z', r'_KEY_PAIR', self.name) """The key type macro name for the corresponding key pair type. @@ -164,6 +189,43 @@ class KeyType: # 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. + + This function does not currently handle key derivation or PAKE. + """ + #pylint: disable=too-many-return-statements + if alg.is_wildcard: + return False + if self.head == 'HMAC' and alg.head == 'HMAC': + return True + if self.head in BLOCK_CIPHERS and \ + alg.head in frozenset.union(BLOCK_MAC_MODES, + BLOCK_CIPHER_MODES, + BLOCK_AEAD_MODES): + return True + if self.head == 'CHACHA20' and alg.head == 'CHACHA20_POLY1305': + return True + if self.head in {'ARC4', 'CHACHA20'} and \ + alg.head == 'STREAM_CIPHER': + return True + if self.head == 'RSA' and alg.head.startswith('RSA_'): + return True + if self.head == 'ECC': + assert self.params is not None + eccc = EllipticCurveCategory.from_family(self.params[0]) + if alg.head == 'ECDH' and \ + eccc in {EllipticCurveCategory.SHORT_WEIERSTRASS, + EllipticCurveCategory.MONTGOMERY}: + return True + if alg.head == 'ECDSA' and \ + eccc == EllipticCurveCategory.SHORT_WEIERSTRASS: + return True + if alg.head in {'PURE_EDDSA', 'EDDSA_PREHASH'} and \ + eccc == EllipticCurveCategory.TWISTED_EDWARDS: + return True + return False + class AlgorithmCategory(enum.Enum): """PSA algorithm categories.""" From cba28a7d403d9bb842d9fdf33ec2acc285c3e576 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 17:26:33 +0100 Subject: [PATCH 09/21] Systematically generate test cases for operation setup failure The test suite test_suite_psa_crypto_op_fail now runs a large number of automatically generated test cases which attempt to perform a one-shot operation or to set up a multi-part operation with invalid parameters. The following cases are fully covered (based on the enumeration of valid algorithms and key types): * An algorithm is not supported. * The key type is not compatible with the algorithm (for operations that use a key). * The algorithm is not compatible for the operation. Some test functions allow the library to return PSA_ERROR_NOT_SUPPORTED where the test code generator expects PSA_ERROR_INVALID_ARGUMENT or vice versa. This may be refined in the future. Some corner cases with algorithms combining a key agreement with a key derivation are not handled properly. This will be fixed in follow-up commits. Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 130 +++++-- .../test_suite_psa_crypto_op_fail.function | 329 ++++++++++++++++++ 2 files changed, 438 insertions(+), 21 deletions(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index e7819b91c5..f82a1e5b0f 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -21,6 +21,7 @@ generate only the specified files. # limitations under the License. import argparse +import enum import os import posixpath import re @@ -309,39 +310,126 @@ class OpFail: """Generate test cases for operations that must fail.""" #pylint: disable=too-few-public-methods + class Reason(enum.Enum): + NOT_SUPPORTED = 0 + INVALID = 1 + INCOMPATIBLE = 2 + def __init__(self, info: Information) -> None: self.constructors = info.constructors + key_type_expressions = self.constructors.generate_expressions( + sorted(self.constructors.key_types) + ) + self.key_types = [crypto_knowledge.KeyType(kt_expr) + for kt_expr in key_type_expressions] - @staticmethod - def hash_test_cases(alg: str) -> Iterator[test_case.TestCase]: - """Generate hash failure test cases for the specified algorithm.""" + def make_test_case( + self, + alg: crypto_knowledge.Algorithm, + category: crypto_knowledge.AlgorithmCategory, + reason: 'Reason', + kt: Optional[crypto_knowledge.KeyType] = None, + not_deps: FrozenSet[str] = frozenset(), + ) -> test_case.TestCase: + """Construct a failure test case for a one-key or keyless operation.""" + #pylint: disable=too-many-arguments,too-many-locals tc = test_case.TestCase() - is_hash = (alg.startswith('PSA_ALG_SHA') or - alg.startswith('PSA_ALG_MD') or - alg in frozenset(['PSA_ALG_RIPEMD160', 'PSA_ALG_ANY_HASH'])) - if is_hash: - descr = 'not supported' - status = 'PSA_ERROR_NOT_SUPPORTED' - dependencies = ['!PSA_WANT_' + alg[4:]] + pretty_alg = re.sub(r'PSA_ALG_', r'', alg.expression) + pretty_reason = reason.name.lower() + if kt: + key_type = kt.expression + pretty_type = re.sub(r'PSA_KEY_TYPE_', r'', key_type) else: - descr = 'invalid' - status = 'PSA_ERROR_INVALID_ARGUMENT' - dependencies = automatic_dependencies(alg) - tc.set_description('PSA hash {}: {}' - .format(descr, re.sub(r'PSA_ALG_', r'', alg))) + key_type = '' + pretty_type = '' + tc.set_description('PSA {} {}: {}{}' + .format(category.name.lower(), + pretty_alg, + pretty_reason, + ' with ' + pretty_type if pretty_type else '')) + dependencies = automatic_dependencies(alg.base_expression, key_type) + for i, dep in enumerate(dependencies): + if dep in not_deps: + dependencies[i] = '!' + dep tc.set_dependencies(dependencies) - tc.set_function('hash_fail') - tc.set_arguments([alg, status]) - yield tc + tc.set_function(category.name.lower() + '_fail') + arguments = [] + if kt: + key_material = kt.key_material(kt.sizes_to_test()[0]) + arguments += [key_type, test_case.hex_string(key_material)] + arguments.append(alg.expression) + error = ('NOT_SUPPORTED' if reason == self.Reason.NOT_SUPPORTED else + 'INVALID_ARGUMENT') + arguments.append('PSA_ERROR_' + error) + tc.set_arguments(arguments) + return tc - def test_cases_for_algorithm(self, alg: str) -> Iterator[test_case.TestCase]: + def no_key_test_cases( + self, + alg: crypto_knowledge.Algorithm, + category: crypto_knowledge.AlgorithmCategory, + ) -> Iterator[test_case.TestCase]: + """Generate failure test cases for keyless operations with the specified algorithm.""" + if category == alg.category: + # Compatible operation, unsupported algorithm + for dep in automatic_dependencies(alg.base_expression): + yield self.make_test_case(alg, category, + self.Reason.NOT_SUPPORTED, + not_deps=frozenset([dep])) + else: + # Incompatible operation, supported algorithm + yield self.make_test_case(alg, category, self.Reason.INVALID) + + def one_key_test_cases( + self, + alg: crypto_knowledge.Algorithm, + category: crypto_knowledge.AlgorithmCategory, + ) -> Iterator[test_case.TestCase]: + """Generate failure test cases for one-key operations with the specified algorithm.""" + for kt in self.key_types: + key_is_compatible = kt.can_do(alg) + # To do: public key for a private key operation + if key_is_compatible and category == alg.category: + # Compatible key and operation, unsupported algorithm + for dep in automatic_dependencies(alg.base_expression): + yield self.make_test_case(alg, category, + self.Reason.NOT_SUPPORTED, + kt=kt, not_deps=frozenset([dep])) + elif key_is_compatible: + # Compatible key, incompatible operation, supported algorithm + yield self.make_test_case(alg, category, + self.Reason.INVALID, + kt=kt) + elif category == alg.category: + # Incompatible key, compatible operation, supported algorithm + yield self.make_test_case(alg, category, + self.Reason.INCOMPATIBLE, + kt=kt) + else: + # Incompatible key and operation. Don't test cases where + # multiple things are wrong, to keep the number of test + # cases reasonable. + pass + + def test_cases_for_algorithm( + self, + alg: crypto_knowledge.Algorithm, + ) -> Iterator[test_case.TestCase]: """Generate operation failure test cases for the specified algorithm.""" - yield from self.hash_test_cases(alg) + for category in crypto_knowledge.AlgorithmCategory: + if category == crypto_knowledge.AlgorithmCategory.PAKE: + # PAKE operations are not implemented yet + pass + elif category.requires_key(): + yield from self.one_key_test_cases(alg, category) + else: + yield from self.no_key_test_cases(alg, category) def all_test_cases(self) -> Iterator[test_case.TestCase]: """Generate all test cases for operations that must fail.""" algorithms = sorted(self.constructors.algorithms) - for alg in self.constructors.generate_expressions(algorithms): + for expr in self.constructors.generate_expressions(algorithms): + alg = crypto_knowledge.Algorithm(expr) yield from self.test_cases_for_algorithm(alg) diff --git a/tests/suites/test_suite_psa_crypto_op_fail.function b/tests/suites/test_suite_psa_crypto_op_fail.function index 21dbafaf97..4640649e54 100644 --- a/tests/suites/test_suite_psa_crypto_op_fail.function +++ b/tests/suites/test_suite_psa_crypto_op_fail.function @@ -3,6 +3,37 @@ #include "psa/crypto.h" #include "test/psa_crypto_helpers.h" +static int test_equal_status( const char *test, + int line_no, const char* filename, + psa_status_t value1, + psa_status_t value2 ) +{ + if( ( value1 == PSA_ERROR_INVALID_ARGUMENT && + value2 == PSA_ERROR_NOT_SUPPORTED ) || + ( value1 == PSA_ERROR_NOT_SUPPORTED && + value2 == PSA_ERROR_INVALID_ARGUMENT ) ) + { + return( 1 ); + } + return( mbedtls_test_equal( test, line_no, filename, value1, value2 ) ); +} + +/** Like #TEST_EQUAL, but expects #psa_status_t values and treats + * #PSA_ERROR_INVALID_ARGUMENT and #PSA_ERROR_NOT_SUPPORTED as + * interchangeable. + * + * This test suite currently allows NOT_SUPPORTED and INVALID_ARGUMENT + * to be interchangeable in places where the library's behavior does not + * match the strict expectations of the test case generator. In the long + * run, it would be better to clarify the expectations and reconcile the + * library and the test case generator. + */ +#define TEST_STATUS( expr1, expr2 ) \ + do { \ + if( ! test_equal_status( #expr1 " == " #expr2, __LINE__, __FILE__, \ + expr1, expr2 ) ) \ + goto exit; \ + } while( 0 ) /* END_HEADER */ @@ -37,3 +68,301 @@ exit: PSA_DONE( ); } /* END_CASE */ + +/* BEGIN_CASE */ +void mac_fail( int key_type_arg, data_t *key_data, + int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + uint8_t input[1] = {'A'}; + uint8_t output[PSA_MAC_MAX_SIZE] = {0}; + size_t length = SIZE_MAX; + + PSA_INIT( ); + + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_SIGN_HASH | + PSA_KEY_USAGE_VERIFY_HASH ); + psa_set_key_algorithm( &attributes, alg ); + PSA_ASSERT( psa_import_key( &attributes, + key_data->x, key_data->len, + &key_id ) ); + + TEST_STATUS( expected_status, + psa_mac_sign_setup( &operation, key_id, alg ) ); + TEST_STATUS( expected_status, + psa_mac_verify_setup( &operation, key_id, alg ) ); + TEST_STATUS( expected_status, + psa_mac_compute( key_id, alg, + input, sizeof( input ), + output, sizeof( output ), &length ) ); + TEST_STATUS( expected_status, + psa_mac_verify( key_id, alg, + input, sizeof( input ), + output, sizeof( output ) ) ); + +exit: + psa_mac_abort( &operation ); + psa_destroy_key( key_id ); + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void cipher_fail( int key_type_arg, data_t *key_data, + int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + uint8_t input[1] = {'A'}; + uint8_t output[64] = {0}; + size_t length = SIZE_MAX; + + PSA_INIT( ); + + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_ENCRYPT | + PSA_KEY_USAGE_DECRYPT ); + psa_set_key_algorithm( &attributes, alg ); + PSA_ASSERT( psa_import_key( &attributes, + key_data->x, key_data->len, + &key_id ) ); + + TEST_STATUS( expected_status, + psa_cipher_encrypt_setup( &operation, key_id, alg ) ); + TEST_STATUS( expected_status, + psa_cipher_decrypt_setup( &operation, key_id, alg ) ); + TEST_STATUS( expected_status, + psa_cipher_encrypt( key_id, alg, + input, sizeof( input ), + output, sizeof( output ), &length ) ); + TEST_STATUS( expected_status, + psa_cipher_decrypt( key_id, alg, + input, sizeof( input ), + output, sizeof( output ), &length ) ); + +exit: + psa_cipher_abort( &operation ); + psa_destroy_key( key_id ); + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void aead_fail( int key_type_arg, data_t *key_data, + int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_aead_operation_t operation = PSA_AEAD_OPERATION_INIT; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + uint8_t input[16] = "ABCDEFGHIJKLMNO"; + uint8_t output[64] = {0}; + size_t length = SIZE_MAX; + + PSA_INIT( ); + + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_ENCRYPT | + PSA_KEY_USAGE_DECRYPT ); + psa_set_key_algorithm( &attributes, alg ); + PSA_ASSERT( psa_import_key( &attributes, + key_data->x, key_data->len, + &key_id ) ); + + TEST_STATUS( expected_status, + psa_aead_encrypt_setup( &operation, key_id, alg ) ); + TEST_STATUS( expected_status, + psa_aead_decrypt_setup( &operation, key_id, alg ) ); + TEST_STATUS( expected_status, + psa_aead_encrypt( key_id, alg, + input, sizeof( input ), + NULL, 0, input, sizeof( input ), + output, sizeof( output ), &length ) ); + TEST_STATUS( expected_status, + psa_aead_decrypt( key_id, alg, + input, sizeof( input ), + NULL, 0, input, sizeof( input ), + output, sizeof( output ), &length ) ); + +exit: + psa_aead_abort( &operation ); + psa_destroy_key( key_id ); + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void sign_fail( int key_type_arg, data_t *key_data, + int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + uint8_t input[1] = {'A'}; + uint8_t output[PSA_SIGNATURE_MAX_SIZE] = {0}; + size_t length = SIZE_MAX; + + PSA_INIT( ); + + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_SIGN_HASH | + PSA_KEY_USAGE_VERIFY_HASH ); + psa_set_key_algorithm( &attributes, alg ); + PSA_ASSERT( psa_import_key( &attributes, + key_data->x, key_data->len, + &key_id ) ); + + TEST_STATUS( expected_status, + psa_sign_hash( key_id, alg, + input, sizeof( input ), + output, sizeof( output ), &length ) ); + TEST_STATUS( expected_status, + psa_verify_hash( key_id, alg, + input, sizeof( input ), + output, sizeof( output ) ) ); + +exit: + psa_destroy_key( key_id ); + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void asymmetric_encryption_fail( int key_type_arg, data_t *key_data, + int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + uint8_t plaintext[PSA_ASYMMETRIC_DECRYPT_OUTPUT_MAX_SIZE] = {0}; + uint8_t ciphertext[PSA_ASYMMETRIC_ENCRYPT_OUTPUT_MAX_SIZE] = {0}; + size_t length = SIZE_MAX; + + PSA_INIT( ); + + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_ENCRYPT | + PSA_KEY_USAGE_DECRYPT ); + psa_set_key_algorithm( &attributes, alg ); + PSA_ASSERT( psa_import_key( &attributes, + key_data->x, key_data->len, + &key_id ) ); + + TEST_STATUS( expected_status, + psa_asymmetric_encrypt( key_id, alg, + plaintext, 1, + NULL, 0, + ciphertext, sizeof( ciphertext ), + &length ) ); + TEST_STATUS( expected_status, + psa_asymmetric_decrypt( key_id, alg, + ciphertext, sizeof( ciphertext ), + NULL, 0, + plaintext, sizeof( plaintext ), + &length ) ); + +exit: + psa_destroy_key( key_id ); + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void key_derivation_fail( int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_algorithm_t alg = alg_arg; + psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; + + PSA_INIT( ); + + TEST_EQUAL( expected_status, + psa_key_derivation_setup( &operation, alg ) ); + +exit: + psa_key_derivation_abort( &operation ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void key_agreement_fail( int key_type_arg, data_t *key_data, + int alg_arg, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + uint8_t public_key[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE] = {0}; + size_t public_key_length = SIZE_MAX; + uint8_t output[PSA_SIGNATURE_MAX_SIZE] = {0}; + size_t length = SIZE_MAX; + psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; + + PSA_INIT( ); + + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_DERIVE ); + psa_set_key_algorithm( &attributes, alg ); + PSA_ASSERT( psa_import_key( &attributes, + key_data->x, key_data->len, + &key_id ) ); + if( PSA_KEY_TYPE_IS_KEY_PAIR( key_type ) || + PSA_KEY_TYPE_IS_PUBLIC_KEY( key_type ) ) + { + PSA_ASSERT( psa_export_public_key( key_id, + public_key, sizeof( public_key ), + &public_key_length ) ); + } + + TEST_STATUS( expected_status, + psa_raw_key_agreement( alg, key_id, + public_key, public_key_length, + output, sizeof( output ), &length ) ); + +#if defined(PSA_WANT_ALG_HKDF) && defined(PSA_WANT_ALG_SHA_256) + PSA_ASSERT( psa_key_derivation_setup( &operation, + PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ) ); + TEST_STATUS( expected_status, + psa_key_derivation_key_agreement( + &operation, + PSA_KEY_DERIVATION_INPUT_SECRET, + key_id, + public_key, public_key_length ) ); +#endif + +exit: + psa_key_derivation_abort( &operation ); + psa_destroy_key( key_id ); + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); +} +/* END_CASE */ From ae3a1008b7a028abfeb19d18054bee9bd00b7552 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 19:50:30 +0100 Subject: [PATCH 10/21] Add a few manual test cases They're redundant with the automatically generated test cases, but it's useful to have them when debugging issues with the test code. Signed-off-by: Gilles Peskine --- .../test_suite_psa_crypto_op_fail.misc.data | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 tests/suites/test_suite_psa_crypto_op_fail.misc.data diff --git a/tests/suites/test_suite_psa_crypto_op_fail.misc.data b/tests/suites/test_suite_psa_crypto_op_fail.misc.data new file mode 100644 index 0000000000..8ba6d94771 --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_op_fail.misc.data @@ -0,0 +1,15 @@ +# Most operation failure test cases are automatically generated in +# test_suite_psa_crypto_op_fail.generated.data. The manually written +# test cases in this file are only intended to help with debugging the +# test code. + +PSA hash: invalid algorithm +hash_fail:PSA_ALG_ECDSA_ANY:PSA_ERROR_INVALID_ARGUMENT + +PSA sign RSA_PSS(SHA_256): incompatible key type +depends_on:PSA_WANT_ALG_RSA_PSS:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES +sign_fail:PSA_KEY_TYPE_AES:"48657265006973206b6579a064617461":PSA_ALG_RSA_PSS(PSA_ALG_SHA_256):PSA_ERROR_INVALID_ARGUMENT + +PSA sign RSA_PSS(SHA_256): RSA_PSS not enabled, key pair +depends_on:!PSA_WANT_ALG_RSA_PSS:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR +sign_fail:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_ALG_RSA_PSS(PSA_ALG_SHA_256):PSA_ERROR_NOT_SUPPORTED From d79e3b92fa88e514fad03b30a8722897faa0f7f4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 21:35:03 +0200 Subject: [PATCH 11/21] In NOT_SUPPORTED test case descriptions, show what is not supported Signed-off-by: Gilles Peskine --- tests/scripts/generate_psa_tests.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index f82a1e5b0f..6c05b89377 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -335,7 +335,12 @@ class OpFail: #pylint: disable=too-many-arguments,too-many-locals tc = test_case.TestCase() pretty_alg = re.sub(r'PSA_ALG_', r'', alg.expression) - pretty_reason = reason.name.lower() + 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)) + else: + pretty_reason = reason.name.lower() if kt: key_type = kt.expression pretty_type = re.sub(r'PSA_KEY_TYPE_', r'', key_type) From a401386f82af96fa40958d325f89dd7e64636fcc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 20:54:40 +0200 Subject: [PATCH 12/21] A key agreement algorithm can contain a key derivation PSA_ALG_KEY_AGREEMENT(..., kdf) is a valid key derivation algorithm when kdf is one. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 20 ++++++++++++++++++++ tests/scripts/generate_psa_tests.py | 6 +++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index adda02f520..73890f81a4 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -366,3 +366,23 @@ class Algorithm: self.head = self.determine_head(self.base_expression) self.category = self.determine_category(self.base_expression, self.head) self.is_wildcard = self.determine_wildcard(self.expression) + + def is_key_agreement_with_derivation(self) -> bool: + """Whether this is a combined key agreement and key derivation algorithm.""" + if self.category != AlgorithmCategory.KEY_AGREEMENT: + return False + m = re.match(r'PSA_ALG_KEY_AGREEMENT\(\w+,\s*(.*)\)\Z', self.expression) + if not m: + return False + kdf_alg = m.group(1) + # Assume kdf_alg is either a valid KDF or 0. + return not re.match(r'(?:0[Xx])?0+\s*\Z', kdf_alg) + + def can_do(self, category: AlgorithmCategory) -> bool: + """Whether this algorithm fits the specified operation category.""" + if category == self.category: + return True + if category == AlgorithmCategory.KEY_DERIVATION and \ + self.is_key_agreement_with_derivation(): + return True + return False diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 6c05b89377..81abce7bcb 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -375,7 +375,7 @@ class OpFail: category: crypto_knowledge.AlgorithmCategory, ) -> Iterator[test_case.TestCase]: """Generate failure test cases for keyless operations with the specified algorithm.""" - if category == alg.category: + if alg.can_do(category): # Compatible operation, unsupported algorithm for dep in automatic_dependencies(alg.base_expression): yield self.make_test_case(alg, category, @@ -394,7 +394,7 @@ class OpFail: for kt in self.key_types: key_is_compatible = kt.can_do(alg) # To do: public key for a private key operation - if key_is_compatible and category == alg.category: + if key_is_compatible and alg.can_do(category): # Compatible key and operation, unsupported algorithm for dep in automatic_dependencies(alg.base_expression): yield self.make_test_case(alg, category, @@ -405,7 +405,7 @@ class OpFail: yield self.make_test_case(alg, category, self.Reason.INVALID, kt=kt) - elif category == alg.category: + elif alg.can_do(category): # Incompatible key, compatible operation, supported algorithm yield self.make_test_case(alg, category, self.Reason.INCOMPATIBLE, From 9efde4f2ecb89fa31f7c7b450791499b73d1a8f2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 21:10:00 +0200 Subject: [PATCH 13/21] Simplify is_kdf_alg_supported in psa_key_derivation_setup_kdf No behavior change. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 62 +++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b2c21528c3..ebf1ea7a30 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5019,51 +5019,47 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut /****************************************************************/ #if defined(AT_LEAST_ONE_BUILTIN_KDF) +static int is_kdf_alg_supported( psa_algorithm_t kdf_alg ) +{ +#if defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF) + if( PSA_ALG_IS_HKDF( kdf_alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_TLS12_PRF) + if( PSA_ALG_IS_TLS12_PRF( kdf_alg ) ) + return( 1 ); +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_TLS12_PSK_TO_MS) + if( PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) + return( 1 ); +#endif + return( 0 ); +} + static psa_status_t psa_key_derivation_setup_kdf( psa_key_derivation_operation_t *operation, psa_algorithm_t kdf_alg ) { - int is_kdf_alg_supported; - /* Make sure that operation->ctx is properly zero-initialised. (Macro * initialisers for this union leave some bytes unspecified.) */ memset( &operation->ctx, 0, sizeof( operation->ctx ) ); /* Make sure that kdf_alg is a supported key derivation algorithm. */ -#if defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF) - if( PSA_ALG_IS_HKDF( kdf_alg ) ) - is_kdf_alg_supported = 1; - else -#endif -#if defined(MBEDTLS_PSA_BUILTIN_ALG_TLS12_PRF) - if( PSA_ALG_IS_TLS12_PRF( kdf_alg ) ) - is_kdf_alg_supported = 1; - else -#endif -#if defined(MBEDTLS_PSA_BUILTIN_ALG_TLS12_PSK_TO_MS) - if( PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) - is_kdf_alg_supported = 1; - else -#endif - is_kdf_alg_supported = 0; + if( ! is_kdf_alg_supported( kdf_alg ) ) + return( PSA_ERROR_NOT_SUPPORTED ); - if( is_kdf_alg_supported ) + psa_algorithm_t hash_alg = PSA_ALG_HKDF_GET_HASH( kdf_alg ); + size_t hash_size = PSA_HASH_LENGTH( hash_alg ); + if( hash_size == 0 ) + return( PSA_ERROR_NOT_SUPPORTED ); + if( ( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || + PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) && + ! ( hash_alg == PSA_ALG_SHA_256 || hash_alg == PSA_ALG_SHA_384 ) ) { - psa_algorithm_t hash_alg = PSA_ALG_HKDF_GET_HASH( kdf_alg ); - size_t hash_size = PSA_HASH_LENGTH( hash_alg ); - if( hash_size == 0 ) - return( PSA_ERROR_NOT_SUPPORTED ); - if( ( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || - PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) && - ! ( hash_alg == PSA_ALG_SHA_256 || hash_alg == PSA_ALG_SHA_384 ) ) - { - return( PSA_ERROR_NOT_SUPPORTED ); - } - operation->capacity = 255 * hash_size; - return( PSA_SUCCESS ); + return( PSA_ERROR_NOT_SUPPORTED ); } - - return( PSA_ERROR_NOT_SUPPORTED ); + operation->capacity = 255 * hash_size; + return( PSA_SUCCESS ); } #endif /* AT_LEAST_ONE_BUILTIN_KDF */ From 0cc417d34b582295ccbf6aa2c1b518425f9e7f9b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 21:18:14 +0200 Subject: [PATCH 14/21] Make psa_key_derivation_setup return early if the hash is not supported Otherwise the systematically generated algorithm-not-supported tests complain when they try to start an operation and succeed. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ebf1ea7a30..1758824957 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5036,6 +5036,14 @@ static int is_kdf_alg_supported( psa_algorithm_t kdf_alg ) return( 0 ); } +static psa_status_t psa_hash_try_support( psa_algorithm_t alg ) +{ + psa_hash_operation_t operation = PSA_HASH_OPERATION_INIT; + psa_status_t status = psa_hash_setup( &operation, alg ); + psa_hash_abort( &operation ); + return( status ); +} + static psa_status_t psa_key_derivation_setup_kdf( psa_key_derivation_operation_t *operation, psa_algorithm_t kdf_alg ) @@ -5048,16 +5056,27 @@ static psa_status_t psa_key_derivation_setup_kdf( if( ! is_kdf_alg_supported( kdf_alg ) ) return( PSA_ERROR_NOT_SUPPORTED ); + /* All currently supported key derivation algorithms are based on a + * hash algorithm. */ psa_algorithm_t hash_alg = PSA_ALG_HKDF_GET_HASH( kdf_alg ); size_t hash_size = PSA_HASH_LENGTH( hash_alg ); if( hash_size == 0 ) return( PSA_ERROR_NOT_SUPPORTED ); + + /* Make sure that hash_alg is a supported hash algorithm. Otherwise + * we might fail later, which is somewhat unfriendly and potentially + * risk-prone. */ + psa_status_t status = psa_hash_try_support( hash_alg ); + if( status != PSA_SUCCESS ) + return( status ); + if( ( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) && ! ( hash_alg == PSA_ALG_SHA_256 || hash_alg == PSA_ALG_SHA_384 ) ) { return( PSA_ERROR_NOT_SUPPORTED ); } + operation->capacity = 255 * hash_size; return( PSA_SUCCESS ); } From 0c3a071300777c3b10628f74c4f64605df9a8666 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 21:34:33 +0200 Subject: [PATCH 15/21] Make psa_key_derivation_setup return early if the key agreement is not supported Otherwise the systematically generated algorithm-not-supported tests complain when they try to start an operation and succeed. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 1758824957..661b152623 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5080,6 +5080,16 @@ static psa_status_t psa_key_derivation_setup_kdf( operation->capacity = 255 * hash_size; return( PSA_SUCCESS ); } + +static psa_status_t psa_key_agreement_try_support( psa_algorithm_t alg ) +{ +#if defined(PSA_WANT_ALG_ECDH) + if( alg == PSA_ALG_ECDH ) + return( PSA_SUCCESS ); +#endif + (void) alg; + return( PSA_ERROR_NOT_SUPPORTED ); +} #endif /* AT_LEAST_ONE_BUILTIN_KDF */ psa_status_t psa_key_derivation_setup( psa_key_derivation_operation_t *operation, @@ -5096,6 +5106,10 @@ psa_status_t psa_key_derivation_setup( psa_key_derivation_operation_t *operation { #if defined(AT_LEAST_ONE_BUILTIN_KDF) psa_algorithm_t kdf_alg = PSA_ALG_KEY_AGREEMENT_GET_KDF( alg ); + psa_algorithm_t ka_alg = PSA_ALG_KEY_AGREEMENT_GET_BASE( alg ); + status = psa_key_agreement_try_support( ka_alg ); + if( status != PSA_SUCCESS ) + return( status ); status = psa_key_derivation_setup_kdf( operation, kdf_alg ); #else return( PSA_ERROR_NOT_SUPPORTED ); From e6300959df9d51534f5c5b1e99fed7a0733b68f4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Apr 2021 21:56:59 +0200 Subject: [PATCH 16/21] Test attempts to use a public key for a private-key operation Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/crypto_knowledge.py | 13 +++++++ tests/scripts/generate_psa_tests.py | 9 ++++- .../test_suite_psa_crypto_op_fail.function | 38 ++++++++++++------- .../test_suite_psa_crypto_op_fail.misc.data | 4 +- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 73890f81a4..1bd011fe21 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -104,6 +104,10 @@ class KeyType: `self.name`. """ + def is_public(self) -> bool: + """Whether the key type is for public keys.""" + return self.name.endswith('_PUBLIC_KEY') + ECC_KEY_SIZES = { 'PSA_ECC_FAMILY_SECP_K1': (192, 224, 256), 'PSA_ECC_FAMILY_SECP_R1': (225, 256, 384, 521), @@ -242,8 +246,17 @@ class AlgorithmCategory(enum.Enum): PAKE = 10 def requires_key(self) -> bool: + """Whether operations in this category are set up with a key.""" return self not in {self.HASH, self.KEY_DERIVATION} + def is_asymmetric(self) -> bool: + """Whether operations in this category involve asymmetric keys.""" + return self in { + self.SIGN, + self.ASYMMETRIC_ENCRYPTION, + self.KEY_AGREEMENT + } + class AlgorithmNotRecognized(Exception): def __init__(self, expr: str) -> None: diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 81abce7bcb..ca94d7d324 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -314,6 +314,7 @@ class OpFail: NOT_SUPPORTED = 0 INVALID = 1 INCOMPATIBLE = 2 + PUBLIC = 3 def __init__(self, info: Information) -> None: self.constructors = info.constructors @@ -363,6 +364,8 @@ class OpFail: key_material = kt.key_material(kt.sizes_to_test()[0]) arguments += [key_type, test_case.hex_string(key_material)] arguments.append(alg.expression) + if category.is_asymmetric(): + arguments.append('1' if reason == self.Reason.PUBLIC else '0') error = ('NOT_SUPPORTED' if reason == self.Reason.NOT_SUPPORTED else 'INVALID_ARGUMENT') arguments.append('PSA_ERROR_' + error) @@ -393,13 +396,17 @@ class OpFail: """Generate failure test cases for one-key operations with the specified algorithm.""" for kt in self.key_types: key_is_compatible = kt.can_do(alg) - # To do: public key for a private key operation if key_is_compatible and alg.can_do(category): # Compatible key and operation, unsupported algorithm for dep in automatic_dependencies(alg.base_expression): yield self.make_test_case(alg, category, self.Reason.NOT_SUPPORTED, kt=kt, not_deps=frozenset([dep])) + # Public key for a private-key operation + if category.is_asymmetric() and kt.is_public(): + yield self.make_test_case(alg, category, + self.Reason.PUBLIC, + kt=kt) elif key_is_compatible: # Compatible key, incompatible operation, supported algorithm yield self.make_test_case(alg, category, diff --git a/tests/suites/test_suite_psa_crypto_op_fail.function b/tests/suites/test_suite_psa_crypto_op_fail.function index 4640649e54..4ab8b526d4 100644 --- a/tests/suites/test_suite_psa_crypto_op_fail.function +++ b/tests/suites/test_suite_psa_crypto_op_fail.function @@ -211,7 +211,8 @@ exit: /* BEGIN_CASE */ void sign_fail( int key_type_arg, data_t *key_data, - int alg_arg, int expected_status_arg ) + int alg_arg, int private_only, + int expected_status_arg ) { psa_status_t expected_status = expected_status_arg; psa_key_type_t key_type = key_type_arg; @@ -237,10 +238,13 @@ void sign_fail( int key_type_arg, data_t *key_data, psa_sign_hash( key_id, alg, input, sizeof( input ), output, sizeof( output ), &length ) ); - TEST_STATUS( expected_status, - psa_verify_hash( key_id, alg, - input, sizeof( input ), - output, sizeof( output ) ) ); + if( ! private_only ) + { + TEST_STATUS( expected_status, + psa_verify_hash( key_id, alg, + input, sizeof( input ), + output, sizeof( output ) ) ); + } exit: psa_destroy_key( key_id ); @@ -251,7 +255,8 @@ exit: /* BEGIN_CASE */ void asymmetric_encryption_fail( int key_type_arg, data_t *key_data, - int alg_arg, int expected_status_arg ) + int alg_arg, int private_only, + int expected_status_arg ) { psa_status_t expected_status = expected_status_arg; psa_key_type_t key_type = key_type_arg; @@ -273,12 +278,15 @@ void asymmetric_encryption_fail( int key_type_arg, data_t *key_data, key_data->x, key_data->len, &key_id ) ); - TEST_STATUS( expected_status, - psa_asymmetric_encrypt( key_id, alg, - plaintext, 1, - NULL, 0, - ciphertext, sizeof( ciphertext ), - &length ) ); + if( ! private_only ) + { + TEST_STATUS( expected_status, + psa_asymmetric_encrypt( key_id, alg, + plaintext, 1, + NULL, 0, + ciphertext, sizeof( ciphertext ), + &length ) ); + } TEST_STATUS( expected_status, psa_asymmetric_decrypt( key_id, alg, ciphertext, sizeof( ciphertext ), @@ -313,7 +321,8 @@ exit: /* BEGIN_CASE */ void key_agreement_fail( int key_type_arg, data_t *key_data, - int alg_arg, int expected_status_arg ) + int alg_arg, int private_only, + int expected_status_arg ) { psa_status_t expected_status = expected_status_arg; psa_key_type_t key_type = key_type_arg; @@ -359,6 +368,9 @@ void key_agreement_fail( int key_type_arg, data_t *key_data, public_key, public_key_length ) ); #endif + /* There are no public-key operations. */ + (void) private_only; + exit: psa_key_derivation_abort( &operation ); psa_destroy_key( key_id ); diff --git a/tests/suites/test_suite_psa_crypto_op_fail.misc.data b/tests/suites/test_suite_psa_crypto_op_fail.misc.data index 8ba6d94771..147c3b76fd 100644 --- a/tests/suites/test_suite_psa_crypto_op_fail.misc.data +++ b/tests/suites/test_suite_psa_crypto_op_fail.misc.data @@ -8,8 +8,8 @@ hash_fail:PSA_ALG_ECDSA_ANY:PSA_ERROR_INVALID_ARGUMENT PSA sign RSA_PSS(SHA_256): incompatible key type depends_on:PSA_WANT_ALG_RSA_PSS:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES -sign_fail:PSA_KEY_TYPE_AES:"48657265006973206b6579a064617461":PSA_ALG_RSA_PSS(PSA_ALG_SHA_256):PSA_ERROR_INVALID_ARGUMENT +sign_fail:PSA_KEY_TYPE_AES:"48657265006973206b6579a064617461":PSA_ALG_RSA_PSS(PSA_ALG_SHA_256):0:PSA_ERROR_INVALID_ARGUMENT PSA sign RSA_PSS(SHA_256): RSA_PSS not enabled, key pair depends_on:!PSA_WANT_ALG_RSA_PSS:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR -sign_fail:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_ALG_RSA_PSS(PSA_ALG_SHA_256):PSA_ERROR_NOT_SUPPORTED +sign_fail:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_ALG_RSA_PSS(PSA_ALG_SHA_256):0:PSA_ERROR_NOT_SUPPORTED From b24ed5261ee22cf84fb6034928c18bc516e55fda Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 19:51:53 +0100 Subject: [PATCH 17/21] Use a plausible input size with asymmetric verification Otherwise the error status can be PSA_ERROR_INVALID_SIGNATURE instead of the expected PSA_ERROR_NOT_SUPPORTED in some configurations. For example, the RSA verification code currently checks the signature size first whenever PSA_KEY_TYPE_RSA_PUBLIC_KEY is enabled, and only gets into algorithm-specific code if this passes, so it returns INVALID_SIGNATURE even if the specific algorithm is not supported. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto_op_fail.function | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_op_fail.function b/tests/suites/test_suite_psa_crypto_op_fail.function index 4ab8b526d4..8b50f10291 100644 --- a/tests/suites/test_suite_psa_crypto_op_fail.function +++ b/tests/suites/test_suite_psa_crypto_op_fail.function @@ -240,10 +240,20 @@ void sign_fail( int key_type_arg, data_t *key_data, output, sizeof( output ), &length ) ); if( ! private_only ) { + /* Determine a plausible signature size to avoid an INVALID_SIGNATURE + * error based on this. */ + PSA_ASSERT( psa_get_key_attributes( key_id, &attributes ) ); + size_t key_bits = psa_get_key_bits( &attributes ); + size_t output_length = sizeof( output ); + if( PSA_KEY_TYPE_IS_RSA( key_type ) ) + output_length = PSA_BITS_TO_BYTES( key_bits ); + else if( PSA_KEY_TYPE_IS_ECC( key_type ) ) + output_length = 2 * PSA_BITS_TO_BYTES( key_bits ); + TEST_ASSERT( output_length <= sizeof( output ) ); TEST_STATUS( expected_status, psa_verify_hash( key_id, alg, input, sizeof( input ), - output, sizeof( output ) ) ); + output, output_length ) ); } exit: From 695c4cb7eabbaba7e62833b19e9a6685d726531a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 16 Mar 2022 12:25:17 +0100 Subject: [PATCH 18/21] If a cipher algorithm is not supported, fail during setup In some cases, a cipher operation for an unsupported algorithm could succeed in psa_cipher_{encrypt,decrypt}_setup() and fail only when input is actually fed. This is not a major bug, but it has several minor downsides: fail-late is harder to diagnose for users than fail-early; some code size can be gained; tests that expect failure for not-supported parameters would have to be accommodated to also accept success. This commit at least partially addresses the issue. The only completeness goal in this commit is to pass our full CI, which discovered that disabling only PSA_WANT_ALG_STREAM_CIPHER or PSA_WANT_ALG_ECB_NO_PADDING (but keeping the relevant key type) allowed cipher setup to succeed, which caused failures in test_suite_psa_crypto_op_fail.generated in component_test_psa_crypto_config_accel_xxx. Changes in this commit: * mbedtls_cipher_info_from_psa() now returns NULL for unsupported cipher algorithms. (No change related to key types.) * Some code that is only relevant for ECB is no longer built if PSA_WANT_ALG_ECB_NO_PADDING is disabled. Signed-off-by: Gilles Peskine --- library/psa_crypto_cipher.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index ae30e5fb61..fafe68b077 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -47,39 +47,61 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( { switch( alg ) { +#if defined(MBEDTLS_PSA_BUILTIN_ALG_STREAM_CIPHER) case PSA_ALG_STREAM_CIPHER: mode = MBEDTLS_MODE_STREAM; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CTR) case PSA_ALG_CTR: mode = MBEDTLS_MODE_CTR; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CFB) case PSA_ALG_CFB: mode = MBEDTLS_MODE_CFB; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_OFB) case PSA_ALG_OFB: mode = MBEDTLS_MODE_OFB; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING) case PSA_ALG_ECB_NO_PADDING: mode = MBEDTLS_MODE_ECB; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CBC_NO_PADDING) case PSA_ALG_CBC_NO_PADDING: mode = MBEDTLS_MODE_CBC; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CBC_PKCS7) case PSA_ALG_CBC_PKCS7: mode = MBEDTLS_MODE_CBC; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CCM_STAR_NO_TAG) case PSA_ALG_CCM_STAR_NO_TAG: mode = MBEDTLS_MODE_CCM_STAR_NO_TAG; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CCM) case PSA_ALG_AEAD_WITH_SHORTENED_TAG( PSA_ALG_CCM, 0 ): mode = MBEDTLS_MODE_CCM; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_GCM) case PSA_ALG_AEAD_WITH_SHORTENED_TAG( PSA_ALG_GCM, 0 ): mode = MBEDTLS_MODE_GCM; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_ALG_CHACHA20_POLY1305) case PSA_ALG_AEAD_WITH_SHORTENED_TAG( PSA_ALG_CHACHA20_POLY1305, 0 ): mode = MBEDTLS_MODE_CHACHAPOLY; break; +#endif default: return( NULL ); } @@ -91,12 +113,17 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( switch( key_type ) { +#if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_AES) case PSA_KEY_TYPE_AES: cipher_id_tmp = MBEDTLS_CIPHER_ID_AES; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_ARIA) case PSA_KEY_TYPE_ARIA: cipher_id_tmp = MBEDTLS_CIPHER_ID_ARIA; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_DES) case PSA_KEY_TYPE_DES: /* key_bits is 64 for Single-DES, 128 for two-key Triple-DES, * and 192 for three-key Triple-DES. */ @@ -110,12 +137,17 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( if( key_bits == 128 ) key_bits = 192; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_CAMELLIA) case PSA_KEY_TYPE_CAMELLIA: cipher_id_tmp = MBEDTLS_CIPHER_ID_CAMELLIA; break; +#endif +#if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_CHACHA20) case PSA_KEY_TYPE_CHACHA20: cipher_id_tmp = MBEDTLS_CIPHER_ID_CHACHA20; break; +#endif default: return( NULL ); } @@ -239,6 +271,7 @@ psa_status_t mbedtls_psa_cipher_set_iv( iv, iv_length ) ) ); } +#if defined(MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING) /** Process input for which the algorithm is set to ECB mode. * * This requires manual processing, since the PSA API is defined as being @@ -342,6 +375,7 @@ static psa_status_t psa_cipher_update_ecb( exit: return( status ); } +#endif /* MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING */ psa_status_t mbedtls_psa_cipher_update( mbedtls_psa_cipher_operation_t *operation, @@ -369,6 +403,7 @@ psa_status_t mbedtls_psa_cipher_update( if( output_size < expected_output_size ) return( PSA_ERROR_BUFFER_TOO_SMALL ); +#if defined(MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING) if( operation->alg == PSA_ALG_ECB_NO_PADDING ) { /* mbedtls_cipher_update has an API inconsistency: it will only @@ -381,6 +416,7 @@ psa_status_t mbedtls_psa_cipher_update( output_length ); } else +#endif /* MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING */ { status = mbedtls_to_psa_error( mbedtls_cipher_update( &operation->ctx.cipher, input, From a9b6c8074ad0e5e6738ea0ef7bc23b76a9e506cd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 16 Mar 2022 13:54:49 +0100 Subject: [PATCH 19/21] Fix psa_mac_verify() returning BUFFER_TOO_SMALL It doesn't make sense for psa_mac_verify() to return PSA_ERROR_BUFFER_TOO_SMALL since it doesn't have an output buffer. But this was happening when requesting the verification of an unsupported algorithm whose output size is larger than the maximum supported MAC size, e.g. HMAC-SHA-512 when building with only SHA-256 support. Arrange to return PSA_ERROR_NOT_SUPPORTED instead. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 661b152623..3262e09e21 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2333,6 +2333,20 @@ static psa_status_t psa_mac_finalize_alg_and_key_validation( return( PSA_ERROR_INVALID_ARGUMENT ); } + if( *mac_size > PSA_MAC_MAX_SIZE ) + { + /* PSA_MAC_LENGTH returns the correct length even for a MAC algorithm + * that is disabled in the compile-time configuration. The result can + * therefore be larger than PSA_MAC_MAX_SIZE, which does take the + * configuration into account. In this case, force a return of + * PSA_ERROR_NOT_SUPPORTED here. Otherwise psa_mac_verify(), or + * psa_mac_compute(mac_size=PSA_MAC_MAX_SIZE), would return + * PSA_ERROR_BUFFER_TOO_SMALL for an unsupported algorithm whose MAC size + * is larger than PSA_MAC_MAX_SIZE, which is misleading and which breaks + * systematically generated tests. */ + return( PSA_ERROR_NOT_SUPPORTED ); + } + return( PSA_SUCCESS ); } From 7a2e83b839542d4c25c166dc7a463e7db9723fef Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Mar 2022 11:09:23 +0100 Subject: [PATCH 20/21] Add missing logic for accelerated ECB under MBEDTLS_PSA_CRYPTO_CONFIG Signed-off-by: Gilles Peskine --- include/mbedtls/config_psa.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config_psa.h b/include/mbedtls/config_psa.h index 6eccd5a6cf..c52acc57a2 100644 --- a/include/mbedtls/config_psa.h +++ b/include/mbedtls/config_psa.h @@ -390,7 +390,8 @@ extern "C" { #endif #endif /* PSA_WANT_ALG_XTS */ -#if defined(PSA_WANT_ALG_ECB_NO_PADDING) +#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 #endif From ebfee6e315611940e5c0deb4bcbfd41cbed210dd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Apr 2022 14:08:09 +0200 Subject: [PATCH 21/21] check-generated-files.sh -u: don't update file timestamps When running check-generated-files in update mode, all generated files were regenerated. As a consequence, ``` tests/scripts/check-generated-files.sh -u && make ``` always caused most of the code to be rebuilt. Now, if a file hasn't changed, preserve its original modification time (and other metadata), so the command above doesn't rebuild anything that has actually not changed. Signed-off-by: Gilles Peskine --- tests/scripts/check-generated-files.sh | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/scripts/check-generated-files.sh b/tests/scripts/check-generated-files.sh index f42ecd6fb5..1736f24d25 100755 --- a/tests/scripts/check-generated-files.sh +++ b/tests/scripts/check-generated-files.sh @@ -76,7 +76,7 @@ check() for FILE in "$@"; do if [ -e "$FILE" ]; then - cp "$FILE" "$FILE.bak" + cp -p "$FILE" "$FILE.bak" else rm -f "$FILE.bak" fi @@ -86,17 +86,18 @@ check() # Compare the script output to the old files and remove backups for FILE in "$@"; do - if ! diff "$FILE" "$FILE.bak" >/dev/null 2>&1; then + if diff "$FILE" "$FILE.bak" >/dev/null 2>&1; then + # Move the original file back so that $FILE's timestamp doesn't + # change (avoids spurious rebuilds with make). + mv "$FILE.bak" "$FILE" + else echo "'$FILE' was either modified or deleted by '$SCRIPT'" if [ -z "$UPDATE" ]; then exit 1 + else + rm -f "$FILE.bak" fi fi - if [ -z "$UPDATE" ]; then - mv "$FILE.bak" "$FILE" - else - rm -f "$FILE.bak" - fi done if [ -n "$directory" ]; then