Merge pull request #257 from gilles-peskine-arm/psa-remove_zero_length_keys
Forbid zero-length keys
diff --git a/docs/architecture/Makefile b/docs/architecture/Makefile
index f763c9c..258abcd 100644
--- a/docs/architecture/Makefile
+++ b/docs/architecture/Makefile
@@ -4,6 +4,7 @@
all_markdown = \
mbed-crypto-storage-specification.md \
+ testing/driver-interface-test-strategy.md \
# This line is intentionally left blank
html: $(all_markdown:.md=.html)
@@ -17,3 +18,6 @@
$(PANDOC) -o $@ $<
.md.pdf:
$(PANDOC) -o $@ $<
+
+clean:
+ rm -f *.html *.pdf
diff --git a/docs/architecture/testing/driver-interface-test-strategy.md b/docs/architecture/testing/driver-interface-test-strategy.md
new file mode 100644
index 0000000..d6769da
--- /dev/null
+++ b/docs/architecture/testing/driver-interface-test-strategy.md
@@ -0,0 +1,115 @@
+# Mbed Crypto driver interface test strategy
+
+This document describes the test strategy for the driver interfaces in Mbed Crypto. Mbed Crypto has interfaces for secure element drivers, accelerator drivers and entropy drivers. This document is about testing Mbed Crypto itself; testing drivers is out of scope.
+
+The driver interfaces are standardized through PSA Cryptography functional specifications.
+
+## Secure element driver interface
+
+The secure element driver interface (SE interface for short) is defined by [`psa/crypto_se_driver.h`](../../../include/psa/crypto_se_driver.h). This is an interface between Mbed Crypto and one or more third-party drivers.
+
+The SE interface consists of one function provided by Mbed Crypto (`psa_register_se_driver`) and many functions that drivers must implement. To make a driver usable by Mbed Crypto, the initialization code must call `psa_register_se_driver` with a structure that describes the driver. The structure mostly contains function pointers, pointing to the driver's methods. All calls to a driver function are triggered by a call to a PSA crypto API function.
+
+### SE driver interface unit tests
+
+This section describes unit tests that must be implemented to validate the secure element driver interface. Note that a test case may cover multiple requirements; for example a “good case” test can validate that the proper function is called, that it receives the expected inputs and that it produces the expected outputs.
+
+Many SE driver interface unit tests could be covered by running the existing API tests with a key in a secure element.
+
+#### SE driver registration
+
+* Test `psa_register_se_driver` with valid and with invalid arguments.
+* Make at least one failing call to `psa_register_se_driver` followed by a successful call.
+* Make at least one test that successfully registers the maximum number of drivers and fails to register one more.
+
+#### Dispatch to SE driver
+
+For each API function that can lead to a driver call (more precisely, for each driver method call site, but this is practically equivalent):
+
+* Make at least one test with a key in a secure element that checks that the driver method is called. A few API functions involve multiple driver methods; these should validate that all the expected driver methods are called.
+* Make at least one test with a key that is not in a secure element that checks that the driver method is not called.
+* Make at least one test with a key in a secure element with a driver that does not have the requisite method (i.e. the method pointer is `NULL`) but has the substructure containing that method, and check that the return value is `PSA_ERROR_NOT_SUPPORTED`.
+* Make at least one test with a key in a secure element with a driver that does not have the substructure containing that method (i.e. the pointer to the substructure is `NULL`), and check that the return value is `PSA_ERROR_NOT_SUPPORTED`.
+* At least one test should register multiple drivers with a key in each driver and check that the expected driver is called. This does not need to be done for all operations (use a white-box approach to determine if operations may use different code paths to choose the driver).
+* At least one test should register the same driver structure with multiple lifetime values and check that the driver receives the expected lifetime value.
+
+Some methods only make sense as a group (for example a driver that provides the MAC methods must provide all or none). In those cases, test with all of them null and none of them null.
+
+#### SE driver inputs
+
+For each API function that can lead to a driver call (more precisely, for each driver method call site, but this is practically equivalent):
+
+* Wherever the specification guarantees parameters that satisfy certain preconditions, check these preconditions whenever practical.
+* If the API function can take parameters that are invalid and must not reach the driver, call the API function with such parameters and verify that the driver method is not called.
+* Check that the expected inputs reach the driver. This may be implicit in a test that checks the outputs if the only realistic way to obtain the correct outputs is to start from the expected inputs (as is often the case for cryptographic material, but not for metadata).
+
+#### SE driver outputs
+
+For each API function that leads to a driver call, call it with parameters that cause a driver to be invoked and check how Mbed Crypto handles the outputs.
+
+* Correct outputs.
+* Incorrect outputs such as an invalid output length.
+* Expected errors (e.g. `PSA_ERROR_INVALID_SIGNATURE` from a signature verification method).
+* Unexpected errors. At least test that if the driver returns `PSA_ERROR_GENERIC_ERROR`, this is propagated correctly.
+
+Key creation functions invoke multiple methods and need more complex error handling:
+
+* Check the consequence of errors detected at each stage (slot number allocation or validation, key creation method, storage accesses).
+* Check that the storage ends up in the expected state. At least make sure that no intermediate file remains after a failure.
+
+#### Persistence of SE keys
+
+The following tests must be performed at least one for each key creation method (import, generate, ...).
+
+* Test that keys in a secure element survive `psa_close_key(); psa_open_key()`.
+* Test that keys in a secure element survive `mbedtls_psa_crypto_free(); psa_crypto_init()`.
+* Test that the driver's persistent data survives `mbedtls_psa_crypto_free(); psa_crypto_init()`.
+* Test that `psa_destroy_key()` does not leave any trace of the key.
+
+#### Resilience for SE drivers
+
+Creating or removing a key in a secure element involves multiple storage modifications (M<sub>1</sub>, ..., M<sub>n</sub>). If the operation is interrupted by a reset at any point, it must be either rolled back or completed.
+
+* For each potential interruption point (before M<sub>1</sub>, between M<sub>1</sub> and M<sub>2</sub>, ..., after M<sub>n</sub>), call `mbedtls_psa_crypto_free(); psa_crypto_init()` at that point and check that this either rolls back or completes the operation that was started.
+* This must be done for each key creation method and for key destruction.
+* This must be done for each possible flow, including error cases (e.g. a key creation that fails midway due to `OUT_OF_MEMORY`).
+* The recovery during `psa_crypto_init` can itself be interrupted. Test those interruptions too.
+* Two things need to be tested: the key that is being created or destroyed, and the driver's persistent storage.
+* Check both that the storage has the expected content (this can be done by e.g. using a key that is supposed to be present) and does not have any unexpected content (for keys, this can be done by checking that `psa_open_key` fails with `PSA_ERRROR_DOES_NOT_EXIST`).
+
+This requires instrumenting the storage implementation, either to force it to fail at each point or to record successive storage states and replay each of them. Each `psa_its_xxx` function call is assumed to be atomic.
+
+### SE driver system tests
+
+#### Real-world use case
+
+We must have at least one driver that is close to real-world conditions:
+
+* With its own source tree.
+* Running on actual hardware.
+* Run the full driver validation test suite (which does not yet exist).
+* Run at least one test application (e.g. the Mbed OS TLS example).
+
+This requirement shall be fulfilled by the [Microchip ATECC508A driver](https://github.com/ARMmbed/mbed-os-atecc608a/).
+
+#### Complete driver
+
+We should have at least one driver that covers the whole interface:
+
+* With its own source tree.
+* Implementing all the methods.
+* Run the full driver validation test suite (which does not yet exist).
+
+A PKCS#11 driver would be a good candidate. It would be useful as part of our product offering.
+
+## Accelerator driver interface
+
+The accelerator driver interface is defined by [`psa/crypto_accel_driver.h`](../../../include/psa/crypto_accel_driver.h).
+
+TODO
+
+## Entropy driver interface
+
+The entropy driver interface is defined by [`psa/crypto_entropy_driver.h`](../../../include/psa/crypto_entropy_driver.h).
+
+TODO
diff --git a/include/psa/crypto.h b/include/psa/crypto.h
index 7c88bd6..89392da 100644
--- a/include/psa/crypto.h
+++ b/include/psa/crypto.h
@@ -455,6 +455,140 @@
*/
psa_status_t psa_close_key(psa_key_handle_t handle);
+/** Make a copy of a key.
+ *
+ * Copy key material from one location to another.
+ *
+ * This function is primarily useful to copy a key from one location
+ * to another, since it populates a key using the material from
+ * another key which may have a different lifetime.
+ *
+ * This function may be used to share a key with a different party,
+ * subject to implementation-defined restrictions on key sharing.
+ *
+ * The policy on the source key must have the usage flag
+ * #PSA_KEY_USAGE_COPY set.
+ * This flag is sufficient to permit the copy if the key has the lifetime
+ * #PSA_KEY_LIFETIME_VOLATILE or #PSA_KEY_LIFETIME_PERSISTENT.
+ * Some secure elements do not provide a way to copy a key without
+ * making it extractable from the secure element. If a key is located
+ * in such a secure element, then the key must have both usage flags
+ * #PSA_KEY_USAGE_COPY and #PSA_KEY_USAGE_EXPORT in order to make
+ * a copy of the key outside the secure element.
+ *
+ * The resulting key may only be used in a way that conforms to
+ * both the policy of the original key and the policy specified in
+ * the \p attributes parameter:
+ * - The usage flags on the resulting key are the bitwise-and of the
+ * usage flags on the source policy and the usage flags in \p attributes.
+ * - If both allow the same algorithm or wildcard-based
+ * algorithm policy, the resulting key has the same algorithm policy.
+ * - If either of the policies allows an algorithm and the other policy
+ * allows a wildcard-based algorithm policy that includes this algorithm,
+ * the resulting key allows the same algorithm.
+ * - If the policies do not allow any algorithm in common, this function
+ * fails with the status #PSA_ERROR_INVALID_ARGUMENT.
+ *
+ * The effect of this function on implementation-defined attributes is
+ * implementation-defined.
+ *
+ * \param source_handle The key to copy. It must be a valid key handle.
+ * \param[in] attributes The attributes for the new key.
+ * They are used as follows:
+ * - The key type and size may be 0. If either is
+ * nonzero, it must match the corresponding
+ * attribute of the source key.
+ * - The key location (the lifetime and, for
+ * persistent keys, the key identifier) is
+ * used directly.
+ * - The policy constraints (usage flags and
+ * algorithm policy) are combined from
+ * the source key and \p attributes so that
+ * both sets of restrictions apply, as
+ * described in the documentation of this function.
+ * \param[out] target_handle On success, a handle to the newly created key.
+ * \c 0 on failure.
+ *
+ * \retval #PSA_SUCCESS
+ * \retval #PSA_ERROR_INVALID_HANDLE
+ * \p source_handle is invalid.
+ * \retval #PSA_ERROR_ALREADY_EXISTS
+ * This is an attempt to create a persistent key, and there is
+ * already a persistent key with the given identifier.
+ * \retval #PSA_ERROR_INVALID_ARGUMENT
+ * The lifetime or identifier in \p attributes are invalid.
+ * \retval #PSA_ERROR_INVALID_ARGUMENT
+ * The policy constraints on the source and specified in
+ * \p attributes are incompatible.
+ * \retval #PSA_ERROR_INVALID_ARGUMENT
+ * \p attributes specifies a key type or key size
+ * which does not match the attributes of the source key.
+ * \retval #PSA_ERROR_NOT_PERMITTED
+ * The source key does not have the #PSA_KEY_USAGE_COPY usage flag.
+ * \retval #PSA_ERROR_NOT_PERMITTED
+ * The source key is not exportable and its lifetime does not
+ * allow copying it to the target's lifetime.
+ * \retval #PSA_ERROR_INSUFFICIENT_MEMORY
+ * \retval #PSA_ERROR_INSUFFICIENT_STORAGE
+ * \retval #PSA_ERROR_COMMUNICATION_FAILURE
+ * \retval #PSA_ERROR_HARDWARE_FAILURE
+ * \retval #PSA_ERROR_STORAGE_FAILURE
+ * \retval #PSA_ERROR_CORRUPTION_DETECTED
+ * \retval #PSA_ERROR_BAD_STATE
+ * The library has not been previously initialized by psa_crypto_init().
+ * It is implementation-dependent whether a failure to initialize
+ * results in this error code.
+ */
+psa_status_t psa_copy_key(psa_key_handle_t source_handle,
+ const psa_key_attributes_t *attributes,
+ psa_key_handle_t *target_handle);
+
+
+/**
+ * \brief Destroy a key.
+ *
+ * This function destroys a key from both volatile
+ * memory and, if applicable, non-volatile storage. Implementations shall
+ * make a best effort to ensure that that the key material cannot be recovered.
+ *
+ * This function also erases any metadata such as policies and frees
+ * resources associated with the key. To free all resources associated with
+ * the key, all handles to the key must be closed or destroyed.
+ *
+ * Destroying the key makes the handle invalid, and the key handle
+ * must not be used again by the application. Using other open handles to the
+ * destroyed key in a cryptographic operation will result in an error.
+ *
+ * If a key is currently in use in a multipart operation, then destroying the
+ * key will cause the multipart operation to fail.
+ *
+ * \param handle Handle to the key to erase.
+ *
+ * \retval #PSA_SUCCESS
+ * The key material has been erased.
+ * \retval #PSA_ERROR_NOT_PERMITTED
+ * The key cannot be erased because it is
+ * read-only, either due to a policy or due to physical restrictions.
+ * \retval #PSA_ERROR_INVALID_HANDLE
+ * \retval #PSA_ERROR_COMMUNICATION_FAILURE
+ * There was an failure in communication with the cryptoprocessor.
+ * The key material may still be present in the cryptoprocessor.
+ * \retval #PSA_ERROR_STORAGE_FAILURE
+ * The storage is corrupted. Implementations shall make a best effort
+ * to erase key material even in this stage, however applications
+ * should be aware that it may be impossible to guarantee that the
+ * key material is not recoverable in such cases.
+ * \retval #PSA_ERROR_CORRUPTION_DETECTED
+ * An unexpected condition which is not a storage corruption or
+ * a communication failure occurred. The cryptoprocessor may have
+ * been compromised.
+ * \retval #PSA_ERROR_BAD_STATE
+ * The library has not been previously initialized by psa_crypto_init().
+ * It is implementation-dependent whether a failure to initialize
+ * results in this error code.
+ */
+psa_status_t psa_destroy_key(psa_key_handle_t handle);
+
/**@}*/
/** \defgroup import_export Key import and export
@@ -537,50 +671,7 @@
size_t data_length,
psa_key_handle_t *handle);
-/**
- * \brief Destroy a key.
- *
- * This function destroys a key from both volatile
- * memory and, if applicable, non-volatile storage. Implementations shall
- * make a best effort to ensure that that the key material cannot be recovered.
- *
- * This function also erases any metadata such as policies and frees
- * resources associated with the key. To free all resources associated with
- * the key, all handles to the key must be closed or destroyed.
- *
- * Destroying the key makes the handle invalid, and the key handle
- * must not be used again by the application. Using other open handles to the
- * destroyed key in a cryptographic operation will result in an error.
- *
- * If a key is currently in use in a multipart operation, then destroying the
- * key will cause the multipart operation to fail.
- *
- * \param handle Handle to the key to erase.
- *
- * \retval #PSA_SUCCESS
- * The key material has been erased.
- * \retval #PSA_ERROR_NOT_PERMITTED
- * The key cannot be erased because it is
- * read-only, either due to a policy or due to physical restrictions.
- * \retval #PSA_ERROR_INVALID_HANDLE
- * \retval #PSA_ERROR_COMMUNICATION_FAILURE
- * There was an failure in communication with the cryptoprocessor.
- * The key material may still be present in the cryptoprocessor.
- * \retval #PSA_ERROR_STORAGE_FAILURE
- * The storage is corrupted. Implementations shall make a best effort
- * to erase key material even in this stage, however applications
- * should be aware that it may be impossible to guarantee that the
- * key material is not recoverable in such cases.
- * \retval #PSA_ERROR_CORRUPTION_DETECTED
- * An unexpected condition which is not a storage corruption or
- * a communication failure occurred. The cryptoprocessor may have
- * been compromised.
- * \retval #PSA_ERROR_BAD_STATE
- * The library has not been previously initialized by psa_crypto_init().
- * It is implementation-dependent whether a failure to initialize
- * results in this error code.
- */
-psa_status_t psa_destroy_key(psa_key_handle_t handle);
+
/**
* \brief Export a key in binary format.
@@ -740,93 +831,7 @@
size_t data_size,
size_t *data_length);
-/** Make a copy of a key.
- *
- * Copy key material from one location to another.
- *
- * This function is primarily useful to copy a key from one location
- * to another, since it populates a key using the material from
- * another key which may have a different lifetime.
- *
- * This function may be used to share a key with a different party,
- * subject to implementation-defined restrictions on key sharing.
- *
- * The policy on the source key must have the usage flag
- * #PSA_KEY_USAGE_COPY set.
- * This flag is sufficient to permit the copy if the key has the lifetime
- * #PSA_KEY_LIFETIME_VOLATILE or #PSA_KEY_LIFETIME_PERSISTENT.
- * Some secure elements do not provide a way to copy a key without
- * making it extractable from the secure element. If a key is located
- * in such a secure element, then the key must have both usage flags
- * #PSA_KEY_USAGE_COPY and #PSA_KEY_USAGE_EXPORT in order to make
- * a copy of the key outside the secure element.
- *
- * The resulting key may only be used in a way that conforms to
- * both the policy of the original key and the policy specified in
- * the \p attributes parameter:
- * - The usage flags on the resulting key are the bitwise-and of the
- * usage flags on the source policy and the usage flags in \p attributes.
- * - If both allow the same algorithm or wildcard-based
- * algorithm policy, the resulting key has the same algorithm policy.
- * - If either of the policies allows an algorithm and the other policy
- * allows a wildcard-based algorithm policy that includes this algorithm,
- * the resulting key allows the same algorithm.
- * - If the policies do not allow any algorithm in common, this function
- * fails with the status #PSA_ERROR_INVALID_ARGUMENT.
- *
- * The effect of this function on implementation-defined attributes is
- * implementation-defined.
- *
- * \param source_handle The key to copy. It must be a valid key handle.
- * \param[in] attributes The attributes for the new key.
- * They are used as follows:
- * - The key type and size may be 0. If either is
- * nonzero, it must match the corresponding
- * attribute of the source key.
- * - The key location (the lifetime and, for
- * persistent keys, the key identifier) is
- * used directly.
- * - The policy constraints (usage flags and
- * algorithm policy) are combined from
- * the source key and \p attributes so that
- * both sets of restrictions apply, as
- * described in the documentation of this function.
- * \param[out] target_handle On success, a handle to the newly created key.
- * \c 0 on failure.
- *
- * \retval #PSA_SUCCESS
- * \retval #PSA_ERROR_INVALID_HANDLE
- * \p source_handle is invalid.
- * \retval #PSA_ERROR_ALREADY_EXISTS
- * This is an attempt to create a persistent key, and there is
- * already a persistent key with the given identifier.
- * \retval #PSA_ERROR_INVALID_ARGUMENT
- * The lifetime or identifier in \p attributes are invalid.
- * \retval #PSA_ERROR_INVALID_ARGUMENT
- * The policy constraints on the source and specified in
- * \p attributes are incompatible.
- * \retval #PSA_ERROR_INVALID_ARGUMENT
- * \p attributes specifies a key type or key size
- * which does not match the attributes of the source key.
- * \retval #PSA_ERROR_NOT_PERMITTED
- * The source key does not have the #PSA_KEY_USAGE_COPY usage flag.
- * \retval #PSA_ERROR_NOT_PERMITTED
- * The source key is not exportable and its lifetime does not
- * allow copying it to the target's lifetime.
- * \retval #PSA_ERROR_INSUFFICIENT_MEMORY
- * \retval #PSA_ERROR_INSUFFICIENT_STORAGE
- * \retval #PSA_ERROR_COMMUNICATION_FAILURE
- * \retval #PSA_ERROR_HARDWARE_FAILURE
- * \retval #PSA_ERROR_STORAGE_FAILURE
- * \retval #PSA_ERROR_CORRUPTION_DETECTED
- * \retval #PSA_ERROR_BAD_STATE
- * The library has not been previously initialized by psa_crypto_init().
- * It is implementation-dependent whether a failure to initialize
- * results in this error code.
- */
-psa_status_t psa_copy_key(psa_key_handle_t source_handle,
- const psa_key_attributes_t *attributes,
- psa_key_handle_t *target_handle);
+
/**@}*/
@@ -2708,13 +2713,25 @@
*
* The operation must have been set up with psa_aead_decrypt_setup().
*
- * This function finishes the authentication of the additional data
- * formed by concatenating the inputs passed to preceding calls to
- * psa_aead_update_ad() with the ciphertext formed by concatenating the
- * inputs passed to preceding calls to psa_aead_update().
+ * This function finishes the authenticated decryption of the message
+ * components:
+ *
+ * - The additional data consisting of the concatenation of the inputs
+ * passed to preceding calls to psa_aead_update_ad().
+ * - The ciphertext consisting of the concatenation of the inputs passed to
+ * preceding calls to psa_aead_update().
+ * - The tag passed to this function call.
+ *
+ * If the authentication tag is correct, this function outputs any remaining
+ * plaintext and reports success. If the authentication tag is not correct,
+ * this function returns #PSA_ERROR_INVALID_SIGNATURE.
*
* When this function returns, the operation becomes inactive.
*
+ * \note Implementations shall make the best effort to ensure that the
+ * comparison between the actual tag and the expected tag is performed
+ * in constant time.
+ *
* \param[in,out] operation Active AEAD operation.
* \param[out] plaintext Buffer where the last part of the plaintext
* is to be written. This is the remaining data
@@ -2733,6 +2750,9 @@
*
* \retval #PSA_SUCCESS
* Success.
+ * \retval #PSA_ERROR_INVALID_SIGNATURE
+ * The calculations were successful, but the authentication tag is
+ * not correct.
* \retval #PSA_ERROR_BAD_STATE
* The operation state is not valid (not set up, nonce not set,
* encryption, or already completed).
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 09254b2..fe737d2 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2732,7 +2732,7 @@
status = psa_hash_update( &hmac->hash_ctx, ipad, block_size );
cleanup:
- mbedtls_platform_zeroize( ipad, key_length );
+ mbedtls_platform_zeroize( ipad, sizeof(ipad) );
return( status );
}
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index a8e97e0..18159e7 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -808,6 +808,14 @@
# Either INVALID_ARGUMENT or NOT_SUPPORTED would be reasonable here
mac_setup:PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CMAC:PSA_ERROR_NOT_SUPPORTED
+PSA MAC setup: algorithm known but not supported, long key
+depends_on:!MBEDTLS_MD5_C
+mac_setup:PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f":PSA_ALG_HMAC(PSA_ALG_MD5):PSA_ERROR_NOT_SUPPORTED
+
+PSA MAC setup: algorithm known but not supported, short key
+depends_on:!MBEDTLS_MD5_C
+mac_setup:PSA_KEY_TYPE_HMAC:"000102030405060708":PSA_ALG_HMAC(PSA_ALG_MD5):PSA_ERROR_NOT_SUPPORTED
+
PSA MAC: bad order function calls
depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C
mac_bad_order: