Add EVP_PKEY_check and EVP_PKEY_public_check for KEMs by jakemas · Pull Request #2709 · aws/aws-lc

jakemas · 2025-09-24T06:51:19Z

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.

This PR also implements a Pairwise Consistency Test (PCT) between the provided public and secret key -- this means the the functionKEM_check_key enforces pub key is present when secret key is present, this is intended behaviour -- we need both to be able to check consistency, otherwise the function serves little functionality.

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())));

Also added test suite KEMCheckKeyNegativeTests for testing KEM_check_key via. EVP_PKEY_check and EVP_PKEY_public_check. These checks include:

Valid Key Baseline - Verifies both EVP_PKEY_check and EVP_PKEY_public_check pass on a properly generated KEM keypair.
Corrupted Public Key - Tests that validation fails when public key bytes are corrupted while secret key remains valid.
Corrupted Secret Key - Tests that validation fails when secret key hash is corrupted while public key remains valid.
Mismatched Keypair - Tests Pairwise Consistency Test (PCT) failure detection when public and secret keys are from different keypairs.
Public Key Only Valid - Tests that validation passes when only public key material is provided.
Public Key Only Corrupted - Tests that validation fails when only corrupted public key material is provided.
Secret Key Only - Tests that validation fails when only secret key is provided without required public key.

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 · 2025-09-24T07:35:24Z

Codecov Report

❌ Patch coverage is 83.91960% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.96%. Comparing base (0f99f4e) to head (416f603).
⚠️ Report is 243 commits behind head on main.

Files with missing lines	Patch %	Lines
crypto/fipsmodule/ml_kem/ml_kem.c	79.41%	21 Missing ⚠️
crypto/fipsmodule/kem/kem.c	83.33%	9 Missing ⚠️
crypto/evp_extra/evp_extra_test.cc	94.87%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2709      +/-   ##
==========================================
+ Coverage   78.95%   78.96%   +0.01%     
==========================================
  Files         671      671              
  Lines      114801   114998     +197     
  Branches    16153    16179      +26     
==========================================
+ Hits        90643    90811     +168     
- Misses      23372    23399      +27     
- Partials      786      788       +2

☔ 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.

justsmth · 2025-09-24T13:04:51Z

crypto/fipsmodule/kem/kem.c

+  // Check that at least the public key exists
+  if (key->public_key == NULL) {
+    OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET);
+    return 0;
+  }


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.

Currently intended behaviour, my idea is to merge the PRs first that ensure we can populate both. Then we fail this check if pub_key isn't set when secrect_key is? How does that sound?

Once caveat is that EVP_PKEY_public_check also calls the same function e.g. EC_KEY_check_key that just handles it. See here. Little bit strange, but if we follow the pattern, we'd have to make KEM_CHECK ok with pub only. Or we could implement two functions, the second KEM_CHECK_PUBLIC_KEY and call the it from EVP_PKEY_public_check: drafted that idea out here 0616873, but I don't think I like the anti-pattern. Looking at EC check for inspo, I will implement similar to how they do it, If only public key: validates public key only, if secret key is present: requires public key and validates public key, secret key, and PCT.

I've landed on f30bfd4 that preserves functionality with other EVP_PKEY_public_check/EVP_PKEY_check functions. As documented, it

// KEM_check_key: validates the key based on available key material // - If only public key: validates public key only // - If secret key is present: requires public key and validates public key, secret key, and PCT

I added negative testing for a bunch of cases, including the one you mention:

// ---- 7. Test with secret key only (no public key) ---- // Create EVP_PKEY with only secret key (no public key)

crypto/fipsmodule/kem/kem.c

jakemas · 2025-10-07T19:05:51Z

Upstream work discussed with Hanno on exposing the check_{sk/pk} functions pq-code-package/mlkem-native#1216. We may perhaps need to do an import/update of mlkem-native before this can merge.

dkostic · 2025-10-10T20:59:19Z

crypto/evp_extra/evp_extra_test.cc

+  corrupted_pk[1] = 0xFF;
+  if (pk_len > 2) {
+    corrupted_pk[2] = 0xFF;
+  }


isn't it enough to corrupt a single bit? why do we change two more bytes?

dkostic · 2025-10-10T20:59:29Z

crypto/evp_extra/evp_extra_test.cc

+
+  // Create a corrupted public key
+  std::vector<uint8_t> corrupted_pk = pk_copy;
+  corrupted_pk[0] = 0xFF;


this will produce a flaky test because with some probability the valid keys will have the bytes equal to 0xFF :)
to be sure that you are corrupting the keys you can just XOR with 1 (^ 1), that always changes the least significant bit.

dkostic · 2025-10-10T21:03:21Z

crypto/evp_extra/evp_extra_test.cc

+  // Create a corrupted secret key
+  std::vector<uint8_t> corrupted_sk = sk_copy;
+
+  // Corrupt the last 64 bytes of secret key (in ML-KEM this is the hash)


shouldn't corrupting any single bit in the secret key be sufficient?

dkostic · 2025-10-10T21:05:34Z

crypto/evp_extra/evp_extra_test.cc

+  // Public key check will fail PCT since secret key is present, and corrupted
+  EXPECT_FALSE(EVP_PKEY_public_check(corrupted_sk_ctx.get()));
+
+  // ---- 4. Test mismatched keypair (PCT failure) ----


is there a differentiation between mismatched keypair and corrupted key pare cases above? is a mismatched key pair going to fail differently than a corrupted key pair?

dkostic · 2025-10-10T21:08:46Z

crypto/fipsmodule/kem/kem.c

+  // Call appropriate ML-KEM check functions based on KEM NID
+  switch (key->kem->nid) {
+    case NID_MLKEM512:
+    case NID_KYBER512_R3:


I don't think we should mix-up Kyber with ML-KEM, technically they are not the same algorithms. I'd drop Kyber NIDs from here.

dkostic · 2025-10-10T21:09:39Z

crypto/fipsmodule/ml_kem/ml_kem.c

+// These functions perform FIPS 203 compliant PCT by performing encapsulation
+// and decapsulation operations and comparing the resulting shared secrets
+
+int ml_kem_512_check_pct(const uint8_t *public_key, const uint8_t *secret_key) {


mlkem-native doesn't have pct already implemented?

sgmenda · 2026-03-06T18:12:36Z

@jakemas is this ready for review or still a draft?

jakemas added 2 commits September 23, 2025 23:50

Add EVP_PKEY_check and EVP_PKEY_public_check for KEMs

1d332de

remove extern and fix up documentation

a09e674

justsmth reviewed Sep 24, 2025

View reviewed changes

crypto/fipsmodule/kem/kem.c Show resolved Hide resolved

jakemas added 3 commits October 2, 2025 15:31

Merge branch 'main' into mlkem-evp-pkey-check

9f59888

Merge branch 'main' into mlkem-evp-pkey-check

1fa9fe8

add PCT tests to KEM check

d24a116

dkostic self-requested a review October 6, 2025 22:46

justsmth requested a review from hanno-becker October 7, 2025 13:51

jakemas marked this pull request as ready for review October 7, 2025 19:09

jakemas requested a review from a team as a code owner October 7, 2025 19:09

jakemas marked this pull request as draft October 7, 2025 19:10

jakemas added 4 commits October 7, 2025 12:59

drafted public key check function

0616873

cleaner solution and more negative testing

f30bfd4

doc nit

5b01330

Merge branch 'main' into mlkem-evp-pkey-check

c419507

jakemas marked this pull request as ready for review October 7, 2025 21:05

remove unnecessary tests

416f603

dkostic reviewed Oct 10, 2025

View reviewed changes

Conversation

jakemas commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues:

Description of changes:

Call-outs:

Testing:

Uh oh!

codecov-commenter commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakemas Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakemas Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakemas commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

sgmenda commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jakemas commented Sep 24, 2025 •

edited

Loading

codecov-commenter commented Sep 24, 2025 •

edited

Loading

jakemas Oct 7, 2025 •

edited

Loading

jakemas Oct 7, 2025 •

edited

Loading

jakemas commented Oct 7, 2025 •

edited

Loading