Skip to content

Commit f42e970

Browse files
peterdettmanpeterdettman
committed
Tidied the CRL entry loops in X509CRLImpl and the legacy jce.provider.X509CRLObject: each entry's certificate issuer is resolved once and handed to the entry, isRevoked() tracks it from null like the other loops and skips the scan when a non-indirect CRL cannot cover the certificate's issuer, and loadCRLEntries() is folded into getRevokedCertificates().
1 parent a24b5bd commit f42e970

4 files changed

Lines changed: 91 additions & 93 deletions

File tree

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLEntryObject.java‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,29 +49,21 @@ protected X509CRLEntryObject(TBSCertList.CRLEntry c)
4949
}
5050

5151
/**
52-
* Constructor for CRLEntries of indirect CRLs. If <code>isIndirect</code>
53-
* is <code>false</code> {@link #getCertificateIssuer()} will always
54-
* return <code>null</code>, <code>previousCertificateIssuer</code> is
55-
* ignored. If this <code>isIndirect</code> is specified and this CRLEntry
56-
* has no certificate issuer CRL entry extension
57-
* <code>previousCertificateIssuer</code> is returned by
58-
* {@link #getCertificateIssuer()}.
59-
*
52+
* Constructor for a CRLEntry whose certificate issuer the caller has already worked out with
53+
* {@link #loadCertificateIssuer}, which it has to do anyway to track the issuer from one entry
54+
* to the next.
55+
*
6056
* @param c
6157
* TBSCertList.CRLEntry object.
62-
* @param isIndirect
63-
* <code>true</code> if the corresponding CRL is a indirect
64-
* CRL.
65-
* @param previousCertificateIssuer
66-
* Certificate issuer of the previous CRLEntry.
58+
* @param certificateIssuer
59+
* the certificate issuer of this CRLEntry, <code>null</code> for the CRL issuer.
6760
*/
6861
protected X509CRLEntryObject(
6962
TBSCertList.CRLEntry c,
70-
boolean isIndirect,
71-
X500Name previousCertificateIssuer)
63+
X500Name certificateIssuer)
7264
{
7365
this.c = c;
74-
this.certificateIssuer = loadCertificateIssuer(c, isIndirect, previousCertificateIssuer);
66+
this.certificateIssuer = certificateIssuer;
7567
}
7668

