Allow the ML-DSA and SLH-DSA signature context to be set before initSign/initVerify as well as after, rather than dereferencing the absent key, relates to github #2396.

Author: dghgit

docs/releasenotes.html

@@ -121,6 +121,7 @@ <h3>2.1.2 Defects Fixed</h3>
 <li>The JCA/JCE provider classes for the pre-standardisation SPHINCS+ and Kyber have been removed from the BCPQC provider hierarchy: org.bouncycastle.pqc.jcajce.provider.sphincsplus and .kyber, their SPHINCSPlus and Kyber Mappings classes, and the unused org.bouncycastle.jcajce.provider.asymmetric.SPHINCSPlus mappings alongside them. Neither was listed in BouncyCastlePQCProvider's algorithm set or in BouncyCastleProvider's, so none of the services they described - "SPHINCS+-SHA2-128S", the BCPQC-side "ML-KEM-512", and the rest - had been obtainable through either provider; they were superseded by SLH-DSA and ML-KEM in the BC provider, which are registered and are what callers should use. The corresponding exports have been dropped from both module-info descriptors. The lightweight implementations are untouched, as is the unrelated org.bouncycastle.pqc.legacy.sphincsplus package.</li>
 <li>The parameter-set specific HashML-DSA Signature services in the BC provider refused a pure ML-DSA key of their own parameter set, so a caller who obtained a key as "ML-DSA-65" and a signature as "ML-DSA-65-WITH-SHA512" was met with InvalidKeyException("signature configured for ML-DSA-65-WITH-SHA512"). FIPS 204 sec. 5 defines one key generation algorithm per parameter set and the keys it produces carry no commitment to the pure mode over the pre-hash one, so the pure key was a perfectly good HashML-DSA key; the SPIs were comparing the key's algorithm name against their own, which differ by the "-WITH-SHA512" suffix. It particularly bit certificate-sourced keys, which carry the pure OID (RFC 9881), and the provider was already inconsistent about it - the unparameterised "HASH-ML-DSA" and "HASH-ML-DSA-EXTERNAL-HASH" services accepted pure keys all along. The comparison is now against the parameter set rather than the name, so a pure key is accepted wherever its parameter set matches. Signatures are unchanged: HashMLDSASigner derives the same SHA-512 pre-hash for a pure and a pre-hash key of the same parameter set, so the bytes produced are identical either way. <b>The tolerance is one way only</b> - a key that names a HashML-DSA parameter set has been narrowed to the pre-hash mode and is still refused by the pure ML-DSA services - and a key of any other parameter set is still refused in both directions (github #2397).</li>
 <li>The parameter-set specific SLH-DSA Signature services were not bound to their parameter set at all: every one of the twenty four algorithm names, and their OIDs, was registered as an alias of the unparameterised "SLH-DSA" or "HASH-SLH-DSA" service, so the name a caller asked for had no effect on which keys were accepted. Signature.getInstance("SLH-DSA-SHAKE-256S") would sign quite happily with an SLH-DSA-SHA2-128F key, producing a SHA2-128F signature, and likewise across every other pair. This matters to a caller using the algorithm name as a policy gate - a service that means to sign or verify only at a chosen parameter set - which had no way to tell that the name was being ignored. Each name and OID is now registered against an SPI bound to its own parameter set, applying the same rule described for ML-DSA above: a pure key is accepted by the pre-hash service of its own parameter set, a pre-hash key is refused by the pure service, and a key of any other parameter set is refused either way. <b>This is a behavioural change for a caller that relied on a parameter-set named SLH-DSA Signature accepting a key of a different parameter set</b>, which now raises InvalidKeyException; the unparameterised "SLH-DSA" and "HASH-SLH-DSA" services name no parameter set and are unchanged, so they remain the way to work with a key whose parameter set is not known in advance. Note also that the pre-hash OID table was in a different order from the algorithm names it is now index-matched against, which is corrected here - while everything aliased to a single service the ordering could not be observed.</li>
