FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

KeyAgreement: report an initialisation the unified and VKO agreements… · rootvector2/bc-java@033fe35 · GitHub

Repository navigation

Commit 033fe35

Browse files
committed
KeyAgreement: report an initialisation the unified and VKO agreements cannot carry out as the parameter error the JCA declares rather than as an unchecked exception, and apply the RFC 7836 default UKM of 1 when none is given.
1 parent 1121e2b commit 033fe35

8 files changed

Lines changed: 281 additions & 2 deletions

File tree

‎docs/releasenotes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ Date: 2026, TBD
4242
- Both copies of PKIXCertPathReviewer (org.bouncycastle.pkix.jcajce and the legacy org.bouncycastle.x509) took the first date-valid CRL issued by the certificate's issuer as an answer about that certificate, applying neither of the RFC 5280 sec. 6.3.3 rules that decide whether a CRL covers it: the (b)(2)(i) match between a name in the CRL's issuing distribution point and a name in the certificate's distribution point, and the (d) intersection of the revocation reasons the two assert. Only the (b)(2)(ii) to (iv) onlyContains booleans were applied. A CA-signed, in-date CRL with no entries, scoped to another distribution point or to a partition of the revocation reasons, was therefore reported as proof of non-revocation - isValidCertPath() true with an empty error list for a certificate its own CA had revoked for key compromise, where CertPathValidator("PKIX") rejects the same chain against the same trust anchor - and it suppressed the distribution point fetch that would otherwise have retrieved the authoritative CRL. Where the reviewer makes the trust decision rather than serving as diagnostics beside a real validation this is a revocation bypass, and SignedMailValidator (bcmail) reaches it with the CRLs carried inside the signed message. Both copies now apply the (b)(2)(i) name match and require a CRL to cover every revocation reason before it can settle the certificate's status, through the public PKIXCRLValidator helpers the validation engine already uses, and keep looking when a candidate does not qualify - falling back, as before, to the distribution point fetch and then to the existing "no valid CRL found" error. A CRL carrying no issuing distribution point, one naming the certificate's own distribution point, and one naming the certificate issuer (the distribution point the engine falls back to) are all still accepted.
4343
- RFC3280CertPathUtilities.checkCRL threw java.lang.NullPointerException rather than a CertPathValidatorException when every candidate CRL for a distribution point was skipped instead of rejected, which is what happens when the reasons a CRL covers add nothing to those already checked - the RFC 5280 sec. 6.3.3 (d) case - since the exception it rethrows is only ever recorded in a catch block. Validation failed closed either way, but outside the declared contract of CertPathValidator.validate(); a run with nothing recorded now reports "No valid CRL found.". All four copies are corrected: pkix, prov, and the prov jdk1.3 and jdk1.4 overlays.
4444
- TupleHash prefixed an element of 2^28 bytes or more with a length computed in int arithmetic: org.bouncycastle.crypto.digests.XofUtils built the encode_string prefix of NIST SP 800-185 sec. 2.3.3 as left_encode(len * 8) with len an int, so the bit length wrapped before it reached the long parameter it was passed to. A single 256 MiB update wrapped it negative, and left_encode sizes its output by shifting its argument right eight bits at a time, which never reaches zero from a negative value, so the call did not return; at 512 MiB the length wrapped to zero and the element carried the prefix of an empty one, letting two different tuples absorb the same byte string - the ambiguity the tuple encoding of sec. 5.3 exists to prevent. The multiply is now widened, as the other left_encode and right_encode call sites in CSHAKEDigest, KMAC, TupleHash and ParallelHash already were, and left_encode and right_encode refuse a negative length rather than spinning on one, so a negative output length handed to the three-argument doFinal of TupleHash, ParallelHash or KMAC reports IllegalArgumentException instead of not returning. An element below 2^28 bytes is unaffected, the two arithmetics agreeing exactly there.
45+
- 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.
4546

4647
### 2.1.3 Additional Features and Functionality
4748

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dh/KeyAgreementSpi.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,16 @@ else if (params instanceof UserKeyingMaterialSpec)
342342
this.g = privKey.getParams().getG();
343343
}
344344

345+
// the unified and MQV agreements take their ephemeral keys from the spec, and doPhase reads it back - without one they would fail there rather than here.
346+
if (unifiedAgreement != null && dheParameters == null)
347+
{
348+
throw new InvalidAlgorithmParameterException(kaAlgorithm + " key agreement requires DHUParameterSpec for initialisation");
349+
}
350+
if (mqvAgreement != null && mqvParameters == null)
351+
{
352+
throw new InvalidAlgorithmParameterException(kaAlgorithm + " key agreement requires MQVParameterSpec for initialisation");
353+
}
354+
345355
this.x = privKey.getX();
346356
}
347357

@@ -357,6 +367,16 @@ protected void engineInit(
357367
throw new InvalidKeyException("DHKeyAgreement requires DHPrivateKey");
358368
}
359369

370+
// the ephemeral half of a unified or MQV agreement only arrives with a spec, so this entry point cannot initialise one.
371+
if (unifiedAgreement != null)
372+
{
373+
throw new InvalidKeyException(kaAlgorithm + " key agreement requires DHUParameterSpec for initialisation");
374+
}
375+
if (mqvAgreement != null)
376+
{
377+
throw new InvalidKeyException(kaAlgorithm + " key agreement requires MQVParameterSpec for initialisation");
378+
}
379+
360380
DHPrivateKey privKey = (DHPrivateKey)key;
361381

362382
this.p = privKey.getParams().getP();

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ else if (parameterSpec instanceof DHUParameterSpec)
271271
{
272272
throw new InvalidAlgorithmParameterException("no KDF specified for UserKeyingMaterialSpec");
273273
}
274+
if (!(agreement instanceof RawAgreement))
275+
{
276+
throw new InvalidAlgorithmParameterException(kaAlgorithm + " key agreement requires "
277+
+ getSimpleName(DHUParameterSpec.class) + " for initialisation");
278+
}
274279
ECPrivateKeyParameters privKey = (ECPrivateKeyParameters)ECUtils.generatePrivateKeyParameter((PrivateKey)key);
275280
this.parameters = privKey.getParameters();
276281
ukmParameters = (parameterSpec instanceof UserKeyingMaterialSpec) ? ((UserKeyingMaterialSpec)parameterSpec).getUserKeyingMaterial() : null;

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ecgost/KeyAgreementSpi.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
public class KeyAgreementSpi
2828
extends BaseAgreementSpi
2929
{
30+
private static final byte[] DEFAULT_UKM = new byte[]{ 1 };
31+
3032
private static final X9IntegerConverter converter = new X9IntegerConverter();
3133

3234
private String kaAlgorithm;
@@ -109,7 +111,10 @@ protected void doInitFromKey(Key key, AlgorithmParameterSpec parameterSpec, Secu
109111

110112
ECPrivateKeyParameters privKey = (ECPrivateKeyParameters)ECUtil.generatePrivateKeyParameter((PrivateKey)key);
111113
this.parameters = privKey.getParameters();
112-
ukmParameters = (parameterSpec instanceof UserKeyingMaterialSpec) ? ((UserKeyingMaterialSpec)parameterSpec).getUserKeyingMaterial() : null;
114+
// RFC 7836 sec. 4.3: the UKM is optional for VKO and takes the value 1 when it is not given.
115+
ukmParameters = (parameterSpec instanceof UserKeyingMaterialSpec)
116+
? ((UserKeyingMaterialSpec)parameterSpec).getUserKeyingMaterial()
117+
: DEFAULT_UKM;
113118
agreement.init(new ParametersWithUKM(privKey, ukmParameters));
114119
}
115120

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ecgost12/KeyAgreementSpi.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
public class KeyAgreementSpi
2626
extends BaseAgreementSpi
2727
{
28+
private static final byte[] DEFAULT_UKM = new byte[]{ 1 };
29+
2830
private String kaAlgorithm;
2931

3032
private ECDomainParameters parameters;
@@ -105,7 +107,10 @@ protected void doInitFromKey(Key key, AlgorithmParameterSpec parameterSpec, Secu
105107

106108
ECPrivateKeyParameters privKey = (ECPrivateKeyParameters)ECUtil.generatePrivateKeyParameter((PrivateKey)key);
107109
this.parameters = privKey.getParameters();
108-
ukmParameters = (parameterSpec instanceof UserKeyingMaterialSpec) ? ((UserKeyingMaterialSpec)parameterSpec).getUserKeyingMaterial() : null;
110+
// RFC 7836 sec. 4.3: the UKM is optional for VKO and takes the value 1 when it is not given.
111+
ukmParameters = (parameterSpec instanceof UserKeyingMaterialSpec)
112+
? ((UserKeyingMaterialSpec)parameterSpec).getUserKeyingMaterial()
113+
: DEFAULT_UKM;
109114
agreement.init(new ParametersWithUKM(privKey, ukmParameters));
110115
}
111116

‎prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/edec/KeyAgreementSpi.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ else if (priv instanceof X448PrivateKeyParameters)
9292
}
9393
else if (params != null)
9494
{
95+
if (kaAlgorithm.indexOf('U') >= 0)
96+
{
97+
throw new InvalidAlgorithmParameterException(kaAlgorithm + " key agreement requires DHUParameterSpec for initialisation");
98+
}
99+
95100
agreement.init(priv);
96101

97102
if (params instanceof UserKeyingMaterialSpec)
@@ -110,6 +115,11 @@ else if (params != null)
110115
}
111116
else
112117
{
118+
if (kaAlgorithm.indexOf('U') >= 0)
119+
{
120+
throw new InvalidAlgorithmParameterException(kaAlgorithm + " key agreement requires DHUParameterSpec for initialisation");
121+
}
122+
113123
agreement.init(priv);
114124
}
115125

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
package org.bouncycastle.jce.provider.test;
2+
3+
import java.security.InvalidAlgorithmParameterException;
4+
import java.security.InvalidKeyException;
5+
import java.security.KeyPair;
6+
import java.security.KeyPairGenerator;
7+
import java.security.NoSuchAlgorithmException;
8+
import java.security.Security;
9+
import java.security.spec.AlgorithmParameterSpec;
10+
11+
import javax.crypto.KeyAgreement;
12+
13+
import org.bouncycastle.jcajce.spec.UserKeyingMaterialSpec;
14+
import org.bouncycastle.jce.provider.BouncyCastleProvider;
15+
import org.bouncycastle.jce.spec.ECNamedCurveGenParameterSpec;
16+
import org.bouncycastle.util.Arrays;
17+
import org.bouncycastle.util.test.SimpleTest;
18+
19+
/**
20+
* KeyAgreement.init() declares InvalidKeyException and InvalidAlgorithmParameterException, so an
21+
* initialisation it cannot carry out has to be reported as one of those.
22+
* <p>
23+
* The unified agreements (ECCDHU, DHU, X25519U/X448U) take the ephemeral half of the agreement from
24+
* a DHUParameterSpec, and the MQV agreements from an MQVParameterSpec. Given anything else - a plain
25+
* UserKeyingMaterialSpec, or no spec at all - ECMQV reported it, while the three unified families
26+
* carried the wrong object into the agreement instead: the EC and XDH ones threw ClassCastException
27+
* from the cast that follows, and the DH one accepted the initialisation and then threw
28+
* NullPointerException from doPhase, where the absent spec is read back.
29+
* </p><p>
30+
* The VKO agreements are the other side of the same coin: RFC 7836 sec. 4.3 makes the UKM optional
31+
* with the value 1, but initialising without a UserKeyingMaterialSpec put a null where the UKM
32+
* belongs and threw NullPointerException.
33+
* </p>
34+
*/
35+
public class KeyAgreementInitParametersTest
36+
extends SimpleTest
37+
{
38+
private static final byte[] UKM = new byte[]{ 1, 2, 3, 4, 5, 6, 7, 8 };
39+
40+
public String getName()
41+
{
42+
return "KeyAgreementInitParameters";
43+
}
44+
45+
public void performTest()
46+
throws Exception
47+
{
48+
KeyPair ecKey = generate("EC", new ECNamedCurveGenParameterSpec("P-256"));
49+
KeyPair dhKey = generate("DH", null);
50+
KeyPair xKey = generate("X25519", null);
51+
KeyPair gostKey = generate("ECGOST3410-2012", new ECNamedCurveGenParameterSpec("Tc26-Gost-3410-12-256-paramSetA"));
52+
53+
// the ephemeral half only arrives with the right spec - anything else is a parameter error
54+
checkRejectsPlainUKM("ECCDHUwithSHA256KDF", ecKey);
55+
checkRejectsPlainUKM("ECMQVwithSHA256KDF", ecKey);
56+
checkRejectsPlainUKM("DHUwithSHA256KDF", dhKey);
57+
checkRejectsPlainUKM("MQVwithSHA256KDF", dhKey);
58+
checkRejectsPlainUKM("X25519UwithSHA256KDF", xKey);
59+
60+
checkRejectsNoSpec("ECCDHUwithSHA256KDF", ecKey);
61+
checkRejectsNoSpec("ECMQVwithSHA256KDF", ecKey);
62+
checkRejectsNoSpec("DHUwithSHA256KDF", dhKey);
63+
checkRejectsNoSpec("MQVwithSHA256KDF", dhKey);
64+
checkRejectsNoSpec("X25519UwithSHA256KDF", xKey);
65+
66+
checkVKODefaultUKM(gostKey);
67+
checkUKMReachesTheKDF(ecKey, xKey);
68+
}
69+
70+
private void checkRejectsPlainUKM(String algorithm, KeyPair kp)
71+
throws Exception
72+
{
73+
KeyAgreement agreement = agreement(algorithm);
74+
if (agreement == null)
75+
{
76+
return;
77+
}
78+
79+
try
80+
{
81+
agreement.init(kp.getPrivate(), new UserKeyingMaterialSpec(UKM));
82+
83+
fail(algorithm + " accepted a UserKeyingMaterialSpec in place of its own spec");
84+
}
85+
catch (InvalidAlgorithmParameterException e)
86+
{
87+
// expected
88+
}
89+
}
90+
91+
private void checkRejectsNoSpec(String algorithm, KeyPair kp)
92+
throws Exception
93+
{
94+
KeyAgreement agreement = agreement(algorithm);
95+
if (agreement == null)
96+
{
97+
return;
98+
}
99+
100+
try
101+
{
102+
agreement.init(kp.getPrivate());
103+
104+
fail(algorithm + " accepted an initialisation with no spec at all");
105+
}
106+
catch (InvalidKeyException e)
107+
{
108+
// expected
109+
}
110+
}
111+
112+
/**
113+
* RFC 7836 sec. 4.3: the UKM is optional for VKO and takes the value 1 when it is absent.
114+
*/
115+
private void checkVKODefaultUKM(KeyPair kp)
116+
throws Exception
117+
{
118+
KeyAgreement agreement = agreement("ECGOST3410-2012-256");
119+
if (agreement == null)
120+
{
121+
return;
122+
}
123+
124+
KeyPair other = generate("ECGOST3410-2012", new ECNamedCurveGenParameterSpec("Tc26-Gost-3410-12-256-paramSetA"));
125+
126+
byte[] withoutSpec = deriveVKO(kp, other, null);
127+
byte[] withOne = deriveVKO(kp, other, new byte[]{ 1 });
128+
byte[] withTwo = deriveVKO(kp, other, new byte[]{ 2 });
129+
130+
isTrue("VKO without a UKM does not use the RFC 7836 default of 1",
131+
Arrays.areEqual(withoutSpec, withOne));
132+
isTrue("VKO derived the same key for two different UKMs",
133+
!Arrays.areEqual(withOne, withTwo));
134+
}
135+
136+
/**
137+
* A UKM that does not reach the key derivation function would leave the derived key unchanged.
138+
*/
139+
private void checkUKMReachesTheKDF(KeyPair ecKey, KeyPair xKey)
140+
throws Exception
141+
{
142+
String[] algorithms = new String[]{ "ECDHwithSHA256KDF", "ECCDHwithSHA256KDF", "X25519withSHA256KDF",
143+
"X25519withSHA256HKDF" };
144+
KeyPair[] keys = new KeyPair[]{ ecKey, ecKey, xKey, xKey };
145+
146+
for (int i = 0; i != algorithms.length; i++)
147+
{
148+
if (agreement(algorithms[i]) == null)
149+
{
150+
continue;
151+
}
152+
153+
KeyPair other = (keys[i] == ecKey)
154+
? generate("EC", new ECNamedCurveGenParameterSpec("P-256"))
155+
: generate("X25519", null);
156+
157+
byte[] first = derive(algorithms[i], keys[i], other, UKM);
158+
byte[] second = derive(algorithms[i], keys[i], other, new byte[]{ 9, 9, 9, 9, 9, 9, 9, 9 });
159+
160+
isTrue(algorithms[i] + " derived the same key for two different UKMs",
161+
!Arrays.areEqual(first, second));
162+
}
163+
}
164+
165+
private byte[] deriveVKO(KeyPair a, KeyPair b, byte[] ukm)
166+
throws Exception
167+
{
168+
KeyAgreement agreement = KeyAgreement.getInstance("ECGOST3410-2012-256", "BC");
169+
170+
if (ukm == null)
171+
{
172+
agreement.init(a.getPrivate());
173+
}
174+
else
175+
{
176+
agreement.init(a.getPrivate(), new UserKeyingMaterialSpec(ukm));
177+
}
178+
179+
agreement.doPhase(b.getPublic(), true);
180+
181+
return agreement.generateSecret("AES").getEncoded();
182+
}
183+
184+
private byte[] derive(String algorithm, KeyPair a, KeyPair b, byte[] ukm)
185+
throws Exception
186+
{
187+
KeyAgreement agreement = KeyAgreement.getInstance(algorithm, "BC");
188+
189+
agreement.init(a.getPrivate(), new UserKeyingMaterialSpec(ukm));
190+
agreement.doPhase(b.getPublic(), true);
191+
192+
return agreement.generateSecret("AES").getEncoded();
193+
}
194+
195+
private KeyAgreement agreement(String algorithm)
196+
throws Exception
197+
{
198+
try
199+
{
200+
return KeyAgreement.getInstance(algorithm, "BC");
201+
}
202+
catch (NoSuchAlgorithmException e)
203+
{
204+
return null; // not in this distribution
205+
}
206+
}
207+
208+
private KeyPair generate(String algorithm, AlgorithmParameterSpec spec)
209+
throws Exception
210+
{
211+
KeyPairGenerator kpGen = KeyPairGenerator.getInstance(algorithm, "BC");
212+
213+
if (spec != null)
214+
{
215+
kpGen.initialize(spec);
216+
}
217+
else if (algorithm.equals("DH"))
218+
{
219+
kpGen.initialize(2048);
220+
}
221+
222+
return kpGen.generateKeyPair();
223+
}
224+
225+
public static void main(
226+
String[] args)
227+
{
228+
Security.addProvider(new BouncyCastleProvider());
229+
230+
runTest(new KeyAgreementInitParametersTest());
231+
}
232+
}

‎prov/src/test/java/org/bouncycastle/jce/provider/test/RegressionTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public class RegressionTest
6464
new JKSStoreTest(),
6565
new KeccakTest(),
6666
new KeyAgreementKeySizeTest(),
67+
new KeyAgreementInitParametersTest(),
6768
new Argon2KeyFactoryTest(),
6869
new KeyStoreTest(),
6970
new MacTest(),

0 commit comments

Comments
 (0)

Back | FazBrowse Home | New Git URL