ECC wolfBoot image manager verify methods - #522
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It modifies security-critical boot-image verification logic (cryptographic verification and DMA-aware parsing), so it warrants focused human review despite tests and documentation updates.
Pull request overview
This PR adds wolfBoot image-verification support for ECC P-256 (ECC256) + SHA-256 so wolfHSM can verify the default wolfBoot signing method on tc4. It extends the server image manager with an ECC-specific wolfBoot verify method, updates documentation accordingly, and expands the test suite and test-data generation to cover ECC wolfBoot artifacts.
Changes:
- Add
wh_Server_ImgMgrVerifyMethodWolfBootEcc256WithSha256()and the corresponding wolfBoot auth-type constant (ECC256). - Refactor shared wolfBoot header hashing/validation to support multiple auth types (RSA4096 and ECC256) and implement ECC signature verification.
- Update test data generation and add positive/negative tests for ECC wolfBoot images (including corrupt-signature negatives).
File summaries
| File | Description |
|---|---|
wolfhsm/wh_server_img_mgr.h |
Exposes ECC256+SHA256 wolfBoot verify method and auth-type constant. |
src/wh_server_img_mgr.c |
Implements ECC wolfBoot verification and generalizes wolfBoot header validation for multiple auth types. |
test/wh_test_server_img_mgr.c |
Adds ECC wolfBoot test coverage plus signature-corruption negative tests for wolfBoot variants. |
test-refactor/server/wh_test_server_img_mgr.c |
Ports the same ECC wolfBoot and corrupt-signature negative coverage into the refactored test harness. |
test/scripts/gen_wolfboot_test_data.sh |
Extends generated test vectors to include ECC P-256 key material and an ECC-signed wolfBoot image/header. |
docs/src/8-Integration.md |
Updates integration guidance to reflect wolfBoot image types and ECC method support. |
docs/src/5-Features.md |
Documents the new ECC wolfBoot verify method and clarifies method/type pairing behavior. |
test-refactor/README.md |
Updates translated-test documentation to list wolfBoot ECC256 coverage. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #522
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
4cc8898 to
9d7f6a4
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #522
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #522
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
LGTM besides the open comments from @padelsbach.
Adds support for wolfBoot images signed with ECC256+SHA256 to support the default verify method on tc4.
Note: AI review agents might complain about returning
ABORTEDvsNOTVERIFIEDas well as which wolfCrypt error codes are squashed and which are propagated through to the caller. IMO the current implementation is fine for ECC and I think unifying the various algorithms is scope for a future PR.