-
Notifications
You must be signed in to change notification settings - Fork 791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[aes,rtl] Switch to Bivium-based masking PRNG implementation #20852
Conversation
45b2a58
to
6f3c923
Compare
@@ -337,6 +341,7 @@ module prim_trivium_tb ( | |||
.rst_ni(rst_ni), | |||
|
|||
.en_i (bivium_en), | |||
.allow_lockup_i (), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing assignment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was no lint error/warning in Verilator. Most likely because the input isn't used if the StrictLockupProtection
parameter is 0 (default). But it's cleaner. I've changed the design and factored this commit out into #20885 to unblock the vendoring PRs. I think the prim change here is uncontroversial, the rest needs more thorough reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! One small thing in the PR message. As we discussed prior, there was a third commit which isn't there anymore and the PR message refers to a third commit.
While the LFSR-based masking PRNG seems to be able to produce randomness of sufficient quality for the masking implementation in the AES cipher core (as indicated both by our FPGA-based side-channel analysis and by the PROLEAD tool) it may be vulnerable to brute-forcing attacks on the LFSR states. Therefore, this commit replaces the LFSR-based masking PRNG implementation inside AES with an implementation based on an unrolled Bivium stream cipher primitive as suggested in the paper: Cassiers, "Randomness Generation for Secure Hardware Masking - Unrolled Trivium to the Rescue" available at https://eprint.iacr.org/2023/1134 Thanks to bigger individual state chunks - the smallest non-linear feedback shift register (NFSR) has a width of 84 bits - brute-forcing attacks are rendered infeasible. The overall PRNG state width increases from 160 to 177 bits, meaning the randomness consumption for reseeding increases by one 32-bit word per reseed. Thanks to the strong diffusion properties of Bivium, fresh entropy received from EDN can still be injected directly into the PRNG without additional buffering. From an area perspective, the overall AES area increases slightly (+1.81 kGE or +1.1% based on Yosys + nangate45). Thanks again @cassiersg and @AeinRezaeiShahmirzadi for reporting the issue in the first place and for the constructive conversations. This is related to lowRISC#19091. Signed-off-by: Pirmin Vogel <[email protected]>
With the new Bivium-based masking PRNG, the evaluation with PROLEAD consumes a lot more memory and the memory consumption keeps growing with increasing number of simulations in normal mode. Therefore, this commit switches to the compact evaluation mode and instead increases the number of simulations by roughly a factor of 10x as recommended in the PROLEAD wiki. Also, the reported results are updated to match what's achievable with the Bivium-based PRNG. Signed-off-by: Pirmin Vogel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for integrating this @vogelpi!
// - If SecAllowForcingMasks == 1 and force_masks_i == 1, the primitive keeps the all-zero value. | ||
// This may be required to switch off the masking for evaluating the SCA resistance. | ||
logic unused_prng_err; | ||
assign unused_prng_err = prng_err; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we intend to change that behavior and use prng_err
to detect for the SecAllowForcingMasks
case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about this but it would incur bigger design and most importantly DV changes:
- Right now, the cipher core can only trigger fatal alerts but not recoverable alerts. I think this one here shouldn't be fatal. Even if we agreed this was a fatal error unless
SecAllowForcingMasks && force_masks_i
is set, it would incur DV changes. - Right now, all recoverable alerts in AES need to be handled by restarting the message. This here would rather be an informational alert and the module can actually continue operating.
- Also, unless
SecAllowForcingMasks && force_masks_i
is set, the PRNG is cleared to a secret netlist constant immediately like in any other LFSR we instantiate.
Thus, I think this should be sufficient for the current IP version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. SGTM then!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks for your review @msfschaffner ! |
Alright, I am merging this. The PR was now open for 10 days and I got 3 approvals. In case, any further comments / feedback should come in after the merge, I am happy to work out a follow-up PR. Thanks again all for your reviews! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rejoining after a few months and catching up to some of the changes. I like how tidy this makes the PRNG operation for the AES!
While the LFSR-based masking PRNG seems to be able to produce randomness of sufficient quality for the masking implementation in the AES cipher core (as indicated both by our FPGA-based side-channel analysis and by the PROLEAD tool) it may be vulnerable to brute-forcing attacks on the LFSR states.
Therefore, this PR replaces the LFSR-based masking PRNG implementation inside AES with an implementation based on an unrolled Bivium stream cipher primitive as suggested in the paper:
Thanks to bigger individual state chunks - the smallest non-linear feedback shift register (NFSR) has a width of 84 bits - brute-forcing attacks are rendered infeasible. The overall PRNG state width increases from 160 to 177 bits, meaning the randomness consumption for reseeding increases by one 32-bit word per reseed. Thanks to the strong diffusion properties of Bivium, fresh entropy received from EDN can still be injected directly into the PRNG without additional buffering. From an area perspective, the overall AES area increases slightly (+1.81 kGE or +1.1% based on Yosys + nangate45).
This is related to #19091.
The SCA properties of the changes have been evaluated both using PROLEAD and our FPGA-based evaluation framework (using the Englishbreakfast top-level on the CW305 board). All experiments look good.
The first commit prepares the Bivium/Trivium primitive to allow disabling the lockup protection at run time which is more suitable for the optional masking off feature of AES. The second commit contains the actual RTL integration, DV changes, top-level changes due to different seed constants etc. The third commit contains changes to avoid the memory consumption of PROLEAD going much beyond 100 GB during evaluation.