[crypto/aes] Cleanup on failures

Use the cleanup attribute in order to attach cleanup functions to
sensitive variables together with a guard to cleanup the hardware.
These functions are called when the variables go out of scope.
Hence, we clear sensitive variables (hardened_memshred) or the hardware
on both the succesful execution (as it was before) and on bad status
output (which is new).

Apply this new setting to the AES where we split the driver to no longer
clean the HW when calling aes_end. Instead, we always call it from impl
(and users using the drivers still can manuallt call the clean in
drivers).

Signed-off-by: Siemen Dhooghe <sdhooghe@google.com>

Author: siemen11

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

@@ -343,6 +343,10 @@ status_t aes_end(aes_block_t *iv) {
     HARDENED_CHECK_EQ(i, ARRAYSIZE(iv->data));
   }
 
+  return spin_until(AES_STATUS_IDLE_BIT);
+}
+
+status_t aes_clear(void) {
   uint32_t trigger_reg = 0;
   trigger_reg = bitfield_bit32_write(
       trigger_reg, AES_TRIGGER_KEY_IV_DATA_IN_CLEAR_BIT, true);

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

@@ -173,6 +173,14 @@ status_t aes_update(aes_block_t *dest, const aes_block_t *src);
 OT_WARN_UNUSED_RESULT
 status_t aes_end(aes_block_t *iv);
 
+/**
+ * Clears the AES hardware data registers and triggers.
+ *
+ * @return The result of the operation.
+ */
+OT_WARN_UNUSED_RESULT
+status_t aes_clear(void);
+
 /**
  * Compute the checksum of an AES key.
  *

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

@@ -279,6 +279,28 @@ otcrypto_status_t otcrypto_aes_padded_plaintext_length(
   return OTCRYPTO_OK;
 }
 
+/**
+ * Consolidated state for AES operations.
+ * Grouping these reduces the compiler RAII tracking overhead to a single
+ * variable.
+ */
+typedef struct {
+  aes_key_t key;
+  aes_block_t iv;
+  aes_block_t block_in;
+  aes_block_t block_out;
+} aes_impl_state_t;
+
+/**
+ * Cleanup function.
+ */
+static void cleanup_aes_state(aes_impl_state_t *state) {
+  (void)aes_clear();
+  (void)keymgr_sideload_clear_aes();
+  hardened_memshred((uint32_t *)state,
+                    sizeof(aes_impl_state_t) / sizeof(uint32_t));
+}
+
 /**
  * Performs the AES operation.
  *
@@ -297,6 +319,10 @@ static otcrypto_status_t otcrypto_aes_impl(
     otcrypto_aes_mode_t aes_mode, otcrypto_aes_operation_t aes_operation,
     const otcrypto_const_byte_buf_t *cipher_input,
     otcrypto_aes_padding_t aes_padding, otcrypto_byte_buf_t *cipher_output) {
+  // Set the attribute with the state to clean up all sensitive variables and
+  // clear the HW upon exit.
+  aes_impl_state_t state __attribute__((cleanup(cleanup_aes_state))) = {0};
+
   // Calculate the number of blocks for the input, including the padding for
   // encryption.
   size_t input_nblocks;
@@ -324,7 +350,6 @@ static otcrypto_status_t otcrypto_aes_impl(
 
   // Construct the IV and check its length. ECB mode will ignore the IV, so in
   // this case it is left uninitialized.
-  aes_block_t aes_iv;
   if (aes_mode == kAesCipherModeEcb) {
     HARDENED_CHECK_EQ(launder32(aes_mode), kAesCipherModeEcb);
   } else {
@@ -335,23 +360,22 @@ static otcrypto_status_t otcrypto_aes_impl(
       return OTCRYPTO_BAD_ARGS;
     }
     HARDENED_CHECK_EQ(launder32(iv->len), kAesBlockNumWords);
-    HARDENED_TRY(hardened_memcpy(aes_iv.data, iv->data, kAesBlockNumWords));
+    HARDENED_TRY(hardened_memcpy(state.iv.data, iv->data, kAesBlockNumWords));
   }
 
   // Parse the AES key.
-  aes_key_t aes_key;
-  HARDENED_TRY(aes_key_construct(key, aes_mode, &aes_key));
+  HARDENED_TRY(aes_key_construct(key, aes_mode, &state.key));
 
   // Start the operation (encryption or decryption).
   otcrypto_aes_operation_t aes_operation_started = launder32(0);
   switch (aes_operation) {
     case kOtcryptoAesOperationEncrypt:
-      HARDENED_TRY(aes_encrypt_begin(aes_key, &aes_iv));
+      HARDENED_TRY(aes_encrypt_begin(state.key, &state.iv));
       aes_operation_started =
           launder32(aes_operation_started) | kOtcryptoAesOperationEncrypt;
       break;
     case kOtcryptoAesOperationDecrypt:
-      HARDENED_TRY(aes_decrypt_begin(aes_key, &aes_iv));
+      HARDENED_TRY(aes_decrypt_begin(state.key, &state.iv));
       aes_operation_started =
           launder32(aes_operation_started) | kOtcryptoAesOperationDecrypt;
       break;
@@ -389,45 +413,44 @@ static otcrypto_status_t otcrypto_aes_impl(
   //
   // See the AES driver for details.
   const size_t block_offset = input_nblocks >= 3 ? 2 : 1;
-  aes_block_t block_in;
-  aes_block_t block_out;
   size_t i;
 
   // Provide the first `block_offset` number of input blocks and call the AES
   // cipher.
   for (i = 0; launder32(i) < block_offset; ++i) {
-    HARDENED_TRY(get_block(cipher_input, aes_padding, i, &block_in));
-    HARDENED_TRY(aes_update(/*dest=*/NULL, &block_in));
+    HARDENED_TRY(get_block(cipher_input, aes_padding, i, &state.block_in));
+    HARDENED_TRY(aes_update(/*dest=*/NULL, &state.block_in));
   }
   // Check that the loop ran for the correct number of iterations.
   HARDENED_CHECK_EQ(i, block_offset);
 
   // Call the AES cipher while providing new input and copying data to the
   // output buffer.
   for (i = block_offset; launder32(i) < input_nblocks; ++i) {
-    HARDENED_TRY(get_block(cipher_input, aes_padding, i, &block_in));
-    HARDENED_TRY(hardened_memshred(block_out.data, ARRAYSIZE(block_out.data)));
-    HARDENED_TRY(aes_update(&block_out, &block_in));
+    HARDENED_TRY(get_block(cipher_input, aes_padding, i, &state.block_in));
+    HARDENED_TRY(hardened_memshred(state.block_out.data,
+                                   ARRAYSIZE(state.block_out.data)));
+    HARDENED_TRY(aes_update(&state.block_out, &state.block_in));
     // Byte buffers passed as input may not be word-aligned, so we cannot
     // use `hardened_memcpy`.
     // Hence, use `randomized_bytecopy` instead.
     HARDENED_TRY(randomized_bytecopy(
         &cipher_output->data[(i - block_offset) * kAesBlockNumBytes],
-        block_out.data, kAesBlockNumBytes));
+        state.block_out.data, kAesBlockNumBytes));
   }
   // Check that the loop ran for the correct number of iterations.
   HARDENED_CHECK_EQ(i, input_nblocks);
 
   // Retrieve the output from the final `block_offset` blocks (providing no
   // input).
   for (i = block_offset; i > 0; --i) {
-    HARDENED_TRY(aes_update(&block_out, /*src=*/NULL));
+    HARDENED_TRY(aes_update(&state.block_out, /*src=*/NULL));
     // Byte buffers passed as input may not be word-aligned, so we cannot
     // use `hardened_memcpy`.
     // Hence, use `randomized_bytecopy` instead.
     HARDENED_TRY(randomized_bytecopy(
         &cipher_output->data[(input_nblocks - i) * kAesBlockNumBytes],
-        block_out.data, kAesBlockNumBytes));
+        state.block_out.data, kAesBlockNumBytes));
   }
   // Check that the loop ran for the correct number of iterations.
   HARDENED_CHECK_EQ(launder32(i), 0);
