1694689 - Firefox should use optionally allow the use of modern algorithms in PKCS#12 files by default

Status: RESOLVED FIXED

(Core :: Security: PSM, task, P5)

Description

[Jan Horak [:jhorak]]

[Kind of lost about component, please check if you know more]
I was asked to implement the usage of newer encryption methods for the PKCS#12 export to file:
https://bugzilla.redhat.com/show_bug.cgi?id=1764205

From the report: The file should use AES-128-CBC with PBKDF2 and SHA-256.

Reproduction:

1. Preferences/View certificates/Your Certificates/Backup...

Expected to get similar ouput:

$ openssl pkcs12 -passin pass: -passout pass: -in exported_cert.p12 -out /dev/null -info -noout
MAC: sha256, Iteration 600000
MAC length: 32, salt length: 8
PKCS7 Encrypted data: PBES2, PBKDF2, AES-128-CBC, Iteration 600000, PRF hmacWithSHA256
Certificate bag
PKCS7 Data
Shrouded Keybag: PBES2, PBKDF2, AES-128-CBC, Iteration 600000, PRF hmacWithSHA256

Comment 1

[Julien Cristau [:jcristau] (back August 18)]

AFAICT this lives in nsPKCS12Blob::ExportToFile in security/manager/ssl/nsPKCS12Blob.cpp

Comment 2

[Ryan Sleevi]

Note: There are interop concerns with other products regarding changing the default. That doesn't mean it shouldn't be done, but is something to be aware of and communicate intentionality.

e.g. Windows intentionally calls it "not backwards compatible" (because it generally doesn't work pre-Win 10), as covered at https://twitter.com/rmhrisk/status/1115029625296416769 and https://unmitigatedrisk.com/?p=543

Comment 3

[Robert Relyea]

It should be done slowly, but AES has been supported for a while now. We should check with several existing uses to make sure they work. We doe need to start moving off of older algorithms, though.

bob

Comment 4

[Jan Horak [:jhorak]]

Attachment: pkcs12-aes3.patch

Adding functional patch for feedback, do you think it's ready for review?

Comment 5

[Robert Relyea]

It looks good, the only thing I would do would check the firefox sources and see where they may already be calling SEC_PKCS12EnableCipher. It might be in the prefs system. I'm pretty sure they must already be calling it, otherwise importing 3DES encrypted keys would fail.

Comment 6

[Robert Relyea]

Comment on attachment 9211036 [details] [diff] [review]
pkcs12-aes3.patch

Review of attachment 9211036 [details] [diff] [review]:
-----------------------------------------------------------------

feedback was given in comment 5, clearing the feedback flag.

Comment 7

[Jan Horak [:jhorak]]

Hm, it seems that SEC_PKCS12EnableCipher is no longer needed for the nightly: https://searchfox.org/mozilla-central/source/security/manager/ssl/nsNSSComponent.cpp#2735. So it seems that the changes for import is not needed on nightly. For ESR I need to do that change: https://searchfox.org/mozilla-esr78/source/security/manager/ssl/nsNSSComponent.cpp#2634

Comment 8

[Jan Horak [:jhorak]]

Attachment: Bug 1694689 Use AES-128-CBC with PBKDF2 and SHA-256 for the exported certificates; r=rrelyea

Comment 9

[Jan Horak [:jhorak]]

Dana, could you please give some feedback to: https://phabricator.services.mozilla.com/D110220#3615919 and Bob's comment. Thanks.

Comment 10

[Dana Keeler (she/her) [:keeler]]

Responded in phabricator.

Comment 11

[Jan Horak [:jhorak]]

Attachment: Bug 1694689 Add userpref to use AES-128-CBC and AES_256_CBC for the exported certificates; r=keeler

Set the 'security.pki.export-cert-with-new-crypto' preference to true if user wants to backup
own certificates to the file using AES-128-CBC and SHA-256 algorithms.

Comment 12

[Jan Horak [:jhorak]]

Attachment: Bug 1694689 Use own implementation of NSS ASCIIConversionFunction to allow passing passwords in utf8; r=keeler

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2c342d57939b
Add userpref to use AES-128-CBC and AES_256_CBC for the exported certificates; r=keeler
https://hg.mozilla.org/integration/autoland/rev/b37e735f374f
Use own implementation of NSS ASCIIConversionFunction to allow passing passwords in utf8; r=keeler

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2c342d57939b
https://hg.mozilla.org/mozilla-central/rev/b37e735f374f

Comment 16

[Kai Engert [:KaiE:]]

The work here didn't change the default, but introduced the option to use modern algorithms.

I want to change the bug subject to reflect that, and will file a separate bug to request changing the default.