fix: check PKCS#8 publicKey against private key

Author: MarkAtwood

ChangeLog.md

@@ -39,6 +39,7 @@
   dimensions is rejected with `CACHE_MATCH_ERROR` instead of being copied in.
   A saved cache from an older release cannot be restored by this one.
 * **BREAKING (RFC 5958)**: The `publicKey` field of PKCS#8 v2 private keys (`OneAsymmetricKey`) for Ed25519, Ed448, X25519, ML-DSA and Falcon is a BIT STRING and now carries its unused-bits byte, so keys written by wolfSSL load in ring and other conformant parsers, and wolfSSL reads theirs. X25519 key values are still stored in reversed byte order, a separate bug, so X25519 PKCS#8 keys do not yet interoperate. Keys written by earlier wolfSSL still decode, but earlier wolfSSL cannot read keys written by this release. To write keys that every version reads, use `wc_Ed25519PrivateKeyToDer` and the other `*PrivateKeyToDer` functions, which omit `publicKey`. `WC_MLDSA_44/65/87_BOTH_KEY_DER_SIZE` and `CURVE25519_MAX_KEY_TO_DER_SZ` each grow by one byte, so applications that size buffers from them must be recompiled. A non-empty `publicKey` that matches neither form is now rejected with `ASN_PARSE_E`; an empty one is still treated as absent. by @MarkAtwood (PR 11674)
+* `wc_Falcon_PrivateKeyDecode()` now rejects a PKCS#8 key whose public key, in `publicKey` or in the legacy concatenated `privateKey`, does not match the private key, returning `PUBLIC_KEY_E` and leaving the key unset; verify-only and crypto-callback-only builds check only that both halves are present. by @MarkAtwood (issue 11671)
 
 ## Post-Quantum Cryptography (PQC)
 
@@ -48,6 +49,7 @@
 * Made `wc_falcon_check_key()` constant time, and cut Falcon key generation's peak heap by about 20% with a constant-time inversion of f. by @Frauschi
 * Added `--enable-falcon=smallest-mem`, a Falcon signer working in 20*n bytes (12KB peak heap at Falcon-512, 22KB at Falcon-1024). by @Frauschi
 * Added ML-KEM (FIPS 203) key OIDs, SubjectPublicKeyInfo and PKCS#8 encoding, and X.509 certificate support, including issuing an ML-KEM certificate with `wc_MakeCert_ex`. A key initialised with the new `WC_ML_KEM_TYPE_UNSET` takes its parameter set from the DER being decoded. by @Frauschi
+* `wc_MlDsaKey_PrivateKeyDecode` now returns `PUBLIC_KEY_E` for an expanded ML-DSA private key whose public key does not match the `tr = H(pk)` value the private key embeds (not checked in `WOLF_CRYPTO_CB_ONLY_MLDSA` builds). Such keys previously loaded and then produced signatures that failed to verify. by @MarkAtwood
 
 ## Bug Fixes
 

doc/dox_comments/header_files-ja/wc_mldsa.h