7769
/**

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLImpl.java‎

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -419,39 +419,21 @@ public Date getNextUpdate()
419419
return null == nextUpdate ? null : nextUpdate.getDate();
420420
}
421421

422-
private Set loadCRLEntries()
423-
{
424-
Set entrySet = new HashSet();
425-
Enumeration certs = c.getRevokedCertificateEnumeration();
426-
427-
X500Name previousCertificateIssuer = null; // the issuer
428-
while (certs.hasMoreElements())
429-
{
430-
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
431-
X509CRLEntryObject crlEntry = new X509CRLEntryObject(entry, isIndirect, previousCertificateIssuer);
432-
entrySet.add(crlEntry);
433-
previousCertificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, previousCertificateIssuer);
434-
}
435-
436-
return entrySet;
437-
}
438-
439422
// returns the first entry with the serial number whatever its issuer - the cert path validators rely on that.
440423
public X509CRLEntry getRevokedCertificate(BigInteger serialNumber)
441424
{
442425
Enumeration certs = c.getRevokedCertificateEnumeration();
443426

444-
X500Name previousCertificateIssuer = null; // the issuer
427+
X500Name certificateIssuer = null; // null means the CRL issuer
445428
while (certs.hasMoreElements())
446429
{
447430
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
431+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
448432

449433
if (entry.getUserCertificate().hasValue(serialNumber))
450434
{
451-
return new X509CRLEntryObject(entry, isIndirect, previousCertificateIssuer);
435+
return new X509CRLEntryObject(entry, certificateIssuer);
452436
}
453-
454-
previousCertificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, previousCertificateIssuer);
455437
}
456438

457439
return null;
@@ -475,34 +457,42 @@ public X509CRLEntry getRevokedCertificate(X509Certificate certificate)
475457

476458
Enumeration certs = c.getRevokedCertificateEnumeration();
477459

478-
X500Name previousCertificateIssuer = null; // the issuer
460+
X500Name certificateIssuer = null; // null means the CRL issuer
479461
while (certs.hasMoreElements())
480462
{
481463
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
482-
X500Name certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, previousCertificateIssuer);
464+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
483465

484-
if (entry.getUserCertificate().hasValue(serialNumber)
485-
&& issuer.equals(certificateIssuer == null ? c.getIssuer() : certificateIssuer))
466+
// a serial number is only unique within its issuer, so another issuer's entry is not this one.
467+
if (entry.getUserCertificate().hasValue(serialNumber) &&
468+
issuer.equals(certificateIssuer == null ? c.getIssuer() : certificateIssuer))
486469
{
487-
return new X509CRLEntryObject(entry, isIndirect, previousCertificateIssuer);
470+
return new X509CRLEntryObject(entry, certificateIssuer);
488471
}
489-
490-
previousCertificateIssuer = certificateIssuer;
491472
}
492473

493474
return null;
494475
}
495476

496477
public Set getRevokedCertificates()
497478
{
498-
Set entrySet = loadCRLEntries();
479+
Enumeration certs = c.getRevokedCertificateEnumeration();
480+
if (!certs.hasMoreElements())
481+
{
482+
return null;
483+
}
499484

500-
if (!entrySet.isEmpty())
485+
Set entrySet = new HashSet();
486+
X500Name certificateIssuer = null; // null means the CRL issuer
487+
do
501488
{
502-
return Collections.unmodifiableSet(entrySet);
489+
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
490+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
491+
entrySet.add(new X509CRLEntryObject(entry, certificateIssuer));
503492
}
493+
while (certs.hasMoreElements());
504494

505-
return null;
495+
return Collections.unmodifiableSet(entrySet);
506496
}
507497

508498
public byte[] getTBSCertList()
@@ -666,8 +656,6 @@ public boolean isRevoked(Certificate cert)
666656

667657
Enumeration revokedCerts = c.getRevokedCertificateEnumeration();
668658

669-
X500Name caName = c.getIssuer();
670-
671659
if (revokedCerts.hasMoreElements())
672660
{
673661
BigInteger serial;
@@ -697,22 +685,28 @@ public boolean isRevoked(Certificate cert)
697685
issuer = certStruct.getIssuer();
698686
}
699687

700-
while (revokedCerts.hasMoreElements())
688+
boolean issuedByCRLIssuer = issuer.equals(c.getIssuer());
689+
if (!isIndirect && !issuedByCRLIssuer)
690+
{
691+
return false;
692+
}
693+
694+
X500Name certificateIssuer = null; // null means the CRL issuer
695+
696+
do
701697
{
702698
TBSCertList.CRLEntry entry = TBSCertList.CRLEntry.getInstance(revokedCerts.nextElement());
703699

704-
caName = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, caName);
705-
if (caName == null)
706-
{
707-
caName = c.getIssuer(); // an entry with no certificate issuer of its own is the CRL issuer's
708-
}
700+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
709701

710702
// a serial number is only unique within its issuer, so another issuer's entry is not this one.
711-
if (entry.getUserCertificate().hasValue(serial) && caName.equals(issuer))
703+
if (entry.getUserCertificate().hasValue(serial) &&
704+
(certificateIssuer == null ? issuedByCRLIssuer : issuer.equals(certificateIssuer)))
712705
{
713706
return true;
714707
}
715708
}
709+
while (revokedCerts.hasMoreElements());
716710
}
717711

718712
return false;

‎prov/src/main/java/org/bouncycastle/jce/provider/X509CRLEntryObject.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,26 @@ public X509CRLEntryObject(
6767
TBSCertList.CRLEntry c,
6868
boolean isIndirect,
6969
X500Name previousCertificateIssuer)
70+
{
71+
this(c, loadCertificateIssuer(c, isIndirect, previousCertificateIssuer));
72+
}
73+
74+
/**
75+
* Constructor for a CRLEntry whose certificate issuer the caller has already worked out with
76+
* {@link #loadCertificateIssuer}, which it has to do anyway to track the issuer from one entry
77+
* to the next.
78+
*
79+
* @param c
80+
* TBSCertList.CRLEntry object.
81+
* @param certificateIssuer
82+
* the certificate issuer of this CRLEntry, <code>null</code> for the CRL issuer.
83+
*/
84+
X509CRLEntryObject(
85+
TBSCertList.CRLEntry c,
86+
X500Name certificateIssuer)
7087
{
7188
this.c = c;
72-
this.certificateIssuer = loadCertificateIssuer(c, isIndirect, previousCertificateIssuer);
89+
this.certificateIssuer = certificateIssuer;
7390
}
7491

