8176350: Usage constraints don't take effect when using PKIX

Reviewed-by: xuelei, mullan

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

@@ -106,9 +106,8 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
     private boolean trustedMatch = false;
 
     /**
-     * Create a new {@code AlgorithmChecker} with the algorithm
+     * Create a new {@code AlgorithmChecker} with the given algorithm
-     * constraints specified in security property
+     * given {@code TrustAnchor} and {@code String} variant.
-     * "jdk.certpath.disabledAlgorithms".
      *
      * @param anchor the trust anchor selected to validate the target
      *     certificate
@@ -116,13 +115,14 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
      *                passed will set it to Validator.GENERIC.
      */
     public AlgorithmChecker(TrustAnchor anchor, String variant) {
-        this(anchor, certPathDefaultConstraints, null, variant);
+        this(anchor, certPathDefaultConstraints, null, null, variant);
     }
 
     /**
      * Create a new {@code AlgorithmChecker} with the given
-     * {@code AlgorithmConstraints}, {@code Timestamp}, and/or {@code Variant}.
+     * {@code AlgorithmConstraints}, {@code Timestamp}, and {@code String}
-     * <p>
+     * variant.
+     *
      * Note that this constructor can initialize a variation of situations where
      * the AlgorithmConstraints, Timestamp, or Variant maybe known.
      *
@@ -134,32 +134,28 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
      */
     public AlgorithmChecker(AlgorithmConstraints constraints,
             Timestamp jarTimestamp, String variant) {
-        this.prevPubKey = null;
+        this(null, constraints, null, jarTimestamp, variant);
-        this.trustedPubKey = null;
-        this.constraints = (constraints == null ? certPathDefaultConstraints :
-                constraints);
-        this.pkixdate = (jarTimestamp != null ? jarTimestamp.getTimestamp() :
-                null);
-        this.jarTimestamp = jarTimestamp;
-        this.variant = (variant == null ? Validator.VAR_GENERIC : variant);
     }
 
     /**
      * Create a new {@code AlgorithmChecker} with the
-     * given {@code TrustAnchor} and {@code AlgorithmConstraints}.
+     * given {@code TrustAnchor}, {@code AlgorithmConstraints},
+     * {@code Timestamp}, and {@code String} variant.
      *
      * @param anchor the trust anchor selected to validate the target
      *     certificate
      * @param constraints the algorithm constraints (or null)
-     * @param pkixdate Date the constraints are checked against. The value is
+     * @param pkixdate The date specified by the PKIXParameters date.  If the
-     *             either the PKIXParameter date or null for the current date.
+     *                 PKIXParameters is null, the current date is used.  This
+     *                 should be null when jar files are being checked.
+     * @param jarTimestamp Timestamp passed for JAR timestamp constraint
+     *                     checking. Set to null if not applicable.
      * @param variant is the Validator variants of the operation. A null value
      *                passed will set it to Validator.GENERIC.
-     *
-     * @throws IllegalArgumentException if the {@code anchor} is null
      */
     public AlgorithmChecker(TrustAnchor anchor,
-            AlgorithmConstraints constraints, Date pkixdate, String variant) {
+            AlgorithmConstraints constraints, Date pkixdate,
+            Timestamp jarTimestamp, String variant) {
 
         if (anchor != null) {
             if (anchor.getTrustedCert() != null) {
@@ -179,28 +175,30 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
             }
         }
 
-        this.prevPubKey = trustedPubKey;
+        this.prevPubKey = this.trustedPubKey;
-        this.constraints = constraints;
+        this.constraints = (constraints == null ? certPathDefaultConstraints :
-        this.pkixdate = pkixdate;
+                constraints);
-        this.jarTimestamp = null;
+        // If we are checking jar files, set pkixdate the same as the timestamp
+        // for certificate checking
+        this.pkixdate = (jarTimestamp != null ? jarTimestamp.getTimestamp() :
+                pkixdate);
+        this.jarTimestamp = jarTimestamp;
         this.variant = (variant == null ? Validator.VAR_GENERIC : variant);
     }
 
     /**
-     * Create a new {@code AlgorithmChecker} with the
+     * Create a new {@code AlgorithmChecker} with the given {@code TrustAnchor},
-     * given {@code TrustAnchor} and {@code PKIXParameter} date.
+     * {@code PKIXParameter} date, and {@code varient}
      *
      * @param anchor the trust anchor selected to validate the target
      *     certificate
      * @param pkixdate Date the constraints are checked against. The value is
-     *             either the PKIXParameter date or null for the current date.
+     *             either the PKIXParameters date or null for the current date.
      * @param variant is the Validator variants of the operation. A null value
      *                passed will set it to Validator.GENERIC.
-     *
-     * @throws IllegalArgumentException if the {@code anchor} is null
      */
     public AlgorithmChecker(TrustAnchor anchor, Date pkixdate, String variant) {
-        this(anchor, certPathDefaultConstraints, pkixdate, variant);
+        this(anchor, certPathDefaultConstraints, pkixdate, null, variant);
     }
 
     // Check this 'cert' for restrictions in the AnchorCertificates
