Skip to content

Commit 3968d4e

Browse files
committed
The ML-DSA public key (rho, t1) constructor and the ML-DSA and SLH-DSA private key component constructors now reject components whose length does not match the parameter set.
1 parent 0a65497 commit 3968d4e

6 files changed

Lines changed: 175 additions & 0 deletions

File tree

‎core/src/main/java/org/bouncycastle/crypto/params/MLDSAPrivateKeyParameters.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,24 @@ public MLDSAPrivateKeyParameters(MLDSAParameters params, byte[] rho, byte[] K, b
4040
public MLDSAPrivateKeyParameters(MLDSAParameters params, byte[] rho, byte[] K, byte[] tr, byte[] s1, byte[] s2, byte[] t0, byte[] t1, byte[] seed)
4141
{
4242
super(true, params);
43+
44+
MLDSAEngine eng = MLDSAEngine.getInstance(params, null);
45+
46+
checkLength(rho, MLDSAEngine.SeedBytes, "rho");
47+
checkLength(K, MLDSAEngine.SeedBytes, "K");
48+
checkLength(tr, MLDSAEngine.TrBytes, "tr");
49+
checkLength(s1, eng.getDilithiumL() * eng.getDilithiumPolyEtaPackedBytes(), "s1");
50+
checkLength(s2, eng.getDilithiumK() * eng.getDilithiumPolyEtaPackedBytes(), "s2");
51+
checkLength(t0, eng.getDilithiumK() * MLDSAEngine.DilithiumPolyT0PackedBytes, "t0");
52+
if (t1 != null)
53+
{
54+
checkLength(t1, eng.getDilithiumK() * MLDSAEngine.DilithiumPolyT1PackedBytes, "t1");
55+
}
56+
if (seed != null)
57+
{
58+
checkLength(seed, MLDSAEngine.SeedBytes, "seed");
59+
}
60+
4361
this.rho = Arrays.clone(rho);
4462
this.k = Arrays.clone(K);
4563
this.tr = Arrays.clone(tr);
@@ -256,6 +274,18 @@ public boolean isDestroyed()
256274
return destroyed;
257275
}
258276

277+
private static void checkLength(byte[] value, int expectedLength, String name)
278+
{
279+
if (value == null)
280+
{
281+
throw new NullPointerException("'" + name + "' cannot be null");
282+
}
283+
if (value.length != expectedLength)
284+
{
285+
throw new IllegalArgumentException("'" + name + "' has invalid length");
286+
}
287+
}
288+
259289
private byte[] cloneWithCheck(byte[] fieldValue)
260290
{
261291
byte[] rv = Arrays.clone(fieldValue);

‎core/src/main/java/org/bouncycastle/crypto/params/MLDSAPublicKeyParameters.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,17 @@ public MLDSAPublicKeyParameters(MLDSAParameters params, byte[] rho, byte[] t1)
3939
{
4040
throw new NullPointerException("t1 cannot be null");
4141
}
42+
43+
MLDSAEngine engine = MLDSAEngine.getInstance(params, null);
44+
if (rho.length != MLDSAEngine.SeedBytes)
45+
{
46+
throw new IllegalArgumentException("'rho' has invalid length");
47+
}
48+
if (t1.length != engine.getDilithiumK() * MLDSAEngine.DilithiumPolyT1PackedBytes)
49+
{
50+
throw new IllegalArgumentException("'t1' has invalid length");
51+
}
52+
4253
this.rho = Arrays.clone(rho);
4354
this.t1 = Arrays.clone(t1);
4455
}

‎core/src/main/java/org/bouncycastle/crypto/params/SLHDSAPrivateKeyParameters.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ public SLHDSAPrivateKeyParameters(SLHDSAParameters parameters, byte[] skpkEncode
2828
public SLHDSAPrivateKeyParameters(SLHDSAParameters parameters, byte[] skSeed, byte[] prf, byte[] pkSeed, byte[] pkRoot)
2929
{
3030
super(true, parameters);
31+
32+
int n = parameters.getN();
33+
checkLength(skSeed, n, "skSeed");
34+
checkLength(prf, n, "prf");
35+
checkLength(pkSeed, n, "pkSeed");
36+
checkLength(pkRoot, n, "pkRoot");
37+
3138
this.sk = new SK(skSeed, prf);
3239
this.pk = new PK(pkSeed, pkRoot);
3340
}
@@ -94,6 +101,18 @@ public boolean isDestroyed()
94101
return destroyed;
95102
}
96103

104+
private static void checkLength(byte[] value, int expectedLength, String name)
105+
{
106+
if (value == null)
107+
{
108+
throw new NullPointerException("'" + name + "' cannot be null");
109+
}
110+
if (value.length != expectedLength)
111+
{
112+
throw new IllegalArgumentException("'" + name + "' has invalid length");
113+
}
114+
}
115+
97116
private byte[] cloneWithCheck(byte[] fieldValue)
98117
{
99118
byte[] rv = Arrays.clone(fieldValue);

‎core/src/test/java/org/bouncycastle/pqc/crypto/test/MLDSATest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
import java.util.regex.Pattern;
1414

1515
import junit.framework.TestCase;
16+
import org.bouncycastle.asn1.ASN1Encodable;
17+
import org.bouncycastle.asn1.DEROctetString;
18+
import org.bouncycastle.asn1.DERSequence;
1619
import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
1720
import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
1821
import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
@@ -762,6 +765,85 @@ public void testPrivateKeyRejectsMalformedLength()
762765
}
763766
}
764767

