Skip to content

Add support for ML-DSA signatures - #535

Open
PhilSchmieder wants to merge 7 commits into
Keats:masterfrom
PhilSchmieder:master
Open

PhilSchmieder wants to merge 7 commits into
Keats:masterfrom
PhilSchmieder:master

Conversation

@PhilSchmieder

Copy link
Copy Markdown

Closes #534

Summary

Adds ML-DSA signature support following FIPS 204 and RFC 9964.

  • Supports ML-DSA-44, ML-DSA-65, and ML-DSA-87.
  • Implements both aws_lc_rs and rust_crypto backends.
  • Adds PKCS#8 private-key and raw/SPKI public-key loading through DER and PEM APIs.
  • Adds RFC 9964 kty: "AKP" JWK import, export, and thumbprints.
  • Validates JWK algorithms and parameter-set-specific public-key lengths.
  • Updates the documented algorithm list.

Implementation Decisions

ML-DSA uses the RFC 9964 wire names (ML-DSA-44, ML-DSA-65, and ML-DSA-87) while retaining regular Rust variants such as Algorithm::MLDSA44.

Signing uses the deterministic variant of ML-DSA with an empty context.

Variable alg exists in both the common JWK parameters and the AKP algorithm parameters as specified in RFC 9964. Serialization to JwkWire flattens them to one parameter which causes all sorts of problems when doing naively. I opted to introduce custom (de-) serialization logic that uses both locations. Serialization rejects conflicting values, and emits one alg member. Deserialization populates the AKP algorithm parameter and the common parameter.

Dependencies Notes

  • aws-lc-rs crate updated from 1.15 to 1.18 because the stable ML-DSA API is only available in 1.18.
  • ml-dsa uses signature 3.x while this crate still directly uses signature 2.2.

Open to discussion on the implementation decisions and how dependencies like the 'signature' crate is handled especially since other issues / PRs mention this too.

@arckoor
arckoor self-requested a review August 17, 2026 10:49

@arckoor arckoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very hasty review only, but looks like the right direction.

  • How were the keys for the tests generated? OpenSSL, or through rust-crypto, or something else entirely? (just curious)
  • The JWK dance with serde I'm not a huge fan of, I would like to find a way to make that unnecessary

Comment thread src/decoding.rs Outdated
Comment thread tests/ml_dsa/mod.rs Outdated
@PhilSchmieder

Copy link
Copy Markdown
Author

Very hasty review only, but looks like the right direction.

* How were the keys for the tests generated? OpenSSL, or through rust-crypto, or something else entirely? (just curious)

* The JWK dance with serde I'm not a huge fan of, I would like to find a way to make that unnecessary

I used OpenSSL to generate the test keys.

I also don't like the serialization stuff. I guess than can be fixed with either changing the "JwkWire" representation or the AKP params.
JwkWire could not be flattened or changed in some other way, but: 1) I don't know what other stuff breaks as a result of that; and 2) That still leaves two spots where the alg is specified and which would potentially be reconciled.
On the other hand, the RFC specifies the alg in the AKP params. Changing that would break the neat encapsulation of the parameters. This would also be annoying when future signature scheme might also rely on the AKP parameters.

I tend to keeping the alg member in the AKP parameters to be RFC compliant and to future proof.

@arckoor

arckoor commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

The wasm_js feature shouldn't be enabled, at least not by us (see https://crates.io/crates/getrandom). Not critical to change / fix right now, I've been meaning to bump some of the deps anyway, which hopefully fixes things then.
As an aside, your commits don't seem to be authored by your github user, you may want to either change that or at least verify the email you're committing with :)

I'll try to do a full review sometime next week.

@arckoor arckoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry this took so long.
I played around with the JwkWire thing for a while, I didn't find a nice way to get rid of it. However I would not duplicate the alg field between them, and instead deser the json into JwkWire, reassemble into Jwk and beforehand check if alg needs to be present

if let AlgorithmParameters::AlgorithmKeyPair(_) = &algorithm {
    common.key_algorithm.ok_or_else(|| de::Error::missing_field("alg"))?;
}

Comment thread src/crypto/aws_lc/ml_dsa.rs Outdated
Comment thread src/crypto/rust_crypto/ml_dsa.rs Outdated
Comment thread src/decoding.rs Outdated
Comment thread src/algorithms.rs
Comment thread src/algorithms.rs
@PhilSchmieder

Copy link
Copy Markdown
Author

Hi, can you please review the changes I made when you get a chance? Thanks!

@arckoor arckoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM from a semi quick glance. The getrandom situation is really unfortunate though, I think we'll update the rust crypto stack as a whole and then this can be merged, see the discussion in #536

Comment thread src/decoding.rs Outdated
@arckoor

arckoor commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

@PhilSchmieder I will give this a look again when it's time to merge it, which won't be at least until the next major and until the rust crypto people release rsa v10 so we can bump that stack. If you want to prepare for that you can remove your getrandom changes from the Cargo.toml so that hopefully when #536 or something similar is merged it'll cause no / fewer conflicts.

If you're very bored you can have a look at the jsonwebtoken-providers repo, I'd take an accompanying PR there, though subject to the same "won't be merged until ..." restrictions, so might be better to tackle that once this is actually merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for ML-DSA signed JWTs

2 participants