8007934: algorithm parameters for PBE Scheme 2 not decoded correctly in PKCS12 keystore

Reviewed-by: mullan

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

@@ -326,7 +326,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
             DerValue val = new DerValue(encrInfo.getAlgorithm().encode());
             DerInputStream in = val.toDerInputStream();
             algOid = in.getOID();
-            algParams = parseAlgParameters(in);
+            algParams = parseAlgParameters(algOid, in);
 
         } catch (IOException ioe) {
             UnrecoverableKeyException uke =
@@ -342,7 +342,8 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                 try {
                     // Use JCE
                     SecretKey skey = getPBEKey(password);
-                    Cipher cipher = Cipher.getInstance(algOid.toString());
+                    Cipher cipher = Cipher.getInstance(
+                        mapPBEParamsToAlgorithm(algOid, algParams));
                     cipher.init(Cipher.DECRYPT_MODE, skey, algParams);
                     keyInfo = cipher.doFinal(encryptedKey);
                     break;
@@ -759,8 +760,8 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
     /*
      * parse Algorithm Parameters
      */
-    private AlgorithmParameters parseAlgParameters(DerInputStream in)
-        throws IOException
+    private AlgorithmParameters parseAlgParameters(ObjectIdentifier algorithm,
+        DerInputStream in) throws IOException
     {
         AlgorithmParameters algParams = null;
         try {
@@ -774,7 +775,11 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                 }
             }
             if (params != null) {
+                if (algorithm.equals(pbes2_OID)) {
+                    algParams = AlgorithmParameters.getInstance("PBES2");
+                } else {
                     algParams = AlgorithmParameters.getInstance("PBE");
+                }
                 algParams.init(params.toByteArray());
             }
         } catch (Exception e) {
@@ -834,13 +839,6 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                 } else {
                     algParams = getAlgorithmParameters(algorithm);
                 }
-                ObjectIdentifier pbeOID = mapPBEAlgorithmToOID(algorithm);
-                if (pbeOID != null) {
-                    algid = new AlgorithmId(pbeOID, algParams);
-                } else {
-                    throw new IOException("PBE algorithm '" + algorithm +
-                        " 'is not supported for key entry protection");
-                }
             } else {
                 // Check default key protection algorithm for PKCS12 keystores
                 algorithm = AccessController.doPrivileged(
@@ -856,12 +854,16 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                             return prop;
                         }
                     });
-                if (algorithm == null) {
+                if (algorithm == null || algorithm.isEmpty()) {
                     algorithm = "PBEWithSHA1AndDESede";
                 }
                 algParams = getAlgorithmParameters(algorithm);
-                algid = new AlgorithmId(pbeWithSHAAnd3KeyTripleDESCBC_OID,
-                    algParams);
+            }
+
+            ObjectIdentifier pbeOID = mapPBEAlgorithmToOID(algorithm);
+            if (pbeOID == null) {
+                    throw new IOException("PBE algorithm '" + algorithm +
+                        " 'is not supported for key entry protection");
             }
 
             // Use JCE
@@ -869,6 +871,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
             Cipher cipher = Cipher.getInstance(algorithm);
             cipher.init(Cipher.ENCRYPT_MODE, skey, algParams);
             byte[] encryptedKey = cipher.doFinal(data);
