8286428: AlgorithmId should understand PBES2

Reviewed-by: valeriep

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

@@ -316,7 +316,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
 
         byte[] encryptedKey;
         AlgorithmParameters algParams;
-        ObjectIdentifier algOid;
+        AlgorithmId aid;
 
         try {
             // get the encrypted private key
@@ -327,8 +327,8 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
             // parse Algorithm parameters
             DerValue val = new DerValue(encrInfo.getAlgorithm().encode());
             DerInputStream in = val.toDerInputStream();
-            algOid = in.getOID();
+            aid = AlgorithmId.parse(val);
-            algParams = parseAlgParameters(algOid, in);
+            algParams = aid.getParameters();
 
         } catch (IOException ioe) {
             UnrecoverableKeyException uke =
@@ -360,8 +360,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
 
             key = RetryWithZero.run(pass -> {
                 // Use JCE
-                Cipher cipher = Cipher.getInstance(
+                Cipher cipher = Cipher.getInstance(aid.getName());
-                        mapPBEParamsToAlgorithm(algOid, algParams));
                 SecretKey skey = getPBEKey(pass);
                 try {
                     cipher.init(Cipher.DECRYPT_MODE, skey, algParams);
@@ -394,7 +393,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                             if (debug != null) {
                                 debug.println("Retrieved a protected private key at alias" +
                                         " '" + alias + "' (" +
-                                        mapPBEParamsToAlgorithm(algOid, algParams) +
+                                        aid.getName() +
                                         " iterations: " + ic + ")");
                             }
                             return tmp;
@@ -435,7 +434,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                             if (debug != null) {
                                 debug.println("Retrieved a protected secret key at alias " +
                                         "'" + alias + "' (" +
-                                        mapPBEParamsToAlgorithm(algOid, algParams) +
+                                        aid.getName() +
                                         " iterations: " + ic + ")");
                             }
                             return tmp;
@@ -979,18 +978,6 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
         return AlgorithmId.get(algorithm).getOID();
     }
 
-    /*
-     * Map a PBE algorithm parameters onto its algorithm name
-     */
-    private static String mapPBEParamsToAlgorithm(ObjectIdentifier algorithm,
-        AlgorithmParameters algParams) throws NoSuchAlgorithmException {
-        // Check for PBES2 algorithms
-        if (algorithm.equals(pbes2_OID) && algParams != null) {
-            return algParams.toString();
-        }
-        return new AlgorithmId(algorithm).getName();
-    }
-
     /**
      * Assigns the given certificate to the given alias.
      *
@@ -2112,9 +2099,8 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                 byte[] rawData = seq[2].getOctetString();
 
                 // parse Algorithm parameters
-                DerInputStream in = seq[1].toDerInputStream();
+                AlgorithmId aid = AlgorithmId.parse(seq[1]);
-                ObjectIdentifier algOid = in.getOID();
+                AlgorithmParameters algParams = aid.getParameters();
-                AlgorithmParameters algParams = parseAlgParameters(algOid, in);
 
                 PBEParameterSpec pbeSpec;
                 int ic = 0;
@@ -2133,23 +2119,21 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                         throw new IOException("cert PBE iteration count too large");
                     }
 
-                    certProtectionAlgorithm
+                    certProtectionAlgorithm = aid.getName();
-                            = mapPBEParamsToAlgorithm(algOid, algParams);
                     certPbeIterationCount = ic;
                     seeEncBag = true;
                 }
 
                 if (debug != null) {
                     debug.println("Loading PKCS#7 encryptedData " +
-                        "(" + mapPBEParamsToAlgorithm(algOid, algParams) +
+                        "(" + certProtectionAlgorithm +
                         " iterations: " + ic + ")");
                 }
 
                 try {
                     RetryWithZero.run(pass -> {
                         // Use JCE
-                        Cipher cipher = Cipher.getInstance(
+                        Cipher cipher = Cipher.getInstance(certProtectionAlgorithm);
-                                mapPBEParamsToAlgorithm(algOid, algParams));
                         SecretKey skey = getPBEKey(pass);
                         try {
                             cipher.init(Cipher.DECRYPT_MODE, skey, algParams);

src/java.base/share/classes/sun/security/x509/AlgorithmId.java

@@ -256,15 +256,25 @@ public class AlgorithmId implements Serializable, DerEncoder {
     }
 
     /**
-     * Returns a name for the algorithm which may be more intelligible
+     * Returns a name for the algorithm which can be used by getInstance()
+     * call of a crypto primitive. The name is usually more intelligible
      * to humans than the algorithm's OID, but which won't necessarily
      * be comprehensible on other systems.  For example, this might
      * return a name such as "MD5withRSA" for a signature algorithm on
-     * some systems.  It also returns names like "OID.1.2.3.4", when
+     * some systems.  It also returns OID names like "1.2.3.4", when
-     * no particular name for the algorithm is known.
+     * no particular name for the algorithm is known. The OID may also be
+     * recognized by getInstance() calls since an OID is usually defined
+     * as an alias for an algorithm by the security provider.
      *
-     * Note: for ecdsa-with-SHA2 plus hash algorithm (Ex: SHA-256), this method
+     * In some special cases where the OID does not include enough info
+     * to return a Java standard algorithm name, an algorithm name
+     * that includes info on the params is returned:
+     *
+     * 1. For ecdsa-with-SHA2 plus hash algorithm (Ex: SHA-256), this method
      * returns the "full" signature algorithm (Ex: SHA256withECDSA) directly.
+     *
+     * 2. For PBES2, this method returns the "full" cipher name containing the
+     * KDF and Enc algorithms (Ex: PBEWithHmacSHA256AndAES_256) directly.
      */
     public String getName() {
         String oidStr = algid.toString();
@@ -281,6 +291,14 @@ public class AlgorithmId implements Serializable, DerEncoder {
                     // ignore
                 }
             }
+        } else if (o == KnownOIDs.PBES2) {
+            if (algParams != null) {
+                return algParams.toString();
+            } else {
+                // when getName() is called in decodeParams(), algParams is
+                // null, where AlgorithmParameters.getInstance("PBES2") will
+                // be used to initialize it.
+            }
         }
         if (o != null) {
             return o.stdName();

test/jdk/sun/security/x509/AlgorithmId/PBES2.java

@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2022, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8286428
+ * @library /test/lib
+ * @modules java.base/sun.security.util
+ *          java.base/sun.security.x509
+ * @summary AlgorithmId should understand PBES2
+ */
+import jdk.test.lib.Asserts;
+import jdk.test.lib.security.DerUtils;
+import sun.security.x509.AlgorithmId;
+
+import javax.crypto.EncryptedPrivateKeyInfo;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+import javax.crypto.spec.SecretKeySpec;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.security.KeyStore;
+import java.util.Arrays;
+import java.util.Map;
+import java.util.Random;
+
+public class PBES2 {
+    public static void main(String[] args) throws Exception {
+
+        var pass = "changeit".toCharArray();
+
+        var ks = KeyStore.getInstance("pkcs12");
+        ks.load(null, null);
+
+        var bytes = new byte[16];
+        new Random().nextBytes(bytes);
+        var key = new SecretKeySpec(bytes, "AES");
+
+        var algos = Map.of(
+                "p1", "PBEWithMD5AndDES",
+                "p2", "PBEWithHmacSHA384AndAES_128",
+                "p3", "PBEWithHmacSHA256AndAES_256");
+
+        // Write 3 SecretKeyEntry objects inside the keystore
+        // PBES1
+        ks.setEntry("p1", new KeyStore.SecretKeyEntry(key), new KeyStore.PasswordProtection(pass, algos.get("p1"), null));
+        // PBES2
+        ks.setEntry("p2", new KeyStore.SecretKeyEntry(key), new KeyStore.PasswordProtection(pass, algos.get("p2"), null));
+        // default
+        ks.setKeyEntry("p3", key, pass, null);
+
+        var bout = new ByteArrayOutputStream();
+        ks.store(bout, pass);
+        var p12 = bout.toByteArray();
+
+        var decryptKey = SecretKeyFactory.getInstance("PBE").generateSecret(new PBEKeySpec(pass));
+        ks.load(new ByteArrayInputStream(p12), pass);
+        for (int i = 0; i < 3; i++) {
+            var name = DerUtils.innerDerValue(p12, "110c010c" + i + "2010").getAsString();
+
+            // AlgorithmId
+            var aid = AlgorithmId.parse(DerUtils.innerDerValue(p12, "110c010c" + i + "1010c0"));
+            Asserts.assertEQ(aid.getName(), algos.get(name), name);
+
+            // EncryptedPrivateKeyInfo
+            var encrypted = DerUtils.innerDerValue(p12, "110c010c" + i + "1010c");
+            var epi = new EncryptedPrivateKeyInfo(encrypted.toByteArray());
+            Asserts.assertEQ(epi.getAlgName(), algos.get(name));
+            var spec = epi.getKeySpec(decryptKey);
+            var specEncoded = spec.getEncoded();
+            Asserts.assertEQ(spec.getAlgorithm(), "AES", name);
+            Asserts.assertTrue(Arrays.equals(bytes, 0, 16, specEncoded, specEncoded.length - 16, specEncoded.length), name);
+
+            // KeyStore API
+            var k = ks.getKey(name, pass);
+            Asserts.assertEQ(k.getAlgorithm(), "AES", name);
+            Asserts.assertTrue(Arrays.equals(bytes, k.getEncoded()), name);
+        }
+    }
+}