Fix PKCS#7 padding for block-aligned input in wc_EncryptPKCS8Key_ex

[yosuke-wolfssl]

Description

wc_EncryptPKCS8Key_ex computed the PKCS#7 pad length with an extra
(blockSz - 1) mask:

padSz = (word32)((blockSz - ((int)keySz & (blockSz - 1))) & (blockSz - 1));

When the plaintext length is an exact multiple of the cipher block size,
keySz & (blockSz - 1) == 0, so the inner term is blockSz and the trailing
mask collapses it to 0 — no padding is emitted. PKCS#7 (and CBC as OpenSSL
implements it) requires a pad length of 1..blockSz, i.e. a whole extra
block when the input is already block-aligned.

The fix removes the mask and guards the computation on blockSz > 1 so stream
ciphers (RC4, blockSz == 1) still receive no padding, matching the sibling
wc_PkcsPad helper and PKCS#7.

Impact

Any private key whose plaintext PKCS#8 DER length is an exact multiple of the
cipher block size produced an EncryptedPrivateKeyInfo that non-wolfSSL parsers
reject (OpenSSL: bad decrypt). Examples with AES (16-byte block):

• X25519 / Ed25519 — 48-byte PKCS#8 (a multiple of 16) → 0 padding → broken.
• RSA / EC / DH in common sizes are usually not block-aligned, so they padded
  correctly and worked, which is why this went unnoticed.

wolfCrypt's own wc_DecryptPKCS8Key strips padding by re-reading the inner DER
SEQUENCE length rather than validating PKCS#7, so wolfSSL↔wolfSSL round-trips
succeeded and the bug hid in self-tests — only wolfSSL↔OpenSSL interop exposed it.

Changes

• wolfcrypt/src/asn.c — correct the padSz computation in
  wc_EncryptPKCS8Key_ex; gate padding on blockSz > 1.

• tests/api.c — add enc_pkcs8_pad_check helper and three cases covering
  block-aligned input at different block sizes plus the stream-cipher path:

  • PBES2 / AES-256-CBC (blockSz 16) — full 16-byte pad block
  • PBES1 / SHA1-DES (blockSz 8) — full 8-byte pad block, exercises the PBES1
    encoder branch
  • PKCS#12 / RC4 (blockSz 1) — no padding, exercises the blockSz > 1 guard

  Each asserts the encrypted OCTET STRING length directly, since a decrypt
  round-trip alone false-passes on a missing pad block.

Notes

• No public API or signature changes; decrypt side needs no change (correctly
  padded output already decrypts, and previously-written blobs still read).
• blockSz is always 1, 8, or 16 where padSz is computed
  (CheckAlgo/GetAlgoV2 set it on every success path), so the mask arithmetic
  is well-defined.

Testing

Built with --enable-des3 --enable-arc4 --enable-aescbc under 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.

[Copilot]

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.

[github-actions]

MemBrowse Memory Report

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-rsa-only
• 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
• linuxkm-pie
• linuxkm-standard
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

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]

🐺 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

[Frauschi]

🟡 [Medium] Length check assumes short-form DER length
💡 SUGGEST test

This reads encKey[encKeySz - expEncLen - 1] as a single short-form length byte and compares it to (byte)expEncLen. That only holds while the encrypted content is < 128 bytes (short-form DER length), which is true for the current fixtures (48/56/64 bytes).

If this helper is later reused for larger keys — e.g. ML-DSA private keys, which run to several KB — the OCTET STRING length switches to long-form (0x82 | len_hi | len_lo). Both reads then land on the wrong bytes: the real tag sits two bytes earlier, and the compared value is a truncated 8-bit slice of a multi-KB length. The helper would silently false-fail (or false-pass).

Optional hardening: guard with expEncLen < 128 so a future larger-plaintext caller fails loudly rather than mis-parsing the length field. Longer term, if PKCS#8-encrypt coverage is added for ML-DSA keys, the helper would need to decode long-form lengths properly.

[Frauschi]

🔵 [Low] Redundant asn.h include
🔧 NIT convention

This #include <wolfssl/wolfcrypt/asn.h> under #if defined(HAVE_PKCS8) && !defined(NO_ASN) duplicates the conditional include at lines 79–81 (under WOLFSSL_SMALL_CERT_VERIFY). asn.h has include guards so it's harmless, and the new guard does correctly ensure the ASN tag / PBE enum symbols are available when the small-cert path isn't compiled.

Optional: fold this into the existing include block to avoid the duplication, or leave as-is.