commit bbec1c1d25c848ed3d3d5c82864feb13eedf9cfd
author Gilles Peskine <Gilles.Peskine@arm.com> Wed Feb 05 19:53:28 2025 +0100
committer Gilles Peskine <Gilles.Peskine@arm.com> Wed May 07 18:42:33 2025 +0200
tree a42f3d8c3eb165eab827949cb278d668c0facb74
parent a7a480bb81d7a7139c5933c52216211f5a3402c8

Initialize MAC context in internal functions for KDF

In functions that bypass the API functions and call the MAC driver wrapper
`psa_driver_wrapper_mac_sign_setup()` directly, make
sure to initialize the driver-specific part of the context. This is a union,
and initializing the union to `{0}` only guarantees that the first member of
the union is initialized, not necessarily the member used by the driver.
Most compilers do initialize the whole union to all-bits-zero, but some
don't. With compilers that don't, the lack of initialization caused failures
of the affected operations. This affected several key derivation operations.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

library/psa_crypto.c

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index f043411..3342b1b 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -5645,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,
@@ -5657,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 could 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);
 
@@ -5881,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;
 
@@ -6082,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;