+<li>Signature.setParameter(...) on any of the ML-DSA or SLH-DSA services threw NullPointerException when called before initSign / initVerify, out of a method declared to throw InvalidAlgorithmParameterException. The context carried by a ContextParameterSpec is applied by re-initialising the underlying signer with the key the Signature holds, and BaseDeterministicOrRandomSignature.engineSetParameter went straight to that re-initialisation without checking there was a key to re-initialise with. The context may now be set either side of initialisation, as the RSASSA-PSS parameters may be: setting it on a signature that already has a key re-initialises the signer at once as before, and setting it beforehand records it to be applied when the key arrives. Initialising no longer discards a context set beforehand, which it previously did by resetting the context to empty - so a caller who set one first and would have had it silently dropped now gets the signature they asked for. Setting a context after initialisation, which is what the existing tests and examples do, produces exactly the signatures it did before, and setting one in the middle of an update is still refused with a ProviderException. Note the context now also survives a subsequent re-initialisation of the same Signature object, again as the RSASSA-PSS parameters do (github #2396).</li>
 <li>An OCSP response carrying no nextUpdate could be cached and reused as though it stated a validity interval, so a response could go on answering for a certificate after the responder had newer information about it - after a revocation, in particular. Any such reuse was bounded only by garbage collection rather than by an interval: OcspCache holds each responder's response map through a WeakReference, itself in a WeakHashMap, and nothing else refers to that map once the call returns, so entries last until the next collection. That is unpredictable rather than long - short on a busy JVM, potentially much longer on a large heap that collects rarely - and it is not a window the responder or the caller had any say in. RFC 6960 sec. 4.2.2.1 says the opposite of what that assumes: "if nextUpdate is not set, the responder is indicating that newer revocation information is available all the time", which is a statement that there is no interval to reuse the response over, not that it never expires. OcspCache now separates the two questions it had been asking with one method: a response arriving from the responder is accepted as before, whether or not it states a nextUpdate, while only a response that states one may be served from the cache afterwards. Nothing is rejected that was previously accepted - a responder that omits nextUpdate simply costs another request per validation, which is what "available all the time" asks for. Additionally, both the cached and the caller-supplied (stapled) paths now apply RFC 6960 sec. 4.2.2.1's other freshness rule, "responses whose thisUpdate time is later than the local system time SHOULD be considered unreliable", which neither had checked: a response dated ahead of the time being validated for by more than a 15 minute clock-skew allowance is treated as unreliable, raising "OCSP response not yet valid" on the stapled path.</li>
 </ul>
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/util/BaseDeterministicOrRandomSignature.java

@@ -42,7 +42,6 @@ final protected void engineInitVerify(PublicKey publicKey)
         throws InvalidKeyException
     {
         verifyInit(publicKey);
-        paramSpec = ContextParameterSpec.EMPTY_CONTEXT_SPEC;
         isInitState = true;
         reInit();
     }
@@ -54,7 +53,6 @@ final protected void engineInitSign(
         throws InvalidKeyException
     {
         signInit(privateKey, null);
-        paramSpec = ContextParameterSpec.EMPTY_CONTEXT_SPEC;
         isInitState = true;
         reInit();
     }
@@ -65,7 +63,6 @@ final protected void engineInitSign(
         throws InvalidKeyException
     {
         signInit(privateKey, random);
-        paramSpec = ContextParameterSpec.EMPTY_CONTEXT_SPEC;
         isInitState = true;
         reInit();
     }
@@ -95,6 +92,17 @@ final protected void engineUpdate(
 
     protected abstract void updateEngine(byte[] buf, int off, int len) throws SignatureException;
 
+    /**
+     * Set the signature context, either before or after engineInitSign / engineInitVerify.
+     * <p>
+     * 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
+     * <a href="https://github.com/bcgit/bc-java/issues/2396">github #2396</a>).
+     * </p>
+     */
     protected void engineSetParameter(
         AlgorithmParameterSpec params)
         throws InvalidAlgorithmParameterException
@@ -118,16 +126,14 @@ protected void engineSetParameter(
 
         if (params instanceof ContextParameterSpec)
         {
-            this.paramSpec = (ContextParameterSpec)params;
-            reInit();
+            setContext((ContextParameterSpec)params);
         }
         else
         {
             byte[] context = SpecUtil.getContextFrom(params);
             if (context != null)
             {                   
-                this.paramSpec = new ContextParameterSpec(context);
-                reInit();
+                setContext(new ContextParameterSpec(context));
             }
             else
             {
@@ -136,6 +142,17 @@ protected void engineSetParameter(
         }
     }
 
+    private void setContext(ContextParameterSpec context)
+    {
+        this.paramSpec = context;
+        this.engineParams = null;
+
+        if (keyParams != null)
+        {
+            reInit();
+        }
+    }
+
     private void reInit()
     {
         CipherParameters param = keyParams;

prov/src/test/java/org/bouncycastle/pqc/jcajce/provider/test/AllTests.java

@@ -69,6 +69,7 @@ public static Test suite()
         suite.addTestSuite(NamedKeyPairGeneratorTest.class);
         suite.addTestSuite(NamedKeyFactoryTest.class);
         suite.addTestSuite(PreHashKeyInteropTest.class);
+        suite.addTestSuite(SignatureSetParameterTest.class);
         suite.addTestSuite(MayoKeyPairGeneratorTest.class);
         suite.addTestSuite(MayoTest.class);
         suite.addTestSuite(SnovaTest.class);

prov/src/test/java/org/bouncycastle/pqc/jcajce/provider/test/SignatureSetParameterTest.java

@@ -0,0 +1,242 @@
+package org.bouncycastle.pqc.jcajce.provider.test;
+
+import java.security.AlgorithmParameters;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.ProviderException;
+import java.security.Security;
+import java.security.Signature;
+
+import junit.framework.TestCase;
+import org.bouncycastle.jcajce.spec.ContextParameterSpec;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.util.Arrays;
+import org.bouncycastle.util.Strings;
+
+/**
+ * Signature.setParameter(...) on the ML-DSA and SLH-DSA services, which take a signature context
+ * through a {@link ContextParameterSpec}.
+ * <p>
+ * The context may be set either side of initSign / initVerify, as it may be for the RSASSA-PSS
+ * parameters: setting it while the signature already has a key re-initialises the underlying signer
+ * at once, and setting it beforehand records it for the key to arrive. Setting it first used to let
+ * a NullPointerException out of a method declared to throw InvalidAlgorithmParameterException,
+ * from dereferencing the key that was not there yet (github #2396).
+ */
+public class SignatureSetParameterTest
+    extends TestCase
+{
+    private static final byte[] MSG = Strings.toByteArray("the quick brown fox");
+    private static final byte[] CONTEXT = Strings.toByteArray("Hello, world!");
+
+    private static final String[] SIGNATURE_ALGORITHMS =
+        {
+            "ML-DSA",
+            "ML-DSA-65",
+            "ML-DSA-65-WITH-SHA512",
+            "HASH-ML-DSA",
+            "SLH-DSA",
+            "SLH-DSA-SHA2-128F",
+            "SLH-DSA-SHA2-128F-WITH-SHA256",
+            "HASH-SLH-DSA"
+        };
+
+    private static final String[] KEY_ALGORITHMS =
+        {
+            "ML-DSA-65",
+            "ML-DSA-65",
+            "ML-DSA-65",
+            "ML-DSA-65",
+            "SLH-DSA-SHA2-128F",
+            "SLH-DSA-SHA2-128F",
+            "SLH-DSA-SHA2-128F",
+            "SLH-DSA-SHA2-128F"
+        };
+
+    public void setUp()
+    {
+        if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null)
+        {
+            Security.addProvider(new BouncyCastleProvider());
+        }
+    }
+
+    /**
+     * Setting the context before initialising has to reach the signer, not be dropped on the way -
+     * so the signature it produces must be the one the established after-init order produces, and
+     * must not verify against a signature service given no context at all.
+     */
+    public void testSetParameterBeforeInit()
+        throws Exception
+    {
+        for (int i = 0; i != SIGNATURE_ALGORITHMS.length; i++)
+        {
+            String algorithm = SIGNATURE_ALGORITHMS[i];
+
+            KeyPair kp = KeyPairGenerator.getInstance(KEY_ALGORITHMS[i], "BC").generateKeyPair();
+
+            Signature before = Signature.getInstance(algorithm, "BC");
+
+            before.setParameter(new ContextParameterSpec(CONTEXT));
+            before.initSign(kp.getPrivate());
+            before.update(MSG);
+
+            byte[] sigBefore = before.sign();
+
+            Signature after = Signature.getInstance(algorithm, "BC");
+
+            after.initSign(kp.getPrivate());
+            after.setParameter(new ContextParameterSpec(CONTEXT));
+            after.update(MSG);
+
+            assertTrue(algorithm + ": context set before init did not reach the signer",
+                Arrays.areEqual(sigBefore, after.sign()));
+
+            Signature verifier = Signature.getInstance(algorithm, "BC");
+
+            verifier.setParameter(new ContextParameterSpec(CONTEXT));
+            verifier.initVerify(kp.getPublic());
+            verifier.update(MSG);
+
+            assertTrue(algorithm, verifier.verify(sigBefore));
+
+            Signature noContext = Signature.getInstance(algorithm, "BC");
+
+            noContext.initVerify(kp.getPublic());
+            noContext.update(MSG);
+
+            assertFalse(algorithm + ": verified without the context", noContext.verify(sigBefore));
+        }
+    }
+
+    public void testSetParameterAfterInit()
+        throws Exception
+    {
+        for (int i = 0; i != SIGNATURE_ALGORITHMS.length; i++)
+        {
+            String algorithm = SIGNATURE_ALGORITHMS[i];
+
+            KeyPair kp = KeyPairGenerator.getInstance(KEY_ALGORITHMS[i], "BC").generateKeyPair();
+
+            Signature signer = Signature.getInstance(algorithm, "BC");
+
+            signer.initSign(kp.getPrivate());
+            signer.setParameter(new ContextParameterSpec(CONTEXT));
+            signer.update(MSG);
+
+            byte[] sig = signer.sign();
+
+            Signature verifier = Signature.getInstance(algorithm, "BC");
+
+            verifier.initVerify(kp.getPublic());
+            verifier.setParameter(new ContextParameterSpec(CONTEXT));
+            verifier.update(MSG);
+
+            assertTrue(algorithm, verifier.verify(sig));
+
+            Signature wrongContext = Signature.getInstance(algorithm, "BC");
+
+            wrongContext.initVerify(kp.getPublic());
+            wrongContext.setParameter(new ContextParameterSpec(Strings.toByteArray("other context")));
+            wrongContext.update(MSG);
+
+            assertFalse(algorithm, wrongContext.verify(sig));
+        }
+    }
+
+    /**
+     * The null spec resolves to the empty context, which is what a signature with no context set
+     * uses anyway, so it is accepted before initialisation as well.
+     */
+    public void testSetNullParameterBeforeInit()
+        throws Exception
+    {
+        KeyPair kp = KeyPairGenerator.getInstance("ML-DSA-65", "BC").generateKeyPair();
+
+        Signature sig = Signature.getInstance("ML-DSA", "BC");
+
+        sig.setParameter(null);
+        sig.initSign(kp.getPrivate());
+        sig.update(MSG);
+
+        byte[] s = sig.sign();
+
+        Signature verifier = Signature.getInstance("ML-DSA", "BC");
+
+        verifier.initVerify(kp.getPublic());
+        verifier.update(MSG);
+
+        assertTrue(verifier.verify(s));
+    }
+
+    public void testGetParametersReflectsContextSetBeforeInit()
+        throws Exception
+    {
+        KeyPair kp = KeyPairGenerator.getInstance("ML-DSA-65", "BC").generateKeyPair();
+
+        Signature sig = Signature.getInstance("ML-DSA", "BC");
+
+        sig.setParameter(new ContextParameterSpec(CONTEXT));
+        sig.initSign(kp.getPrivate());
+
+        AlgorithmParameters params = sig.getParameters();
+
+        assertNotNull(params);
+        assertTrue(Arrays.areEqual(CONTEXT,
+            params.getParameterSpec(ContextParameterSpec.class).getContext()));
+    }
+
+    /**
+     * Re-setting the context replaces the previous one rather than being reported through a stale
+     * cached AlgorithmParameters.
+     */
+    public void testResetContext()
+        throws Exception
+    {
+        byte[] second = Strings.toByteArray("second context");
+
+        KeyPair kp = KeyPairGenerator.getInstance("ML-DSA-65", "BC").generateKeyPair();
+
+        Signature sig = Signature.getInstance("ML-DSA", "BC");
+
+        sig.setParameter(new ContextParameterSpec(CONTEXT));
+        sig.initSign(kp.getPrivate());
+        sig.setParameter(new ContextParameterSpec(second));
+
+        assertTrue(Arrays.areEqual(second,
+            sig.getParameters().getParameterSpec(ContextParameterSpec.class).getContext()));
+
+        sig.update(MSG);
+
+        byte[] s = sig.sign();
+
+        Signature verifier = Signature.getInstance("ML-DSA", "BC");
+
+        verifier.setParameter(new ContextParameterSpec(second));
+        verifier.initVerify(kp.getPublic());
+        verifier.update(MSG);
+
+        assertTrue(verifier.verify(s));
+    }
+
+    public void testSetParameterMidUpdateStillRejected()
+        throws Exception
+    {
+        KeyPair kp = KeyPairGenerator.getInstance("ML-DSA-65", "BC").generateKeyPair();
+
+        Signature sig = Signature.getInstance("ML-DSA", "BC");
+
+        sig.initSign(kp.getPrivate());
+        sig.update(MSG);
+
+        try
+        {
+            sig.setParameter(new ContextParameterSpec(CONTEXT));
+            fail("no exception");
+        }
+        catch (ProviderException e)
+        {
+            assertEquals("cannot call setParameter in the middle of update", e.getMessage());
+        }
+    }
+}