Change default algorithms in PKCS12_create() and PKCS12_set_mac()

Use the modern defaults as now set in the pkcs12 app. This also allows modifying the application to not override the default values when calling the API. Fixes #14034 Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> (Merged from #14450)
openssl · Mar 10, 2021 · 762970b · 762970b
1 parent 18fdebf
commit 762970b
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 40 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -127,6 +127,16 @@ OpenSSL 3.0
 
    *Paul Dale*
 
+ * The default algorithms for pkcs12 creation with the PKCS12_create() function
+   were changed to more modern PBKDF2 and AES based algorithms. The default
+   MAC iteration count was changed to PKCS12_DEFAULT_ITER to make it equal
+   with the password-based encryption iteration count. The default digest
+   algorithm for the MAC computation was changed to SHA-256. The pkcs12
+   application now supports -legacy option that restores the previous
+   default algorithms to support interoperability with legacy systems.
+
+   *Tomáš Mráz and Sahana Prasad*
+
  * The openssl speed command does not use low-level API calls anymore. This
    implies some of the performance numbers might not be fully comparable
    with the previous releases due to higher overhead. This applies

diff --git a/apps/pkcs12.c b/apps/pkcs12.c
@@ -28,7 +28,6 @@
 #define CACERTS         0x10
 
 #define PASSWD_BUF_SIZE 2048
-#define PKCS12_DEFAULT_PBE NID_aes_256_cbc
 
 #define WARN_EXPORT(opt) \
     BIO_printf(bio_err, "Warning: -%s option ignored with -export\n", opt);
@@ -151,9 +150,10 @@ int pkcs12_main(int argc, char **argv)
     char *name = NULL, *csp_name = NULL;
     char pass[PASSWD_BUF_SIZE] = "", macpass[PASSWD_BUF_SIZE] = "";
     int export_pkcs12 = 0, options = 0, chain = 0, twopass = 0, keytype = 0, use_legacy = 0;
-    int iter = PKCS12_DEFAULT_ITER, maciter = PKCS12_DEFAULT_ITER;
-    int cert_pbe = PKCS12_DEFAULT_PBE;
-    int key_pbe = PKCS12_DEFAULT_PBE;
+    /* use library defaults for the iter, maciter, cert, and key PBE */
+    int iter = 0, maciter = 0;
+    int cert_pbe = NID_undef;
+    int key_pbe = NID_undef;
     int ret = 1, macver = 1, add_lmk = 0, private = 0;
     int noprompt = 0;
     char *passinarg = NULL, *passoutarg = NULL, *passarg = NULL;
@@ -397,13 +397,13 @@ int pkcs12_main(int argc, char **argv)
             WARN_NO_EXPORT("keyex");
         if (keytype == KEY_SIG)
             WARN_NO_EXPORT("keysig");
-        if (key_pbe != PKCS12_DEFAULT_PBE)
+        if (key_pbe != NID_undef)
             WARN_NO_EXPORT("keypbe");
-        if (cert_pbe != PKCS12_DEFAULT_PBE && cert_pbe != -1)
+        if (cert_pbe != NID_undef && cert_pbe != -1)
             WARN_NO_EXPORT("certpbe and -descert");
         if (macalg != NULL)
             WARN_NO_EXPORT("macalg");
-        if (iter != PKCS12_DEFAULT_ITER)
+        if (iter != 0)
             WARN_NO_EXPORT("iter and -noiter");
         if (maciter == 1)
             WARN_NO_EXPORT("nomaciter");
@@ -419,7 +419,7 @@ int pkcs12_main(int argc, char **argv)
             if (!app_provider_load(app_get0_libctx(), "default"))
                 goto end;
         }
-        if (cert_pbe == PKCS12_DEFAULT_PBE) {
+        if (cert_pbe == NID_undef) {
             /* Adapt default algorithm */
 #ifndef OPENSSL_NO_RC2
             cert_pbe = NID_pbe_WithSHA1And40BitRC2_CBC;
@@ -428,10 +428,12 @@ int pkcs12_main(int argc, char **argv)
 #endif
         }
 
-        if (key_pbe == PKCS12_DEFAULT_PBE)
+        if (key_pbe == NID_undef)
             key_pbe = NID_pbe_WithSHA1And3_Key_TripleDES_CBC;
         if (enc == default_enc)
             enc = EVP_des_ede3_cbc();
+        if (macalg == NULL)
+            macalg = "sha1";
     }
 
 