@@ -694,13 +694,14 @@ int wc_MlDsaKey_ExportKey(wc_MlDsaKey* key, byte* priv, word32 *privSz,
 /*!
     \ingroup ML_DSA
 
-    \brief DER/ASN.1でエンコードされたバッファ(PKCS#8 OneAsymmetricKey)からML-DSA秘密鍵を解析します。パラメータセットはエンコード内のアルゴリズム識別子から推定されるため、事前に設定する必要はありません。成功時、*inOutIdxは消費したバイト数分進められます。
+    \brief DER/ASN.1でエンコードされたバッファ(PKCS#8 OneAsymmetricKey)からML-DSA秘密鍵を解析します。パラメータセットはエンコード内のアルゴリズム識別子から推定されるため、事前に設定する必要はありません。成功時、*inOutIdxは消費したバイト数分進められます。秘密鍵が展開形式のみの場合、エンコードに含まれる公開鍵(publicKeyフィールド、または展開形式の秘密鍵の後に連結された形式)は秘密鍵内のtr = H(pk)の値と照合されます。WOLFSSL_ASN_TEMPLATEかつ鍵生成が有効なビルドでは、シードを含む秘密鍵はシードから鍵ペアが再導出され、エンコードされた公開鍵は無視されます。WOLF_CRYPTO_CB_ONLY_MLDSAが定義されている場合、この照合は行われません。
 
     WOLFSSL_MLDSA_NO_ASN1が定義されていない場合にのみ利用できます。
 
     \return 0 成功した場合に返されます。
     \return BAD_FUNC_ARG 必要なポインタのいずれかがNULLの場合に返されます。
     \return ASN_PARSE_E エンコードの形式が不正な場合に返されます。
+    \return PUBLIC_KEY_E エンコードされた公開鍵が秘密鍵と一致しない場合に返されます。拒否された鍵ペアは鍵オブジェクトに読み込まれません。
 
     \param [in,out] key 初期化済みのwc_MlDsaKeyへのポインタ。
     \param [in] input DERエンコードされた秘密鍵バイト列。

doc/dox_comments/header_files/falcon.h

@@ -508,9 +508,16 @@ int wc_falcon_sig_size(falcon_key* key);
     \ingroup Falcon
 
     \brief Decodes a DER/ASN.1 (PKCS#8) Falcon private key into key. On return
-    *inOutIdx is advanced past the consumed input.
+    *inOutIdx is advanced past the consumed input. When the input carries a
+    public key, either in the publicKey field or after the private key in the
+    legacy concatenated privateKey layout, it is checked against the private
+    key with wc_falcon_check_key(). The check is cryptographic only when
+    native signing is compiled in; verify-only and crypto-callback-only builds
+    check only that both halves are present. On a failed check the key is
+    left with neither half set.
 
     \return 0 on success.
+    \return PUBLIC_KEY_E if the public key does not match the private key.
     \return ASN_PARSE_E or other negative error on a malformed input.
 
     \param [in] input DER-encoded private key.
@@ -520,6 +527,7 @@ int wc_falcon_sig_size(falcon_key* key);
 
     \sa wc_Falcon_PrivateKeyToDer
     \sa wc_Falcon_PublicKeyDecode
+    \sa wc_falcon_check_key
 */
 int wc_Falcon_PrivateKeyDecode(const byte* input, word32* inOutIdx,
     falcon_key* key, word32 inSz);

doc/dox_comments/header_files/wc_mldsa.h

@@ -876,13 +876,21 @@ int wc_MlDsaKey_ExportKey(wc_MlDsaKey* key, byte* priv, word32 *privSz,
     buffer (PKCS#8 OneAsymmetricKey). The parameter set is inferred
     from the algorithm identifier in the encoding, so it does NOT
     need to be set beforehand. On success *inOutIdx is advanced past
-    the consumed bytes.
+    the consumed bytes. When the private key is in expanded form only,
+    a public key carried in the encoding, in the publicKey field or
+    appended to the expanded private key, is checked against the
+    tr = H(pk) value in the private key. In WOLFSSL_ASN_TEMPLATE builds
+    with key generation, a private key holding a seed is re-derived from
+    the seed and any encoded public key is ignored. The check is not made when WOLF_CRYPTO_CB_ONLY_MLDSA
+    is defined.
 
     Only available when WOLFSSL_MLDSA_NO_ASN1 is not defined.
 
     \return 0 on success.
     \return BAD_FUNC_ARG if any required pointer is NULL.
     \return ASN_PARSE_E on malformed encoding.
+    \return PUBLIC_KEY_E if the encoded public key does not match the
+    private key. The rejected keys are not loaded.
 
     \param [in,out] key Pointer to an initialized wc_MlDsaKey.
     \param [in] input DER-encoded private key bytes.

tests/api/test_falcon.c

@@ -648,6 +648,127 @@ int test_wc_falcon_der(void)
     return EXPECT_RESULT();
 }
 
+#if defined(WC_FALCON_HAVE_NATIVE_SIGN) && \
+    (!defined(WOLFSSL_NO_FALCON_LEVEL1) || !defined(WOLFSSL_NO_FALCON_LEVEL5))
+/*
+ * fix is a liboqs-generated PKCS#8: SEQUENCE, version and AlgorithmIdentifier
+ * (bytes 4..15), a privateKey OCTET STRING wrapping an OCTET STRING whose raw
+ * key starts at 24, then a 4-byte [1] header and the raw public key, which
+ * ends the DER. h is fixed by (f, g), so changing one public coefficient
+ * yields a mismatched pair.
+ */
+static int falcon_pkcs8_mismatch_level(const byte* fix, word32 fixSz,
+    byte level, word32 keySz, word32 pubSz)
+{
+    EXPECT_DECLS;
+    const word32 privIdx = 24;
+    const word32 pubIdx = fixSz - pubSz;
+    const word32 rawSz = keySz + pubSz;
+    const word32 catSz = rawSz + 24;
+    static const byte msg[] = { 0x61, 0x62, 0x63 };
+    falcon_key key;
+    WC_RNG rng;
+    byte* der = NULL;
+    word32 idx;
+    word32 outLen;
+
+    ExpectIntEQ(privIdx + keySz + 4, pubIdx);
+    ExpectIntEQ(fix[privIdx - 4], 0x04);
+    ExpectIntEQ(fix[pubIdx - 4], 0x81);
+    ExpectIntLE(catSz, fixSz);
+
+    XMEMSET(&rng, 0, sizeof(rng));
+    ExpectIntEQ(wc_InitRng(&rng), 0);
+    der = (byte*)XMALLOC(fixSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    ExpectNotNull(der);
+
+    /* [1] publicKey form: a valid key is loaded first, then the same object
+     * gets the mismatched DER. */
+    XMEMSET(&key, 0, sizeof(key));
+    ExpectIntEQ(wc_falcon_init(&key), 0);
+    ExpectIntEQ(wc_falcon_set_level(&key, level), 0);
+    idx = 0;
+    ExpectIntEQ(wc_Falcon_PrivateKeyDecode(fix, &idx, &key, fixSz), 0);
+    if (der != NULL) {
+        XMEMCPY(der, fix, fixSz);
+        /* Last coefficient goes 246 -> 247 (L1) or 6704 -> 6705 (L5), still
+         * < q: only the h*f == g relation fails. */
+        der[fixSz - 1] ^= 0x01;
+    }
+    idx = 0;
+    ExpectIntEQ(wc_Falcon_PrivateKeyDecode(der, &idx, &key, fixSz),
+        WC_NO_ERR_TRACE(PUBLIC_KEY_E));
+    /* Neither the rejected pair nor the earlier key may remain usable. */
+    outLen = fixSz;
+    ExpectIntEQ(wc_falcon_sign_msg(msg, (word32)sizeof(msg), der, &outLen,
+        &key, &rng), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    outLen = fixSz;
+    ExpectIntEQ(wc_falcon_export_private_only(&key, der, &outLen),
+        WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    outLen = fixSz;
+    ExpectIntEQ(wc_falcon_export_public(&key, der, &outLen),
+        WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    wc_falcon_free(&key);
+
+    /* Legacy form: privateKey holds concat(priv, pub) and there is no [1]. */
+    if (der != NULL) {
+        der[0] = 0x30;
+        der[1] = 0x82;
+        der[2] = (byte)((catSz - 4) >> 8);
+        der[3] = (byte)(catSz - 4);
+        XMEMCPY(der + 4, fix + 4, 12);
+        der[16] = 0x04;
+        der[17] = 0x82;
+        der[18] = (byte)((rawSz + 4) >> 8);
+        der[19] = (byte)(rawSz + 4);
+        der[20] = 0x04;
+        der[21] = 0x82;
+        der[22] = (byte)(rawSz >> 8);
+        der[23] = (byte)rawSz;
+        XMEMCPY(der + 24, fix + privIdx, keySz);
+        XMEMCPY(der + 24 + keySz, fix + pubIdx, pubSz);
+    }
+    XMEMSET(&key, 0, sizeof(key));
+    ExpectIntEQ(wc_falcon_init(&key), 0);
+    ExpectIntEQ(wc_falcon_set_level(&key, level), 0);
+    idx = 0;
+    ExpectIntEQ(wc_Falcon_PrivateKeyDecode(der, &idx, &key, catSz), 0);
+    ExpectIntEQ(idx, catSz);
+    if (der != NULL) {
+        der[catSz - 1] ^= 0x01;
+    }
+    idx = 0;
+    ExpectIntEQ(wc_Falcon_PrivateKeyDecode(der, &idx, &key, catSz),
+        WC_NO_ERR_TRACE(PUBLIC_KEY_E));
+    outLen = fixSz;
+    ExpectIntEQ(wc_falcon_sign_msg(msg, (word32)sizeof(msg), der, &outLen,
+        &key, &rng), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    wc_falcon_free(&key);
+
+    XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    wc_FreeRng(&rng);
+    return EXPECT_RESULT();
+}
+#endif
+
+/* A PKCS#8 public key that does not match the private key must be rejected,
+ * whether it arrives in [1] or in the legacy concat privateKey. */
+int test_wc_falcon_pkcs8_pub_mismatch(void)
+{
+    EXPECT_DECLS;
+#if defined(WC_FALCON_HAVE_NATIVE_SIGN) && !defined(WOLFSSL_NO_FALCON_LEVEL1)
+    ExpectIntEQ(falcon_pkcs8_mismatch_level(bench_falcon_level1_key,
+        (word32)sizeof_bench_falcon_level1_key, FALCON_LEVEL1,
+        FALCON_LEVEL1_KEY_SIZE, FALCON_LEVEL1_PUB_KEY_SIZE), TEST_SUCCESS);
+#endif
+#if defined(WC_FALCON_HAVE_NATIVE_SIGN) && !defined(WOLFSSL_NO_FALCON_LEVEL5)
+    ExpectIntEQ(falcon_pkcs8_mismatch_level(bench_falcon_level5_key,
+        (word32)sizeof_bench_falcon_level5_key, FALCON_LEVEL5,
+        FALCON_LEVEL5_KEY_SIZE, FALCON_LEVEL5_PUB_KEY_SIZE), TEST_SUCCESS);
+#endif
+    return EXPECT_RESULT();
+}
+
 /*
  * Exhaustive argument sanitising for the always-present entry points. Runs in
  * every HAVE_FALCON build (including verify-only / crypto-cb-only); make_key is

tests/api/test_falcon.h

@@ -30,6 +30,7 @@ int test_wc_falcon_sign_vfy(void);
 int test_wc_falcon_import_export(void);
 int test_wc_falcon_check_key(void);
 int test_wc_falcon_der(void);
+int test_wc_falcon_pkcs8_pub_mismatch(void);
 int test_wc_falcon_error_paths(void);
 int test_wc_falcon_deterministic(void);
 int test_wc_falcon_key_reuse(void);
@@ -45,6 +46,7 @@ int test_falcon_cb_free(void);
     TEST_DECL_GROUP("falcon", test_wc_falcon_import_export),                  \
     TEST_DECL_GROUP("falcon", test_wc_falcon_check_key),                      \
     TEST_DECL_GROUP("falcon", test_wc_falcon_der),                            \
+    TEST_DECL_GROUP("falcon", test_wc_falcon_pkcs8_pub_mismatch),             \
     TEST_DECL_GROUP("falcon", test_wc_falcon_error_paths),                    \
     TEST_DECL_GROUP("falcon", test_wc_falcon_deterministic),                  \
     TEST_DECL_GROUP("falcon", test_wc_falcon_key_reuse),                      \