-
Notifications
You must be signed in to change notification settings - Fork 480
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
Use seed as private key format for ML-KEM #1994
base: main
Are you sure you want to change the base?
Conversation
I'm tracking this closely, as we are heavily invested in ML-KEM at SoftIron. I'm not really sure what we could do here that would be backwards compatible with existing expanded keys. As a side note, I'm suspecting changes will be needed in oqs-provider as well?
|
@kvanals - The idea of doing it with backward compatibility would be to define a separate scheme using the same upstream library. And yes, there would probably be changes needed in oqs-provider as well, to enable callers to reference the additional scheme. You would have something like |
Completely sensible! Sorry I wasn't tracking that. |
Yes. A full implementation of course would have to use the code generation concepts of "copy_from_upstream" and pass all NIST tests.
The overarching goal should be to create as little new original code (as opposed to generated code) as possible / be orthogonal to the current logic.
In that case, allow me to invite "SoftIron" to contribute to the work in this project. Right now this project's mission is to support research and prototyping (only) as we don't have the resources to be "everything for everyone", e.g., maintain backwards compatibility (or other "product level" qualities). The proposal @bifurcation kindly works on here is the result of the discussion at #1985 with the goal to work with these limitations. Either way, currently, there is absolutely no guarantee that OQS will keep supporting both types of keys: For example, if the upstream ceases to support both modes, so will OQS. If the general consensus is to only use the seed-based approach, support for expanded keys most likely will vanish.
This seems to indicate you want to use this code via OpenSSL. If so, you may want to follow the discussion (or --again-- even better, contribute to the project) at openssl/openssl#25885 where we're using an entirely different code base and aim to only implement the relevant standards for exactly the same reason: Provide the best possible code with the smallest amount of development and maintenance resources possible. |
Cheers @baentsch, happy to upstream anything where applicable. Presently we're using oqs-provider as-is and only maintain one out-of-tree patch to liboqs which I don't believe would meet your copy_from_upstream requirements to build for some of our ARM build targets that are lacking some of the requisite ARM NEON intrinsics. Let me know if there is anything you'd like us to sign or review in this regard.
I absolutely am tracking that. And correct, we do very little work today with liboqs directly, but rather abstract its use behind high level OpenSSL EVP functions. |
Regarding OQS contributions, all there is is (acceptance of) the license (by way of the DCO "signoff"; #1760). If you'd like to see your code/contributions also become used in OpenSSL, see here.
Good approach. Will allow use of the most well-maintained PQC code base -- whichever that is. FWIW, I like the approach of supporting both key formats via OSSL_PARAM. Which one(s) gets an encoder/decoder will probably be a matter of which format ultimately gets an official/standardized OID assigned. If both do, the approach taken in this PR (creating a "twin" alg set that each could get its own OIDs) is probably the most simple to integrate&use. |
@baentsch - Happy to work along these lines, but I'm not seeing documentation of how to do this. I see the |
It looks like @baentsch has been working on getting ML-KEM into OpenSSL from BoringSSL's implementation in https://github.com/openssl/openssl/tree/feature/ml-kem If I'm reading this right, this supports using "OSSL_PKEY_PARAM_MLKEM_SEED" in the manner described above via an OSSL_PARAM. |
This is a community effort, but yes, that's what we're doing there. As always, feedback welcome to the approach taken (documentation proposal up at openssl/openssl#26037). |
@bifurcation FWIW, I ended up basing at least one implementation on the work in this PR as a bit of a one-off. If useful for anyone else, I fixed a few issues with the work done in this PR and applied the logic to all three ML-KEM variants and attached an updated patch to this comment. For my use this works, since the ASN.1 data will have the ML-KEM seed (as private key) and the ML-KEM public key as well, albeit concatenated in the private key since liboqs/oqs-provider do not yet support key derivation from the private key. If/when this becomes more standardized, I can always rewrite the ASN.1 structure to match with the raw data needed and I avoid the issue of losing the seeds when persistent keys needs to exist. I appreciate the initial work on this, but I also realize this isn't anything that can actually be adopted upstream. Attached patch: liboqs-mlkem-coins-as-secret.patch |
This PR is a WIP demonstrating an approach to storing ML-KEM decapsulation keys in the form of a 64-byte seed, as opposed to the expanded form currently. Currently, the proposed scheme is implemented (a) for ML-KEM-512 only, and (b) in-place, as opposed to defining a new scheme. I am posting it in this state to get feedback on whether the structure of the change looks good, because the remainder of the PR will basically just be copy/pasting to cover the other parameter sets and maybe make a new KEM scheme.
If maintainers could answer the following questions, I can implement the rest of the PR:
Fixes #1985
Tests are currently failing because they are providing / looking for the expanded private key. The ACVP test vectors do provide the seed values (
d
andz
), so it should be possible to fix the tests simply by using those values instead of the expanded values (dk
).