Skip to content

Commit 8e2752c

Browse files
dghgitdghgit
committed
CertPathBuilder: count only non-self-issued intermediates against the maximum path length, and carry the caller's maximum path length and excluded certificates into the indirect CRL signer's path build.
1 parent 004f0ba commit 8e2752c

14 files changed

Lines changed: 407 additions & 23 deletions

File tree

‎docs/releasenotes.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ Date: 2026, TBD
4545
- KeyAgreement.init() reported an initialisation it could not carry out as an unchecked exception for the unified and VKO agreements, where the JCA declares InvalidKeyException and InvalidAlgorithmParameterException. The ECCDHU and X25519U/X448U services took a plain UserKeyingMaterialSpec, or no spec at all, and carried it into the agreement, throwing ClassCastException from the cast that follows; the DHU and DH MQV services accepted the initialisation and then threw NullPointerException from doPhase, where the absent spec is read back; and the ECGOST3410 and ECGOST3410-2012 VKO services put a null where the UKM belongs, although RFC 7836 sec. 4.3 makes the UKM optional with the value 1. Each of them now reports a parameter error as ECMQV already did, and the VKO agreements apply the RFC 7836 default, so an agreement initialised without a UserKeyingMaterialSpec derives the key a UKM of 1 gives rather than failing. The user keying material itself was not being dropped anywhere: every key agreement service carrying a key derivation function was checked for it.
4646
- The RFC 5990 RSA-KTS CMS recipients (JceKTSKeyTransEnvelopedRecipient and JceKTSKeyTransAuthenticatedRecipient, through JceKTSKeyUnwrapper) took the keyLength carried in the message's RsaKemParameters as the number of octets for the key encapsulation mechanism to derive, and only compared it with the key-wrapping algorithm once the derivation had been done. EnvelopedData is not integrity protected and the recipient reaches this before anything about the message has been verified, so the declared length decided how much the key derivation function was asked to produce, and a large enough one also overflowed the bit count it was converted into. RFC 5990 sec. 4 fixes the length of the derived key as the key length of the data encapsulation mechanism's key-wrapping algorithm, so the recipient now derives that length and rejects a keyLength which disagrees with it before deriving anything, reporting it as the CMSException the API declares - the check the RFC 9629 KEMRecipientInfo path already made against its kekLength.
4747
- The read-side well-formedness checks on UTCTime and GeneralizedTime bounded the day at 01-31 without regard to the month, so the 30th of February, the 31st of April and the 29th of February in a common year were all accepted and the lenient calendar behind getDate() rolled each into the following month: "180230101423Z" was read back as the 2nd of March 2018, an instant some other encoding already denotes, and a certificate validity or CRL update time could name a day that does not exist. The day is now checked against the length of the month, February following the Gregorian leap rule and a UTCTime's two-digit year resolved through the RFC 5280 sec. 4.1.2.5.1 window, so such a value is rejected on read as OpenSSL rejects it. The JDK's own CertificateFactory accepts and rolls these, so org.bouncycastle.asn1.allow_non_der_time - already the switch for the write-side DER restrictions - also admits the impossible day for a caller that has to read what it accepts; the impossible month and the zero day stay rejected either way.
48+
- When the provider's CertPathBuilder validated the certification path of an indirect CRL's signer, it did so with the builder defaults rather than the caller's PKIXBuilderParameters: the maximum path length was always 5 and the excluded certificates set was always empty, so a signer the caller had excluded could still vouch for a CRL, a caller's tighter path length limit was not applied to the signer's path, and a caller's looser one (or no limit at all) could leave a longer but otherwise valid signer path rejected. Both settings now carry through to the CRL signer's path build.
49+
- The provider's CertPathBuilder applied PKIXBuilderParameters.setMaxPathLength() one certificate too leniently, building a path with one intermediate certificate more than the limit (so a limit of 0 admitted a target and one intermediate, and the default of 5 admitted six), and counted self-issued certificates against it. The limit now counts the non-self-issued intermediate certificates, as PKIXBuilderParameters defines it, so a path which only built because of the extra certificate, one with six intermediates under the default limit for example, now needs the limit raised.
4850
- The DSTU 7624 (Kalyna) GCM mode and MAC, KGCMBlockCipher and KGMac, accepted an operation with both the associated text and the data empty, which DSTU 7624:2014 sec. 12.1 excludes. Its tag is then E_K(0), which is the GHASH key itself: Kalyna GCM does not mask the tag with a nonce-derived block as NIST GCM does, so a caller who authenticated an empty message disclosed the authentication key, and anyone holding it can construct a different message with the same tag as any other message seen under that key. The decrypting side accepted E_K(0) as the tag of an empty message in the same way. Both directions now reject the case, encryption and KGMac with a DataLengthException and decryption with an InvalidCipherTextException; either one of the associated text and the data may still be empty. The Mac.DSTU7624GMAC services were affected through KGMac.
4951
- DSTU7624Mac accepted an empty message and gave it the tag of a single all-zero block, so the two distinct messages could not be told apart. DSTU 7624:2014 sec. 9.1 defines the MAC for one or more blocks, and doFinal now rejects an empty message with a DataLengthException, as it already rejected a partial block.
5052

