Backport cryptolib commits to earlgrey 1.0.0 #28625

h-filali · 2025-10-31T13:36:15Z

This PR is a backport to earlgrey 1.0.0 for my cryptolib PRs that require manual intervention.

This PR also includes one commit from @siemen11

h-filali · 2025-10-31T14:17:05Z

I'm currently doing the diffs between the branches to see what else still needs to go in this PR. I will post an update here once I'm done.

Harden the OTBN code for p256 against side-channel analysis. - Remove the second load of shifting w7 (was not needed) - Shift the second share of the scalar with randomness instead of with zeros - Perform bn.sel without unmasking the scalar Solutions include - decision on xor: L ? a : b = L1 ? (L0 ? b : a) : (L0 ? a : b) - decision on or: L ? a : b = L0 ? a : (L1 ? a : b) Note that in p256.c you can view the instruction count difference. Signed-off-by: Siemen Dhooghe <[email protected]> (cherry picked from commit a37bcf8)

This commit adds hardening for the internal scalar point multiplication. Before the shares would be combined in the main loop to decide whether 0, P or 2P should be added. This commit determines this without combining the shares. If the MSB of both shares are both 1 we want to add 2P. If only one bit is 1 we want to add P. If both bits are 0 we want to add 0. In a simplified way this is done as explained below. In a first step we pick P if MSB(d0) ^ MSB(d1) is 1 and 2P otherwise. This is done as follows: p_temp0 = d0 ? (d1 ? 2P : P) : (d1 ? P : 2P) In a second step we pick between p_temp and 0. If MSB(d0) | MSB(d1) = 1 then we pick p_temp otherwise we pick 0. This is done as follows: p_temp1 = d0 ? p_temp0 : 0 p_temp2 = d1 ? p_temp0 : p_temp1 Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 45453ec)

This commit moves down the instruction count checks for the ECC ECDH finalize functions to after the OTBN status response check. This is necessary since when a user input error occurs, OTBN doesnt escalate to a fatal fault but returns a status code other than OK. In this case the instruction count is not as expected and should not be checked. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 40efc39)

The error reported in case of the is on curve check should only be an input error when the initial public key check is executed. After the initial check, the is on curve check would only return an error in case of an FI attack. In that case we should not report the error as a user error. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit e543156)

The trigger_fault_if_fg0 functions had a bug where we never trigger a fault. This new implementation should now correctly trigger the fault. For the case where no fault is triggered, we load address 0 into w31. For the error case we try to load address 0 into w39 (which doesn't exist), which triggers a ILLEGAL_INSN error. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 7fc6955)

This allows subfunctions to be replaced by NOPs for constant time tests. This is useful for the case where a function runs in constant time under normal conditions but is not necessarily constant time in case of FI. Co-authored-by: Rupert Swarbrick <[email protected]> Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 4795b06)

To follow the OTBN styleguide I added an instruction to clear the flags after a subtraction. The flags however are still needed in the subsequent instructions for the bn.sel. I moved the clear sub instruction down to clear all 4 flags. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 3cd1cf6)

This commit changes the expected values for the tests that have an expected result of 0. This makes sure we don't run into issues where the tests are not executing properly and we still get a correct result due to the expected value of 0. Furthermore, this commit moves the tests to the hjson framework. The input and output values don't change from before this change. This commit merely skips the unnecessary calculations. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 59173d6)

This commit updates the ECC random secret scalar generation function header. I checked the implementation for FIPS 186-5 compliance and updated the header comment accordingly. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 012d8ae)

This commit fixes a bug where the most significant 64 bits were constantly set to 0 instead to the actual seed. Thanks Yi-Hsuan Deng for spotting this! Signed-off-by: Pascal Nasahl <[email protected]> (cherry picked from commit ee334bb)

The comment describes that the code selects bits [192:0] for the shares stored in w9 and w7. However, the code actually selects bits [255:64]. Signed-off-by: Pascal Nasahl <[email protected]> (cherry picked from commit 2fd09e1)

…delines in docs Signed-off-by: Kat Fox <[email protected]> (cherry picked from commit 35f7de9)

It seems like our compiler does not auto convert .data to .bss if all zero vars are defined. This fixes respective .data sections for P384 code which should have been .bss. This saves ca. 2kB of OTBN binary. Signed-off-by: Johann Heyszl <[email protected]> (cherry picked from commit 85c5580)

The added test ensure the shift issue is fixed correctly that two shares are both 320-byte long. Change-Id: I3b94c1ae73076d07c134ddbcee45178281ab0c50 Signed-off-by: Yi-Hsuan Deng <[email protected]> (cherry picked from commit f2ab181) (cherry picked from commit a4b6fc5)

Co-authored-by: Pham Hoang Nguyen Hien <[email protected]> Signed-off-by: Amin Abdulrahman <[email protected]> (cherry picked from commit e9991c0)

