8170131: Certificates not being blocked by jdk.tls.disabledAlgorithms property

8166393: disabledAlgorithms property should not be strictly parsed
Reviewed-by: mullan

src/share/classes/sun/security/provider/certpath/AlgorithmChecker.java

@@ -255,16 +255,17 @@ final public class AlgorithmChecker extends PKIXCertPathChecker {
 
         PublicKey currPubKey = cert.getPublicKey();
 
-        // Check against DisabledAlgorithmConstraints certpath constraints.
-        // permits() will throw exception on failure.
-        certPathDefaultConstraints.permits(primitives,
+        if (constraints instanceof DisabledAlgorithmConstraints) {
+            // Check against DisabledAlgorithmConstraints certpath constraints.
+            // permits() will throw exception on failure.
+            ((DisabledAlgorithmConstraints)constraints).permits(primitives,
                 new CertConstraintParameters((X509Certificate)cert,
                         trustedMatch));
-                // new CertConstraintParameters(x509Cert, trustedMatch));
-        // If there is no previous key, set one and exit
-        if (prevPubKey == null) {
-            prevPubKey = currPubKey;
-            return;
+            // If there is no previous key, set one and exit
+            if (prevPubKey == null) {
+                prevPubKey = currPubKey;
+                return;
+            }
         }
 
         X509CertImpl x509Cert;

src/share/classes/sun/security/util/DisabledAlgorithmConstraints.java

@@ -268,7 +268,8 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                 }
 
                 // Convert constraint conditions into Constraint classes
-                Constraint c, lastConstraint = null;
+                Constraint c = null;
+                Constraint lastConstraint = null;
                 // Allow only one jdkCA entry per constraint entry
                 boolean jdkCALimit = false;
 
@@ -296,9 +297,6 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                         }
                         c = new jdkCAConstraint(algorithm);
                         jdkCALimit = true;
-                    } else {
-                        throw new IllegalArgumentException("Error in security" +
-                                " property. Constraint unknown: " + entry);
                     }
 
                     // Link multiple conditions for a single constraint
@@ -308,7 +306,9 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                             constraintsMap.putIfAbsent(algorithm,
                                     new HashSet<>());
                         }
-                        constraintsMap.get(algorithm).add(c);
+                        if (c != null) {
+                            constraintsMap.get(algorithm).add(c);
+                        }
                     } else {
                         lastConstraint.nextConstraint = c;
                     }

test/sun/security/ssl/com/sun/net/ssl/internal/ssl/X509TrustManagerImpl/PKIXExtendedTM.java

 /*
- * Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2016, 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
@@ -28,9 +28,12 @@
 
 /*
  * @test
- * @bug 6916074
+ * @bug 6916074 8170131
  * @summary Add support for TLS 1.2
- * @run main/othervm PKIXExtendedTM
+ * @run main/othervm PKIXExtendedTM 0
+ * @run main/othervm PKIXExtendedTM 1
+ * @run main/othervm PKIXExtendedTM 2
+ * @run main/othervm PKIXExtendedTM 3
  */
 
 import java.net.*;
@@ -42,6 +45,7 @@ import java.security.KeyStore;
 import java.security.KeyFactory;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateFactory;
+import java.security.cert.CertPathValidatorException;
 import java.security.spec.*;
 import java.security.interfaces.*;
 import java.math.BigInteger;
@@ -792,20 +796,85 @@ public class PKIXExtendedTM {
     volatile Exception serverException = null;
     volatile Exception clientException = null;
 
-    public static void main(String args[]) throws Exception {
+    static class Test {
+        String tlsDisAlgs;
+        String certPathDisAlgs;
+        boolean fail;
+        Test(String tlsDisAlgs, String certPathDisAlgs, boolean fail) {
+            this.tlsDisAlgs = tlsDisAlgs;
+            this.certPathDisAlgs = certPathDisAlgs;
+            this.fail = fail;
+        }
+    }
+
+    static Test[] tests = {
         // MD5 is used in this test case, don't disable MD5 algorithm.
+        new Test(
+            "SSLv3, RC4, DH keySize < 768",
+            "MD2, RSA keySize < 1024",
+            false),
+        // Disable MD5 but only if cert chains back to public root CA, should
+        // pass because the MD5 cert in this test case is issued by test CA
+        new Test(
+            "SSLv3, RC4, DH keySize < 768",
+            "MD2, MD5 jdkCA, RSA keySize < 1024",
+            false),
+        // Disable MD5 alg via TLS property and expect failure
+        new Test(
+            "SSLv3, MD5, RC4, DH keySize < 768",
+            "MD2, RSA keySize < 1024",
+            true),
+        // Disable MD5 alg via certpath property and expect failure
+        new Test(
+            "SSLv3, RC4, DH keySize < 768",
+            "MD2, MD5, RSA keySize < 1024",
+            true),
+    };
+
+    public static void main(String args[]) throws Exception {
+        if (args.length != 1) {
+            throw new Exception("Incorrect number of arguments");
+        }
+        Test test = tests[Integer.parseInt(args[0])];
+        Security.setProperty("jdk.tls.disabledAlgorithms", test.tlsDisAlgs);
         Security.setProperty("jdk.certpath.disabledAlgorithms",
-                "MD2, RSA keySize < 1024");
-        Security.setProperty("jdk.tls.disabledAlgorithms",
-                "SSLv3, RC4, DH keySize < 768");
+                             test.certPathDisAlgs);
 
-        if (debug)
+        if (debug) {
             System.setProperty("javax.net.debug", "all");
+        }
 
         /*
          * Start the tests.
          */
-        new PKIXExtendedTM();
+        try {
+            new PKIXExtendedTM();
+            if (test.fail) {
+                throw new Exception("Expected MD5 certificate to be blocked");
+            }
+        } catch (Exception e) {
+            if (test.fail) {
+                // find expected cause
+                boolean correctReason = false;
+                Throwable cause = e.getCause();
+                while (cause != null) {
+                    if (cause instanceof CertPathValidatorException) {
+                        CertPathValidatorException cpve =
+                            (CertPathValidatorException)cause;
+                        if (cpve.getReason() == CertPathValidatorException.BasicReason.ALGORITHM_CONSTRAINED) {
+                            correctReason = true;
+                            break;
+                        }
+                    }
+                    cause = cause.getCause();
+                }
+                if (!correctReason) {
+                    throw new Exception("Unexpected exception", e);
+                }
+            } else {
+                throw e;
+            }
+        }
     }
 
     Thread clientThread = null;