Skip to content

Validate 8-char lowercase hex key hashes across note constructors - #295

Open
mhutchinson wants to merge 1 commit into
transparency-dev:mainfrom
mhutchinson:note-keyhash
Open

mhutchinson wants to merge 1 commit into
transparency-dev:mainfrom
mhutchinson:note-keyhash

Conversation

@mhutchinson

Copy link
Copy Markdown
Contributor

Several signer and verifier constructors previously checked only that the
key ID was 8 characters long, quietly ignoring its value and recomputing
the hash from the key material — allowing corrupted, spliced, or non-hex
vkeys (including two in our own tests) to pass unnoticed while failing in
stricter C2SP implementations.

Enforce C2SP signed-note key ID rules across all signer and verifier
constructors in formats/note:

  • Require key IDs (hash16) to be 8 lowercase hexadecimal digits.
  • Validate that the parsed key ID matches the computed 32-bit key hash
    in NewMLDSAVerifier, NewSignerForCosignatureV1,
    NewVerifierForCosignatureV1, and NewRFC6962Verifier.

Several signer and verifier constructors previously checked only that the
key ID was 8 characters long, quietly ignoring its value and recomputing
the hash from the key material — allowing corrupted, spliced, or non-hex
vkeys (including two in our own tests) to pass unnoticed while failing in
stricter C2SP implementations.

Enforce C2SP signed-note key ID rules across all signer and verifier
constructors in formats/note:
- Require key IDs (hash16) to be 8 lowercase hexadecimal digits.
- Validate that the parsed key ID matches the computed 32-bit key hash
  in NewMLDSAVerifier, NewSignerForCosignatureV1,
  NewVerifierForCosignatureV1, and NewRFC6962Verifier.
@mhutchinson
mhutchinson requested a review from a team as a code owner November 5, 2026 09:36
@mhutchinson
mhutchinson requested a review from phbnf November 5, 2026 09:36
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.44%. Comparing base (fa00c16) to head (44b7fac).
⚠️ Report is 203 commits behind head on main.

Files with missing lines Patch % Lines
note/note.go 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #295      +/-   ##
==========================================
+ Coverage   82.57%   91.44%   +8.87%     
==========================================
  Files           5       10       +5     
  Lines         241     1087     +846     
==========================================
+ Hits          199      994     +795     
- Misses         30       93      +63     
+ Partials       12        0      -12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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