fix: compare RSA blocks instead of parsing - #1203
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates wolfSSH_RsaVerify() to follow RFC 8332 §3 guidance by verifying RSASSA-PKCS1-v1_5 signatures via a raw public RSA operation and constant-time comparison of full modulus-sized EMSA-PKCS1-v1_5 blocks, rather than parsing the decrypted output. It also adds a unit test that exercises malformed padding cases to ensure the implementation rejects non–well-formed encodings.
Changes:
- Reworked
wolfSSH_RsaVerify()to build an expected EMSA-PKCS1-v1_5 block, recover the signature block viawc_RsaFunction(), and compare full blocks in constant time. - Added key-size validation and refactored temporary buffers into a single heap allocation.
- Added
test_RsaVerify_BadPadding()to validate rejection of malformed padding and incorrect block type.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/unit.c | Adds a padding-focused RSA verify unit test and wires it into the unit test runner. |
| src/internal.c | Implements block-based RSA PKCS#1 v1.5 verification using raw public operation + constant-time full-block compare. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
3235bfd to
71358b5
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1203
Scan targets checked: wolfssh-bugs, wolfssh-src
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.
RFC 8332 section 3 advises against verifying RSASSA-PKCS1-v1_5 by applying the key and parsing the output. wolfSSH_RsaVerify() builds the expected EMSA-PKCS1-v1_5 block, applies the key with a raw public operation, and compares whole modulus-sized blocks. - verify through wc_RsaFunction() and a constant-time block compare - reject a signature not less than the modulus, which the raw operation would otherwise reduce into a second encoding of the same signature - reject a key too small for the digest, and a wc_RsaEncryptSize() error - carve the four working blocks from one allocation - add test_RsaVerify_BadPadding() and test_RsaVerify_SigRange() Offload builds leave the modulus empty, so the range check is skipped there. That check walks the bytes itself rather than calling memcmp(), which some libcs compare as signed char, ordering the modulus's high byte backwards. The padding cases pass against the old code as well; the range case does not. Issue: F-10574
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1203
Scan targets checked: wolfssh-bugs, wolfssh-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
RFC 8332 section 3 advises against verifying RSASSA-PKCS1-v1_5 by applying the key and parsing the output. wolfSSH_RsaVerify() builds the expected EMSA-PKCS1-v1_5 block, applies the key with a raw public operation, and compares whole modulus-sized blocks.
The test passes against the old code too; wolfSSL's unpadding is already strict, so it guards the property rather than a live defect.
Issue: F-10574