Skip to content

Commit ffca364

Browse files
committed
Bind the HSS and XMSS^MT private key indices to the traversal state stored beside them, cross-check the XMSS and XMSS^MT root against the BDS state's own copy, carry a checksum over the encoded BDS state bound to the owning key's public seed since its node values cannot be recomputed cheaply, and regenerate the byte-level format documents the state change moves, relates to github #2414.
1 parent 32ebf8f commit ffca364

23 files changed

Lines changed: 901 additions & 47 deletions

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ The detailed guidance is split across the files below (imported automatically).
77
- @docs/claude/build-and-test.md — driving the Gradle build, running individual tests fast, the stash-the-fix verification discipline, why a green Gradle run can mean the tests never ran and the tree can move under a long one.
88
- @docs/claude/build-jdk14.md — the jdk1.4 (build1-4) legacy build: source preprocessing (what the regexes fix and what they can't), the real-1.4-javac compile floor, overlay delete-vs-sync discipline, the build → bcsign4 → test flow and its signing/staleness traps.
99
- @docs/claude/architecture.md — module graph and the `core`-into-`prov` trap, MR-jar overlays, `module-info.java` / `package-info.java` upkeep, where examples live, JCE provider registration, adding a PQC algorithm, `.bc` vs `.jcajce` package layering.
10-
- @docs/claude/conventions.md — test conventions, X.509 / ASN.1 RFC discipline, strict cert parse vs reviewer, DER lenient-read/strict-write, exception-message contract, `SecurityExceptions` cause-chaining, property constants, non-standard interop, PKCS#12 SPI pair, CMS streaming I/O, CMS recipient constraints, operator close discipline, duplicated OID tables, receiver-side `AlgorithmIdentifier` comparison, the name-finder/test count lock, checking whether the type already solves it, release notes, crediting reporters in CONTRIBUTORS.md, commit messages, URL checking, code style.
10+
- @docs/claude/conventions.md — test conventions, X.509 / ASN.1 RFC discipline, strict cert parse vs reviewer, DER lenient-read/strict-write, exception-message contract, `SecurityExceptions` cause-chaining, property constants, non-standard interop, PKCS#12 SPI pair, CMS streaming I/O, CMS recipient constraints, operator close discipline, duplicated OID tables, receiver-side `AlgorithmIdentifier` comparison, stateful hash-based private keys (the position is stored twice - cross-check it; checksum vs recomputation; the `docs/formats` byte-level docs move with the encoding), compatibility retries that mask the real parse error, the name-finder/test count lock, checking whether the type already solves it, release notes, crediting reporters in CONTRIBUTORS.md, commit messages, URL checking, code style.
1111
- @docs/claude/upstream-lts-ports.md — the `UPSTREAM-*.md` porting workflow: verifying the doc against this tree, diffing against the BC-LTS checkout, tests/release-note trimmings, keeping the .md out of the commit.

CONTRIBUTORS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ We also wish to acknowledge financial and collaborative support from [CISCO](htt
542542
- jmeeder \<https://github.com/jmeeder\> - reporting that RFC 4998 evidence-record generation rejected time stamps from an authority naming the digest with NULL parameters where BC names it with them absent, both of which RFC 5754 requires a receiver to accept (issue #2379).
543543
- rimuln \<https://github.com/rimuln\> - diagnosis and fix for PKCS12 getCertificateAlias returning the alias of an unrelated certificate, tracing it to the alias and certificate enumerations of the keystore's certs table diverging in order once keys() enumerated a copy (issue #2384, PR #2385).
544544
- Yu Bao \<yubao&#064;paypal.com\> - reporting an API gap, on behalf of the PayPal Cyber Security Team, that the high-level OpenPGP message API (OpenPGPMessageProcessor / OpenPGPMessageInputStream) gave a caller no way to bound how far a compressed data packet expands, where the low-level PGPCompressedData it wraps has carried a bounded getDataStream(long) overload all along, and that OpenPGPPolicy exposed no equivalent property to set. Suggesting a protocol whitelist for CRL Distribution Point fetching, which is now the org.bouncycastle.x509.CRLDP_protocols property, and suggesting that an OCSP response was read up to the length the responder declared for itself, now capped by org.bouncycastle.ocsp.max_response_size, and suggesting bounds on the OpenPGP ASCII armor headers, now capped by org.bouncycastle.openpgp.max_armor_header_length and org.bouncycastle.openpgp.max_armor_headers.
545-
- Arpan Sharma \<https://github.com/Arpan0995\> - initial audit of BCPQC provider consistency starting with HQC, which led to the exposure of a number of issues in the JCA provider service interfaces for other BCPQC algorithms. In-depth auditing of PQC signature algorithms in the provider leading to the correction of a number of JCA API compliance issues. Initial implementation of the guard that lets a signature context be set on the composite ML-DSA services before initSign / initVerify, the composite counterpart of the base-engine fix for issue #2396 (issue #2412). Audit of the LMS / HSS stateful private key decoder, establishing that the level count, the one-time index and the persisted tree cache were all accepted without validation (issue #2414).
545+
- Arpan Sharma \<https://github.com/Arpan0995\> - initial audit of BCPQC provider consistency starting with HQC, which led to the exposure of a number of issues in the JCA provider service interfaces for other BCPQC algorithms. In-depth auditing of PQC signature algorithms in the provider leading to the correction of a number of JCA API compliance issues. Initial implementation of the guard that lets a signature context be set on the composite ML-DSA services before initSign / initVerify, the composite counterpart of the base-engine fix for issue #2396 (issue #2412). Audit of the LMS / HSS stateful private key decoder, establishing that the level count, the one-time index and the persisted tree cache were all accepted without validation (issue #2414); that issue was also the inspiration for the further investigation which found the HSS and XMSS^MT decoders accepting a private key whose declared index disagreed with the traversal state stored beside it, allowing a one-time key to be used twice.
546546
- Flowdalic \<https://github.com/Flowdalic\> - initial implementation of an AnimalSniffer-based Android API-level compatibility check for the Gradle build (PR #336).
547547
- hannesa2 \<https://github.com/hannesa2\> - initial Dependabot configuration for the Gradle and GitHub Actions ecosystems (PR #883).
548548
- vladhuma \<https://github.com/vladhuma\> - initial implementation of server-side OCSP stapling for the BCJSSE provider, on behalf of Thales Group (PR #1740).

core/src/main/java/org/bouncycastle/crypto/params/HSSPrivateKeyParameters.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,46 @@ public static HSSPrivateKeyParameters getInstance(byte[] privEnc, byte[] pubEnc)
103103
return pKey;
104104
}
105105

106+
/**
107+
* The HSS index and the component keys' one-time indices are two records of the same position in
108+
* the key, and a decoded key whose records disagree is refused. RFC 8554 sec. 1 requires each
109+
* one-time key to be used once; a stored key whose index has been rolled back while its
110+
* component keys stayed advanced - a partial write, a restore from backup, a buggy storage layer
111+
* - would otherwise sign a second message under a one-time key already used, and that signature
112+
* would verify, so nothing would surface it. The check is the identity the two records satisfy:
113+
* a level below the last contributes (q - 1) leaves of the levels beneath it, because its q has
114+
* already advanced past the subtree it signed, and the last level contributes its q directly.
115+
* Verified against every index of a two-level key and across a level boundary of a three-level
116+
* one (github #2414).
117+
* <p>
118+
* Applied at decode only. The constructor is also reached from the hierarchy update, which
119+
* rebuilds lower levels and is momentarily inconsistent by design; corrupt stored state can only
120+
* arrive here.
121+
*/
122+
private static void checkIndexAgainstKeys(int d, List keys, long index)
123+
throws IOException
124+
{
125+
long implied = ((LMSPrivateKeyParameters)keys.get(d - 1)).getIndex();
126+
int shift = 0;
127+
128+
for (int i = d - 2; i >= 0; i--)
129+
{
130+
shift += ((LMSPrivateKeyParameters)keys.get(i + 1)).getSigParameters().getH();
131+
if (shift >= 63)
132+
{
133+
// taller than the 64-bit index can address, so the two records cannot be compared
134+
return;
135+
}
136+
implied += (((long)((LMSPrivateKeyParameters)keys.get(i)).getIndex()) - 1L) << shift;
137+
}
138+
139+
if (implied != index)
140+
{
141+
throw new IOException("HSS private key index " + index
142+
+ " does not match the component key indices, which imply " + implied);
143+
}
144+
}
145+
106146
public static HSSPrivateKeyParameters getInstance(Object src)
107147
throws IOException
108148
{
@@ -149,6 +189,8 @@ else if (src instanceof DataInputStream)
149189
signatures.add(LMSSignature.getInstance(src));
150190
}
151191

192+
checkIndexAgainstKeys(d, keys, index);
193+
152194
return new HSSPrivateKeyParameters(d, keys, signatures, index, maxIndex, limited);
153195
}
154196
else if (src instanceof byte[])

