cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

AES256 support for PKCS#12

Open jobec opened this issue 2 years ago • 14 comments

I came across this piece of code in the openssl backend: https://github.com/pyca/cryptography/blob/4a4f4d94ce5a641de3020042c70c1734af265d5e/src/cryptography/hazmat/backends/openssl/backend.py#L2194-L2202

It's part of what gets called when creating a PKXS#12 file and It uses 3DES for the encryption.

Is there any way to switch this to something like AES256?

When exporting to a PFX in windows 10, you can do this as mentioned here

Also, you can make such PFX file though openSSL as mentioned here (and here for v1.1.1):

C:\>openssl pkcs12 -export -in cert.pem-inkey private.key -out some.pfx -certpbe AES-256-CBC -keypbe AES-256-CBC
Enter pass phrase for key_private_pem.key:
Enter Export Password:
Verifying - Enter Export Password:

C:\>openssl pkcs12 -noout -info -in some.pfx
Enter Import Password:
MAC: sha1, Iteration 2048
MAC length: 20, salt length: 8
PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256
Certificate bag
PKCS7 Data
Shrouded Keybag: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256

jobec avatar Apr 05 '22 14:04 jobec

If I locally patch Cryptography and change these lines https://github.com/pyca/cryptography/blob/4a4f4d94ce5a641de3020042c70c1734af265d5e/src/cryptography/hazmat/backends/openssl/backend.py#L2196-L2199 into

 nid_cert = 427 
 nid_key = 427 
 # At least we can set this higher than OpenSSL's default 
 pkcs12_iter = 2048

(427 being the value of NID_aes_256_cbc) The created PKCS#12 PFX file is AES-256-CBC encrypted:

c:\>openssl pkcs12 -noout -info -in some.pfx
Enter Import Password:
MAC: sha1, Iteration 1
MAC length: 20, salt length: 8
PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256
Certificate bag
Certificate bag
Certificate bag
PKCS7 Data
Shrouded Keybag: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256

And Windows seems to load and import it into it's certificate store just fine.

jobec avatar Apr 05 '22 18:04 jobec

@reaperhulk is there any reason not to move to AES for PKCS#12? I assume one concern is compatibility, but with what?

alex avatar Apr 05 '22 22:04 alex

We originally chose this because PKCS12 is trash and trying to use AES resulted in lots of things being unable to read the files we created. However, OpenSSL 3.0 defaults to NID_aes_256_cbc so it seems likely we could make this change as well. Changelog entry from OpenSSL:

pkcs12 now uses defaults of PBKDF2, AES and SHA-256, with a MAC iteration count of PKCS12_DEFAULT_ITER.

reaperhulk avatar Apr 05 '22 23:04 reaperhulk

Let's do it!

On Tue, Apr 5, 2022 at 7:58 PM Paul Kehrer @.***> wrote:

We originally chose this because PKCS12 is trash and trying to use AES resulted in lots of things being unable to read the files we created. However, OpenSSL 3.0 defaults to NID_aes_256_cbc so it seems likely we could make this change as well. Changelog entry from OpenSSL:

pkcs12 now uses defaults of PBKDF2, AES and SHA-256, with a MAC iteration count of PKCS12_DEFAULT_ITER.

— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/issues/7043#issuecomment-1089544992, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBD74LKXQCG4DERODQ3VDTHSXANCNFSM5STAZ6QA . You are receiving this because you commented.Message ID: @.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

alex avatar Apr 06 '22 00:04 alex

We originally chose this because PKCS12 is trash and trying to use AES resulted in lots of things being unable to read the files we created. However, OpenSSL 3.0 defaults to NID_aes_256_cbc so it seems likely we could make this change as well. Changelog entry from OpenSSL:

pkcs12 now uses defaults of PBKDF2, AES and SHA-256, with a MAC iteration count of PKCS12_DEFAULT_ITER.

Regarding compatibility: It might be an idea to let people select 3DES still. For example NoEncryption , LegacyEncryption and BestAvailableEncryption

I can also send in a merge request with the things above changed. But I have no experience with the rest of Cryptography's code base and certainly not the Rust/C part of it.

So, happy to send in a merge request but not sure if I can finish it 'till the end.

