changeset 12597:49f05b4e886d

8170131: Certificates not being blocked by jdk.tls.disabledAlgorithms property 8166393: disabledAlgorithms property should not be strictly parsed Reviewed-by: mullan
author robm
date Fri, 09 Dec 2016 16:22:45 +0000
parents f3d7734ad739
children fca24a0be2f5
files src/share/classes/sun/security/provider/certpath/AlgorithmChecker.java src/share/classes/sun/security/util/DisabledAlgorithmConstraints.java test/sun/security/ssl/com/sun/net/ssl/internal/ssl/X509TrustManagerImpl/PKIXExtendedTM.java
diffstat 3 files changed, 92 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/sun/security/provider/certpath/AlgorithmChecker.java	Thu Dec 08 14:06:39 2016 -0500
+++ b/src/share/classes/sun/security/provider/certpath/AlgorithmChecker.java	Fri Dec 09 16:22:45 2016 +0000
@@ -255,16 +255,17 @@
 
         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;
--- a/src/share/classes/sun/security/util/DisabledAlgorithmConstraints.java	Thu Dec 08 14:06:39 2016 -0500
+++ b/src/share/classes/sun/security/util/DisabledAlgorithmConstraints.java	Fri Dec 09 16:22:45 2016 +0000
@@ -268,7 +268,8 @@
                 }
 
                 // 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 @@
                         }
                         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 @@
                             constraintsMap.putIfAbsent(algorithm,
                                     new HashSet<>());
                         }
-                        constraintsMap.get(algorithm).add(c);
+                        if (c != null) {
+                            constraintsMap.get(algorithm).add(c);
+                        }
                     } else {
                         lastConstraint.nextConstraint = c;
                     }
--- a/test/sun/security/ssl/com/sun/net/ssl/internal/ssl/X509TrustManagerImpl/PKIXExtendedTM.java	Thu Dec 08 14:06:39 2016 -0500
+++ b/test/sun/security/ssl/com/sun/net/ssl/internal/ssl/X509TrustManagerImpl/PKIXExtendedTM.java	Fri Dec 09 16:22:45 2016 +0000
@@ -1,5 +1,5 @@
 /*
- * 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.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 @@
     volatile Exception serverException = null;
     volatile Exception clientException = null;
 
+    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 {
-        // MD5 is used in this test case, don't disable MD5 algorithm.
+        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;