Skip to content

Conversation

@vogelpi
Copy link
Contributor

@vogelpi vogelpi commented Jan 9, 2023

Previously, it was possible to glitch data_rvalid_i at the interconnect level and if the data integrity bits happened to be valid, Ibex would write the current data_rdata_i into the register file even if it wasn't doing a load. Since the glitch is inserted at the interconnect level, both the main and the shadow core are affected equally.

This commit changes the WB stage to only forward the LSU write enable, which is generated from data_rvalid_i, when Ibex is actually waiting for an interconnect response for a load instruction. This substantially narrows down the window for attacks at the interconnect level.

@vogelpi vogelpi requested a review from GregAC January 9, 2023 17:24
Previously, it was possible to glitch data_rvalid_i at the interconnect
level and if the data integrity bits happened to be valid, Ibex would
write the current data_rdata_i into the register file even if it wasn't
doing a load. Since the glitch is inserted at the interconnect level,
both the main and the shadow core are affected equally.

This commit changes the WB stage to only forward the LSU write enable,
which is generated from data_rvalid_i, when Ibex is actually waiting for
an interconnect response for a load instruction. This substantially
narrows down the window for attacks at the interconnect level.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
@vogelpi vogelpi force-pushed the wb-rf-we-hardening branch from fdd952f to 6ce8726 Compare January 9, 2023 17:26
@vogelpi vogelpi requested a review from andreaskurth January 9, 2023 17:27
Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good catch!

Do we also need to harden the Ibex configuration without WB stage against this? (Probably not for OT, but we could create an issue in the Ibex repo to track this?)

@vogelpi
Copy link
Contributor Author

vogelpi commented Jan 10, 2023

Very good catch!

Do we also need to harden the Ibex configuration without WB stage against this? (Probably not for OT, but we could create an issue in the Ibex repo to track this?)

Thanks @andreaskurth . I think I wouldn't harden the configuration without WB, as this config is less relevant and the hardening there would be more involved. The nice thing about hardening the WB configuration is that the required flop is already there. So it's just about adding a single AND gate.

@andreaskurth
Copy link
Contributor

I think I wouldn't harden the configuration without WB, as this config is less relevant and the hardening there would be more involved. The nice thing about hardening the WB configuration is that the required flop is already there. So it's just about adding a single AND gate.

Thanks for the explanation. I agree that hardening the config without WB is not necessary at the moment, but I would suggest to either track this as an issue or document that some hardening requires the WB stage to be instantiated. Right now, the Ibex docs state "Ibex implements a set of extra features (when the SecureIbex parameter is set) to support security-critical applications", which is logically correct but does not mention that there are other parameters that influence the implemented security features.

@vogelpi
Copy link
Contributor Author

vogelpi commented Jan 16, 2023

This sounds reasonable @andreaskurth . I've created an issue to track this here #1970 and will now merge this PR.

@vogelpi vogelpi merged commit 590d196 into lowRISC:master Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants