Skip to content

Commit f79ac68

Browse files
committed
JKSKeyStoreSpi: a store shorter than the SHA-1 checksum it ends with threw ArrayIndexOutOfBoundsException out of KeyStore.load; validateStream now checks the length against the checksum plus the 12-byte header and reports a truncated store as EOFException, with the jdk1.1 and jdk1.4 copies in step, relates to github #2451.
1 parent 4ee1a79 commit f79ac68

6 files changed

Lines changed: 179 additions & 30 deletions

File tree

‎docs/releasenotes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Date: 2026, TBD
2424
- The name-constraint host canonicalisation removed a single RFC 1034 root-label dot, the only empty label a name may legally carry, but nothing refused the ones that are not legal: a dNSName, rfc822Name host or uniformResourceIdentifier host such as "example.com.." kept a phantom empty label after the strip and so matched no constraint at all, escaping an excluded subtree naming the host it appears to carry. A tested name whose host carries an empty label - a second trailing dot, a doubled dot or a leading dot - is now refused outright wherever a constraint of that type is in force, rather than canonicalised into a name it is not: removing the extra dots would decide on the caller's behalf that "example.com.." names example.com, which is not how a consumer resolving or comparing the name reads it, and refusing fails closed in both directions where canonicalising would newly admit such a name under a permitted subtree. The single trailing dot is canonicalised as before, a bare "." remains the root label rather than an empty one, and the guard is scoped to the host, so the doubled dot a quoted local part may legally carry is unaffected. Constraints are untouched - one may still begin with a dot, which is how this implementation spells "subdomains only" (github PR #2436).
2525
- SSLContext.createSSLEngine() from the BCJSSE provider in the 1.86 bctls jar failed with NoSuchMethodError on every JDK from 9 up, leaving engine-based users of the provider (Netty, Vert.x and the like) unable to open a connection. The jdk1.5 and jdk1.9 copies of the package-private SSLEngineUtil had declared create(ContextData) with different return types since 2019 - SSLEngine and ProvSSLEngine - and the root ProvSSLContextSpi, compiled against the first, is paired at runtime with the versions/9 copy on any modern JDK. Until 1.86 the java9 compile had hidden this by implicitly recompiling the whole base tree into META-INF/versions/9 (447 classes, ProvSSLContextSpi among them); the -implicit:none added in 1.86 to stop that duplication exposed the mismatch. The jdk1.9 copy now declares the same return type as the root one, a JDK 25 test creates engines against the built jar, and a new multiReleaseCheck Gradle task on every distributed jar reads the constant pool of each class in the jar and in the sibling BC jars it depends on and fails the build when a member reference does not resolve against the copy of its target that a JDK would pair it with, so the class of defect cannot ship again; the same check runs on arbitrary jars, a published release included, as multiReleaseCheckJar (github #2448).
2626
- The BCFKS key store derived its scrypt keys with the block size r in place of the parallelization parameter p, while writing the p the caller configured out to the store: BcFKSKeyStoreSpi passed getBlockSize() to SCrypt.generate for both arguments and never read the encoded parallelization parameter at all, so every store whose ScryptConfig gave a p other than its r encoded parameters that do not derive its own keys. The store was self-consistent - BC read back what BC wrote - but a conformant RFC 7914 reader computed a different key and so failed the integrity check and the store decryption, and BC could not open such a store written by anyone else. Derivation now follows RFC 7914. A store written by 1.86 or earlier is still read: the integrity check is retried under the old convention, and where a signature check leaves no MAC to settle it the store decryption is retried instead, in both cases reporting the failure of the encoded parameters rather than of the fallback. The write side is governed by org.bouncycastle.bcfks.scrypt_p_eq_r, default true, which writes p equal to r whatever the ScryptConfig asked for: the two conventions then agree, so a store written here is both RFC 7914 correct and readable by 1.86 and earlier. Clearing the property honours the configured p, which those releases cannot read unless p already equals r; the default is intended to become false in a later release, once enough of the installed base is writing parameters that describe themselves. Loading with a BCFKSLoadStoreParameter carrying a ScryptConfig accepts an encoded p equal to either the configured p or the block size, so a store round trips under the configuration that wrote it whichever way the property was set; every other parameter is compared as before. The parallelization parameter is now bounded alongside the cost parameter before the derivation, as the PKCS#8 and PKCS#12 scrypt paths already bound it.
27+
- A JKS store shorter than the SHA-1 checksum it ends with threw an unchecked ArrayIndexOutOfBoundsException out of KeyStore.load, which declares IOException for a store it cannot read: JKSKeyStoreSpi.validateStream subtracted the digest size from the raw store length without checking it, so the digest update clamped its negative length to zero and the System.arraycopy that lifted the stored checksum out failed on a negative source index. The length is now checked against the checksum plus the 12-byte header before the checksum position is used, and a store too short to carry either is reported as an EOFException. The JKS store is reached through the compatibility probe in AdaptingKeyStoreSpi, so any key store type that probes for it was exposed, and the legacy jdk1.1 and jdk1.4 provider copies carry the same fix (github #2451).
2728

2829
### 2.1.3 Additional Features and Functionality
2930

‎prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/util/JKSKeyStoreSpi.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.io.ByteArrayInputStream;
44
import java.io.DataInputStream;
5+
import java.io.EOFException;
56
import java.io.IOException;
67
import java.io.InputStream;
78
import java.io.OutputStream;
@@ -364,31 +365,40 @@ private void addPassword(Digest digest, char[] password)
364365
private ErasableByteStream validateStream(InputStream inputStream, char[] password)
365366
throws IOException
366367
{
367-
Digest checksumCalculator = DigestFactory.getDigest("SHA-1");
368368
byte[] rawStore = Streams.readAll(inputStream);
369369

370+
Digest checksumCalculator = DigestFactory.getDigest("SHA-1");
371+
int checksumSize = checksumCalculator.getDigestSize();
372+
int checksumPos = getChecksumPos(rawStore, checksumSize);
373+
370374
if (password != null)
371375
{
372376
addPassword(checksumCalculator, password);
373-
checksumCalculator.update(rawStore, 0, rawStore.length - checksumCalculator.getDigestSize());
374-
375-
byte[] checksum = new byte[checksumCalculator.getDigestSize()];
377+
checksumCalculator.update(rawStore, 0, checksumPos);
376378

379+
byte[] checksum = new byte[checksumSize];
377380
checksumCalculator.doFinal(checksum, 0);
378381

379-
byte[] streamChecksum = new byte[checksum.length];
380-
System.arraycopy(rawStore, rawStore.length - checksum.length, streamChecksum, 0, checksum.length);
381-
382-
if (!Arrays.constantTimeAreEqual(checksum, streamChecksum))
382+
if (!Arrays.constantTimeAreEqual(checksumSize, checksum, 0, rawStore, checksumPos))
383383
{
384384
Arrays.fill(rawStore, (byte)0);
385385
throw new IOException("password incorrect or store tampered with");
386386
}
387+
}
388+
389+
return new ErasableByteStream(rawStore, 0, checksumPos);
390+
}
387391

388-
return new ErasableByteStream(rawStore, 0, rawStore.length - checksum.length);
392+
private static int getChecksumPos(byte[] rawStore, int checksumSize)
393+
throws EOFException
394+
{
395+
// A store is at least the 12-byte header (magic, version, entry count) plus a checksum
396+
if (rawStore.length - checksumSize < 12)
397+
{
398+
throw new EOFException("Invalid keystore format");
389399
}
390400

391-
return new ErasableByteStream(rawStore, 0, rawStore.length - checksumCalculator.getDigestSize());
401+
return rawStore.length - checksumSize;
392402
}
393403

394404
/**

‎prov/src/main/jdk1.1/org/bouncycastle/jcajce/provider/keystore/util/JKSKeyStoreSpi.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.io.ByteArrayInputStream;
44
import java.io.DataInputStream;
5+
import java.io.EOFException;
56
import java.io.IOException;
67
import java.io.InputStream;
78
import java.io.OutputStream;
@@ -344,31 +345,40 @@ private void addPassword(Digest digest, char[] password)
344345
private ErasableByteStream validateStream(InputStream inputStream, char[] password)
345346
throws IOException
346347
{
347-
Digest checksumCalculator = DigestFactory.getDigest("SHA-1");
348348
byte[] rawStore = Streams.readAll(inputStream);
349349

350+
Digest checksumCalculator = DigestFactory.getDigest("SHA-1");
351+
int checksumSize = checksumCalculator.getDigestSize();
352+
int checksumPos = getChecksumPos(rawStore, checksumSize);
353+
350354
if (password != null)
351355
{
352356
addPassword(checksumCalculator, password);
353-
checksumCalculator.update(rawStore, 0, rawStore.length - checksumCalculator.getDigestSize());
354-
355-
byte[] checksum = new byte[checksumCalculator.getDigestSize()];
357+
checksumCalculator.update(rawStore, 0, checksumPos);
356358

359+
byte[] checksum = new byte[checksumSize];
357360
checksumCalculator.doFinal(checksum, 0);
358361

359-
byte[] streamChecksum = new byte[checksum.length];
360-
System.arraycopy(rawStore, rawStore.length - checksum.length, streamChecksum, 0, checksum.length);
361-
362-
if (!Arrays.constantTimeAreEqual(checksum, streamChecksum))
362+
if (!Arrays.constantTimeAreEqual(checksumSize, checksum, 0, rawStore, checksumPos))
363363
{
364364
Arrays.fill(rawStore, (byte)0);
365365
throw new IOException("password incorrect or store tampered with");
366366
}
367+
}
368+
369+
return new ErasableByteStream(rawStore, 0, checksumPos);
370+
}
367371

368-
return new ErasableByteStream(rawStore, 0, rawStore.length - checksum.length);
372+
private static int getChecksumPos(byte[] rawStore, int checksumSize)
373+
throws EOFException
374+
{
375+
// A store is at least the 12-byte header (magic, version, entry count) plus a checksum
376+
if (rawStore.length - checksumSize < 12)
377+
{
378+
throw new EOFException("Invalid keystore format");
369379
}
370380

371-
return new ErasableByteStream(rawStore, 0, rawStore.length - checksumCalculator.getDigestSize());
381+
return rawStore.length - checksumSize;
372382
}
373383

374384
/**

‎prov/src/main/jdk1.4/org/bouncycastle/jcajce/provider/keystore/util/JKSKeyStoreSpi.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.io.ByteArrayInputStream;
44
import java.io.DataInputStream;
5+
import java.io.EOFException;
56
import java.io.IOException;
67
import java.io.InputStream;
78
import java.io.OutputStream;
@@ -343,31 +344,40 @@ private void addPassword(Digest digest, char[] password)
343344
private ErasableByteStream validateStream(InputStream inputStream, char[] password)
344345
throws IOException
345346
{
346-
Digest checksumCalculator = DigestFactory.getDigest("SHA-1");
347347
byte[] rawStore = Streams.readAll(inputStream);
348348

349+
Digest checksumCalculator = DigestFactory.getDigest("SHA-1");
350+
int checksumSize = checksumCalculator.getDigestSize();
351+
int checksumPos = getChecksumPos(rawStore, checksumSize);
352+
349353
if (password != null)
350354
{
351355
addPassword(checksumCalculator, password);
352-
checksumCalculator.update(rawStore, 0, rawStore.length - checksumCalculator.getDigestSize());
353-
354-
byte[] checksum = new byte[checksumCalculator.getDigestSize()];
356+
checksumCalculator.update(rawStore, 0, checksumPos);
355357

358+
byte[] checksum = new byte[checksumSize];
356359
checksumCalculator.doFinal(checksum, 0);
357360

358-
byte[] streamChecksum = new byte[checksum.length];
359-
System.arraycopy(rawStore, rawStore.length - checksum.length, streamChecksum, 0, checksum.length);
360-
361-
if (!Arrays.constantTimeAreEqual(checksum, streamChecksum))
361+
if (!Arrays.constantTimeAreEqual(checksumSize, checksum, 0, rawStore, checksumPos))
362362
{
363363
Arrays.fill(rawStore, (byte)0);
364364
throw new IOException("password incorrect or store tampered with");
365365
}
366+
}
367+
368+
return new ErasableByteStream(rawStore, 0, checksumPos);
369+
}
366370

367-
return new ErasableByteStream(rawStore, 0, rawStore.length - checksum.length);
371+
private static int getChecksumPos(byte[] rawStore, int checksumSize)
372+
throws EOFException
373+
{
374+
// A store is at least the 12-byte header (magic, version, entry count) plus a checksum
375+
if (rawStore.length - checksumSize < 12)
376+
{
377+
throw new EOFException("Invalid keystore format");
368378
}
369379

370-
return new ErasableByteStream(rawStore, 0, rawStore.length - checksumCalculator.getDigestSize());
380+
return rawStore.length - checksumSize;
371381
}
372382

373383
/**
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
package org.bouncycastle.jce.provider.test;
2+
3+
import java.io.ByteArrayInputStream;
4+
import java.io.EOFException;
5+
import java.io.IOException;
6+
import java.security.KeyStore;
7+
import java.security.Security;
8+
9+
import org.bouncycastle.jcajce.provider.keystore.util.AdaptingKeyStoreSpi;
10+
import org.bouncycastle.jcajce.provider.keystore.util.JKSKeyStoreSpi;
11+
import org.bouncycastle.jcajce.util.BCJcaJceHelper;
12+
import org.bouncycastle.jce.provider.BouncyCastleProvider;
13+
import org.bouncycastle.util.Arrays;
14+
import org.bouncycastle.util.test.SimpleTest;
15+
16+
public class JKSStoreTest
17+
extends SimpleTest
18+
{
19+
public String getName()
20+
{
21+
return "JKSStore";
22+
}
23+
24+
public void performTest()
25+
throws Exception
26+
{
27+
testLoadTruncatedStore();
28+
testLoadTruncatedStoreThroughKeyStore();
29+
}
30+
31+
private void testLoadTruncatedStore()
32+
throws Exception
33+
{
34+
// shorter than the checksum alone, and shorter than the header plus the checksum
35+
int[] lengths = new int[]{ 0, 4, 19, 31 };
36+
37+
for (int i = 0; i != lengths.length; i++)
38+
{
39+
byte[] store = Arrays.copyOfRange(PKCS12StoreTest.JKS_Store, 0, lengths[i]);
40+
41+
implLoadTruncatedStore(store, null);
42+
implLoadTruncatedStore(store, PKCS12StoreTest.JKS_TEST_PWD);
43+
}
44+
}
45+
46+
private void implLoadTruncatedStore(byte[] store, char[] password)
47+
throws Exception
48+
{
49+
JKSKeyStoreSpi jksStore = new JKSKeyStoreSpi(new BCJcaJceHelper());
50+
51+
try
52+
{
53+
jksStore.engineLoad(new ByteArrayInputStream(store), password);
54+
55+
fail("truncated store of " + store.length + " bytes accepted");
56+
}
57+
catch (EOFException e)
58+
{
59+
// expected
60+
}
61+
}
62+
63+
/**
64+
* The JKS store is reached through the compatibility probe in AdaptingKeyStoreSpi, so a
65+
* truncated store has to leave KeyStore.load() by way of its declared IOException rather
66+
* than an ArrayIndexOutOfBoundsException (github #2451).
67+
*/
68+
private void testLoadTruncatedStoreThroughKeyStore()
69+
throws Exception
70+
{
71+
String compatValue = System.getProperty(AdaptingKeyStoreSpi.COMPAT_OVERRIDE);
72+
73+
System.setProperty(AdaptingKeyStoreSpi.COMPAT_OVERRIDE, "true");
74+
75+
try
76+
{
77+
int[] lengths = new int[]{ 0, 4, 19, 31 };
78+
79+
for (int i = 0; i != lengths.length; i++)
80+
{
81+
byte[] store = Arrays.copyOfRange(PKCS12StoreTest.JKS_Store, 0, lengths[i]);
82+
83+
KeyStore ks = KeyStore.getInstance("PKCS12", "BC");
84+
85+
try
86+
{
87+
ks.load(new ByteArrayInputStream(store), PKCS12StoreTest.JKS_TEST_PWD);
88+
89+
fail("truncated store of " + store.length + " bytes accepted");
90+
}
91+
catch (IOException e)
92+
{
93+
// expected
94+
}
95+
}
96+
}
97+
finally
98+
{
99+
if (compatValue == null)
100+
{
101+
System.getProperties().remove(AdaptingKeyStoreSpi.COMPAT_OVERRIDE);
102+
}
103+
else
104+
{
105+
System.setProperty(AdaptingKeyStoreSpi.COMPAT_OVERRIDE, compatValue);
106+
}
107+
}
108+
}
109+
110+
public static void main(
111+
String[] args)
112+
{
113+
Security.addProvider(new BouncyCastleProvider());
114+
115+
runTest(new JKSStoreTest());
116+
}
117+
}

‎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
@@ -61,6 +61,7 @@ public class RegressionTest
6161
new HardwareKeyInvalidKeyExceptionTest(),
6262
new IESTest(),
6363
new ImplicitlyCaTest(),
64+
new JKSStoreTest(),
6465
new KeccakTest(),
6566
new KeyAgreementKeySizeTest(),
6667
new Argon2KeyFactoryTest(),

0 commit comments

Comments
 (0)