Skip to content

Read endorsement VNB/VNA at the start of the signed structure - #4

Open
erkki wants to merge 1 commit into
Fahdonn:drip-hardeningfrom
bear-metal:fix/endorsement-structure-parse
Open

Read endorsement VNB/VNA at the start of the signed structure#4
erkki wants to merge 1 commit into
Fahdonn:drip-hardeningfrom
bear-metal:fix/endorsement-structure-parse

Conversation

@erkki

@erkki erkki commented Sep 1, 2026

Copy link
Copy Markdown

The UA-Signed Evidence Structure (RFC 9575 §4.1) is
[VNB (4)][VNA (4)][Evidence][UA DET (16)][UA Signature (64)].

handleEndorsementStructure advanced the buffer position past the signed content
before calling getInt(), so VNB/VNA were decoded from the first 8 signature bytes:
verifyTimestamps evaluated random values and rejected nearly every Wrapper/Manifest;
when the garbage happened to pass, evidenceSize went negative and Buffer.limit()
threw IllegalArgumentException on the scanner thread. Additionally both
handleWrapper/handleManifest received the whole signed structure instead of the
Evidence field: the wrapper % 25 check ran over VNB/VNA+DET, and the manifest
consumed VNB/VNA as the previous/current manifest hashes.

This is a regression from 1e08f74 ("DET verification"): the initial-commit
version read VNB/VNA in place and passed the Evidence slice to the handlers; that
refactor added the premature position() advance (invalidating the
evidenceSize computation written for the old cursor position) and swapped the
handler arguments from evidence to entire_message.

Change: read VNB/VNA in place, derive evidenceSize from messageSize, reject
structures shorter than VNB+VNA+DET, pass the Evidence slice to the handlers.
Device-verified: manifests pass timestamp, DET, and Ed25519 signature checks.

Tests: EndorsementStructureTest builds Manifest SAMs signed with a deterministic
Ed25519 test key and feeds them through handleSAMMessage. Against the previous
parsing, the positive cases fail (VNB/VNA decoded from signature bytes reject the
structure) and the tampered-signature case reproduces the Buffer.limit(-24)
crash. Also covers expired VNA, tampered signature, and truncated-structure
rejection. Test dependencies: androidx.arch.core:core-testing (synchronous
LiveData) and org.mockito:mockito-coreandroid.util.Log is statically mocked
per test class instead of enabling the module-global
unitTests.returnDefaultValues, which would silently default every android.jar
stub. The mockito line is identical in #4 and merges cleanly.

The UA-Signed Evidence Structure (RFC 9575 section 4.1) is
[VNB (4)] [VNA (4)] [Evidence] [UA DET (16)] [UA Signature (64)].

handleEndorsementStructure advanced the buffer position past the signed
content before calling getInt(), so VNB/VNA were decoded from the first
8 signature bytes. verifyTimestamps then evaluated random values and
rejected nearly every Wrapper/Manifest; when the garbage happened to
pass, evidenceSize went negative (remaining - SIGNATURE_SIZE - DET_SIZE
with only the signature remaining) and Buffer.limit() threw
IllegalArgumentException on the scanner thread.

Read VNB/VNA in place, derive evidenceSize from messageSize, and reject
structures shorter than VNB+VNA+DET. Pass only the Evidence field to
handleWrapper/handleManifest: both previously received the whole signed
structure, so the wrapper %25 check ran over VNB/VNA+DET too and the
manifest consumed VNB/VNA as the previous/current manifest hashes.

EndorsementStructureTest feeds synthetic Ed25519-signed Manifest SAMs
through handleSAMMessage. Against the previous parsing, the positive
cases fail (VNB/VNA decoded from signature bytes reject the structure)
and the tampered-signature case reproduces the Buffer.limit(-24) crash.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant