[crypto] Fix the KMAC-KDF implementation to return the output in shares.

Previously, the KMAC driver always unmasked the shares of the digest returned
by the KMAC block. This is OK for hash functions like SHA-3, but problematic
for KMAC-KDF where the output is key material. The implementation also did not
modify the second half of the keyblob buffer in KMAC-KDF, which would cause
correctness issues if those values were not initialized to zero.

This commit refactors the KMAC driver to allow returning the digest in masked
form, and adjusts KMAC-KDF to use this new capability.

Signed-off-by: Jade Philipoom <[email protected]>

sw/device/lib/crypto/drivers/kmac.c

@@ -595,20 +595,23 @@ static status_t kmac_write_key_block(kmac_blinded_key_t *key) {
  * The caller must ensure that `message_len` bytes (rounded up to the next 32b
  * word) are allocated at the location pointed to by `message`, and similarly
  * that `digest_len_words` 32-bit words are allocated at the location pointed
- * to by `digest`.
+ * to by `digest`. If `masked_digest` is set, then `digest` must contain 2x
+ * `digest_len_words` to fit both shares.
  *
  * @param operation The operation type.
  * @param message Input message string.
  * @param message_len Message length in bytes.
  * @param digest The struct to which the result will be written.
  * @param digest_len_words Requested digest length in 32-bit words.
+ * @param masked_digest Whether to return the digest in two shares.
  * @return Error code.
  */
 OT_WARN_UNUSED_RESULT
 static status_t kmac_process_msg_blocks(kmac_operation_t operation,
                                         const uint8_t *message,
                                         size_t message_len, uint32_t *digest,
-                                        size_t digest_len_words) {
+                                        size_t digest_len_words,
+                                        hardened_bool_t masked_digest) {
   // Block until KMAC is idle.
   HARDENED_TRY(wait_status_bit(KMAC_STATUS_SHA3_IDLE_BIT, 1));
 
@@ -691,14 +694,38 @@ static status_t kmac_process_msg_blocks(kmac_operation_t operation,
     // Read words from the state registers (either `digest_len_words` or the
     // maximum number of words available).
     size_t offset = 0;
-    for (; launder32(idx) < digest_len_words && offset < keccak_rate_words;
-         offset++) {
-      uint32_t share0 =
-          abs_mmio_read32(kKmacStateShare0Addr + offset * sizeof(uint32_t));
-      uint32_t share1 =
-          abs_mmio_read32(kKmacStateShare1Addr + offset * sizeof(uint32_t));
-      digest[idx] = share0 ^ share1;
-      ++idx;
+    if (launder32(masked_digest) == kHardenedBoolTrue) {
+      HARDENED_CHECK_EQ(masked_digest, kHardenedBoolTrue);
+      // Read the digest into each share in turn. Do this in separate loops so
+      // corresponding shares aren't handled close together.
+      for (offset = 0; launder32(idx + offset) < digest_len_words &&
+                       offset < keccak_rate_words;
+           offset++) {
+        digest[idx + offset] =
+            abs_mmio_read32(kKmacStateShare0Addr + offset * sizeof(uint32_t));
+      }
+      for (offset = 0; launder32(idx + offset) < digest_len_words &&
+                       offset < keccak_rate_words;
+           offset++) {
+        digest[idx + offset + digest_len_words] =
+            abs_mmio_read32(kKmacStateShare1Addr + offset * sizeof(uint32_t));
+      }
+      idx += offset;
+    } else {
+      // Skip right to the hardened check here instead of returning
+      // `OTCRYPTO_BAD_ARGS` if the value is not `kHardenedBoolFalse`; this
+      // value always comes from within the cryptolib, so we expect it to be
+      // valid and should be suspicious if it's not.
+      HARDENED_CHECK_EQ(masked_digest, kHardenedBoolFalse);
+      // Unmask the digest as we read it.
+      for (; launder32(idx) < digest_len_words && offset < keccak_rate_words;
+           offset++) {
+        digest[idx] =
+            abs_mmio_read32(kKmacStateShare0Addr + offset * sizeof(uint32_t));
+        digest[idx] ^=
+            abs_mmio_read32(kKmacStateShare1Addr + offset * sizeof(uint32_t));
+        idx++;
+      }
     }
 
     // If we read all the remaining words and still need more digest, issue
@@ -732,7 +759,8 @@ status_t kmac_sha3_224(const uint8_t *message, size_t message_len,
 
   size_t digest_len_words = kSha3_224DigestWords;
   return kmac_process_msg_blocks(kKmacOperationSHA3, message, message_len,
-                                 digest, digest_len_words);
+                                 digest, digest_len_words,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_sha3_256(const uint8_t *message, size_t message_len,
@@ -742,7 +770,8 @@ status_t kmac_sha3_256(const uint8_t *message, size_t message_len,
 
   size_t digest_len_words = kSha3_256DigestWords;
   return kmac_process_msg_blocks(kKmacOperationSHA3, message, message_len,
-                                 digest, digest_len_words);
+                                 digest, digest_len_words,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_sha3_384(const uint8_t *message, size_t message_len,
@@ -752,7 +781,8 @@ status_t kmac_sha3_384(const uint8_t *message, size_t message_len,
 
   size_t digest_len_words = kSha3_384DigestWords;
   return kmac_process_msg_blocks(kKmacOperationSHA3, message, message_len,
-                                 digest, digest_len_words);
+                                 digest, digest_len_words,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_sha3_512(const uint8_t *message, size_t message_len,
@@ -762,7 +792,8 @@ status_t kmac_sha3_512(const uint8_t *message, size_t message_len,
 
   size_t digest_len_words = kSha3_512DigestWords;
   return kmac_process_msg_blocks(kKmacOperationSHA3, message, message_len,
-                                 digest, digest_len_words);
+                                 digest, digest_len_words,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_shake_128(const uint8_t *message, size_t message_len,
@@ -771,7 +802,8 @@ status_t kmac_shake_128(const uint8_t *message, size_t message_len,
                          /*hw_backed=*/kHardenedBoolFalse));
 
   return kmac_process_msg_blocks(kKmacOperationSHAKE, message, message_len,
-                                 digest, digest_len);
+                                 digest, digest_len,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_shake_256(const uint8_t *message, size_t message_len,
@@ -780,7 +812,8 @@ status_t kmac_shake_256(const uint8_t *message, size_t message_len,
                          /*hw_backed=*/kHardenedBoolFalse));
 
   return kmac_process_msg_blocks(kKmacOperationSHAKE, message, message_len,
-                                 digest, digest_len);
+                                 digest, digest_len,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_cshake_128(const uint8_t *message, size_t message_len,
@@ -794,7 +827,8 @@ status_t kmac_cshake_128(const uint8_t *message, size_t message_len,
                                        func_name_len, cust_str, cust_str_len));
 
   return kmac_process_msg_blocks(kKmacOperationCSHAKE, message, message_len,
-                                 digest, digest_len);
+                                 digest, digest_len,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
 status_t kmac_cshake_256(const uint8_t *message, size_t message_len,
@@ -808,13 +842,14 @@ status_t kmac_cshake_256(const uint8_t *message, size_t message_len,
                                        func_name_len, cust_str, cust_str_len));
 
   return kmac_process_msg_blocks(kKmacOperationCSHAKE, message, message_len,
-                                 digest, digest_len);
+                                 digest, digest_len,
+                                 /*masked_digest=*/kHardenedBoolFalse);
 }
 
-status_t kmac_kmac_128(kmac_blinded_key_t *key, const uint8_t *message,
-                       size_t message_len, const unsigned char *cust_str,
-                       size_t cust_str_len, uint32_t *digest,
-                       size_t digest_len) {
+status_t kmac_kmac_128(kmac_blinded_key_t *key, hardened_bool_t masked_digest,
+                       const uint8_t *message, size_t message_len,
+                       const unsigned char *cust_str, size_t cust_str_len,
+                       uint32_t *digest, size_t digest_len) {
   HARDENED_TRY(
       kmac_init(kKmacOperationKMAC, kKmacSecurityStrength128, key->hw_backed));
 
@@ -837,13 +872,13 @@ status_t kmac_kmac_128(kmac_blinded_key_t *key, const uint8_t *message,
                                        cust_str_len));
 
   return kmac_process_msg_blocks(kKmacOperationKMAC, message, message_len,
-                                 digest, digest_len);
+                                 digest, digest_len, masked_digest);
 }
 
-status_t kmac_kmac_256(kmac_blinded_key_t *key, const uint8_t *message,
-                       size_t message_len, const unsigned char *cust_str,
-                       size_t cust_str_len, uint32_t *digest,
-                       size_t digest_len) {
+status_t kmac_kmac_256(kmac_blinded_key_t *key, hardened_bool_t masked_digest,
+                       const uint8_t *message, size_t message_len,
+                       const unsigned char *cust_str, size_t cust_str_len,
+                       uint32_t *digest, size_t digest_len) {
   HARDENED_TRY(
       kmac_init(kKmacOperationKMAC, kKmacSecurityStrength256, key->hw_backed));
 
@@ -866,5 +901,5 @@ status_t kmac_kmac_256(kmac_blinded_key_t *key, const uint8_t *message,
                                        cust_str_len));
 
   return kmac_process_msg_blocks(kKmacOperationKMAC, message, message_len,
-                                 digest, digest_len);
+                                 digest, digest_len, masked_digest);
 }

sw/device/lib/crypto/drivers/kmac.h

@@ -240,8 +240,12 @@ status_t kmac_cshake_256(const uint8_t *message, size_t message_len,
  * pointers must be correctly configured and `len` must match the key length.
  *
  * The caller must ensure that `digest_len` words are allocated at the location
- * pointed to by `digest`. `cust_str_len` must not exceed `kKmacCustStrMaxSize`.
+ * pointed to by `digest`. `cust_str_len` must not exceed
+ * `kKmacCustStrMaxSize`. If `masked_digest` is true, the `digest` buffer must
+ * have enough space for 2x `digest_len` words.
  *
+ * @param key The KMAC key.
+ * @param masked_digest Whether to return the digest in concatenated shares.
  * @param message The input message.
  * @param message_len The input message length in bytes.
  * @param cust_str The customization string.
@@ -251,10 +255,10 @@ status_t kmac_cshake_256(const uint8_t *message, size_t message_len,
  * @return Error status.
  */
 OT_WARN_UNUSED_RESULT
-status_t kmac_kmac_128(kmac_blinded_key_t *key, const uint8_t *message,
-                       size_t message_len, const unsigned char *cust_str,
-                       size_t cust_str_len, uint32_t *digest,
-                       size_t digest_len);
+status_t kmac_kmac_128(kmac_blinded_key_t *key, hardened_bool_t masked_digest,
+                       const uint8_t *message, size_t message_len,
+                       const unsigned char *cust_str, size_t cust_str_len,
+                       uint32_t *digest, size_t digest_len);
 
 /**
  * Compute KMAC-256 in one-shot.
@@ -269,8 +273,12 @@ status_t kmac_kmac_128(kmac_blinded_key_t *key, const uint8_t *message,
  * pointers must be correctly configured and `len` must match the key length.
  *
  * The caller must ensure that `digest_len` words are allocated at the location
- * pointed to by `digest`. `cust_str_len` must not exceed `kKmacCustStrMaxSize`.
+ * pointed to by `digest`. `cust_str_len` must not exceed
+ * `kKmacCustStrMaxSize`. If `masked_digest` is true, the `digest` buffer must
+ * have enough space for 2x `digest_len` words.
  *
+ * @param key The KMAC key.
+ * @param masked_digest Whether to return the digest in concatenated shares.
  * @param message The input message.
  * @param message_len The input message length in bytes.
  * @param cust_str The customization string.
@@ -280,10 +288,10 @@ status_t kmac_kmac_128(kmac_blinded_key_t *key, const uint8_t *message,
  * @return Error status.
  */
 OT_WARN_UNUSED_RESULT
-status_t kmac_kmac_256(kmac_blinded_key_t *key, const uint8_t *message,
-                       size_t message_len, const unsigned char *cust_str,
-                       size_t cust_str_len, uint32_t *digest,
-                       size_t digest_len);
+status_t kmac_kmac_256(kmac_blinded_key_t *key, hardened_bool_t masked_digest,
+                       const uint8_t *message, size_t message_len,
+                       const unsigned char *cust_str, size_t cust_str_len,
+                       uint32_t *digest, size_t digest_len);
 
 #ifdef __cplusplus
 }

sw/device/lib/crypto/impl/kdf.c

@@ -291,10 +291,10 @@ otcrypto_status_t otcrypto_kdf_kmac(
     }
     // No need to further check key size against security level because
     // `kmac_key_length_check` ensures that the key is at least 128-bit.
-    HARDENED_TRY(kmac_kmac_128(&kmac_key, kdf_context.data, kdf_context.len,
-                               kdf_label.data, kdf_label.len,
-                               keying_material->keyblob,
-                               required_byte_len / sizeof(uint32_t)));
+    HARDENED_TRY(kmac_kmac_128(
+        &kmac_key, /*masked_digest=*/kHardenedBoolTrue, kdf_context.data,
+        kdf_context.len, kdf_label.data, kdf_label.len,
+        keying_material->keyblob, required_byte_len / sizeof(uint32_t)));
   } else if (kmac_mode == kOtcryptoKmacModeKmac256) {
     // Check if `key_mode` of the key derivation key matches `kmac_mode`.
     if (key_derivation_key.config.key_mode != kOtcryptoKeyModeKdfKmac256) {
@@ -305,10 +305,10 @@ otcrypto_status_t otcrypto_kdf_kmac(
     if (key_derivation_key.config.key_length < 256 / 8) {
       return OTCRYPTO_BAD_ARGS;
     }
-    HARDENED_TRY(kmac_kmac_256(&kmac_key, kdf_context.data, kdf_context.len,
-                               kdf_label.data, kdf_label.len,
-                               keying_material->keyblob,
-                               required_byte_len / sizeof(uint32_t)));
+    HARDENED_TRY(kmac_kmac_256(
+        &kmac_key, /*masked_digest=*/kHardenedBoolTrue, kdf_context.data,
+        kdf_context.len, kdf_label.data, kdf_label.len,
+        keying_material->keyblob, required_byte_len / sizeof(uint32_t)));
   } else {
     return OTCRYPTO_BAD_ARGS;
   }

sw/device/lib/crypto/impl/mac.c

@@ -279,19 +279,21 @@ otcrypto_status_t otcrypto_kmac(const otcrypto_blinded_key_t *key,
       if (key->config.key_mode != kOtcryptoKeyModeKmac128) {
         return OTCRYPTO_BAD_ARGS;
       }
-      HARDENED_TRY(kmac_kmac_128(&kmac_key, input_message.data,
-                                 input_message.len, customization_string.data,
-                                 customization_string.len, tag.data, tag.len));
+      HARDENED_TRY(kmac_kmac_128(
+          &kmac_key, /*masked_digest=*/kHardenedBoolFalse, input_message.data,
+          input_message.len, customization_string.data,
+          customization_string.len, tag.data, tag.len));
       break;
     case kOtcryptoKmacModeKmac256:
       // Check `key_mode` matches `mac_mode`
       if (key->config.key_mode != kOtcryptoKeyModeKmac256) {
         return OTCRYPTO_BAD_ARGS;
       }
 
-      HARDENED_TRY(kmac_kmac_256(&kmac_key, input_message.data,
-                                 input_message.len, customization_string.data,
-                                 customization_string.len, tag.data, tag.len));
+      HARDENED_TRY(kmac_kmac_256(
+          &kmac_key, /*masked_digest=*/kHardenedBoolFalse, input_message.data,
+          input_message.len, customization_string.data,
+          customization_string.len, tag.data, tag.len));
       break;
     default:
       return OTCRYPTO_BAD_ARGS;

sw/device/tests/crypto/BUILD

@@ -468,6 +468,7 @@ opentitan_test(
         "//sw/device/lib/crypto/drivers:kmac",
         "//sw/device/lib/crypto/impl:hash",
         "//sw/device/lib/crypto/impl:kdf",
+        "//sw/device/lib/crypto/impl:key_transport",
         "//sw/device/lib/crypto/impl:mac",
         "//sw/device/lib/testing/test_framework:check",
         "//sw/device/lib/testing/test_framework:ottf_main",

sw/device/tests/crypto/kdf_kmac_functest.c

@@ -8,6 +8,7 @@
 #include "sw/device/lib/crypto/include/datatypes.h"
 #include "sw/device/lib/crypto/include/hash.h"
 #include "sw/device/lib/crypto/include/kdf.h"
+#include "sw/device/lib/crypto/include/key_transport.h"
 #include "sw/device/lib/crypto/include/mac.h"
 #include "sw/device/lib/runtime/log.h"
 #include "sw/device/lib/testing/test_framework/check.h"
@@ -50,13 +51,8 @@ status_t get_kmac_mode(kdf_test_operation_t test_operation,
 static status_t run_test_vector(void) {
   // Below, `km` prefix refers to keying_material, and
   // `kdk` prefix refers to key derivation key
-  size_t km_key_length = current_test_vector->keying_material.config.key_length;
-
-  size_t km_num_words = km_key_length / sizeof(uint32_t);
-  if (km_key_length % sizeof(uint32_t) != 0) {
-    km_num_words++;
-  }
-  uint32_t km_buffer[km_num_words];
+  size_t km_num_words = current_test_vector->expected_output.len;
+  uint32_t km_buffer[2 * km_num_words];
 
   otcrypto_kmac_mode_t mode;
   TRY(get_kmac_mode(current_test_vector->test_operation, &mode));
@@ -67,32 +63,42 @@ static status_t run_test_vector(void) {
               // The following key_mode is a dummy placeholder. It does not
               // necessarily match the `key_length`.
               .key_mode = kOtcryptoKeyModeKdfKmac128,
-              .key_length = km_key_length,
+              .key_length = km_num_words * sizeof(uint32_t),
               .hw_backed = kHardenedBoolFalse,
               .security_level = kOtcryptoKeySecurityLevelLow,
               .exportable = kHardenedBoolTrue,
           },
       .keyblob = km_buffer,
-      .keyblob_length = 2 * km_key_length};
+      .keyblob_length = sizeof(km_buffer),
+  };
 
   // Populate `checksum` and `config.security_level` fields.
   current_test_vector->key_derivation_key.checksum =
       integrity_blinded_checksum(&current_test_vector->key_derivation_key);
 
-  TRY(otcrypto_kdf_kmac(
-      current_test_vector->key_derivation_key, mode, current_test_vector->label,
-      current_test_vector->context, km_key_length, &keying_material));
+  TRY(otcrypto_kdf_kmac(current_test_vector->key_derivation_key, mode,
+                        current_test_vector->label,
+                        current_test_vector->context,
+                        keying_material.config.key_length, &keying_material));
 
   HARDENED_CHECK_EQ(integrity_blinded_key_check(&keying_material),
                     kHardenedBoolTrue);
 
-  // At the moment, the function `kmac_kmac_128` called under the hood does not
-  // return the digest in 2 shares. Therefore we can simply compare the first
-  // share of the result from the test vector, since the second share is always
-  // 0.
-  TRY_CHECK_ARRAYS_EQ((uint8_t *)keying_material.keyblob,
-                      (uint8_t *)current_test_vector->keying_material.keyblob,
-                      km_key_length);
+  // Export the derived blinded key.
+  uint32_t km_share0[km_num_words];
+  uint32_t km_share1[km_num_words];
+  TRY(otcrypto_export_blinded_key(
+      keying_material,
+      (otcrypto_word32_buf_t){.data = km_share0, .len = ARRAYSIZE(km_share0)},
+      (otcrypto_word32_buf_t){.data = km_share1, .len = ARRAYSIZE(km_share1)}));
+
+  // Unmask the derived key and compare to the expected value.
+  uint32_t actual_output[km_num_words];
+  for (size_t i = 0; i < ARRAYSIZE(actual_output); i++) {
+    actual_output[i] = km_share0[i] ^ km_share1[i];
+  }
+  TRY_CHECK_ARRAYS_EQ(actual_output, current_test_vector->expected_output.data,
+                      km_num_words);
   return OTCRYPTO_OK;
 }