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 <tomas@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(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 c33eed21d5ad9069e06d8402c9b1f56bd6529ade..824bfb050dd1f3a10a70c3d99c159fedf550304c 100644 (file)
--- 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 cd11659eff16a40237519a738e6de4a369bc8c56..a2fb4ad939a13c14a27cf30f0e8ddea1634ef018 100644 (file)
--- 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 587fc95b75756b7e0fc6fa74d1f140334e7d47f8..03e6c56a3668ac26bce12b986fa2b182f879250b 100644 (file)
--- 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));
 }