mirror of
https://github.com/openssl/openssl.git
synced 2026-05-07 20:12:39 +00:00
Fix double-free in mlx_kem_dup() default case
Null mkey/xkey immediately after OPENSSL_memdup() so that any failure
path (including propq strdup) can safely call mlx_kem_key_free() without
risking a double-free on the source key's material. Use key->* rather
than ret->* for source-state checks to make ownership explicit.
Test that mlx_kem_dup() with partial key selection (e.g.
EVP_PKEY_PUBLIC_KEY) does not corrupt the original key's mkey/xkey
sub-objects. Covers X25519MLKEM768, SecP256r1MLKEM768,
and SecP384r1MLKEM1024.
Fixes: 4b1c73d2dd "ML-KEM hybrids for TLS"
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
MergeDate: Sun Apr 26 11:14:12 2026
(Merged from https://github.com/openssl/openssl/pull/30511)
This commit is contained in:
committed by
Eugene Syromiatnikov
parent
7fb28b9cd0
commit
aeea7dfaff
@@ -719,15 +719,17 @@ static void *mlx_kem_dup(const void *vkey, int selection)
|
||||
|| (ret = OPENSSL_memdup(key, sizeof(*ret))) == NULL)
|
||||
return NULL;
|
||||
|
||||
if (ret->propq != NULL
|
||||
&& (ret->propq = OPENSSL_strdup(ret->propq)) == NULL) {
|
||||
ret->mkey = ret->xkey = NULL;
|
||||
|
||||
if (key->propq != NULL
|
||||
&& (ret->propq = OPENSSL_strdup(key->propq)) == NULL) {
|
||||
OPENSSL_free(ret);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Absent key material, nothing left to do */
|
||||
if (ret->mkey == NULL) {
|
||||
if (ret->xkey == NULL)
|
||||
if (key->mkey == NULL) {
|
||||
if (key->xkey == NULL)
|
||||
return ret;
|
||||
/* Fail if the source key is an inconsistent state */
|
||||
OPENSSL_free(ret->propq);
|
||||
@@ -737,7 +739,6 @@ static void *mlx_kem_dup(const void *vkey, int selection)
|
||||
|
||||
switch (selection & OSSL_KEYMGMT_SELECT_KEYPAIR) {
|
||||
case 0:
|
||||
ret->xkey = ret->mkey = NULL;
|
||||
ret->state = MLX_HAVE_NOKEYS;
|
||||
return ret;
|
||||
case OSSL_KEYMGMT_SELECT_KEYPAIR:
|
||||
|
||||
@@ -20,6 +20,7 @@
|
||||
#include <openssl/param_build.h>
|
||||
#include <openssl/rand.h>
|
||||
#include <crypto/ml_kem.h>
|
||||
#include "crypto/evp.h"
|
||||
#include "testutil.h"
|
||||
|
||||
static OSSL_LIB_CTX *testctx = NULL;
|
||||
@@ -420,6 +421,77 @@ static int test_ml_kem_from_data_propq(void)
|
||||
return ret;
|
||||
}
|
||||
|
||||
#ifndef OPENSSL_NO_EC
|
||||
static const char *mlx_kem_algs[] = {
|
||||
#ifndef OPENSSL_NO_ECX
|
||||
"X25519MLKEM768",
|
||||
#endif
|
||||
"SecP256r1MLKEM768",
|
||||
"SecP384r1MLKEM1024",
|
||||
};
|
||||
#endif
|
||||
|
||||
/*
|
||||
* Test that mlx_kem_dup() with partial selection (public-only) does not
|
||||
* corrupt the original key. Before the fix, the default branch of the
|
||||
* switch in mlx_kem_dup() would call mlx_kem_key_free() on a shallow copy
|
||||
* without nulling mkey/xkey first, causing a double-free when the original
|
||||
* key was later freed.
|
||||
*/
|
||||
#ifndef OPENSSL_NO_EC
|
||||
static int test_mlx_kem_dup_partial_selection(int idx)
|
||||
{
|
||||
const char *alg = mlx_kem_algs[idx];
|
||||
EVP_PKEY_CTX *genctx = NULL;
|
||||
EVP_PKEY_CTX *encctx = NULL;
|
||||
EVP_PKEY *keypair = NULL;
|
||||
EVP_PKEY *dest = NULL;
|
||||
size_t wrpkeylen = 0, genkeylen = 0;
|
||||
int ret = 0;
|
||||
|
||||
/* Generate an MLX KEM keypair */
|
||||
if (!TEST_ptr(genctx = EVP_PKEY_CTX_new_from_name(testctx, alg, NULL))
|
||||
|| !TEST_int_eq(EVP_PKEY_keygen_init(genctx), 1)
|
||||
|| !TEST_int_eq(EVP_PKEY_keygen(genctx, &keypair), 1))
|
||||
goto err;
|
||||
|
||||
/*
|
||||
* Attempt a partial copy (public-key only). EVP_PKEY_PUBLIC_KEY includes
|
||||
* OSSL_KEYMGMT_SELECT_PUBLIC_KEY (0x02) but not private, so
|
||||
* selection & OSSL_KEYMGMT_SELECT_KEYPAIR == 0x02 which hits the default
|
||||
* branch in mlx_kem_dup(). This should fail gracefully without corrupting
|
||||
* the source key.
|
||||
*/
|
||||
if (!TEST_ptr(dest = EVP_PKEY_new()))
|
||||
goto err;
|
||||
/* Expected to fail — partial duplication is not supported for MLX KEM */
|
||||
evp_keymgmt_util_copy(dest, keypair, EVP_PKEY_PUBLIC_KEY);
|
||||
ERR_clear_error();
|
||||
|
||||
/*
|
||||
* Verify the original keypair is still intact by performing an
|
||||
* encapsulate operation. If the partial copy corrupted the key
|
||||
* (double-freed mkey/xkey), this would crash or trigger ASan.
|
||||
*/
|
||||
if (!TEST_ptr(encctx = EVP_PKEY_CTX_new_from_pkey(testctx, keypair, NULL))
|
||||
|| !TEST_int_gt(EVP_PKEY_encapsulate_init(encctx, NULL), 0)
|
||||
|| !TEST_int_gt(EVP_PKEY_encapsulate(encctx, NULL, &wrpkeylen,
|
||||
NULL, &genkeylen),
|
||||
0)
|
||||
|| !TEST_size_t_gt(wrpkeylen, 0)
|
||||
|| !TEST_size_t_gt(genkeylen, 0))
|
||||
goto err;
|
||||
|
||||
ret = 1;
|
||||
err:
|
||||
EVP_PKEY_CTX_free(encctx);
|
||||
EVP_PKEY_free(dest);
|
||||
EVP_PKEY_free(keypair);
|
||||
EVP_PKEY_CTX_free(genctx);
|
||||
return ret;
|
||||
}
|
||||
#endif /* OPENSSL_NO_EC */
|
||||
|
||||
int setup_tests(void)
|
||||
{
|
||||
int test_rand = 0;
|
||||
@@ -448,5 +520,8 @@ int setup_tests(void)
|
||||
|
||||
ADD_TEST(test_ml_kem);
|
||||
ADD_TEST(test_ml_kem_from_data_propq);
|
||||
#ifndef OPENSSL_NO_EC
|
||||
ADD_ALL_TESTS(test_mlx_kem_dup_partial_selection, OSSL_NELEM(mlx_kem_algs));
|
||||
#endif
|
||||
return 1;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user