Skip to content

fix: skip RSA_MIN_PAD_SZ check for PSS padding in RsaPublicEncryptEx#10255

Open
MarkAtwood wants to merge 1 commit intowolfSSL:masterfrom
MarkAtwood:fix/rsa-pss-pubenc-size-check
Open

fix: skip RSA_MIN_PAD_SZ check for PSS padding in RsaPublicEncryptEx#10255
MarkAtwood wants to merge 1 commit intowolfSSL:masterfrom
MarkAtwood:fix/rsa-pss-pubenc-size-check

Conversation

@MarkAtwood
Copy link
Copy Markdown

Summary

The RSA_MIN_PAD_SZ guard (inLen > sz - 11RSA_BUFFER_E) is a PKCS#1 v1.5 concept. PSS has its own length check inside RsaPad_PSS (emLen >= hLen + sLen + 2 per RFC 8017 §9.1.1) and the outer guard fires first, before RsaPad_PSS ever runs.

For keys in the range [hLen+2, hLen+10] bytes, the outer guard incorrectly returns RSA_BUFFER_E for combinations where PSS with saltLen=0 would be geometrically valid. Keys in this range are non-standard but valid — they can be loaded from external DER.

Fix: add a WC_RSA_PSS #ifdef that skips the RSA_BUFFER_E return when pad_type == WC_RSA_PSS_PAD, mirroring the existing WC_RSA_NO_PADDING exception for raw mode.

Test plan

  • PSS encrypt with non-standard key sizes in [hLen+2, hLen+10] bytes no longer returns RSA_BUFFER_E
  • PSS still correctly rejects combinations that exceed its own length constraint (PSS_SALTLEN_E)
  • Existing RSA PSS tests unaffected

/cc @wolfSSL-Fenrir-bot please review

The RSA_MIN_PAD_SZ guard (inLen > sz - 11) is a PKCS#1 v1.5 constraint.
PSS has its own length check inside RsaPad_PSS (emLen >= hLen + sLen + 2
per RFC 8017) and does not need this guard. For keys in the range
[hLen+2, hLen+10] bytes, the outer guard fires and returns RSA_BUFFER_E
before RsaPad_PSS ever runs, even though PSS with saltLen=0 would be
geometrically valid for those key sizes.

Add a WC_RSA_PSS ifdef that skips the RSA_BUFFER_E return when
pad_type == WC_RSA_PSS_PAD, mirroring the existing WC_RSA_NO_PADDING
exception for raw (no-pad) mode.
@MarkAtwood MarkAtwood requested review from SparkiDev and Copilot April 17, 2026 22:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adjusts RsaPublicEncryptEx so the RSA_MIN_PAD_SZ (PKCS#1 v1.5) input-length guard does not preempt PSS padding’s own size validation, enabling certain non-standard small key sizes to proceed to RsaPad_PSS.

Changes:

  • Skips the RSA_BUFFER_E early-return when pad_type == WC_RSA_PSS_PAD (under WC_RSA_PSS).
  • Keeps the existing exception path for WC_RSA_NO_PAD behavior.

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

Comment thread wolfcrypt/src/rsa.c
Comment on lines 3455 to 3464
/* In the case that no padding is used the input length can and should
* be the same size as the RSA key. */
if (pad_type != WC_RSA_NO_PAD)
#endif
#ifdef WC_RSA_PSS
/* PSS performs its own input-length check inside RsaPad_PSS; the
* RSA_MIN_PAD_SZ guard applies only to PKCS#1 v1.5 padding. */
if (pad_type != WC_RSA_PSS_PAD)
#endif
return RSA_BUFFER_E;
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This relies on an implicit nesting effect where the second if becomes the statement-body of the first if when both WC_RSA_NO_PADDING and WC_RSA_PSS are enabled. That’s fragile (any future added statement between them would change control flow) and hard to read. Make the intent explicit by restructuring (e.g., a single boolean/compound condition that returns RSA_BUFFER_E only when padding is neither WC_RSA_NO_PAD nor WC_RSA_PSS_PAD, or by using braces with an explicit combined condition under the relevant #ifdefs).

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants