[pentest] Fix masking off for AES-SCA #25932
Conversation
Previously, the pentest_set_trigger_high/low functions were called at different places inside the code. However, this is not really - instead we could do it only inside the aes_encrypt function. This simplifies the code. Signed-off-by: Pascal Nasahl <[email protected]>
When porting the AES SCA tests from the old simpleserial to the new uJSON framework, we also copied the aes_sca_error_t structure. This structure was used to indicate whether a function inside the aes_sca program returned with an error. Instead, we simply can use the default status_t with the TRY() construct. This (a) simplifies the code and (b) follows the coding style of the OpenTitan code base. Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa
added
the
CherryPick:earlgrey_1.0.0
| pentest_set_trigger_high(); | ||
| for (uint32_t i = 0; i < num_encryptions; ++i) { | ||
| aes_key_mask_and_config(batch_keys[i], kAesKeyLength); | ||
| aes_encrypt(batch_plaintexts[i], kAesTextLength); | ||
| } | ||
| pentest_set_trigger_low(); |
There was a problem hiding this comment.
We moved those functions out of the encrypt to not have to repeatedly call it inside the hot loop during batch encryptions. I think it did have a notable performance impact but don't remember the numbers to be honest.
There was a problem hiding this comment.
I've checked whether this makes a big difference:
this PR:
simpleserial aes_fvsr_key_batch 20 batches: 615 traces / s
simpleserial aes_fvsr_key_batch 50 batches: 1444 traces / s
before this PR:
simpleserial aes_fvsr_key_batch 20 batches: 617 traces / s
simpleserial aes_fvsr_key_batch 50 batches: 1450 traces / s
So I think the change is reasonable :-)
sw/device/sca/aes_serial.c
Outdated
| aes_sca_wait_for_idle(); | ||
| SS_CHECK_DIF_OK(dif_aes_trigger(&aes, kDifAesTriggerPrngReseed)); | ||
| aes_sca_wait_for_idle(); | ||
| SS_CHECK_DIF_OK(dif_aes_trigger(&aes, kDifAesTriggerDataOutClear)); |
There was a problem hiding this comment.
I didn't remember this was needed but it turns out it really is! It maybe worth to add a comment why.
There was a problem hiding this comment.
I quickly checked, we actually do not need to clear the data out registers with dif_aes_trigger(&aes, kDifAesTriggerDataOutClear). Only triggering the Prng Reseed is sufficient and gives us the TVLA plot from above.
Added a comment & removed the data register clearing.
| AES_TESTUTILS_WAIT_FOR_STATUS(&aes, kDifAesStatusIdle, true, kTestTimeout); | ||
| TRY(dif_aes_trigger(&aes, kDifAesTriggerPrngReseed)); | ||
| AES_TESTUTILS_WAIT_FOR_STATUS(&aes, kDifAesStatusIdle, true, kTestTimeout); | ||
| TRY(dif_aes_trigger(&aes, kDifAesTriggerDataOutClear)); |
There was a problem hiding this comment.
Also here I would add a comment.
| #if !OT_IS_ENGLISH_BREAKFAST | ||
| if (transaction.force_masks) { | ||
| // Disable masking. Force the masking PRNG output value to 0. | ||
| TRY(aes_sca_load_fixed_seed()); | ||
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
IIRC, it doesn't hurt to call this on the CW305. Because the trigger register is there but the reseeding is just ignored (there is no CSRNG though which is why we need to exclude the CSRNG setup).
There was a problem hiding this comment.
It seems when adding this also for CW305, the code does not compile anymore. So I've kept the if guards.
With the recent changes in the PRNG, it is necessary to reseed the internal AES state after the AUX register has been set by using the dif_aes_start() function. We need to do this when we want to switch off masking for AES SCA tests. Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa
force-pushed
the
fix_aes_masks_off
branch
from
559314c to
8a379c6
Compare
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin earlgrey_1.0.0
git worktree add -d .worktree/backport-25932-to-earlgrey_1.0.0 origin/earlgrey_1.0.0
cd .worktree/backport-25932-to-earlgrey_1.0.0
git switch --create backport-25932-to-earlgrey_1.0.0
git cherry-pick -x 5b864fede5d3630cb62436cb0cd6114322e9088f e91a4d2037e4249724f5581ba604588e261f5dce 8a379c6e26e5902cddd8063942c5b09ba224bafb |
github-actions
bot
added
the
Manually CherryPick
This PR consists of three different commits:
dif_aes_start()) do a reseed.Before this commit, turning off the masks for AES gave us the following TVLA figure:
As shown in the figure, leakage is only detected in the first AES rounds. However, with masking disabled, we expect leakage in all rounds.
With these changes, the masking off now works again:
