From: Viktor Dukhovni Date: Tue, 18 Feb 2025 07:42:41 +0000 (+1100) Subject: More consistent ML-KEM key checks X-Git-Tag: openssl-3.5.0-alpha1~156 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4465bf694ea4505c544a96f2cfb329d86e8b711;p=thirdparty%2Fopenssl.git More consistent ML-KEM key checks - Cross-check seed `z` value on import as well as load. - In import/load When re-generating from a seed, check hash of any explicit private key when both provided. - Avoid leak of expanded key encoding when load fails. Reviewed-by: Tomas Mraz Reviewed-by: Tim Hudson (Merged from https://github.com/openssl/openssl/pull/26812) --- diff --git a/crypto/encode_decode/decoder_pkey.c b/crypto/encode_decode/decoder_pkey.c index c33eed21d5a..824bfb050dd 100644 --- a/crypto/encode_decode/decoder_pkey.c +++ b/crypto/encode_decode/decoder_pkey.c @@ -149,15 +149,7 @@ static int decoder_construct_pkey(OSSL_DECODER_INSTANCE *decoder_inst, * result in the keymgmt. */ if (keymgmt_prov == decoder_prov) { - /* - * When load returns NULL, because, though the provided key material - * is syntactically valid (parsed OK), it is not an acceptable key, - * the reason why the key is rejected would be lost, unless we - * signal a hard error, and suppress resetting for another try. - */ keydata = evp_keymgmt_load(keymgmt, object_ref, object_ref_sz); - if (keydata == NULL) - ossl_decoder_ctx_set_harderr(data->ctx); } else { struct evp_keymgmt_util_try_import_data_st import_data; @@ -180,6 +172,14 @@ static int decoder_construct_pkey(OSSL_DECODER_INSTANCE *decoder_inst, keydata = import_data.keydata; import_data.keydata = NULL; } + /* + * When load or import fails, because this is not an acceptable key + * (despite the provided key material being syntactically valid), the + * reason why the key is rejected would be lost, unless we signal a + * hard error, and suppress resetting for another try. + */ + if (keydata == NULL) + ossl_decoder_ctx_set_harderr(data->ctx); if (keydata != NULL && (pkey = evp_keymgmt_util_make_pkey(keymgmt, keydata)) == NULL) diff --git a/providers/implementations/keymgmt/ml_kem_kmgmt.c b/providers/implementations/keymgmt/ml_kem_kmgmt.c index cd11659eff1..a2fb4ad939a 100644 --- a/providers/implementations/keymgmt/ml_kem_kmgmt.c +++ b/providers/implementations/keymgmt/ml_kem_kmgmt.c @@ -312,6 +312,36 @@ static const OSSL_PARAM *ml_kem_imexport_types(int selection) return NULL; } +static int check_seed(const uint8_t *seed, const uint8_t *prvenc, + ML_KEM_KEY *key) +{ + size_t zlen = ML_KEM_RANDOM_BYTES; + + if (memcmp(seed + ML_KEM_SEED_BYTES - zlen, + prvenc + key->vinfo->prvkey_bytes - zlen, zlen) == 0) + return 1; + ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_KEY, + "private %s key implicit rejection secret does" + " not match seed", key->vinfo->algorithm_name); + return 0; +} + +static int check_pkhash(const uint8_t *prvenc, ML_KEM_KEY *key) +{ + size_t off; + + /* point to the H(ek) offset in dk = DKpke||ek||H(ek)||z */ + off = key->vinfo->prvkey_bytes - ML_KEM_RANDOM_BYTES - ML_KEM_PKHASH_BYTES; + if (memcmp(key->pkhash, prvenc + off, ML_KEM_PKHASH_BYTES) == 0) + return 1; + + ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_KEY, + "explicit %s private key does not match seed", + key->vinfo->algorithm_name); + ossl_ml_kem_key_reset(key); + return 0; +} + static int ml_kem_key_fromdata(ML_KEM_KEY *key, const OSSL_PARAM params[], int include_private) @@ -372,7 +402,7 @@ static int ml_kem_key_fromdata(ML_KEM_KEY *key, /* Check any explicit public key against embedded value in private key */ if (publen > 0 && prvlen > 0) { - /* point to the ek offset in the DKpke||ek||H(ek)||z */ + /* point to the ek offset in dk = DKpke||ek||H(ek)||z */ puboff = prvlen - ML_KEM_RANDOM_BYTES - ML_KEM_PKHASH_BYTES - publen; if (memcmp(pubenc, (unsigned char *)prvenc + puboff, publen) != 0) { ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_KEY, @@ -382,11 +412,16 @@ static int ml_kem_key_fromdata(ML_KEM_KEY *key, } } - if (seedlen != 0 && (prvlen == 0 || key->prefer_seed)) - return ossl_ml_kem_set_seed(seedenc, seedlen, key) - && ossl_ml_kem_genkey(NULL, 0, key); - else if (prvlen != 0) + if (seedlen != 0 && (prvlen == 0 || key->prefer_seed)) { + if (prvlen != 0 && !check_seed(seedenc, prvenc, key)) + return 0; + if (!ossl_ml_kem_set_seed(seedenc, seedlen, key) + || !ossl_ml_kem_genkey(NULL, 0, key)) + return 0; + return prvlen == 0 || check_pkhash(prvenc, key); + } else if (prvlen != 0) { return ossl_ml_kem_parse_private_key(prvenc, prvlen, key); + } return ossl_ml_kem_parse_public_key(pubenc, publen, key); } @@ -438,9 +473,8 @@ static const OSSL_PARAM *ml_kem_gettable_params(void *provctx) void *ml_kem_load(const void *reference, size_t reference_sz) { ML_KEM_KEY *key = NULL; - uint8_t *encoded_dk; + uint8_t *encoded_dk = NULL; uint8_t seed[ML_KEM_SEED_BYTES]; - size_t zlen = ML_KEM_RANDOM_BYTES; if (ossl_prov_is_running() && reference_sz == sizeof(key)) { /* The contents of the reference is the address to our object */ @@ -449,24 +483,15 @@ void *ml_kem_load(const void *reference, size_t reference_sz) key->encoded_dk = NULL; /* We grabbed, so we detach it */ *(ML_KEM_KEY **)reference = NULL; - /* - * Reject |z| mismatch between seed and key, the seed buffer holds |z| - * followed by |d|. - */ if (encoded_dk != NULL && ossl_ml_kem_encode_seed(seed, sizeof(seed), key) - && memcmp(seed + sizeof(seed) - zlen, - encoded_dk + key->vinfo->prvkey_bytes - zlen, - zlen) != 0) { - ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_KEY, - "private %s key implicit rejection secret does" - " not match seed", key->vinfo->algorithm_name); + && !check_seed(seed, encoded_dk, key)) goto err; - } /* Generate the key now, if it holds only a stashed seed. */ if (ossl_ml_kem_have_seed(key) && (encoded_dk == NULL || key->prefer_seed)) { - if (!ossl_ml_kem_genkey(NULL, 0, key)) + if (!ossl_ml_kem_genkey(NULL, 0, key) + || (encoded_dk != NULL && !check_pkhash(encoded_dk, key))) goto err; } else if (encoded_dk != NULL) { if (!ossl_ml_kem_parse_private_key(encoded_dk, @@ -484,6 +509,7 @@ void *ml_kem_load(const void *reference, size_t reference_sz) } err: + OPENSSL_free(encoded_dk); ossl_ml_kem_key_free(key); return NULL; } diff --git a/test/recipes/15-test_ml_kem_codecs.t b/test/recipes/15-test_ml_kem_codecs.t index 587fc95b757..03e6c56a366 100644 --- a/test/recipes/15-test_ml_kem_codecs.t +++ b/test/recipes/15-test_ml_kem_codecs.t @@ -25,7 +25,7 @@ my @formats = qw(seed-priv priv-only seed-only oqskeypair bare-seed bare-priv); plan skip_all => "ML-KEM isn't supported in this build" if disabled("ml-kem"); -plan tests => @algs * (23 + 10 * @formats); +plan tests => @algs * (24 + 10 * @formats); my $seed = join ("", map {sprintf "%02x", $_} (0..63)); my $weed = join ("", map {sprintf "%02x", $_} (1..64)); my $ikme = join ("", map {sprintf "%02x", $_} (0..31)); @@ -164,31 +164,42 @@ foreach my $alg (@algs) { my $real = sprintf('real-%s.der', $alg); my $fake = sprintf('fake-%s.der', $alg); my $mixt = sprintf('mixt-%s.der', $alg); + my $mash = sprintf('mash-%s.der', $alg); my $slen = $alg * 3 / 2; # Secret vector |s| my $plen = $slen + 64; # Public |t|, |rho| and hash my $zlen = 32; # FO implicit reject seed ok(run(app([qw(openssl genpkey -algorithm), "ml-kem-$alg", - qw(-provparam ml-kem.output_formats=bare-priv -pkeyopt), - "hexseed:$seed", qw(-outform DER -out), $real], - sprintf("create real private key: %s", $alg)))); + qw(-provparam ml-kem.output_formats=seed-priv -pkeyopt), + "hexseed:$seed", qw(-outform DER -out), $real])), + sprintf("create real private key: %s", $alg)); ok(run(app([qw(openssl genpkey -algorithm), "ml-kem-$alg", - qw(-provparam ml-kem.output_formats=bare-priv -pkeyopt), - "hexseed:$weed", qw(-outform DER -out), $fake], - sprintf("create fake private key: %s", $alg)))); + qw(-provparam ml-kem.output_formats=seed-priv -pkeyopt), + "hexseed:$weed", qw(-outform DER -out), $fake])), + sprintf("create fake private key: %s", $alg)); my $realfh = IO::File->new($real, "r"); my $fakefh = IO::File->new($fake, "r"); local $/ = undef; my $realder = <$realfh>; my $fakeder = <$fakefh>; - ok (length($realder) == 24 + $slen + $plen + $zlen - && length($fakeder) == 24 + $slen + $plen + $zlen); - my $mixtder = substr($realder, 0, 24 + $slen) - . substr($fakeder, 24 + $slen, $plen) - . substr($realder, 24 + $slen + $plen, $zlen); + ok (length($realder) == 28 + (2 + 64) + (4 + $slen + $plen + $zlen) + && length($fakeder) == 28 + (2 + 64) + (4 + $slen + $plen + $zlen)); + my $mixtder = substr($realder, 0, 28 + 66 + 4 + $slen) + . substr($fakeder, 28 + 66 + 4 + $slen, $plen) + . substr($realder, 28 + 66 + 4 + $slen + $plen, $zlen); my $mixtfh = IO::File->new($mixt, "w"); print $mixtfh $mixtder; + $mixtfh->close(); ok(run(app([qw(openssl pkey -inform DER -noout -in), $real], sprintf("accept valid keypair: %s", $alg)))); - ok(!run(app([qw(openssl pkey -inform DER -noout -in), $mixt], - sprintf("reject real private and fake public: %s", $alg)))); + ok(!run(app([qw(openssl pkey -provparam ml-kem.prefer_seed=no), + qw(-inform DER -noout -in), $mixt])), + sprintf("reject real private and fake public: %s", $alg)); + # Mutate the public key hash + my $mashder = $realder; + substr($mashder, -64, 1) =~ s{(.)}{chr(ord($1)^1)}es; + my $mashfh = IO::File->new($mash, "w"); + print $mashfh $mashder; + $mashfh->close(); + ok(!run(app([qw(openssl pkey -inform DER -noout -in), $mash])), + sprintf("reject real private and mutated public: %s", $alg)); }