Skip to content

Fix EncryptedPrivateKeyInfo encode + decode (PBES2) across all key types#426

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4081
Open

Fix EncryptedPrivateKeyInfo encode + decode (PBES2) across all key types#426
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4081

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Fix EncryptedPrivateKeyInfo encode + decode (PBES2) across all key types

Summary

wolfProvider's EncryptedPrivateKeyInfo (encrypted PKCS#8) support was
both weak and incomplete:

  1. Encoder (finding f_4081): the EPKI encoders derived the key with
    MD5 / PBKDF1 / 1 iteration, encrypted the DER in place, and emitted it
    under the unencrypted -----BEGIN PRIVATE KEY----- label. The output was
    not a valid EncryptedPrivateKeyInfo and could not be decoded by OpenSSL or
    by wolfProvider itself.
  2. Decoder: only RSA could decode a raw-DER encrypted PKCS#8; EC/DH/ECX/ML-DSA
    decoders did not handle encrypted input, so those keys failed to load in DER
    form (PEM happened to work via a different path).

This PR replaces the encode path with standards-compliant PBES2
(PBKDF2 + AES-CBC)
and makes the decode path handle encrypted PKCS#8 for every
key type, routed through shared helpers.

Changes

Encoder (finding f_4081)

  • New shared wp_encrypt_key_pkcs8() / _size() in wp_internal.c wrapping
    wc_EncryptPKCS8Key (PBKDF2 + AES-CBC, WP_PKCS12_ITERATIONS_DEFAULT).
  • All EPKI encoders (RSA, EC, DH, ECX, ML-DSA) route through it; RSA's existing
    PrivateKeyInfo+cipher path unified onto the same helper.
  • Fixed PEM label to PKCS8_ENC_PRIVATEKEY_TYPE (ENCRYPTED PRIVATE KEY) and
    fixed output sizing to use the real LENGTH_ONLY_E query (block-rounding
    under-sized the structure).
  • Removed the weak wp_encrypt_key / wp_BufferKeyEncrypt / wp_EncryptedInfoGet
    helpers and their MD5 dependency.

Decoder

  • New shared wp_decrypt_key_pkcs8() in wp_internal.c (detect PBES2 via the
    PBKDF2 OID, get passphrase, wc_DecryptPKCS8Key in place).
  • RSA wp_rsa_decode_enc_pki refactored onto it (removed the RSA-local
    pbkdf2_oid table / wp_rsa_find_pbkdf2_oid).
  • Added wp_*_decode_enc_pki fallbacks to EC/DH/ECX/ML-DSA: on plaintext-decode
    failure, decrypt then re-decode. No change to wp_dec_epki2pki.c.

Common

  • Both helpers guarded on HAVE_PKCS8 && !NO_PWDBASED and fail closed rather
    than emit weak/plaintext output. This matches the pre-existing decode-side
    requirement (wc_DecryptPKCS8Key, also HAVE_PKCS8).
  • Passphrase buffers use the WOLFSSL_SMALL_STACK pattern and wc_ForceZero.

Tests

New encrypted-PKCS#8 round-trip tests via a shared test_epki_encode_decode
helper, gated on WP_HAVE_EPKI_TEST (WOLFSSL_ENCRYPTED_KEYS && HAVE_PKCS8 && !NO_PWDBASED).

Key type DER wp→OpenSSL PEM wp→OpenSSL DER wp→wp PEM wp→wp
RSA
EC (P-256)
DH
ECX (Ed25519)
ML-DSA-44

wp→OpenSSL = stock OpenSSL decrypts wolfProvider's output (interop guarantee).
wp→wp = wolfProvider round-trips its own output.

Verification

  • Non-PQC build + make test: green (RSA/EC/DH/ECX; ML-DSA guarded out).
  • PQC build (OpenSSL 3.6 + wolfSSL ML-DSA) + make test: green (all 5 EPKI
    tests).
  • ASan + UBSan: clean across all modified sources.

Known limitations / follow-ups (out of scope)

  • ECX interop (⏸): X25519/Ed25519 plaintext PKCS#8 is block-aligned;
    wc_EncryptPKCS8Key_ex omits the PKCS#7 padding block for block-aligned input
    (asn.c), which OpenSSL rejects. Fix belongs in wolfSSL; ECX wp→OpenSSL
    interop is not asserted until it lands.
    On going: Fix PKCS#7 padding for block-aligned input in wc_EncryptPKCS8Key_ex wolfssl#10836
  • ML-DSA interop (—): stock OpenSSL does not decode wolfProvider's ML-DSA
    EPKI — a separate ML-DSA/PQC-decode matter, independent of this change.
  • Iteration count stays at WP_PKCS12_ITERATIONS_DEFAULT (2048) for parity with
    OpenSSL's default output; raising it is a separate decision.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jul 2, 2026
Copilot AI review requested due to automatic review settings July 2, 2026 07:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR upgrades wolfProvider’s EncryptedPrivateKeyInfo (encrypted PKCS#8) handling to be standards-compliant PBES2 (PBKDF2 + AES-CBC) and extends encrypted-PKCS#8 decode support across all key types, with shared helpers and new round-trip tests.

Changes:

  • Introduces shared PKCS#8 PBES2 encrypt/decrypt helpers (wp_encrypt_key_pkcs8*, wp_decrypt_key_pkcs8) and routes key-type encoders/decoders through them.
  • Fixes EncryptedPrivateKeyInfo PEM labeling/output sizing and removes the prior weak/incorrect encryption path.
  • Adds encrypted-PKCS#8 encode/decode round-trip tests across RSA/EC/DH/ECX/ML-DSA (with feature gating).

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/unit.h Adds WP_HAVE_EPKI_TEST feature gate and new EPKI test declarations.
test/unit.c Registers new EPKI test cases under feature gates.
test/test_rsa.c Adds RSA EncryptedPrivateKeyInfo encode/decode interop and self-roundtrip test.
test/test_pkey.c Adds shared test_epki_encode_decode() helper using OpenSSL encoder/decoder APIs.
test/test_mldsa.c Adds ML-DSA EncryptedPrivateKeyInfo self round-trip test.
test/test_ecx.c Adds ECX (Ed25519) EncryptedPrivateKeyInfo self round-trip test.
test/test_ecc.c Adds ECC EncryptedPrivateKeyInfo interop + self round-trip test.
test/test_dh.c Adds DH EncryptedPrivateKeyInfo interop + self round-trip test.
src/wp_rsa_kmgmt.c Refactors RSA encrypted PKCS#8 encode/decode onto shared PBES2 helpers and fixes PEM type handling.
src/wp_mldsa_kmgmt.c Adds encrypted-PKCS#8 decode fallback and PBES2 EncryptedPrivateKeyInfo encode path with separate ciphertext buffer.
src/wp_internal.c Implements shared PBES2 PKCS#8 encrypt/size/decrypt helpers and PBKDF2-based detection.
src/wp_ecx_kmgmt.c Adds encrypted-PKCS#8 decode fallback and PBES2 EncryptedPrivateKeyInfo encode path with separate ciphertext buffer.
src/wp_ecc_kmgmt.c Adds encrypted-PKCS#8 decode fallback and PBES2 EncryptedPrivateKeyInfo encode path using shared helpers.
src/wp_dh_kmgmt.c Adds encrypted-PKCS#8 decode fallback and PBES2 EncryptedPrivateKeyInfo encode path using shared helpers.
include/wolfprovider/internal.h Updates internal API declarations to new shared PKCS#8 encrypt/decrypt helpers.

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

Comment thread src/wp_internal.c Outdated
Comment thread src/wp_internal.c Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #426

Scan targets checked: wolfprovider-bugs, wolfprovider-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread test/test_pkey.c
@yosuke-wolfssl

Copy link
Copy Markdown
Contributor Author

PRM-master-job failure would be fixed with #425

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor Author

Hi @ColtonWilley ,
I solved the merge conflict. Could you please review this ?

@ColtonWilley ColtonWilley added the ci:all PR OSP toggle: run all label Jul 14, 2026
@ColtonWilley

Copy link
Copy Markdown
Contributor

We should consider adding this as well:

We're emitting PBES2 with the SHA-1 PRF — wc_EncryptPKCS8Key passes hmacOid=0, so PBKDF2 defaults to HMAC-SHA1 (OpenSSL defaults to SHA-256). It's interoperable and FIPS-legal, but we should match SHA-256.

Switching to wc_EncryptPKCS8Key_ex with HMAC_SHA256_OID would do it, gated on version since _ex only exists in certain versions of wolfssl.

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

Labels

ci:all PR OSP toggle: run all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants