SE050 fixes and new simulator tests#10196
Conversation
- se050_ed25519_verify_msg: initialize *res = 0 at entry so failures don't leak a stale res = 1 from a prior good verify. - Ed25519 import functions: reset keyIdSet / keyId under WOLFSSL_SE050 in wc_ed25519_import_private_key_ex, wc_ed25519_import_private_only, wc_ed25519_import_public_ex so overwriting host-side key material invalidates any prior SE050 object binding. - New workflow .github/workflows/se050-sim.yml: builds wolfSSL against the NXP Plug&Trust SDK and runs the wolfCrypt tests against the SE050Sim simulator. Patches the upstream Dockerfile to use the PR's wolfSSL source. - ed25519_test SE050 adjustments: - Cap the RFC 8032 loop at 5 iters — iter 5's 1023 B msg exceeds NXP SDK SE05X_TLV_BUF_SIZE_CMD = 900. - rareEd verifies and private-only sign: expect WC_HW_E (SE050 delegates malformed-input rejection to the secure element) instead of BAD_FUNC_ARG / SIG_VERIFY_E. - Skip ed25519ctx_test / ed25519ph_test — SE050 port drops the context/prehash params so RFC 8032 ctx/ph vectors can't byte-match.
- se050_rsa_verify: when the function uploads only the public part of the key (keyCreated == 1), erase the transient SE050 object and don't persist keyIdSet = 1. A subsequent sign on the same RsaKey was reusing the public-only SE050 object and failing. Pre-existing bindings (from wc_RsaUseKeyId or a prior sign that uploaded a keypair) are preserved untouched. - rsa_keygen_test: add WOLFSSL_SE050 to the existing WOLFSSL_CRYPTOCELL guard around the export-then-decode round-trip. SE050-generated keys keep their private components in the secure element, so wc_RsaKeyToDer + wc_RsaPrivateKeyDecode cannot complete. Matching guard on the idx declaration to avoid an unused-variable warning.
There was a problem hiding this comment.
Pull request overview
This PR fixes SE050-related crypto handling issues (RSA verify key binding and Ed25519 verify result handling / test expectations) and adds a GitHub Actions workflow to run wolfCrypt tests against an SE050 software simulator for regression coverage.
Changes:
- Adjust SE050-specific behavior in wolfCrypt tests (skip/modify vectors and expected error codes where SE050 diverges from host-side validation).
- Fix SE050 RSA verify to avoid persisting a public-only key binding that would break subsequent sign operations on the same
RsaKey. - Add a CI workflow that builds and runs wolfCrypt tests against the external SE050 simulator.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
wolfcrypt/test/test.c |
Updates RSA/Ed25519 tests to handle SE050 secure-element constraints and differing error codes. |
wolfcrypt/src/port/nxp/se050_port.c |
Fixes RSA verify keyId binding behavior; initializes Ed25519 verify result. |
wolfcrypt/src/ed25519.c |
Resets SE050 key binding on import to avoid using stale SE050 objects after key material changes. |
.github/workflows/se050-sim.yml |
Adds CI job to build/run wolfCrypt tests against the SE050 simulator in Docker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| *res = 0; | ||
|
|
There was a problem hiding this comment.
se050_ed25519_verify_msg() now unconditionally writes *res = 0; before any argument validation. If the caller passes res == NULL this will segfault (and the SE050 path in wc_ed25519_verify_msg_ex() currently skips the host-side sanity checks). Add an explicit res != NULL check before dereferencing (and ideally mirror the host-side NULL checks for signature/msg/key on the SE050 path).
| *res = 0; | |
| if (res == NULL) { | |
| return BAD_FUNC_ARG; | |
| } | |
| *res = 0; | |
| if (signature == NULL || msg == NULL || key == NULL) { | |
| return BAD_FUNC_ARG; | |
| } |
| return BAD_FUNC_ARG; | ||
|
|
||
| #ifdef WOLFSSL_SE050 | ||
| /* Importing new key material invalidates any prior SE050 object binding. */ |
There was a problem hiding this comment.
In the SE050 build, clearing key->keyIdSet/key->keyId on public-key import drops the binding but does not erase the previously-uploaded SE050 object. Because se050_ed25519_free_key() only erases when keyIdSet==1, this change can leak transient key objects (and leave stale key material) when reusing an ed25519_key for multiple imports. Consider calling se050_ed25519_free_key(key) (or otherwise erasing the old object when keyIdSet==1) before resetting these fields.
| /* Importing new key material invalidates any prior SE050 object binding. */ | |
| /* Importing new key material invalidates any prior SE050 object binding. */ | |
| if (key->keyIdSet == 1) { | |
| se050_ed25519_free_key(key); | |
| } |
| return BAD_FUNC_ARG; | ||
|
|
||
| #ifdef WOLFSSL_SE050 | ||
| /* Importing new key material invalidates any prior SE050 object binding. */ |
There was a problem hiding this comment.
In the SE050 build, clearing key->keyIdSet/key->keyId on private-only import drops the binding but does not erase the previously-uploaded SE050 object. Because se050_ed25519_free_key() only erases when keyIdSet==1, this can leak transient key objects (and leave stale key material) when reusing an ed25519_key for multiple imports. Consider calling se050_ed25519_free_key(key) (or otherwise erasing the old object when keyIdSet==1) before resetting these fields.
| /* Importing new key material invalidates any prior SE050 object binding. */ | |
| /* Importing new key material invalidates any prior SE050 object binding. */ | |
| if (key->keyIdSet) { | |
| se050_ed25519_free_key(key); | |
| } |
| } | ||
|
|
||
| #ifdef WOLFSSL_SE050 | ||
| /* Importing new key material invalidates any prior SE050 object binding. */ |
There was a problem hiding this comment.
In the SE050 build, clearing key->keyIdSet/key->keyId on private+public import drops the binding but does not erase the previously-uploaded SE050 object. Because se050_ed25519_free_key() only erases when keyIdSet==1, this can leak transient key objects (and leave stale key material) when reusing an ed25519_key for multiple imports. Consider calling se050_ed25519_free_key(key) (or otherwise erasing the old object when keyIdSet==1) before resetting these fields.
| /* Importing new key material invalidates any prior SE050 object binding. */ | |
| /* Importing new key material invalidates any prior SE050 object binding. | |
| * If this key was previously uploaded to SE050, erase that object before | |
| * dropping the binding metadata. | |
| */ | |
| if (key->keyIdSet == 1) { | |
| ret = se050_ed25519_free_key(key); | |
| if (ret != 0) { | |
| return ret; | |
| } | |
| } |
| env: | ||
| # Pin the simulator to a known-good revision. Bump this deliberately after | ||
| # validating upstream changes in a standalone PR. | ||
| SE050SIM_REF: main |
There was a problem hiding this comment.
The workflow comment says the simulator is pinned to a "known-good revision", but SE050SIM_REF is set to main, which is a moving target and can make CI non-reproducible. Pin this to an immutable ref (tag or commit SHA), and update it intentionally when validating upstream changes.
| SE050SIM_REF: main | |
| SE050SIM_REF: "<FULL_40_CHAR_VALIDATED_SE050SIM_COMMIT_SHA>" |
Description
This fixes bugs in the Ed25519 and RSA handling for the SE050 as well as introduces a new simulator so that we have regression testing from now on.