This change speeds up Ed25519 and X25519 by about 30%. The core algorithm is the same, but by using OTBN's particular interface slightly more cleverly and combining the multiply with the reduce, we can save quite a few instructions. Signed-off-by: Jade Philipoom <[email protected]> (cherry picked from commit 238e164)

Add support for arbitrary-length inputs to the `bignum_mul` routine. Signed-off-by: Andrea Caforio <[email protected]> (cherry picked from commit 3df0ce7)

Witness and small prime tests are not needed anymore as these routines are obsolete in the subsequent hardened RSA key generation. Signed-off-by: Andrea Caforio <[email protected]> (cherry picked from commit e5038e3)

1. Add a script to transform the mapfile to json and visualize the space utilization with a `d3.js` treemap. 2. Add a build config to disable hardening and measure the impact of hardening on code size. Signed-off-by: Chris Frantz <[email protected]> (cherry picked from commit bc285b7)

h-filali · 2025-10-31T17:11:16Z

The diff in the following folders/files should now be zero:

sw/otbn/crypto
sw/device/lib/base/hardened.c/h
sw/device/lib/base/hardened_memory.c/h

In this folder I checked for any missing commits from @nasahlpa @andrea-caforio and @h-filali :

sw/device/lib/crypto

Let's see what CI says and then it should be ready for review.

johannheyszl

thanks for checking @h-filali

nasahlpa

I think we need to not include the [hmac, sw] Revert hash stop hang workaround commit as the bug is present in eg100?

At certain points the HMAC ctx struct has to be wiped to avoid leaking secrets. The sensitive fields need to be overwritten by randomness and the non-sensitive fields are just zeroized. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit 7389d6c)

As secret information is written by this function into DMEM, randomize the write order using `random_order` to mitigate SCA. Harden the loop against FI by checking the loop counter afterwards. Signed-off-by: Pascal Nasahl <[email protected]> (cherry picked from commit 3be9202)

h-filali · 2025-10-31T18:34:22Z

Thanks @nasahlpa I dropped the last two commits.

OTBN offers the feature to read back the checksum when writing data into memory. This commit extends the `otbn_dmem_write` function such that we compare the checksum computed by OTBN after writing to its memory to an expected checksum we compute on the fly. Signed-off-by: Pascal Nasahl <[email protected]> (cherry picked from commit 98c92ec)

cfrantz · 2025-10-31T19:55:56Z

I think we have to determine of the new OTBN binary is ABI-compatible with the old one. An ABI incompatibility (such as changes to DMEM offsets or IMEM entry points) will cause the otbn_boot_services module to interact with the program loaded by ROM differently than expected, and thus not function correctly.

I don't think any of the current test cases are equipped to detect this problem.

nasahlpa · 2025-10-31T20:29:47Z

sw/otbn/crypto/boot.s

     FG0.Z <= (y^2) mod p == (x^2 + ax + b) mod p */
  bn.cmp   w18, w19
-  jal      x1, trigger_fault_if_fg0_not_z
+  jal      x1, trigger_fault_if_fg0_z


@cfrantz if I unterstand your comment correctly this change is problematic as the adress for the call likely is incorrect?

We could keep the old function call name?

My comment wasn't specifically directed at this line of code, but more generally that any changes to the OTBN boot services program (an older version of which is materialized in the silicon's ROM) need to be tested for ABI compatibility with any changes.

Notably, these symbols need to evaluate to the same values as the version that landed in ROM:
https://github.com/lowRISC/opentitan/blob/earlgrey_1.0.0/sw/device/silicon_creator/lib/otbn_boot_services.c#L22-L44

Should they be different, the ROM_EXT and IMM_SECTION built after a change will be unable to interact with the program loaded into OTBN by ROM. Unfortunately, we cannot include a newer version of the OTBN program into ROM_EXT because there just isn't enough room left.

We can accept changes to the boot services program, but only if they are ABI compatible with the one that exists in ROM.

A reasonable ad-hoc test is to check the symbol values (above) vs the ROM release Earlgrey-PROD-A2-M6-ROM-RC1.

I checked out the ROM release and added this patch: https://gist.github.com/cfrantz/9ab5a20bff6313b0f7e18709c0b9da87

When I run the otbn_boot_services_functest, I see the following:

kOtbnVarBootMode = 000000e0 kOtbnVarBootMsg = 00000100 kOtbnVarBootX = 00000160 kOtbnVarBootY = 00000180 kOtbnVarBootR = 00000120 kOtbnVarBootS = 00000140 kOtbnVarBootXr = 000001a0 kOtbnVarBootOk = 000000e4 kOtbnVarBootAttestationAdditionalSeed = 000001c0

When I check out this PR, apply the patch and run the same test, I get the same values. I believe this means that the OTBN boot program is ABI compatible.

Thanks Chris for the analysis!

This commit adds instruction count checks to ECC functions that run in constant time. This helps protect against fault injection. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit efb425a)