@@ -439,19 +462,18 @@ static otcrypto_status_t otcrypto_aes_impl(
   hardened_bool_t encrypt = kHardenedBoolTrue;
   if (aes_operation == kOtcryptoAesOperationDecrypt)
     encrypt = kHardenedBoolFalse;
-  HARDENED_TRY(aes_verify_ctrl_reg(aes_key, encrypt));
+  HARDENED_TRY(aes_verify_ctrl_reg(state.key, encrypt));
   HARDENED_TRY(aes_verify_ctrl_aux_reg());
 
   // Deinitialize the AES block and update the IV (in ECB mode, skip the IV).
   if (aes_mode == kAesCipherModeEcb) {
     HARDENED_TRY(aes_end(NULL));
   } else {
-    HARDENED_TRY(aes_end(&aes_iv));
-    HARDENED_TRY(hardened_memcpy(iv->data, aes_iv.data, kAesBlockNumWords));
+    HARDENED_TRY(aes_end(&state.iv));
+    HARDENED_TRY(hardened_memcpy(iv->data, state.iv.data, kAesBlockNumWords));
   }
 
-  // In case the key was sideloaded, clear it.
-  return otcrypto_eval_exit(keymgr_sideload_clear_aes());
+  return otcrypto_eval_exit(OTCRYPTO_OK);
 }
 
 otcrypto_status_t otcrypto_aes_padding_strip(