ml_kem: return an error on catastrophic failure in decap

ML-KEM decapsulation applies implicit rejection by copying the failure
key into the shared secret when the FO re-encryption check fails. This
is correct for a syntactically valid but incorrect ciphertext, and must
stay constant-time and ciphertext-dependent.

However, the same path was also taken when hash_kr() or encrypt_cpa()
failed outright, for example on a memory allocation failure inside
EVP_DigestInit_ex(). In that case decap() copied the failure key and
still returned success, so the caller derived a wrong shared secret with
no error reported. For QUIC this produces a handshake that cannot
converge: the derived keys diverge from the peer, packets fail to
decrypt, and the connection stalls until it times out, with no
diagnostic pointing at the real cause.

These primitive failures are not dependent on the ciphertext, so
reporting them as a hard error does not create a chosen-ciphertext
oracle and does not weaken the constant-time implicit rejection that
happens later via CRYPTO_memcmp() and constant_time_select_8(). Treat
them the same way the existing kdf() failure is already treated, by
raising an error and returning 0.

Also fix the comment, which referred to hash_g() where the code actually
calls hash_kr().

Reviewed-by: Bob Beck <beck@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Mon Jun 15 13:58:32 2026
(Merged from https://github.com/openssl/openssl/pull/31432)

diff --git a/crypto/ml_kem/ml_kem.c b/crypto/ml_kem/ml_kem.c
index e40047e5962b2c40eecc584b5b2c41b8e6489f5f..8fa5db9cc877a9746fdb1a0fe60fe75259e50d72 100644 (file)
--- a/crypto/ml_kem/ml_kem.c
+++ b/crypto/ml_kem/ml_kem.c
@@ -1496,21 +1496,15 @@ static int decap(uint8_t secret[ML_KEM_SHARED_SECRET_BYTES],
     const ML_KEM_VINFO *vinfo = key->vinfo;
     int i;
     uint8_t mask;
+    int ret = 0;
 
     /*
-     * If our KDF is unavailable, fail early! Otherwise, keep going ignoring
-     * any further errors, returning success, and whatever we got for a shared
-     * secret.  The decrypt_cpa() function is just arithmetic on secret data,
-     * so should not be subject to failure that makes its output predictable.
-     *
-     * We guard against "should never happen" catastrophic failure of the
-     * "pure" function |hash_g| by overwriting the shared secret with the
-     * content of the failure key and returning early, if nevertheless hash_g
-     * fails.  This is not constant-time, but a failure of |hash_g| already
-     * implies loss of side-channel resistance.
-     *
-     * The same action is taken, if also |encrypt_cpa| should catastrophically
-     * fail, due to failure of the |PRF| underlying the CBD functions.
+     * The functions called below (kdf, hash_kr, encrypt_cpa) only fail on
+     * catastrophic failure of an underlying SHA3/SHAKE primitive, for example
+     * a memory allocation failure in EVP_DigestInit_ex(). None of these
+     * failures are dependent on the ciphertext content, so reporting them as a
+     * hard error does not create a chosen-ciphertext oracle and does not affect
+     * the constant-time properties of the implicit rejection path below.
      */
     if (!kdf(failure_key, key->z, ctext, vinfo->ctext_bytes, mdctx, key)) {
         ERR_raise_data(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR,
@@ -1521,16 +1515,19 @@ static int decap(uint8_t secret[ML_KEM_SHARED_SECRET_BYTES],
     decrypt_cpa(m, ctext, tmp, key);
     if (!hash_kr(Kr, m, mdctx, key)
         || !encrypt_cpa(tmp_ctext, m, r, tmp, mdctx, key)) {
-        memcpy(secret, failure_key, ML_KEM_SHARED_SECRET_BYTES);
+        ERR_raise_data(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR,
+            "internal error while performing %s decapsulation",
+            vinfo->algorithm_name);
         goto end;
     }
     mask = constant_time_eq_int_8(0,
         CRYPTO_memcmp(ctext, tmp_ctext, vinfo->ctext_bytes));
     for (i = 0; i < ML_KEM_SHARED_SECRET_BYTES; i++)
         secret[i] = constant_time_select_8(mask, Kr[i], failure_key[i]);
+    ret = 1;
 end:
     OPENSSL_cleanse(buf, DECAP_BUFFER_SZ);
-    return 1;
+    return ret;
 }
 
 /*

diff --git a/pkcs11-provider b/pkcs11-provider
index e83bb0ad91f8ce9739f2965eb8e66507b74af24c..663dea335c80bec7fd96d544ff875af08d6461a9 160000 (submodule)
--- a/pkcs11-provider
+++ b/pkcs11-provider
@@ -1 +1 @@
-Subproject commit e83bb0ad91f8ce9739f2965eb8e66507b74af24c
+Subproject commit 663dea335c80bec7fd96d544ff875af08d6461a9

diff --git a/test/ml_kem_internal_test.c b/test/ml_kem_internal_test.c
index 23eeb2aa477234e23ec6b264bb7d27f0282af923..14487dab942eeb3f139b7272b515f7d83b04e431 100644 (file)
--- a/test/ml_kem_internal_test.c
+++ b/test/ml_kem_internal_test.c
@@ -242,11 +242,81 @@ err:
     return ret == 0;
 }
 
+static int decap_mfail_test(void)
+{
+    EVP_RAND_CTX *privctx, *pubctx;
+    OSSL_PARAM params[3];
+    uint8_t shared_secret[ML_KEM_SHARED_SECRET_BYTES];
+    uint8_t decap_secret[ML_KEM_SHARED_SECRET_BYTES];
+    uint8_t *encoded_public_key = NULL;
+    uint8_t *ciphertext = NULL;
+    ML_KEM_KEY *private_key = NULL;
+    ML_KEM_KEY *public_key = NULL;
+    unsigned int strength = 256;
+    const ML_KEM_VINFO *v;
+    int rc, ret = -1;
+
+    if (!TEST_ptr(privctx = RAND_get0_private(NULL))
+        || !TEST_ptr(pubctx = RAND_get0_public(NULL)))
+        return 0;
+
+    params[0] = OSSL_PARAM_construct_octet_string(OSSL_RAND_PARAM_TEST_ENTROPY,
+        ml_kem_private_entropy, sizeof(ml_kem_private_entropy));
+    params[1] = OSSL_PARAM_construct_uint(OSSL_RAND_PARAM_STRENGTH, &strength);
+    params[2] = OSSL_PARAM_construct_end();
+    if (!TEST_true(EVP_RAND_CTX_set_params(privctx, params)))
+        goto err;
+
+    public_key = ossl_ml_kem_key_new(NULL, NULL, EVP_PKEY_ML_KEM_768);
+    private_key = ossl_ml_kem_key_new(NULL, NULL, EVP_PKEY_ML_KEM_768);
+    if (private_key == NULL || public_key == NULL
+        || (v = ossl_ml_kem_key_vinfo(public_key)) == NULL)
+        goto err;
+
+    encoded_public_key = OPENSSL_malloc(v->pubkey_bytes);
+    ciphertext = OPENSSL_malloc(v->ctext_bytes);
+    if (encoded_public_key == NULL || ciphertext == NULL)
+        goto err;
+
+    if (!ossl_ml_kem_genkey(encoded_public_key, v->pubkey_bytes, private_key)
+        || !ossl_ml_kem_parse_public_key(encoded_public_key, v->pubkey_bytes,
+            public_key))
+        goto err;
+
+    params[0] = OSSL_PARAM_construct_octet_string(OSSL_RAND_PARAM_TEST_ENTROPY,
+        ml_kem_public_entropy, sizeof(ml_kem_public_entropy));
+    if (!TEST_true(EVP_RAND_CTX_set_params(pubctx, params)))
+        goto err;
+
+    if (!ossl_ml_kem_encap_rand(ciphertext, v->ctext_bytes,
+            shared_secret, sizeof(shared_secret), public_key))
+        goto err;
+
+    MFAIL_start();
+    rc = ossl_ml_kem_decap(decap_secret, sizeof(decap_secret),
+        ciphertext, v->ctext_bytes, private_key);
+    MFAIL_end();
+
+    if (rc == 1
+        && !TEST_mem_eq(decap_secret, sizeof(decap_secret),
+            shared_secret, sizeof(shared_secret)))
+        goto err;
+
+    ret = rc;
+err:
+    ossl_ml_kem_key_free(private_key);
+    ossl_ml_kem_key_free(public_key);
+    OPENSSL_free(encoded_public_key);
+    OPENSSL_free(ciphertext);
+    return ret;
+}
+
 int setup_tests(void)
 {
     if (!TEST_true(RAND_set_DRBG_type(NULL, "TEST-RAND", "fips=no", NULL, NULL)))
         return 0;
 
     ADD_TEST(sanity_test);
+    ADD_MFAIL_TEST(decap_mfail_test);
     return 1;
 }