‎pkix/src/test/java/org/bouncycastle/cert/test/IndirectCRLSignerTest.java‎

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.security.cert.X509CertSelector;
1616
import java.security.cert.X509Certificate;
1717
import java.util.ArrayList;
18+
import java.util.Collections;
1819
import java.util.Date;
1920
import java.util.HashSet;
2021
import java.util.List;
@@ -36,6 +37,7 @@
3637
import org.bouncycastle.cert.jcajce.JcaX509CRLConverter;
3738
import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
3839
import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils;
40+
import org.bouncycastle.jcajce.PKIXExtendedBuilderParameters;
3941
import org.bouncycastle.jce.provider.BouncyCastleProvider;
4042
import org.bouncycastle.operator.ContentSigner;
4143
import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
@@ -70,6 +72,9 @@ public void performTest()
7072

7173
singleGenerationValidates();
7274
rolledRootReportsTheRealFailure();
75+
excludedSignerIsNotUsed();
76+
maxPathLengthBoundsSignerPath();
77+
maxPathLengthUnboundedAdmitsLongSignerPath();
7378
}
7479

7580
/**
@@ -109,6 +114,69 @@ private void rolledRootReportsTheRealFailure()
109114
}
110115
}
111116

117+
/**
118+
* The caller's excluded certificates apply to the CRL signer's certification path too, so an
119+
* excluded signer cannot vouch for the CRL.
120+
*/
121+
private void excludedSignerIsNotUsed()
122+
throws Exception
123+
{
124+
Pki pki = buildPki(1, 0);
125+
126+
signerRejected("excluded CRL signer accepted", pki, 5, Collections.singleton(pki.signers.get(0)));
127+
}
128+
129+
/**
130+
* The caller's maximum path length bounds the CRL signer's certification path, which here has two
131+
* intermediates between the signer and the root while the certificate under check has none.
132+
*/
133+
private void maxPathLengthBoundsSignerPath()
134+
throws Exception
135+
{
136+
Pki pki = buildPki(1, 2);
137+
138+
if (validate(pki, 2, null) == null)
139+
{
140+
fail("CRL signer path within the maximum path length rejected");
141+
}
142+
143+
signerRejected("CRL signer path longer than the maximum path length accepted", pki, 1, null);
144+
}
145+
146+
/**
147+
* A caller who lifts the path length limit gets it lifted for the CRL signer's path as well,
148+
* rather than having that path held to the builder default of 5.
149+
*/
150+
private void maxPathLengthUnboundedAdmitsLongSignerPath()
151+
throws Exception
152+
{
153+
Pki pki = buildPki(1, 6);
154+
155+
if (validate(pki, -1, null) == null)
156+
{
157+
fail("CRL signer path with an unlimited maximum path length rejected");
158+
}
159+
160+
signerRejected("CRL signer path longer than the default maximum path length accepted", pki, 5, null);
161+
}
162+
163+
private void signerRejected(String failMessage, Pki pki, int maxPathLength, Set excluded)
164+
throws Exception
165+
{
166+
try
167+
{
168+
validate(pki, maxPathLength, excluded);
169+
fail(failMessage);
170+
}
171+
catch (CertPathBuilderException e)
172+
{
173+
String chain = messageChain(e);
174+
175+
isTrue("failure of the CRL signer's own path not reported: " + chain,
176+
chain.indexOf("CertPath for CRL signer failed to validate") >= 0);
177+
}
178+
}
179+
112180
private static String messageChain(Throwable t)
113181
{
114182
StringBuffer sb = new StringBuffer();
@@ -124,6 +192,12 @@ private static String messageChain(Throwable t)
124192

125193
private Object validate(Pki pki)
126194
throws Exception
195+
{
196+
return validate(pki, 5, null);
197+
}
198+
199+
private Object validate(Pki pki, int maxPathLength, Set excluded)
200+
throws Exception
127201
{
128202
Set anchors = new HashSet();
129203
for (int i = 0; i != pki.roots.size(); i++)
@@ -132,6 +206,7 @@ private Object validate(Pki pki)
132206
}
133207

134208
List storeContents = new ArrayList(pki.signers);
209+
storeContents.addAll(pki.intermediates);
135210
storeContents.add(pki.subCa);
136211
storeContents.add(pki.crl);
137212

@@ -141,14 +216,22 @@ private Object validate(Pki pki)
141216
PKIXBuilderParameters params = new PKIXBuilderParameters(anchors, target);
142217
params.addCertStore(CertStore.getInstance("Collection", new CollectionCertStoreParameters(storeContents), "BC"));
143218
params.setRevocationEnabled(true);
219+
params.setMaxPathLength(maxPathLength);
144220

145-
return CertPathBuilder.getInstance("PKIX", "BC").build(params);
221+
CertPathBuilder builder = CertPathBuilder.getInstance("PKIX", "BC");
222+
if (excluded != null)
223+
{
224+
return builder.build(new PKIXExtendedBuilderParameters.Builder(params).addExcludedCerts(excluded).build());
225+
}
226+
227+
return builder.build(params);
146228
}
147229

