Skip to content

Commit

Permalink
[sival, keymgr] Fix keymgr sealing/attestation test in unmasked envs
Browse files Browse the repository at this point in the history
The `chip_sw_keymgr_derive_{sealing,attestation}` test would fail on
FPGAs with masking disabled due to faulty logic that checks whether
derived keymgr outputs differ between states by not taking into
account that the second output share is always 0 when masking is
disabled.

This commit resolves this issue by flagging two outputs as different
if and only if they differ in at least one share instead of requiring
both shares to be different which was the cause of the failure.

Signed-off-by: Andrea Caforio <[email protected]>
  • Loading branch information
andrea-caforio authored and engdoreis committed Nov 21, 2024
1 parent 11d86df commit 096be43
Showing 1 changed file with 11 additions and 13 deletions.
24 changes: 11 additions & 13 deletions sw/device/tests/keymgr_derive_cdi_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,13 @@ static void init_peripheral_handles(void) {
}

/**
* Compare two masked key manager outputs without unmasking.
* Equality check of two masked key manager outputs without unmasking.
*
* @param output_a First key manager output.
* @param output_b Second key manager output.
* @return Result of the equality check.
*/
static void compare_outputs(const dif_keymgr_output_t *output_a,
static bool compare_outputs(const dif_keymgr_output_t *output_a,
const dif_keymgr_output_t *output_b) {
uint32_t scratch[kKeymgrOutputSizeWords];
for (int i = 0; i < kKeymgrOutputSizeWords; i++) {
Expand All @@ -118,8 +119,11 @@ static void compare_outputs(const dif_keymgr_output_t *output_a,
// In order to prevent an accidental unmasking through reordering of the
// operation after compilation, `t` is read into an auxiliary array.
scratch[i] = output_a->value[0][i] ^ output_b->value[0][i];
CHECK((scratch[i] ^ output_a->value[1][i] ^ output_b->value[1][i]) == 0x0);
if ((scratch[i] ^ output_a->value[1][i] ^ output_b->value[1][i]) != 0x0) {
return false;
}
}
return true;
}

/**
Expand Down Expand Up @@ -266,14 +270,8 @@ static void derive_keys(const char *state_name,
derive_sideload_otbn_key(state_name, next_outputs->sideload_key);

if (prev_outputs) {
CHECK_ARRAYS_NE(prev_outputs->identity.value[0],
next_outputs->identity.value[0], kKeymgrOutputSizeWords);
CHECK_ARRAYS_NE(prev_outputs->identity.value[1],
next_outputs->identity.value[1], kKeymgrOutputSizeWords);
CHECK_ARRAYS_NE(prev_outputs->sw_key.value[0],
next_outputs->sw_key.value[0], kKeymgrOutputSizeWords);
CHECK_ARRAYS_NE(prev_outputs->sw_key.value[1],
next_outputs->sw_key.value[1], kKeymgrOutputSizeWords);
CHECK(!compare_outputs(&prev_outputs->identity, &next_outputs->identity));
CHECK(!compare_outputs(&prev_outputs->sw_key, &next_outputs->sw_key));
CHECK_ARRAYS_NE(prev_outputs->sideload_key, next_outputs->sideload_key,
kKeymgrOutputSizeWords);
}
Expand All @@ -284,8 +282,8 @@ static void derive_keys(const char *state_name,
cdi_outputs_t scratch;
ret_sram_read_keys(&scratch, offset);

compare_outputs(&scratch.identity, &next_outputs->identity);
compare_outputs(&scratch.sw_key, &next_outputs->sw_key);
CHECK(compare_outputs(&scratch.identity, &next_outputs->identity));
CHECK(compare_outputs(&scratch.sw_key, &next_outputs->sw_key));
CHECK_ARRAYS_EQ(scratch.sideload_key, next_outputs->sideload_key,
kKeymgrOutputSizeWords);
}
Expand Down

0 comments on commit 096be43

Please sign in to comment.