Fix PKCS#7 padding for block-aligned input in wc_EncryptPKCS8Key_ex#10836
Fix PKCS#7 padding for block-aligned input in wc_EncryptPKCS8Key_ex#10836yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes PKCS#7 padding generation in wc_EncryptPKCS8Key_ex when the plaintext PKCS#8 DER is already block-aligned (ensuring a full extra padding block for CBC modes), and adds API tests that validate the encrypted OCTET STRING length to catch this interop issue.
Changes:
- Correct PKCS#7 pad length calculation for block-aligned inputs (emit full block for CBC; no pad for stream ciphers).
- Add targeted API tests that assert the ciphertext OCTET STRING length for AES-CBC (16), DES (8), and RC4 (1) paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
wolfcrypt/src/asn.c |
Fixes pad size computation so block-aligned plaintext produces a full PKCS#7 padding block for CBC ciphers. |
tests/api.c |
Adds coverage to assert encrypted OCTET STRING lengths for block-aligned plaintext across multiple block sizes and a stream-cipher case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10836
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
Frauschi
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
Posted findings
- [Medium] Length check assumes short-form DER length —
tests/api.c:21883 - [Low] Redundant asn.h include —
tests/api.c:85
Review generated by Skoll via Claude/Codex
|
Jenkins retest this please |
Description
wc_EncryptPKCS8Key_excomputed the PKCS#7 pad length with an extra(blockSz - 1)mask:When the plaintext length is an exact multiple of the cipher block size,
keySz & (blockSz - 1) == 0, so the inner term isblockSzand the trailingmask collapses it to
0— no padding is emitted. PKCS#7 (and CBC as OpenSSLimplements it) requires a pad length of
1..blockSz, i.e. a whole extrablock when the input is already block-aligned.
The fix removes the mask and guards the computation on
blockSz > 1so streamciphers (RC4,
blockSz == 1) still receive no padding, matching the siblingwc_PkcsPadhelper and PKCS#7.Impact
Any private key whose plaintext PKCS#8 DER length is an exact multiple of the
cipher block size produced an
EncryptedPrivateKeyInfothat non-wolfSSL parsersreject (OpenSSL:
bad decrypt). Examples with AES (16-byte block):correctly and worked, which is why this went unnoticed.
wolfCrypt's own
wc_DecryptPKCS8Keystrips padding by re-reading the inner DERSEQUENCElength rather than validating PKCS#7, so wolfSSL↔wolfSSL round-tripssucceeded and the bug hid in self-tests — only wolfSSL↔OpenSSL interop exposed it.
Changes
wolfcrypt/src/asn.c— correct thepadSzcomputation inwc_EncryptPKCS8Key_ex; gate padding onblockSz > 1.tests/api.c— addenc_pkcs8_pad_checkhelper and three cases coveringblock-aligned input at different block sizes plus the stream-cipher path:
encoder branch
blockSz > 1guardEach asserts the encrypted
OCTET STRINGlength directly, since a decryptround-trip alone false-passes on a missing pad block.
Notes
padded output already decrypts, and previously-written blobs still read).
blockSzis always1,8, or16wherepadSzis computed(
CheckAlgo/GetAlgoV2set it on every success path), so the mask arithmeticis well-defined.
Testing
Built with
--enable-des3 --enable-arc4 --enable-aescbcunder ASan+UBSan(macOS, no LeakSanitizer); all four PKCS#8 API tests pass, full API suite clean
(0 failures). Negative control confirmed: reverting the fix fails the
block-aligned length assertion at exactly the padding check.