fix(secure-boot): sign the whole image header, not just the payload hash - #40
Merged
srpatcha merged 3 commits intoAug 27, 2026
Merged
Conversation
…ead flash eos_crc32() returned 0 when eos_hal_flash_read() failed. That value is indistinguishable from a region that genuinely hashes to 0, so the CRC32 branch of eos_image_verify_integrity() reported EOS_OK for an image whose payload could not be read at all — provided the stored CRC was 0. The stored CRC is the first four bytes of hdr->hash, which lives in the image header. The header is not covered by the Ed25519 signature (that is computed over hdr->hash only), so setting those bytes to 0 costs nothing. The SHA-256 branch has always propagated the read error: eos_crypto_verify_image() returns rc from eos_hal_flash_read(). The two branches now behave the same. Reproduced against the simulated flash in tests/unit/test_image_verify.c by placing an image so its payload runs past the end of the device: flash read of payload tail -> -6 (EOS_ERR_FLASH) eos_image_verify_integrity -> 0 (EOS_OK) <-- verified an unreadable image Adds eos_crc32_checked(), which reports read failures through its return value, and switches the verification path to it. eos_crc32() is kept for API compatibility, now implemented in terms of the checked variant and documented as unsuitable for deciding whether an image is intact. Two smaller fixes in the same function, both the same "do not verify something you did not read" shape: a zero image_size is rejected rather than CRC'd to a fixed value, and an addr + hdr_size that wraps uint32_t is rejected rather than producing a payload address that is not the payload. Tests: seven cases added to tests/unit/test_image_verify.c (12 total, was 5), including a matching-CRC image that must verify, a wrong-CRC image that must not, and the unreadable-payload regression. The CRC32 used to build expectations is implemented independently in the test so it does not simply restate image_verify.c. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eos_image_verify_signature() verified the Ed25519 signature over hdr->hash —
32 of the header's 156 bytes. Every other field sat outside the signature:
image_size, load_addr, entry_addr, image_version, flags, sig_type and sig_len.
An attacker holding any legitimately signed image could therefore keep the
signature intact and still:
- move entry_addr, redirecting execution within the image;
- change load_addr, relocating where the image is copied to;
- clear EOS_IMG_FLAG_HASH_SHA256, which downgrades
eos_image_verify_integrity() from SHA-256 to CRC32 — and CRC32 is
trivially forgeable, so the payload then becomes editable too.
The payload hash is unchanged in all three cases, so the signature still
verified. Demonstrated with real Ed25519 keys: a header with entry_addr moved
from 0x08010100 to 0x08099999, load_addr to 0x20000000 and the SHA-256 flag
cleared is ACCEPTED under the old scheme and REJECTED under the new one.
The signature now covers EOS_IMG_SIGNED_LEN bytes — the whole header except
signature[] itself. hash[] is inside that prefix, so the payload stays covered
transitively. This is a format change: EOS_IMAGE_HDR_VERSION goes to 2 and
existing signed images must be re-signed. No v1 compatibility path is offered,
because accepting v1 signatures would leave the downgrade open.
eos_image_parse_header() now validates hdr_version instead of ignoring it.
The signing tools move in lockstep:
- sign_image.py signs the header prefix, sets sig_type/sig_len before
signing (they are inside the signed region), and stamps hdr_version = 2.
- --verify was declared in argparse but never used by main(); it is now
implemented, checking the payload hash and the signature over the prefix.
The security review checklist already assumed this flag worked.
- eos_sign.py had constants that did not match include/eos_types.h:
SIG_TYPE_ED25519 was 1 (that is EOS_SIG_CRC32, which the bootloader
rejects outright) and IMG_FLAG_SIGNED was 1 << 2 (EOS_IMG_FLAG_DEBUG). It
also never set EOS_IMG_FLAG_HASH_SHA256, so the bootloader read its stored
SHA-256 as a CRC32.
Struct offsets are now static-asserted, because the signing tools address the
header by absolute byte offset and a silent layout change would break signing
without breaking the build.
Tests: 4 C cases pinning that every trusted field lies inside the signed
prefix and that unsigned signature types are rejected (16 total, was 12), plus
tests/unit/test_sign_image.py — 14 end-to-end cases that pack, sign and verify
a real image, then tamper each header field and the payload and require every
one to be rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kartikey1306
requested review from
hshanmug12,
maheshmunnangi and
srpatcha
as code owners
August 26, 2026 06:37
srpatcha
previously approved these changes
Aug 27, 2026
6 tasks
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.
Summary
eos_image_verify_signature()verified the Ed25519 signature overhdr->hash— 32 of the header's 156 bytes. Every other field sat outside the
signature:
image_size,load_addr,entry_addr,image_version,flags,sig_type,sig_len.An attacker holding any legitimately signed image could keep that signature
byte-for-byte and still rewrite the metadata the bootloader acts on.
The bug
Because the payload hash is unchanged by header edits, the signature keeps
verifying. Three concrete consequences:
entry_addrload_addrEOS_IMG_FLAG_HASH_SHA256eos_image_verify_integrity()drops from SHA-256 to CRC32The third is the worst: CRC32 is trivially forgeable, so once integrity
checking is downgraded the payload itself becomes editable. The flag that
selects the algorithm was not covered by the signature that was supposed to
protect it.
Demonstrated with real Ed25519 keys — one image, signed once, then tampered:
Approach
Sign the header prefix — everything except
signature[]itself:hash[]is inside that prefix, so the payload stays covered transitively — onesignature now binds the metadata and the payload together.
No crypto changes were needed:
eos_crypto_verify_signature()already passesthe message straight through to
eos_ed25519_verify(), which takes anarbitrary length.
Ordering detail:
sig_typeandsig_lenare themselves inside the signedregion, so the signer must finalise them before computing the signature. That
works because Ed25519 signatures are always 64 bytes, so
sig_lenis known upfront.
Breaking change, deliberately
EOS_IMAGE_HDR_VERSIONgoes to 2. Existing signed images must bere-signed; their v1 signatures do not verify under v2.
I did not add a v1 compatibility path. Accepting v1 signatures would leave
the entire vulnerability open — an attacker would simply present a v1 image.
A clean break is the only version of this fix that actually fixes it.
eos_image_parse_header()now validateshdr_version(it previously ignoredit), rejecting
0and anything newer than the build understands. v1 stillparses — it is a real format — but fails at signature verification.
Tooling moved in lockstep
A verifier change without a matching signer just makes every image unbootable.
sign_image.pysigns the header prefix, setssig_type/sig_lenbeforesigning, and stamps
hdr_version = 2.--verifyis now implemented. It was declared inargparsebutmain()never read it — the flag silently did nothing, while
docs/security_review_checklist.mdalready says signing changes should be"tested with
--verifyflag against known-good test vectors". It nowrecomputes the payload hash and checks the signature over the prefix.
eos_sign.pyhad constants that did not matchinclude/eos_types.h:SIG_TYPE_ED25519 = 1— that value isEOS_SIG_CRC32, whicheos_image_verify_signature()rejects outright — andIMG_FLAG_SIGNED = 1 << 2, which isEOS_IMG_FLAG_DEBUG. It also never setEOS_IMG_FLAG_HASH_SHA256, so the bootloader read its stored SHA-256 as aCRC32. Its images could never have booted.
Layout is now static-asserted
The signing tools address the header by absolute byte offset, so the struct
layout is part of the on-disk format:
Without these, reordering a field would silently break signing without breaking
the build. The macro degrades gracefully on pre-C11 and works under C++.
Testing
C — 16 tests, was 12. Four new, all passing:
test_signed_region_covers_all_metadatasignature[]is the only thing outsidetest_flags_are_inside_the_signed_regiontest_unsigned_signature_types_are_rejectedsig_typeNONE/CRC32/SHA256 and wrongsig_lennever satisfy the checktest_header_version_is_validatedThe coverage test fails if anyone adds a field after
signature[]— which isexactly the mistake that created this bug.
Python —
tests/unit/test_sign_image.py, 14 new end-to-end tests. Thesepack a real image, sign it, verify it, then tamper and require rejection:
Covered:
entry_addr,load_addr,image_size,image_version,sig_type,hdr_version, the SHA-256 flag, the payload, and verification under the wrongkey. One test checks the signed message independently of the tool that produced
it — it verifies the signature over bytes
[0, 92)withcryptographydirectly, and asserts it does not verify over
hash[]alone.Writing that suite caught a bug in my own
--verify: it reported success foran image whose
sig_typehad been downgraded to CRC32, because it fell back to"integrity only". Passing
--keynow requires a valid signature, matchingwhat the bootloader does.
Build:
With #39 merged in locally, the combined tree is BUILD=0, 14/14.
A finding I am reporting, not fixing
eos_ed25519_verify()is not RFC 8032 Ed25519. It computesk = SHA-256(R ‖ A ‖ M)where the standard uses SHA-512(
core/ed25519_verify.c:696). The file says so itself:sign_image.pyusescryptography's standard Ed25519, which is SHA-512. Thestated requirement is not met, so no signature produced by the tool can
verify on-device.
tests/unit/test_ed25519.cdoes not catch it because everyone of its assertions is negative — NULL arguments, malformed signatures, zero
keys. Not one test asserts that a valid signature verifies.
This is orthogonal to what is signed, which is what this PR fixes, and
closing it means adding SHA-512 and reworking the reduction — a substantial
change to the most security-critical file in the tree. It deserves its own PR
with RFC 8032 test vectors. I would rather flag it precisely than bolt it on
here. Happy to open the issue, or that PR, if useful.
Note this does not weaken the change: with the header unsigned, a working
verifier would have been worse, because tampered images would have booted.
Limitations and considerations
the intended, unavoidable consequence — see Breaking change above.
eos_sign.pystill emits[header][TLV][payload], but the bootloadercomputes the payload address as
addr + hdr_sizeand there is no caller ofeos_tlv_parse()anywhere in the tree. Images from that tool still failintegrity checking because the TLV sits where the payload is expected. I
fixed its constants and signing so it does not drift further, and documented
the remaining gap at the top of the file — resolving it is a format decision
(move the TLV after the payload, or teach the boot path to parse it), not a
tooling fix.
EBLDR_REQUIRE_SIGNATURES=OFFreopens the downgrade. With signatureverification disabled,
flagsis unauthenticated again by construction. Thedefault is
ON.docs/quickstart.mddocumented commands that do not exist(
--generate-key,--key/--input/--output). Corrected to the real CLI whiledocumenting the new
--verifystep.core/image_verify.candCHANGELOG.mdaremixed CRLF/LF and
sign_image.pyis CRLF; edits were applied byte-wise, soimage_verify.cis +21 lines rather than a whole-file rewrite.Type of Change
Changes
core/image_verify.c— signature covers the header prefix;hdr_versionvalidated.include/eos_image.h—EOS_IMG_SIGNED_LEN, version 2, layout static asserts.tools/sign_image.py— sign the prefix; implement--verify.tools/eos_sign.py— correct constants; sign the prefix; document the TLV gap.tests/unit/test_image_verify.c— 4 new cases (16 total).tests/unit/test_sign_image.py— new, 14 end-to-end tamper tests.docs/quickstart.md,docs/secure_boot_chain.md,docs/security_review_checklist.md,docs/threat_model.md(new row T-303),CHANGELOG.md.Pre-Submission Checklist