7592
/**

‎prov/src/main/java/org/bouncycastle/jce/provider/X509CRLObject.java‎

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -303,53 +303,44 @@ public Date getNextUpdate()
303303
return null == nextUpdate ? null : nextUpdate.getDate();
304304
}
305305

306-
private Set loadCRLEntries()
307-
{
308-
Set entrySet = new HashSet();
309-
Enumeration certs = c.getRevokedCertificateEnumeration();
310-
311-
X500Name previousCertificateIssuer = null; // the issuer
312-
while (certs.hasMoreElements())
313-
{
314-
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
315-
X509CRLEntryObject crlEntry = new X509CRLEntryObject(entry, isIndirect, previousCertificateIssuer);
316-
entrySet.add(crlEntry);
317-
previousCertificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, previousCertificateIssuer);
318-
}
319-
320-
return entrySet;
321-
}
322-
323306
public X509CRLEntry getRevokedCertificate(BigInteger serialNumber)
324307
{
325308
Enumeration certs = c.getRevokedCertificateEnumeration();
326309

327-
X500Name previousCertificateIssuer = null; // the issuer
310+
X500Name certificateIssuer = null; // null means the CRL issuer
328311
while (certs.hasMoreElements())
329312
{
330313
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
314+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
331315

332316
if (entry.getUserCertificate().hasValue(serialNumber))
333317
{
334-
return new X509CRLEntryObject(entry, isIndirect, previousCertificateIssuer);
318+
return new X509CRLEntryObject(entry, certificateIssuer);
335319
}
336-
337-
previousCertificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, previousCertificateIssuer);
338320
}
339321

340322
return null;
341323
}
342324

343325
public Set getRevokedCertificates()
344326
{
345-
Set entrySet = loadCRLEntries();
327+
Enumeration certs = c.getRevokedCertificateEnumeration();
328+
if (!certs.hasMoreElements())
329+
{
330+
return null;
331+
}
346332

347-
if (!entrySet.isEmpty())
333+
Set entrySet = new HashSet();
334+
X500Name certificateIssuer = null; // null means the CRL issuer
335+
do
348336
{
349-
return Collections.unmodifiableSet(entrySet);
337+
TBSCertList.CRLEntry entry = (TBSCertList.CRLEntry)certs.nextElement();
338+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
339+
entrySet.add(new X509CRLEntryObject(entry, certificateIssuer));
350340
}
341+
while (certs.hasMoreElements());
351342

352-
return null;
343+
return Collections.unmodifiableSet(entrySet);
353344
}
354345

355346
public byte[] getTBSCertList()
@@ -529,9 +520,7 @@ public boolean isRevoked(Certificate cert)
529520

530521
Enumeration certs = c.getRevokedCertificateEnumeration();
531522

532-
X500Name caName = c.getIssuer();
533-
534-
if (certs != null)
523+
if (certs.hasMoreElements())
535524
{
536525
BigInteger serial;
537526
X500Name issuer;
@@ -560,22 +549,28 @@ public boolean isRevoked(Certificate cert)
560549
issuer = certStruct.getIssuer();
561550
}
562551

563-
while (certs.hasMoreElements())
552+
boolean issuedByCRLIssuer = issuer.equals(c.getIssuer());
553+
if (!isIndirect && !issuedByCRLIssuer)
554+
{
555+
return false;
556+
}
557+
558+
X500Name certificateIssuer = null; // null means the CRL issuer
559+
560+
do
564561
{
565562
TBSCertList.CRLEntry entry = TBSCertList.CRLEntry.getInstance(certs.nextElement());
566563

567-
caName = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, caName);
568-
if (caName == null)
569-
{
570-
caName = c.getIssuer(); // an entry with no certificate issuer of its own is the CRL issuer's
571-
}
564+
certificateIssuer = X509CRLEntryObject.loadCertificateIssuer(entry, isIndirect, certificateIssuer);
572565

573566
// a serial number is only unique within its issuer, so another issuer's entry is not this one.
574-
if (entry.getUserCertificate().hasValue(serial) && caName.equals(issuer))
567+
if (entry.getUserCertificate().hasValue(serial) &&
568+
(certificateIssuer == null ? issuedByCRLIssuer : issuer.equals(certificateIssuer)))
575569
{
576570
return true;
577571
}
578572
}
573+
while (certs.hasMoreElements());
579574
}
580575

581576
return false;

0 commit comments

Comments
 (0)