ML-KEM768 by baentsch · Pull Request #25848 · openssl/openssl

baentsch · 2024-11-01T09:07:11Z

First cut integrating ML-KEM768 based on the boringssl implementation.

Lots of work required to complete this -- will add issues for those so that more people can contribute in case of interest.

For now, all this code does is make ML-KEM768 available for simple initial testing, e.g., openssl s_client -brief -groups MLKEM-768 -connect test.openquantumsafe.org:6020.

Thanks to @andrewkdinh @t8m @mattcaswell @paulidale @slontis for help and guidance preparing this PR.

mattcaswell · 2024-11-01T09:11:05Z

Not looked at this yet, but we should acknowledge the original source of the code in the first commit message.

baentsch · 2024-11-01T09:22:38Z

Not looked at this yet, but we should acknowledge the original source of the code in the first commit message.

Lots of references -- most referencing only generally to "bssl", though. Any suggestion as to how to make it explicit welcome!

mattcaswell · 2024-11-01T09:25:58Z

Something simple like this would do it:

Based on code from BoringSSL covered under Google CLA (insert link to original source code here)

beldmit · 2024-11-01T10:11:27Z

providers/common/capabilities.c

    int maxtls;              /* Maximum TLS version (or 0 for undefined) */
    int mindtls;             /* Minimum DTLS version, -1 unsupported */
    int maxdtls;             /* Maximum DTLS version (or 0 for undefined) */
+    int is_kem;              /* Indicates utility as KEM */


Should it be a bit flag?

that's IMO premature optimization.

include/internal/tlsgroups.h

baentsch · 2024-11-02T06:43:10Z

Something simple like this would do it:

Based on code from BoringSSL covered under Google CLA (insert link to original source code here)

See latest change to first commit message: OK?

test/mlkem_internal_test.c

include/crypto/mlkem.h

t8m · 2024-11-04T07:54:26Z

Something simple like this would do it:
Based on code from BoringSSL covered under Google CLA (insert link to original source code here)

See latest change to first commit message: OK?

I would put the link into the commit message body. Also to keep the formatting of the commit message titles aligned with most of the other commits in the repository, you should use uppercase for the beginning of the title message - i.e., Initial transfer of boringssl mlkem code under CCLA. Then in the body you can add the link, preferably with the git commit ID that was used for the initial transfer so we know when the code was exactly forked.

mattcaswell · 2024-11-04T11:14:21Z

crypto/bsslmlkem/mlkem768.c

This directory should just be called mlkem. The bssl reference is no longer relevant.

So changed in 467e70f

mattcaswell · 2024-11-04T11:16:32Z

crypto/bsslmlkem/mlkem768.c

+static const int kDU1024 = 11;
+static const int kDV1024 = 5;
+
+static size_t compressed_vector_size(int rank)


Perhaps this and the other *_size functions should be ossl_inline?

So changed in 467e70f

mattcaswell · 2024-11-04T11:20:38Z

crypto/bsslmlkem/mlkem768.c

+        shake128_cache = EVP_MD_fetch(NULL, "SHAKE128", NULL);
+        shake256_cache = EVP_MD_fetch(NULL, "SHAKE256", NULL);
+        sha3_256_cache = EVP_MD_fetch(NULL, "SHA3-256", NULL);
+        sha3_512_cache = EVP_MD_fetch(NULL, "SHA3-512", NULL);


These global caches become problematic in the case where the provider that supplied them gets unloaded.

This problem is gone with baentsch#2 merged.

Please check the resolution, @mattcaswell -- ascertained by your test code from #25403

mattcaswell · 2024-11-04T11:25:13Z

crypto/bsslmlkem/mlkem768.c

+
+    if (msg)
+        printf("%s: \n", msg);
+    for (i = 0; i < len; i++) {


This is basically reimplementing BIO_dump_fp

Thanks for the pointer! So changed in 467e70f

mattcaswell · 2024-11-04T11:46:39Z

crypto/bsslmlkem/mlkem768.c

+{
+    uint8_t seed[MLKEM_SEED_BYTES];
+
+    RAND_bytes(seed, sizeof(seed));


Probably this should be RAND_priv_bytes_ex...and this function can fail.

Very good catch!

So changed in 467e70f

mattcaswell · 2024-11-04T14:39:47Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+    { OSSL_FUNC_KEYMGMT_GEN_CLEANUP, (void (*)(void))mlkem_gen_cleanup },
+    { OSSL_FUNC_KEYMGMT_DUP, (void (*)(void))mlkem_dup },
+    /*
+     * don't do for now, see https://github.com/openssl/private/issues/698


Add TODO(ML-KEM)

mattcaswell · 2024-11-04T14:40:29Z

test/mlkem_internal_test.c

+
+int main(void)
+{
+#ifndef OPENSSL_NO_MLKEM


Better to just not build this test at all in the case of no-mlkem, i.e. detect this in the build.info file.

imo there's many other tests not carrying such enable check (e.g., everything EC). But whatever, let me try to find out how to do this....

mattcaswell · 2024-11-04T14:41:28Z

test/mlkem_internal_test.c

+#include "testutil.h"
+#include "testutil/output.h"
+
+int main(void)


this should be implemented as a standard test, i.e. using the test framework with a setup_tests() function etc

AFAIK @andrewkdinh already has that in the works, so leaving as-is for now.

mattcaswell · 2024-11-04T14:42:08Z

test/mlkem_internal_test.c

+        return 6;
+#endif
+    return 0;
+}


This needs a test recipe in test/recipes to run this test

As per above -- rely on @andrewkdinh's change on this one.

mattcaswell · 2024-11-04T14:43:05Z

test/mlkem_internal_test.c

+    /* public key component to be created from private key */
+    ossl_mlkem768_public_from_private(&public_key, &private_key);
+    /* try to re-create public key structure from encoded public key */
+    ossl_mlkem768_recreate_public_key(out_encoded_public_key, &recreated_public_key);


This function can fail so we should check the return code

Thanks for the catch. So changed in 7b7cdfb

mattcaswell · 2024-11-05T10:48:57Z

crypto/mlkem/mlkem768.c

+} matrix;
+
+typedef struct public_key_RANK768
+{


nit: Put the { on the end of the previous line

so changed.

mattcaswell · 2024-11-05T10:49:13Z

crypto/mlkem/mlkem768.c

+} public_key_RANK768;
+
+typedef struct private_key_RANK768
+{


nit: Placement of { as above

so changed.

mattcaswell · 2024-11-05T11:25:04Z

crypto/mlkem/mlkem768.c

+        return 0;
+
+    /* TODO(ML-KEM): Review requested randomness strength */
+    if (RAND_bytes_ex(mlkem_ctx->libctx, seed, sizeof(seed), 256) == 1) {


RAND_priv_bytes_ex?

so changed in 7b7cdfb

mattcaswell · 2024-11-05T11:36:59Z

providers/common/capabilities.c

    TLS_GROUP_ENTRY("ffdhe8192", "ffdhe8192", "DH", 37),
 # endif
+    /* TODO(ML-KEM): Decide final name, e.g., ML-KEM768 or MLKEM768 */
+    TLS_GROUP_ENTRY("MLKEM-768", "MLKEM-768", "MLKEM-768", 38),


Hmm. The alg name now matches - but the group name still differs from the IANA name.

mattcaswell · 2024-11-05T11:45:11Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+        /*
+         * ideally, this is a one-time allocation and ctx that should be within the
+         * provider context: OK to move it there to improve performance?? It would be
+         * the first algorithmspecific context stored: Feels weird (TODO(ML-KEM)).


Moving it there has implications. It brings back some problems which were present previously but are now no longer present. E.g. if providers get loaded/unloaded after the default provider then the cache might reference the wrong instances. Also if the global property query setting gets changed.

It might be a good idea to assess the performance impact of this, so we can understand whether it is worth it or not.

Captured in #25879, so consider this dealt with for now.

mattcaswell · 2024-11-05T11:49:34Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+            return 0;
+        if (privkeylen != MLKEM768_SECRETKEYBYTES) {
+            printf("sec key len mismatch in import: %ld vs %d: HOWCAN?\n",
+                   privkeylen, MLKEM768_SECRETKEYBYTES);


debug_print?

so changed in 7b7cdfb

mattcaswell · 2024-11-05T11:49:47Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+
+    if ((param_pub_key != NULL && pubkeylen != ossl_mlkem768_PUBLIC_KEY_BYTES)) {
+        printf("sec key len mismatch in import: %ld vs %d: HOWCAN?\n",
+               pubkeylen, ossl_mlkem768_PUBLIC_KEY_BYTES);


debug_print?

so changed in 7b7cdfb

baentsch · 2024-11-05T16:05:08Z

@mattcaswell I think I addressed all your comments with the latest commit. Thus, I'd like to consider at least the API "settled" for now, will not touch it unnecessarily and thus invite @andrewkdinh to use/improve on it as discussed.

tomato42 · 2024-11-05T17:08:21Z

I may be missing something, but there's no support for writing the private and public keys to a file included?
if so, may I ask for a follow up issue for it?

baentsch · 2024-11-05T17:39:13Z

may I ask for a follow up issue for it?

There's lots of issues waiting to be published. I waited raising all "further improvement" issues so the community can lend hands with them until this code is ready to go onto the feature branch as the basis for other contributions -- which seems to be getting close now.

slontis · 2024-11-06T20:40:16Z

Not sure if this is useful to you or not, but here is a python3 file I use to convert hex strings into byte arrays

e.g.

python3 str2hex.py -columns 8 -b BEEFCAFE

import argparse
from hashlib import sha256

parser = argparse.ArgumentParser(description="")
parser.add_argument('-b', type=str)
parser.add_argument("-columns", type=int, default=8)
parser.add_argument("-spacing", type=int, default=1)
parser.add_argument("-indent", type=int, default=4)
parser.add_argument("-sha256", action="store_true")
args = parser.parse_args()

hx = bytes.fromhex(args.b);
if args.sha256:
    hx = sha256(hx).digest();

print("static const unsigned char bytes[] = {", end='')
i = 0
for b in hx:
    if i % args.columns == 0:
        print("\n{indent}".format(indent=' ' * args.indent), end='')
    else:
        print(' ' * args.spacing, end='')
    print('0x{:02x},'.format(b), end='')
    i += 1
print("\n};")

t8m · 2024-11-08T07:56:24Z

crypto/mlkem/mlkem768.c

+    nctx->sha3_512_cache = EVP_MD_fetch(libctx, "SHA3-512", properties);
+    nctx->libctx = libctx;
+    if (properties != NULL)
+        nctx->properties = OPENSSL_strdup(properties);


Suggested change

nctx->properties = OPENSSL_strdup(properties);

if ((nctx->properties = OPENSSL_strdup(properties)) == NULL)

goto err;

t8m · 2024-11-08T07:57:27Z

crypto/mlkem/mlkem768.c

+        if (ctx->shake128_cache != NULL)
+            EVP_MD_free(ctx->shake128_cache);
+        if (ctx->shake256_cache != NULL)
+            EVP_MD_free(ctx->shake256_cache);
+        if (ctx->sha3_256_cache != NULL)
+            EVP_MD_free(ctx->sha3_256_cache);
+        if (ctx->sha3_512_cache != NULL)
+            EVP_MD_free(ctx->sha3_512_cache);


Please remove all the checks for !=NULL EVP_MDs. All the free functions handle NULL parameter gracefully.

t8m · 2024-11-08T07:58:28Z

crypto/mlkem/mlkem768.c

+static void prf(uint8_t *out, size_t out_len, const uint8_t in[33],
+                ossl_mlkem_ctx *mlkem_ctx)
+{
+    single_keccak(out, out_len, in, 33, mlkem_ctx->shake256_cache);


The return value must be checked and returned.

t8m · 2024-11-08T07:58:36Z

crypto/mlkem/mlkem768.c

+static void hash_h(uint8_t *out, const uint8_t *in, size_t len,
+                   ossl_mlkem_ctx *mlkem_ctx)
+{
+    single_keccak(out, 32, in, len, mlkem_ctx->sha3_256_cache);


The return value must be checked and returned.

t8m · 2024-11-08T07:58:41Z

crypto/mlkem/mlkem768.c

+static void hash_g(uint8_t *out, const uint8_t *in, size_t len,
+                   ossl_mlkem_ctx *mlkem_ctx)
+{
+    single_keccak(out, 64, in, len, mlkem_ctx->sha3_512_cache);


The return value must be checked and returned.

t8m · 2024-11-08T08:11:23Z

providers/common/capabilities.c

    int maxtls;              /* Maximum TLS version (or 0 for undefined) */
    int mindtls;             /* Minimum DTLS version, -1 unsupported */
    int maxdtls;             /* Maximum DTLS version (or 0 for undefined) */
+    int is_kem;              /* Indicates utility as KEM */


that's IMO premature optimization.

providers/implementations/keymgmt/mlkem_kmgmt.c

t8m · 2024-11-08T08:16:49Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+                                                &len_stored))
+            return 0;
+        debug_print("encoded pub key successfully stored with %ld bytes\n", len_stored);
+        ossl_mlkem768_recreate_public_key(mkey->encoded_pubkey, &mkey->pubkey,


Return value must be checked here

t8m · 2024-11-08T08:17:34Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+    ossl_mlkem768_generate_key(mkey->encoded_pubkey, NULL, &mkey->seckey,
+                               mkey->mlkem_ctx);
+    mkey->seckey_initialized = 1;
+    ossl_mlkem768_public_from_private(&mkey->pubkey, &mkey->seckey);


Return values must be checked here.

t8m · 2024-11-08T08:21:40Z

providers/implementations/keymgmt/mlkem_kmgmt.c

+    dstkey->keytype = srckey->keytype;
+    if (srckey->pubkey_initialized == 1
+            && (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) {
+        /* TODO(ML-KEM)/TBC: By commenting out these, the EVP level test still passes: WHY?? */


Perhaps the pubkey value from the duplicated key is not really used?

As well as the private key value. It indeed does not seem like the duplicated key is operated at all. Why is it then created in the first place?

The duplication is done during the EVP_PKEY_copy_parameters() call. That call does not set OSSL_KEYMGMT_SELECT_PUBLIC_KEY. So the current test does not exercise this code at all.

OK -- that explains it then, thanks. Will remove this comment then when squashing all into one commit after the last PR review comment is resolved.

baentsch · 2024-11-09T17:27:33Z

@t8m Thanks for the thorough check on retval functions. Is there no automated check that could find such problems (./config -pedantic didn't help)? Anyway, now added return value checks in all places I could find.

t8m

Just nits to fix and I'll approve

t8m · 2024-11-11T07:44:59Z

crypto/mlkem/mlkem768.c

-    hash_h(priv->pub.public_key_hash, out_encoded_public_key,
-           encoded_public_key_size(RANK768), mlkem_ctx);
+    if (!mlkem_marshal_public_key(out_encoded_public_key, &priv->pub)
+        || (!hash_h(priv->pub.public_key_hash, out_encoded_public_key,


Nit: please remove the extraneous parentheses.

t8m · 2024-11-11T07:45:31Z

crypto/mlkem/mlkem768.c

-    hash_g(key_and_randomness, input, sizeof(input), mlkem_ctx);
-    encrypt_cpa(out_ciphertext, pub, entropy, key_and_randomness + 32, mlkem_ctx);
+    if (!hash_g(key_and_randomness, input, sizeof(input), mlkem_ctx)
+        || (!encrypt_cpa(out_ciphertext, pub, entropy, key_and_randomness + 32, mlkem_ctx)))


Nit: please remove the extraneous parentheses

t8m · 2024-11-11T07:46:13Z

crypto/mlkem/mlkem768.c


    /* TODO(ML-KEM): Review requested randomness strength */
-    if (RAND_bytes_ex(mlkem_ctx->libctx, entropy, MLKEM_ENCAP_ENTROPY, 256) != 1)
+    if ((RAND_bytes_ex(mlkem_ctx->libctx, entropy, MLKEM_ENCAP_ENTROPY, 256) != 1)


Nit: please remove the extraneous parentheses

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem VSCode automatic formatting ([email protected]) Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) Move variable init to beginning of each function ([email protected]) replace CBB API fixing up constants and parameter lists replace BORINGSSL_keccac calls with EVP calls added library symbols and low-level test case switch boringssl constant time routines for OpenSSL ones data type assertion and negative test added moved mlkem.h to include/crypto changed function naming to be in line with ossl convention remove Google license terms based on CCLA add constant_time_lt_32 convert asserts to ossl_asserts where possible add bssl keccak, pubK recreation, formatting add provider interface to utilize mlkem768 code enabling TLS1.3 use revert to OpenSSL DigestXOF use EVP_MD_xof() to determine digest finalisation ([email protected]) change APIs to return error codes; reference new IANA number; move static asserts to one place remove boringssl keccak for good fix coding style and return value checks ANSI C compatibility changes remove static cache objects all internal retval functions used leading to some new retval functions

baentsch · 2024-11-13T06:58:10Z

Thanks for review and approvals, @t8m @mattcaswell ! Anything else missing to / Who can now merge this into the feature branch?

mattcaswell · 2024-11-13T09:23:29Z

Just waiting on the 24 hour timer which expires very soon.

openssl-machine · 2024-11-13T10:00:33Z

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

mattcaswell · 2024-11-13T10:46:14Z

Pushed to the feature/ml-kem branch!!

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem VSCode automatic formatting ([email protected]) Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) Move variable init to beginning of each function ([email protected]) replace CBB API fixing up constants and parameter lists replace BORINGSSL_keccac calls with EVP calls added library symbols and low-level test case switch boringssl constant time routines for OpenSSL ones data type assertion and negative test added moved mlkem.h to include/crypto changed function naming to be in line with ossl convention remove Google license terms based on CCLA add constant_time_lt_32 convert asserts to ossl_asserts where possible add bssl keccak, pubK recreation, formatting add provider interface to utilize mlkem768 code enabling TLS1.3 use revert to OpenSSL DigestXOF use EVP_MD_xof() to determine digest finalisation ([email protected]) change APIs to return error codes; reference new IANA number; move static asserts to one place remove boringssl keccak for good fix coding style and return value checks ANSI C compatibility changes remove static cache objects all internal retval functions used leading to some new retval functions Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #25848)

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem VSCode automatic formatting ([email protected]) Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) Move variable init to beginning of each function ([email protected]) replace CBB API fixing up constants and parameter lists replace BORINGSSL_keccac calls with EVP calls added library symbols and low-level test case switch boringssl constant time routines for OpenSSL ones data type assertion and negative test added moved mlkem.h to include/crypto changed function naming to be in line with ossl convention remove Google license terms based on CCLA add constant_time_lt_32 convert asserts to ossl_asserts where possible add bssl keccak, pubK recreation, formatting add provider interface to utilize mlkem768 code enabling TLS1.3 use revert to OpenSSL DigestXOF use EVP_MD_xof() to determine digest finalisation ([email protected]) change APIs to return error codes; reference new IANA number; move static asserts to one place remove boringssl keccak for good fix coding style and return value checks ANSI C compatibility changes remove static cache objects all internal retval functions used leading to some new retval functions Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#25848)

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem VSCode automatic formatting ([email protected]) Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) Move variable init to beginning of each function ([email protected]) replace CBB API fixing up constants and parameter lists replace BORINGSSL_keccac calls with EVP calls added library symbols and low-level test case switch boringssl constant time routines for OpenSSL ones data type assertion and negative test added moved mlkem.h to include/crypto changed function naming to be in line with ossl convention remove Google license terms based on CCLA add constant_time_lt_32 convert asserts to ossl_asserts where possible add bssl keccak, pubK recreation, formatting add provider interface to utilize mlkem768 code enabling TLS1.3 use revert to OpenSSL DigestXOF use EVP_MD_xof() to determine digest finalisation ([email protected]) change APIs to return error codes; reference new IANA number; move static asserts to one place remove boringssl keccak for good fix coding style and return value checks ANSI C compatibility changes remove static cache objects all internal retval functions used leading to some new retval functions Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #25848)

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem VSCode automatic formatting ([email protected]) Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) Move variable init to beginning of each function ([email protected]) replace CBB API fixing up constants and parameter lists replace BORINGSSL_keccac calls with EVP calls added library symbols and low-level test case switch boringssl constant time routines for OpenSSL ones data type assertion and negative test added moved mlkem.h to include/crypto changed function naming to be in line with ossl convention remove Google license terms based on CCLA add constant_time_lt_32 convert asserts to ossl_asserts where possible add bssl keccak, pubK recreation, formatting add provider interface to utilize mlkem768 code enabling TLS1.3 use revert to OpenSSL DigestXOF use EVP_MD_xof() to determine digest finalisation ([email protected]) change APIs to return error codes; reference new IANA number; move static asserts to one place remove boringssl keccak for good fix coding style and return value checks ANSI C compatibility changes remove static cache objects all internal retval functions used leading to some new retval functions Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#25848)

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem - VSCode automatic formatting ([email protected]) - Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) - Move variable init to beginning of each function ([email protected]) - Replace CBB API - Fixing up constants and parameter lists - Replace BORINGSSL_keccak calls with EVP calls - Added library symbols and low-level test case - Switch boringssl constant time routines for OpenSSL ones - Data type assertion and negative test added - Moved mlkem.h to include/crypto - Changed function naming to be in line with ossl convention - Remove Google license terms based on CCLA - Add constant_time_lt_32 - Convert asserts to ossl_asserts where possible - Add bssl keccak, pubK recreation, formatting - Add provider interface to utilize mlkem768 code enabling TLS1.3 use - Revert to OpenSSL DigestXOF - Use EVP_MD_xof() to determine digest finalisation ([email protected]) - Change APIs to return error codes; reference new IANA number; move static asserts to one place - Remove boringssl keccak for good - Fix coding style and return value checks - ANSI C compatibility changes - Remove static cache objects - All internal retval functions used leading to some new retval functions Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #25848)

Based on code from BoringSSL covered under Google CCLA Original code at https://boringssl.googlesource.com/boringssl/+/HEAD/crypto/mlkem - VSCode automatic formatting ([email protected]) - Just do some basic formatting to make diffs easier to read later: convert from 2 to 4 spaces, add newlines after function declarations, and move function open curly brace to new line ([email protected]) - Move variable init to beginning of each function ([email protected]) - Replace CBB API - Fixing up constants and parameter lists - Replace BORINGSSL_keccak calls with EVP calls - Added library symbols and low-level test case - Switch boringssl constant time routines for OpenSSL ones - Data type assertion and negative test added - Moved mlkem.h to include/crypto - Changed function naming to be in line with ossl convention - Remove Google license terms based on CCLA - Add constant_time_lt_32 - Convert asserts to ossl_asserts where possible - Add bssl keccak, pubK recreation, formatting - Add provider interface to utilize mlkem768 code enabling TLS1.3 use - Revert to OpenSSL DigestXOF - Use EVP_MD_xof() to determine digest finalisation ([email protected]) - Change APIs to return error codes; reference new IANA number; move static asserts to one place - Remove boringssl keccak for good - Fix coding style and return value checks - ANSI C compatibility changes - Remove static cache objects - All internal retval functions used leading to some new retval functions Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#25848)

beldmit reviewed Nov 1, 2024

View reviewed changes

baentsch force-pushed the bsslmlkem768 branch from 308efaf to f83bffa Compare November 2, 2024 06:41

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Nov 2, 2024

andrewkdinh reviewed Nov 2, 2024

View reviewed changes

test/mlkem_internal_test.c Show resolved Hide resolved

andrewkdinh reviewed Nov 2, 2024

View reviewed changes

include/crypto/mlkem.h Show resolved Hide resolved

mattcaswell mentioned this pull request Nov 4, 2024

Support X25519Kyber768Draft00 or X25519Kyber768 #24622

Closed

mattcaswell requested changes Nov 4, 2024

View reviewed changes

baentsch mentioned this pull request Nov 4, 2024

remove static cache objects baentsch/openssl#2

Merged

mattcaswell requested changes Nov 5, 2024

View reviewed changes

baentsch mentioned this pull request Nov 5, 2024

Performance enhancement for ML-KEM #25879

Open

4 tasks

mattcaswell approved these changes Nov 5, 2024

View reviewed changes

t8m added triaged: feature The issue/pr requests/adds a feature tests: deferred Tests will be added in a subsequent PR (label should be removed when the PR with tests is merged) approval: review pending This pull request needs review by a committer labels Nov 5, 2024

This was referenced Nov 7, 2024

Apply code conventions to MLKEM768 code openssl/project#1016

Closed

Change void-returning functions to error-code returning functions openssl/project#1018

Closed

Replace SHA3 global cache openssl/project#1007

Closed

t8m requested changes Nov 8, 2024

View reviewed changes

t8m requested changes Nov 11, 2024

View reviewed changes

mattcaswell added the approval: done This pull request has the required number of approvals label Nov 12, 2024

baentsch mentioned this pull request Nov 12, 2024

Create provider interface for MLKEM-768 openssl/project#1014

Closed

mattcaswell added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Nov 13, 2024

mattcaswell closed this Nov 13, 2024

mkannwischer mentioned this pull request Nov 22, 2024

Adding OpenSSL CLA requirement pq-code-package/tsc#113

Closed

baentsch mentioned this pull request Nov 22, 2024

Enable ML-KEM in FIPS provider #26036

Closed

baentsch mentioned this pull request Dec 9, 2024

Add a SBOM template in CycloneDX format open-quantum-safe/oqs-provider#585

Merged

	nctx->properties = OPENSSL_strdup(properties);
	if ((nctx->properties = OPENSSL_strdup(properties)) == NULL)
	goto err;

Uh oh!

Comments

Conversation

baentsch commented Nov 1, 2024

Uh oh!

mattcaswell commented Nov 1, 2024

Uh oh!

baentsch commented Nov 1, 2024

Uh oh!

mattcaswell commented Nov 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

baentsch commented Nov 2, 2024

Uh oh!

Uh oh!

Uh oh!

t8m commented Nov 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!