Merge pull request #10182 from gilles-peskine-arm/f_rng-documentation-3.6
3.6 only: document f_rng callbacks
diff --git a/ChangeLog.d/move-crypto-struct-inclusion.txt b/ChangeLog.d/move-crypto-struct-inclusion.txt
new file mode 100644
index 0000000..c5f6d2b
--- /dev/null
+++ b/ChangeLog.d/move-crypto-struct-inclusion.txt
@@ -0,0 +1,3 @@
+Bugfix
+ * Resolved build issue with C++ projects using TF-PSA-Crypto when compiling
+ with the MSVC toolset v142 and earlier. Fixes mbedtls issue #7087.
diff --git a/ChangeLog.d/union-initialization.txt b/ChangeLog.d/union-initialization.txt
new file mode 100644
index 0000000..a63e1eb
--- /dev/null
+++ b/ChangeLog.d/union-initialization.txt
@@ -0,0 +1,15 @@
+Bugfix
+ * Fix failures of PSA multipart or interruptible operations when the
+ library or the application is built with a compiler where
+ "union foo x = {0}" does not initialize non-default members of the
+ union, such as GCC 15 and some versions of Clang 18. This affected MAC
+ multipart operations, MAC-based key derivation operations, interruptible
+ signature, interruptible verification, and potentially other operations
+ when using third-party drivers. This also affected one-shot MAC
+ operations using the built-in implementation. Fixes #9814.
+ * On entry to PSA driver entry points that set up a multipart operation
+ ("xxx_setup"), the operation object is supposed to be all-bits-zero.
+ This was sometimes not the case when an operation object is reused,
+ or with compilers where "union foo x = {0}" does not initialize
+ non-default members of the union. The PSA core now ensures that this
+ guarantee is met in all cases. Fixes #9975.
diff --git a/include/psa/crypto.h b/include/psa/crypto.h
index 2bbcea3..2fe9f35 100644
--- a/include/psa/crypto.h
+++ b/include/psa/crypto.h
@@ -59,6 +59,18 @@
* of integral types defined in "crypto_types.h". */
#include "crypto_values.h"
+/* The file "crypto_sizes.h" contains definitions for size calculation
+ * macros whose definitions are implementation-specific. */
+#include "crypto_sizes.h"
+
+/* The file "crypto_struct.h" contains definitions for
+ * implementation-specific structs that are declared above. */
+#if defined(MBEDTLS_PSA_CRYPTO_STRUCT_FILE)
+#include MBEDTLS_PSA_CRYPTO_STRUCT_FILE
+#else
+#include "crypto_struct.h"
+#endif
+
/** \defgroup initialization Library initialization
* @{
*/
@@ -4958,18 +4970,6 @@
}
#endif
-/* The file "crypto_sizes.h" contains definitions for size calculation
- * macros whose definitions are implementation-specific. */
-#include "crypto_sizes.h"
-
-/* The file "crypto_struct.h" contains definitions for
- * implementation-specific structs that are declared above. */
-#if defined(MBEDTLS_PSA_CRYPTO_STRUCT_FILE)
-#include MBEDTLS_PSA_CRYPTO_STRUCT_FILE
-#else
-#include "crypto_struct.h"
-#endif
-
/* The file "crypto_extra.h" contains vendor-specific definitions. This
* can include vendor-defined algorithms, extra functions, etc. */
#include "crypto_extra.h"
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index e7f99ad..9c28609 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2400,8 +2400,11 @@
goto exit;
}
- /* Ensure all of the context is zeroized, since PSA_HASH_OPERATION_INIT only
- * directly zeroes the int-sized dummy member of the context union. */
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
memset(&operation->ctx, 0, sizeof(operation->ctx));
status = psa_driver_wrapper_hash_setup(operation, alg);
@@ -2596,6 +2599,13 @@
return PSA_ERROR_BAD_STATE;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&target_operation->ctx, 0, sizeof(target_operation->ctx));
+
psa_status_t status = psa_driver_wrapper_hash_clone(source_operation,
target_operation);
if (status != PSA_SUCCESS) {
@@ -2693,6 +2703,13 @@
goto exit;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
status = psa_get_and_lock_key_slot_with_policy(
key,
&slot,
@@ -3619,6 +3636,13 @@
return PSA_ERROR_BAD_STATE;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
status = psa_sign_verify_check_alg(0, alg);
if (status != PSA_SUCCESS) {
operation->error_occurred = 1;
@@ -3779,6 +3803,13 @@
return PSA_ERROR_BAD_STATE;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
status = psa_sign_verify_check_alg(0, alg);
if (status != PSA_SUCCESS) {
operation->error_occurred = 1;
@@ -4446,6 +4477,14 @@
}
operation->default_iv_length = PSA_CIPHER_IV_LENGTH(slot->attr.type, alg);
+
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
/* Try doing the operation through a driver before using software fallback. */
if (cipher_operation == MBEDTLS_ENCRYPT) {
status = psa_driver_wrapper_cipher_encrypt_setup(operation,
@@ -5079,6 +5118,13 @@
goto exit;
}
+ /* Make sure the driver-dependent part of the operation is zeroed.
+ * This is a guarantee we make to drivers. Initializing the operation
+ * does not necessarily take care of it, since the context is a
+ * union and initializing a union does not necessarily initialize
+ * all of its members. */
+ memset(&operation->ctx, 0, sizeof(operation->ctx));
+
if (is_encrypt) {
key_usage = PSA_KEY_USAGE_ENCRYPT;
} else {
@@ -5599,6 +5645,17 @@
#if defined(BUILTIN_ALG_ANY_HKDF) || \
defined(MBEDTLS_PSA_BUILTIN_ALG_TLS12_PRF) || \
defined(MBEDTLS_PSA_BUILTIN_ALG_TLS12_PSK_TO_MS)
+
+/** Internal helper to set up an HMAC operation with a key passed directly.
+ *
+ * \param[in,out] operation A MAC operation object. It does not need to
+ * be initialized.
+ * \param hash_alg The hash algorithm used for HMAC.
+ * \param hmac_key The HMAC key.
+ * \param hmac_key_length Length of \p hmac_key in bytes.
+ *
+ * \return A PSA status code.
+ */
static psa_status_t psa_key_derivation_start_hmac(
psa_mac_operation_t *operation,
psa_algorithm_t hash_alg,
@@ -5611,6 +5668,14 @@
psa_set_key_bits(&attributes, PSA_BYTES_TO_BITS(hmac_key_length));
psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_SIGN_HASH);
+ /* Make sure the whole the operation is zeroed.
+ * It isn't enough to require the caller to initialize operation to
+ * PSA_MAC_OPERATION_INIT, since one field is a union and initializing
+ * a union does not necessarily initialize all of its members.
+ * psa_mac_setup() would handle PSA_MAC_OPERATION_INIT, but here we
+ * bypass it and call lower-level functions directly. */
+ memset(operation, 0, sizeof(*operation));
+
operation->is_sign = 1;
operation->mac_size = PSA_HASH_LENGTH(hash_alg);
@@ -5835,7 +5900,7 @@
{
psa_algorithm_t hash_alg = PSA_ALG_HKDF_GET_HASH(alg);
uint8_t hash_length = PSA_HASH_LENGTH(hash_alg);
- psa_mac_operation_t hmac = PSA_MAC_OPERATION_INIT;
+ psa_mac_operation_t hmac;
size_t hmac_output_length;
psa_status_t status, cleanup_status;
@@ -6036,7 +6101,14 @@
psa_key_attributes_t *attributes)
{
psa_status_t status;
- psa_mac_operation_t mac_operation = PSA_MAC_OPERATION_INIT;
+ psa_mac_operation_t mac_operation;
+ /* Make sure the whole the operation is zeroed.
+ * PSA_MAC_OPERATION_INIT does not necessarily do it fully,
+ * since one field is a union and initializing a union does not
+ * necessarily initialize all of its members.
+ * psa_mac_setup() would do it, but here we bypass it and call
+ * lower-level functions directly. */
+ memset(&mac_operation, 0, sizeof(mac_operation));
size_t mac_output_length;
uint8_t U_i[PSA_MAC_MAX_SIZE];
uint8_t *U_accumulator = pbkdf2->output_block;
@@ -8629,7 +8701,11 @@
goto exit;
}
- memset(&operation->data.inputs, 0, sizeof(operation->data.inputs));
+ /* Make sure the variable-purpose part of the operation is zeroed.
+ * Initializing the operation does not necessarily take care of it,
+ * since the context is a union and initializing a union does not
+ * necessarily initialize all of its members. */
+ memset(&operation->data, 0, sizeof(operation->data));
operation->alg = cipher_suite->algorithm;
operation->primitive = PSA_PAKE_PRIMITIVE(cipher_suite->type,
diff --git a/library/psa_crypto_mac.c b/library/psa_crypto_mac.c
index 8fe6218..4464158 100644
--- a/library/psa_crypto_mac.c
+++ b/library/psa_crypto_mac.c
@@ -465,6 +465,15 @@
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
mbedtls_psa_mac_operation_t operation = MBEDTLS_PSA_MAC_OPERATION_INIT;
+ /* Make sure the whole operation is zeroed.
+ * PSA_MAC_OPERATION_INIT does not necessarily do it fully,
+ * since one field is a union and initializing a union does not
+ * necessarily initialize all of its members.
+ * In multipart operations, this is done in the API functions,
+ * before driver dispatch, since it needs to be done before calling
+ * the driver entry point. Here, we bypass the multipart API,
+ * so it's our job. */
+ memset(&operation, 0, sizeof(operation));
status = psa_mac_setup(&operation,
attributes, key_buffer, key_buffer_size,
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index ce14c7d..b5bea75 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -5762,7 +5762,7 @@
/* NOTICE: see above */
#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
-static uint16_t ssl_tls12_preset_default_sig_algs[] = {
+static const uint16_t ssl_tls12_preset_default_sig_algs[] = {
#if defined(MBEDTLS_MD_CAN_SHA512)
#if defined(MBEDTLS_KEY_EXCHANGE_ECDSA_CERT_REQ_ALLOWED_ENABLED)
@@ -5826,7 +5826,7 @@
/* NOTICE: see above */
#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
-static uint16_t ssl_tls12_preset_suiteb_sig_algs[] = {
+static const uint16_t ssl_tls12_preset_suiteb_sig_algs[] = {
#if defined(MBEDTLS_MD_CAN_SHA256)
#if defined(MBEDTLS_KEY_EXCHANGE_ECDSA_CERT_REQ_ALLOWED_ENABLED)
diff --git a/tests/scripts/components-compiler.sh b/tests/scripts/components-compiler.sh
index d69d529..2864087 100644
--- a/tests/scripts/components-compiler.sh
+++ b/tests/scripts/components-compiler.sh
@@ -94,10 +94,7 @@
loc_cflags="$ASAN_CFLAGS -DPSA_CRYPTO_DRIVER_TEST_ALL"
loc_cflags="${loc_cflags} '-DMBEDTLS_USER_CONFIG_FILE=\"../tests/configs/user-config-for-test.h\"'"
loc_cflags="${loc_cflags} -I../framework/tests/include -O2"
- # Until https://github.com/Mbed-TLS/mbedtls/issues/9814 is fixed,
- # disable the new problematic optimization.
- loc_cflags="${loc_cflags} -fzero-init-padding-bits=unions"
- # Also allow a warning that we don't yet comply to.
+ # Allow a warning that we don't yet comply to.
# https://github.com/Mbed-TLS/mbedtls/issues/9944
loc_cflags="${loc_cflags} -Wno-error=unterminated-string-initialization"