From f070a5e5d5c7c867e10bd5dc06d924c18763b29e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:40:45 +0200 Subject: [PATCH 01/13] Document how PSA identifiers are generally constructed Signed-off-by: Gilles Peskine --- include/psa/crypto_types.h | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index 2cf965d81..ec6890ad0 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -70,10 +70,16 @@ typedef int32_t psa_status_t; */ /** \brief Encoding of a key type. + * + * Values of this type are generally constructed by macros called + * `PSA_KEY_TYPE_xxx`. */ typedef uint16_t psa_key_type_t; /** The type of PSA elliptic curve family identifiers. + * + * Values of this type are generally constructed by macros called + * `PSA_ECC_FAMILY_xxx`. * * The curve identifier is required to create an ECC key using the * PSA_KEY_TYPE_ECC_KEY_PAIR() or PSA_KEY_TYPE_ECC_PUBLIC_KEY() @@ -85,6 +91,9 @@ typedef uint16_t psa_key_type_t; typedef uint8_t psa_ecc_family_t; /** The type of PSA Diffie-Hellman group family identifiers. + * + * Values of this type are generally constructed by macros called + * `PSA_DH_FAMILY_xxx`. * * The group identifier is required to create an Diffie-Hellman key using the * PSA_KEY_TYPE_DH_KEY_PAIR() or PSA_KEY_TYPE_DH_PUBLIC_KEY() @@ -96,6 +105,9 @@ typedef uint8_t psa_ecc_family_t; typedef uint8_t psa_dh_family_t; /** \brief Encoding of a cryptographic algorithm. + * + * Values of this type are generally constructed by macros called + * `PSA_ALG_xxx`. * * For algorithms that can be applied to multiple key types, this type * does not encode the key type. For example, for symmetric ciphers @@ -143,6 +155,9 @@ typedef uint32_t psa_algorithm_t; * #PSA_KEY_LIFETIME_PERSISTENT is supported if persistent storage is * available. Other lifetime values may be supported depending on the * library configuration. + * + * Values of this type are generally constructed by macros called + * `PSA_KEY_LIFETIME_xxx`. */ typedef uint32_t psa_key_lifetime_t; @@ -247,7 +262,11 @@ typedef struct * @{ */ -/** \brief Encoding of permitted usage on a key. */ +/** \brief Encoding of permitted usage on a key. + * + * Values of this type are generally constructed as bitwise-ors of macros + * called `PSA_KEY_USAGE_xxx`. + */ typedef uint32_t psa_key_usage_t; /**@}*/ @@ -376,7 +395,11 @@ typedef uint64_t psa_key_slot_number_t; * @{ */ -/** \brief Encoding of the step of a key derivation. */ +/** \brief Encoding of the step of a key derivation. + * + * Values of this type are generally constructed by macros called + * `PSA_KEY_DERIVATION_INPUT_xxx`. + */ typedef uint16_t psa_key_derivation_step_t; /**@}*/ From 7973399f7bf03448ac91c9550365622ab10831f0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:41:20 +0200 Subject: [PATCH 02/13] Add compatibility notes regarding values embedded in the key store Certain numerical values are written to the key store. Changing those numerical values would break the backward compatibility of stored keys. Add a note to the affected types. Add comments near the definitions of affected values. Signed-off-by: Gilles Peskine --- include/psa/crypto_types.h | 58 +++++++++++++++++++++++++++++++++++++ include/psa/crypto_values.h | 39 +++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index ec6890ad0..e619cf537 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -73,6 +73,11 @@ typedef int32_t psa_status_t; * * Values of this type are generally constructed by macros called * `PSA_KEY_TYPE_xxx`. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint16_t psa_key_type_t; @@ -87,6 +92,11 @@ typedef uint16_t psa_key_type_t; * * Values defined by this standard will never be in the range 0x80-0xff. * Vendors who define additional families must use an encoding in this range. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint8_t psa_ecc_family_t; @@ -101,6 +111,11 @@ typedef uint8_t psa_ecc_family_t; * * Values defined by this standard will never be in the range 0x80-0xff. * Vendors who define additional families must use an encoding in this range. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint8_t psa_dh_family_t; @@ -114,6 +129,11 @@ typedef uint8_t psa_dh_family_t; * based on a block cipher, #psa_algorithm_t encodes the block cipher * mode and the padding mode while the block cipher itself is encoded * via #psa_key_type_t. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint32_t psa_algorithm_t; @@ -158,6 +178,11 @@ typedef uint32_t psa_algorithm_t; * * Values of this type are generally constructed by macros called * `PSA_KEY_LIFETIME_xxx`. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint32_t psa_key_lifetime_t; @@ -189,6 +214,11 @@ typedef uint32_t psa_key_lifetime_t; * \note Key persistence levels are 8-bit values. Key management * interfaces operate on lifetimes (type ::psa_key_lifetime_t) which * encode the persistence as the lower 8 bits of a 32-bit value. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint8_t psa_key_persistence_t; @@ -225,6 +255,11 @@ typedef uint8_t psa_key_persistence_t; * \note Key location indicators are 24-bit values. Key management * interfaces operate on lifetimes (type ::psa_key_lifetime_t) which * encode the location as the upper 24 bits of a 32-bit value. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint32_t psa_key_location_t; @@ -236,9 +271,27 @@ typedef uint32_t psa_key_location_t; * #PSA_KEY_ID_VENDOR_MIN to #PSA_KEY_ID_VENDOR_MAX. * - 0 is reserved as an invalid key identifier. * - Key identifiers outside these ranges are reserved for future use. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to how values are allocated must require careful + * consideration to allow backward compatibility. */ typedef uint32_t psa_key_id_t; +/** Encoding of key identifiers as seen inside the PSA Crypto implementation. + * + * When PSA Crypto is built as a library inside an application, this type + * is identical to #psa_key_id_t. When PSA Crypto is built as a service + * that can store keys on behalf of multiple clients, this type + * encodes the #psa_key_id_t value seen by each client application as + * well as extra information that identifies the client that owns + * the key. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. +*/ #if !defined(MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER) typedef psa_key_id_t mbedtls_svc_key_id_t; @@ -266,6 +319,11 @@ typedef struct * * Values of this type are generally constructed as bitwise-ors of macros * called `PSA_KEY_USAGE_xxx`. + * + * \note Values of this type are encoded in the persistent key store. + * Any changes to existing values will require bumping the storage + * format version and providing a translation when reading the old + * format. */ typedef uint32_t psa_key_usage_t; diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h index 17d7a9bcd..99be43395 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -12,6 +12,11 @@ * designations of cryptographic algorithms, and error codes returned by * the library. * + * Note that many of the constants defined in this file are embedded in + * the persistent key store, as part of key metadata (including usage + * policies). As a consequence, they must not be changed (unless the storage + * format version changes). + * * This header file only defines preprocessor macros. */ /* @@ -41,6 +46,11 @@ /* PSA error codes */ +/* Error codes are standardized across PSA domains (framework, crypto, storage, + * etc.). Do not change the values in this section. If you must add a new + * value, check with the Arm PSA framework group to pick one that other + * domains aren't already using. */ + /** The action was completed successfully. */ #define PSA_SUCCESS ((psa_status_t)0) @@ -317,6 +327,12 @@ * @{ */ +/* Note that key type values, including ECC family and DH group values, are + * embedded in the persistent key store, as part of key metadata. As a + * consequence, they must not be changed (unless the storage format version + * changes). + */ + /** An invalid key type value. * * Zero is not the encoding of any key type. @@ -719,6 +735,11 @@ 1u << PSA_GET_KEY_TYPE_BLOCK_SIZE_EXPONENT(type) : \ 0u) +/* Note that algorithm values are embedded in the persistent key store, + * as part of key metadata. As a consequence, they must not be changed + * (unless the storage format version changes). + */ + /** Vendor-defined algorithm flag. * * Algorithms defined by this standard will never have the #PSA_ALG_VENDOR_FLAG @@ -2095,6 +2116,11 @@ * @{ */ +/* Note that location and persistence level values are embedded in the + * persistent key store, as part of key metadata. As a consequence, they + * must not be changed (unless the storage format version changes). + */ + /** The default lifetime for volatile keys. * * A volatile key only exists as long as the identifier to it is not destroyed. @@ -2210,6 +2236,11 @@ #define PSA_KEY_LOCATION_VENDOR_FLAG ((psa_key_location_t)0x800000) +/* Note that key identifier values are embedded in the + * persistent key store, as part of key metadata. As a consequence, they + * must not be changed (unless the storage format version changes). + */ + /** The null key identifier. */ #define PSA_KEY_ID_NULL ((psa_key_id_t)0) @@ -2321,6 +2352,11 @@ static inline int mbedtls_svc_key_id_is_null( mbedtls_svc_key_id_t key ) * @{ */ +/* Note that key usage flags are embedded in the + * persistent key store, as part of key metadata. As a consequence, they + * must not be changed (unless the storage format version changes). + */ + /** Whether the key may be exported. * * A public key or the public part of a key pair may always be exported @@ -2447,6 +2483,9 @@ static inline int mbedtls_svc_key_id_is_null( mbedtls_svc_key_id_t key ) * @{ */ +/* Key input steps are not embedded in the persistent storage, so you can + * change them if needed: it's only an ABI change. */ + /** A secret input for key derivation. * * This should be a key of type #PSA_KEY_TYPE_DERIVE From 98473c4523a8f0bc33c07ba32f2503ec6a3ef279 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:46:22 +0200 Subject: [PATCH 03/13] Officially deprecate MBEDTLS_PSA_CRYPTO_SE_C This was intended as experimental, and we've been saying for a long time that it's superseded by the "unified driver interface", but we hadn't documented that inside the Mbed TLS source code. So announce it as deprecated. Signed-off-by: Gilles Peskine --- ChangeLog.d/psa_crypto_se.txt | 5 +++++ include/mbedtls/check_config.h | 8 ++++++++ include/mbedtls/mbedtls_config.h | 6 +++--- 3 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 ChangeLog.d/psa_crypto_se.txt diff --git a/ChangeLog.d/psa_crypto_se.txt b/ChangeLog.d/psa_crypto_se.txt new file mode 100644 index 000000000..f8136b1e8 --- /dev/null +++ b/ChangeLog.d/psa_crypto_se.txt @@ -0,0 +1,5 @@ +New deprecations + * Secure element drivers enabled by MBEDTLS_PSA_CRYPTO_SE_C are deprecated. + This was intended as an experimental feature, but had not been explicitly + documented as such. Use opaque drivers with the interface enabled by + MBEDTLS_PSA_CRYPTO_DRIVERS instead. diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index bdc32e183..aa9c9c324 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -581,6 +581,14 @@ #error "MBEDTLS_PSA_CRYPTO_SE_C defined, but not all prerequisites" #endif +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#if defined(MBEDTLS_DEPRECATED_REMOVED) +#error "MBEDTLS_PSA_CRYPTO_SE_C is deprecated and will be removed in a future version of Mbed TLS" +#elif defined(MBEDTLS_DEPRECATED_WARNING) +#warning "MBEDTLS_PSA_CRYPTO_SE_C is deprecated and will be removed in a future version of Mbed TLS" +#endif +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) && \ ! defined(MBEDTLS_PSA_CRYPTO_C) #error "MBEDTLS_PSA_CRYPTO_STORAGE_C defined, but not all prerequisites" diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 9c8ec11a7..65260bc57 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -2686,11 +2686,11 @@ /** * \def MBEDTLS_PSA_CRYPTO_SE_C * - * Enable secure element support in the Platform Security Architecture + * Enable dynamic secure element support in the Platform Security Architecture * cryptography API. * - * \warning This feature is not yet suitable for production. It is provided - * for API evaluation and testing purposes only. + * \deprecated This feature is deprecated. Please switch to the driver + * interface enabled by #MBEDTLS_PSA_CRYPTO_DRIVERS. * * Module: library/psa_crypto_se.c * From 43e51fa88c48e364a2d3c4f1269166fa597cb1ad Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:48:06 +0200 Subject: [PATCH 04/13] Backward compatibility: add a note about the configuration Signed-off-by: Gilles Peskine --- BRANCHES.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/BRANCHES.md b/BRANCHES.md index bc8e75014..9f02bb138 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -36,6 +36,14 @@ undocumented features, then you should be able to re-compile it without modification with any later release x.y'.z' with the same major version number, and your code will still build, be secure, and work. +Note that this guarantee only applies if you either use the default +compile-time configuration (`mbedtls/mbedtls_config.h`) or the same modified +compile-time configuration. Changing compile-time configuration options can +result in an incompatible API or ABI, altough features will generally not +affect independent features (for example, enabling or disabling a +cryptographic algorithm does not break code that does not use that +algorithm). + Note that new releases of Mbed TLS may extend the API. Here are some examples of changes that are common in minor releases of Mbed TLS, and are not considered API compatibility breaks: From 9956efaf32efaf302c69fa629afccd23d7fd7303 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:48:52 +0200 Subject: [PATCH 05/13] Backward compatibility: the key store Promise that we will keep supporting existing key store formats, at least until a major version comes along. Signed-off-by: Gilles Peskine --- BRANCHES.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/BRANCHES.md b/BRANCHES.md index 9f02bb138..e08ae871b 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -28,7 +28,7 @@ the API of 3.(x+1) is backward compatible with 3.x). We only break API compatibility on major version changes (e.g. from 3.x to 4.0). We also maintain ABI compatibility within LTS branches; see the next section for details. -## Backwards Compatibility +## Backwards Compatibility for application code We maintain API compatibility in released versions of Mbed TLS. If you have code that's working and secure with Mbed TLS x.y.z and does not rely on @@ -65,6 +65,19 @@ crypto that was found to be weak) may need to be changed. In case security comes in conflict with backwards compatibility, we will put security first, but always attempt to provide a compatibility option. +## Backward compatibility for the key store + +We maintain backward compatibility with previous versions of versions of the +PSA Crypto persistent storage since Mbed TLS 2.25.0, provided that the +storage backend (PSA ITS implementation) is configured in a compatible way. +We intend to maintain this backward compatibilty throughout a major version +of Mbed TLS (for example, all Mbed TLS 3.y versions will be able to read +keys written under any Mbed TLS 3.x with x < y). + +Mbed TLS 3.x can also read keys written by Mbed TLS 2.25.0 through 2.28.x +LTS, but future major version upgrades (for example from 2.28.x/3.x to 4.y) +may require the use of an upgrade tool. + ## Long-time support branches For the LTS branches, additionally we try very hard to also maintain ABI From 4b873874a3269363ec7317f065dd4570be29d4bb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:50:09 +0200 Subject: [PATCH 06/13] Backward compatibility: the key store with drivers Promise that we will try to keep backward compatibility with basic driver usage, but not with more experimental aspects. Signed-off-by: Gilles Peskine --- BRANCHES.md | 6 ++++++ include/mbedtls/mbedtls_config.h | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/BRANCHES.md b/BRANCHES.md index e08ae871b..f6cdd65b1 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -78,6 +78,12 @@ Mbed TLS 3.x can also read keys written by Mbed TLS 2.25.0 through 2.28.x LTS, but future major version upgrades (for example from 2.28.x/3.x to 4.y) may require the use of an upgrade tool. +Note that this guarantee does not currently fully extend to drivers, which +are an experimental feature. We intend to maintain compatibility with the +basic use of drivers from Mbed TLS 2.28.0 onwards, even if driver APIs +change. However, for more experimental parts of the driver interface, such +as the use of driver state, we do not yet guarantee backward compatibility. + ## Long-time support branches For the LTS branches, additionally we try very hard to also maintain ABI diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 65260bc57..f9642e71a 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1184,8 +1184,9 @@ * * Requires: MBEDTLS_PSA_CRYPTO_C * - * \warning This interface is experimental and may change or be removed - * without notice. + * \warning This interface is experimental. We intend to maintain backward + * compatibility with application code that relies on drivers, + * but the driver interfaces may change without notice. */ //#define MBEDTLS_PSA_CRYPTO_DRIVERS From 6100d3c93c00a5d4f0f8817ce05b1c49f55fc573 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:51:18 +0200 Subject: [PATCH 07/13] Remove redundant sentence Signed-off-by: Gilles Peskine --- scripts/abi_check.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index f11cdf259..0a80d98d1 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -1,6 +1,5 @@ #!/usr/bin/env python3 -""" -This script compares the interfaces of two versions of Mbed TLS, looking +"""This script compares the interfaces of two versions of Mbed TLS, looking for backward incompatibilities between two different Git revisions within an Mbed TLS repository. It must be run from the root of a Git working tree. @@ -20,7 +19,6 @@ at a configurable location, or are given as a brief list of problems. Returns 0 on success, 1 on non-compliance, and 2 if there is an error while running the script. -You must run this test from an Mbed TLS root. """ # Copyright The Mbed TLS Contributors From 228d99b57e05a8e30f1a303654a0e918ed0ef8da Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 18:51:44 +0200 Subject: [PATCH 08/13] Document how to interpret negative reports The abi_check script has common false positives. Document the intent of each family of checks and typical cases of false positives that can be overridden. Signed-off-by: Gilles Peskine --- scripts/abi_check.py | 62 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index 0a80d98d1..c2288432c 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -3,6 +3,8 @@ for backward incompatibilities between two different Git revisions within an Mbed TLS repository. It must be run from the root of a Git working tree. +### How the script works ### + For the source (API) and runtime (ABI) interface compatibility, this script is a small wrapper around the abi-compliance-checker and abi-dumper tools, applying them to compare the header and library files. @@ -19,6 +21,66 @@ at a configurable location, or are given as a brief list of problems. Returns 0 on success, 1 on non-compliance, and 2 if there is an error while running the script. +### How to interpret non-compliance ### + +This script has relatively common false positives. In many scenarios, it only +reports a pass if there is a strict textual match between the old version and +the new version, and it reports problems where there is a sufficient semantic +match but not a textual match. This section lists some common false positives. +This is not an exhaustive list: in the end what matters is whether we are +breaking a backward compatibility goal. + +**API**: the goal is that if an application works with the old version of the +library, it can be recompiled against the new version and will still work. +This is normally validated by comparing the declarations in `include/*/*.h`. +A failure is a declaration that has disappeared or that now has a different +type. + + * It's ok to change or remove macros and functions that are documented as + for internal use only or as experimental. + * It's ok to rename function or macro parameters as long as the semantics + has not changed. + * It's ok to change or remove structure fields that are documented as + private. + * It's ok to add fields to a structure that already had private fields + or was documented as extensible. + +**ABI**: the goal is that if an application was built against the old version +of the library, the same binary will work when linked against the new version. +This is normally validated by comparing the symbols exported by `libmbed*.so`. +A failure is a symbol that is no longer exported by the same library or that +now has a different type. + + * All ABI changes are acceptable if the library version is bumped + (see `scripts/bump_version.sh`). + * ABI changes that concern functions which are declared only inside the + library directory, and not in `include/*/*.h`, are acceptable only if + the function was only ever used inside the same library (libmbedcrypto, + libmbedx509, libmbedtls). As a counter example, if the old version + of libmbedtls calls mbedtls_foo() from libmbedcrypto, and the new version + of libmbedcrypto no longer has a compatible mbedtls_foo(), this does + require a version bump for libmbedcrypto. + +**Storage format**: the goal is to check that persistent keys stored by the +old version can be read by the new version. This is normally validated by +comparing the `*read*` test cases in `test_suite*storage_format*.data`. +A failure is a storage read test case that is no longer present with the same +function name and parameter list. + + * It's ok if the same test data is present, but its presentation has changed, + for example if a test function is renamed or has different parameters. + * It's ok if redundant tests are removed. + +**Generated test coverage**: the goal is to check that automatically +generated tests have as much coverage as before. This is normally validated +by comparing the test cases that are automatically generated by a script. +A failure is a generated test case that is no longer present with the same +function name and parameter list. + + * It's ok if the same test data is present, but its presentation has changed, + for example if a test function is renamed or has different parameters. + * It's ok if redundant tests are removed. + """ # Copyright The Mbed TLS Contributors From 76851ae3a6f91373d853d5a387fc18b3f0cde053 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Jun 2022 19:10:35 +0200 Subject: [PATCH 09/13] Add warnings to test code and data about storage format stability Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/psa_storage.py | 10 ++++++++++ tests/suites/test_suite_psa_crypto_persistent_key.data | 5 +++++ .../test_suite_psa_crypto_persistent_key.function | 5 +++++ tests/suites/test_suite_psa_its.function | 5 +++++ 4 files changed, 25 insertions(+) diff --git a/scripts/mbedtls_dev/psa_storage.py b/scripts/mbedtls_dev/psa_storage.py index 45f0380e7..a06dce13b 100644 --- a/scripts/mbedtls_dev/psa_storage.py +++ b/scripts/mbedtls_dev/psa_storage.py @@ -1,4 +1,9 @@ """Knowledge about the PSA key store as implemented in Mbed TLS. + +Note that if you need to make a change that affects how keys are +stored, this may indicate that the key store is changing in a +backward-incompatible way! Think carefully about backward compatibility +before changing how test data is constructed or validated. """ # Copyright The Mbed TLS Contributors @@ -146,6 +151,11 @@ class Key: This is the content of the PSA storage file. When PSA storage is implemented over stdio files, this does not include any wrapping made by the PSA-storage-over-stdio-file implementation. + + Note that if you need to make a change in this function, + this may indicate that the key store is changing in a + backward-incompatible way! Think carefully about backward + compatibility before making any change here. """ header = self.MAGIC + self.pack('L', self.version) if self.version == 0: diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.data b/tests/suites/test_suite_psa_crypto_persistent_key.data index b25063465..6d208e9e6 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.data +++ b/tests/suites/test_suite_psa_crypto_persistent_key.data @@ -1,3 +1,8 @@ +# Note that if you need to make a change that affects how keys are +# stored, this may indicate that the key store is changing in a +# backward-incompatible way! Think carefully about backward compatibility +# before changing how test data is constructed or validated. + Format for storage: RSA private key format_storage_data_check:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"505341004b455900000000000100000001700004010000000000000700000006620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function index bd9b9c97f..08db34aa4 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.function +++ b/tests/suites/test_suite_psa_crypto_persistent_key.function @@ -5,6 +5,11 @@ * on the the storage format. On the other hand, these tests treat the storage * subsystem as a black box, and in particular have no reliance on the * internals of the ITS implementation. + * + * Note that if you need to make a change that affects how files are + * stored, this may indicate that the key store is changing in a + * backward-incompatible way! Think carefully about backward compatibility + * before changing how test data is constructed or validated. */ #include diff --git a/tests/suites/test_suite_psa_its.function b/tests/suites/test_suite_psa_its.function index e16c05010..12878b533 100644 --- a/tests/suites/test_suite_psa_its.function +++ b/tests/suites/test_suite_psa_its.function @@ -3,6 +3,11 @@ /* This test file is specific to the ITS implementation in PSA Crypto * on top of stdio. It expects to know what the stdio name of a file is * based on its keystore name. + * + * Note that if you need to make a change that affects how files are + * stored, this may indicate that the key store is changing in a + * backward-incompatible way! Think carefully about backward compatibility + * before changing how test data is constructed or validated. */ #include "../library/psa_crypto_its.h" From ed5c21dc3775bebaeb09d4d32cfbd149bc6dcac1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 27 Jun 2022 23:02:09 +0200 Subject: [PATCH 10/13] Declare deprecated option for no_deprecated configs Signed-off-by: Gilles Peskine --- scripts/config.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/config.py b/scripts/config.py index 356b99883..f045f98f9 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -324,6 +324,9 @@ def crypto_adapter(adapter): return adapter(name, active, section) return continuation +DEPRECATED = frozenset([ + 'MBEDTLS_PSA_CRYPTO_SE_C', +]) def no_deprecated_adapter(adapter): """Modify an adapter to disable deprecated symbols. @@ -334,6 +337,8 @@ def no_deprecated_adapter(adapter): def continuation(name, active, section): if name == 'MBEDTLS_DEPRECATED_REMOVED': return True + if name in DEPRECATED: + return False if adapter is None: return active return adapter(name, active, section) From 3dc9ac95ec7ab9fde365447be87600482c98762a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 27 Jun 2022 23:02:58 +0200 Subject: [PATCH 11/13] Spelling Signed-off-by: Gilles Peskine --- BRANCHES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BRANCHES.md b/BRANCHES.md index f6cdd65b1..446d00e1b 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -39,7 +39,7 @@ number, and your code will still build, be secure, and work. Note that this guarantee only applies if you either use the default compile-time configuration (`mbedtls/mbedtls_config.h`) or the same modified compile-time configuration. Changing compile-time configuration options can -result in an incompatible API or ABI, altough features will generally not +result in an incompatible API or ABI, although features will generally not affect independent features (for example, enabling or disabling a cryptographic algorithm does not break code that does not use that algorithm). @@ -70,7 +70,7 @@ but always attempt to provide a compatibility option. We maintain backward compatibility with previous versions of versions of the PSA Crypto persistent storage since Mbed TLS 2.25.0, provided that the storage backend (PSA ITS implementation) is configured in a compatible way. -We intend to maintain this backward compatibilty throughout a major version +We intend to maintain this backward compatibility throughout a major version of Mbed TLS (for example, all Mbed TLS 3.y versions will be able to read keys written under any Mbed TLS 3.x with x < y). From 4fd898e87663311ac26d4ad8dd7c63689c2a82a5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 29 Jun 2022 14:29:52 +0200 Subject: [PATCH 12/13] More wording improvements Signed-off-by: Gilles Peskine --- BRANCHES.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/BRANCHES.md b/BRANCHES.md index 446d00e1b..3aca15990 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -40,7 +40,7 @@ Note that this guarantee only applies if you either use the default compile-time configuration (`mbedtls/mbedtls_config.h`) or the same modified compile-time configuration. Changing compile-time configuration options can result in an incompatible API or ABI, although features will generally not -affect independent features (for example, enabling or disabling a +affect unrelated features (for example, enabling or disabling a cryptographic algorithm does not break code that does not use that algorithm). @@ -67,12 +67,12 @@ but always attempt to provide a compatibility option. ## Backward compatibility for the key store -We maintain backward compatibility with previous versions of versions of the +We maintain backward compatibility with previous versions of the PSA Crypto persistent storage since Mbed TLS 2.25.0, provided that the storage backend (PSA ITS implementation) is configured in a compatible way. We intend to maintain this backward compatibility throughout a major version of Mbed TLS (for example, all Mbed TLS 3.y versions will be able to read -keys written under any Mbed TLS 3.x with x < y). +keys written under any Mbed TLS 3.x with x <= y). Mbed TLS 3.x can also read keys written by Mbed TLS 2.25.0 through 2.28.x LTS, but future major version upgrades (for example from 2.28.x/3.x to 4.y) From 955993c4b5ade57d5345839ab7f71a45fcbaaa6f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 29 Jun 2022 14:37:17 +0200 Subject: [PATCH 13/13] For status values, the macro expansions must not change either Signed-off-by: Gilles Peskine --- include/psa/crypto_values.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h index 99be43395..e60257d7f 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -47,7 +47,14 @@ /* PSA error codes */ /* Error codes are standardized across PSA domains (framework, crypto, storage, - * etc.). Do not change the values in this section. If you must add a new + * etc.). Do not change the values in this section or even the expansions + * of each macro: it must be possible to `#include` both this header + * and some other PSA component's headers in the same C source, + * which will lead to duplicate definitions of the `PSA_SUCCESS` and + * `PSA_ERROR_xxx` macros, which is ok if and only if the macros expand + * to the same sequence of tokens. + * + * If you must add a new * value, check with the Arm PSA framework group to pick one that other * domains aren't already using. */