+            algid = new AlgorithmId(pbeOID, cipher.getParameters());
 
             if (debug != null) {
                 debug.println("  (Cipher algorithm: " + cipher.getAlgorithm() +
@@ -894,7 +897,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
     /*
      * Map a PBE algorithm name onto its object identifier
      */
-    private ObjectIdentifier mapPBEAlgorithmToOID(String algorithm)
+    private static ObjectIdentifier mapPBEAlgorithmToOID(String algorithm)
         throws NoSuchAlgorithmException {
         // Check for PBES2 algorithms
         if (algorithm.toLowerCase().startsWith("pbewithhmacsha")) {
@@ -903,6 +906,18 @@ 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 algorithm.toString();
+    }
+
     /**
      * Assigns the given certificate to the given alias.
      *
@@ -1933,7 +1948,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi {
                 // parse Algorithm parameters
                 DerInputStream in = seq[1].toDerInputStream();
                 ObjectIdentifier algOid = in.getOID();
-                AlgorithmParameters algParams = parseAlgParameters(in);
+                AlgorithmParameters algParams = parseAlgParameters(algOid, in);
 
                 while (true) {
                     try {

test/java/security/KeyStore/PBETest.java

@@ -29,6 +29,7 @@
 
 import java.io.*;
 import java.security.*;
+import javax.crypto.*;
 import javax.crypto.spec.*;
 
 // Retrieve a keystore entry, protected by the default encryption algorithm.
@@ -36,13 +37,20 @@ import javax.crypto.spec.*;
 
 public class PBETest {
     private final static String DIR = System.getProperty("test.src", ".");
-    //private static final String PBE_ALGO = "PBEWithHmacSHA1AndAES_128";
-    private static final String PBE_ALGO = "PBEWithSHA1AndDESede";
+    private final static String KEY_PROTECTION_PROPERTY =
+        "keystore.PKCS12.keyProtectionAlgorithm";
+    private static final String[] PBE_ALGOS = {
+        "PBEWithSHA1AndDESede",
+        "PBEWithHmacSHA1AndAES_128",
+        "PBEWithHmacSHA224AndAES_128",
+        "PBEWithHmacSHA256AndAES_128",
+        "PBEWithHmacSHA384AndAES_128",
+        "PBEWithHmacSHA512AndAES_128"
+    };
     private static final char[] PASSWORD = "passphrase".toCharArray();
     private static final String KEYSTORE_TYPE = "JKS";
     private static final String KEYSTORE = DIR + "/keystore.jks";
     private static final String NEW_KEYSTORE_TYPE = "PKCS12";
-    private static final String NEW_KEYSTORE = PBE_ALGO + ".p12";
     private static final String ALIAS = "vajra";
 
     private static final byte[] IV = {
@@ -55,32 +63,87 @@ public class PBETest {
     private static final int ITERATION_COUNT = 1024;
 
     public static void main(String[] args) throws Exception {
+        for (String algo : PBE_ALGOS) {
+            String filename = algo + ".p12";
+            main0(algo, filename, true);
+            main0(algo, filename, false);
+            Security.setProperty(KEY_PROTECTION_PROPERTY, algo);
+            main0(null, "PBE.p12", false);
+            Security.setProperty(KEY_PROTECTION_PROPERTY, "");
+        }
+        main0(null, "default.p12", false); // default algorithm
+    }
 
-        new File(NEW_KEYSTORE).delete();
+    private static void main0(String algo, String filename, boolean useParams)
+        throws Exception {
 
         KeyStore keystore = load(KEYSTORE_TYPE, KEYSTORE, PASSWORD);
         KeyStore.Entry entry =
             keystore.getEntry(ALIAS,
                 new KeyStore.PasswordProtection(PASSWORD));
-        System.out.println("Retrieved entry named '" + ALIAS + "'");
+        System.out.println("Retrieved key entry named '" + ALIAS + "'");
+        Key originalKey = null;
+        if (entry instanceof KeyStore.PrivateKeyEntry) {
+            originalKey = ((KeyStore.PrivateKeyEntry) entry).getPrivateKey();
+        } else if (entry instanceof KeyStore.SecretKeyEntry) {
+            originalKey = ((KeyStore.SecretKeyEntry) entry).getSecretKey();
+        }
 
         // Set entry
         KeyStore keystore2 = load(NEW_KEYSTORE_TYPE, null, null);
+        if (useParams) {
             keystore2.setEntry(ALIAS, entry,
-            new KeyStore.PasswordProtection(PASSWORD, PBE_ALGO,
+                new KeyStore.PasswordProtection(PASSWORD, algo,
                     new PBEParameterSpec(SALT, ITERATION_COUNT,
                         new IvParameterSpec(IV))));
-        System.out.println("Encrypted entry using: " + PBE_ALGO);
+            System.out.println("Encrypted key entry using: " + algo +
+                " (with PBE parameters)");
+        } else if (algo != null) {
+            keystore2.setEntry(ALIAS, entry,
+                new KeyStore.PasswordProtection(PASSWORD, algo, null));
+            System.out.println("Encrypted key entry using: " + algo +
+                " (without PBE parameters)");
+        } else {
+            keystore2.setEntry(ALIAS, entry,
+                new KeyStore.PasswordProtection(PASSWORD));
+            String prop = Security.getProperty(KEY_PROTECTION_PROPERTY);
+            if (prop == null || prop.isEmpty()) {
+                System.out.println("Encrypted key entry using: " +
+                    "default PKCS12 key protection algorithm");
+            } else {
+                System.out.println("Encrypted key entry using: " +
+                    "keyProtectionAlgorithm=" + prop);
+            }
+        }
 
-        try (FileOutputStream outStream = new FileOutputStream(NEW_KEYSTORE)) {
-            System.out.println("Storing keystore to: " + NEW_KEYSTORE);
+        try (FileOutputStream outStream = new FileOutputStream(filename)) {
+            System.out.println("Storing keystore to: " + filename);
             keystore2.store(outStream, PASSWORD);
         }
 
-        keystore2 = load(NEW_KEYSTORE_TYPE, NEW_KEYSTORE, PASSWORD);
+        try {
+            keystore2 = load(NEW_KEYSTORE_TYPE, filename, PASSWORD);
             entry = keystore2.getEntry(ALIAS,
                 new KeyStore.PasswordProtection(PASSWORD));
-        System.out.println("Retrieved entry named '" + ALIAS + "'");
+            Key key;
+            if (entry instanceof KeyStore.PrivateKeyEntry) {
+                key = ((KeyStore.PrivateKeyEntry) entry).getPrivateKey();
+            } else if (entry instanceof KeyStore.SecretKeyEntry) {
+                key = ((KeyStore.SecretKeyEntry) entry).getSecretKey();
+            } else {
+                throw new Exception("Failed to retrieve key entry");
+            }
+            if (originalKey.equals(key)) {
+                System.out.println("Retrieved key entry named '" + ALIAS + "'");
+                System.out.println();
+            } else {
+                throw new Exception(
+                    "Failed: recovered key does not match the original key");
+            }
+
+        } finally {
+            new File(filename).delete();
+        }
     }
 
     private static KeyStore load(String type, String path, char[] password)