pyca / cryptography

cryptography is a package designed to expose cryptographic primitives and recipes to Python developers.
https://cryptography.io
Other
6.66k stars 1.53k forks source link

Can we increase PKCS12 MAC iterations count? #9644

Closed intgr closed 2 months ago

intgr commented 1 year ago

Working with PKCS12 export, I noticed that PKCS12 files created by cryptography version 41.0.4 always use a MAC iteration count of 1.

Output from openssl pkcs12 -info of a file generated by cryptography:

MAC: sha256, Iteration 1
MAC length: 32, salt length: 8
PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 20000, PRF hmacWithSHA256

Code comments in cryptography suggest this is done based on guidance from OpenSSL man page, but it seems like OpenSSL itself is not following their own advice. Maybe we should follow what OpenSSL actually does, rather than what its documentation states?

Relevant code comment from cryptography:

https://github.com/pyca/cryptography/blob/fc11bce6930e591ce26a2317b31b9ce2b3e25512/src/cryptography/hazmat/backends/openssl/backend.py#L1721-L1724

Relevant quote from the OpenSSL man page:

These defaults are: 40 bit RC2 encryption for certificates, triple DES encryption for private keys, a key iteration count of PKCS12_DEFAULT_ITER (currently 2048) and a MAC iteration count of 1.

The default MAC iteration count is 1 in order to retain compatibility with old software which did not interpret MAC iteration counts. If such compatibility is not required then mac_iter should be set to PKCS12_DEFAULT_ITER.

This comment is present even in the latest OpensSSL man page. However, OpenSSL's own openssl pkcs12 -export CLI command does not follow this guidance, with or without the -legacy option...

OpenSSL CLI commands ```shell openssl version openssl pkcs12 -export -in cert.pem -inkey key.pem \ -out openssl-modern.p12 -password pass:pass openssl pkcs12 -noenc -info -passin pass:pass -in openssl-modern.p12 2>&1 |head -n3 openssl pkcs12 -export -in cert.pem -inkey key.pem \ -out openssl-legacy.p12 -password pass:pass -legacy openssl pkcs12 -noenc -info -passin pass:pass -in openssl-legacy.p12 -legacy 2>&1 |head -n3 ```

Output:

OpenSSL 3.1.2 1 Aug 2023 (Library: OpenSSL 3.1.2 1 Aug 2023)
# without -legacy
MAC: sha256, Iteration 2048
MAC length: 32, salt length: 8
PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256
# with -legacy
MAC: sha1, Iteration 2048
MAC length: 20, salt length: 8
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048

Notice they're using Iteration 2048.

Also tested this with OpenSSL 1.1.1n, which does not have the `-legacy` option, but also uses 2048 iterations ```shell openssl version openssl pkcs12 -export -in cert.pem -inkey key.pem \ -out openssl-modern.p12 -password pass:pass openssl pkcs12 -noenc -info -passin pass:pass -in openssl-modern.p12 2>&1 |head -n3 # output OpenSSL 1.1.1n 15 Mar 2022 MAC: sha1, Iteration 2048 MAC length: 20, salt length: 8 PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048 ```
intgr commented 1 year ago

The MAC iterations count should be manually configurable via the PrivateFormat.PKCS12.encryption_builder() API.

And I think the default for BestAvailableEncryption should also be changed?

reaperhulk commented 1 year ago

While I don't think this is a particularly big deal, I'm also not opposed to changing it since OpenSSL has done so. Making it configurable is also fine, although at some point soon-ish we'll be converting the PKCS12 builder and doing much of the PKCS12 structure generation via rust-asn1 (parsing will remain OpenSSL).

alex commented 1 year ago

We'd be happy to take a PR for this.

alex commented 2 months ago

We've raised this to 2048. If there's interest in making it configurable, we'd take a PR for that.

intgr commented 2 months ago

Thanks! That's an improvement.