diff --git a/src/java.base/share/classes/sun/security/provider/certpath/AdjacencyList.java b/src/java.base/share/classes/sun/security/provider/certpath/AdjacencyList.java index 283fb649da6..b6d9bcddd34 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/AdjacencyList.java +++ b/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 mStepList; - // the original list, just for the toString method + // the original list private final List> 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. Follow is the parent BuildStep diff --git a/src/java.base/share/classes/sun/security/provider/certpath/Builder.java b/src/java.base/share/classes/sun/security/provider/certpath/Builder.java index 23c12519ad7..50f0cfbd2ea 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/Builder.java +++ b/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 - * and therefore ignored. + * selector to resultCerts. * * 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 - (targetCert, buildParams.sigProvider())) { + if (selector.match(targetCert)) { if (debug != null) { debug.println("Builder.addMatchingCerts: " + "adding target cert" + @@ -446,11 +444,8 @@ abstract class Builder { Collection certs = store.getCertificates(selector); for (Certificate cert : certs) { - if (!X509CertImpl.isSelfSigned - ((X509Certificate)cert, buildParams.sigProvider())) { - if (resultCerts.add((X509Certificate)cert)) { - add = true; - } + if (resultCerts.add((X509Certificate)cert)) { + add = true; } } if (!checkAll && add) { diff --git a/src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java b/src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java index 17a11b4e7fe..26f97efc3d9 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java +++ b/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) { - return; - } + caCerts.add(trustedCert); } } @@ -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 - * signature and revocation status of the previous cert. + * Trust anchor certs are not validated. * * 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 - * certificate twice + * Abort if we encounter the same certificate or a certificate with + * 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 (debug != null) { - debug.println("loop detected!!"); - } - throw new CertPathValidatorException("loop detected"); - } + for (X509Certificate cpListCert : certPathList) { + if (repeated(cpListCert, cert)) { + throw new CertPathValidatorException( + "cert with repeated subject, public key, and " + + "subjectAltNames detected"); } } @@ -789,21 +785,48 @@ final class ForwardBuilder extends Builder { */ KeyChecker.verifyCAKeyUsage(cert); } + } - /* - * the following checks are performed even when the cert - * is a trusted cert, since we are only extracting the - * subjectDN, and publicKey from the cert - * in order to verify a previous cert - */ + /** + * Return true if two certificates are equal or have the same subject, + * public key, and subject alternative names. + */ + 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 - * encountered. - */ - if (!currState.keyParamsNeeded()) { - (currState.cert).verify(cert.getPublicKey(), - buildParams.sigProvider()); + /** + * Return true if two certificates have the same subject alternative names. + */ + private static boolean altNamesEqual( + X509Certificate currCert, X509Certificate nextCert) { + 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); } } diff --git a/src/java.base/share/classes/sun/security/provider/certpath/ForwardState.java b/src/java.base/share/classes/sun/security/provider/certpath/ForwardState.java index 24219a5a389..c93f0bcb3a0 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/ForwardState.java +++ b/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 forwardCheckers; - /* Flag indicating if key needing to inherit key parameters has been - * encountered. - */ - boolean keyParamsNeededFlag = false; + /* Flag indicating if last cert in path is self-issued */ + boolean selfIssued; /** * 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)); diff --git a/src/java.base/share/classes/sun/security/provider/certpath/State.java b/src/java.base/share/classes/sun/security/provider/certpath/State.java index a18ef2ad760..890689c9df5 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/State.java +++ b/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(); } diff --git a/src/java.base/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java b/src/java.base/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java index 6fed8f7df35..8c9081b3d9f 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java +++ b/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,18 +131,21 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi { List> adjList = new ArrayList<>(); PKIXCertPathBuilderResult result = buildCertPath(false, adjList); if (result == null) { - if (debug != null) { - debug.println("SunCertPathBuilder.engineBuild: 2nd pass; " + + if (buildParams.certStores().size() > 1 || Builder.USE_AIA) { + if (debug != null) { + debug.println("SunCertPathBuilder.engineBuild: 2nd pass; " + "try building again searching all certstores"); + } + // try again + adjList.clear(); + result = buildCertPath(true, adjList); + if (result != null) { + return result; + } } - // try again - adjList.clear(); - result = buildCertPath(true, adjList); - if (result == null) { - throw new SunCertPathBuilderException("unable to find valid " - + "certification path to requested target", - new AdjacencyList(adjList)); - } + 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() - * recursively for each good cert. + * and check for certs with repeated public key and subject. + * Call depthFirstSearchForward() recursively for each good cert. */ vertices: @@ -346,26 +350,24 @@ 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(); - if (debug != null) - debug.println( - "SunCertPathBuilder.depthFirstSearchForward " + - "using buildParams public key: " + - rootKey.toString()); - } - TrustAnchor anchor = new TrustAnchor - (cert.getSubjectX500Principal(), rootKey, null); + PublicKey rootKey = cert.getPublicKey(); + if (builder.trustAnchor.getTrustedCert() == null) { + rootKey = builder.trustAnchor.getCAPublicKey(); + if (debug != null) + debug.println( + "SunCertPathBuilder.depthFirstSearchForward " + + "using buildParams public key: " + + rootKey.toString()); + } + TrustAnchor anchor = new TrustAnchor + (cert.getSubjectX500Principal(), rootKey, null); - // add the basic checker - basicChecker = new BasicChecker(anchor, buildParams.date(), + // add the basic checker + BasicChecker basicChecker = new BasicChecker(anchor, + buildParams.date(), buildParams.sigProvider(), true); - checkers.add(basicChecker); - } + 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); }