diff --git a/crypto/pkcs12/p12_crt.c b/crypto/pkcs12/p12_crt.c
@@ -41,18 +41,14 @@ PKCS12 *PKCS12_create(const char *pass, const char *name, EVP_PKEY *pkey, X509 *
     unsigned int keyidlen = 0;
 
     /* Set defaults */
-    if (!nid_cert)
-#ifdef OPENSSL_NO_RC2
-        nid_cert = NID_pbe_WithSHA1And3_Key_TripleDES_CBC;
-#else
-        nid_cert = NID_pbe_WithSHA1And40BitRC2_CBC;
-#endif
-    if (!nid_key)
-        nid_key = NID_pbe_WithSHA1And3_Key_TripleDES_CBC;
+    if (nid_cert == NID_undef)
+        nid_cert = NID_aes_256_cbc;
+    if (nid_key == NID_undef)
+        nid_key = NID_aes_256_cbc;
     if (!iter)
         iter = PKCS12_DEFAULT_ITER;
     if (!mac_iter)
-        mac_iter = 1;
+        mac_iter = PKCS12_DEFAULT_ITER;
 
     if (pkey == NULL && cert == NULL && ca == NULL) {
         ERR_raise(ERR_LIB_PKCS12, PKCS12_R_INVALID_NULL_ARGUMENT);

diff --git a/crypto/pkcs12/p12_mutl.c b/crypto/pkcs12/p12_mutl.c
@@ -186,8 +186,11 @@ int PKCS12_set_mac(PKCS12 *p12, const char *pass, int passlen,
     unsigned int maclen;
     ASN1_OCTET_STRING *macoct;
 
-    if (!md_type)
-        md_type = EVP_sha1();
+    if (md_type == NULL)
+        /* No need to do a fetch as the md_type is used only to get a NID */
+        md_type = EVP_sha256();
+    if (!iter)
+        iter = PKCS12_DEFAULT_ITER;
     if (PKCS12_setup_mac(p12, iter, salt, saltlen, md_type) == PKCS12_ERROR) {
         ERR_raise(ERR_LIB_PKCS12, PKCS12_R_MAC_SETUP_ERROR);
         return 0;

diff --git a/doc/man3/PKCS12_create.pod b/doc/man3/PKCS12_create.pod
@@ -16,50 +16,47 @@ PKCS12_create - create a PKCS#12 structure
 
 PKCS12_create() creates a PKCS#12 structure.
 
-B<pass> is the passphrase to use. B<name> is the B<friendlyName> to use for
-the supplied certificate and key. B<pkey> is the private key to include in
-the structure and B<cert> its corresponding certificates. B<ca>, if not B<NULL>
+I<pass> is the passphrase to use. I<name> is the B<friendlyName> to use for
+the supplied certificate and key. I<pkey> is the private key to include in
+the structure and I<cert> its corresponding certificates. I<ca>, if not NULL
 is an optional set of certificates to also include in the structure.
 
-B<nid_key> and B<nid_cert> are the encryption algorithms that should be used
+I<nid_key> and I<nid_cert> are the encryption algorithms that should be used
 for the key and certificate respectively. The modes
-GCM, CCM, XTS, and OCB are unsupported. B<iter> is the encryption algorithm
-iteration count to use and B<mac_iter> is the MAC iteration count to use.
-B<keytype> is the type of key.
+GCM, CCM, XTS, and OCB are unsupported. I<iter> is the encryption algorithm
+iteration count to use and I<mac_iter> is the MAC iteration count to use.
+I<keytype> is the type of key.
 
 =head1 NOTES
 
-The parameters B<nid_key>, B<nid_cert>, B<iter>, B<mac_iter> and B<keytype>
+The parameters I<nid_key>, I<nid_cert>, I<iter>, I<mac_iter> and I<keytype>
 can all be set to zero and sensible defaults will be used.
 
-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.
+These defaults are: AES password based encryption (PBES2 with PBKDF2 and
+AES-256-CBC) for private keys and certificates, the PBKDF2 and MAC key
+derivation iteration count of B<PKCS12_DEFAULT_ITER> (currently 2048), and
+MAC algorithm HMAC with SHA2-256.
 
-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 B<mac_iter> should be set to PKCS12_DEFAULT_ITER.
-
-B<keytype> adds a flag to the store private key. This is a non standard extension
+I<keytype> adds a flag to the store private key. This is a non standard extension
 that is only currently interpreted by MSIE. If set to zero the flag is omitted,
 if set to B<KEY_SIG> the key can be used for signing only, if set to B<KEY_EX>
 it can be used for signing and encryption. This option was useful for old
 export grade software which could use signing only keys of arbitrary size but
 had restrictions on the permissible sizes of keys which could be used for
 encryption.
 
-If a certificate contains an B<alias> or B<keyid> then this will be
+If a certificate contains an I<alias> or I<keyid> then this will be
 used for the corresponding B<friendlyName> or B<localKeyID> in the
 PKCS12 structure.
 
-Either B<pkey>, B<cert> or both can be B<NULL> to indicate that no key or
+Either I<pkey>, I<cert> or both can be NULL to indicate that no key or
 certificate is required. In previous versions both had to be present or
 a fatal error is returned.
 
-B<nid_key> or B<nid_cert> can be set to -1 indicating that no encryption
+I<nid_key> or I<nid_cert> can be set to -1 indicating that no encryption
 should be used.
 
-B<mac_iter> can be set to -1 and the MAC will then be omitted entirely.
+I<mac_iter> can be set to -1 and the MAC will then be omitted entirely.
 
 PKCS12_create() makes assumptions regarding the encoding of the given pass
 phrase.
@@ -74,6 +71,12 @@ PKCS12_create() returns a valid B<PKCS12> structure or NULL if an error occurred
 L<d2i_PKCS12(3)>,
 L<passphrase-encoding(7)>
 
+=head1 HISTORY
+
+The defaults for encryption algorithms, MAC algorithm, and the MAC key
+derivation iteration count were changed in OpenSSL 3.0 to more modern
+standards.
+
 =head1 COPYRIGHT
 
 Copyright 2002-2018 The OpenSSL Project Authors. All Rights Reserved.