core/src/main/java/org/bouncycastle/pqc/crypto/lms/HSSPrivateKeyParameters.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,46 @@ public static HSSPrivateKeyParameters getInstance(byte[] privEnc, byte[] pubEnc)
103103
return pKey;
104104
}
105105

106+
/**
107+
* The HSS index and the component keys' one-time indices are two records of the same position in
108+
* the key, and a decoded key whose records disagree is refused. RFC 8554 sec. 1 requires each
109+
* one-time key to be used once; a stored key whose index has been rolled back while its
110+
* component keys stayed advanced - a partial write, a restore from backup, a buggy storage layer
111+
* - would otherwise sign a second message under a one-time key already used, and that signature
112+
* would verify, so nothing would surface it. The check is the identity the two records satisfy:
113+
* a level below the last contributes (q - 1) leaves of the levels beneath it, because its q has
114+
* already advanced past the subtree it signed, and the last level contributes its q directly.
115+
* Verified against every index of a two-level key and across a level boundary of a three-level
116+
* one (github #2414).
117+
* <p>
118+
* Applied at decode only. The constructor is also reached from the hierarchy update, which
119+
* rebuilds lower levels and is momentarily inconsistent by design; corrupt stored state can only
120+
* arrive here.
121+
*/
122+
private static void checkIndexAgainstKeys(int d, List keys, long index)
123+
throws IOException
124+
{
125+
long implied = ((LMSPrivateKeyParameters)keys.get(d - 1)).getIndex();
126+
int shift = 0;
127+
128+
for (int i = d - 2; i >= 0; i--)
129+
{
130+
shift += ((LMSPrivateKeyParameters)keys.get(i + 1)).getSigParameters().getH();
131+
if (shift >= 63)
132+
{
133+
// taller than the 64-bit index can address, so the two records cannot be compared
134+
return;
135+
}
136+
implied += (((long)((LMSPrivateKeyParameters)keys.get(i)).getIndex()) - 1L) << shift;
137+
}
138+
139+
if (implied != index)
140+
{
141+
throw new IOException("HSS private key index " + index
142+
+ " does not match the component key indices, which imply " + implied);
143+
}
144+
}
145+
106146
public static HSSPrivateKeyParameters getInstance(Object src)
107147
throws IOException
108148
{
@@ -149,6 +189,8 @@ else if (src instanceof DataInputStream)
149189
signatures.add(LMSSignature.getInstance(src));
150190
}
151191

192+
checkIndexAgainstKeys(d, keys, index);
193+
152194
return new HSSPrivateKeyParameters(d, keys, signatures, index, maxIndex, limited);
153195
}
154196
else if (src instanceof byte[])

core/src/main/java/org/bouncycastle/pqc/crypto/util/PrivateKeyFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ else if (algOID.equals(PQCObjectIdentifiers.xmss))
337337

338338
if (xmssPrivateKey.getBdsState() != null)
339339
{
340-
BDS bds = (BDS)XMSSUtil.deserialize(xmssPrivateKey.getBdsState(), BDS.class);
340+
BDS bds = (BDS)XMSSUtil.deserialize(xmssPrivateKey.getBdsState(), BDS.class, xmssPrivateKey.getPublicSeed());
341341
keyBuilder.withBDSState(bds.withWOTSDigest(treeDigest));
342342
}
343343

@@ -372,7 +372,7 @@ else if (algOID.equals(PQCObjectIdentifiers.xmss_mt))
372372

373373
if (xmssMtPrivateKey.getBdsState() != null)
374374
{
375-
BDSStateMap bdsState = (BDSStateMap)XMSSUtil.deserialize(xmssMtPrivateKey.getBdsState(), BDSStateMap.class);
375+
BDSStateMap bdsState = (BDSStateMap)XMSSUtil.deserialize(xmssMtPrivateKey.getBdsState(), BDSStateMap.class, xmssMtPrivateKey.getPublicSeed());
376376
keyBuilder.withBDSState(bdsState.withWOTSDigest(treeDigest));
377377
}
378378

