Skip to content

Commit 009b7de

Browse files
committed
ERS: the sorted hash lists now sort a copy in toList() and answer getFirst() with a scan, so reading one no longer reorders it, relates to github #2456.
1 parent af132f4 commit 009b7de

5 files changed

Lines changed: 114 additions & 49 deletions

File tree

‎CONTRIBUTORS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,3 +548,4 @@ We also wish to acknowledge financial and collaborative support from [CISCO](htt
548548
- vladhuma \<https://github.com/vladhuma\> - initial implementation of server-side OCSP stapling for the BCJSSE provider, on behalf of Thales Group (PR #1740).
549549
- Bhargava Shastry \<bshastry&#064;posteo.de\> - reporting, with a self-checking reproducer, that the C509 validity fields were read as unbounded seconds and then given two meanings: displacing a type-3 certificate's validityNotAfter by 2^61 seconds left the reconstructed DER TBSCertificate and the issuer signature untouched, while C509CertificateHolder.isValidOn took the raw value and read an expired certificate as valid.
550550
- Rob Augustinus \<rob&#064;opensolutions.nl\> - a constant-time analysis of the elliptic-curve and AES code paths, with proof-of-concept code, which independently reached the variable-point scalar multiplication exposure addressed in 1.86.
551+
- Radim Dejmek \<dejmekr&#064;seznam.cz\> - initial implementation of the rebuild of the ERS hash lists around a single sort, which had been finding the insertion point for each hash by walking a LinkedList (PR #2457).

‎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+
- Building an evidence record was cubic in the number of data objects: SortedHashList and SortedIndexedHashList held their hashes in a LinkedList and found each insertion point by walking it with get(index), so a single add() was quadratic in the position it inserted at and building a list of n hashes cubic, and both lists sit on the generation path - the reduced hash tree of ERSArchiveTimeStampGenerator, the Merkle tree of BinaryTreeRootCalculator.computeRootHash(), and the hash list of every ERSDataGroup. Each now collects its hashes and sorts them once, in toList(); the sort is stable and the old insertion placed a hash after the last one comparing equal to it, which is where a stable sort puts it, so the order of the leaves and every root hash are unchanged. getFirst() answers with a scan rather than a sort and toList() sorts a copy, so neither accessor disturbs what has been added. Generating a time-stamp request over 8,000 data objects goes from about 210 seconds to under a tenth of a second, and 100,000 objects, which the old code could not reach in any practical time, takes about 0.2 seconds (github #2456).
2728

2829
### 2.1.3 Additional Features and Functionality
2930

‎pkix/src/main/java/org/bouncycastle/tsp/ers/SortedHashList.java‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ public class SortedHashList
1515

1616
private final List<byte[]> baseList = new ArrayList<byte[]>();
1717

18-
private boolean isSorted = true;
19-
2018
public SortedHashList()
2119
{
2220
}
@@ -28,43 +26,45 @@ public byte[] getFirst()
2826
throw new NoSuchElementException();
2927
}
3028

31-
sort();
29+
byte[] first = (byte[])baseList.get(0);
30+
31+
for (int i = 1; i != baseList.size(); i++)
32+
{
33+
byte[] next = (byte[])baseList.get(i);
34+
35+
// strictly less than, so the earliest added of a set of equal hashes is returned
36+
if (hashComp.compare(next, first) < 0)
37+
{
38+
first = next;
39+
}
40+
}
3241

33-
return (byte[])baseList.get(0);
42+
return first;
3443
}
3544

3645
public void add(byte[] hash)
3746
{
3847
baseList.add(hash);
39-
isSorted = false;
4048
}
4149

4250
public int size()
4351
{
4452
return baseList.size();
4553
}
4654

47-
public List<byte[]> toList()
48-
{
49-
sort();
50-
51-
return new ArrayList<byte[]>(baseList);
52-
}
53-
5455
/**
55-
* Sorting is deferred to the accessors. Inserting each hash on add() meant searching a
56-
* LinkedList for the insertion point with get(index), which is O(index), so a single add()
57-
* was O(n^2) and building a list of n hashes was O(n^3).
56+
* Return the hashes added so far in ascending order.
5857
* <p>
59-
* Collections.sort() is stable, so hashes comparing equal keep the order they were added
60-
* in - which is where inserting after the last equal element used to put them.
58+
* The sort is stable, so hashes comparing equal come back in the order they were added in.
59+
*
60+
* @return a sorted list of the hashes added.
6161
*/
62-
private void sort()
62+
public List<byte[]> toList()
6363
{
64-
if (!isSorted)
65-
{
66-
Collections.sort(baseList, hashComp);
67-
isSorted = true;
68-
}
64+
List<byte[]> sorted = new ArrayList<byte[]>(baseList);
65+
66+
Collections.sort(sorted, hashComp);
67+
68+
return sorted;
6969
}
7070
}

‎pkix/src/main/java/org/bouncycastle/tsp/ers/SortedIndexedHashList.java‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ public int compare(IndexedHash l, IndexedHash r)
2323

2424
private final List<IndexedHash> baseList = new ArrayList<IndexedHash>();
2525

26-
private boolean isSorted = true;
27-
2826
public SortedIndexedHashList()
2927
{
3028
}
@@ -36,40 +34,45 @@ public IndexedHash getFirst()
3634
throw new NoSuchElementException();
3735
}
3836