148230
private static class Pki
149231
{
150232
final List roots = new ArrayList();
151233
final List signers = new ArrayList();
234+
final List intermediates = new ArrayList();
152235
X509Certificate subCa;
153236
X509CRL crl;
154237
}
@@ -161,6 +244,16 @@ private static class Pki
161244
*/
162245
private Pki buildPki(int generations)
163246
throws Exception
247+
{
248+
return buildPki(generations, 0);
249+
}
250+
251+
/**
252+
* As above, with each generation's CRL signer issued at the end of a chain of signerDepth
253+
* intermediate CAs under its root, all of them covered by the same indirect CRL.
254+
*/
255+
private Pki buildPki(int generations, int signerDepth)
256+
throws Exception
164257
{
165258
Pki pki = new Pki();
166259

@@ -180,10 +273,21 @@ private Pki buildPki(int generations)
180273
rootKeys.add(rootKey);
181274
pki.roots.add(root);
182275

276+
KeyPair issuerKey = rootKey;
277+
X509Certificate issuer = root;
278+
for (int d = 1; d <= signerDepth; d++)
279+
{
280+
KeyPair caKey = kpg.generateKeyPair();
281+
issuer = subCa(caKey.getPublic(), new X500Name("CN=Test-Int" + d + ".CA, O=Test-PKI, C=DE, SERIALNUMBER=" + g),
282+
issuerKey, issuer, crlDp);
283+
issuerKey = caKey;
284+
pki.intermediates.add(issuer);
285+
}
286+
183287
KeyPair signerKey = kpg.generateKeyPair();
184288
// Self-referencing CRLDP: the signer's own path is validated with revocation enabled
185289
// before its key is trusted, so the signer needs a resolvable CRLDP of its own.
186-
pki.signers.add(crlSigner(signerKey.getPublic(), signerDn, rootKey, root, crlDp));
290+
pki.signers.add(crlSigner(signerKey.getPublic(), signerDn, issuerKey, issuer, crlDp));
187291
signerKeys.add(signerKey);
188292
}
189293

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ protected CertPathBuilderResult build(X509Certificate tbvCert,
162162
// test if certificate path exceeds maximum length
163163
if (pkixParams.getMaxPathLength() != -1)
164164
{
165-
if (tbvPath.size() - 1 > pkixParams.getMaxPathLength())
165+
if (countIntermediates(tbvPath, tbvCert) > pkixParams.getMaxPathLength())
166166
{
167167
return null;
168168
}
@@ -288,4 +288,26 @@ private static class NodeBudgetExceededException
288288
super(message);
289289
}
290290
}
291+
292+
/**
293+
* Return the number of non-self-issued intermediate certificates the path would hold with tbvCert
294+
* added to it, the first certificate in the path being the target.
295+
*/
296+
private static int countIntermediates(List tbvPath, X509Certificate tbvCert)
297+
{
298+
if (tbvPath.isEmpty())
299+
{
300+
return 0;
301+
}
302+
303+
int count = CertPathValidatorUtilities.isSelfIssued(tbvCert) ? 0 : 1;
304+
for (int i = 1; i < tbvPath.size(); i++)
305+
{
306+
if (!CertPathValidatorUtilities.isSelfIssued((X509Certificate)tbvPath.get(i)))
307+
{
308+
count++;
309+
}
310+
}
311+
return count;
312+
}
291313
}

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ protected CertPathBuilderResult build(X509Certificate tbvCert,
171171
// test if certificate path exceeds maximum length
172172
if (pkixParams.getMaxPathLength() != -1)
173173
{
174-
if (tbvPath.size() - 1 > pkixParams.getMaxPathLength())
174+
if (countIntermediates(tbvPath, tbvCert) > pkixParams.getMaxPathLength())
175175
{
176176
return null;
177177
}
@@ -297,4 +297,26 @@ private static class NodeBudgetExceededException
297297
super(message);
298298
}
299299
}
300+
301+
/**
302+
* Return the number of non-self-issued intermediate certificates the path would hold with tbvCert
303+
* added to it, the first certificate in the path being the target.
304+
*/
305+
private static int countIntermediates(List tbvPath, X509Certificate tbvCert)
306+
{
307+
if (tbvPath.isEmpty())
308+
{
309+
return 0;
310+
}
311+
312+
int count = CertPathValidatorUtilities.isSelfIssued(tbvCert) ? 0 : 1;
313+
for (int i = 1; i < tbvPath.size(); i++)
314+
{
315+
if (!CertPathValidatorUtilities.isSelfIssued((X509Certificate)tbvPath.get(i)))
316+
{
317+
count++;
318+
}
319+
}
320+
return count;
321+
}
300322
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public CertPathValidatorResult engineValidate(
5959
InvalidAlgorithmParameterException
6060
{
6161
PKIXExtendedParameters paramsPKIX;
62+
PKIXExtendedBuilderParameters builderParams = null;
6263
if (params instanceof PKIXParameters)
6364
{
6465
PKIXExtendedParameters.Builder paramsPKIXBldr = new PKIXExtendedParameters.Builder((PKIXParameters)params);
@@ -75,7 +76,8 @@ public CertPathValidatorResult engineValidate(
7576
}
7677
else if (params instanceof PKIXExtendedBuilderParameters)
7778
{
78-
paramsPKIX = ((PKIXExtendedBuilderParameters)params).getBaseParameters();
79+
builderParams = (PKIXExtendedBuilderParameters)params;
80+
paramsPKIX = builderParams.getBaseParameters();
7981
}
8082
else if (params instanceof PKIXExtendedParameters)
8183
{
@@ -333,7 +335,7 @@ else if (params instanceof PKIXExtendedParameters)
333335
// 6.1.3
334336
//
335337

336-
RFC3280CertPathUtilities.processCertA(certPath, paramsPKIX, validityDate, revocationChecker, index,
338+
RFC3280CertPathUtilities.processCertA(certPath, paramsPKIX, builderParams, validityDate, revocationChecker, index,
337339
workingPublicKey, verificationAlreadyPerformed, workingIssuerName, sign);
338340

339341
RFC3280CertPathUtilities.processCertBC(certPath, index, nameConstraintValidator, isForCRLCheck);

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public CertPathValidatorResult engineValidate(
6767
InvalidAlgorithmParameterException
6868
{
6969
PKIXExtendedParameters paramsPKIX;
70+
PKIXExtendedBuilderParameters builderParams = null;
7071
if (params instanceof PKIXParameters)
7172
{
7273
PKIXExtendedParameters.Builder paramsPKIXBldr = new PKIXExtendedParameters.Builder((PKIXParameters)params);
@@ -83,7 +84,8 @@ public CertPathValidatorResult engineValidate(
8384
}
8485
else if (params instanceof PKIXExtendedBuilderParameters)
8586
{
86-
paramsPKIX = ((PKIXExtendedBuilderParameters)params).getBaseParameters();
87+
builderParams = (PKIXExtendedBuilderParameters)params;
88+
paramsPKIX = builderParams.getBaseParameters();
8789
}
8890
else if (params instanceof PKIXExtendedParameters)
8991
{
@@ -352,7 +354,7 @@ else if (params instanceof PKIXExtendedParameters)
352354
// 6.1.3
353355
//
354356

355-
RFC3280CertPathUtilities.processCertA(certPath, paramsPKIX, validityDate, revocationChecker, index,
357+
RFC3280CertPathUtilities.processCertA(certPath, paramsPKIX, builderParams, validityDate, revocationChecker, index,
356358
workingPublicKey, verificationAlreadyPerformed, workingIssuerName, sign);
357359

358360
RFC3280CertPathUtilities.processCertBC(certPath, index, nameConstraintValidator, isForCRLCheck);

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package org.bouncycastle.jce.provider;
2+
3+
import java.security.PublicKey;
4+
import java.security.cert.CertPath;
5+
import java.security.cert.X509Certificate;
6+
import java.util.Date;
7+
8+
import org.bouncycastle.jcajce.PKIXCertRevocationCheckerParameters;
9+
import org.bouncycastle.jcajce.PKIXExtendedBuilderParameters;
10+
import org.bouncycastle.jcajce.PKIXExtendedParameters;
11+
12+
/**
13+
* Revocation checker parameters which also carry the builder parameters the validation was started
14+
* with, so the certification path build for an indirect CRL's signer is held to the caller's
15+
* maximum path length and excluded certificates rather than the builder defaults.
16+
*/
17+
class ProvCertRevocationCheckerParameters
18+
extends PKIXCertRevocationCheckerParameters
19+
{
20+
private final PKIXExtendedBuilderParameters builderParams;
21+
22+
ProvCertRevocationCheckerParameters(PKIXExtendedParameters paramsPKIX, PKIXExtendedBuilderParameters builderParams,
23+
Date validDate, CertPath certPath, int index, X509Certificate signingCert, PublicKey workingPublicKey)
24+
{
25+
super(paramsPKIX, validDate, certPath, index, signingCert, workingPublicKey);
26+
27+
this.builderParams = builderParams;
28+
}
29+
30+
/**
31+
* Return the builder parameters of the path under validation, null if it was not started by a builder.
32+
*/
33+
PKIXExtendedBuilderParameters getBuilderParams()
34+
{
35+
return builderParams;
36+
}
37+
38+
static PKIXExtendedBuilderParameters getBuilderParams(PKIXCertRevocationCheckerParameters params)
39+
{
40+
if (params instanceof ProvCertRevocationCheckerParameters)
41+
{
42+
return ((ProvCertRevocationCheckerParameters)params).getBuilderParams();
43+
}
44+
return null;
45+
}
46+
}

0 commit comments

Comments
 (0)