Skip to content

Add ML-DSA signing and verification support - #54018

Closed
adilalperenciftci wants to merge 1 commit into
keycloak:mainfrom
adilalperenciftci:fix/keycloak-52816-mldsa
Closed

Add ML-DSA signing and verification support#54018
adilalperenciftci wants to merge 1 commit into
keycloak:mainfrom
adilalperenciftci:fix/keycloak-52816-mldsa

Conversation

@adilalperenciftci

@adilalperenciftci adilalperenciftci commented Oct 15, 2026

Copy link
Copy Markdown

Summary

  • add RFC 9964 ML-DSA-44, ML-DSA-65, and ML-DSA-87 JOSE/JCA mappings
  • add capability-gated server and client signature providers plus generated realm keys
  • strictly validate AKP algorithm, encoding, key length, key type, and parameter-set consistency
  • publish AKP realm keys without private material and compute canonical AKP thumbprints
  • enforce asymmetric algorithms in attestation-based client authentication and preserve canonical JWA names through verification paths

The factories are unavailable when the active crypto provider does not expose the required ML-DSA services, including BC-FIPS configurations without ML-DSA support.

Testing

  • ./mvnw -pl core -Dtest=MLDSAAlgorithmTest test
  • ./mvnw -pl crypto/default -Dtest=DefaultCryptoAKPJWKTest test
  • ./mvnw -pl services "-Dtest=CryptoUtilsTest,MLDSASignatureProviderTest,MLDSAClientSignatureVerifierProviderTest,GeneratedMLDSAKeyProviderTest,JWKSServerUtilsTest" test
  • ./mvnw -pl server-spi-private test

Fixes #52816
Addresses #52815

@adilalperenciftci
adilalperenciftci requested a review from a team as a code owner October 15, 2026 12:34
Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:34
Copilot AI reviewed Oct 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@adilalperenciftci

adilalperenciftci commented Oct 20, 2026

Copy link
Copy Markdown
Author

DCO passes on the current head, but the GitHub Actions workflows still need maintainer approval and there is no human review yet. Could a maintainer approve the workflows and take a first look?

@adilalperenciftci
Add ML-DSA signing and verification support
53f8dac 
Closes keycloak#52816

Signed-off-by: Adil Alperen Ciftci <adilalperenciftci@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 6, 2026 22:04
Copilot AI reviewed Nov 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@adilalperenciftci

adilalperenciftci commented Nov 6, 2026

Copy link
Copy Markdown
Author

Rebased onto current main (d4d5554) in 53f8dac. The client-attestation conflict now retains upstream's x5c certificate-chain argument and the canonical algorithm handling. The remaining adjustments are import-only formatting fixes in the existing PR files.

Validated in the Ubuntu lab with JDK 21:

  • ./mvnw -B -ntp -pl core,crypto/default,server-spi-private,services -Dsurefire.memory.Xmx=1024m -DforkCount=1 -DreuseForks=true test: 1,681 tests, zero failures/errors, six skips.
  • ./mvnw -B -ntp -pl core,crypto/default,server-spi-private,services spotless:check: passed.

FIPS and server integration environments were not tested. Could a maintainer approve the updated workflows and review the ML-DSA support?

@quillsecurity quillsecurity Bot mentioned this pull request Nov 10, 2026
Open
@stianst
stianst requested a max review from Copilot November 12, 2026 07:33
Copilot AI reviewed Nov 12, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Premature capability checks exclude all ML-DSA factories from clean Quarkus builds, while algorithm-name mismatches break integration paths and the truncated-signature test fails on JDK 25.

5 open issues

Review effort: Max

Comment thread services/src/main/java/org/keycloak/crypto/MLDSAUtils.java
Comment on lines +34 to +35
if (!ALGORITHMS.contains(algorithm) || !CryptoIntegration.isInitialised()) {
return false;
Comment thread services/src/test/java/org/keycloak/crypto/MLDSASignatureProviderTest.java
MLDSASignatureProvider provider = new MLDSASignatureProvider(session, Algorithm.ML_DSA_44);
byte[] signature = provider.signer(key).sign(PAYLOAD);

assertFalse(provider.verifier(key).verify(PAYLOAD, Arrays.copyOf(signature, signature.length - 1)));
Comment thread core/src/main/java/org/keycloak/jose/jws/Algorithm.java
Comment on lines +49 to +51
ML_DSA_44(AlgorithmType.ML_DSA, null, org.keycloak.crypto.Algorithm.ML_DSA_44),
ML_DSA_65(AlgorithmType.ML_DSA, null, org.keycloak.crypto.Algorithm.ML_DSA_65),
ML_DSA_87(AlgorithmType.ML_DSA, null, org.keycloak.crypto.Algorithm.ML_DSA_87)
Comment thread ...src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java
throw new VCIssuerException(ErrorType.INVALID_PROOF, "Key with kid '" + header.getKeyId() + "' not found in trusted key registry");
}
verifier = verifierFromResolvedJWK(resolvedJwk, header.getAlgorithm().name(), keycloakSession);
verifier = verifierFromResolvedJWK(resolvedJwk, header.getRawAlgorithm(), keycloakSession);
Comment thread services/src/main/resources/META-INF/services/org.keycloak.crypto.SignatureProviderFactory
Comment on lines +14 to +16
org.keycloak.crypto.MLDSA44SignatureProviderFactory
org.keycloak.crypto.MLDSA65SignatureProviderFactory
org.keycloak.crypto.MLDSA87SignatureProviderFactory
@stianst

stianst commented Nov 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the PR, but I'm going to close this as it is trying to add too many things in one go, completely lacking on functional tests, and does not work at all beyond in unit tests. The way the providers are enabled/disabled basically means they will never be available in the server.

@stianst stianst closed this Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Attestation-Based Client Auth: Implement algorithm enforcement and fix AKP cnf.jwk thumbprint path

3 participants

@adilalperenciftci @stianst