Bring the stateful hash-based signers into line with the JCA contract the newer signers already follow: LMSSignatureSpi.engineVerify() answers false for a signature it cannot decode and raises SignatureException through SecurityExceptions for one naming an OTS type other than the verifying key's rather than letting the lightweight API's unchecked exceptions out, clearing the accumulated message in a finally either way, XMSSSignature.Builder.withSignature() rejects any length other than the RFC 8391 sec. 4.1.8 fixed size so trailing data no longer gives a valid signature a second encoding the way XMSSMTSignature has always refused, and the uninitialised XMSS / XMSS^MT generateKeyPair() branches set their tree digest and default to the constructible XMSSMT-SHA2_20/2_512 parameter set, with the verify contract and the two keygen and parse pitfalls written up in the conventions, architecture and BCPQC skill docs, relates to github #2408.

Author: dghgit

.claude/skills/bcpqc-add-signature-algorithm/SKILL.md

@@ -173,12 +173,16 @@ Extends `java.security.KeyPairGenerator`. Contains:
 - `generateKeyPair()` runs the engine, wraps in BC key classes, returns a `KeyPair`.
 - **One public static inner class per parameter set** calling `super(<Alg>Parameters.<paramset>)`.
 
+If the SPI has an uninitialised-default branch in `generateKeyPair()` (`if (!initialised) { … }`), treat it as a second, parallel initialisation path: it must set **every** field `initialize(spec, random)` sets, not just `param`, and its parameters must actually be constructible. Both halves went wrong in the older XMSS/XMSS^MT generators (github #2408) — XMSS^MT defaulted to a height/layer pair that is not a legal parameter set, so the call threw outright, and both left `treeDigest` null, so a default-generated key threw `NullPointerException` from `equals()` / `hashCode()` / `getTreeDigest()`. Cover it with a test that calls `KeyPairGenerator.getInstance("<Alg>", "BCPQC").generateKeyPair()` with no `initialize()`, round-trips the key through its `KeyFactory` and compares it to itself.
+
 ### Step 10 — `prov/.../pqc/jcajce/provider/<alg>/SignatureSpi.java`
 
 Extends `java.security.Signature`. Contains a `ByteArrayOutputStream bOut` for message accumulation, the standard `engineInitSign` / `engineInitVerify` / `engineUpdate` / `engineSign` / `engineVerify` overrides, a `Base` inner class with no parameter binding (used when the caller selects via `"Faest"` and the actual parameter set comes from the key), and **one public static inner class per parameter set** that hard-pins the parameter check.
 
 In `engineInitVerify` / `engineInitSign`, when the SPI was constructed with a specific parameter set, verify the key's algorithm matches the SPI's parameter set with an exact-message error — `"signature configured for " + canonicalAlg`. Tests in other algorithms assert on that exact string, so the message format is part of the contract; copy it verbatim from SNOVA's `SignatureSpi`.
 
+`engineVerify` must never let an unchecked exception out: a signature that will not decode is `false`, one this engine cannot process is a `SignatureException` (built through `SecurityExceptions.signatureException`), and the accumulated message is cleared in a `finally` so the object survives a rejection. The lightweight signer throws unchecked for malformed input by design — translating that is the SPI's job. Full contract, and the malformed-signature battery to check it with, in the `Signature.verify()` section of `docs/claude/conventions.md`.
+
 ### Step 11 — `prov/.../pqc/jcajce/provider/<Alg>.java` (the Mappings class)
 
 The `<Alg>$Mappings` class (extends `AsymmetricAlgorithmProvider`) is what BCPQC loads via reflection. It calls:

CONTRIBUTORS.html

@@ -604,7 +604,7 @@
 <li>jmeeder &lt;https://github.com/jmeeder&gt; - 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).</li>
 <li>rimuln &lt;https://github.com/rimuln&gt; - 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).</li>
 <li>Yu Bao &lt;yubao@paypal.com&gt; - 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.</li>
-<li>Arpan Sharma &lt;https://github.com/Arpan0995&gt; - 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.</li>
+<li>Arpan Sharma &lt;https://github.com/Arpan0995&gt; - 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.</li>
 <li>Flowdalic &lt;https://github.com/Flowdalic&gt; - initial implementation of an AnimalSniffer-based Android API-level compatibility check for the Gradle build (PR #336).</li>
 <li>hannesa2 &lt;https://github.com/hannesa2&gt; - initial Dependabot configuration for the Gradle and GitHub Actions ecosystems (PR #883).</li>
 <li>vladhuma &lt;https://github.com/vladhuma&gt; - initial implementation of server-side OCSP stapling for the BCJSSE provider, on behalf of Thales Group (PR #1740).</li>

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

@@ -82,6 +82,14 @@ public Builder withSignature(byte[] val)
             int randomSize = n;
             int signatureSize = len * n;
             int authPathSize = height * n;
+            int totalSize = indexSize + randomSize + signatureSize + authPathSize;
+            if (val.length != totalSize)
+            {
+                /* an XMSS signature is a fixed-size encoding - anything longer or shorter, in
+                 * particular a valid signature carrying trailing data, is not a signature for
+                 * these parameters (RFC 8391 sec. 4.1.8). XMSSMTSignature checks the same way. */
+                throw new IllegalArgumentException("signature has wrong size");
+            }
             int position = 0;
             /* extract index */
             index = Pack.bigEndianToInt(val, position);

core/src/test/java/org/bouncycastle/pqc/crypto/test/XMSSSignatureTest.java

@@ -41,6 +41,39 @@ public void testSignatureParsingSHA512()
         assertEquals(true, Arrays.areEqual(sig1, sig3));
     }
 
