From: Viktor Dukhovni Date: Sat, 14 Mar 2026 09:47:06 +0000 (+1100) Subject: Once initialised, ML-DSA keys should be immutable X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2fabd5d2741a577e2c26c74baf4f50405ece6814;p=thirdparty%2Fopenssl.git Once initialised, ML-DSA keys should be immutable ML-DSA keys should become immutable once key material has been added. This is already the case for at least ML-KEM keys, and should generally be the case across all key types. - Added the requisite check in the key management provider ml_dsa_import() function. - Also, consolidated the ML-KEM checks in ml_kem_import(). These were previously partly in ml_kem_key_fromdata(). Reviewed-by: Shane Lontis Reviewed-by: Paul Dale MergeDate: Thu Jun 25 02:03:35 2026 (Merged from https://github.com/openssl/openssl/pull/30421) --- diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index d993f02d69c..14658d76341 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1106,6 +1106,7 @@ PROV_R_INVALID_THREAD_POOL_SIZE:234:invalid thread pool size PROV_R_INVALID_UKM_LENGTH:200:invalid ukm length PROV_R_INVALID_X931_DIGEST:170:invalid x931 digest PROV_R_IN_ERROR_STATE:192:in error state +PROV_R_KEY_IMMUTABLE_ONCE_SET:266:key immutable once set PROV_R_KEY_SETUP_FAILED:101:key setup failed PROV_R_KEY_SIZE_TOO_SMALL:171:key size too small PROV_R_LENGTH_TOO_LARGE:202:length too large diff --git a/include/openssl/proverr.h b/include/openssl/proverr.h index 79bbc6628ec..0db3a82b470 100644 --- a/include/openssl/proverr.h +++ b/include/openssl/proverr.h @@ -97,6 +97,7 @@ #define PROV_R_INVALID_UKM_LENGTH 200 #define PROV_R_INVALID_X931_DIGEST 170 #define PROV_R_IN_ERROR_STATE 192 +#define PROV_R_KEY_IMMUTABLE_ONCE_SET 266 #define PROV_R_KEY_SETUP_FAILED 101 #define PROV_R_KEY_SIZE_TOO_SMALL 171 #define PROV_R_LENGTH_TOO_LARGE 202 diff --git a/providers/common/provider_err.c b/providers/common/provider_err.c index 7e3e189dd24..33e7edb454b 100644 --- a/providers/common/provider_err.c +++ b/providers/common/provider_err.c @@ -144,6 +144,8 @@ static const ERR_STRING_DATA PROV_str_reasons[] = { { ERR_PACK(ERR_LIB_PROV, 0, PROV_R_INVALID_X931_DIGEST), "invalid x931 digest" }, { ERR_PACK(ERR_LIB_PROV, 0, PROV_R_IN_ERROR_STATE), "in error state" }, + { ERR_PACK(ERR_LIB_PROV, 0, PROV_R_KEY_IMMUTABLE_ONCE_SET), + "key immutable once set" }, { ERR_PACK(ERR_LIB_PROV, 0, PROV_R_KEY_SETUP_FAILED), "key setup failed" }, { ERR_PACK(ERR_LIB_PROV, 0, PROV_R_KEY_SIZE_TOO_SMALL), "key size too small" }, diff --git a/providers/implementations/keymgmt/ml_dsa_kmgmt.c b/providers/implementations/keymgmt/ml_dsa_kmgmt.c index 83217066aad..24406ac602a 100644 --- a/providers/implementations/keymgmt/ml_dsa_kmgmt.c +++ b/providers/implementations/keymgmt/ml_dsa_kmgmt.c @@ -293,8 +293,18 @@ static int ml_dsa_import(void *keydata, int selection, const OSSL_PARAM params[] int include_priv; int res; + /* + * Once a key is fully initialised (has at least a public component), + * further mutation is no longer safe and disallowed. + */ if (!ossl_prov_is_running() || key == NULL) return 0; + if (ossl_ml_dsa_key_has(key, OSSL_KEYMGMT_SELECT_PUBLIC_KEY)) { + /* Invalid attempt to mutate a key. */ + ERR_raise_data(ERR_LIB_PROV, PROV_R_KEY_IMMUTABLE_ONCE_SET, + "Keys are immutable once key material has been loaded or generated"); + return 0; + } if ((selection & OSSL_KEYMGMT_SELECT_KEYPAIR) == 0) return 0; diff --git a/providers/implementations/keymgmt/ml_kem_kmgmt.c b/providers/implementations/keymgmt/ml_kem_kmgmt.c index f82f3e6f68a..d7f2d876857 100644 --- a/providers/implementations/keymgmt/ml_kem_kmgmt.c +++ b/providers/implementations/keymgmt/ml_kem_kmgmt.c @@ -401,10 +401,7 @@ static int ml_kem_key_fromdata(ML_KEM_KEY *key, const OSSL_PARAM params[], const ML_KEM_VINFO *v; struct ml_kem_import_params_st p; - /* Invalid attempt to mutate a key, what is the right error to report? */ - if (key == NULL - || ossl_ml_kem_have_pubkey(key) - || !ml_kem_import_params_decoder(params, &p)) + if (!ml_kem_import_params_decoder(params, &p)) return 0; v = ossl_ml_kem_key_vinfo(key); @@ -489,11 +486,16 @@ static int ml_kem_import(void *vkey, int selection, const OSSL_PARAM params[]) int include_private; int res; - if (!ossl_prov_is_running() || key == NULL) + if (!ossl_prov_is_running() + || (selection & OSSL_KEYMGMT_SELECT_KEYPAIR) == 0 + || key == NULL) return 0; - - if ((selection & OSSL_KEYMGMT_SELECT_KEYPAIR) == 0) + if (ossl_ml_kem_have_pubkey(key)) { + /* Invalid attempt to mutate a key. */ + ERR_raise_data(ERR_LIB_PROV, PROV_R_KEY_IMMUTABLE_ONCE_SET, + "Keys are immutable once key material has been loaded or generated"); return 0; + } include_private = selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY ? 1 : 0; res = ml_kem_key_fromdata(key, params, include_private); @@ -686,9 +688,8 @@ static int ml_kem_set_params(void *vkey, const OSSL_PARAM params[]) /* Key mutation is reportedly generally not allowed */ if (ossl_ml_kem_have_pubkey(key)) { - ERR_raise_data(ERR_LIB_PROV, - PROV_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE, - "ML-KEM keys cannot be mutated"); + ERR_raise_data(ERR_LIB_PROV, PROV_R_KEY_IMMUTABLE_ONCE_SET, + "Keys are immutable once key material has been loaded or generated"); return 0; } diff --git a/test/ml_dsa_test.c b/test/ml_dsa_test.c index 1553f66c8f7..ca1c1225e28 100644 --- a/test/ml_dsa_test.c +++ b/test/ml_dsa_test.c @@ -8,10 +8,13 @@ */ #include +#include #include +#include #include "internal/nelem.h" #include "testutil.h" #include "ml_dsa.inc" +#include "crypto/evp.h" #include "crypto/ml_dsa.h" typedef enum OPTION_choice { @@ -644,6 +647,108 @@ err: return ret; } +/* + * Test that the keymgmt import dispatch refuses to import into a key whose + * public component is already set, i.e. the key is immutable once initialised. + * + * Four sub-cases are exercised: + * 1. public-key-only → re-import public key → must fail + * 2. public-key-only → import keypair → must fail + * 3. full keypair → re-import keypair → must fail + * 4. full keypair → import public key only → must fail + * + * All failures must raise PROV_R_KEY_IMMUTABLE_ONCE_SET. + */ +static int ml_dsa_key_immutable_test(void) +{ + int ret = 0; + EVP_KEYMGMT *keymgmt = NULL; + void *keydata = NULL; + const ML_DSA_KEYGEN_TEST_DATA *tst = &ml_dsa_keygen_testdata[0]; + OSSL_PARAM pub_params[2], keypair_params[3]; + + pub_params[0] = OSSL_PARAM_construct_octet_string( + OSSL_PKEY_PARAM_PUB_KEY, (void *)tst->pub, tst->pub_len); + pub_params[1] = OSSL_PARAM_construct_end(); + + keypair_params[0] = OSSL_PARAM_construct_octet_string( + OSSL_PKEY_PARAM_PRIV_KEY, (void *)tst->priv, tst->priv_len); + keypair_params[1] = OSSL_PARAM_construct_octet_string( + OSSL_PKEY_PARAM_PUB_KEY, (void *)tst->pub, tst->pub_len); + keypair_params[2] = OSSL_PARAM_construct_end(); + + if (!TEST_ptr(keymgmt = EVP_KEYMGMT_fetch(lib_ctx, tst->name, NULL))) + goto end; + + /* Sub-case 1 & 2: start from a public-key-only import */ + if (!TEST_ptr(keydata = evp_keymgmt_newdata(keymgmt, NULL))) + goto end; + + if (!TEST_true(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + pub_params))) + goto end; + + /* Re-import of the same public key must fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + pub_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + /* Import of a full keypair into a public-key-only key must also fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_KEYPAIR, + keypair_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + evp_keymgmt_freedata(keymgmt, keydata); + keydata = NULL; + + /* Sub-case 3 & 4: start from a full keypair import */ + if (!TEST_ptr(keydata = evp_keymgmt_newdata(keymgmt, NULL))) + goto end; + + if (!TEST_true(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_KEYPAIR, + keypair_params))) + goto end; + + /* Re-import of the same keypair must fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_KEYPAIR, + keypair_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + /* Import of a public-key-only into a full keypair must also fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + pub_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + ret = 1; +end: + if (keymgmt != NULL) + evp_keymgmt_freedata(keymgmt, keydata); + EVP_KEYMGMT_free(keymgmt); + return ret; +} + const OPTIONS *test_get_options(void) { static const OPTIONS options[] = { @@ -692,6 +797,13 @@ int setup_tests(void) ADD_TEST(from_data_bad_input_test); ADD_TEST(ml_dsa_digest_sign_verify_test); ADD_TEST(ml_dsa_priv_pub_bad_t0_test); + + /* + * Tested only in the default configuration, with a non-default provider + * configuration this test is expected to fail for some older providers. + */ + if (config_file == NULL) + ADD_TEST(ml_dsa_key_immutable_test); return 1; } diff --git a/test/ml_kem_evp_extra_test.c b/test/ml_kem_evp_extra_test.c index 4d8e506c5f1..2aa704427ac 100644 --- a/test/ml_kem_evp_extra_test.c +++ b/test/ml_kem_evp_extra_test.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include "crypto/evp.h" @@ -492,6 +493,154 @@ err: } #endif /* OPENSSL_NO_EC */ +/* + * Test that ML-KEM keys are immutable once key material is set. + * + * Part 1 — keymgmt import dispatch (ml_kem_import): + * Sub-case A: public-key-only first import succeeds; re-import of the + * same public key and any keypair import must fail with + * PROV_R_KEY_IMMUTABLE_ONCE_SET. + * Sub-case B: full keypair first import succeeds; re-import of the + * keypair and public-key-only import must fail with + * PROV_R_KEY_IMMUTABLE_ONCE_SET. + * + * Part 2 — EVP_PKEY_set1_encoded_public_key (ml_kem_set_params): + * The second call on a key that already has a public component must + * also fail with PROV_R_KEY_IMMUTABLE_ONCE_SET.. + */ +static int test_ml_kem_key_immutable(void) +{ + int ret = 0; + EVP_PKEY *akey = NULL, *bkey = NULL; + EVP_KEYMGMT *keymgmt = NULL; + void *keydata = NULL; + uint8_t *rawpub = NULL, *rawprv = NULL; + size_t publen = 0, prvlen = 0; + OSSL_PARAM pub_params[2], keypair_params[3]; + + /* Generate a key pair and extract the raw public and private key bytes. */ + if (!TEST_ptr(akey = EVP_PKEY_Q_keygen(testctx, NULL, "ML-KEM-768"))) + goto end; + if (!TEST_int_eq(EVP_PKEY_get_raw_public_key(akey, NULL, &publen), 1) + || !TEST_ptr(rawpub = OPENSSL_malloc(publen)) + || !TEST_int_eq(EVP_PKEY_get_raw_public_key(akey, rawpub, &publen), 1)) + goto end; + if (!TEST_int_eq(EVP_PKEY_get_raw_private_key(akey, NULL, &prvlen), 1) + || !TEST_ptr(rawprv = OPENSSL_malloc(prvlen)) + || !TEST_int_eq(EVP_PKEY_get_raw_private_key(akey, rawprv, &prvlen), 1)) + goto end; + + pub_params[0] = OSSL_PARAM_construct_octet_string( + OSSL_PKEY_PARAM_PUB_KEY, rawpub, publen); + pub_params[1] = OSSL_PARAM_construct_end(); + + keypair_params[0] = OSSL_PARAM_construct_octet_string( + OSSL_PKEY_PARAM_PRIV_KEY, rawprv, prvlen); + keypair_params[1] = OSSL_PARAM_construct_octet_string( + OSSL_PKEY_PARAM_PUB_KEY, rawpub, publen); + keypair_params[2] = OSSL_PARAM_construct_end(); + + if (!TEST_ptr(keymgmt = EVP_KEYMGMT_fetch(testctx, "ML-KEM-768", NULL))) + goto end; + + /* --- Part 1A: public-key-only import then re-import --- */ + if (!TEST_ptr(keydata = evp_keymgmt_newdata(keymgmt, NULL))) + goto end; + + if (!TEST_true(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + pub_params))) + goto end; + + /* Re-import of the same public key must fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + pub_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + /* Import of a full keypair into a public-key-only key must also fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_KEYPAIR, + keypair_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + evp_keymgmt_freedata(keymgmt, keydata); + keydata = NULL; + + /* --- Part 1B: full keypair import then re-import --- */ + if (!TEST_ptr(keydata = evp_keymgmt_newdata(keymgmt, NULL))) + goto end; + + if (!TEST_true(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_KEYPAIR, + keypair_params))) + goto end; + + /* Re-import of the same keypair must fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_KEYPAIR, + keypair_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + /* Import of a public-key-only into a full keypair must also fail */ + if (!TEST_false(evp_keymgmt_import(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + pub_params))) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + evp_keymgmt_freedata(keymgmt, keydata); + keydata = NULL; + + /* --- Part 2: EVP_PKEY_set1_encoded_public_key immutability --- */ + + /* + * Create an empty typed key (algorithm set, no key material) by + * copying parameters from the generated key. + */ + if (!TEST_ptr(bkey = EVP_PKEY_new()) + || !TEST_int_gt(EVP_PKEY_copy_parameters(bkey, akey), 0)) + goto end; + + /* First call must succeed: the key is still embryonic */ + if (!TEST_int_eq(EVP_PKEY_set1_encoded_public_key(bkey, rawpub, publen), 1)) + goto end; + + /* Second call must fail: the key now has a public component */ + if (!TEST_int_eq(EVP_PKEY_set1_encoded_public_key(bkey, rawpub, publen), 0)) + goto end; + if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_last_error()), + PROV_R_KEY_IMMUTABLE_ONCE_SET)) + goto end; + ERR_clear_error(); + + ret = 1; +end: + if (keymgmt != NULL) + evp_keymgmt_freedata(keymgmt, keydata); + EVP_KEYMGMT_free(keymgmt); + EVP_PKEY_free(akey); + EVP_PKEY_free(bkey); + OPENSSL_free(rawpub); + OPENSSL_free(rawprv); + return ret; +} + int setup_tests(void) { int test_rand = 0; @@ -520,6 +669,7 @@ int setup_tests(void) ADD_TEST(test_ml_kem); ADD_TEST(test_ml_kem_from_data_propq); + ADD_TEST(test_ml_kem_key_immutable); #ifndef OPENSSL_NO_EC ADD_ALL_TESTS(test_mlx_kem_dup_partial_selection, OSSL_NELEM(mlx_kem_algs)); #endif