diff --git a/.github/workflows/se050-sim.yml b/.github/workflows/se050-sim.yml index 2e2c0cbb40..0f7cb3de49 100644 --- a/.github/workflows/se050-sim.yml +++ b/.github/workflows/se050-sim.yml @@ -20,8 +20,6 @@ concurrency: # We patch it to COPY the PR checkout instead so CI reflects the PR's source. env: - # Pin the simulator to a known-good revision. Bump this deliberately after - # validating upstream changes in a standalone PR. SE050SIM_REF: main jobs: @@ -48,7 +46,7 @@ jobs: working-directory: se050sim run: | sed -i 's|^RUN git clone --depth 1 https://github.com/wolfSSL/wolfssl.git /app/wolfssl$|COPY wolfssl /app/wolfssl|' Dockerfile.wolfcrypt - # Fail fast if the pattern drifted upstream — better a clear error + # Fail fast if the pattern drifted upstream -- better a clear error # than a CI run that silently tests master. grep -q '^COPY wolfssl /app/wolfssl$' Dockerfile.wolfcrypt ! grep -q 'git clone .*wolfssl\.git' Dockerfile.wolfcrypt diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index 6150242806..8a78f2d480 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -1170,9 +1170,15 @@ int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, return BAD_FUNC_ARG; #ifdef WOLFSSL_SE050 - /* Importing new key material invalidates any prior SE050 object binding. */ - key->keyIdSet = 0; + /* Importing new key material invalidates any prior SE050 object binding; + * erase the old object (no-op when keyIdSet == 0) so the host and the + * secure element agree on what's bound. Clear the binding fields + * explicitly afterwards so a stale keyId never survives, even when + * se050_ed25519_free_key() returns early because the SE050 session isn't + * configured yet. */ + se050_ed25519_free_key(key); key->keyId = 0; + key->keyIdSet = 0; #endif /* compressed prefix according to draft @@ -1262,9 +1268,15 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, return BAD_FUNC_ARG; #ifdef WOLFSSL_SE050 - /* Importing new key material invalidates any prior SE050 object binding. */ - key->keyIdSet = 0; + /* Importing new key material invalidates any prior SE050 object binding; + * erase the old object (no-op when keyIdSet == 0) so the host and the + * secure element agree on what's bound. Clear the binding fields + * explicitly afterwards so a stale keyId never survives, even when + * se050_ed25519_free_key() returns early because the SE050 session isn't + * configured yet. */ + se050_ed25519_free_key(key); key->keyId = 0; + key->keyIdSet = 0; #endif XMEMCPY(key->k, priv, ED25519_KEY_SIZE); @@ -1324,9 +1336,18 @@ int wc_ed25519_import_private_key_ex(const byte* priv, word32 privSz, } #ifdef WOLFSSL_SE050 - /* Importing new key material invalidates any prior SE050 object binding. */ - key->keyIdSet = 0; + /* Importing new key material invalidates any prior SE050 object binding; + * erase the old object (no-op when keyIdSet == 0) so the host and the + * secure element agree on what's bound. key->k is overwritten before the + * wc_ed25519_import_public_ex() call below, so the binding must be + * dropped here first in case that function fails its own early-return + * argument checks before reaching its reset. Clear the binding fields + * explicitly afterwards so a stale keyId never survives, even when + * se050_ed25519_free_key() returns early because the SE050 session isn't + * configured yet. */ + se050_ed25519_free_key(key); key->keyId = 0; + key->keyIdSet = 0; #endif XMEMCPY(key->k, priv, ED25519_KEY_SIZE); diff --git a/wolfcrypt/src/port/nxp/se050_port.c b/wolfcrypt/src/port/nxp/se050_port.c index ea951452fd..1bf08224f2 100644 --- a/wolfcrypt/src/port/nxp/se050_port.c +++ b/wolfcrypt/src/port/nxp/se050_port.c @@ -1483,7 +1483,7 @@ int se050_rsa_verify(const byte* in, word32 inLen, byte* out, word32 outLen, keyId = se050_allocate_key(SE050_RSA_KEY); status = sss_key_object_allocate_handle(&newKey, keyId, kSSS_KeyPart_Public, kSSS_CipherType_RSA, keySz, - kKeyObject_Mode_Persistent); + kKeyObject_Mode_Transient); } if (status == kStatus_SSS_Success) { /* Try to delete existing key first, ignore return since will @@ -1540,7 +1540,7 @@ int se050_rsa_verify(const byte* in, word32 inLen, byte* out, word32 outLen, if (status == kStatus_SSS_Success) { if (keyCreated) { /* We uploaded only the public part of the key for this verify. - * Don't persist keyIdSet=1 — a later sign on the same RsaKey + * Don't persist keyIdSet=1 -- a later sign on the same RsaKey * would reuse this binding and fail because the SE050 object has * no private material. Erase the transient object so the next * SE050 op (sign or verify) re-uploads from whatever the host @@ -3053,6 +3053,10 @@ int se050_ed25519_verify_msg(const byte* signature, word32 signatureLen, key, signature, signatureLen, msg, msgLen); #endif + if (signature == NULL || msg == NULL || key == NULL || res == NULL) { + return BAD_FUNC_ARG; + } + *res = 0; if (cfg_se050_i2c_pi == NULL) { @@ -3115,8 +3119,21 @@ int se050_ed25519_verify_msg(const byte* signature, word32 signatureLen, } if (status == kStatus_SSS_Success) { - key->keyId = keyId; - key->keyIdSet = 1; + if (keyCreated) { + /* We uploaded only the public part of the key for this verify. + * Don't persist keyIdSet=1 -- a later sign on the same ed25519_key + * would reuse this binding and fail because the SE050 object has + * no private material. Erase the transient object so the next + * SE050 op re-uploads. Mirrors the fix in se050_rsa_verify. */ + sss_key_store_erase_key(&host_keystore, &newKey); + sss_key_object_free(&newKey); + } + else { + /* Pre-existing keyIdSet=1 binding (from prior sign that uploaded + * a keypair, or explicit caller setup). Preserve it. */ + key->keyId = keyId; + key->keyIdSet = 1; + } *res = 1; ret = 0; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 8d9ebd5924..2ad8137031 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -41429,7 +41429,7 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t ed25519_test(void) /* These cases exercise host-side signature-encoding pre-validation (e.g., * sig == curve order). The SE050 port delegates verify to the secure * element, which rejects all four inputs with WC_HW_E rather than the - * BAD_FUNC_ARG / SIG_VERIFY_E the host-side path produces — so the + * BAD_FUNC_ARG / SIG_VERIFY_E the host-side path produces -- so the * expected error code differs below when built against an SE050. */ { /* Run tests for some rare code paths */