jobec avatar Apr 06 '22 05:04 jobec

I've also done some tests regarding compatibility with AES-256-CBC encrypted PKCS12 files:

  1. ❌ Windows server 2016
  2. ✔ Windows Server 2019
  3. ✔ Windows 10
  4. ❌ keytool of Java 1.8.0 (ref: https://bugs.openjdk.java.net/browse/JDK-8153005)
  5. ✔ keytool of Java 11.0.14 (ref: https://bugs.openjdk.java.net/browse/JDK-8153005)
  6. ✔ F5 LTM

jobec avatar Apr 06 '22 06:04 jobec

Thanks for doing those tests, that's really helpful.

I think the path forward here is implementing a new, more flexible encryption option that lets you pick TDES or AES and then maybe have a one release window where we warn people that the encryption default will change in the next release for BestAvailable. LegacyEncryption is tempting due to simplicity, but not flexible enough IMO.

What that API should look like is unclear to me though. There was a past experiment with expanding what PKCS8 encryption could do (https://github.com/pyca/cryptography/pull/5656) but it never defined a public API.

reaperhulk avatar Apr 08 '22 03:04 reaperhulk

While you are at it, please consider also adding support for selecting the MAC algorithm, which is done like so on the openssl command line:

openssl pkcs12 -export -macalg SHA256 -certpbe AES-256-CBC -keypbe AES-256-CBC -in cert.cer -inkey private.key -certfile ca_chain -out out.p12

I expected I would be able to implement a subclass to KeySerializationEncryption to which I could provide the algorithms from the openssl command line and hand that subclass instance to pkcs12.serialize_key_and_certificate but, alas, no.

Using the above command line gives the following info:

% openssl pkcs12 -in out.p12 -passin pass:password -noout -info
 MAC: sha256, Iteration 2048
 MAC length: 32, salt length: 8
 PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256
 ...

AHalvar avatar Apr 09 '22 17:04 AHalvar

It seems like @AHalvar 's remark about being able to set the MAC algo is an important one.

With v37.0's move to OpenSSL 3, it also changed to it's defaults. So now the PKCS12 functions create files with 3DES encryption but with a SHA256 MAC.

Guess what... Windows 2016 server cannot read such files 🤦 .it must have a MAC of SHA1.

So, it has become impossible as of version 37 to create windows 2016 and older compatible PFX files.

I tried adding this code to the serialize_key_and_certificates_to_pkcs12 function, but seems PKCS12_set_mac isn't exposed.

from cryptography.hazmat.primitives.hashes import SHA1
...
md = self._evp_md_non_null_from_algorithm(SHA1)
self._lib.PKCS12_set_mac(
    p12, password_buf, -1, self._ffi.NULL, 0, mac_iter, md
)

jobec avatar May 09 '22 14:05 jobec

Server 2016 should be ashamed of itself, but here we are.

There's some work going on in https://github.com/pyca/cryptography/pull/7178 but we need to land some new features in boringssl for that to merge. It does not currently do anything with the MAC, but it would appear it needs to.

reaperhulk avatar May 09 '22 15:05 reaperhulk

Would it be possible to expose PKCS12_set_mac in a next release? Then I can try myself to override a few things.

I tried compiling on windows but I can't manage to get OpenSSL bindings to work. Keeps complaining about DLLs that aren't found and the _openssl.pyd file is also just a couple of hundred KB.

jobec avatar May 12 '22 08:05 jobec

Adding that binding is fine, although it's not available in all versions we support so the standard set of conditionals will have to be used.

reaperhulk avatar May 12 '22 09:05 reaperhulk

@reaperhulk any chance for the extra binding to be available in a release any time soon?

I see it didn't got included in 37.0.3

I now need to tell users to get OpenSSL installed and then to run a bunch of cryptic openSSL commands to create usable PKCS#12 files.

jobec avatar Jun 27 '22 09:06 jobec

It will be in 38.0, but I also intend to diagnose and fix this generally before we release 38.0. We could potentially backport for 37.0.4 as well, although we’re waiting on openssl for 3.0.5 there.

reaperhulk avatar Jun 27 '22 10:06 reaperhulk