Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can we increase PKCS12 MAC iterations count? #9644

Open
intgr opened this issue Sep 22, 2023 · 3 comments
Open

Can we increase PKCS12 MAC iterations count? #9644

intgr opened this issue Sep 22, 2023 · 3 comments

Comments

@intgr
Copy link
Contributor

intgr commented Sep 22, 2023

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:

# mac_iter chosen for compatibility reasons, see:
# https://www.openssl.org/docs/man1.1.1/man3/PKCS12_create.html
# Did we mention how lousy PKCS12 encryption is?
mac_iter = 1

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
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
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
Copy link
Contributor Author

intgr commented Sep 22, 2023

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
Copy link
Member

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
Copy link
Member

alex commented Nov 1, 2023

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

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

No branches or pull requests

3 participants