add wolfBoot image verification to image manager#339
add wolfBoot image verification to image manager#339bigbrett wants to merge 3 commits intowolfSSL:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Extends the server image manager to support verifying wolfBoot-format images (manifest TLV header carrying hash/signature and optional cert chain) in addition to existing “raw blob + NVM signature” verification.
Changes:
- Adds wolfBoot image type support and new wolfBoot verification method APIs (RSA4096+SHA256, with and without cert-chain validation).
- Updates server image-manager verification flow to load key/signature material based on image type (RAW vs WOLFBOOT vs WOLFBOOT_CERT).
- Adds test coverage for the two new verification methods and a script to generate offline wolfBoot test vectors.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
wolfhsm/wh_server_img_mgr.h |
Adds wolfBoot constants, new image type enum, expands image descriptor struct, declares new verify methods. |
src/wh_server_img_mgr.c |
Implements wolfBoot TLV parsing + hashing + RSA4096 verification; adds cert-chain verification path using server cert API; updates per-image key/sig loading logic. |
test/wh_test_server_img_mgr.c |
Adds positive/negative tests for wolfBoot RSA4096 verification and cert-chain verification. |
test/scripts/gen_wolfboot_test_data.sh |
Adds a generator script intended to produce the wolfBoot test artifact header used by the new tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #339
Scan targets checked: wolfhsm-consttime, wolfhsm-core-bugs, wolfhsm-defaults, wolfhsm-mutation, wolfhsm-proptest, wolfhsm-src, wolfhsm-zeroize
Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
bigbrett
force-pushed
the
img-mgr-wolfboot
branch
from
d4e7a9e to
dfb7ec0
Compare
bigbrett
force-pushed
the
img-mgr-wolfboot
branch
from
dfb7ec0 to
02953a7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static int _wolfBootImgHashSha256(const uint8_t* hdr, | ||
| const uint8_t* stored_sha_ptr, | ||
| const uint8_t* img, uint32_t img_size, | ||
| uint8_t* hash_out) | ||
| { | ||
| wc_Sha256 sha256_ctx; | ||
| const uint8_t* end_sha; | ||
| int ret; | ||
| const size_t tlv_sz = 4; | ||
|
|
||
| /* Hash the header from byte 0 up to the hash TLV's type+len fields | ||
| * (i.e., exclude the TLV header: 2 bytes type + 2 bytes len = 4 bytes) */ | ||
| end_sha = stored_sha_ptr - tlv_sz; | ||
|
|
||
| ret = wc_InitSha256(&sha256_ctx); | ||
| if (ret != 0) { | ||
| return WH_ERROR_ABORTED; | ||
| } | ||
|
|
||
| /* Hash header portion */ | ||
| ret = wc_Sha256Update(&sha256_ctx, hdr, (word32)(end_sha - hdr)); | ||
| if (ret != 0) { |
There was a problem hiding this comment.
In the wolfBoot hashing path you're using wc_InitSha256()/wc_Sha256Update()/wc_Sha256Final without a devId. Other image-manager verify methods in this file use the *_ex variants (e.g., wc_Sha256Hash_ex(..., context->server->devId)) to ensure hashing routes through the configured wolfCrypt device/crypto-callback. Consider switching to wc_InitSha256_ex (or otherwise threading server->devId into this helper) so wolfBoot verification behaves consistently on builds that rely on devId-backed crypto/hardware acceleration.
| static int _wolfBootImgVerifyPubKeyHint(const uint8_t* pubkey, | ||
| uint32_t pubkeySz, const uint8_t* hint, | ||
| uint16_t hintSz) | ||
| { | ||
| wc_Sha256 sha256_ctx; | ||
| uint8_t key_hash[WC_SHA256_DIGEST_SIZE]; | ||
| int ret; | ||
|
|
||
| if (hintSz != WC_SHA256_DIGEST_SIZE) { | ||
| return WH_ERROR_NOTVERIFIED; | ||
| } | ||
|
|
||
| ret = wc_InitSha256(&sha256_ctx); | ||
| if (ret != 0) { | ||
| return WH_ERROR_ABORTED; | ||
| } | ||
|
|
||
| ret = wc_Sha256Update(&sha256_ctx, pubkey, pubkeySz); | ||
| if (ret != 0) { | ||
| wc_Sha256Free(&sha256_ctx); | ||
| return WH_ERROR_ABORTED; | ||
| } | ||
|
|
||
| ret = wc_Sha256Final(&sha256_ctx, key_hash); | ||
| wc_Sha256Free(&sha256_ctx); |
There was a problem hiding this comment.
_wolfBootImgVerifyPubKeyHint() also uses wc_InitSha256() without a devId, which can bypass the configured crypto device/crypto-callback path. To match the rest of the server-side crypto usage, initialize the SHA context with wc_InitSha256_ex(..., context->server->devId) (or pass a devId into this helper) so the pubkey-hint check works in devId-dependent builds.
bigbrett
force-pushed
the
img-mgr-wolfboot
branch
from
02953a7 to
dcd7772
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const uint8_t* p = hdr + WH_IMG_MGR_WOLFBOOT_HDR_OFFSET; | ||
| const uint8_t* max_p = hdr + hdrSize; | ||
| uint16_t htype; | ||
| uint16_t len; | ||
|
|
||
| *ptr = NULL; | ||
|
|
||
| if (p > max_p) { | ||
| return 0; | ||
| } | ||
|
|
||
| while ((p + 4) < max_p) { | ||
| htype = (uint16_t)((uint16_t)p[0] | ((uint16_t)p[1] << 8)); | ||
| if (htype == 0) { |
There was a problem hiding this comment.
_wolfBootImgFindHeaderField uses while ((p + 4) < max_p), but if hdrSize is <= WH_IMG_MGR_WOLFBOOT_HDR_OFFSET + 3 then p can be at (or one-past) the end of the header and computing p + 4 is undefined behavior in C (even before dereferencing). Add an early size check (e.g., require at least 4 bytes of TLV header after the offset) or rewrite the loop condition using integer remaining-length (max_p - p) instead of pointer addition.
Add wolfBoot image verification to server image manager
Extends the server image manager to verify wolfBoot-format images in addition to opaque blobs. wolfBoot images carry their signature (and optionally a certificate chain) in a TLV-formatted manifest header, so the manager now knows how to parse the header and source verification material from it rather than from fixed NVM IDs.
What's new
whServerImgMgrImgTypeenum distinguishes image loading behavior:RAW(existing — key from keystore, sig from NVM),WOLFBOOT(key from keystore, sig from header), andWOLFBOOT_CERT(root CA from NVM, cert chain + sig from header).whServerImgMgrImggainshdrAddr/hdrSize/imgTypefields;sigNvmIdis repurposed as the root CA NVM ID in the cert-chain case.wh_Server_ImgMgrVerifyMethodWolfBootRsa4096WithSha256— standard wolfBoot verification against a keystore-provisioned public key.wh_Server_ImgMgrVerifyMethodWolfBootCertChainRsa4096WithSha256— verifies the embedded cert chain against an HSM-provisioned root CA using the server cert API, then uses the leaf public key to verify the image signature.test/gen/wh_test_wolfboot_img_data.h) produced offline bytest/scripts/gen_wolfboot_test_data.sh.Future work
Only RSA4096+SHA256 is supported in this PR. Follow-ups will add additional wolfBoot-supported verification mechanisms (ECC, ed25519, etc.), reusing the same header-parsing and cert-chain plumbing.