Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds first-class crypto callback (cryptoCb) support to wolfBoot by introducing configurable per-class wolfCrypt devIds and a simulator-based CI test that verifies crypto dispatch through cryptoCb.
Changes:
- Introduces
WOLFBOOT_DEVID_{HASH,PUBKEY,CRYPT}macros (defaultINVALID_DEVID) and updates crypto init paths to use them. - Adds simulator cryptoCb callback + “sunnyday update” verification script and a GitHub Actions workflow to exercise many algorithm/config combinations.
- Updates Renesas and wolfHSM build/config wiring to set devIds using the new abstraction.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/scripts/sim-cryptocb-sunnyday-update.sh | New script to run simulator update flow and grep cryptoCb dispatch in logs |
| src/libwolfboot.c | Routes AES init and device registration through WOLFBOOT_DEVID_CRYPT |
| src/image.c | Routes PK + hash init through WOLFBOOT_DEVID_* devId macros |
| options.mk | Adds sim cryptoCb test toggles + wires wolfHSM devId macros |
| include/wolfboot/wolfboot.h | Defines default per-class devId macros |
| include/user_settings.h | Renesas devId adjustments + forward declaration for WC_RNG when needed |
| hal/sim.c | Adds simulator cryptoCb implementation + registers callback + flush before exec |
| arch.mk | Renesas RX TSIP assigns pubkey/crypt devIds |
| IDE/Renesas/e2studio/RZN2L/user_settings.h | Sets WOLFBOOT_DEVID_{PUBKEY,CRYPT} for RSIP |
| IDE/Renesas/e2studio/RX72N/include/user_settings.h | Sets WOLFBOOT_DEVID_{PUBKEY,CRYPT} for TSIP |
| IDE/Renesas/e2studio/RA6M4/wolfBoot/user_settings.h | Sets RENESAS_DEVID and WOLFBOOT_DEVID_{PUBKEY,CRYPT} for SCE |
| .github/workflows/test-cryptocb-simulator.yml | New CI workflow to validate cryptoCb dispatch across many configs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Adds first-class support to wolfBoot for using crypto callbacks. The goal is to facilitate platform-specific hardware acceleration through cryptoCbs without requiring a ton of hard-coded/
#ifdefblocks, or at least minimizing core library intrusiveness.INVALID_DEVIDbut can be set inoptions.mk/arch.mkfor the platform.devIdsKnown limitation: wolfBoot currently uses a wolfCrypt ECC API that DOES NOT dispatch to crypto callbacks. IMO this is something that should be changed in wolfCrypt, or an additional "non-raw" verify method should be introduced to wolfBoot that can be opt-in. This is left for a future PR.
Note: Renesas integration is a bit janky given the dual IDE and build system integration. Would like to see it tested it on real hardware.