Skip to content

Conversation

jakemas
Copy link
Collaborator

@jakemas jakemas commented Sep 24, 2025

Issues:

N/A - ML-KEM support.

Description of changes:

mlkem-native provides pk and sk check functions crypto_kem_check_pk and crypto_kem_check_sk that are namespaced to ml_kem_{512/768/1024}_check_{pk/sk}. This PR hooks them up to EVP_PKEY_check and EVP_PKEY_public_check so we can call them on PKEY of typeKEM.

Call-outs:

Due to the way the MLKEM multi-level build is compiled, we can't call mlkem{512/768/1024}_check_sk directly, so have to have those little wrapper functions in crypto/fipsmodule/ml_kem/ml_kem.c, as with other functions in that file.

Testing:

KEM tests are modified to:

EXPECT_TRUE(EVP_PKEY_check(kem_key_ctx.get()));
EXPECT_TRUE(EVP_PKEY_public_check((kem_key_ctx.get())));

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 59.70149% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.81%. Comparing base (a043dc7) to head (a09e674).

Files with missing lines Patch % Lines
crypto/fipsmodule/kem/kem.c 46.15% 21 Missing ⚠️
crypto/fipsmodule/ml_kem/ml_kem.c 75.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2709      +/-   ##
==========================================
- Coverage   78.82%   78.81%   -0.02%     
==========================================
  Files         667      667              
  Lines      114077   114143      +66     
  Branches    16045    16052       +7     
==========================================
+ Hits        89925    89960      +35     
- Misses      23378    23409      +31     
  Partials      774      774              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +322 to +326
// Check that at least the public key exists
if (key->public_key == NULL) {
OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET);
return 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

NP: It's currently possible (although I wish it weren't) for the secret_key to be set but not the public_key. For those, this would always return an error.

Comment on lines +333 to +342
if (ml_kem_512_check_pk(key->public_key) != 0) {
OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR);
return 0;
}
// Check secret key validity if present
if (key->secret_key != NULL && ml_kem_512_check_sk(key->secret_key) != 0) {
OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR);
return 0;
}
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

We would also need a pair-consistency check, otherwise the secret and public keys might be unrelated. (?)

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

Successfully merging this pull request may close these issues.

3 participants