diff --git a/docs/releasenotes.md b/docs/releasenotes.md index 4fb42a2046..f584193aa9 100644 --- a/docs/releasenotes.md +++ b/docs/releasenotes.md @@ -28,6 +28,7 @@ Date: 2026, TBD - An ERSDataGroup recomputed its hash on every request rather than taking it from the cache ERSCachingData exists to provide: the group overrode getHash(), which left the calculateHash() the cache calls unreachable - and wrong, as it copied the member hashes with a loop bounded by the size of the empty list it was copying into, so it would have digested nothing had anything reached it. The computation is back in calculateHash() and the override is gone, so a group's hash is computed once per digest algorithm and previous-chain hash, as every other ERSData's is. The value itself is unchanged. - ERSArchiveTimeStampGenerator rebuilt its reduced hash tree from the data objects on every call, so the usual generateTimeStampRequest() followed by generateArchiveTimeStamp() or generateArchiveTimeStamps() built it twice. The leaves are now built once and dropped when data or a previous chain is added. A Set of the data groups it had been given, which nothing ever read, has gone with it. - Grain-128AEAD returned corrupted plaintext from a decryption driven in chunks. The stream cipher data operator splits a processBytes() call that spans the buffered authentication tag into two output segments, the bytes released from the tag buffer and then the bytes taken straight from the caller's input, and wrote the second segment at the caller's output offset instead of after the first, so the second segment overwrote the head of the first and the tail of the reported output was never written at all. The call still returned the full byte count, and because the engine's state update depends on the input and the keystream rather than on where the output lands, the tag still verified: the wrong plaintext came back with no error raised. Any chunk after the first that carried more than the 8 byte tag length was affected. Grain-128AEAD is the only engine that uses this operator, and one shot decryption, the encryption path and every other AEAD engine were unaffected. The second segment is now written at the advanced offset, matching the equivalent step of the general decryption path (github PR #2447). +- The certificate path validators looked a certificate up in an indirect CRL by serial number alone and then compared the issuer of whichever entry came first, so where two issuers had each revoked the same serial number - an indirect CRL (RFC 5280 sec. 5.2.5) lists certificates from more than one issuer, and a serial number is only unique within its issuer - a certificate whose own entry followed the other issuer's was reported as not revoked. The BC provider's PKIX validator, the pkix X509RevocationChecker and the legacy org.bouncycastle.x509 validator now check every entry carrying the serial number against the issuer it applies to, the one named by its certificateIssuer extension or inherited from the entries before it (sec. 5.3.3), and the provider's X509CRL implements getRevokedCertificate(X509Certificate), the JDK's lookup for indirect CRLs, the same way rather than inheriting the default that assumes a single issuer. - The AEAD stream cipher data operator, which Grain-128AEAD alone uses, wrote its output without first checking that the caller's buffer was long enough, so a short output buffer surfaced as an ArrayIndexOutOfBoundsException from inside the engine rather than as the OutputLengthException the general path reports for every other AEAD engine. Both directions of processBytes(), and processByte(), now check before anything is written or buffered, and only when the call releases output, as the general path does. - The RFC 9709 content-encryption AlgorithmIdentifier, which carries the real algorithm inside the parameters of an outer id-alg-cek-hkdf-sha256, was unwrapped at only one of the points where a CMS recipient makes a decision about it. Key-size validation was corrected for plain key transport in 1.86, but the same call in the KEK, RSA-KTS and KEM recipients, and in the key-transport recipient's own ORI-KEM branch, still compared the recovered key against the outer identifier, which registers no key size, so setKeySizeValidation(true) silently checked nothing there; the setAllowedContentAlgorithms allow-list and the setMinimumTagSize floor were applied to the outer identifier on every recipient family, including the one already corrected, so neither constrained an RFC 9709 message. The unwrap now happens once for the key-size check and once for the two policy checks, and every recipient polices and validates the content-encryption algorithm the message actually carries. A recipient with no allow-list, no tag floor and no key-size validation configured behaves exactly as before; a caller who listed id-alg-cek-hkdf-sha256 in an allow-list in order to admit RFC 9709 messages must now list the content-encryption algorithms themselves (github PR #2446). - A CMS message whose EncryptedContentInfo named the RFC 9709 key derivation but carried no readable content-encryption AlgorithmIdentifier in its parameters was reported as a NullPointerException, or as an IllegalArgumentException from the ASN.1 decoder, out of methods declared to throw CMSException, RecipientInformation.getContent() among them. The four places that resolve the wrapper - the CEK derivation, the content cipher selection, the key-size check, and the recipient's allowed-algorithm and tag-size checks - now share one resolver, which reports an absent or unreadable inner algorithm as a CMSException. diff --git a/pkix/src/main/java/org/bouncycastle/pkix/jcajce/CertPathValidatorUtilities.java b/pkix/src/main/java/org/bouncycastle/pkix/jcajce/CertPathValidatorUtilities.java index 58af8b9b97..d2456f5701 100644 --- a/pkix/src/main/java/org/bouncycastle/pkix/jcajce/CertPathValidatorUtilities.java +++ b/pkix/src/main/java/org/bouncycastle/pkix/jcajce/CertPathValidatorUtilities.java @@ -403,36 +403,12 @@ protected static void getCertStatus( if (isIndirect) { - crl_entry = crl.getRevokedCertificate(getSerialNumber(cert)); + crl_entry = getIndirectCRLEntry(crl, getSerialNumber(cert), getEncodedIssuerPrincipal(cert)); if (crl_entry == null) { return; } - - X500Principal certIssuer; - try - { - certIssuer = crl_entry.getCertificateIssuer(); - } - catch (RuntimeException e) - { - // getCertificateIssuer() builds a new X500Principal from the entry's certificateIssuer - // DN, which can throw an unchecked IllegalArgumentException on a name that decodes - // structurally but is semantically invalid. Fail closed with the checked contract type - // rather than let it escape (or swallow it to null, which would fail revocation open). - throw new AnnotatedException("CRL entry certificate issuer could not be parsed.", e); - } - - if (certIssuer == null) - { - certIssuer = getIssuerPrincipal(crl); - } - - if (!getEncodedIssuerPrincipal(cert).equals(certIssuer)) - { - return; - } } else if (!getEncodedIssuerPrincipal(cert).equals(getIssuerPrincipal(crl))) { @@ -479,6 +455,60 @@ else if (!getEncodedIssuerPrincipal(cert).equals(getIssuerPrincipal(crl))) } } + /** + * Find the entry of an indirect CRL for the certificate with the given serial number and issuer. + * An indirect CRL (RFC 5280 sec. 5.2.5) lists certificates from more than one issuer and a serial + * number is only unique within its issuer, so the entry cannot be located by serial number alone: + * every entry carrying the serial number is checked against the issuer it applies to, the one + * named by its certificateIssuer extension or inherited from the entries before it (sec. 5.3.3). + * + * @return the entry naming the certificate, or null if the CRL carries none. + */ + private static X509CRLEntry getIndirectCRLEntry(X509CRL crl, BigInteger serialNumber, X500Principal certIssuer) + throws AnnotatedException + { + Set entries = crl.getRevokedCertificates(); + if (entries == null) + { + return null; + } + + for (Iterator it = entries.iterator(); it.hasNext();) + { + X509CRLEntry entry = (X509CRLEntry)it.next(); + if (!serialNumber.equals(entry.getSerialNumber())) + { + continue; + } + + X500Principal entryIssuer; + try + { + entryIssuer = entry.getCertificateIssuer(); + } + catch (RuntimeException e) + { + // getCertificateIssuer() builds a new X500Principal from the entry's certificateIssuer + // DN, which can throw an unchecked IllegalArgumentException on a name that decodes + // structurally but is semantically invalid. Fail closed with the checked contract type + // rather than let it escape (or swallow it to null, which would fail revocation open). + throw new AnnotatedException("CRL entry certificate issuer could not be parsed.", e); + } + + if (entryIssuer == null) + { + entryIssuer = getIssuerPrincipal(crl); + } + + if (certIssuer.equals(entryIssuer)) + { + return entry; + } + } + + return null; + } + /** * Return the next working key inheriting DSA parameters if necessary. *

diff --git a/pkix/src/test/java/org/bouncycastle/cert/test/AllTests.java b/pkix/src/test/java/org/bouncycastle/cert/test/AllTests.java index d611199934..5ed836f76a 100644 --- a/pkix/src/test/java/org/bouncycastle/cert/test/AllTests.java +++ b/pkix/src/test/java/org/bouncycastle/cert/test/AllTests.java @@ -35,6 +35,7 @@ public void testSimpleTests() new GOST3410_2012CMSTest(), new GOSTR3410_2012_256GenerateCertificate(), new IndirectCRLSignerTest(), + new IndirectCRLSerialCollisionTest(), new MLDSACredentialsTest(), new PKCS10Test(), new RelatedCertificateDescriptorTest(), diff --git a/pkix/src/test/java/org/bouncycastle/cert/test/IndirectCRLSerialCollisionTest.java b/pkix/src/test/java/org/bouncycastle/cert/test/IndirectCRLSerialCollisionTest.java new file mode 100644 index 0000000000..4694838dae --- /dev/null +++ b/pkix/src/test/java/org/bouncycastle/cert/test/IndirectCRLSerialCollisionTest.java @@ -0,0 +1,286 @@ +package org.bouncycastle.cert.test; + +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.Security; +import java.security.cert.CRL; +import java.security.cert.CertPath; +import java.security.cert.CertPathValidator; +import java.security.cert.CertPathValidatorException; +import java.security.cert.CertStore; +import java.security.cert.CertificateFactory; +import java.security.cert.CollectionCertStoreParameters; +import java.security.cert.PKIXParameters; +import java.security.cert.TrustAnchor; +import java.security.cert.X509CRL; +import java.security.cert.X509CRLEntry; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.bouncycastle.asn1.x500.X500Name; +import org.bouncycastle.asn1.x509.BasicConstraints; +import org.bouncycastle.asn1.x509.CRLReason; +import org.bouncycastle.asn1.x509.Extension; +import org.bouncycastle.asn1.x509.ExtensionsGenerator; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.asn1.x509.GeneralNames; +import org.bouncycastle.asn1.x509.IssuingDistributionPoint; +import org.bouncycastle.asn1.x509.KeyUsage; +import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo; +import org.bouncycastle.cert.X509v2CRLBuilder; +import org.bouncycastle.cert.X509v3CertificateBuilder; +import org.bouncycastle.cert.jcajce.JcaX509CRLConverter; +import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.bouncycastle.operator.ContentSigner; +import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; +import org.bouncycastle.pkix.jcajce.X509RevocationChecker; +import org.bouncycastle.util.CollectionStore; +import org.bouncycastle.util.test.SimpleTest; + +/** + * An indirect CRL (RFC 5280 sec. 5.2.5) lists certificates from more than one issuer, and a serial + * number is only unique within its issuer, so two of its entries may carry the same serial number + * for different issuers, each applying to the issuer named by its own or a preceding entry's + * certificateIssuer extension (sec. 5.3.3). An entry looked up by serial number alone is therefore + * whichever of them comes first, and a certificate whose own entry followed another issuer's entry + * with the same serial number was reported as not revoked. + */ +public class IndirectCRLSerialCollisionTest + extends SimpleTest +{ + private static final String SIG_ALG = "SHA256withRSA"; + private static final BigInteger SHARED_SERIAL = BigInteger.valueOf(7); + private static final X500Name OTHER_CA = new X500Name("CN=Other.CA, O=Test-PKI, C=DE"); + + private KeyPair caKey; + private X500Name caName; + private X509Certificate caCert; + + public String getName() + { + return "IndirectCRLSerialCollision"; + } + + public void performTest() + throws Exception + { + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA", "BC"); + kpg.initialize(1024); + + caKey = kpg.generateKeyPair(); + caName = new X500Name("CN=Test-Root.CA, O=Test-PKI, C=DE"); + caCert = selfSigned(caKey, caName); + + X509Certificate ee = certificate(kpg.generateKeyPair().getPublic(), caName, + new X500Name("CN=Test-EE, O=Test-PKI, C=DE"), SHARED_SERIAL); + // the same serial number under the other issuer: only that issuer's entry applies to it. + X509Certificate other = certificate(kpg.generateKeyPair().getPublic(), OTHER_CA, + new X500Name("CN=Other-EE, O=Test-PKI, C=DE"), SHARED_SERIAL); + + // the entry naming the CA follows the other issuer's entry for the same serial number. + X509CRL otherFirst = indirectCrl(new X500Name[]{ OTHER_CA, caName }); + // the compatibility half: the entry naming the CA comes first. + X509CRL caFirst = indirectCrl(new X500Name[]{ caName, OTHER_CA }); + // only the other issuer has revoked this serial number. + X509CRL otherOnly = indirectCrl(new X500Name[]{ OTHER_CA }); + + revokedBy(ee, caFirst, false); + revokedBy(ee, otherFirst, false); + validatesWith(ee, otherOnly, false); + + revokedBy(ee, caFirst, true); + revokedBy(ee, otherFirst, true); + validatesWith(ee, otherOnly, true); + + entryLookup(ee, other, otherFirst, otherOnly); + } + + private void revokedBy(X509Certificate cert, X509CRL crl, boolean useChecker) + throws Exception + { + try + { + validate(cert, crl, useChecker); + fail("revoked certificate accepted (" + (useChecker ? "checker" : "validator") + ")"); + } + catch (CertPathValidatorException e) + { + String chain = messageChain(e); + + isTrue("unexpected failure: " + chain, chain.indexOf("revocation") >= 0 || chain.indexOf("revoked") >= 0); + } + } + + private void validatesWith(X509Certificate cert, X509CRL crl, boolean useChecker) + throws Exception + { + validate(cert, crl, useChecker); + } + + /** + * The BC provider's validator processes the CRL from a certificate store; the pkix + * X509RevocationChecker takes it from a Store and runs as a PKIXCertPathChecker. + */ + private void validate(X509Certificate cert, X509CRL crl, boolean useChecker) + throws Exception + { + Set anchors = new HashSet(); + anchors.add(new TrustAnchor(caCert, null)); + + PKIXParameters params = new PKIXParameters(anchors); + + if (useChecker) + { + List crls = new ArrayList(); + crls.add(crl); + + params.setRevocationEnabled(false); + params.addCertPathChecker(new X509RevocationChecker.Builder(new TrustAnchor(caCert, null)) + .addCrls(new CollectionStore(crls)) + .build()); + } + else + { + List storeContents = new ArrayList(); + storeContents.add(crl); + + params.addCertStore(CertStore.getInstance("Collection", new CollectionCertStoreParameters(storeContents), "BC")); + params.setRevocationEnabled(true); + } + + CertPath path = CertificateFactory.getInstance("X.509", "BC").generateCertPath(Collections.singletonList(cert)); + + CertPathValidator.getInstance("PKIX", "BC").validate(path, params); + } + + /** + * X509CRL.getRevokedCertificate(X509Certificate) is the JDK's lookup for indirect CRLs, keyed + * by issuer and serial number; it has to find the entry naming the certificate's issuer, and + * no other. + */ + private void entryLookup(X509Certificate ee, X509Certificate other, X509CRL otherFirst, X509CRL otherOnly) + throws Exception + { + // the two entries for the shared serial number encode identically and differ only in the + // issuer they inherit, so the set of entries must keep them both. + isTrue("entry for a second issuer lost from the set of entries", otherFirst.getRevokedCertificates().size() == 4); + + X509CRLEntry entry = otherFirst.getRevokedCertificate(ee); + + isTrue("no entry found for the certificate", entry != null); + isTrue("another issuer's entry returned for the certificate", namesIssuer(entry, otherFirst, caName)); + + entry = otherFirst.getRevokedCertificate(other); + + isTrue("no entry found for the other issuer's certificate", entry != null); + isTrue("wrong entry returned for the other issuer's certificate", namesIssuer(entry, otherFirst, OTHER_CA)); + + isTrue("entry of another issuer returned for an unrevoked certificate", otherOnly.getRevokedCertificate(ee) == null); + } + + private static boolean namesIssuer(X509CRLEntry entry, X509CRL crl, X500Name issuer) + { + X500Name entryIssuer = (entry.getCertificateIssuer() == null) + ? X500Name.getInstance(crl.getIssuerX500Principal().getEncoded()) + : X500Name.getInstance(entry.getCertificateIssuer().getEncoded()); + + return issuer.equals(entryIssuer); + } + + private static String messageChain(Throwable t) + { + StringBuffer sb = new StringBuffer(); + + while (t != null) + { + sb.append(t.getMessage()).append(" | "); + t = t.getCause(); + } + + return sb.toString(); + } + + private X509Certificate selfSigned(KeyPair key, X500Name subject) + throws Exception + { + X509v3CertificateBuilder b = builder(subject, subject, key.getPublic(), BigInteger.ONE); + + b.addExtension(Extension.basicConstraints, true, new BasicConstraints(true)); + b.addExtension(Extension.keyUsage, true, new KeyUsage(KeyUsage.keyCertSign | KeyUsage.cRLSign)); + + return sign(b, key.getPrivate()); + } + + private X509Certificate certificate(PublicKey pub, X500Name issuer, X500Name subject, BigInteger serial) + throws Exception + { + X509v3CertificateBuilder b = builder(issuer, subject, pub, serial); + + b.addExtension(Extension.basicConstraints, true, new BasicConstraints(false)); + + return sign(b, caKey.getPrivate()); + } + + /** + * An indirect CRL issued by the CA, revoking SHARED_SERIAL once under each of the given + * issuers, in that order. Each issuer's entries are introduced by one carrying the + * certificateIssuer extension, which the entries after it inherit (RFC 5280 sec. 5.3.3). + */ + private X509CRL indirectCrl(X500Name[] issuers) + throws Exception + { + long now = System.currentTimeMillis(); + Date revocationDate = new Date(now - 3600000L); + X509v2CRLBuilder b = new X509v2CRLBuilder(caName, revocationDate); + + b.setNextUpdate(new Date(now + 30L * 24 * 3600 * 1000)); + b.addExtension(Extension.issuingDistributionPoint, true, + new IssuingDistributionPoint(null, false, false, null, true, false)); + + for (int i = 0; i != issuers.length; i++) + { + ExtensionsGenerator gen = new ExtensionsGenerator(); + + gen.addExtension(Extension.certificateIssuer, true, new GeneralNames(new GeneralName(issuers[i]))); + + b.addCRLEntry(BigInteger.valueOf(100 + i), revocationDate, gen.generate()); + b.addCRLEntry(SHARED_SERIAL, revocationDate, CRLReason.keyCompromise); + } + + ContentSigner cs = new JcaContentSignerBuilder(SIG_ALG).setProvider("BC").build(caKey.getPrivate()); + + return new JcaX509CRLConverter().setProvider("BC").getCRL(b.build(cs)); + } + + private X509v3CertificateBuilder builder(X500Name issuer, X500Name subject, PublicKey pub, BigInteger serial) + { + long now = System.currentTimeMillis(); + + return new X509v3CertificateBuilder(issuer, serial, new Date(now - 24L * 3600 * 1000), + new Date(now + 365L * 24 * 3600 * 1000), subject, SubjectPublicKeyInfo.getInstance(pub.getEncoded())); + } + + private X509Certificate sign(X509v3CertificateBuilder b, PrivateKey key) + throws Exception + { + ContentSigner cs = new JcaContentSignerBuilder(SIG_ALG).setProvider("BC").build(key); + + return new JcaX509CertificateConverter().setProvider("BC").getCertificate(b.build(cs)); + } + + public static void main(String[] args) + { + Security.addProvider(new BouncyCastleProvider()); + + runTest(new IndirectCRLSerialCollisionTest()); + } +} diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLEntryObject.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLEntryObject.java index 634d3944e1..217dfd473f 100644 --- a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLEntryObject.java +++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLEntryObject.java @@ -24,6 +24,7 @@ import org.bouncycastle.asn1.x509.GeneralName; import org.bouncycastle.asn1.x509.GeneralNames; import org.bouncycastle.asn1.x509.TBSCertList; +import org.bouncycastle.util.Objects; import org.bouncycastle.util.Strings; /** @@ -206,10 +207,13 @@ public boolean equals(Object other) } } - return this.c.equals(otherBC.c); + // an entry of an indirect CRL applies to the issuer named by its own or a preceding + // entry's certificateIssuer extension (RFC 5280 sec. 5.3.3), so two entries with the + // same encoding under different issuers are different entries. + return this.c.equals(otherBC.c) && Objects.areEqual(this.certificateIssuer, otherBC.certificateIssuer); } - return super.equals(this); + return super.equals(other); } public byte[] getEncoded() diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLImpl.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLImpl.java index 9bf39e1fd7..d2bbc876a1 100644 --- a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLImpl.java +++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLImpl.java @@ -456,6 +456,41 @@ public X509CRLEntry getRevokedCertificate(BigInteger serialNumber) return null; } + /** + * Return the entry for the given certificate, matching its issuer as well as its serial number: + * an indirect CRL (RFC 5280 sec. 5.2.5) lists certificates from more than one issuer and a serial + * number is only unique within its issuer, so the entry cannot be located by serial number alone. + */ + public X509CRLEntry getRevokedCertificate(X509Certificate certificate) + { + if (!isIndirect) + { + return super.getRevokedCertificate(certificate); + } + + BigInteger serialNumber = certificate.getSerialNumber(); + X500Name issuer = X500Name.getInstance(certificate.getIssuerX500Principal().getEncoded()); + + Enumeration certs = c.getRevokedCertificateEnumeration(); + + X500Name previousCertificateIssuer = null; // the issuer + while (certs.hasMoreElements()) + { + TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement(); + X500Name certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, previousCertificateIssuer); + + if (entry.getUserCertificate().hasValue(serialNumber) + && issuer.equals(certificateIssuer == null ? c.getIssuer() : certificateIssuer)) + { + return new X509CRLEntryObject(entry, isIndirect, previousCertificateIssuer); + } + + previousCertificateIssuer = certificateIssuer; + } + + return null; + } + public Set getRevokedCertificates() { Set entrySet = loadCRLEntries(); diff --git a/prov/src/main/java/org/bouncycastle/jce/provider/CertPathValidatorUtilities.java b/prov/src/main/java/org/bouncycastle/jce/provider/CertPathValidatorUtilities.java index 6c3b04223a..0f2e86501e 100644 --- a/prov/src/main/java/org/bouncycastle/jce/provider/CertPathValidatorUtilities.java +++ b/prov/src/main/java/org/bouncycastle/jce/provider/CertPathValidatorUtilities.java @@ -824,41 +824,12 @@ protected static void getCertStatus( X509CRLEntry crl_entry; if (isIndirect) { - crl_entry = crl.getRevokedCertificate(getSerialNumber(cert)); + crl_entry = getIndirectCRLEntry(crl, getSerialNumber(cert), PrincipalUtils.getEncodedIssuerPrincipal(cert)); if (crl_entry == null) { return; } - - X500Principal certificateIssuer; - try - { - certificateIssuer = crl_entry.getCertificateIssuer(); - } - catch (RuntimeException e) - { - // getCertificateIssuer() builds a new X500Principal from the entry's certificateIssuer - // DN, which can throw an unchecked IllegalArgumentException on a name that decodes - // structurally but is semantically invalid. Fail closed with the checked contract type - // rather than let it escape (or swallow it to null, which would fail revocation open). - throw new AnnotatedException("CRL entry certificate issuer could not be parsed.", e); - } - - X500Name certIssuer; - if (certificateIssuer == null) - { - certIssuer = PrincipalUtils.getIssuerPrincipal(crl); - } - else - { - certIssuer = PrincipalUtils.getX500Name(certificateIssuer); - } - - if (!PrincipalUtils.getEncodedIssuerPrincipal(cert).equals(certIssuer)) - { - return; - } } else if (!PrincipalUtils.getEncodedIssuerPrincipal(cert).equals(PrincipalUtils.getIssuerPrincipal(crl))) { @@ -902,6 +873,65 @@ else if (!PrincipalUtils.getEncodedIssuerPrincipal(cert).equals(PrincipalUtils.g } } + /** + * Find the entry of an indirect CRL for the certificate with the given serial number and issuer. + * An indirect CRL (RFC 5280 sec. 5.2.5) lists certificates from more than one issuer and a serial + * number is only unique within its issuer, so the entry cannot be located by serial number alone: + * every entry carrying the serial number is checked against the issuer it applies to, the one + * named by its certificateIssuer extension or inherited from the entries before it (sec. 5.3.3). + * + * @return the entry naming the certificate, or null if the CRL carries none. + */ + private static X509CRLEntry getIndirectCRLEntry(X509CRL crl, BigInteger serialNumber, X500Name certIssuer) + throws AnnotatedException + { + Set entries = crl.getRevokedCertificates(); + if (entries == null) + { + return null; + } + + for (Iterator it = entries.iterator(); it.hasNext();) + { + X509CRLEntry entry = (X509CRLEntry)it.next(); + if (!serialNumber.equals(entry.getSerialNumber())) + { + continue; + } + + X500Principal certificateIssuer; + try + { + certificateIssuer = entry.getCertificateIssuer(); + } + catch (RuntimeException e) + { + // getCertificateIssuer() builds a new X500Principal from the entry's certificateIssuer + // DN, which can throw an unchecked IllegalArgumentException on a name that decodes + // structurally but is semantically invalid. Fail closed with the checked contract type + // rather than let it escape (or swallow it to null, which would fail revocation open). + throw new AnnotatedException("CRL entry certificate issuer could not be parsed.", e); + } + + X500Name entryIssuer; + if (certificateIssuer == null) + { + entryIssuer = PrincipalUtils.getIssuerPrincipal(crl); + } + else + { + entryIssuer = PrincipalUtils.getX500Name(certificateIssuer); + } + + if (certIssuer.equals(entryIssuer)) + { + return entry; + } + } + + return null; + } + /** * Fetches delta CRLs according to RFC 3280 section 5.2.4. * diff --git a/prov/src/main/java/org/bouncycastle/x509/CertPathValidatorUtilities.java b/prov/src/main/java/org/bouncycastle/x509/CertPathValidatorUtilities.java index 1d3d8f044a..c00c5b3321 100644 --- a/prov/src/main/java/org/bouncycastle/x509/CertPathValidatorUtilities.java +++ b/prov/src/main/java/org/bouncycastle/x509/CertPathValidatorUtilities.java @@ -465,36 +465,12 @@ protected static void getCertStatus( if (isIndirect) { - crl_entry = crl.getRevokedCertificate(getSerialNumber(cert)); + crl_entry = getIndirectCRLEntry(crl, getSerialNumber(cert), getEncodedIssuerPrincipal(cert)); if (crl_entry == null) { return; } - - X500Principal certIssuer; - try - { - certIssuer = crl_entry.getCertificateIssuer(); - } - catch (RuntimeException e) - { - // getCertificateIssuer() builds a new X500Principal from the entry's certificateIssuer - // DN, which can throw an unchecked IllegalArgumentException on a name that decodes - // structurally but is semantically invalid. Fail closed with the checked contract type - // rather than let it escape (or swallow it to null, which would fail revocation open). - throw new AnnotatedException("CRL entry certificate issuer could not be parsed.", e); - } - - if (certIssuer == null) - { - certIssuer = getIssuerPrincipal(crl); - } - - if (!getEncodedIssuerPrincipal(cert).equals(certIssuer)) - { - return; - } } else if (!getEncodedIssuerPrincipal(cert).equals(getIssuerPrincipal(crl))) { @@ -541,6 +517,60 @@ else if (!getEncodedIssuerPrincipal(cert).equals(getIssuerPrincipal(crl))) } } + /** + * Find the entry of an indirect CRL for the certificate with the given serial number and issuer. + * An indirect CRL (RFC 5280 sec. 5.2.5) lists certificates from more than one issuer and a serial + * number is only unique within its issuer, so the entry cannot be located by serial number alone: + * every entry carrying the serial number is checked against the issuer it applies to, the one + * named by its certificateIssuer extension or inherited from the entries before it (sec. 5.3.3). + * + * @return the entry naming the certificate, or null if the CRL carries none. + */ + private static X509CRLEntry getIndirectCRLEntry(X509CRL crl, BigInteger serialNumber, X500Principal certIssuer) + throws AnnotatedException + { + Set entries = crl.getRevokedCertificates(); + if (entries == null) + { + return null; + } + + for (Iterator it = entries.iterator(); it.hasNext();) + { + X509CRLEntry entry = (X509CRLEntry)it.next(); + if (!serialNumber.equals(entry.getSerialNumber())) + { + continue; + } + + X500Principal entryIssuer; + try + { + entryIssuer = entry.getCertificateIssuer(); + } + catch (RuntimeException e) + { + // getCertificateIssuer() builds a new X500Principal from the entry's certificateIssuer + // DN, which can throw an unchecked IllegalArgumentException on a name that decodes + // structurally but is semantically invalid. Fail closed with the checked contract type + // rather than let it escape (or swallow it to null, which would fail revocation open). + throw new AnnotatedException("CRL entry certificate issuer could not be parsed.", e); + } + + if (entryIssuer == null) + { + entryIssuer = getIssuerPrincipal(crl); + } + + if (certIssuer.equals(entryIssuer)) + { + return entry; + } + } + + return null; + } + /** * Return the next working key inheriting DSA parameters if necessary. *