@@ -216,10 +214,6 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
         return AnchorCertificates.contains(cert);
     }
 
-    Timestamp getJarTimestamp() {
-        return jarTimestamp;
-    }
-
     @Override
     public void init(boolean forward) throws CertPathValidatorException {
         //  Note that this class does not support forward mode.

jdk/src/java.base/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java

@@ -172,12 +172,8 @@ public final class PKIXCertPathValidator extends CertPathValidatorSpi {
         List<PKIXCertPathChecker> certPathCheckers = new ArrayList<>();
         // add standard checkers that we will be using
         certPathCheckers.add(untrustedChecker);
-        if (params.timestamp() == null) {
+        certPathCheckers.add(new AlgorithmChecker(anchor, null, params.date(),
-            certPathCheckers.add(new AlgorithmChecker(anchor, params.date(), null));
-        } else {
-            certPathCheckers.add(new AlgorithmChecker(null,
                 params.timestamp(), params.variant()));
-        }
         certPathCheckers.add(new KeyChecker(certPathLen,
                                             params.targetCertConstraints()));
         certPathCheckers.add(new ConstraintsChecker(certPathLen));
@@ -195,13 +191,10 @@ public final class PKIXCertPathValidator extends CertPathValidatorSpi {
         certPathCheckers.add(pc);
         // default value for date is current time
         BasicChecker bc;
-        if (params.timestamp() == null) {
+        bc = new BasicChecker(anchor,
-            bc = new BasicChecker(anchor, params.date(), params.sigProvider(),
+                (params.timestamp() == null ? params.date() :
-                    false);
+                        params.timestamp().getTimestamp()),
-        } else {
-            bc = new BasicChecker(anchor, params.timestamp().getTimestamp(),
                 params.sigProvider(), false);
-        }
         certPathCheckers.add(bc);
 
         boolean revCheckerAdded = false;

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

@@ -27,6 +27,8 @@ package sun.security.util;
 
 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;
@@ -670,8 +672,14 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                 }
 
                 if (debug != null) {
-                    debug.println("Checking if usage constraint " + v +
+                    debug.println("Checking if usage constraint \"" + v +
-                            " matches " + cp.getVariant());
+                            "\" matches \"" + cp.getVariant() + "\"");
+                    // Because usage checking can come from many places
+                    // a stack trace is very helpful.
+                    ByteArrayOutputStream ba = new ByteArrayOutputStream();
+                    PrintStream ps = new PrintStream(ba);
+                    (new Exception()).printStackTrace(ps);
+                    debug.println(ba.toString());
                 }
                 if (cp.getVariant().compareTo(v) == 0) {
                     if (next(cp)) {

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

@@ -195,19 +195,17 @@ public final class PKIXValidator extends Validator {
                 ("null or zero-length certificate chain");
         }
 
-        // Check if 'parameter' affects 'pkixParameters'
+        // Use PKIXExtendedParameters for timestamp and variant additions
         PKIXBuilderParameters pkixParameters = null;
-        if (parameter instanceof Timestamp && plugin) {
         try {
             pkixParameters = new PKIXExtendedParameters(
                     (PKIXBuilderParameters) parameterTemplate.clone(),
-                        (Timestamp) parameter, variant);
+                    (parameter instanceof Timestamp) ?
+                            (Timestamp) parameter : null,
+                    variant);
         } catch (InvalidAlgorithmParameterException e) {
             // ignore exception
         }
-        } else {
-            pkixParameters = (PKIXBuilderParameters) parameterTemplate.clone();
-        }
 
         // add new algorithm constraints checker
         if (constraints != null) {

jdk/src/java.base/share/classes/sun/security/validator/SimpleValidator.java

@@ -162,7 +162,7 @@ public final class SimpleValidator extends Validator {
         AlgorithmChecker appAlgChecker = null;
         if (constraints != null) {
             appAlgChecker = new AlgorithmChecker(anchor, constraints, null,
-                    variant);
+                    null, variant);
         }
 
         // verify top down, starting at the certificate issued by