768+
public void testComponentLengthChecks()
769+
throws Exception
770+
{
771+
MLDSAKeyPairGenerator kpGen = new MLDSAKeyPairGenerator();
772+
kpGen.init(new MLDSAKeyGenerationParameters(new SecureRandom(), MLDSAParameters.ml_dsa_44));
773+
AsymmetricCipherKeyPair kp = kpGen.generateKeyPair();
774+
MLDSAPrivateKeyParameters priv = (MLDSAPrivateKeyParameters)kp.getPrivate();
775+
MLDSAPublicKeyParameters pub = (MLDSAPublicKeyParameters)kp.getPublic();
776+
777+
byte[][] parts = new byte[][]{ priv.getRho(), priv.getK(), priv.getTr(), priv.getS1(), priv.getS2(), priv.getT0(), priv.getT1(), priv.getSeed() };
778+
String[] names = new String[]{ "rho", "K", "tr", "s1", "s2", "t0", "t1", "seed" };
779+
780+
// correct lengths are accepted, with and without the optional seed and t1
781+
new MLDSAPrivateKeyParameters(MLDSAParameters.ml_dsa_44, parts[0], parts[1], parts[2], parts[3], parts[4], parts[5], parts[6], parts[7]);
782+
new MLDSAPrivateKeyParameters(MLDSAParameters.ml_dsa_44, parts[0], parts[1], parts[2], parts[3], parts[4], parts[5], null);
783+
784+
for (int i = 0; i != parts.length; i++)
785+
{
786+
for (int delta = -1; delta <= 1; delta += 2)
787+
{
788+
byte[][] bad = new byte[parts.length][];
789+
System.arraycopy(parts, 0, bad, 0, parts.length);
790+
bad[i] = Arrays.copyOf(parts[i], parts[i].length + delta);
791+
try
792+
{
793+
new MLDSAPrivateKeyParameters(MLDSAParameters.ml_dsa_44, bad[0], bad[1], bad[2], bad[3], bad[4], bad[5], bad[6], bad[7]);
794+
fail("no exception for " + names[i] + " of length " + bad[i].length);
795+
}
796+
catch (IllegalArgumentException e)
797+
{
798+
assertEquals("'" + names[i] + "' has invalid length", e.getMessage());
799+
}
800+
}
801+
}
802+
803+
byte[] rho = pub.getRho();
804+
byte[] t1 = pub.getT1();
805+
806+
new MLDSAPublicKeyParameters(MLDSAParameters.ml_dsa_44, rho, t1);
807+
808+
for (int delta = -1; delta <= 1; delta += 2)
809+
{
810+
try
811+
{
812+
new MLDSAPublicKeyParameters(MLDSAParameters.ml_dsa_44, Arrays.copyOf(rho, rho.length + delta), t1);
813+
fail("no exception for rho of length " + (rho.length + delta));
814+
}
815+
catch (IllegalArgumentException e)
816+
{
817+
assertEquals("'rho' has invalid length", e.getMessage());
818+
}
819+
820+
byte[] badT1 = Arrays.copyOf(t1, t1.length + delta);
821+
try
822+
{
823+
new MLDSAPublicKeyParameters(MLDSAParameters.ml_dsa_44, rho, badT1);
824+
fail("no exception for t1 of length " + badT1.length);
825+
}
826+
catch (IllegalArgumentException e)
827+
{
828+
assertEquals("'t1' has invalid length", e.getMessage());
829+
}
830+
831+
// SEQUENCE { OCTET STRING rho, OCTET STRING t1 } public key form via PublicKeyFactory
832+
SubjectPublicKeyInfo spki = SubjectPublicKeyInfoFactory.createSubjectPublicKeyInfo(pub);
833+
SubjectPublicKeyInfo badSpki = new SubjectPublicKeyInfo(spki.getAlgorithm(),
834+
new DERSequence(new ASN1Encodable[]{ new DEROctetString(rho), new DEROctetString(badT1) }));
835+
try
836+
{
837+
PublicKeyFactory.createKey(badSpki);
838+
fail("no exception for SEQUENCE encoded public key with t1 of length " + badT1.length);
839+
}
840+
catch (IllegalArgumentException e)
841+
{
842+
// expected
843+
}
844+
}
845+
}
846+
765847
private interface RejectionOperation
766848
{
767849
byte[] processSign(MLDSAPrivateKeyParameters privParams, byte[] msg)

‎core/src/test/java/org/bouncycastle/pqc/crypto/test/SLHDSATest.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,38 @@ public void testBasicKeyGenerationShake256128fSimpleSign()
579579
assertTrue(Arrays.areEqual(sigGenerated, signature));
580580
}
581581

582+
public void testComponentLengthChecks()
583+
{
584+
byte[] sk = Hex.decode("2F896D61D9CD9038CA303394FADAA22A24AC5EC1D86A989CA2196C3C8632419C1A05A42FE300E87B16AEE116CB2E236358E2C3E62632C9DE03D08A535A0EB7E7");
585+
int n = SLHDSAParameters.sha2_128s.getN();
586+
587+
byte[][] parts = new byte[][]{
588+
Arrays.copyOfRange(sk, 0, n), Arrays.copyOfRange(sk, n, 2 * n), Arrays.copyOfRange(sk, 2 * n, 3 * n), Arrays.copyOfRange(sk, 3 * n, 4 * n) };
589+
String[] names = new String[]{ "skSeed", "prf", "pkSeed", "pkRoot" };
590+
591+
SLHDSAPrivateKeyParameters privParams = new SLHDSAPrivateKeyParameters(SLHDSAParameters.sha2_128s, parts[0], parts[1], parts[2], parts[3]);
592+
assertTrue(Arrays.areEqual(sk, privParams.getEncoded()));
593+
594+
for (int i = 0; i != parts.length; i++)
595+
{
596+
for (int delta = -1; delta <= 1; delta += 2)
597+
{
598+
byte[][] bad = new byte[parts.length][];
599+
System.arraycopy(parts, 0, bad, 0, parts.length);
600+
bad[i] = Arrays.copyOf(parts[i], parts[i].length + delta);
601+
try
602+
{
603+
new SLHDSAPrivateKeyParameters(SLHDSAParameters.sha2_128s, bad[0], bad[1], bad[2], bad[3]);
604+
fail("no exception for " + names[i] + " of length " + bad[i].length);
605+
}
606+
catch (IllegalArgumentException e)
607+
{
608+
assertEquals("'" + names[i] + "' has invalid length", e.getMessage());
609+
}
610+
}
611+
}
612+
}
613+
582614
public void testUninitialisedSignerThrowsIllegalStateException()
583615
throws Exception
584616
{

‎docs/releasenotes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ Date: 2026, TBD
6262
- JceKeyAgreeRecipientInfoGenerator created its 1-pass ECMQV ephemeral key pair once per generator rather than once per message, so a generator used for more than one message sent the same ephemeral key in each and derived the same key-encryption key for every recipient each time, withdrawing the ephemeral contribution RFC 5753 sec. 3.2 relies on. The messages were well formed and decrypted correctly. A fresh key pair is now generated for each KeyAgreeRecipientInfo, still shared by all of its recipients.
6363
- The ML-KEM KeyGenerator (KEMGenerateSpec/KEMExtractSpec), Cipher (wrap/unwrap), javax.crypto.KEM and KeyFactory.translateKey services accepted only BC's own ML-KEM key objects, and the KeyGenerator failed with a ClassCastException at generateKey() rather than at init, so an ML-KEM key from another provider could not be used with BC even though its standard encoding was one BC reads. This broke BCJSSE handshakes over the ML-KEM and hybrid groups whenever another provider ahead of BC decoded the peer's key or generated the ephemeral key pair. A foreign key is now converted from its X.509 or PKCS#8 encoding, with the usual parameter-set checks, and an unusable one is rejected at init (github #2466).
6464
- The raw JCA provider bounded the PBKDF2 iteration count taken from an encoding (org.bouncycastle.pbe.max_iteration_count, default 10,000,000) but not the counts of the legacy PBES1 (PKCS#5 scheme 1) and PKCS#12 PBE families beside it. Their AlgorithmParameters (PKCS12PBE and its OID aliases, PBKDF1) accepted any count, narrowing one beyond the int range with intValue() so that 2^32 arrived as 0, and every Cipher, Mac and SecretKeyFactory derivation ran with whatever count it was given - including a count decoded by another provider's AlgorithmParameters, as when javax.crypto.EncryptedPrivateKeyInfo.getKeySpec() decrypts a PKCS#12 PBE-protected key with BC. As these schemes carry the count in unauthenticated parameters and derive before anything can be checked, a supplied blob could hold a derivation for tens of minutes. The parameter parse now rejects a negative, beyond-int or over-limit count, and the derivations reject a negative or over-limit count, under the same property as PBKDF2. The PKCS#12 key store derives through the same code, so a org.bouncycastle.pkcs12.max_it_count raised above 10,000,000 now needs org.bouncycastle.pbe.max_iteration_count raised with it.
65+
- The ML-DSA public key constructor taking rho and t1 separately checked only for null, so an ML-DSA SubjectPublicKeyInfo in the SEQUENCE { OCTET STRING rho, OCTET STRING t1 } form with a t1 of the wrong length was accepted by PublicKeyFactory and the JCA KeyFactory, unlike the same key in the standard raw encoding. The component constructors for ML-DSA private keys (rho, K, tr, s1, s2, t0, t1, seed) and SLH-DSA private keys (skSeed, prf, pkSeed, pkRoot) likewise accepted components of any length. All three now reject a component whose length does not match the parameter set with an IllegalArgumentException.
6566

6667
### 2.1.3 Additional Features and Functionality
6768

0 commit comments

Comments
 (0)