feat(pkcs12): add RC2-CBC PBE decryption support#2284
Open
MarkAtwood wants to merge 5 commits intoRustCrypto:masterfrom
Open
feat(pkcs12): add RC2-CBC PBE decryption support#2284MarkAtwood wants to merge 5 commits intoRustCrypto:masterfrom
MarkAtwood wants to merge 5 commits intoRustCrypto:masterfrom
Conversation
Implements pbeWithSHAAnd128BitRC2-CBC (OID 1.2.840.113549.1.12.1.5) and pbeWithSHAAnd40BitRC2-CBC (OID 1.2.840.113549.1.12.1.6) as defined in RFC 7292 Appendix C. New public API (behind feature "encryption"): EncryptedPrivateKeyInfo::decrypt_rc2_128_cbc(&self, password: &str) EncryptedPrivateKeyInfo::decrypt_rc2_40_cbc(&self, password: &str) Key design decisions: - OID verified before any KDF work (fail cheap on wrong method) - MAX_ITERATIONS = 1_000_000 cap prevents DoS from crafted .p12 files - EKB is set implicitly by key length via Rc2::new_from_slice: 16-byte key -> EKB=128, 5-byte key -> EKB=40 - Zeroizing<Vec<u8>> used for key, IV, working buffer, and return value New dependencies (optional, behind "encryption" feature): sha1 = "0.11.0" cbc = "0.2.0-rc.4" <- pre-release pin, intentional monorepo release rc2 = "0.9.0" Test coverage: - RFC 2268 §5 ECB KATs 1-8 (normative RC2 spec vectors) - EKB sensitivity proof (same key, EKB=64 vs 128 -> different output) - drh-consultancy / Bouncy Castle PKCS12KDF SHA-1 vectors 1-10 - OpenSSL CBC interop vectors for both RC2-128 and RC2-40 - Cipher-layer isolation tests (KDF + cipher, no PFX parsing) - Full-stack fingerprint oracle (SHA-256 of decrypted DER vs OpenSSL + pyca) - OID mismatch rejection and cross-vendor (pyca) parse test Pre-existing issues spotted but left out of scope (tracked separately): - Pkcs12PbeParams.salt field doc says "MAC digest info" (wrong) - derive_key_bmp has truncated doc comment "Derive" - derive_key() intermediate hash state not zeroized - derive_key() returns plain Vec<u8> not Zeroizing<Vec<u8>>
- derive_key_bmp: replace truncated stub with full doc - Pkcs12PbeParams.salt: fix wrong "MAC digest info" label - lib.rs: remove four TODO comments (tracked in beads) - rustfmt: reorder imports alphabetically
- add #[deprecated] alias for renamed PKCS_12_PBEWITH_SHAAND40_BIT_RC2_CBC - clarify Zeroizing spare-capacity behavior in decrypt.rs comment - add non-BMP password rejection tests for both RC2 decrypt methods - remove redundant rc2 dev-dependency (covered by optional main dep)
Add two explanatory comments in decrypt_rc2: - Zeroizing::new() wrappers on derive_key_utf8 return values are intentional on this branch (kdf returns Vec<u8>); note they will be removed once kdf PR RustCrypto#2283 (Zeroizing<Vec<u8>> return) lands. - Salt length is bounded by DER input size; no separate allocation cap is needed.
The SHA-256 recorded for the pyca EC PrivateKeyInfo DER was wrong (c5eacb73...). Correct value confirmed by three independent sources: OpenSSL, pyca/cryptography private_bytes(DER, PKCS8, NoEncryption), and carl-wallace/pkcs12_builder — all agree on 956890dd...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
decrypt_rc2_128_cbcanddecrypt_rc2_40_cbconEncryptedPrivateKeyInfo, covering:pbeWithSHAAnd128BitRC2-CBC(OID1.2.840.113549.1.12.1.5)pbeWithSHAAnd40BitRC2-CBC(OID1.2.840.113549.1.12.1.6)support reading legacy
.p12files produced by OpenSSL's-legacyflag and similar tools.
encryptionfeature (mirrors the convention inpkcs8).Public API change
PKCS_12_PBEWITH_SHAAND40_BIT_RC2_CBC(missing underscore) is renamedto
PKCS_12_PBE_WITH_SHAAND40_BIT_RC2_CBC. The old name is kept as a#[deprecated(since = "0.2.0")]alias for backwards compatibility. Happyto remove the alias if you prefer a clean break at 0.2.0.
Dependency note
cbc = "0.2.0-rc.4"is an exact pre-release pin — intentionalcoordinated monorepo release, same as the existing
cmspin.Zeroizing note
decrypt_rc2wraps thederive_key_utf8return values inZeroizing::new()because on this branchkdf::derive_key_utf8stillreturns
Vec<u8>. Once PR #2283 lands (derive_key_utf8returningZeroizing<Vec<u8>>), those wrappers will be removed in a follow-up.Pre-existing doc bugs fixed (in scope)
lib.rswere all copy-pasted as`pbeWithSHAAnd128BitRC4`; corrected to match each constant.Pkcs12PbeParams::saltdoc was/// the MAC digest info(wrong);corrected to describe the salt.
derive_key_bmpdoc was the stub/// Derive; expanded to fulldescription with RFC reference.
lib.rsremoved (tracked as separateissues).
Pre-existing issue spotted but left out of scope
EncryptedPrivateKeyInfocarries#[allow(missing_docs)]on itsfields. Not addressed here.
Test coverage
tests/rc2_cipher.rs: RFC 2268 §5 ECB known-answer tests (vectors1–8) + EKB sensitivity proof — contractually pins the
rc2crate'seffective-key-bits behaviour before any pkcs12 code is exercised.
tests/kdf.rs: 10 drh-consultancy / Bouncy Castle SHA-1 vectors(passwords "smeg" and "queeg"), confirmed against OpenSSL 3.0.13
openssl kdf PKCS12KDF; plus RC2-specific key sizes (16 B, 5 B).tests/decrypt_rc2.rs: full integration tests —happy-path (iter=1, 2048, 100 000), wrong-password, non-BMP password,
OID-swap rejection, cipher-layer isolation vectors (both OIDs),
pyca/cryptography cross-vendor fixture, SHA-256 fingerprint oracle
(independently confirmed by OpenSSL and pyca/cryptography).
tests/data/README.md.Test plan
cargo test --features encryption,kdf -p pkcs12— 36 tests + 2 doc-testscargo hack check --feature-powerset --no-dev-deps -p pkcs12— all 10 combinationsRUSTDOCFLAGS="--cfg docsrs -D warnings" cargo +nightly doc --no-deps --all-features -p pkcs12cargo +1.85 test --features encryption,kdf -p pkcs12(MSRV)typos pkcs12/All pass locally.