39-
sort();
37+
IndexedHash first = (IndexedHash)baseList.get(0);
38+
39+
for (int i = 1; i != baseList.size(); i++)
40+
{
41+
IndexedHash next = (IndexedHash)baseList.get(i);
4042

41-
return (IndexedHash)baseList.get(0);
43+
// strictly less than, so the earliest added of a set of equal hashes is returned
44+
if (digestComp.compare(next, first) < 0)
45+
{
46+
first = next;
47+
}
48+
}
49+
50+
return first;
4251
}
4352

4453
public void add(IndexedHash hash)
4554
{
4655
baseList.add(hash);
47-
isSorted = false;
4856
}
4957

5058
public int size()
5159
{
5260
return baseList.size();
5361
}
5462

63+
/**
64+
* Return the hashes added so far in ascending order of digest.
65+
* <p>
66+
* The sort is stable, so hashes comparing equal come back in the order they were added in.
67+
*
68+
* @return a sorted list of the hashes added.
69+
*/
5570
public List<IndexedHash> toList()
5671
{
57-
sort();
72+
List<IndexedHash> sorted = new ArrayList<IndexedHash>(baseList);
5873

59-
return new ArrayList<IndexedHash>(baseList);
60-
}
74+
Collections.sort(sorted, digestComp);
6175

62-
/**
63-
* Sorting is deferred to the accessors, for the reason given on SortedHashList.sort():
64-
* finding the insertion point with LinkedList.get(index) made building a list of n hashes
65-
* O(n^3). Collections.sort() is stable, so hashes comparing equal keep ascending order.
66-
*/
67-
private void sort()
68-
{
69-
if (!isSorted)
70-
{
71-
Collections.sort(baseList, digestComp);
72-
isSorted = true;
73-
}
76+
return sorted;
7477
}
7578
}

‎pkix/src/test/java/org/bouncycastle/tsp/test/ERSTest.java‎

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.HashSet;
2222
import java.util.LinkedList;
2323
import java.util.List;
24+
import java.util.NoSuchElementException;
2425
import java.util.Random;
2526

2627
import junit.framework.TestCase;
@@ -1457,13 +1458,72 @@ private int compareUnsigned(byte[] left, byte[] right)
14571458
}
14581459

14591460
/**
1460-
* A reduced hash tree over a large number of data objects. This reaches both sorted lists -
1461+
* toList() sorts a copy, so the list handed back belongs to the caller and the accessors can
1462+
* be interleaved with add() in any order. getFirst() answers without sorting at all.
1463+
*/
1464+
public void testSortedHashListAccessors()
1465+
{
1466+
SortedHashList list = new SortedHashList();
1467+
1468+
assertEquals(0, list.size());
1469+
assertTrue(list.toList().isEmpty());
1470+
1471+
try
1472+
{
1473+
list.getFirst();
1474+
fail("no exception on empty list");
1475+
}
1476+
catch (NoSuchElementException e)
1477+
{
1478+
// expected - as LinkedList.getFirst() did
1479+
}
1480+
1481+
byte[] three = Hex.decode("03");
1482+
byte[] oneA = Hex.decode("01");
1483+
byte[] two = Hex.decode("02");
1484+
byte[] oneB = Hex.decode("01");
1485+
1486+
byte[][] values = new byte[][]{three, oneA, two, oneB};
1487+
for (int i = 0; i != values.length; i++)
1488+
{
1489+
list.add(values[i]);
1490+
}
1491+
1492+
List<byte[]> first = list.toList();
1493+
1494+
assertEquals(4, first.size());
1495+
// equal hashes come back in the order they were added in
1496+
assertTrue(oneA == first.get(0));
1497+
assertTrue(oneB == first.get(1));
1498+
assertTrue(two == first.get(2));
1499+
assertTrue(three == first.get(3));
1500+
assertTrue(oneA == list.getFirst());
1501+
1502+
// the caller owns the list returned, and the one before it
1503+
first.clear();
1504+
1505+
List<byte[]> second = list.toList();
1506+
1507+
assertEquals(4, list.size());
1508+
assertEquals(4, second.size());
1509+
assertTrue(oneA == second.get(0));
1510+
1511+
byte[] zero = Hex.decode("00");
1512+
1513+
list.add(zero);
1514+
1515+
assertEquals(5, list.size());
1516+
assertTrue(zero == list.getFirst());
1517+
assertTrue(zero == list.toList().get(0));
1518+
}
1519+
1520+
/**
1521+
* A reduced hash tree over a large number of data objects, reaching both sorted lists -
14611522
* SortedIndexedHashList from ERSArchiveTimeStampGenerator.getPartialHashtrees(), and
1462-
* SortedHashList from BinaryTreeRootCalculator.computeRootHash() - and took about 2.5
1463-
* seconds for these 2,000 objects when the insertion point was found by walking a
1464-
* LinkedList, rising by roughly a factor of eight per doubling (10,000 objects took 347
1465-
* seconds). The root is also checked to be independent of the order the objects were added
1466-
* in, which is what the sorting is there for.
1523+
* SortedHashList from BinaryTreeRootCalculator.computeRootHash(). Finding each insertion
1524+
* point by walking a LinkedList made this cubic in the number of data objects; the root is
1525+
* also checked to be independent of the order the objects were added in, which is what the
1526+
* sorting is there for.
14671527
*/
14681528
public void testLargeDataObjectSet()
14691529
throws Exception

0 commit comments

Comments
 (0)