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 https://github.com/openssl/openssl/pull/14450)

5 files changed, 54 insertions, 40 deletions

diff --git a/CHANGES.md b/CHANGES.md
index 0eaeba02af..b5b9583287 100644
--- 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
index bd87fd4920..e3f22c30ed 100644
--- 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
index 9bc53f789b..985b458cda 100644
--- 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
index 4873d43e24..20984055df 100644
--- 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
index 58e1437bc2..994ff9f9e3 100644
--- a/doc/man3/PKCS12_create.pod
+++ b/doc/man3/PKCS12_create.pod
@@ -16,31 +16,28 @@ 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
@@ -48,18 +45,18 @@ 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.