[SparkiDev]

Description

The separator scan in RsaUnPad_OAEP() used a 0/1 mask that Clang turns into a branch on each byte being zero on PowerPC, Thumb-1 and x86_64 when optimizing. This leaked which bytes are zero and, through the loop timing, the separator position. The separator byte was then loaded by its index, putting the separator position on the address bus.

Find the separator with one masked pass that records its index and value, so no load depends on the separator position. Pass the byte and the per-byte mask through volatiles so the compiler can't tell the byte is less than 256 or that the mask is either 0 or all ones, which is what let it turn the masking back into a branch.

Fixes zd#22793

Testing

./configure

[Copilot]

🟡 Changes recommended

The OAEP scan lacks separator-focused regression coverage and leaves the final plaintext byte in volatile local storage after cleanup.

2 open issues

• Add OAEP separator-scan regression tests · New
• Wipe the final plaintext byte after scanning · New

What changed in this PR

This PR hardens wolfCrypt’s software RSA OAEP unpadding against separator-position leaks from compiler-generated branches and indexed memory reads.

Changes:

• Replaces the separator search with a full masked scan that records both its index and value.
• Uses volatile intermediates to prevent the identified branch-producing optimizations.
• Validates the recorded separator value without reloading it by index.

File	Description
wolfcrypt/​src/​rsa.c	Updates RsaUnPad_OAEP() to use masked separator selection and validation.

_{Review effort: Balanced}

[github-actions]

MemBrowse Memory Report · Explore what changed →

gcc-arm-cortex-m4-rsa-only

• FLASH: .text +64 B (+0.0%, 344,400 B / 1,048,576 B, total: 33% used)

stm32-sim-stm32h753

• FLASH: .text +64 B (+0.0%, 205,484 B / 2,097,152 B, total: 10% 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-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

---

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

[Copilot]

🟡 Changes recommended

The new RSA tests can overrun a fixed OAEP stack buffer in 1024-bit builds and invoke unavailable software paths in hardware-only builds.

2 open issues

• Avoid 256-byte OAEP test input with 1024-bit RSA scratch buffer · New
• Skip software RSA tests when WOLFSSL_NO_RSA_SW is defined · New

2 resolved since last review

• Wipe the final plaintext byte after scanning
• Add OAEP separator-scan regression tests

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The test remains enabled in callback-only RSA builds without a valid device or registered provider, causing MAKE_RSA_KEY() to return NO_VALID_DEVID instead of generating a key.

1 open issue

• Exclude callback-only RSA builds from software-path test · New

2 resolved since last review

• Avoid 256-byte OAEP test input with 1024-bit RSA scratch buffer
• Skip software RSA tests when WOLFSSL_NO_RSA_SW is defined

_{Review effort: Balanced}

[SparkiDev]

@claude review

[Copilot]

🔵 Needs a closer look

The constant-time security guarantee requires target-specific validation of optimized Clang output on PowerPC, Thumb-1, and x86_64; source review and functional regression tests cannot establish it.

No open issues 🎉

1 resolved since last review

• Exclude callback-only RSA builds from software-path test

_{Review effort: Balanced}

[claude]

Beyond the inline findings, I also checked the changes to the separator scan in RsaUnPad_OAEP. These are not affected:

• The loop now runs to the last byte. If the separator is the last byte, idx == pkcsBlockLen and the message is zero-length, which is correct.
• If no non-zero byte is found, sepVal stays 0 and the error path returns 0 length.
• The c sum can only be zero when lHash, the separator and Y all match, because every term is non-negative.

Extended reasoning...

The PR changes the OAEP unpadding separator search in wolfcrypt/src/rsa.c, which is constant-time crypto code. It also adds unit tests in tests/api/test_rsa.c. Two findings are posted inline, so a human review is needed anyway. This note only records what else was checked: the wider loop bound, the case where no separator is found, and how the error accumulator is combined.

[SparkiDev]

@claude review

[Copilot]

🔵 Needs a closer look

The constant-time security claim requires checking optimized Clang output on PowerPC, Thumb-1, and x86_64 for secret-dependent branches and memory accesses, which source review and functional tests cannot establish.

No open issues 🎉

_{Review effort: Balanced}

[claude]

Both of my earlier findings are fixed in the updated commit, and this pass found no new bugs. Because this is constant-time code in RSA OAEP decryption, a human should still review it.

What was reviewed:

• The RSA_MAX_SIZE/8 cap in RsaUnPad_OAEP now applies only when tmp is on the stack, so small-stack malloc builds still accept larger blocks. test_wc_RsaUnPad_OaepBlockLen now expects the matching result for each build type.
• The OAEP test helpers are now compiled only when TEST_RSA_OAEP_SEPARATOR or TEST_RSA_OAEP_ZEROLEN is defined, which are the conditions under which their callers exist. Builds with no caller no longer get unused static functions.
• The masked separator scan now covers the last byte. An all-zero tail leaves sepVal at 0, so the 0x01 check fails as it should.

Extended reasoning...

The change rewrites the separator search in RsaUnPad_OAEP (wolfcrypt/src/rsa.c) to remove timing and address-bus leaks, adds a stack-buffer length cap, and adds three OAEP tests. This is cryptographic decryption code where constant-time behavior is a security property. That cannot be confirmed from source alone, because it depends on what the compiler generates. Both earlier findings are fixed in commit 2aee1ac. The decision to defer rests only on how security-sensitive the code is.