Adding ML-DSA support #43857
Adding ML-DSA support #43857
Conversation
stianst
left a comment
There was a problem hiding this comment.
Nice work, will try to find time to look at this more in detail next week.
I suspect we need to descope the initial PR a bit though, especially since this will need tests added as well.
Honestly I'd probably start with just supporting one ML-DSA variant, let's say 44, not touch SAML, WebAuthN, etc.
That will reduce the size of the initial PR signficantly.
|
Why do you need BouncyCastlePQCProvider? Java 25 onwards support ML-DSA. |
I thought it would be good to have some backwards compatibility. With BouncyCastle, you can use Java 8 or later. |
stianst
left a comment
There was a problem hiding this comment.
Done a partial review for now; two things we'll need is this needs to be wrapped in a feature and marked as experimental. Second is we need tests.
In general this looks like a very good start.
js/package-lock.json
Outdated
services/src/main/java/org/keycloak/authentication/requiredactions/WebAuthnRegister.java
Show resolved
Hide resolved
services/src/main/java/org/keycloak/keys/AbstractMldsaKeyProviderFactory.java
Show resolved
Hide resolved
|
In terms of testing and reviews it may be better to break this up slightly, so we can start getting things merged. I would probably have broken this down into the following separate PRs:
Let me know what you think about that. |
|
Thanks for the partial review. I’ve now changed Dilithium to ML-DSA, added some tests, and introduced a feature flag that hides ML-DSA in the Token Algorithm dropdown menu (let me know if that’s fine). |
|
|
||
| public JWK akp(Key key, KeyUse keyUse) { | ||
| AKPPublicJWK k = new AKPPublicJWK(); | ||
| byte[] encodedKey = key.getEncoded(); |
There was a problem hiding this comment.
This is not correct, but have no idea what the correct way or how to do it :/
| } | ||
|
|
||
| byte[] decodedKey = Base64Url.decode(pub); | ||
| X509EncodedKeySpec keySpec = new X509EncodedKeySpec(decodedKey); |
There was a problem hiding this comment.
This is not correct, but have no idea what the correct way or how to do it :/
48c94ea to
0305696
Compare
Signed-off-by: Jonas Kawohl <[email protected]>
0305696 to
b6f94bf
Compare
Adding providers for ML-DSA and AKP Public JWK.
Contains Server, Client and Signature Providers; Contains KeyProviders; Contains AKPPublicJWK; Currently works with BouncyCastlePQCProvider and Dilithium, will be changed to BouncyCastleProvider and ML-DSA when unknown error is resolved; Tests will be added; webauthn4j does not support ML-DSA at the moment.
Closes #43684