8231107: Allow store password to be null when saving a PKCS12 KeyStore

Reviewed-by: mullan

src/java.base/share/classes/java/security/KeyStore.java

@@ -1384,7 +1384,9 @@ public class KeyStore {
      * integrity with the given password.
      *
      * @param stream the output stream to which this keystore is written.
-     * @param password the password to generate the keystore integrity check
+     * @param password the password to generate the keystore integrity check.
+     *                 May be {@code null} if the keystore does not support
+     *                 or require an integrity check.
      *
      * @throws    KeyStoreException if the keystore has not been initialized
      * (loaded).

src/java.base/share/classes/java/security/KeyStoreSpi.java

@@ -289,7 +289,9 @@ public abstract class KeyStoreSpi {
      * integrity with the given password.
      *
      * @param stream the output stream to which this keystore is written.
-     * @param password the password to generate the keystore integrity check
+     * @param password the password to generate the keystore integrity check.
+     *                 May be {@code null} if the keystore does not support
+     *                 or require an integrity check.
      *
      * @throws    IOException if there was an I/O problem with data
      * @throws    NoSuchAlgorithmException if the appropriate data integrity

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java

@@ -1259,14 +1259,20 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                     " certificate(s) in a PKCS#7 encryptedData");
             }
 
-            byte[] encrData = createEncryptedData(password);
+            byte[] certsData = getCertificateData();
-            if (!certProtectionAlgorithm.equalsIgnoreCase("NONE")) {
+            if (password != null && !certProtectionAlgorithm.equalsIgnoreCase("NONE")) {
+                // -- SEQUENCE of EncryptedData
+                DerOutputStream encrData = new DerOutputStream();
+                encrData.putInteger(0);
+                encrData.write(encryptContent(certsData, password));
+                DerOutputStream encrDataContent = new DerOutputStream();
+                encrDataContent.write(DerValue.tag_Sequence, encrData);
                 ContentInfo encrContentInfo =
                         new ContentInfo(ContentInfo.ENCRYPTED_DATA_OID,
-                                new DerValue(encrData));
+                                new DerValue(encrDataContent.toByteArray()));
                 encrContentInfo.encode(authSafeContentInfo);
             } else {
-                ContentInfo dataContentInfo = new ContentInfo(encrData);
+                ContentInfo dataContentInfo = new ContentInfo(certsData);
                 dataContentInfo.encode(authSafeContentInfo);
             }
         }
@@ -1289,7 +1295,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
         if (macIterationCount < 0) {
             macIterationCount = defaultMacIterationCount();
         }
-        if (!macAlgorithm.equalsIgnoreCase("NONE")) {
+        if (password != null && !macAlgorithm.equalsIgnoreCase("NONE")) {
             byte[] macData = calculateMac(password, authenticatedSafe);
             pfx.write(macData);
         }
@@ -1704,12 +1710,11 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
     }
 
     /*
-     * Create EncryptedData content type, that contains EncryptedContentInfo.
+     * Create Data content type, includes certificates in individual
-     * Includes certificates in individual SafeBags of type CertBag.
+     * SafeBags of type CertBag. Each CertBag may include pkcs12 attributes
-     * Each CertBag may include pkcs12 attributes
      * (see comments in getBagAttributes)
      */
-    private byte[] createEncryptedData(char[] password)
+    private byte[] getCertificateData()
         throws CertificateException, IOException
     {
         DerOutputStream out = new DerOutputStream();
@@ -1803,22 +1808,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
         // wrap as SequenceOf SafeBag
         DerOutputStream safeBagValue = new DerOutputStream();
         safeBagValue.write(DerValue.tag_SequenceOf, out);
-        byte[] safeBagData = safeBagValue.toByteArray();
+        return safeBagValue.toByteArray();
-
-        // encrypt the content (EncryptedContentInfo)
-        if (!certProtectionAlgorithm.equalsIgnoreCase("NONE")) {
-            byte[] encrContentInfo = encryptContent(safeBagData, password);
-
-            // -- SEQUENCE of EncryptedData
-            DerOutputStream encrData = new DerOutputStream();
-            DerOutputStream encrDataContent = new DerOutputStream();
-            encrData.putInteger(0);
-            encrData.write(encrContentInfo);
-            encrDataContent.write(DerValue.tag_Sequence, encrData);
-            return encrDataContent.toByteArray();
-        } else {
-            return safeBagData;
-        }
     }
 
     /*

test/jdk/sun/security/pkcs12/EmptyPassword.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,11 +23,11 @@
 
 /*
  * @test
- * @bug 8202299
+ * @bug 8202299 8231107
  * @modules java.base/sun.security.tools.keytool
  *          java.base/sun.security.x509
  * @library /test/lib
- * @summary Java Keystore fails to load PKCS12/PFX certificates created in WindowsServer2016
+ * @summary Testing empty (any of null, "", "\0") password behaviors
  */
 
 import jdk.test.lib.Asserts;
@@ -38,6 +38,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.security.KeyStore;
 import java.security.cert.Certificate;
+import java.util.Arrays;
 
 public class EmptyPassword {
 
@@ -52,13 +53,39 @@ public class EmptyPassword {
                 new Certificate[] {
                         gen.getSelfCertificate(new X500Name("CN=Me"), 100)
                 });
-        try (FileOutputStream fos = new FileOutputStream("p12")) {
+
-            ks.store(fos, new char[1]);
+        // 8202299: interop between new char[0] and new char[1]
-        }
+        store(ks, "p12", new char[1]);
 
         // It can be loaded with password "".
         ks = KeyStore.getInstance(new File("p12"), new char[0]);
         Asserts.assertTrue(ks.getKey("a", new char[0]) != null);
         Asserts.assertTrue(ks.getCertificate("a") != null);
+
+        ks = KeyStore.getInstance(new File("p12"), new char[1]);
+        Asserts.assertTrue(ks.getKey("a", new char[1]) != null);
+        Asserts.assertTrue(ks.getCertificate("a") != null);
+
+        // 8231107: Store with null password makes it password-less
+        store(ks, "p00", null);
+
+        // Can read cert and key with any password
+        for (char[] pass: new char[][] {
+                new char[0],    // password actually used before 8202299
+                new char[1],    // the interoperability before 8202299
+                null,           // password-less after 8202299
+                "whatever".toCharArray()
+        }) {
+            System.out.println("with password " + Arrays.toString(pass));
+            ks = KeyStore.getInstance(new File("p00"), pass);
+            Asserts.assertTrue(ks.getKey("a", new char[1]) != null);
+            Asserts.assertTrue(ks.getCertificate("a") != null);
+        }
+    }
+
+    static void store(KeyStore ks, String file, char[] pass) throws Exception {
+        try (FileOutputStream fos = new FileOutputStream(file)) {
+            ks.store(fos, pass);
+        }
     }
 }