Skip to content

[rom_ext] Clear the OTBN DMEM after endorsement#30705

Merged
siemen11 merged 5 commits into
lowRISC:earlgrey_1.0.0from
sasdf:mjTvonxmspv
Jul 13, 2026
Merged

[rom_ext] Clear the OTBN DMEM after endorsement#30705
siemen11 merged 5 commits into
lowRISC:earlgrey_1.0.0from
sasdf:mjTvonxmspv

Conversation

@sasdf

@sasdf sasdf commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

This change adds a call to otbn_boot_attestation_key_clear() right after fetching all the data from DMEM in otbn_boot_attestation_endorse().

This ensures the secrets are wiped as soon as possible after it is no longer needed.

@sasdf sasdf requested review from cfrantz and siemen11 July 10, 2026 01:40
@sasdf sasdf marked this pull request as ready for review July 10, 2026 06:45
@sasdf sasdf requested a review from a team as a code owner July 10, 2026 06:45
@sasdf sasdf requested review from engdoreis and nasahlpa and removed request for a team and engdoreis July 10, 2026 06:45
Comment thread sw/device/silicon_creator/lib/otbn_boot_services.c Outdated
Comment thread sw/device/silicon_creator/lib/otbn_boot_services.c

@siemen11 siemen11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @sasdf! This is a cleaner way to handle the OTBN. Cleanest would be to handle the full wipe, but we can discuss that at a later point

sasdf added 2 commits July 10, 2026 14:33
Introduce a backup mechanism for the OTBN boot app DMEM data
section in `otbn_boot_attestation_key_clear()`.

Because the function wipes the entire DMEM section to clear the
attestation key, we must first back up and then restore the read-only
data section of the boot services app so that future OTBN operations do not
fail due to missing or wiped constant data.

Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com>
Change-Id: I36b77038f4cb60670bbc703654799cae6a6a6964
This change adds a call to `otbn_boot_attestation_key_clear()` right
after fetching all the data from DMEM in `otbn_boot_attestation_endorse()`.
This ensures the secrets are wiped as soon as possible after it is no
longer needed.

Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com>
Change-Id: I4bc2d7a481a16d01bc5c162a944c7afb6a6a6964
@sasdf

sasdf commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I have added more comments in the code to clarify what is being backed up and the reasoning behind it.

This PR already performs a full wipe. The data we back up consists of read-only public constants for the application.
Since we want to avoid inflating ROM_EXT with this section embedded to restore the app, we read it from DMEM instead.

@sasdf sasdf requested review from nasahlpa and siemen11 July 10, 2026 14:43

@siemen11 siemen11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the work!

@cfrantz

cfrantz commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Does this change have any measureable effect on boot timing?

Have we run the downstream provisioning CI with this change?

sasdf added 3 commits July 10, 2026 17:42
This change ensures that during the personalization process,
the CDI_0 and CDI_1 attestation keys are explicitly saved using
`otbn_boot_attestation_key_save`.

Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com>
Change-Id: I1292d5a0b556662320bc863e099a8eae6a6a6964
Instead of saving the key before going to the next stage, we save the
issuer key before key manager advancement.
This avoids leaving key material in OTBN DMEM for longer than necessary.

Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com>
Change-Id: I0464c1d8661f971fd56591e10fb0a3536a6a6964
The `dice_chain` module now handles saving the issuer key to OTBN DMEM,
rendering these individual `key_save` calls in the `dice` and `dice_cwt`
libraries redundant.

Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com>
Change-Id: I9ea1f54d927ae48ace06e7ba1c0ee5d46a6a6964
@sasdf

sasdf commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Does this change have any measureable effect on boot timing?

The latency difference is negligible. Measured with ibex mcycle on FPGA, it has about 0.86% increase for the cdi_1 builder runtime (1962813 -> 1979719), and it has far less impact when comparing to the overall boot time.

Have we run the downstream provisioning CI with this change?

Yes, the tests pass. (with some existing flakiness)

@siemen11 siemen11 merged commit 6770e06 into lowRISC:earlgrey_1.0.0 Jul 13, 2026
35 checks passed
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.

4 participants