Conversation
Add the ML-KEM reference implementation from: https://github.com/pq-crystals/kyber Imported based on commit d5b791c0c6 According to the LICENSE file in the original repo: Public Domain (https://creativecommons.org/share-your-work/public-domain/cc0/) The AUTHORS file lists the following people: Joppe Bos, Léo Ducas, Eike Kiltz, Tancrède Lepoint, Vadim Lyubashevsky, John Schanck, Peter Schwabe, Gregor Seiler, Damien Stehlé
We base the initial implmentation off the ref code and remove stuff we don't need. We also integrate the ref code into the build system. kem.c was renamed to mlkem.c because otherwise the build system complains about a clash with crypto/evp/kem.c. kem.h is renamed to mlkem.h at the same time. We leave the avx2 code alone for now.
We also rename some defines/functions to be more OpenSSL like and suitable
Previously it returned 0. We change it to be more consistent with OpenSSL style.
Generate an ML-KEM pkey, then encapsulate a shared secret, decapsualte it, and check we ended up with the same shared secret.
|
Given the list of "TODOs" above it might be worth considering putting this on a feature branch. |
github-actions
bot
added
the
severity: fips change
|
This is great work @mattcaswell ! To the list of TODOs I would probably add that we should probably also think about how to do the plumbing to support PQ/T Hybrid schemes (e.g., X25519+ML-KEM768 or P256+ML-KEM768). In EU, at least during the transition, hybrid solutions are a requirement. |
|
Regarding CLA, https://github.com/pq-crystals/kyber/blob/main/AUTHORS list the authors. Maybe @cryptojedi can help us collecting the necessary CLA forms? |
|
Nicola Tuveri ***@***.***> wrote:
Regarding CLA, https://github.com/pq-crystals/kyber/blob/main/AUTHORS list the authors.
Maybe @cryptojedi can help us collecting the necessary CLA forms?
Happy to help. What forms exactly do you need?
|
|
The contributors lists is longer BTW: https://github.com/pq-crystals/kyber/graphs/contributors |
paulidale
left a comment
paulidale
left a comment
There was a problem hiding this comment.
Just highlighting a few items that ought to go onto the to do list.
| @@ -0,0 +1,80 @@ | |||
| #include <stddef.h> | |||
There was a problem hiding this comment.
This entire file should be replaced by calls to RAND_bytes or RAND_priv_bytes
| # endif | ||
| { PROV_NAMES_EC, "provider=default", ossl_ec_asym_kem_functions }, | ||
| #endif | ||
| { PROV_NAMES_MLKEM, "provider=default", ossl_mlkem_asym_key_functions }, |
There was a problem hiding this comment.
Similar should be added to the FIPS provider.
There was a problem hiding this comment.
I would leave this out initially, as it also requires self tests before adding
| @@ -0,0 +1,142 @@ | |||
| /* | |||
There was a problem hiding this comment.
Do we really want a new Keccak implementation included?
| @@ -0,0 +1 @@ | |||
| ../ref/fips202.c No newline at end of file | |||
There was a problem hiding this comment.
This file should be deduped and the work done in build.info I suspect.
@cryptojedi - thanks for the offer of help. What we need is CLAs in place for anyone that has made any commits to the "standard" branch of the reference implementation. The CLA forms are here: Once completed they need to be sent to [email protected] (please reference that they are in relation to this PR). If the commits were made on behalf of an employer then both a CCLA and ICLA are required. Otherwise just an ICLA is enough. If sending ICLAs only you will be asked to confirm that the commits were made individually and not on behalf of an employer. Anything you can do to help us collect the necessary paperwork would be greatly appreciated. |
Yes - agreed. I didn't want to look too far ahead at this stage. The initial problem is getting an ML-KEM implementation at all!! |
|
This PR no longer seems relevant. See the feature/ml-kem branch for the current state of ML-KEM development. |
6 participants
This PR add initial support for ML-KEM.
It is based on the Kyber reference implementation from the "standard" branch here (which according the LICENSE file is available as "public domain" software):
https://github.com/pq-crystals/kyber
We will need to figure out whether we do about CLAs for this reference code. Most of the work in this PR is OpenSSL plumbing, so if we decide to incorporate a different implementation of ML-KEM instead much of the work here could be reused. Also the ref code has been incorporated "as is". It will need updating to OpenSSL coding standards.
This is a WIP. So far I have only integrated support for the "ref" code which is a pure C based implementation. There is also some code optimised for AVX2 available, which I have included in this PR but have not yet integrated - so it's not currently used.
The reference code can support ML-KEM-512, ML-KEM-768 and ML-KEM-1024 - but the way the code is structured you can only compile one of those at a time. So for this PR we only support ML-KEM-768. It will need some refactoring to support all 3 variants.
I have included a basic test, but more extensive testing will be required. There are some challenges to this due to the non-deterministic nature of the algorithm (although a deterministic variant is possible).
I have also not included any support for serialization/deserialization, or import/export from the provider.
So work left to do:
With the above caveats and issues in mind, this does provide a somewhat useable ML-KEM implementation.