Avoid writing into caller ikm buffer in wc_Tls13_HKDF_Extract

Author: yosuke-wolfssl

doc/dox_comments/header_files-ja/hmac.h

@@ -333,25 +333,25 @@ int wc_HKDF_Expand_ex(
     \brief この関数は、TLS v1.3鍵導出のためのRFC 5869 HMACベース抽出拡張鍵導出関数（HKDF）へのアクセスを提供します。
 
     \return 0 指定された入力で鍵の生成に成功した場合に返されます。
-    \return BAD_FUNC_ARG 無効なハッシュタイプが指定された場合に返されます（typeパラメータを参照）。
+    \return BAD_FUNC_ARG 無効なハッシュタイプが指定された場合（digestパラメータを参照）、prkがNULLの場合、またはikmがNULLでikmLenが0でない場合に返されます。
     \return MEMORY_E メモリ割り当てエラーがある場合に返されます。
     \return HMAC_MIN_KEYLEN_E FIPS実装を使用していて、指定された鍵の長さが最小許容FIPS標準より短い場合に返される可能性があります。
 
     \param prk 生成された疑似ランダム鍵。
-    \param salt ソルト。
+    \param salt ソルト。NULLの場合、saltLenは無視されます。
     \param saltLen ソルトの長さ。
-    \param ikm 鍵材料の出力へのポインタ。
-    \param ikmLen 入力鍵材料バッファの長さ。
+    \param ikm 入力鍵材料。ikmLenが0の場合はNULLにできます。
+    \param ikmLen 入力鍵材料の長さ。0の場合、RFC 8446に従い、空の入力鍵材料ではなくダイジェスト長のゼロ値が使用されます。
     \param digest HKDFに使用するハッシュタイプ。有効なタイプは：WC_SHA256、WC_SHA384、またはWC_SHA512。
 
     _Example_
     \code
-    byte secret[] = { // ランダム鍵で初期化 };
+    byte ikm[] = { // 入力鍵材料で初期化 };
     byte salt[] = { // オプションのソルトで初期化 };
-    byte masterSecret[MAX_DIGEST_SIZE];
+    byte prk[MAX_DIGEST_SIZE];
 
-    int ret = wc_Tls13_HKDF_Extract(secret, salt, sizeof(salt), 0,
-        masterSecret, sizeof(masterSecret), WC_SHA512);
+    int ret = wc_Tls13_HKDF_Extract(prk, salt, sizeof(salt), ikm, sizeof(ikm),
+        WC_SHA512);
     if ( ret != 0 ) {
 	    // 導出鍵の生成エラー
     }
@@ -374,27 +374,27 @@ int wc_Tls13_HKDF_Extract(
     \brief この関数は、TLS v1.3鍵導出のためのRFC 5869 HMACベース抽出拡張鍵導出関数（HKDF）へのアクセスを提供します。これは、ヒープヒントとデバイス識別子を追加する_exバージョンです。
 
     \return 0 指定された入力で鍵の生成に成功した場合に返されます。
-    \return BAD_FUNC_ARG 無効なハッシュタイプが指定された場合に返されます（typeパラメータを参照）。
+    \return BAD_FUNC_ARG 無効なハッシュタイプが指定された場合（digestパラメータを参照）、prkがNULLの場合、またはikmがNULLでikmLenが0でない場合に返されます。
     \return MEMORY_E メモリ割り当てエラーがある場合に返されます。
     \return HMAC_MIN_KEYLEN_E FIPS実装を使用していて、指定された鍵の長さが最小許容FIPS標準より短い場合に返される可能性があります。
 
     \param prk 生成された疑似ランダム鍵。
-    \param salt ソルト。
+    \param salt ソルト。NULLにできます。その場合、暗号コールバックが処理しない限りsaltLenは無視されます。
     \param saltLen ソルトの長さ。
-    \param ikm 鍵材料の出力へのポインタ。
-    \param ikmLen 入力鍵材料バッファの長さ。
+    \param ikm 入力鍵材料。ikmLenが0の場合はNULLにできます。
+    \param ikmLen 入力鍵材料の長さ。0の場合、RFC 8446に従い、空の入力鍵材料ではなくダイジェスト長のゼロ値が使用されます。
     \param digest HKDFに使用するハッシュタイプ。有効なタイプは：WC_SHA256、WC_SHA384、またはWC_SHA512。
     \param heap メモリに使用するヒープヒント。NULLにできます。
     \param devId 暗号コールバックまたは非同期ハードウェアで使用するID。使用しない場合はINVALID_DEVID（-2）に設定します。
 
     _Example_
     \code
-    byte secret[] = { // ランダム鍵で初期化 };
+    byte ikm[] = { // 入力鍵材料で初期化 };
     byte salt[] = { // オプションのソルトで初期化 };
-    byte masterSecret[MAX_DIGEST_SIZE];
+    byte prk[MAX_DIGEST_SIZE];
 
-    int ret = wc_Tls13_HKDF_Extract_ex(secret, salt, sizeof(salt), 0,
-        masterSecret, sizeof(masterSecret), WC_SHA512, NULL, INVALID_DEVID);
+    int ret = wc_Tls13_HKDF_Extract_ex(prk, salt, sizeof(salt), ikm,
+        sizeof(ikm), WC_SHA512, NULL, INVALID_DEVID);
     if ( ret != 0 ) {
 	    // 導出鍵の生成エラー
     }

doc/dox_comments/header_files/hmac.h

@@ -405,27 +405,29 @@ int wc_HKDF_Expand_ex(
     key derivation
 
     \return 0 Returned upon successfully generating a key with the given inputs
-    \return BAD_FUNC_ARG Returned if an invalid hash type is given (see type param)
+    \return BAD_FUNC_ARG Returned if an invalid hash type is given (see digest
+    param), if prk is NULL, or if ikm is NULL and ikmLen is not 0
     \return MEMORY_E Returned if there is an error allocating memory
     \return HMAC_MIN_KEYLEN_E May be returned when using a FIPS implementation
     and the key length specified is shorter than the minimum acceptable FIPS
     standard
 
     \param prk     Generated pseudorandom key
-    \param salt    salt.
+    \param salt    salt. May be NULL, in which case saltLen is ignored
     \param saltLen length of the salt
-    \param ikm     pointer to putput for keying material
-    \param ikmLen  length of the input keying material buffer
+    \param ikm     input keying material. May be NULL when ikmLen is 0
+    \param ikmLen  length of the input keying material. 0 substitutes a digest
+    length zeroed IKM per RFC 8446, not an empty IKM
     \param digest  hash type to use for the HKDF. Valid types are: WC_SHA256, WC_SHA384 or WC_SHA512
 
     _Example_
     \code
-    byte secret[] = { // initialize with random key };
+    byte ikm[] = { // initialize with input keying material };
     byte salt[] = { // initialize with optional salt };
-    byte masterSecret[MAX_DIGEST_SIZE];
+    byte prk[MAX_DIGEST_SIZE];
 
-    int ret = wc_Tls13_HKDF_Extract(secret, salt, sizeof(salt), 0,
-        masterSecret, sizeof(masterSecret), WC_SHA512);
+    int ret = wc_Tls13_HKDF_Extract(prk, salt, sizeof(salt), ikm, sizeof(ikm),
+        WC_SHA512);
     if ( ret != 0 ) {
 	    // error generating derived key
     }
@@ -450,29 +452,32 @@ int wc_Tls13_HKDF_Extract(
     key derivation. This is the _ex version adding heap hint and device identifier.
 
     \return 0 Returned upon successfully generating a key with the given inputs
-    \return BAD_FUNC_ARG Returned if an invalid hash type is given (see type param)
+    \return BAD_FUNC_ARG Returned if an invalid hash type is given (see digest
+    param), if prk is NULL, or if ikm is NULL and ikmLen is not 0
     \return MEMORY_E Returned if there is an error allocating memory
     \return HMAC_MIN_KEYLEN_E May be returned when using a FIPS implementation
     and the key length specified is shorter than the minimum acceptable FIPS
     standard
 
     \param prk     Generated pseudorandom key
-    \param salt    Salt.
+    \param salt    Salt. May be NULL; saltLen is then ignored unless a crypto
+    callback handles the request
     \param saltLen Length of the salt
-    \param ikm     Pointer to output for keying material
-    \param ikmLen  Length of the input keying material buffer
+    \param ikm     Input keying material. May be NULL when ikmLen is 0
+    \param ikmLen  Length of the input keying material. 0 substitutes a digest
+    length zeroed IKM per RFC 8446, not an empty IKM
     \param digest  Hash type to use for the HKDF. Valid types are: WC_SHA256, WC_SHA384 or WC_SHA512
     \param heap    Heap hint to use for memory. Can be NULL
     \param devId   ID to use with crypto callbacks or async hardware. Set to INVALID_DEVID (-2) if not used
 
     _Example_
     \code
-    byte secret[] = { // initialize with random key };
+    byte ikm[] = { // initialize with input keying material };
     byte salt[] = { // initialize with optional salt };
-    byte masterSecret[MAX_DIGEST_SIZE];
+    byte prk[MAX_DIGEST_SIZE];
 
-    int ret = wc_Tls13_HKDF_Extract_ex(secret, salt, sizeof(salt), 0,
-        masterSecret, sizeof(masterSecret), WC_SHA512, NULL, INVALID_DEVID);
+    int ret = wc_Tls13_HKDF_Extract_ex(prk, salt, sizeof(salt), ikm,
+        sizeof(ikm), WC_SHA512, NULL, INVALID_DEVID);
     if ( ret != 0 ) {
 	    // error generating derived key
     }

tests/api/test_hmac.c

@@ -29,6 +29,7 @@
 #endif
 
 #include <wolfssl/wolfcrypt/hmac.h>
+#include <wolfssl/wolfcrypt/kdf.h>
 #include <wolfssl/wolfcrypt/types.h>
 #include <wolfssl/internal.h>
 #ifdef WOLF_CRYPTO_CB
@@ -1057,3 +1058,148 @@ int test_wc_HKDF_NullKeyEdgeCases(void)
 #endif
     return EXPECT_RESULT();
 } /* END test_wc_HKDF_NullKeyEdgeCases */
+
+#if defined(HAVE_HKDF) && !defined(NO_HMAC) && !defined(NO_KDF) && \
+    !defined(NO_SHA256) && !defined(HAVE_SELFTEST) && !defined(HAVE_FIPS) && \
+    defined(WOLF_CRYPTO_CB)
+#define TEST_KDF_CRYPTOCB_DEVID 0x4b444658 /* "KDFX" */
+
+typedef struct {
+    int    called;
+    int    inKeyNull;
+    int    inKeyZeroed;
+    word32 inKeySz;
+} TestKdfExtractCbCtx;
+
+/* Records the IKM handed to a callback, then declines so the software path
+ * still derives the PRK. */
+static int test_kdf_cryptocb_extract_cb(int cbDevId, wc_CryptoInfo* info,
+    void* ctx)
+{
+    TestKdfExtractCbCtx* cbCtx = (TestKdfExtractCbCtx*)ctx;
+    word32 j;
+
+    (void)cbDevId;
+    if (info->algo_type == WC_ALGO_TYPE_KDF &&
+            info->kdf.type == WC_KDF_TYPE_HKDF_EXTRACT) {
+        cbCtx->called++;
+        cbCtx->inKeySz = info->kdf.hkdf_extract.inKeySz;
+        cbCtx->inKeyNull = (info->kdf.hkdf_extract.inKey == NULL);
+        cbCtx->inKeyZeroed = 1;
+        for (j = 0; !cbCtx->inKeyNull && j < info->kdf.hkdf_extract.inKeySz;
+                j++) {
+            if (info->kdf.hkdf_extract.inKey[j] != 0) {
+                cbCtx->inKeyZeroed = 0;
+            }
+        }
+    }
+    return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE);
+}
+#endif
+
+/* A zero length IKM must not write into the caller's ikm buffer, whose
+ * required size is not implied by the length the caller passes in. */
+int test_wc_Tls13_HKDF_Extract_ZeroLenIkm(void)
+{
+    EXPECT_DECLS;
+/* kdf.c sits inside the FIPS module boundary, so FIPS and selftest builds use a
+ * frozen copy that still writes into the caller's ikm buffer. Skip until a
+ * module revision carrying this fix ships. */
+#if defined(HAVE_HKDF) && !defined(NO_HMAC) && !defined(NO_KDF) && \
+    !defined(NO_SHA256) && !defined(HAVE_SELFTEST) && !defined(HAVE_FIPS)
+    static const int digests[] = {
+        WC_SHA256,
+    #ifdef WOLFSSL_SHA384
+        WC_SHA384,
+    #endif
+    #ifdef WOLFSSL_TLS13_SHA512
+        WC_SHA512,
+    #endif
+    #ifdef WOLFSSL_SM3
+        WC_SM3,
+    #endif
+    };
+    byte prkNull[WC_MAX_DIGEST_SIZE];
+    byte prkOther[WC_MAX_DIGEST_SIZE];
+    byte zeros[WC_MAX_DIGEST_SIZE];
+    byte sentinel[WC_MAX_DIGEST_SIZE];
+    byte filled[WC_MAX_DIGEST_SIZE];
+    byte small[4];
+    word32 i;
+    int len = 0;
+#ifdef WOLF_CRYPTO_CB
+    TestKdfExtractCbCtx cbCtx;
+#endif
+
+    XMEMSET(zeros, 0, sizeof(zeros));
+    XMEMSET(filled, 0xA5, sizeof(filled));
+
+    /* Every digest the switch accepts, up to the WC_MAX_DIGEST_SIZE the scratch
+     * buffer is sized against. */
+    for (i = 0; i < sizeof(digests) / sizeof(digests[0]); i++) {
+        ExpectIntGT(len = wc_HmacSizeByType(digests[i]), 0);
+
+        /* ikmLen 0 feeds a digest length zeroed IKM rather than an empty one,
+         * so a NULL ikm must derive the same PRK as passing those zeros. */
+        ExpectIntEQ(wc_Tls13_HKDF_Extract(prkNull, NULL, 0, NULL, 0,
+            digests[i]), 0);
+        ExpectIntEQ(wc_Tls13_HKDF_Extract(prkOther, NULL, 0, zeros,
+            (word32)len, digests[i]), 0);
+        ExpectBufEQ(prkNull, prkOther, (word32)len);
+
+        /* The caller's buffer must come back untouched, through the _ex entry
+         * point that TLS 1.3 itself calls. */
+        XMEMSET(sentinel, 0xA5, sizeof(sentinel));
+        ExpectIntEQ(wc_Tls13_HKDF_Extract_ex(prkOther, NULL, 0, sentinel, 0,
+            digests[i], NULL, INVALID_DEVID), 0);
+        ExpectBufEQ(sentinel, filled, (word32)len);
+        ExpectBufEQ(prkNull, prkOther, (word32)len);
+
+        XMEMSET(sentinel, 0xA5, sizeof(sentinel));
+        ExpectIntEQ(wc_Tls13_HKDF_Extract(prkOther, NULL, 0, sentinel, 0,
+            digests[i]), 0);
+        ExpectBufEQ(sentinel, filled, (word32)len);
+        ExpectBufEQ(prkNull, prkOther, (word32)len);
+
+        /* A NULL salt stays valid whatever saltLen says. */
+        ExpectIntEQ(wc_Tls13_HKDF_Extract(prkOther, NULL, 5, NULL, 0,
+            digests[i]), 0);
+        ExpectBufEQ(prkNull, prkOther, (word32)len);
+    }
+
+    /* ASan tripwire: a revert overruns this and may abort the binary under a
+     * stack protector. The sentinel above is what fails cleanly. */
+    XMEMSET(small, 0xA5, sizeof(small));
+    ExpectIntEQ(wc_Tls13_HKDF_Extract(prkNull, NULL, 0, small, 0, WC_SHA256), 0);
+    ExpectIntEQ(small[0], 0xA5);
+    ExpectIntEQ(small[3], 0xA5);
+
+    ExpectIntEQ(wc_Tls13_HKDF_Extract(NULL, NULL, 0, zeros,
+        WC_SHA256_DIGEST_SIZE, WC_SHA256), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    ExpectIntEQ(wc_Tls13_HKDF_Extract(prkNull, NULL, 0, NULL, 5, WC_SHA256),
+        WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    ExpectIntEQ(wc_Tls13_HKDF_Extract_ex(NULL, NULL, 0, zeros,
+        WC_SHA256_DIGEST_SIZE, WC_SHA256, NULL, INVALID_DEVID),
+        WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    ExpectIntEQ(wc_Tls13_HKDF_Extract_ex(prkNull, NULL, 0, NULL, 5, WC_SHA256,
+        NULL, INVALID_DEVID), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+#ifdef WOLF_CRYPTO_CB
+    /* A callback dispatches before the software path, so it must receive the
+     * substituted zeroed IKM rather than the caller's untouched buffer. */
+    XMEMSET(&cbCtx, 0, sizeof(cbCtx));
+    XMEMSET(sentinel, 0xA5, sizeof(sentinel));
+    ExpectIntEQ(wc_CryptoCb_RegisterDevice(TEST_KDF_CRYPTOCB_DEVID,
+        test_kdf_cryptocb_extract_cb, &cbCtx), 0);
+    ExpectIntEQ(wc_Tls13_HKDF_Extract_ex(prkOther, NULL, 0, sentinel, 0,
+        WC_SHA256, NULL, TEST_KDF_CRYPTOCB_DEVID), 0);
+    ExpectIntEQ(cbCtx.called, 1);
+    ExpectIntEQ(cbCtx.inKeyNull, 0);
+    ExpectIntEQ(cbCtx.inKeyZeroed, 1);
+    ExpectIntEQ(cbCtx.inKeySz, WC_SHA256_DIGEST_SIZE);
+    ExpectBufEQ(sentinel, filled, WC_SHA256_DIGEST_SIZE);
+    wc_CryptoCb_UnRegisterDevice(TEST_KDF_CRYPTOCB_DEVID);
+#endif
+#endif
+    return EXPECT_RESULT();
+} /* END test_wc_Tls13_HKDF_Extract_ZeroLenIkm */

tests/api/test_hmac.h

@@ -46,6 +46,7 @@ int test_wc_HmacInit_Id(void);
 int test_wc_HmacInit_Label(void);
 int test_wc_HmacFree_CryptoCb(void);
 int test_wc_HKDF_NullKeyEdgeCases(void);
+int test_wc_Tls13_HKDF_Extract_ZeroLenIkm(void);
 
 #define TEST_HMAC_DECLS                                 \
     TEST_DECL_GROUP("hmac", test_wc_Md5HmacSetKey),     \
@@ -69,6 +70,7 @@ int test_wc_HKDF_NullKeyEdgeCases(void);
     TEST_DECL_GROUP("hmac", test_wc_HmacInit_Id),       \
     TEST_DECL_GROUP("hmac", test_wc_HmacInit_Label),    \
     TEST_DECL_GROUP("hmac", test_wc_HmacFree_CryptoCb), \
-    TEST_DECL_GROUP("hmac", test_wc_HKDF_NullKeyEdgeCases)
+    TEST_DECL_GROUP("hmac", test_wc_HKDF_NullKeyEdgeCases), \
+    TEST_DECL_GROUP("hmac", test_wc_Tls13_HKDF_Extract_ZeroLenIkm)
 
 #endif /* WOLFCRYPT_TEST_HMAC_H */

wolfcrypt/src/kdf.c

@@ -348,9 +348,15 @@ int wc_PRF_TLS(byte* digest, word32 digLen, const byte* secret, word32 secLen,
     int wc_Tls13_HKDF_Extract_ex(byte* prk, const byte* salt, word32 saltLen,
         byte* ikm, word32 ikmLen, int digest, void* heap, int devId)
     {
-        int ret;
+        byte   tmp[WC_MAX_DIGEST_SIZE]; /* localIkm helper */
+        const  byte* localIkm;  /* either points to user input or tmp */
+        int    ret;
         word32 len = 0;
 
+        if (prk == NULL || (ikm == NULL && ikmLen > 0)) {
+            return BAD_FUNC_ARG;
+        }
+
         switch (digest) {
             #ifndef NO_SHA256
             case WC_SHA256:
@@ -380,25 +386,28 @@ int wc_PRF_TLS(byte* digest, word32 digLen, const byte* secret, word32 secLen,
                 return BAD_FUNC_ARG;
         }
 
-        /* When length is 0 then use zeroed data of digest length. */
+        /* When length is 0 then use zeroed data of digest length. The caller's
+         * buffer is not sized for this, so use a local one. */
+        localIkm = ikm;
         if (ikmLen == 0) {
+            XMEMSET(tmp, 0, len);
+            localIkm = tmp;
             ikmLen = len;
-            XMEMSET(ikm, 0, len);
         }
 
 #ifdef WOLFSSL_DEBUG_TLS
         WOLFSSL_MSG("  Salt");
         WOLFSSL_BUFFER(salt, saltLen);
         WOLFSSL_MSG("  IKM");
-        WOLFSSL_BUFFER(ikm, ikmLen);
+        WOLFSSL_BUFFER(localIkm, ikmLen);
 #endif
 
 #if !defined(HAVE_SELFTEST) && (!defined(HAVE_FIPS) || \
     (defined(FIPS_VERSION_GE) && FIPS_VERSION_GE(5,3)))
-        ret = wc_HKDF_Extract_ex(digest, salt, saltLen, ikm, ikmLen, prk, heap,
-            devId);
+        ret = wc_HKDF_Extract_ex(digest, salt, saltLen, localIkm, ikmLen, prk,
+            heap, devId);
 #else
-        ret = wc_HKDF_Extract(digest, salt, saltLen, ikm, ikmLen, prk);
+        ret = wc_HKDF_Extract(digest, salt, saltLen, localIkm, ikmLen, prk);
         (void)heap;
         (void)devId;
 #endif