Skip to content

Composite ML-DSA Signature.setParameter() before init throws NullPointerException, the composite counterpart of #2396 #2412

Description

@Arpan0995

Summary

The composite signature provider has its own SignatureSpi with its own engineSetParameter, a separate class that never extended the base engines, and it has the same defect the #2396 fix (69fd0df) addressed for the base ML-DSA / SLH-DSA engines. Calling Signature.setParameter(new ContextParameterSpec(...)) before initSign / initVerify on a composite ML-DSA service throws an unchecked NullPointerException out of a method declared to throw the checked InvalidAlgorithmParameterException.

The JCA does not require setParameter to work before init, so this is not about ordering. The defect is the exception contract: engineSetParameter dereferences the not-yet-set key and lets an unchecked NullPointerException escape a method whose signature promises InvalidAlgorithmParameterException. The base-class javadoc added in 69fd0df describes this precise failure in its own words (quoted below), and the composite path was not brought into line with it.

The defect is one code path, not 36: the fault is in the single engineSetParameter of the shared parent org.bouncycastle.jcajce.provider.asymmetric.compositesignatures.SignatureSpi, and all 36 registered composite ML-DSA services (the 18 draft-ietf-lamps-pq-composite-sigs combinations, each in its plain and its -PREHASH form) inherit it unchanged. As a completeness check, 36 of 36 fail identically.

The #2396 consistency point

The #2396 fix gave the base signature engines set-before-init semantics and wrote the contract into BaseDeterministicOrRandomSignature.engineSetParameter:

Set the signature context, either before or after engineInitSign / engineInitVerify. Where the signature has a key already the underlying signer is re-initialised with the new context at once; where it does not the context is recorded and applied when the key arrives, as the RSASSA-PSS implementation does with its own parameters. Calling this first used to let a NullPointerException out of a method declared to throw InvalidAlgorithmParameterException, from dereferencing the key that was not there yet (see github #2396).

ML-DSA-65 and BC's RSASSA-PSS both honour that today. The composite SignatureSpi never got the change, so it still re-initialises unconditionally and dereferences the absent key, which is the same failure the javadoc above describes.

Environment

Anchors below are from origin/main at e0dce82d3. Reproduced on the current 1.86-SNAPSHOT beta (bcprov-jdk18on, Bundle-Version 1.86.0.20692) on JDK 27. The composite services are registered in the BC provider.

Reproduction

Security.addProvider(new BouncyCastleProvider());

Signature s = Signature.getInstance("MLDSA65-Ed25519-SHA512", "BC");
s.setParameter(new ContextParameterSpec("app context".getBytes()));
// NullPointerException: Cannot invoke
// "org.bouncycastle.jcajce.CompositePrivateKey.getPrivateKeys()" because "<local1>" is null

// contrast: the same order on plain ML-DSA works since the #2396 fix
Signature m = Signature.getInstance("ML-DSA-65", "BC");
m.setParameter(new ContextParameterSpec("app context".getBytes()));   // fine

Swept all 36 composite ML-DSA signature services: 36 of 36 throw the identical NullPointerException, with this stack:

java.lang.NullPointerException: Cannot invoke
    "org.bouncycastle.jcajce.CompositePrivateKey.getPrivateKeys()" because "<local1>" is null
  at org.bouncycastle.jcajce.provider.asymmetric.compositesignatures.SignatureSpi.sigInitSign
  at org.bouncycastle.jcajce.provider.asymmetric.compositesignatures.SignatureSpi.engineSetParameter
  at java.base/java.security.Signature$Delegate.engineSetParameter
  at java.base/java.security.Signature.setParameter

Controls on the same jar: ML-DSA-65 accepts setParameter before init (the #2396 fix); a composite setParameter after initSign / initVerify works, and a sign/verify round trip with a context set that way verifies. A caller intending to verify hits the same exception, since with no key the intent is unknowable and the null key falls into the signing branch.

Root cause

engineSetParameter (prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/compositesignatures/SignatureSpi.java:470, declared throws InvalidAlgorithmParameterException) stores the context at line 480 and then re-initialises the component signatures with no null check:

contextSpec = (ContextParameterSpec)algorithmParameterSpec;   // line 480
if (compositeKey instanceof PublicKey)                        // line 483
{
    sigInitVerify();
}
else
{
    sigInitSign();                                            // line 489: null key lands here
}

compositeKey (field at line 59) is null until a key arrives, and its only writers are engineInitVerify (line 147) and engineInitSign (line 194). Before init null instanceof PublicKey is false, so the else calls sigInitSign() (line 267), which evaluates compositePrivateKey.getPrivateKeys() on the null key at line 274 and throws.

The re-init is only needed when a key is already present. Neither engineInitVerify (line 139) nor engineInitSign (line 186) resets contextSpec, and the context bytes are consumed when the message is assembled in processPreHashedMessage (lines 392 to 401), which every concrete service reaches through engineSign / engineVerify. So a context recorded before init is applied correctly once the key arrives; the store at line 480 already does the recording. The set-before-init flow works end to end except for the one unguarded re-init call. The same unguarded re-init appears once more in the fall-through branch, at lines 537 and 542, so a complete fix guards both sites.

Suggested fix

Guard the re-init on a key being present, which is exactly what the base fix does: BaseDeterministicOrRandomSignature.setContext records the context, then re-initialises only if (keyParams != null). The composite analogue:

contextSpec = (ContextParameterSpec)algorithmParameterSpec;
if (compositeKey != null)
{
    try
    {
        if (compositeKey instanceof PublicKey)
        {
            sigInitVerify();
        }
        else
        {
            sigInitSign();
        }
    }
    catch (InvalidKeyException e)
    {
        throw new InvalidAlgorithmParameterException("keys invalid on reset: " + e.getMessage(), e);
    }
}

To match the base fix fully: apply the same null guard to the second re-init site in the fall-through branch (lines 537 / 542), and clear the cached AlgorithmParameters when the context changes (this.engineParams = null;, as BaseDeterministicOrRandomSignature.setContext does) so a getParameters / setParameter / getParameters sequence cannot return a stale spec from the cache at line 581.

A regression test would sit next to the existing SignatureSetParameterTest: set the context, then init, then a sign/verify round trip, plus a before/after-init equivalence check.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions