From: Jakub Zelenka Date: Tue, 9 Jun 2026 19:07:39 +0000 (+0200) Subject: ml_kem: return an error on catastrophic failure in decap X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e2bd9f8c28;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Tomas Mraz 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 e40047e5962..8fa5db9cc87 100644 --- 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 e83bb0ad91f..663dea335c8 160000 --- 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 23eeb2aa477..14487dab942 100644 --- 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; }