6958869: regression: PKIXValidator fails when multiple trust anchors have same dn

Reviewed-by: xuelei, wetmore, mullan

jdk/src/share/classes/sun/security/validator/PKIXValidator.java

@@ -53,7 +53,7 @@ public final class PKIXValidator extends Validator {
     private int certPathLength = -1;
 
     // needed only for the validator
-    private Map<X500Principal, X509Certificate> trustedSubjects;
+    private Map<X500Principal, List<PublicKey>> trustedSubjects;
     private CertificateFactory factory;
 
     private boolean plugin = false;
@@ -95,9 +95,17 @@ public final class PKIXValidator extends Validator {
         if (TRY_VALIDATOR == false) {
             return;
         }
-        trustedSubjects = new HashMap<X500Principal, X509Certificate>();
+        trustedSubjects = new HashMap<X500Principal, List<PublicKey>>();
         for (X509Certificate cert : trustedCerts) {
-            trustedSubjects.put(cert.getSubjectX500Principal(), cert);
+            X500Principal dn = cert.getSubjectX500Principal();
+            List<PublicKey> keys;
+            if (trustedSubjects.containsKey(dn)) {
+                keys = trustedSubjects.get(dn);
+            } else {
+                keys = new ArrayList<PublicKey>();
+                trustedSubjects.put(dn, keys);
+            }
+            keys.add(cert.getPublicKey());
         }
         try {
             factory = CertificateFactory.getInstance("X.509");
@@ -161,13 +169,21 @@ public final class PKIXValidator extends Validator {
                     // chain is not ordered correctly, call builder instead
                     return doBuild(chain, otherCerts);
                 }
-                if (trustedSubjects.containsKey(dn)
-                        && trustedSubjects.get(dn).getPublicKey()
-                            .equals(cert.getPublicKey())) {
+
+                // Check if chain[i] is already trusted. It may be inside
+                // trustedCerts, or has the same dn and public key as a cert
+                // inside trustedCerts. The latter happens when a CA has
+                // updated its cert with a stronger signature algorithm in JRE
+                // but the weak one is still in circulation.
+
+                if (trustedCerts.contains(cert) ||          // trusted cert
+                        (trustedSubjects.containsKey(dn) && // replacing ...
+                         trustedSubjects.get(dn).contains(  // ... weak cert
+                            cert.getPublicKey()))) {
                     if (i == 0) {
                         return new X509Certificate[] {chain[0]};
                     }
-                    // Remove and call validator
+                    // Remove and call validator on partial chain [0 .. i-1]
                     X509Certificate[] newChain = new X509Certificate[i];
                     System.arraycopy(chain, 0, newChain, 0, i);
                     return doValidate(newChain);
@@ -217,14 +233,17 @@ public final class PKIXValidator extends Validator {
         return doBuild(chain, otherCerts);
     }
 
-    private boolean isSignatureValid(X509Certificate iss, X509Certificate sub) {
+    private boolean isSignatureValid(List<PublicKey> keys, X509Certificate sub) {
         if (plugin) {
-            try {
-                sub.verify(iss.getPublicKey());
-            } catch (Exception ex) {
-                return false;
+            for (PublicKey key: keys) {
+                try {
+                    sub.verify(key);
+                    return true;
+                } catch (Exception ex) {
+                    continue;
+                }
             }
-            return true;
+            return false;
         }
         return true; // only check if PLUGIN is set
     }

jdk/test/sun/security/validator/CertReplace.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright (c) 2010, 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
@@ -16,9 +16,9 @@
  * 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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
- * CA 95054 USA or visit www.sun.com if you need additional information or
- * have any questions.
+ * 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.
  */
 
 /*
@@ -37,25 +37,28 @@ import sun.security.validator.Validator;
 
 public class CertReplace {
 
-    private final static String cacerts = "certreplace.jks";
-    private final static String certs = "certreplace.certs";
-
+    /**
+     * @param args {cacerts keystore, cert chain}
+     */
     public static void main(String[] args) throws Exception {
 
         KeyStore ks = KeyStore.getInstance("JKS");
-        ks.load(new FileInputStream(cacerts), "changeit".toCharArray());
+        ks.load(new FileInputStream(args[0]), "changeit".toCharArray());
         Validator v = Validator.getInstance
             (Validator.TYPE_PKIX, Validator.VAR_GENERIC, ks);
-        X509Certificate[] chain = createPath();
-        System.out.println(Arrays.toString(v.validate(chain)));
-
+        X509Certificate[] chain = createPath(args[1]);
+        System.out.println("Chain: ");
+        for (X509Certificate c: v.validate(chain)) {
+            System.out.println("   " + c.getSubjectX500Principal() +
+                    " issued by " + c.getIssuerX500Principal());
+        }
     }
 
-    public static X509Certificate[] createPath() throws Exception {
+    public static X509Certificate[] createPath(String chain) throws Exception {
         CertificateFactory cf = CertificateFactory.getInstance("X.509");
         List list = new ArrayList();
         for (Certificate c: cf.generateCertificates(
-                new FileInputStream(certs))) {
+                new FileInputStream(chain))) {
             list.add((X509Certificate)c);
         }
         return (X509Certificate[]) list.toArray(new X509Certificate[0]);

jdk/test/sun/security/validator/certreplace.sh

@@ -1,5 +1,5 @@
 #
-# Copyright 2010 Sun Microsystems, Inc.  All Rights Reserved.
+# Copyright (c) 2010, 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
@@ -16,9 +16,9 @@
 # 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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
-# CA 95054 USA or visit www.sun.com if you need additional information or
-# have any questions.
+# 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
@@ -82,4 +82,4 @@ $KT -delete -alias user
 # 5. Build and run test
 
 $JAVAC -d . ${TESTSRC}${FS}CertReplace.java
-$JAVA CertReplace
+$JAVA CertReplace certreplace.jks certreplace.certs

jdk/test/sun/security/validator/samedn.sh

@@ -0,0 +1,82 @@
+#
+# Copyright (c) 2010, 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 6958869
+# @summary regression: PKIXValidator fails when multiple trust anchors
+# have same dn
+#
+
+if [ "${TESTSRC}" = "" ] ; then
+  TESTSRC="."
+fi
+if [ "${TESTJAVA}" = "" ] ; then
+  JAVAC_CMD=`which javac`
+  TESTJAVA=`dirname $JAVAC_CMD`/..
+fi
+
+# set platform-dependent variables
+OS=`uname -s`
+case "$OS" in
+  Windows_* )
+    FS="\\"
+    ;;
+  * )
+    FS="/"
+    ;;
+esac
+
+KT="$TESTJAVA${FS}bin${FS}keytool -storepass changeit \
+    -keypass changeit -keystore samedn.jks"
+JAVAC=$TESTJAVA${FS}bin${FS}javac
+JAVA=$TESTJAVA${FS}bin${FS}java
+
+rm -rf samedn.jks 2> /dev/null
+
+# 1. Generate 3 aliases in a keystore: ca1, ca2, user. The CAs' startdate
+# is set to one year ago so that they are expired now
+
+$KT -genkeypair -alias ca1 -dname CN=CA -keyalg rsa -sigalg md5withrsa -ext bc -startdate -1y
+$KT -genkeypair -alias ca2 -dname CN=CA -keyalg rsa -sigalg sha1withrsa -ext bc -startdate -1y
+$KT -genkeypair -alias user -dname CN=User -keyalg rsa
+
+# 2. Signing: ca -> user
+
+$KT -certreq -alias user | $KT -gencert -rfc -alias ca1 > samedn1.certs
+$KT -certreq -alias user | $KT -gencert -rfc -alias ca2 > samedn2.certs
+
+# 3. Append the ca file
+
+$KT -export -rfc -alias ca1 >> samedn1.certs
+$KT -export -rfc -alias ca2 >> samedn2.certs
+
+# 4. Remove user for cacerts
+
+$KT -delete -alias user
+
+# 5. Build and run test. Make sure the CA certs are ignored for validity check.
+# Check both, one of them might be dropped out of map in old codes.
+
+$JAVAC -d . ${TESTSRC}${FS}CertReplace.java
+$JAVA CertReplace samedn.jks samedn1.certs || exit 1
+$JAVA CertReplace samedn.jks samedn2.certs || exit 2