Add generic ML-DSA KeyFactory and Signature by jasonkatonica · Pull Request #1667 · IBM/OpenJCEPlus

jasonkatonica · 2026-07-22T18:11:39Z

Register a family-level ML-DSA KeyFactory and Signature service so that callers can use the algorithm name ML-DSA without specifying a parameter set, matching the JEP 497 / JDK 24+ API contract.

Register PQCKeyFactory$MLDSA and PQCSignatureImpl$MLDSA as the ML-DSA service. Remove the ML-DSA alias from ML-DSA-65 (it was a
mis-mapping that silently directed all generic lookups to ML-DSA-65).
Split the single name field into familyName (returned by getAlgorithm() such as ML-DSA) and paramSetName (the concrete parameter set such as ML-DSA-65). Add getters and setters for these values such that other code can act accordingly.
Add tests to BaseTestPQCKeyInterop. These tests cover all three ML-DSA parameter sets via @ParameterizedTest.
Add tests to BaseTestPQCKeys. Expand alias coverage to include case variants, OID aliases, and bare OID strings. Added tests for getAlgorithm() family-name correctness for all ML-DSA and ML-KEM aliases
Add tests to BaseTestPQCSignature for generic ML-DSA sign/verify tests to all three parameter sets.

Fixes: #1567

Signed-off-by: Jason Katonica katonica@us.ibm.com

JinhangZhang

LGTM

KostasTsiounis · 2026-08-19T17:02:05Z

    }

+    /**
+     * Returns the family name for a known PQC algorithm, or the param-set name


I don't think the param-set name is ever returned from this method. Am I missing some scenario?

Yes agreed. I updated the javadoc here to state that we always return the family name.

KostasTsiounis · 2026-08-19T17:02:28Z

    }

+    /**
+     * Returns the family name for a known PQC algorithm, or the param-set name


Similar comment.

Yes agreed. I updated the javadoc here to state that we always return the family name.

KostasTsiounis · 2026-08-19T17:05:39Z

+        // The generic "ML-DSA" instance accepts any ML-DSA parameter-set key.
+        String keyParam = keyPrivate.getParamSetName();
+        boolean paramMatches = keyParam.equalsIgnoreCase(this.alg)
+            || ("ML-DSA".equals(this.alg) && keyParam.startsWith("ML-DSA"));


Should this be keyParam.startsWith("ML-DSA-")? Notice the last - in the comparing string.

Yes agreed its best to compare using the - at the end to ensure its the param name not family name being matched upon. Updated this code to reflect this.

KostasTsiounis · 2026-08-19T17:07:06Z


        try {
-            this.signature.initialize(keyPrivate.getPQCKey(), keyPrivate.getAlgorithm().replace('_', '-'));
+            this.signature.initialize(keyPrivate.getPQCKey(), keyPrivate.getParamSetName().replace('_', '-'));


Can the name ever have a _? Do we need the replace here?

You are right, getParamSetName() returns an already formatted param set name so i dont think we need to do this. I've removed the .replace('_', '-') call here and in engineInitVerify.

KostasTsiounis · 2026-08-19T17:07:17Z

+        // The generic "ML-DSA" instance accepts any ML-DSA parameter-set key.
+        String keyParam = keyPublic.getParamSetName();
+        boolean paramMatches = keyParam.equalsIgnoreCase(this.alg)
+            || ("ML-DSA".equals(this.alg) && keyParam.startsWith("ML-DSA"));


Similar comment.

Yes agreed its best to compare using the - at the end to ensure its the param name not family name being matched upon. Updated this code to reflect this.

KostasTsiounis · 2026-08-19T17:09:34Z

        // The original and new keys are the same
        same = Arrays.equals(publicKeyBytesInterop, pub.getEncoded());
-        assertTrue(same);
+        assertTrue(same, "Public key bytes differ for " + pqcAlgorithm);


Similar comment.

Yes agreed we should be using assertArrayEquals() through this entire test. I converted all asserts dealing with byte arrays to use this method.

KostasTsiounis · 2026-08-19T17:10:01Z

+    @ParameterizedTest
+    @CsvSource({"ML-DSA", "ML-DSA-44", "ML-DSA-65", "ML-DSA-87"})
+    public void testGenericMLDSAKeyFactoryImportsInteropKeys(String paramSetName) throws Exception {
+        assumeFalse("OpenJCEPlusFIPS".equals(getProviderName()));


Do we need the assumeFalse here?

Yes agreed that these assumes for the OpenJCEPlusFIPS provider are redundant since we currently do not run these with that provider set in our test paramaters. I removed these redundant checks throughout the BaseTestPQCKeyInterop class

KostasTsiounis · 2026-08-19T17:10:24Z

+        // BC private-key encoding differs; only compare against SunJCE
+        if (getInteropProviderName().equals(Utils.PROVIDER_SunJCE)) {
+            PrivateKey priv = genericKF.generatePrivate(new PKCS8EncodedKeySpec(pkcs8Bytes));
+            assertTrue(Arrays.equals(pkcs8Bytes, priv.getEncoded()),


Why not assertArrayEquals()?

Yes agreed we should be using assertArrayEquals() through this entire test. I converted all asserts dealing with byte arrays to use this method.

KostasTsiounis · 2026-08-19T17:10:41Z

+        PublicKey pubInterop = kfInterop.generatePublic(
+                new X509EncodedKeySpec(keyPairPlus.getPublic().getEncoded()));
+
+        assertTrue(Arrays.equals(keyPairPlus.getPublic().getEncoded(), pubInterop.getEncoded()),


Similar comment.

Yes agreed we should be using assertArrayEquals() through this entire test. I converted all asserts dealing with byte arrays to use this method.

KostasTsiounis · 2026-08-19T17:12:00Z

-            System.out.println("FIPS does not support plain keys.  Returning to caller.");
-            return;
-        }
+        assumeFalse("OpenJCEPlusFIPS".equals(getProviderName()));


Do we need this?

Yes agreed that these assumes for the OpenJCEPlusFIPS provider are redundant since we currently do not run these with that provider set in our test paramaters. I removed these redundant checks throughout the BaseTestPQCSignatureWithAliases class

Register a family-level `ML-DSA` `KeyFactory` and `Signature` service so that callers can use the algorithm name `ML-DSA` without specifying a parameter set, matching the JEP 497 / JDK 24+ API contract. - Register `PQCKeyFactory$MLDSA` and `PQCSignatureImpl$MLDSA` as the `ML-DSA` service. Remove the `ML-DSA` alias from `ML-DSA-65` (it was a mis-mapping that silently directed all generic lookups to ML-DSA-65). - Split the single `name` field into `familyName` (returned by `getAlgorithm()` such as `ML-DSA`) and `paramSetName` (the concrete parameter set such as `ML-DSA-65`). Add getters and setters for these values such that other code can act accordingly. - Add tests to `BaseTestPQCKeyInterop`. These tests cover all three `ML-DSA` parameter sets via `@ParameterizedTest`. - Add tests to `BaseTestPQCKeys`. Expand alias coverage to include case variants, OID aliases, and bare OID strings. Added tests for `getAlgorithm()` family-name correctness for all ML-DSA and ML-KEM aliases - Add tests to `BaseTestPQCSignature` for generic `ML-DSA` sign/verify tests to all three parameter sets. Fixes: IBM#1567 Signed-off-by: Jason Katonica <katonica@us.ibm.com>

Signed-off-by: Jason Katonica <katonica@us.ibm.com>

jasonkatonica force-pushed the katonica/issue1567/genericmldsa branch from 0bcff82 to 96a9583 Compare July 22, 2026 18:13

jasonkatonica requested review from JinhangZhang, KostasTsiounis, johnpeck-us-ibm and taoliult July 23, 2026 11:20

jasonkatonica self-assigned this Jul 23, 2026

johnpeck-us-ibm requested changes Jul 23, 2026

View reviewed changes

Comment thread src/main/java/com/ibm/crypto/plus/provider/MLKEMImpl.java Outdated

Comment thread src/main/java/com/ibm/crypto/plus/provider/MLKEMImpl.java Outdated

JinhangZhang reviewed Jul 24, 2026

View reviewed changes

Comment thread src/main/java/com/ibm/crypto/plus/provider/MLKEMImpl.java Outdated

JinhangZhang reviewed Jul 24, 2026

View reviewed changes

Comment thread src/main/java/com/ibm/crypto/plus/provider/MLKEMImpl.java Outdated

jasonkatonica requested review from JinhangZhang and johnpeck-us-ibm July 27, 2026 14:01

johnpeck-us-ibm approved these changes Jul 28, 2026

View reviewed changes

JinhangZhang approved these changes Jul 29, 2026

View reviewed changes

KostasTsiounis requested changes Aug 19, 2026

View reviewed changes

jasonkatonica force-pushed the katonica/issue1567/genericmldsa branch from bb7ac47 to 9427647 Compare September 1, 2026 13:03

jasonkatonica added 2 commits September 1, 2026 09:04

Address review comments

2f41b6c

Signed-off-by: Jason Katonica <katonica@us.ibm.com>

merge conflicts

00b59b1

Signed-off-by: Jason Katonica <katonica@us.ibm.com>

jasonkatonica force-pushed the katonica/issue1567/genericmldsa branch 3 times, most recently from 5b0d00a to a6e6bdd Compare September 1, 2026 15:26

Updates from code comments

f391d23

Signed-off-by: Jason Katonica <katonica@us.ibm.com>

jasonkatonica force-pushed the katonica/issue1567/genericmldsa branch from a6e6bdd to f391d23 Compare September 1, 2026 17:33

jasonkatonica requested a review from KostasTsiounis September 1, 2026 19:53

Uh oh!

Conversation

jasonkatonica commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JinhangZhang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jasonkatonica commented Jul 22, 2026 •

edited

Loading