8214100: use of keystore probing results in unnecessary exception thrown
authorweijun
Tue, 27 Nov 2018 08:51:20 +0800
changeset 52689 7084dae775f2
parent 52688 3db8758f0f79
child 52690 cecba555360c
8214100: use of keystore probing results in unnecessary exception thrown Reviewed-by: mullan
src/java.base/share/classes/java/security/KeyStore.java
src/java.base/share/classes/sun/security/tools/keytool/Main.java
test/jdk/sun/security/tools/keytool/ProbingFailure.java
test/lib/jdk/test/lib/process/ProcessTools.java
--- a/src/java.base/share/classes/java/security/KeyStore.java	Mon Nov 26 15:06:53 2018 -0800
+++ b/src/java.base/share/classes/java/security/KeyStore.java	Tue Nov 27 08:51:20 2018 +0800
@@ -1813,8 +1813,8 @@
             }
         }
 
-        throw new KeyStoreException("Unrecognized keystore format: " +
-            keystore);
+        throw new KeyStoreException("This keystore does not support probing "
+                + "and must be loaded with a specified type");
     }
 
     /**
--- a/src/java.base/share/classes/sun/security/tools/keytool/Main.java	Mon Nov 26 15:06:53 2018 -0800
+++ b/src/java.base/share/classes/sun/security/tools/keytool/Main.java	Tue Nov 27 08:51:20 2018 +0800
@@ -1328,28 +1328,39 @@
             if (f.exists()) {
                 // Probe for real type. A JKS can be loaded as PKCS12 because
                 // DualFormat support, vice versa.
-                keyStore = KeyStore.getInstance(f, pass);
-                String realType = keyStore.getType();
-                if (realType.equalsIgnoreCase("JKS")
-                        || realType.equalsIgnoreCase("JCEKS")) {
-                    boolean allCerts = true;
-                    for (String a : Collections.list(keyStore.aliases())) {
-                        if (!keyStore.entryInstanceOf(
-                                a, TrustedCertificateEntry.class)) {
-                            allCerts = false;
-                            break;
+                String realType = storetype;
+                try {
+                    keyStore = KeyStore.getInstance(f, pass);
+                    realType = keyStore.getType();
+                    if (realType.equalsIgnoreCase("JKS")
+                            || realType.equalsIgnoreCase("JCEKS")) {
+                        boolean allCerts = true;
+                        for (String a : Collections.list(keyStore.aliases())) {
+                            if (!keyStore.entryInstanceOf(
+                                    a, TrustedCertificateEntry.class)) {
+                                allCerts = false;
+                                break;
+                            }
+                        }
+                        // Don't warn for "cacerts" style keystore.
+                        if (!allCerts) {
+                            weakWarnings.add(String.format(
+                                    rb.getString("jks.storetype.warning"),
+                                    realType, ksfname));
                         }
                     }
-                    // Don't warn for "cacerts" style keystore.
-                    if (!allCerts) {
-                        weakWarnings.add(String.format(
-                                rb.getString("jks.storetype.warning"),
-                                realType, ksfname));
-                    }
+                } catch (KeyStoreException e) {
+                    // Probing not supported, therefore cannot be JKS or JCEKS.
+                    // Skip the legacy type warning at all.
                 }
                 if (inplaceImport) {
-                    String realSourceStoreType = KeyStore.getInstance(
-                            new File(inplaceBackupName), srcstorePass).getType();
+                    String realSourceStoreType = srcstoretype;
+                    try {
+                        realSourceStoreType = KeyStore.getInstance(
+                                new File(inplaceBackupName), srcstorePass).getType();
+                    } catch (KeyStoreException e) {
+                        // Probing not supported. Assuming srcstoretype.
+                    }
                     String format =
                             realType.equalsIgnoreCase(realSourceStoreType) ?
                             rb.getString("backup.keystore.warning") :
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/sun/security/tools/keytool/ProbingFailure.java	Tue Nov 27 08:51:20 2018 +0800
@@ -0,0 +1,258 @@
+/*
+ * Copyright (c) 2018, 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 8214100
+ * @summary use of keystore probing results in unnecessary exception thrown
+ * @library /test/lib
+ * @compile -XDignore.symbol.file ProbingFailure.java
+ * @run main ProbingFailure
+ */
+
+import jdk.test.lib.SecurityTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.Key;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.KeyStoreSpi;
+import java.security.NoSuchAlgorithmException;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateException;
+import java.util.Date;
+import java.util.Enumeration;
+
+public class ProbingFailure {
+
+    public static void main(String[] args) throws Exception {
+
+        // genkeypair
+        kt("-genkeypair -keystore mks -alias a -dname CN=A -storetype MYKS")
+                .shouldHaveExitValue(0);
+
+        // list
+        kt("-list -keystore mks -storetype MYKS")
+                .shouldHaveExitValue(0);
+
+        kt("-list -keystore mks")
+                .shouldHaveExitValue(1)
+                .shouldContain("This keystore does not support probing");
+
+        // importkeystore
+        kt("-importkeystore -srckeystore mks -srcstoretype MYKS -destkeystore p12")
+                .shouldHaveExitValue(0);
+
+        kt("-importkeystore -srckeystore mks -destkeystore p12a")
+                .shouldHaveExitValue(1)
+                .shouldContain("This keystore does not support probing");
+
+        // in-place importkeystore
+        kt("-importkeystore -srckeystore mks -srcstoretype MYKS -destkeystore mks -deststoretype myks")
+                .shouldContain("The original keystore \"mks\" is backed up")
+                .shouldHaveExitValue(0);
+
+        kt("-importkeystore -srckeystore mks -srcstoretype MYKS -destkeystore mks")
+                .shouldContain("Migrated \"mks\" to PKCS12")
+                .shouldHaveExitValue(0);
+
+        kt("-importkeystore -srckeystore p12 -destkeystore p12 -deststoretype MYKS")
+                .shouldContain("Migrated \"p12\" to MYKS")
+                .shouldHaveExitValue(0);
+    }
+
+    static OutputAnalyzer kt(String cmd) throws Exception {
+        return SecurityTools.keytool(
+                "-storepass changeit -keypass changeit -debug "
+                + "-srcstorepass changeit -deststorepass changeit "
+                + "-providerclass ProbingFailure$MyProvider "
+                + "-providerpath " + System.getProperty("test.classes")
+                + " " + cmd);
+    }
+
+    public static class MyProvider extends Provider {
+        public MyProvider() {
+            super("MP", "1.0", "My Provider");
+            put("KeyStore.MYKS", "ProbingFailure$MyKS");
+        }
+    }
+
+    // The MYKS keystore prepends a zero byte before a PKCS12 file
+    // and does not support probing.
+    public static class MyKS extends KeyStoreSpi {
+
+        KeyStore ks;
+
+        public MyKS() {
+            try {
+                ks = KeyStore.getInstance("PKCS12");
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public Key engineGetKey(String alias, char[] password)
+                throws NoSuchAlgorithmException, UnrecoverableKeyException {
+            try {
+                return ks.getKey(alias, password);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public Certificate[] engineGetCertificateChain(String alias) {
+            try {
+                return ks.getCertificateChain(alias);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public Certificate engineGetCertificate(String alias) {
+            try {
+                return ks.getCertificate(alias);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public Date engineGetCreationDate(String alias) {
+            try {
+                return ks.getCreationDate(alias);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public void engineSetKeyEntry(String alias, Key key, char[] password,
+                Certificate[] chain) throws KeyStoreException {
+            ks.setKeyEntry(alias, key, password, chain);
+        }
+
+        @Override
+        public void engineSetKeyEntry(String alias, byte[] key,
+                Certificate[] chain) throws KeyStoreException {
+            ks.setKeyEntry(alias, key, chain);
+        }
+
+        @Override
+        public void engineSetCertificateEntry(String alias, Certificate cert)
+                throws KeyStoreException {
+            ks.setCertificateEntry(alias, cert);
+        }
+
+        @Override
+        public void engineDeleteEntry(String alias) throws KeyStoreException {
+            ks.deleteEntry(alias);
+        }
+
+        @Override
+        public Enumeration<String> engineAliases() {
+            try {
+                return ks.aliases();
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public boolean engineContainsAlias(String alias) {
+            try {
+                return ks.containsAlias(alias);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public int engineSize() {
+            try {
+                return ks.size();
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public boolean engineIsKeyEntry(String alias) {
+            try {
+                return ks.isKeyEntry(alias);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public boolean engineIsCertificateEntry(String alias) {
+            try {
+                return ks.isCertificateEntry(alias);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public String engineGetCertificateAlias(Certificate cert) {
+            try {
+                return ks.getCertificateAlias(cert);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public void engineStore(OutputStream stream, char[] password)
+                throws IOException, NoSuchAlgorithmException, CertificateException {
+            stream.write(0);
+            try {
+                ks.store(stream, password);
+            } catch (KeyStoreException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public void engineLoad(InputStream stream, char[] password)
+                throws IOException, NoSuchAlgorithmException, CertificateException {
+            if (stream != null) {
+                stream.read();
+            }
+            ks.load(stream, password);
+        }
+
+        @Override
+        public boolean engineProbe(InputStream stream) {
+            return false;
+        }
+    }
+}
--- a/test/lib/jdk/test/lib/process/ProcessTools.java	Mon Nov 26 15:06:53 2018 -0800
+++ b/test/lib/jdk/test/lib/process/ProcessTools.java	Tue Nov 27 08:51:20 2018 +0800
@@ -464,7 +464,10 @@
      */
     public static OutputAnalyzer executeCommand(ProcessBuilder pb)
             throws Throwable {
-        String cmdLine = pb.command().stream().collect(Collectors.joining(" "));
+        String cmdLine = pb.command().stream()
+                .map(x -> (x.contains(" ") || x.contains("$"))
+                        ? ("'" + x + "'") : x)
+                .collect(Collectors.joining(" "));
         System.out.println("Command line: [" + cmdLine + "]");
         OutputAnalyzer analyzer = ProcessTools.executeProcess(pb);
         System.out.println(analyzer.getOutput());