+    /**
+     * An XMSS signature is a (4 + n + (len + h) * n)-byte string (RFC 8391 sec. 4.1.8), so a
+     * signature carrying trailing data - or a truncated one - is not a signature for these
+     * parameters. XMSSMTSignature has always checked this way (github #2408).
+     */
+    public void testSignatureWrongSizeRejected()
+    {
+        XMSSParameters params = new XMSSParameters(10, new SHA256Digest());
+        XMSS xmss = new XMSS(params, new NullPRNG());
+        xmss.generateKeys();
+        byte[] sig = xmss.sign(new byte[1024]);
+
+        byte[][] wrongSize = new byte[][]
+            {
+                Arrays.append(sig, (byte)0x2a),
+                Arrays.copyOfRange(sig, 0, sig.length - 1),
+                new byte[0]
+            };
+
+        for (int i = 0; i != wrongSize.length; i++)
+        {
+            try
+            {
+                new XMSSSignature.Builder(params).withSignature(wrongSize[i]).build();
+                fail("no exception on signature of wrong size: " + i);
+            }
+            catch (IllegalArgumentException e)
+            {
+                assertEquals("signature has wrong size", e.getMessage());
+            }
+        }
+    }
+
     public void testConstructor()
     {
         XMSSParameters params = new XMSSParameters(10, new SHA256Digest());

docs/claude/architecture.md

@@ -153,6 +153,33 @@ Practical checklist when porting a new PQC algorithm — easy to leave any of th
 - Tests in `prov/src/test/java/org/bouncycastle/pqc/jcajce/provider/test/<Alg>Test.java` plus an entry in `AllTests.java`, and the signature-encoding assertions above in `core`'s `PqcSignatureEncodingTest` / `PqcMalformedInputTest`. Include a `testBcProviderKeyInfoConverter`-style case that exercises `BouncyCastleProvider.getPublicKey(SubjectPublicKeyInfo)` and `getPrivateKey(PrivateKeyInfo)` against every parameter set, proving the `loadPQCKeys()` registration works.
 - `docs/releasenotes.html` — one `<li>` under the current unreleased version's "Additional Features and Functionality" block.
 
+### Two JCA-boundary details the checklist does not spell out
+
+Both were found in the stateful hash-based signers (github #2408), which predate the contract the
+newer signers settled on — but neither is specific to them, and neither is caught by a
+sign-then-verify round trip.
+
+- **A fixed-size signature encoding has to be length-checked when it is parsed.** If the parse
+  reads its fields at fixed offsets and never looks at the total length, appending arbitrary bytes
+  to a valid signature yields a second, different encoding that still verifies — encoding
+  uniqueness gone, for a scheme whose signature the spec defines as an exact byte count.
+  `XMSSSignature.Builder.withSignature()` had this and `XMSSMTSignature` did not, so the same
+  library disagreed with itself: RFC 8391 sec. 4.1.8 fixes an XMSS signature at
+  `4 + n + (len + h) * n` bytes and the check is a three-line `if`. Assert the appended *and* the
+  truncated case; the truncated one often already fails for an unrelated reason, which is what
+  makes the appended one easy to miss. See also the `Signature.verify()` contract in
+  `conventions.md`.
+- **`generateKeyPair()` with no preceding `initialize()` must return a fully-formed key.** The
+  uninitialised branch is a second, parallel initialisation path, and it has to set *everything*
+  `initialize(spec, random)` sets — not just `param`. Check its parameters are actually
+  constructible (XMSS^MT defaulted to height 10 with 20 layers, which is not a legal parameter set
+  at all, so the call simply threw), and check every *other* field the SPI carries: both the XMSS
+  and XMSS^MT generators left `treeDigest` null there, so a default-generated key threw
+  `NullPointerException` from `equals()`, `hashCode()` and `getTreeDigest()`. Grep the SPI for
+  fields assigned in `initialize` and confirm the default branch assigns each one. A one-line
+  `KeyPairGenerator.getInstance("<Alg>").generateKeyPair()` test that then round-trips the key
+  through its `KeyFactory` and compares it to itself covers both failures.
+
 ## PQC engines should stay package-private — drive KATs through the public API
 
 The lightweight `<Alg>Engine` produced when porting a reference C/Rust implementation tends to expose low-level entry points (`engine.keyGen(seedKey, sk, pk)`, `engine.sign(sk, msg, salt, mseed)`) that KAT vectors need but legitimate callers never touch. Resist the urge to make `<Alg>Engine` public just so `<Alg>KatTest` can poke at internals — that leaks the engine into the published `bcprov` API surface where it becomes load-bearing, and any future internal refactor has to preserve the engine signature too.

docs/claude/conventions.md

@@ -91,7 +91,7 @@ is usually a sign the mapping is wrong.
 
 Many tests assert on exact exception message text (e.g. `isTrue(e.getMessage().equals("..."))` or `getCause().getMessage()` checks). Changing the wording of a thrown exception — even something as small as adding a colon, rewording for clarity, or wrapping with `Exceptions.illegalArgumentException(...)` — will silently break tests in another module. Before modifying any exception message, grep the whole tree for the existing string and update every matching assertion in lockstep.
 
-## Cause-chaining via `SecurityExceptions` for cause-less JDK exceptions
+## Cause-chaining via `SecurityExceptions`
 
 A handful of `java.security` / `javax.crypto` exceptions ship only a `(String)` constructor — no `(String, Throwable)` form — including `UnrecoverableKeyException`, `IllegalBlockSizeException`, `BadPaddingException`, `NoSuchPaddingException`, `NoSuchProviderException`, `CertificateExpiredException`, `CertificateNotYetValidException`, `InvalidParameterSpecException`, `ShortBufferException`, and `AEADBadTagException`. When wrapping a caught exception with one of these inside a `catch (… e)` block, do not fold the underlying text into the new exception's string and discard the cause:
 
@@ -111,10 +111,51 @@ catch (Exception e)
 }
 ```
 
-Factories exist today for `unrecoverableKeyException`, `illegalBlockSizeException` and `badPaddingException`. Add a new factory there (same one-line shape — `return (X) new X(message).initCause(cause);`) when migrating throws of any other cause-less class above; do **not** roll `new X(msg).initCause(e)` ad-hoc at the throw site. The migration is purely additive: keep the existing message text verbatim (it is almost certainly under test assertions per the previous section) and add `e` as the second argument — callers that do not care still see the same exception type and message, while callers that do can walk `getCause()`.
+**The class covers more than the cause-less exceptions**, and that second group is the one people miss. `SignatureException`, `InvalidKeySpecException`, `InvalidAlgorithmParameterException`, `CertPathValidatorException` and friends *do* have a `(String, Throwable)` constructor — but only from **Java 5**, so using it directly breaks the Java-4 source floor (see `build-jdk14.md`). The factories exist for those too, and several carry a comment saying exactly that ("only exists from Java 5; initCause keeps the legacy (Java 4) builds compiling, so do not 'simplify' this to the two-arg constructor"). So the rule is simply: **in `prov`, never write `new X(msg, cause)` for a `java.security` / `javax.crypto` exception — call the factory.** A file that today sits behind an `ant/jdk14.xml` exclude is not an argument for the two-arg form; the excludes move.
+
+Factories exist today for `invalidKeySpecException`, `generalSecurityException`, `invalidKeyException`, `invalidAlgorithmParameterException`, `noSuchAlgorithmException`, `signatureException`, `unrecoverableKeyException`, `illegalBlockSizeException`, `badPaddingException`, `certPathValidatorException`, `certPathBuilderException` and `certificateEncodingException`. Add a new factory there (same one-line shape — `return (X) new X(message).initCause(cause);`) when you need one; do **not** roll `new X(msg).initCause(e)` ad-hoc at the throw site. The migration is purely additive: keep the existing message text verbatim (it is almost certainly under test assertions per the previous section) and add `e` as the second argument — callers that do not care still see the same exception type and message, while callers that do can walk `getCause()`.
+
+**There is a `prov/src/main/jdk1.3` overlay of this class**, and it rots like every other overlay: a new factory has to be added there too, or a 1.3-reachable caller fails to compile in that build. It is one factory behind today (`invalidAlgorithmParameterException` is missing) — latent only because nothing calls that one yet.
 
 Throw sites *outside* a `catch` block — value-check branches like `if (x.size() == 0) throw new UnrecoverableKeyException("…")` — have nothing to chain and stay as plain `new X(msg)`. The audit grep when adding a factory is `grep -rnE "new $Cls\(" prov/src/main/java` filtered by which lines contain `e.getMessage()` / `e.toString()` (the cause-folding pattern); pure-literal throws are not candidates.
 
+## `Signature.verify()` reports; it never throws unchecked
+
+A provider `Signature` / `SignatureSpi` has exactly three answers at the JCA boundary, and an
+unchecked exception is none of them:
+
+- **Well-formed but does not verify** — return `false`.
+- **Will not decode at all** (empty, truncated, trailing data, garbage) — also return `false`. That
+  is an invalid signature, not an error.
+- **Decodes, but this engine cannot process it** — it names a different algorithm, parameter set or
+  OTS type than the key — throw `SignatureException` (via `SecurityExceptions.signatureException`,
+  per the section above).
+
+The lightweight `org.bouncycastle.crypto.*` signers and their `*Signature` / `*Context` parse
+helpers throw unchecked for malformed input *by design*; translating that is the SPI's job. The
+whole provider was measured against one malformed-signature battery (empty / 3 bytes / zeroed /
+signature+1 / signature-1) for github #2408: ML-DSA, SLH-DSA, Falcon, XMSS, XMSS^MT, Ed25519 and
+RSA all answer `false`, and ECDSA throws `SignatureException` on the DER decode failure, which is
+the JCA's own documented behaviour and equally fine. LMS was the sole outlier, letting
+`IllegalStateException("cannot parse signature")` straight out through `Signature.verify()`. Run
+that battery against any signer you touch — it is a dozen lines and it is the only thing that
+surfaces this class of gap.
+
+Two details that are easy to get wrong when adding the translation:
+
+- **Scope the catch to the decode call, not the whole method.** Past the parse, BC's verifiers are
+  written to report an inconsistent signature by returning `false` rather than by throwing — see
+  the "these two can get out of sync with an invalid signature, we'll try and fail gracefully"
+  branch in `LMSEngine.verifySignature`. A `catch (RuntimeException)` around the entire body
+  therefore catches nothing extra today, while silently converting any *future* internal error into
+  a quiet `false`. Read the engine before widening.
+- **Reset the accumulated message in a `finally`.** However `engineVerify` leaves, the object must
+  go back to the state `engineInitVerify` left it in, or the next `update()` appends to stale data
+  and the *following* verify fails for no visible reason. `slhdsa/SignatureSpi` is the model
+  (`try { … } finally { bOut.reset(); }`) and `lms/LMSSignatureSpi` now follows it. On the success
+  path the digest's `doFinal` has usually reset it already, so the `finally` is a no-op there —
+  which is fine, and far safer than one `reset()` per exit that the next branch will forget.
+
 ## System / security property constants
 
 Any system or security property that controls BC behaviour belongs in `core/src/main/java/org/bouncycastle/util/Properties.java` as a `public static final String`, e.g. `Properties.PKCS12_MAX_IT_COUNT`, `Properties.PKCS12_IGNORE_USELESS_PASSWD`, `Properties.EMULATE_ORACLE`. Callers should reference the constant rather than inlining the literal `"org.bouncycastle.…"` name — both in production code and in tests that flip the property via `System.setProperty`. New properties should be added to `Properties` with the same naming pattern (`org.bouncycastle.<area>.<flag>`).
@@ -222,6 +263,13 @@ of older entries do read "Reported ..." for findings that came with substantial
 not an absolute rule — but the default is no entry, and adding one for a bare report is a change
 dgh should make rather than something to assume. Ask if unsure.
 
+**Sustained auditing is the standing exception**, and dgh initiates it. Someone who works through a
+subsystem and turns up several confirmed defects gets an entry describing the audit rather than a
+patch — `Arpan Sharma`'s reads "initial audit of BCPQC provider consistency starting with HQC ..."
+and was extended in the same house style when a later sweep produced github #2408. Note the shape:
+it names the area swept and what the sweep led to, not the individual bugs, and it was **appended to
+the existing entry**. Still don't add one unprompted — wait for dgh to ask.
+
 When an entry is warranted it goes at the end of the list in the house form
 `<li>name-or-handle &lt;email-or-github-url&gt; - what they contributed (PR #NNNN).</li>`; a bare
 GitHub handle with `https://github.com/<handle>` in place of an email is well established. When a