AES reject-no-key guard and RSA-PSS hash-binding test (F-6151, F-6157) by julek-wolfssl · Pull Request #10762 · wolfSSL/wolfssl

julek-wolfssl · 2026-06-23T13:03:06Z

Two independent hardening fixes with regression tests.

AES: reject mode operations when no key has been set (F-6151)

wc_AesInit zeroes the Aes struct, leaving keylen/rounds at 0. The CTR/CBC/GCM/ECB/CFB/OFB entry points never verified a key had been installed via wc_AesSetKey, so calling them right after wc_AesInit ran the software cipher against an all-zero key schedule and returned success. For CTR this exposes a reconstructable keystream; for GCM the hash subkey H is also zero, making the tag forgeable.

Adds the keylen == 0 guard (already used by wc_AesXtsEncrypt) to the software mode paths, placed after the crypto-callback fall-through so device-managed keys are unaffected. New aes_no_key_set_test() (run from aes_test()) confirms every mode rejects use before a key is set.

RSA-PSS: test signature-to-message hash binding (F-6157)

wc_RsaPSS_CheckPadding_ex2 binds a PSS signature to the message with a single comparison of the recomputed hash against the encoded hash H. Every positive PSS test passed the correct digest and the negative tests failed earlier on a wrong salt length, so removing the comparison left all default tests passing even though any well-formed PSS structure would verify against any message.

After each successful sign/verify round-trip in rsa_pss_test, flips one bit of the message digest, keeps the correct salt length, and requires BAD_PADDING_E — making the hash comparison the deciding check.

Copilot

Pull request overview

This PR introduces two security hardening changes in wolfCrypt: (1) prevent AES mode operations from running after wc_AesInit() but before a key is set, and (2) strengthen RSA-PSS regression coverage to ensure signatures are bound to the message digest.

Changes:

Add aes->keylen == 0 rejection guards to multiple AES mode implementations (CBC/CTR/GCM/ECB/CFB/OFB) in wolfcrypt/src/aes.c.
Add a new aes_no_key_set_test() and run it from aes_test() to ensure AES mode APIs reject use before wc_AesSetKey().
Extend rsa_pss_test() to tamper the digest post sign/verify and require BAD_PADDING_E, making the hash-compare step decisive.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
wolfcrypt/src/aes.c	Adds “key not set” guards to prevent AES modes from operating with an uninitialized (zero) key schedule in the software paths.
wolfcrypt/test/test.c	Adds AES regression test for “no key set” behavior and adds RSA-PSS digest-tampering negative verification to ensure hash binding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

github-actions · 2026-06-24T11:13:04Z

retest this please

github-actions · 2026-06-24T11:20:55Z

MemBrowse Memory Report

gcc-arm-cortex-m3

FLASH: .text +100 B (+0.1%, 122,639 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4

FLASH: .text +128 B (+0.1%, 200,917 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m4-baremetal

FLASH: .text +64 B (+0.1%, 66,891 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-dtls13

FLASH: .text +128 B (+0.1%, 181,156 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-openssl-compat

FLASH: .rodata +80 B, .text -2,112 B (-0.3%, 769,852 B / 1,048,576 B, total: 73% used)
RAM: .bss +192 B (+0.1%, 136,936 B / 262,144 B, total: 52% used)

gcc-arm-cortex-m4-pkcs7

FLASH: .text +64 B (+0.0%, 213,460 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-pq

FLASH: .text +64 B (+0.0%, 280,064 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

FLASH: .text +256 B (+0.1%, 326,168 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-tls12

FLASH: .text +128 B (+0.1%, 123,419 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-tls13

FLASH: .text +64 B (+0.0%, 236,743 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

FLASH: .text +128 B (+0.1%, 200,917 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m7-pq

FLASH: .text +64 B (+0.0%, 280,640 B / 1,048,576 B, total: 27% used)

linuxkm-standard

Data: __patchable_function_entries +48 B (+0.1%, 48,656 B)

stm32-sim-stm32h753

FLASH: .text +128 B (+0.1%, 183,956 B / 2,097,152 B, total: 9% used)
No memory changes detected for:
gcc-arm-cortex-m0plus
gcc-arm-cortex-m4-crypto-only
gcc-arm-cortex-m4-min-ecc
gcc-arm-cortex-m4-sp-math
gcc-arm-cortex-m7-tls13
linuxkm-pie

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Frauschi · 2026-07-01T15:51:56Z

@julek-wolfssl please resolve conflicts with master

Frauschi

🐺 Skoll Code Review

Overall recommendation: APPROVE
Findings: 3 total — 3 posted, 1 skipped

Posted findings

[Medium] AES-CCM not covered by the keyInstalled guard (same zero-key exposure as GCM) — wolfcrypt/src/aes.c:14403
[Low] New Aes struct member inserted mid-struct rather than appended at the end — wolfssl/wolfcrypt/aes.h:300
[Info] sz==0 early-return precedes keyInstalled check in some mode paths (inconsistent no-key result) — wolfcrypt/src/aes.c:7001, 15568, 15656, 16113

Skipped findings

[Info] sz==0 early-return precedes keyInstalled check in some mode paths (inconsistent no-key result)

Review generated by Skoll via Claude/Codex

Frauschi

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 1 total — 1 posted, 0 skipped

Posted findings

[High] keyInstalled guard breaks AES modes for ports that supply their own wc_AesSetKey — wolfcrypt/src/aes.c:10867

Review generated by Skoll via Claude/Codex

Frauschi

🐺 Skoll Code Review

Overall recommendation: APPROVE
Findings: 4 total — 2 posted, 2 skipped

Posted findings

[Medium] WOLFSSL_SECO_CAAM key-set path returns success without setting keyInstalled — wolfcrypt/src/aes.c:5560-5566
[Medium] aes_no_key_set_test may fail under WOLF_CRYPTO_CB / device builds — wolfcrypt/test/test.c:16277-16281

Skipped findings

[Low] New keyInstalled bitfield inserted mid-struct rather than appended
[Info] Duplicated keyInstalled guard block repeated across many mode entry points

Review generated by Skoll via Claude/Codex

Frauschi

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 3 total — 1 posted, 3 skipped

Posted findings

[High] FREESCALE_MMCAU / FREESCALE_LTC wc_AesSetKeyLocal never sets keyInstalled, breaking guarded modes — wolfcrypt/src/aes.c:5062 (FREESCALE_MMCAU), wolfcrypt/src/aes.c:4600 (FREESCALE_LTC)

Skipped findings

[Medium] New keyInstalled bitfield inserted mid-struct breaks ABI and packs poorly
[Low] Inconsistent sz==0 handling between generic and PIC32 CBC guards
[High] FREESCALE_MMCAU / FREESCALE_LTC wc_AesSetKeyLocal never sets keyInstalled, breaking guarded modes

Review generated by Skoll via Claude/Codex

julek-wolfssl · 2026-07-16T17:44:09Z

retest this please

wc_AesInit zeroes the Aes struct, leaving keylen and rounds at 0. The CTR/CBC/GCM/ECB/CFB/OFB entry points never verified that a key had been installed with wc_AesSetKey, so calling them right after wc_AesInit ran the software cipher against an all-zero key schedule and returned success instead of an error. For CTR this exposes a reconstructable keystream; for GCM the hash subkey H is also zero, making the tag forgeable. Add the keylen == 0 guard already used by wc_AesXtsEncrypt to the software mode paths. The check is placed after the crypto callback fall-through so device-managed keys are unaffected. Hardware paths that call wc_AesGetKeySize already reject this case via rounds == 0. Add aes_no_key_set_test(), run from aes_test(), which inits an Aes context and confirms every mode rejects use before a key is set.

wc_RsaPSS_CheckPadding_ex2 binds a PSS signature to the message with a single comparison of the recomputed hash against the encoded hash H. Every positive PSS test passes the correct digest and the negative tests use a wrong salt length that fails before that comparison is reached, so deleting the comparison left all default tests passing even though any well-formed PSS structure would then verify against any message. After each successful sign/verify round-trip in rsa_pss_test, flip one bit of the message digest, keep the correct salt length, and require BAD_PADDING_E. This makes the hash comparison the deciding check and catches its removal.

Replace the keylen == 0 guards with a new Aes.keySet bitfield, set by the key-installation paths and checked in the mode APIs. keylen alone is not a reliable "key installed" signal across backends: some ports map a zero key length to a default (e.g. PSOC6 maps keylen 0 to AES-128), so a keylen-based check placed only in the software paths left those builds running with the all-zero key schedule and would fail aes_no_key_set_test. keySet is set in wc_AesSetKeyLocal (the funnel for the software SetKey/SetKeyDirect/GcmSetKey paths) and in the PSOC6 wc_AesSetKey wrapper, and checked in the public CBC/CTR/GCM/ECB/CFB/OFB entry points, including the PSOC6 port variants.

Address review of the keySet bitfield: - Rename keySet to keyInstalled. struct Aes already has a Cavium-only keySet member (HAVE_CAVIUM_OCTEON_SYNC), so an unconditional keySet bitfield was a duplicate that would not compile on that build. - Set keyInstalled on every key install, not just the software funnel. The ARM, PPC64 and other hardware wc_AesSetKey/SetKeyDirect variants populate keylen/rounds directly; without also setting the flag they would have made the shared mode APIs reject a validly-keyed context. The flag is now set at every keylen-assignment point in aes.c and in the PSOC6 SetKey wrapper.

aes_no_key_set_test runs on every CI backend and exposed two ports whose own mode functions ran with an unset key. Other hardware backends (STM32, the secure-element sims) already reject this via wc_AesGetKeySize. - PIC32MZ: guard wc_AesCbcEncrypt/Decrypt. keyInstalled is already set through wc_AesSetKeyLocal, the generic SetKey funnel these use. - RISC-V assembly: set keyInstalled in all three wc_AesSetKey variants and guard CBC/ECB/GCM encrypt and decrypt. CTR already rejects via its rounds switch.

Under FIPS/selftest the AES functions come from the validated module, which does not reject use before a key is installed, so aes_no_key_set_test failed on the CAVP selftest and FIPS customer-config CI jobs. The keyInstalled guard is a non-FIPS hardening, so gate the test on !HAVE_FIPS && !HAVE_SELFTEST. The FIPS CASTs (wc_RunAllCast_fips) pass, confirming the change does not affect validated AES.

Address review feedback on the negative tampered-digest assertion, which encoded ret via WC_TEST_RET_ENC_EC(ret). That is in fact non-zero even for ret == 0, but switch to WC_TEST_RET_ENC_NC so the failure code is unambiguously non-zero and independent of ret. Verified by mutation: deleting the signature-to-message hash compare still fails the test.

CCM has the same zero-key exposure as GCM: CBC-MAC and CTR run against the all-zero key schedule left by wc_AesInit. Add the keyInstalled guard to the software CCM encrypt/decrypt paths, mirroring GCM, and to the RISC-V port so it returns BAD_FUNC_ARG instead of KEYUSAGE_E. Extend aes_no_key_set_test with CCM cases.

In the software CFB/CFB1/CFB8/OFB helpers the sz == 0 early return came before the keyInstalled check, so a no-key call succeeded with sz == 0 but failed with sz > 0. Check the key first; crypto callbacks already ran in the callers. CBC keeps its sz == 0 return first because the DCP dispatch below it cannot handle sz == 0 (reads out + sz - 16); comment the ordering there.

The keyInstalled guard added to the shared aes.c mode functions is only set by aes.c's own key schedule and the RISC-V port. Ports that ship their own compile-time wc_AesSetKey/wc_AesGcmSetKey left keyInstalled at 0, so any AES mode that falls through to a guarded aes.c path returned BAD_FUNC_ARG after a correct key setup. Set keyInstalled at the key-install point of the affected ports so the shared mode guards accept a validly-keyed context: - silabs, af_alg, devcrypto, ti, caam: wc_AesSetKey - af_alg, kcapi: wc_AesGcmSetKey

The RISC-V assembly CBC encrypt/decrypt paths return before the generic keyInstalled guard, so wc_AesCbcEncrypt/Decrypt accepted a call with no key set. aes_no_key_set_test caught this on the riscv64-o0 config. Add the guard to the RISC-V dispatch, matching the other backends.

…6151) The keyInstalled guard added to the shared aes.c mode functions is set by every in-aes.c key-schedule branch except FREESCALE_LTC and FREESCALE_MMCAU. Their wc_AesSetKeyLocal installs the key but left keyInstalled at 0, so any guarded AES mode that falls through to a shared aes.c path returned an error after a valid key setup on those ports. Set keyInstalled at the key-install point in both branches, matching the other ports fixed earlier in this PR.

Frauschi

LGTM

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #10762

Scan targets checked: wolfcrypt-bugs, wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

The AF_ALG AES mode overrides route through wc_AesSetup, which called setsockopt(ALG_SET_KEY, key, 0) with keylen 0 and returned WC_AFALG_SOCK_E instead of BAD_FUNC_ARG when no key was installed, so aes_no_key_set_test failed on WOLFSSL_AFALG. Reject setup when keyInstalled is 0, matching the software backend's no-key guard. Covers CBC, CTR, ECB and GCM.

Copilot AI review requested due to automatic review settings June 23, 2026 13:03

julek-wolfssl self-assigned this Jun 23, 2026

Copilot started reviewing on behalf of julek-wolfssl June 23, 2026 13:04 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread wolfcrypt/src/aes.c

julek-wolfssl requested a review from Copilot June 23, 2026 16:34

Copilot started reviewing on behalf of julek-wolfssl June 23, 2026 16:35 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread wolfssl/wolfcrypt/aes.h

Comment thread wolfcrypt/src/aes.c

Comment thread wolfcrypt/test/test.c

julek-wolfssl requested a review from Copilot June 24, 2026 09:28

Copilot started reviewing on behalf of julek-wolfssl June 24, 2026 09:29 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

julek-wolfssl marked this pull request as ready for review June 24, 2026 11:12

julek-wolfssl requested a review from Copilot June 24, 2026 18:03

Copilot started reviewing on behalf of julek-wolfssl June 24, 2026 18:04 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread wolfcrypt/test/test.c Outdated

julek-wolfssl assigned wolfSSL-Bot and unassigned julek-wolfssl Jun 24, 2026

Frauschi assigned julek-wolfssl and unassigned wolfSSL-Bot Jul 1, 2026

julek-wolfssl force-pushed the fenrir/20260623 branch from 8460187 to ef13b58 Compare July 13, 2026 20:58

Frauschi reviewed Jul 14, 2026

View reviewed changes

Comment thread wolfcrypt/src/aes.c

Comment thread wolfssl/wolfcrypt/aes.h

julek-wolfssl requested a review from Frauschi July 14, 2026 13:26

Frauschi requested changes Jul 14, 2026

View reviewed changes

Comment thread wolfcrypt/src/aes.c

julek-wolfssl requested a review from Frauschi July 15, 2026 10:09

julek-wolfssl force-pushed the fenrir/20260623 branch from 5ff58d0 to dd065cb Compare July 15, 2026 10:49

Frauschi reviewed Jul 15, 2026

View reviewed changes

Comment thread wolfcrypt/src/aes.c

Comment thread wolfcrypt/test/test.c Outdated

Frauschi requested changes Jul 16, 2026

View reviewed changes

julek-wolfssl force-pushed the fenrir/20260623 branch from da26195 to 23363d7 Compare July 21, 2026 16:35

julek-wolfssl added 14 commits July 22, 2026 13:31

Address review: set keyInstalled in SECO CAAM key setup (F-6151)

a268371

Address review: use INVALID_DEVID in aes_no_key_set_test (F-6151)

d27229e

julek-wolfssl force-pushed the fenrir/20260623 branch from 23363d7 to 37b9d4d Compare July 22, 2026 11:31

Frauschi previously approved these changes Jul 22, 2026

View reviewed changes

julek-wolfssl assigned wolfSSL-Bot and unassigned julek-wolfssl Jul 22, 2026

SparkiDev previously approved these changes Jul 23, 2026

View reviewed changes

SparkiDev self-assigned this Jul 23, 2026

SparkiDev requested a review from wolfSSL-Fenrir-bot July 23, 2026 23:21

wolfSSL-Fenrir-bot reviewed Jul 24, 2026

View reviewed changes

Comment thread wolfcrypt/test/test.c

SparkiDev assigned julek-wolfssl and unassigned SparkiDev Jul 24, 2026

julek-wolfssl dismissed stale reviews from SparkiDev and Frauschi via 3917f2f July 24, 2026 07:40

julek-wolfssl removed their assignment Jul 24, 2026

Frauschi approved these changes Jul 24, 2026

View reviewed changes

Uh oh!

Conversation

julek-wolfssl commented Jun 23, 2026

AES: reject mode operations when no key has been set (F-6151)

RSA-PSS: test signature-to-message hash binding (F-6157)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Jun 24, 2026

Uh oh!

github-actions Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Frauschi commented Jul 1, 2026

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

🐺 Skoll Code Review

Posted findings

Uh oh!

Uh oh!

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

🐺 Skoll Code Review

Posted findings

Uh oh!

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

🐺 Skoll Code Review

Posted findings

Uh oh!

Uh oh!

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

🐺 Skoll Code Review

Posted findings

Uh oh!

julek-wolfssl commented Jul 16, 2026

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #10762

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions Bot commented Jun 24, 2026 •

edited

Loading