[night1rider]

Description

Adds wc_CryptoCb_SetKeyLocation(), so a SetKey crypto callback can say where the key it took is kept: WC_KEY_SW_STORED (default) or WC_KEY_HW_STORED (on the device only). It is one API for every key type.

• HMAC: a key marked WC_KEY_HW_STORED is stored in Hmac.keyLocation. If the device then declines wc_HmacUpdate or wc_HmacFinal, they return MISSING_KEY instead of running in software without the key. Opt out with WOLFSSL_NO_HMAC_KEY_LOCATION.
• AES: the location is checked and nothing is stored. Fix AES and GCM state after a crypto callback claims the key #11666 already refuses a claimed AES key with no schedule.
• Other key types return NOT_COMPILED_IN.

Builds with WOLF_CRYPTO_CB and WOLF_CRYPTO_CB_SETKEY.

Testing

• test_wc_HmacNoSwKey_CryptoCb: a HW-stored HMAC key makes Update and Final return MISSING_KEY; a declined key still works in software.
• test_wc_CryptoCb_SetKeyLocation: bad arguments, AES accepts both locations, and NOT_COMPILED_IN for a type with no support. Runs in every SetKey build.
• test_wc_CryptoCb_AesNoSwKey: an AES key the callback marks HW stored is refused by CBC, streaming GCM and XTS. A key the device declines works in software.
• Ran ./tests/unit.test --api --group aes --group hmac on: base cryptocb, --enable-all, WOLF_CRYPTO_CB_FIND, WOLFSSL_NO_AES_KEY_SET_CHECK, WOLFSSL_NO_HMAC_KEY_LOCATION, AES-only SetKey callback, ARM32 asm (qemu) and PPC64 asm (qemu).

[github-actions]

MemBrowse Memory Report · Explore what changed →

zephyr-frdm_rw612

• FLASH: log_const_area -8 B, rodata -8 B, text +1,576 B (+0.6%, 284,024 B / 67,108,864 B, total: 0% used)

No memory changes detected for:

• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• stm32-sim-stm32h753

---

_{Catch memory regressions in CI with MemBrowse. Free for open source.}

[Copilot]

🟡 Changes recommended

XTS assembly can reuse stale software keys after hardware-only rekeying, and new tests fail in supported configurations.

3 open findings

• Reject hardware-only XTS keys before using stale AES-NI schedules · New
• Guard CBC missing-key assertions when key-set checks are disabled · New
• Exclude KCAPI HMAC configurations from mock missing-key expectations · New

What changed in this PR

Extends wolfCrypt’s SetKey callbacks to distinguish software-held keys from hardware-only keys and prevent invalid software fallback.

Changes:

• Adds key-location tracking and a callback-facing setter.
• Updates AES and HMAC fallback handling.
• Adds regression tests for hardware-only keys and software rekeying.

File	Description
wolfssl/​wolfcrypt/​hmac.h	Declares HMAC key-location state and setter.
wolfssl/​wolfcrypt/​cryptocb.h	Exposes key-location constants and API.
wolfssl/​wolfcrypt/​aes.h	Declares AES key-location state and setter.
wolfcrypt/​src/​hmac.c	Rejects fallback with hardware-only keys.
wolfcrypt/​src/​cryptocb.c	Dispatches key-location updates.
wolfcrypt/​src/​aes.c	Tracks key location and adjusts GCM setup.
tests/​api/​test_hmac.h	Registers the HMAC regression test.
tests/​api/​test_hmac.c	Tests hardware-only HMAC behavior.
tests/​api/​test_aes.h	Registers the AES regression test.
tests/​api/​test_aes.c	Tests hardware-only AES behavior.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11754

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 3 of 8 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_aes.c, tests/api/test_hmac.c, wolfcrypt/src/cryptocb.c, wolfssl/wolfcrypt/cryptocb.h, wolfssl/wolfcrypt/hmac.h

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.

Review tier: Lite

[Copilot]

🟡 Changes recommended

Hardware-only AES rekeying can leave stale software state usable through streaming GCM or when key-set checks are disabled.

2 open findings

• Clear stale GCM state when installing hardware-only keys · New
• Reject hardware-only keys despite disabled key-set checks · New

3 resolved since last review

• Reject hardware-only XTS keys before using stale AES-NI schedules
• Exclude KCAPI HMAC configurations from mock missing-key expectations
• Guard CBC missing-key assertions when key-set checks are disabled

🧠 Review effort: Balanced

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11754

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 3 of 3 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

[Copilot]

🟡 Changes recommended

Unsupported AES backends and streaming GCM initialization still allow unsafe handling of hardware-only keys.

2 open findings

• Reject hardware-only keys before GCM software initialization · New
• Reject unsupported backends for key-location enforcement · New

2 resolved since last review

• Reject hardware-only keys despite disabled key-set checks
• Clear stale GCM state when installing hardware-only keys

🧠 Review effort: Balanced

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11754

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 2 of 3 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_aes.c

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11754

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer

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.

Review tier: Lite

[Frauschi]

Hi @night1rider,

This PR overlaps quite a bit with my #11666, which fixes the same AES problem: a SETKEY callback that keeps the key on the device without building a schedule still leaves the context marked as keyed, so software modes run on an empty or stale schedule.

#11666 handles that without an opt-in. A callback that returns 0 and leaves aes->rounds at 0 gets keyInstalled cleared, so the software modes and streaming GCM return MISSING_KEY. It covers both WOLF_CRYPTO_CB_SETKEY and WOLF_CRYPTO_CB_AES_SETKEY, wipes the old key's schedule, devKey and GCM state before the device sees the new key, skips H in wc_AesGcmSetKey, and does the same in the ARM32 and PPC set-key copies.

A trial merge of the two gives three conflicts in aes.c (the SETKEY branch of wc_AesSetKey, the keyInstalled assignment in AesSetKeyLocal, and the block after the set-key in wc_AesGcmSetKey). They also disagree on wc_AesGcmInit: here it returns MISSING_KEY for a HW-stored key, while #11666 caches an IV given before a usable GCM key and returns 0.

What I would suggest: land #11666 for AES, then rebase this one on top and keep the HMAC part, which #11666 doesn't touch. Once #11666 is in, WC_KEY_HW_STORED for AES only restates rounds == 0, so the AES keyLocation field and the aes.c changes could go, or shrink to recording the location if you want wc_CryptoCb_SetKeyLocation() to work the same for every key type. Open to doing it the other way round, I just don't want us to land two mechanisms for the same thing.

[Frauschi]

Thanks for rebasing on #11666. I tried this head with WC_AES_KEY_HW_ONLY() defined to 0 and wc_AesSetKeyLocation() only recording the location. test_wc_CryptoCb_AesNoSwKey still passes, except that XTS returns BAD_FUNC_ARG from master's rounds == 0 check instead of MISSING_KEY. So for AES, WC_KEY_HW_STORED repeats what a claim without a schedule already does, and the new checks never fire while the key-set check is on.

HMAC is different and worth landing. A callback may run the software wc_HmacSetKey itself and return 0, so a claim can't be read as device-only. On master, a claimed HMAC key with no software copy is MAC'd without the key.

Could the AES side shrink to recording the location in wc_CryptoCb_SetKeyLocation(), so callbacks keep one API for both types?

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[night1rider]

Thanks for rebasing on #11666. I tried this head with WC_AES_KEY_HW_ONLY() defined to 0 and wc_AesSetKeyLocation() only recording the location. test_wc_CryptoCb_AesNoSwKey still passes, except that XTS returns BAD_FUNC_ARG from master's rounds == 0 check instead of MISSING_KEY. So for AES, WC_KEY_HW_STORED repeats what a claim without a schedule already does, and the new checks never fire while the key-set check is on.

HMAC is different and worth landing. A callback may run the software wc_HmacSetKey itself and return 0, so a claim can't be read as device-only. On master, a claimed HMAC key with no software copy is MAC'd without the key.

Could the AES side shrink to recording the location in wc_CryptoCb_SetKeyLocation(), so callbacks keep one API for both types?

Thank you for testing it @Frauschi . I dropped the separate AES checks in 1c87ec7f80, and reverted the unit tests accordingly.

I kept one thing on the AES side. When a key is marked HW stored, wc_AesSetKeyLocation() still sets rounds to 0 and turns off the GCM key flag. #11666's rule then sees a claimed key with no schedule and marks it as not installed.

HMAC is unchanged. test_wc_HmacNoSwKey_CryptoCb covers it: a HW-stored key makes Update and Final return MISSING_KEY, and a declined key still works in software.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11754

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): .wolfssl_known_macro_extras, tests/api/test_aes.h, tests/api/test_hmac.h
Coverage: 3 of 6 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_aes.c, tests/api/test_hmac.c, wolfcrypt/src/cryptocb.c

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.

Review tier: Lite

[Frauschi]

Thanks, the HMAC side looks right.

On the AES side, only the test reads Aes.keyLocation now, and the clears in wc_AesSetKeyLocation() are no-ops: the pre-claim wipe already zeroed rounds and the GCM key flag, and wc_AesSetKeyLocal() overwrites keyInstalled after the callback. With the HW branch only recording the location, test_wc_CryptoCb_AesNoSwKey still passes. So the AES case in wc_CryptoCb_SetKeyLocation() could just validate location and return 0, dropping the struct field, the resets and WOLFSSL_NO_AES_KEY_LOCATION.

That also settles the Fenrir comment, since the mark then carries no AES state in any build. Gating on WOLFSSL_AES_REQUIRE_KEY_SET instead would make WOLF_CRYPTO_CB_ONLY_AES reject a callback that marks its key HW stored.

Could you also add a short PR description?

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11754

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): .wolfssl_known_macro_extras, tests/api/test_aes.h, tests/api/test_hmac.h
Coverage: 4 of 6 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_aes.c, tests/api/test_hmac.c

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

Fenrir's latest completed scan found no issues; clearing the prior automated change request.