Skip to content
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

[pentest] Update and add new OTBN FI tests #24293

Merged
merged 4 commits into from
Sep 3, 2024

Conversation

nasahlpa
Copy link
Member

During the penetration testing, new OTBN FI tests and existing ones have been improved. As this work happened in a fork, this PR brings these changes upstream. The first commit two commits update the existing tests and the last one adds new tests.

@nasahlpa nasahlpa marked this pull request as ready for review August 13, 2024 12:48
@nasahlpa nasahlpa requested a review from a team as a code owner August 13, 2024 12:48
@nasahlpa nasahlpa requested review from moidx and removed request for a team and moidx August 13, 2024 12:48
Copy link
Contributor

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

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

We had reviewed the tests in the private fork. The code is also successfully tested. Thanks Pascal!

This commit adds two DIF functions to OTBN allowing to read
and clear the load checksum register.

Signed-off-by: Pascal Nasahl <[email protected]>
To be consistent between each test, this commit clears the OTBN DMEM
and IMEM between each test run.

Signed-off-by: Pascal Nasahl <[email protected]>
This commit pulls over the following tests from the pentest branch of
nasahlpa/opentitan that has been used for the penetration testing:
- otbn.fi.load_integrity
- otbn.fi.key_sideload

Signed-off-by: Pascal Nasahl <[email protected]>
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks Pascal, this is okay to get merged.

For the record: This is not production code but code that was used for penetration testing of Earlgrey ES. We should not implement changes to these tests that might have an impact on the test results. Instead the aim is to integrate the code more or less as is to be able to reproduce results and to have it ready for future penetration testing campaigns. The first PR of this series (#24294) has been reviewed by the SW team. The suggested style changes have also been integrated into this PR.

This is a pure cosmetical change that alphabetically orders the
OTBN FI tests.

Signed-off-by: Pascal Nasahl <[email protected]>
@nasahlpa nasahlpa added the Status:Ready to merge PR is ready to be merged by a committer. label Sep 2, 2024
@andreaskurth andreaskurth merged commit fd4ae24 into lowRISC:master Sep 3, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Ready to merge PR is ready to be merged by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants