Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ml-kem: seed support for DecapsulationKey #53

Open
tarcieri opened this issue Aug 21, 2024 · 15 comments
Open

ml-kem: seed support for DecapsulationKey #53

tarcieri opened this issue Aug 21, 2024 · 15 comments
Assignees

Comments

@tarcieri
Copy link
Member

Seeds provide a shorter secret which is always valid as opposed to having to be validated.

Some have suggested seeds should be the only API for instantiating an ML-KEM decapsulator: https://words.filippo.io/dispatches/ml-kem-seeds/

@tarcieri
Copy link
Member Author

As a data point for comparison, the libcrux-ml-kem API is feature-gating the unpacked API: cryspen/libcrux#522

@str4d
Copy link

str4d commented Aug 22, 2024

The LAMPS WG at IETF seems to be heading towards seeds-only. See https://mailarchive.ietf.org/arch/msg/spasm/OxnYtr1mIzB3GejYswduSfkEIA4/ and earlier emails in the thread (later emails go off topic into RSA-land).

@tarcieri
Copy link
Member Author

BoringSSL has moved to using seeds and only seeds: https://boringssl-review.googlesource.com/c/boringssl/+/70407

@supinie
Copy link
Contributor

supinie commented Sep 6, 2024

Is anyone actively working on this at the moment (@bifurcation)? If not, I would be happy to do so.

@bifurcation
Copy link
Contributor

I have not picked this up. @supinie if you want to take a stab at a PR, I would be happy to review.

@tarcieri
Copy link
Member Author

tarcieri commented Sep 6, 2024

One problem is I haven't managed to find any test vectors, though a few people have claimed they are interested in working on them soon.

I've also been curious if a KDF could be leveraged to provide shorter, more secure seeds: https://groups.google.com/a/list.nist.gov/g/pqc-forum/c/1r6FnG0coiM/m/I9_Jn5lJDQAJ

@bifurcation
Copy link
Contributor

bifurcation commented Sep 6, 2024

Actually, the NIST key generation test vectors are already framed in terms of (d, z) seeds.

So this PR might be as simple as making DecapsulationKey::generate_deterministic public and not feature-conditioned. Or maybe having a public wrapper that has a 64-byte input and splits it into the two 32-byte values.

@supinie
Copy link
Contributor

supinie commented Sep 6, 2024

Or maybe having a public wrapper that has a 64-byte input and splits it into the two 32-byte values.

This is what I had in mind. Have we decided if this will be replacing the current API or be additional? Alternatively, we could have a feature flag to toggle between whether the public API will accept seeds or keys?

@tarcieri
Copy link
Member Author

tarcieri commented Sep 6, 2024

@supinie it should absolutely replace the existing API.

I guess the remaining question is the specific seed format, although multiple could be supported with the specific one inferred from length.

@bifurcation
Copy link
Contributor

I was going to argue the opposite direction :) That since FIPS 203 defines both formats, we should support both.

As far as seed format, the (d, z) approach we have now actually seems right to me, (a) because it is in tune with what FIPS 203 says [1], and (b) because it seems like any format should be parseable to obtain those values.

[1] Page 17, "The seed $(d, z)$... can be stored for later expansion"

@tarcieri
Copy link
Member Author

tarcieri commented Sep 6, 2024

I think it would be OK to keep the existing API under a feature-gate (possibly hazmat) but it permits misuses which aren't possible with the seed-based API. See the BoringSSL example.

@bifurcation
Copy link
Contributor

If we added the required validation checks and had from_bytes() return Result<Self>, that would be safe; doesn't seem like it would merit the hazmat negging. (Cf. this comment) But I admit that I'm more in the "APIs should offer complete capabilities" camp than the "APIs should be very opinionated" camp.

Maybe a compromise could be: Repurpose to_bytes() / from_bytes() to go to/from the seed, and add to_expanded_bytes() / from_expanded_bytes() for the full form. With the idea that the _bytes variants will be more obvious/attractive to developers.

As an aside: The BoringSSL example reminds me that if we're going to have from_seed(), we should probably also have to_seed(). Which means we'll need to carry around d in kem::DecapsulationKey.

@tarcieri
Copy link
Member Author

This IETF thread asked the question of whether both expanded private keys and seeds should be accepted or just seeds, and there was unanimous agreement that only seeds should be supported:

https://mailarchive.ietf.org/arch/msg/spasm/vaAKtzW3BVHN8cm_JbwIaX-ypms/

If we do support the expanded private key format, we may be one of the only implementations that does so.

@bifurcation
Copy link
Contributor

Good data. How about we keep it, behind a feature flag? Seems like we can deprecate/remove it later if it's really not useful.

@tarcieri
Copy link
Member Author

Feature flagging it seems ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants