lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.57k stars 764 forks source link

[csrng] Consider making AES_CIPHER_DISABLE configurable by fuses or RTL parameters #5576

Closed moidx closed 3 years ago

moidx commented 3 years ago

Evaluate the purpose of AES_CIPHER_DISABLE and determine if it needs to have OTP support or be converted to an RTL parameter. If left as a register, document the rationale for doing so.

moidx commented 3 years ago

@mwbranstad @martin-lueker @cdgori @tjaychen

mwbranstad commented 3 years ago

This bit is also qualified by the state of the LC input, which if in "debug" will always ENABLE the AES cipher. Untimately this was a convenience bit for verification and validation, which we cannot completely ignore. @senelson7 any comments on this one?

senelson7 commented 3 years ago

For verification, I can control the LC input and don't really need the register bit unless the LC input also affects something else. But for hardware validation, is the register bit the only way to easily enable/disable AES?

tjaychen commented 3 years ago

technically for the final production design, we "should" have no reason to bypass the aes. I think Miguel's concern is that if we are able to bypass the aes via some kind of attack, it would completely weaken the drbg construction. So maybe it's better to not have it there at all.

On Fri, Mar 12, 2021 at 12:53 PM senelson7 @.***> wrote:

For verification, I can control the LC input and don't really need the register bit unless the LC input also affects something else. But for hardware validation, is the register bit the only way to easily enable/disable AES?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/5576#issuecomment-797748665, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSQ6EVHWO5NOG3TQV6LTDJWLDANCNFSM4ZBQAGFQ .

vogelpi commented 3 years ago

The option to bypass AES inside CSRNG has been removed with #7319. I am thus closing this issue.