Skip to content

Commit d520021

Browse files
committed
Made the X509RevocationChecker soft-fail hard limit inclusive (failHardMaxTime <= elapsed) to match the javadoc and hard-fail at exactly maxTime; removes the same-millisecond flakiness that RevocationTest.testRevokedEndEntityWithSoftFailure papered over with a 1s Thread.sleep, now dropped.
1 parent 9961e6f commit d520021

3 files changed

Lines changed: 8 additions & 11 deletions

File tree

docs/releasenotes.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ <h2>2.0 Release History</h2>
2323
Date:&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 2026, TBD
2424
<h3>2.1.2 Defects Fixed</h3>
2525
<ul>
26+
<li>The soft-fail hard limit in org.bouncycastle.pkix.jcajce.X509RevocationChecker (Builder.setSoftFailHardLimit) compared the elapsed downtime against the limit with a strict "&lt;", so a failure occurring exactly at maxTime was still treated as soft rather than hard - contrary to the javadoc ("At maxTime any failures will be treated as hard"). With maxTime = 0 this meant the second failure only hard-failed once a full millisecond had elapsed since the first, so on a fast machine two revocation checks within the same millisecond both soft-passed. The comparison is now inclusive ("&lt;="), so the limit fires at maxTime as documented and the maxTime = 0 case deterministically hard-fails the second failure regardless of sub-millisecond timing.</li>
2627
<li>KMIPInputStream (org.bouncycastle.kmip.wire) built its XMLEventReader from a bare XMLInputFactory, so a KMIP XML message carrying a DOCTYPE was processed and external SYSTEM entities were resolved during parsing - an XML External Entity (XXE) exposure permitting local file disclosure via file:// URIs, outbound requests (SSRF) via http:// URIs, and information disclosure through parse error messages. The factory is now configured with SUPPORT_DTD = false (which rejects any DOCTYPE outright) and IS_SUPPORTING_EXTERNAL_ENTITIES = false before the reader is created. KMIP messages without a DOCTYPE parse exactly as before (github #2315).</li>
2728
<li>Reading a GnuPG keybox (org.bouncycastle.gpg.keybox.KeyBox / BcKeyBox / JcaKeyBox) stopped at the first EMPTY_BLOB, treating that free/deleted slot as end-of-file: Blob.getInstance() fell through to returning null (the caller's end-of-file signal) for a BlobType.EMPTY_BLOB, so any key blobs following an empty blob were silently dropped - a keybox laid out as OpenPGP, OpenPGP, OpenPGP, EMPTY_BLOB, OpenPGP returned only three key blobs where gnupg reads four. An empty blob is now skipped by advancing past its declared length and parsing continues with the following blobs; a malformed empty-blob length that would fail to advance or point beyond the buffer still terminates the read (issue #2343).</li>
2829
<li>The CMS SignerInfo decoder org.bouncycastle.asn1.cms.SignerInfo cast the version element directly to ASN1Integer and the trailing unsignedAttrs element directly to ASN1TaggedObject, so a malformed-but-parseable SignerInfo whose first element is not an INTEGER, or whose trailing element is not a [1] tagged object, leaked an unchecked ClassCastException instead of the documented IllegalArgumentException. This could escape the throws-CMSException contract of code reaching the SignerInfo decode via getSignerInfos() / getCounterSignatures() outside the getSignedData() cast guard. Both elements now decode via ASN1Integer.getInstance / ASN1TaggedObject.getInstance, matching org.bouncycastle.asn1.pkcs.SignerInfo; well-formed messages are unaffected (issue #2342).</li>

pkix/src/main/java/org/bouncycastle/pkix/jcajce/X509RevocationChecker.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,10 @@ public void check(Certificate certificate, Collection<String> collection)
507507
if (initial != null)
508508
{
509509
long period = System.currentTimeMillis() - initial.longValue();
510-
if (failHardMaxTime != -1 && failHardMaxTime < period)
510+
// "At maxTime any failures will be treated as hard" - use <= so the limit
511+
// is inclusive (a maxTime of 0 hard-fails the second failure regardless of
512+
// sub-millisecond timing, rather than only once a full millisecond elapses).
513+
if (failHardMaxTime != -1 && failHardMaxTime <= period)
511514
{
512515
throw e;
513516
}

pkix/src/test/java/org/bouncycastle/pkix/test/RevocationTest.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,16 +463,9 @@ public void testRevokedEndEntityWithSoftFailure()
463463
PKIXCertPathValidatorResult result =
464464
(PKIXCertPathValidatorResult)cpv.validate(cp, param);
465465

466-
try
467-
{
468-
Thread.sleep(1000); // make sure some time elapses between first and second failure.
469-
}
470-
catch (Exception e)
471-
{
472-
Thread.currentThread().interrupt();
473-
}
474-
475-
// should fail on the second attempt.
466+
// The first failure is allowed (maxTime 0 permits one soft failure); the second must
467+
// fail hard. With the inclusive (<=) hard-limit check this is deterministic and no
468+
// longer depends on a millisecond elapsing between the two validate() calls.
476469
try
477470
{
478471
result =

0 commit comments

Comments
 (0)