fix: guard RsaUnPad_PSS masking when bits==0#10256
fix: guard RsaUnPad_PSS masking when bits==0#10256MarkAtwood wants to merge 1 commit intowolfSSL:masterfrom
Conversation
When the RSA modulus bit count is congruent to 1 mod 8 (e.g. a 2041-bit or 3073-bit key), (mp_count_bits(n) - 1) & 0x7 == 0, so the leading zero byte has already been consumed and the pointer advanced. In that case (1<<0)-1 == 0, zeroing tmp[0] and pkcsBlock[0] and corrupting maskedDB[0] before the XOR separator comparison. RsaPad_PSS guards the equivalent step with 'if (hiBits)'. Add the same guard to RsaUnPad_PSS. Standard power-of-two key sizes (1024, 2048, 4096-bit) are unaffected as their bit counts give bits==7 after the recomputation. The bug affects non-standard key sizes loaded from external DER where mp_count_bits(n) % 8 == 1.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes RSA-PSS unpadding for non-standard modulus bit sizes where bits == 0 to avoid zeroing masked bytes during the MSB masking step.
Changes:
- Add an
if (bits)guard around MSB masking inRsaUnPad_PSSto prevent(1 << 0) - 1from zeroingtmp[0]andpkcsBlock[0]. - Add an explanatory comment describing why the masking is skipped when
bits == 0.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* When bits==0 the key size is an exact multiple of 8 and pkcsBlock was | ||
| * already advanced past the leading 0x00 byte (see above); no masking is | ||
| * needed. (1<<0)-1 == 0 would zero both bytes and corrupt the XOR | ||
| * separator check below. RsaPad_PSS guards the same step with | ||
| * "if (hiBits)" for the same reason. */ |
There was a problem hiding this comment.
The comment’s statement “the key size is an exact multiple of 8” appears inconsistent with how bits is derived for RSA-PSS MSB masking. With the common pattern bits = (mp_count_bits(n) - 1) & 0x7, bits == 0 corresponds to mp_count_bits(n) ≡ 1 (mod 8), not a multiple of 8. Suggest updating the comment to describe the actual condition (e.g., modulus bit length congruent to 1 mod 8, and the leading 0x00 handling) so future readers don’t infer the wrong key-size relationship.
| /* When bits==0 the key size is an exact multiple of 8 and pkcsBlock was | |
| * already advanced past the leading 0x00 byte (see above); no masking is | |
| * needed. (1<<0)-1 == 0 would zero both bytes and corrupt the XOR | |
| * separator check below. RsaPad_PSS guards the same step with | |
| * "if (hiBits)" for the same reason. */ | |
| /* When bits==0, the modulus bit length is congruent to 1 mod 8, so the | |
| * encoded block includes a leading 0x00 byte and pkcsBlock was already | |
| * advanced past it (see above); no masking is needed. (1<<0)-1 == 0 | |
| * would zero both bytes and corrupt the XOR separator check below. | |
| * RsaPad_PSS guards the same step with "if (hiBits)" for the same | |
| * reason. */ |
|
Summary
When
(mp_count_bits(n) - 1) & 0x7 == 0(i.e. the modulus bit count is congruent to 1 mod 8 — e.g. a 2041-bit or 3073-bit key),RsaUnPad_PSSadvances past the leading zero byte and then executes:(1 << 0) - 1 == 0zeros both bytes, corrupting maskedDB[0] before the XOR separator comparison.RsaPad_PSSalready guards the equivalent step withif (hiBits)for exactly this reason.Fix: add
if (bits)guard, mirroringRsaPad_PSS.Standard power-of-two key sizes (1024, 2048, 4096-bit) are unaffected — their bit counts give
bits == 7after the recomputation. The bug affects non-standard key sizes loaded from external DER wheremp_count_bits(n) % 8 == 1. Cross-library PSS verification (verifying a Go/OpenSSL signature with wolfSSL) surfaces this asymmetry.Test plan
mp_count_bits % 8 == 1) against a signature from OpenSSL passes/cc @wolfSSL-Fenrir-bot please review