Match DSS token id format so untrusted-chain reports survive leading zero bytes - #466
Merged
Conversation
…zero bytes DSS renders a certificate token id through BigInteger (Digest#getHexValue), which drops leading zero bytes and pads back only an odd nibble count. A certificate whose DER SHA-256 starts with 0x00 is therefore reported as a 62-character id, not the usual 64. Both sides of the issue #448 enrichment assumed a fixed 64: - DssUntrustedChainReporter's TOKEN_ID pattern did not match the shorter id, so no token id was collected and describe() returned an empty block. - CertificateInfo.tokenId() zero-padded via HexFormat, so even a matched id would not have been found in the signer / timestamp chain index. For roughly one certificate in 256 the enrichment silently degraded to the bare C-<fingerprint> message it was written to replace, and the id= field of the FINE certificate log did not cross-reference DSS's own output. Both regression tests mine a certificate with a leading zero digest byte by reusing one key pair and varying only the serial number, which turns a 1/256 CI flake (it broke the snapshot deployment of 0ae3d85) into a deterministic check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The snapshot deployment for
0ae3d85failed:The reactor stopped at
jsignpdf-engine-dss, so nothing was published. The run before it, and the run after, both passed on the same code — this is a 1-in-256 flake with a real bug behind it.Root cause
The alert DSS raised in that run carried this token id:
That is 62 hex characters, not 64.
Identifier.asXmlId()is"C-" + Digest.getHexValue(), andgetHexValue()is:Leading zero bytes are dropped outright; the odd-length pad only restores a leading zero nibble. So a certificate whose DER SHA-256 begins with
0x00gets a 62-character id — probability 1/256, andEmbeddedCagenerates fresh key pairs on every run.Both sides of the issue #448 enrichment assumed a fixed 64:
DssUntrustedChainReporter.TOKEN_IDwasC-[0-9A-Fa-f]{64}, which did not match the shorter id, socollectTokenIds()came back empty anddescribe()returned"".CertificateInfo.tokenId()zero-padded throughHexFormat, so even a matched id would have missed in the signer / timestamp chain lookup.This is not test-only. For roughly one certificate in 256 the enrichment silently degraded back to the bare
C-<fingerprint>message it was written to replace, and theid=field in the FINE certificate log did not cross-reference DSS's own output.Fix
CertificateInfo.tokenId()now replicatesDigest#getHexValue()exactly.TOKEN_IDaccepts{2,64}hex characters.Tests
Both new tests mine a certificate whose DER SHA-256 starts with
0x00— cheaply, by reusing one key pair and varying only the serial number (~2s), turning the flake into a deterministic check:CertificateInfoTest.tokenIdDropsLeadingZeroBytesLikeDssDssUntrustedChainReporterTest.namesCertificateWhoseTokenIdHasALeadingZeroByte— end-to-end throughdescribe(), with DSS's ownCertificateToken.getDSSIdAsString()as the authoritative idtokenIdIsDssCertificateTokenIdhad the old zero-padded form baked into its expected value and was updated.Verified as real guards: with the two main-source changes reverted and the tests kept, both fail — the second with the exact CI symptom (
must name the signer role). With the fix, the fullmvn testreactor is green.No release-note entry: #448 and #452 are both unreleased in 3.2.0, so this corrects unshipped work.
🤖 Generated with Claude Code