8233228: Disable weak named curves by default in TLS, CertPath, and S…

…igned JAR

Reviewed-by: mullan, xuelei, weijun

src/java.base/share/classes/sun/security/tools/keytool/Main.java

@@ -4654,7 +4654,7 @@ private void checkWeak(String label, String sigAlg, Key key) {
                     rb.getString("whose.key.risk"),
                     label,
                     String.format(rb.getString("key.bit"),
-                            KeyUtil.getKeySize(key), key.getAlgorithm())));
+                            KeyUtil.getKeySize(key), fullDisplayAlgName(key))));
         }
     }
 

src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -29,6 +29,10 @@
 import java.security.AlgorithmConstraints;
 import java.security.PrivilegedAction;
 import java.security.Security;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 
 /**
@@ -44,7 +48,7 @@ protected AbstractAlgorithmConstraints(AlgorithmDecomposer decomposer) {
     }
 
     // Get algorithm constraints from the specified security property.
-    static String[] getAlgorithms(String propertyName) {
+    static List<String> getAlgorithms(String propertyName) {
         String property = AccessController.doPrivileged(
                 new PrivilegedAction<String>() {
                     @Override
@@ -68,12 +72,12 @@ public String run() {
 
         // map the disabled algorithms
         if (algorithmsInProperty == null) {
-            algorithmsInProperty = new String[0];
+            return Collections.emptyList();
         }
-        return algorithmsInProperty;
+        return new ArrayList<>(Arrays.asList(algorithmsInProperty));
     }
 
-    static boolean checkAlgorithm(String[] algorithms, String algorithm,
+    static boolean checkAlgorithm(List<String> algorithms, String algorithm,
             AlgorithmDecomposer decomposer) {
         if (algorithm == null || algorithm.isEmpty()) {
             throw new IllegalArgumentException("No algorithm name specified");

src/java.base/share/classes/sun/security/util/ConstraintsParameters.java

@@ -31,6 +31,9 @@
 import java.security.Key;
 import java.security.Timestamp;
 import java.security.cert.X509Certificate;
+import java.security.interfaces.ECKey;
+import java.security.interfaces.XECKey;
+import java.security.spec.NamedParameterSpec;
 import java.util.Date;
 
 /**
@@ -49,8 +52,8 @@ public class ConstraintsParameters {
     private final String algorithm;
     // AlgorithmParameters to the algorithm being checked
     private final AlgorithmParameters algParams;
-    // Public Key being checked against constraints
-    private final Key publicKey;
+    // Key being checked against constraints
+    private final Key key;
 
     /*
      * New values that are checked against constraints that the current public
@@ -66,6 +69,9 @@ public class ConstraintsParameters {
     // Timestamp of the signed JAR file
     private final Timestamp jarTimestamp;
     private final String variant;
+    // Named Curve
+    private final String[] curveStr;
+    private static final String[] EMPTYLIST = new String[0];
 
     public ConstraintsParameters(X509Certificate c, boolean match,
             Date pkixdate, Timestamp jarTime, String variant) {
@@ -76,14 +82,20 @@ public ConstraintsParameters(X509Certificate c, boolean match,
         this.variant = (variant == null ? Validator.VAR_GENERIC : variant);
         algorithm = null;
         algParams = null;
-        publicKey = null;
+        key = null;
+        if (c != null) {
+            curveStr = getNamedCurveFromKey(c.getPublicKey());
+        } else {
+            curveStr = EMPTYLIST;
+        }
     }
 
     public ConstraintsParameters(String algorithm, AlgorithmParameters params,
             Key key, String variant) {
         this.algorithm = algorithm;
         algParams = params;
-        this.publicKey = key;
+        this.key = key;
+        curveStr = getNamedCurveFromKey(key);
         cert = null;
         trustedMatch = false;
         pkixDate = null;
@@ -109,9 +121,10 @@ public AlgorithmParameters getAlgParams() {
         return algParams;
     }
 
-    public Key getPublicKey() {
-        return publicKey;
+    public Key getKey() {
+        return key;
     }
+
     // Returns if the trust anchor has a match if anchor checking is enabled.
     public boolean isTrustedMatch() {
         return trustedMatch;
@@ -132,4 +145,47 @@ public Timestamp getJARTimestamp() {
     public String getVariant() {
         return variant;
     }
+
+    public String[] getNamedCurve() {
+        return curveStr;
+    }
+
+    public static String[] getNamedCurveFromKey(Key key) {
+        if (key instanceof ECKey) {
+            NamedCurve nc = CurveDB.lookup(((ECKey)key).getParams());
+            return (nc == null ? EMPTYLIST : CurveDB.getNamesByOID(nc.getObjectId()));
+        } else if (key instanceof XECKey) {
+            String[] s = {
+                    ((NamedParameterSpec)((XECKey)key).getParams()).getName()
+            };
+            return s;
+        } else {
+            return EMPTYLIST;
+        }
+    }
+
+    public String toString() {
+        StringBuilder s = new StringBuilder();
+        s.append("Cert:       ");
+        if (cert != null) {
+            s.append(cert.toString());
+            s.append("\nSigAlgo:    ");
+            s.append(cert.getSigAlgName());
+        } else {
+            s.append("None");
+        }
+        s.append("\nAlgParams:  ");
+        if (getAlgParams() != null) {
+            getAlgParams().toString();
+        } else {
+            s.append("None");
+        }
+        s.append("\nNamedCurves: ");
+        for (String c : getNamedCurve()) {
+            s.append(c + " ");
+        }
+        s.append("\nVariant:    " + getVariant());
+        return s.toString();
+    }
+
 }

src/java.base/share/classes/sun/security/util/CurveDB.java

@@ -154,8 +154,27 @@ private static void add(String name, String soid, int type, String sfield,
         }
     }
 
+    private static class Holder {
+        private static final Pattern nameSplitPattern = Pattern.compile(
+                SPLIT_PATTERN);
+    }
+
+    // Return all the names the EC curve could be using.
+    static String[] getNamesByOID(String oid) {
+        NamedCurve nc = oidMap.get(oid);
+        if (nc == null) {
+            return new String[0];
+        }
+        String[] list = Holder.nameSplitPattern.split(nc.getName());
+        int i = 0;
+        do {
+            list[i] = list[i].trim();
+        } while (++i < list.length);
+        return list;
+    }
+
     static {
-        Pattern nameSplitPattern = Pattern.compile(SPLIT_PATTERN);
+        Pattern nameSplitPattern = Holder.nameSplitPattern;
 
         /* SEC2 prime curves */
         add("secp112r1", "1.3.132.0.6", P,

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2019, 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
@@ -27,8 +27,6 @@
 
 import sun.security.validator.Validator;
 
-import java.io.ByteArrayOutputStream;
-import java.io.PrintStream;
 import java.security.CryptoPrimitive;
 import java.security.AlgorithmParameters;
 import java.security.Key;
@@ -37,6 +35,7 @@
 import java.security.cert.X509Certificate;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.HashMap;
@@ -60,19 +59,23 @@
 public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
     private static final Debug debug = Debug.getInstance("certpath");
 
-    // the known security property, jdk.certpath.disabledAlgorithms
+    // Disabled algorithm security property for certificate path
     public static final String PROPERTY_CERTPATH_DISABLED_ALGS =
             "jdk.certpath.disabledAlgorithms";
 
-    // the known security property, jdk.tls.disabledAlgorithms
+    // Disabled algorithm security property for TLS
     public static final String PROPERTY_TLS_DISABLED_ALGS =
             "jdk.tls.disabledAlgorithms";
 
-    // the known security property, jdk.jar.disabledAlgorithms
+    // Disabled algorithm security property for jar
     public static final String PROPERTY_JAR_DISABLED_ALGS =
             "jdk.jar.disabledAlgorithms";
 
-    private final String[] disabledAlgorithms;
+    // Property for disabled EC named curves
+    private static final String PROPERTY_DISABLED_EC_CURVES =
+            "jdk.disabled.namedCurves";
+
+    private final List<String> disabledAlgorithms;
     private final Constraints algorithmConstraints;
 
     /**
@@ -97,6 +100,24 @@ public DisabledAlgorithmConstraints(String propertyName,
             AlgorithmDecomposer decomposer) {
         super(decomposer);
         disabledAlgorithms = getAlgorithms(propertyName);
+
+        // Check for alias
+        int ecindex = -1, i = 0;
+        for (String s : disabledAlgorithms) {
+            if (s.regionMatches(true, 0,"include ", 0, 8)) {
+                if (s.regionMatches(true, 8, PROPERTY_DISABLED_EC_CURVES, 0,
+                        PROPERTY_DISABLED_EC_CURVES.length())) {
+                    ecindex = i;
+                    break;
+                }
+            }
+            i++;
+        }
+        if (ecindex > -1) {
+            disabledAlgorithms.remove(ecindex);
+            disabledAlgorithms.addAll(ecindex,
+                    getAlgorithms(PROPERTY_DISABLED_EC_CURVES));
+        }
         algorithmConstraints = new Constraints(disabledAlgorithms);
     }
 
@@ -164,6 +185,19 @@ public final void permits(String algorithm, Key key,
 
     public final void permits(String algorithm, ConstraintsParameters cp)
             throws CertPathValidatorException {
+
+        // Check if named curves in the ConstraintParameters are disabled.
+        if (cp.getNamedCurve() != null) {
+            for (String curve : cp.getNamedCurve()) {
+                if (!checkAlgorithm(disabledAlgorithms, curve, decomposer)) {
+                    throw new CertPathValidatorException(
+                            "Algorithm constraints check failed on disabled " +
+                                    "algorithm: " + curve,
+                            null, null, -1, BasicReason.ALGORITHM_CONSTRAINED);
+                }
+            }
+        }
+
         algorithmConstraints.permits(algorithm, cp);
     }
 
@@ -199,6 +233,13 @@ private boolean checkConstraints(Set<CryptoPrimitive> primitives,
             return false;
         }
 
+        // If this is an elliptic curve, check disabled the named curve.
+        for (String curve : ConstraintsParameters.getNamedCurveFromKey(key)) {
+            if (!permits(primitives, curve, null)) {
+                return false;
+            }
+        }
+
         // check the key constraints
         return algorithmConstraints.permits(key);
     }
@@ -230,7 +271,7 @@ private static class Holder {
                     "denyAfter\\s+(\\d{4})-(\\d{2})-(\\d{2})");
         }
 
-        public Constraints(String[] constraintArray) {
+        public Constraints(List<String> constraintArray) {
             for (String constraintEntry : constraintArray) {
                 if (constraintEntry == null || constraintEntry.isEmpty()) {
                     continue;
@@ -257,7 +298,9 @@ public Constraints(String[] constraintArray) {
                     constraintsMap.putIfAbsent(alias, constraintList);
                 }
 
-                if (space <= 0) {
+                // If there is no whitespace, it is a algorithm name; however,
+                // if there is a whitespace, could be a multi-word EC curve too.
+                if (space <= 0 || CurveDB.lookup(constraintEntry) != null) {
                     constraintList.add(new DisabledConstraint(algorithm));
                     continue;
                 }
@@ -356,7 +399,7 @@ public boolean permits(Key key) {
             for (Constraint constraint : list) {
                 if (!constraint.permits(key)) {
                     if (debug != null) {
-                        debug.println("keySizeConstraint: failed key " +
+                        debug.println("Constraints: failed key size" +
                                 "constraint check " + KeyUtil.getKeySize(key));
                     }
                     return false;
@@ -375,7 +418,7 @@ public boolean permits(String algorithm, AlgorithmParameters aps) {
             for (Constraint constraint : list) {
                 if (!constraint.permits(aps)) {
                     if (debug != null) {
-                        debug.println("keySizeConstraint: failed algorithm " +
+                        debug.println("Constraints: failed algorithm " +
                                 "parameters constraint check " + aps);
                     }
 
@@ -392,8 +435,7 @@ public void permits(String algorithm, ConstraintsParameters cp)
             X509Certificate cert = cp.getCertificate();
 
             if (debug != null) {
-                debug.println("Constraints.permits(): " + algorithm +
-                        " Variant: " + cp.getVariant());
+                debug.println("Constraints.permits(): " + cp.toString());
             }
 
             // Get all signature algorithms to check for constraints
@@ -406,8 +448,8 @@ public void permits(String algorithm, ConstraintsParameters cp)
             if (cert != null) {
                 algorithms.add(cert.getPublicKey().getAlgorithm());
             }
-            if (cp.getPublicKey() != null) {
-                algorithms.add(cp.getPublicKey().getAlgorithm());
+            if (cp.getKey() != null) {
+                algorithms.add(cp.getKey().getAlgorithm());
             }
             // Check all applicable constraints
             for (String alg : algorithms) {
@@ -546,10 +588,7 @@ boolean next(ConstraintsParameters cp)
          * the constraint denies the operation.
          */
         boolean next(Key key) {
-            if (nextConstraint != null && nextConstraint.permits(key)) {
-                return true;
-            }
-            return false;
+            return nextConstraint != null && nextConstraint.permits(key);
         }
 
         String extendedMsg(ConstraintsParameters cp) {
@@ -799,8 +838,8 @@ public KeySizeConstraint(String algo, Operator operator, int length) {
         public void permits(ConstraintsParameters cp)
                 throws CertPathValidatorException {
             Key key = null;
-            if (cp.getPublicKey() != null) {
-                key = cp.getPublicKey();
+            if (cp.getKey() != null) {
+                key = cp.getKey();
             } else if (cp.getCertificate() != null) {
                 key = cp.getCertificate().getPublicKey();
             }