core/src/main/java/org/bouncycastle/pqc/crypto/util/PrivateKeyInfoFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ private static XMSSPrivateKey xmssCreateKeyStructure(XMSSPrivateKeyParameters ke
411411
BDS bds = null;
412412
try
413413
{
414-
bds = (BDS)XMSSUtil.deserialize(bdsStateBinary, BDS.class);
414+
bds = (BDS)XMSSUtil.deserialize(bdsStateBinary, BDS.class, publicSeed);
415415
}
416416
catch (ClassNotFoundException e)
417417
{
@@ -461,7 +461,7 @@ private static XMSSMTPrivateKey xmssmtCreateKeyStructure(XMSSMTPrivateKeyParamet
461461
BDSStateMap bds = null;
462462
try
463463
{
464-
bds = (BDSStateMap)XMSSUtil.deserialize(bdsStateBinary, BDSStateMap.class);
464+
bds = (BDSStateMap)XMSSUtil.deserialize(bdsStateBinary, BDSStateMap.class, publicSeed);
465465
}
466466
catch (ClassNotFoundException e)
467467
{

core/src/main/java/org/bouncycastle/pqc/crypto/xmss/BDS.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.TreeMap;
1414

1515
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
16+
import org.bouncycastle.util.Arrays;
1617

1718
/**
1819
* BDS.
@@ -599,6 +600,31 @@ void validate(XMSSParameters params)
599600
}
600601
}
601602

603+
/**
604+
* Confirm the root this state carries is the one the enclosing private key declares. The two are
605+
* independent copies of the same value in one encoding and always agree on a genuine key, so a
606+
* disagreement means the stored key has been corrupted. Left unchecked, a corrupted root is
607+
* accepted and then poisons every signature the key makes - the root is hashed into the message
608+
* digest, so the signature simply does not verify, with nothing to say why (github #2414).
609+
* <p>
610+
* A null root on either side is not compared: BDS.validate tolerates an absent root node, and a
611+
* key built without one carries zeros.
612+
*
613+
* @param expectedRoot the root the private key declares.
614+
*/
615+
void validateRoot(byte[] expectedRoot)
616+
{
617+
if (root == null || expectedRoot == null)
618+
{
619+
return;
620+
}
621+
622+
if (!Arrays.areEqual(root.getValue(), expectedRoot))
623+
{
624+
throw new IllegalStateException("BDS state root does not match the private key root");
625+
}
626+
}
627+
602628
void validate(XMSSParameters params, int expectedIndex)
603629
{
604630
validate(params);

0 commit comments

Comments
 (0)