When an error occurs, HARDENED_TRY() immediately returns. As we also want to wipe the OTBN DMEM when an error happens, this commit adds HARDENED_TRY_WIPE_DMEM() which wipes DMEM on an error before returning. Signed-off-by: Pascal Nasahl <[email protected]> (cherry picked from commit a5952c3)

Signed-off-by: Hakim Filali <[email protected]>

h-filali requested a review from a team as a code owner October 31, 2025 13:36

h-filali requested review from alees24 and removed request for a team October 31, 2025 13:36

h-filali requested review from johannheyszl and vogelpi and removed request for alees24 October 31, 2025 14:17

siemen11 and others added 19 commits October 31, 2025 15:18

[crypto] Modify p384_{sign,scalar_mult}.s to comply with OTBN SCA gui…

f400d57

…delines in docs Signed-off-by: Kat Fox <[email protected]> (cherry picked from commit 35f7de9)

[quality,sw/otbn] Add SHA-3 and SHAKE OTBN SW implementations

5208047

Co-authored-by: Pham Hoang Nguyen Hien <[email protected]> Signed-off-by: Amin Abdulrahman <[email protected]> (cherry picked from commit e9991c0)

[crypto/mul] Support arbitrary-length bignum_mul inputs

42a6400

Add support for arbitrary-length inputs to the `bignum_mul` routine. Signed-off-by: Andrea Caforio <[email protected]> (cherry picked from commit 3df0ce7)

[crypto/rsa] Remove obsolete OTBN tests

5c3ba99

Witness and small prime tests are not needed anymore as these routines are obsolete in the subsequent hardened RSA key generation. Signed-off-by: Andrea Caforio <[email protected]> (cherry picked from commit e5038e3)

h-filali force-pushed the backport-cryptolib-prs branch from 4a8029a to 87c3438 Compare October 31, 2025 15:27

h-filali requested a review from cfrantz as a code owner October 31, 2025 15:27

h-filali force-pushed the backport-cryptolib-prs branch from 6168d05 to d5cb56d Compare October 31, 2025 17:29

johannheyszl approved these changes Oct 31, 2025

View reviewed changes

nasahlpa reviewed Oct 31, 2025

View reviewed changes

h-filali and others added 2 commits October 31, 2025 19:29

h-filali force-pushed the backport-cryptolib-prs branch 2 times, most recently from c3a9ab7 to 8f1a429 Compare October 31, 2025 18:32

h-filali force-pushed the backport-cryptolib-prs branch from 8f1a429 to 9fa594b Compare October 31, 2025 18:50

nasahlpa reviewed Oct 31, 2025

View reviewed changes

h-filali and others added 2 commits November 1, 2025 22:51

[cryptolib,sw] Add instruction count checks to ECC functions

94a77ff

This commit adds instruction count checks to ECC functions that run in constant time. This helps protect against fault injection. Signed-off-by: Hakim Filali <[email protected]> (cherry picked from commit efb425a)

h-filali force-pushed the backport-cryptolib-prs branch from df6ddb3 to f5e3353 Compare November 1, 2025 22:53

[sw] clang format fix

67972bc

Signed-off-by: Hakim Filali <[email protected]>

h-filali added the CI:Rerun Rerun failed CI jobs label Nov 2, 2025

github-actions bot removed the CI:Rerun Rerun failed CI jobs label Nov 2, 2025

nasahlpa approved these changes Nov 2, 2025

View reviewed changes

nasahlpa merged commit d404a43 into lowRISC:earlgrey_1.0.0 Nov 2, 2025
67 of 68 checks passed

nasahlpa mentioned this pull request Nov 2, 2025

Backport to earlgrey_1.0.0: [p256] Harden bn.sel and rshi #28576

Closed

johannheyszl mentioned this pull request Nov 3, 2025

[crypto] ABI compatibility test b/w for OTBN boot service app b/w frozen ROM and code changes #28634

Open

cfrantz mentioned this pull request Nov 4, 2025

[sw,otbn] Fix earlgrey otbn patch offset #28578

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Backport cryptolib commits to earlgrey 1.0.0 #28625

Backport cryptolib commits to earlgrey 1.0.0 #28625

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

johannheyszl left a comment

Uh oh!

nasahlpa left a comment

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

cfrantz commented Oct 31, 2025

Uh oh!

nasahlpa Oct 31, 2025

Uh oh!

cfrantz Oct 31, 2025

Uh oh!

cfrantz Oct 31, 2025

Uh oh!

nasahlpa Nov 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Backport cryptolib commits to earlgrey 1.0.0 #28625

Backport cryptolib commits to earlgrey 1.0.0 #28625

Uh oh!

Conversation

h-filali commented Oct 31, 2025

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

johannheyszl left a comment

Choose a reason for hiding this comment

Uh oh!

nasahlpa left a comment

Choose a reason for hiding this comment

Uh oh!

h-filali commented Oct 31, 2025

Uh oh!

cfrantz commented Oct 31, 2025

Uh oh!

nasahlpa Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

cfrantz Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

cfrantz Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

nasahlpa Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants