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

[opentitantool] Correct the SHA256 hash ordering #25969

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

cfrantz
Copy link
Contributor

@cfrantz cfrantz commented Jan 22, 2025

Opentitanlib was computing the SHA256 hash and storing it in a fixed-sized bigint. This led to various problems dealing with the endianness of the resulting digest, including emitting digest in "little-endian" order. A SHA256 digest is not an integer; it is a sequence of bytes.

  1. Eliminate the bigint representation of the SHA256 digest. Give the digest struct serialization, display and parsing traits to eliminate the need for dealing with conversions for those ues cases. Add to_vec and to_vec_rev to produce the raw bytes when needed.
  2. Add a --spx-hash-reversal-bug switch to opentitantool spx commands for cases where firmware erroneously uses a reversed digest.
  3. Eliminate the copious digest reversals present in hsmtool. Add a Sha256HashReversed input format for cases where firmware erroneously uses a reversed digest.
  4. Fix the signing rules to use the reversed format only for keys tied to a firmware implementation with the reversal bug.

Fixes #25870

@vbendeb
Copy link

vbendeb commented Jan 22, 2025

I don't have any comments on the code, it would be good to see a test case when a signature is generated by an external entity and validated by hsmtool for all signature types or better yet by rom_ext when verifying owner's image signature.

@cfrantz
Copy link
Contributor Author

cfrantz commented Jan 22, 2025

Addresses #25967

@cfrantz
Copy link
Contributor Author

cfrantz commented Jan 22, 2025

I don't have any comments on the code, it would be good to see a test case when a signature is generated by an external entity and validated by hsmtool for all signature types or better yet by rom_ext when verifying owner's image signature.

I've added another commit that adds tests for opentitantool and hsmtool. Opentitantool uses files on disk and hsmtool uses softhsm2 as the HSM for these tests.

The tests create a signature with one tool and verify it with the other. Interoperability between the tool-generated signatures and the firmware is tested by the //sw/device/silicon_creator/rom_ext/e2e/verified_boot:keys test suite and by //sw/device/silicon_creator/rom/e2e/sigverify_spx/....

@cfrantz cfrantz force-pushed the fix-tool-endian branch 6 times, most recently from 8b11569 to b3658a9 Compare January 23, 2025 16:45
@cfrantz cfrantz marked this pull request as ready for review January 23, 2025 23:21
@cfrantz cfrantz requested a review from a team as a code owner January 23, 2025 23:21
@cfrantz cfrantz requested review from jon-flatley and removed request for a team January 23, 2025 23:21
Opentitanlib was computing the SHA256 hash and storing it in a fixed-sized
bigint.  This led to various problems dealing with the endianness of the
resulting digest, including emitting digest in "little-endian" order.
A SHA256 digest _is not_ an integer; it is a sequence of bytes.

1. Eliminate the bigint representation of the SHA256 digest.  Give the
   digest struct serialization, display and parsing traits to eliminate
   the need for dealing with conversions for those ues cases.  Add
   `to_vec` and `to_vec_rev` to produce the raw bytes when needed.
2. Add a `--spx-hash-reversal-bug` switch to opentitantool spx commands
   for cases where firmware erroneously uses a reversed digest.
3. Eliminate the copious digest reversals present in hsmtool.  Add a
   `Sha256HashReversed` input format for cases where
   firmware erroneously uses a reversed digest.
4. Fix the signing rules to use the reversed format only for keys
   tied to a firmware implementation with the reversal bug.

Signed-off-by: Chris Frantz <cfrantz@google.com>
1. Add tests that check that signatures generated with opentitantool can
   be verified by hsmtool.
2. Add tests that check that signatures generated with hsmtool can
   be verified by opentitantool.
3. Verify the correctness of the manifest digest calculcated by
   opentitantool.

Signed-off-by: Chris Frantz <cfrantz@google.com>
@cfrantz
Copy link
Contributor Author

cfrantz commented Jan 24, 2025

I also manually tested the offline signing flow and the CloudKMS signing flow.

@cfrantz cfrantz merged commit 12467bb into lowRISC:earlgrey_1.0.0 Jan 24, 2025
32 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.

3 participants