8298310: Enhance TLS session negotiation

Reviewed-by: rhalade, mschoene, weijun, ascarpino

src/java.base/share/classes/sun/security/provider/certpath/AdjacencyList.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, 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
@@ -87,7 +87,7 @@ public class AdjacencyList {
     // the actual set of steps the AdjacencyList represents
     private final ArrayList<BuildStep> mStepList;
 
-    // the original list, just for the toString method
+    // the original list
     private final List<List<Vertex>> mOrigList;
 
     /**
@@ -114,6 +114,13 @@ public class AdjacencyList {
         return Collections.unmodifiableList(mStepList).iterator();
     }
 
+    /**
+     * Returns the number of attempted paths (useful for debugging).
+     */
+    public int numAttemptedPaths() {
+        return mOrigList.size();
+    }
+
     /**
      * Recursive, private method which actually builds the step list from
      * the given adjacency list. <code>Follow</code> is the parent BuildStep

src/java.base/share/classes/sun/security/provider/certpath/Builder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, 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
@@ -406,8 +406,7 @@ abstract class Builder {
 
     /**
      * Search the specified CertStores and add all certificates matching
-     * selector to resultCerts. Self-signed certs are not useful here
+     * selector to resultCerts.
-     * and therefore ignored.
      *
      * If the targetCert criterion of the selector is set, only that cert
      * is examined and the CertStores are not searched.
@@ -426,8 +425,7 @@ abstract class Builder {
         X509Certificate targetCert = selector.getCertificate();
         if (targetCert != null) {
             // no need to search CertStores
-            if (selector.match(targetCert) && !X509CertImpl.isSelfSigned
+            if (selector.match(targetCert)) {
-                (targetCert, buildParams.sigProvider())) {
                 if (debug != null) {
                     debug.println("Builder.addMatchingCerts: " +
                         "adding target cert" +
@@ -446,13 +444,10 @@ abstract class Builder {
                 Collection<? extends Certificate> certs =
                                         store.getCertificates(selector);
                 for (Certificate cert : certs) {
-                    if (!X509CertImpl.isSelfSigned
-                        ((X509Certificate)cert, buildParams.sigProvider())) {
                     if (resultCerts.add((X509Certificate)cert)) {
                         add = true;
                     }
                 }
-                }
                 if (!checkAll && add) {
                     return true;
                 }

src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, 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
@@ -47,6 +47,7 @@ import sun.security.x509.AccessDescription;
 import sun.security.x509.AuthorityInfoAccessExtension;
 import sun.security.x509.AuthorityKeyIdentifierExtension;
 import static sun.security.x509.PKIXExtensions.*;
+import sun.security.x509.SubjectAlternativeNameExtension;
 import sun.security.x509.X500Name;
 import sun.security.x509.X509CertImpl;
 
@@ -293,9 +294,7 @@ final class ForwardBuilder extends Builder {
                         "\n  Issuer: " +
                             trustedCert.getIssuerX500Principal());
                 }
-                if (caCerts.add(trustedCert) && !searchAllCertStores) {
+                caCerts.add(trustedCert);
-                    return;
-                }
             }
         }
 
@@ -668,8 +667,7 @@ final class ForwardBuilder extends Builder {
      * only be executed in a reverse direction are deferred until the
      * complete path has been built.
      *
-     * Trust anchor certs are not validated, but are used to verify the
+     * Trust anchor certs are not validated.
-     * signature and revocation status of the previous cert.
      *
      * If the last certificate is being verified (the one whose subject
      * matches the target subject) then steps in 6.1.4 of the PKIX
@@ -700,17 +698,15 @@ final class ForwardBuilder extends Builder {
         currState.untrustedChecker.check(cert, Collections.emptySet());
 
         /*
-         * check for looping - abort a loop if we encounter the same
+         * Abort if we encounter the same certificate or a certificate with
-         * certificate twice
+         * the same public key, subject DN, and subjectAltNames as a cert
+         * that is already in path.
          */
-        if (certPathList != null) {
         for (X509Certificate cpListCert : certPathList) {
-                if (cert.equals(cpListCert)) {
+            if (repeated(cpListCert, cert)) {
-                    if (debug != null) {
+                throw new CertPathValidatorException(
-                        debug.println("loop detected!!");
+                    "cert with repeated subject, public key, and " +
-                    }
+                    "subjectAltNames detected");
-                    throw new CertPathValidatorException("loop detected");
-                }
             }
         }
 
@@ -789,21 +785,48 @@ final class ForwardBuilder extends Builder {
              */
             KeyChecker.verifyCAKeyUsage(cert);
         }
+    }
 
-        /*
+    /**
-         * the following checks are performed even when the cert
+     * Return true if two certificates are equal or have the same subject,
-         * is a trusted cert, since we are only extracting the
+     * public key, and subject alternative names.
-         * subjectDN, and publicKey from the cert
-         * in order to verify a previous cert
      */
+    private static boolean repeated(
+            X509Certificate currCert, X509Certificate nextCert) {
+        if (currCert.equals(nextCert)) {
+            return true;
+        }
+        return (currCert.getSubjectX500Principal().equals(
+            nextCert.getSubjectX500Principal()) &&
+            currCert.getPublicKey().equals(nextCert.getPublicKey()) &&
+            altNamesEqual(currCert, nextCert));
+    }
 
-        /*
+    /**
-         * Check signature only if no key requiring key parameters has been
+     * Return true if two certificates have the same subject alternative names.
-         * encountered.
      */
-        if (!currState.keyParamsNeeded()) {
+    private static boolean altNamesEqual(
-            (currState.cert).verify(cert.getPublicKey(),
+            X509Certificate currCert, X509Certificate nextCert) {
-                                    buildParams.sigProvider());
+        X509CertImpl curr, next;
+        try {
+            curr = X509CertImpl.toImpl(currCert);
+            next = X509CertImpl.toImpl(nextCert);
+        } catch (CertificateException ce) {
+            return false;
+        }
+
+        SubjectAlternativeNameExtension currAltNameExt =
+            curr.getSubjectAlternativeNameExtension();
+        SubjectAlternativeNameExtension nextAltNameExt =
+            next.getSubjectAlternativeNameExtension();
+        if (currAltNameExt != null) {
+            if (nextAltNameExt == null) {
+                return false;
+            }
+            return Arrays.equals(currAltNameExt.getExtensionValue(),
+                nextAltNameExt.getExtensionValue());
+        } else {
+            return (nextAltNameExt == null);
         }
     }
 

src/java.base/share/classes/sun/security/provider/certpath/ForwardState.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, 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
@@ -80,10 +80,8 @@ class ForwardState implements State {
     /* The list of user-defined checkers that support forward checking */
     ArrayList<PKIXCertPathChecker> forwardCheckers;
 
-    /* Flag indicating if key needing to inherit key parameters has been
+    /* Flag indicating if last cert in path is self-issued */
-     * encountered.
+    boolean selfIssued;
-     */
-    boolean keyParamsNeededFlag = false;
 
     /**
      * Returns a boolean flag indicating if the state is initial
@@ -96,18 +94,6 @@ class ForwardState implements State {
         return init;
     }
 
-    /**
-     * Return boolean flag indicating whether a public key that needs to inherit
-     * key parameters has been encountered.
-     *
-     * @return boolean true if key needing to inherit parameters has been
-     * encountered; false otherwise.
-     */
-    @Override
-    public boolean keyParamsNeeded() {
-        return keyParamsNeededFlag;
-    }
-
     /**
      * Display state for debugging purposes
      */
@@ -117,9 +103,9 @@ class ForwardState implements State {
                 "\n  issuerDN of last cert: " + issuerDN +
                 "\n  traversedCACerts: " + traversedCACerts +
                 "\n  init: " + init +
-                "\n  keyParamsNeeded: " + keyParamsNeededFlag +
                 "\n  subjectNamesTraversed: \n" +
                 subjectNamesTraversed +
+                "\n  selfIssued: " + selfIssued + "\n" +
                 "]\n";
     }
 
@@ -163,18 +149,14 @@ class ForwardState implements State {
 
         X509CertImpl icert = X509CertImpl.toImpl(cert);
 
-        /* see if certificate key has null parameters */
-        if (PKIX.isDSAPublicKeyWithoutParams(icert.getPublicKey())) {
-            keyParamsNeededFlag = true;
-        }
-
         /* update certificate */
         this.cert = icert;
 
         /* update issuer DN */
         issuerDN = cert.getIssuerX500Principal();
 
-        if (!X509CertImpl.isSelfIssued(cert)) {
+        selfIssued = X509CertImpl.isSelfIssued(cert);
+        if (!selfIssued) {
 
             /*
              * update traversedCACerts only if this is a non-self-issued
@@ -187,7 +169,7 @@ class ForwardState implements State {
 
         /* update subjectNamesTraversed only if this is the EE cert or if
            this cert is not self-issued */
-        if (init || !X509CertImpl.isSelfIssued(cert)) {
+        if (init || !selfIssued) {
             X500Principal subjName = cert.getSubjectX500Principal();
             subjectNamesTraversed.add(X500Name.asX500Name(subjName));
 

src/java.base/share/classes/sun/security/provider/certpath/State.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, 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
@@ -62,12 +62,4 @@ interface State extends Cloneable {
      * @return boolean flag indicating if the state is initial (just starting)
      */
     boolean isInitial();
-
-    /**
-     * Returns a boolean flag indicating if a key lacking necessary key
-     * algorithm parameters has been encountered.
-     *
-     * @return boolean flag indicating if key lacking parameters encountered.
-     */
-    boolean keyParamsNeeded();
 }

src/java.base/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, 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
@@ -42,6 +42,7 @@ import javax.security.auth.x500.X500Principal;
 
 import sun.security.provider.certpath.PKIX.BuilderParams;
 import static sun.security.x509.PKIXExtensions.*;
+import sun.security.x509.X509CertImpl;
 import sun.security.util.Debug;
 
 /**
@@ -130,6 +131,7 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
         List<List<Vertex>> adjList = new ArrayList<>();
         PKIXCertPathBuilderResult result = buildCertPath(false, adjList);
         if (result == null) {
+            if (buildParams.certStores().size() > 1 || Builder.USE_AIA) {
                 if (debug != null) {
                     debug.println("SunCertPathBuilder.engineBuild: 2nd pass; " +
                               "try building again searching all certstores");
@@ -137,12 +139,14 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
                 // try again
                 adjList.clear();
                 result = buildCertPath(true, adjList);
-            if (result == null) {
+                if (result != null) {
+                    return result;
+                }
+            }
             throw new SunCertPathBuilderException("unable to find valid "
                 + "certification path to requested target",
                 new AdjacencyList(adjList));
         }
-        }
         return result;
     }
 
@@ -270,8 +274,8 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
         /*
          * For each cert in the collection, verify anything
          * that hasn't been checked yet (signature, revocation, etc.)
-         * and check for loops. Call depthFirstSearchForward()
+         * and check for certs with repeated public key and subject.
-         * recursively for each good cert.
+         * Call depthFirstSearchForward() recursively for each good cert.
          */
 
                vertices:
@@ -346,8 +350,6 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
                 checkers.add(new AlgorithmChecker(builder.trustAnchor,
                         buildParams.timestamp(), buildParams.variant()));
 
-                BasicChecker basicChecker = null;
-                if (nextState.keyParamsNeeded()) {
                 PublicKey rootKey = cert.getPublicKey();
                 if (builder.trustAnchor.getTrustedCert() == null) {
                     rootKey = builder.trustAnchor.getCAPublicKey();
@@ -361,11 +363,11 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
                     (cert.getSubjectX500Principal(), rootKey, null);
 
                 // add the basic checker
-                    basicChecker = new BasicChecker(anchor, buildParams.date(),
+                BasicChecker basicChecker = new BasicChecker(anchor,
+                                                    buildParams.date(),
                                                     buildParams.sigProvider(),
                                                     true);
                 checkers.add(basicChecker);
-                }
 
                 buildParams.setCertPath(cf.generateCertPath(appendedCerts));
 
@@ -511,6 +513,14 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
                 policyTreeResult = policyChecker.getPolicyTree();
                 return;
             } else {
+                // If successive certs are self-issued, don't continue search
+                // on this branch.
+                if (currentState.selfIssued && X509CertImpl.isSelfIssued(cert)) {
+                    if (debug != null) {
+                        debug.println("Successive certs are self-issued");
+                    }
+                    return;
+                }
                 builder.addCertToPath(cert, cpList);
             }