Bound keywrap client requests against the comm data buffer

docs/src/9-Configuration.md

@@ -73,8 +73,8 @@ These macros enable or tune optional cryptographic subsystems built on top of wo
 |---|---|---|
 | `WOLFHSM_CFG_SHE_EXTENSION` | Undefined | If defined, compile the AUTOSAR SHE subsystem (SHE message types, SHE key slots, M1-M5 update protocol, SHE-specific RNG and SREG handling). Requires wolfCrypt built with AES, `WOLFSSL_CMAC`, `WOLFSSL_AES_DIRECT`, and `HAVE_AES_ECB`. |
 | `WOLFHSM_CFG_KEYWRAP` | Undefined | If defined, compile the key-wrap subsystem (`wh_Client_KeyWrap*` / server counterparts). Uses AES-GCM internally and therefore requires wolfCrypt built with AES and `HAVE_AESGCM`. Incompatible with `WOLFHSM_CFG_NO_CRYPTO`. |
-| `WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE` | `2000` | Maximum size, in bytes, of a key that can be wrapped or unwrapped in a single operation. Only consulted when `WOLFHSM_CFG_KEYWRAP` is defined. |
-| `WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE` | `2000` | Maximum size, in bytes, of the plaintext or wrapped payload carried by a single key-wrap request. Only consulted when `WOLFHSM_CFG_KEYWRAP` is defined. |
+| `WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE` | `2000`, or `WOLFHSM_CFG_COMM_DATA_LEN` minus the request overhead when that is smaller | Maximum size, in bytes, of a key that can be wrapped or unwrapped in a single operation. Only consulted when `WOLFHSM_CFG_KEYWRAP` is defined. The key plus its request header and metadata must fit within `WOLFHSM_CFG_COMM_DATA_LEN`; the default is derived so that it always does, and an explicit value that does not fit fails to build. |
+| `WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE` | `2000`, or `WOLFHSM_CFG_COMM_DATA_LEN` minus the request overhead when that is smaller | Maximum size, in bytes, of the plaintext or wrapped payload carried by a single key-wrap request. Only consulted when `WOLFHSM_CFG_KEYWRAP` is defined. The payload plus its request header must fit within `WOLFHSM_CFG_COMM_DATA_LEN`; the default is derived so that it always does, and an explicit value that does not fit fails to build. |
 | `WOLFHSM_CFG_HWKEYSTORE` | Undefined | If defined, compile the hardware keystore front-end (`wh_HwKeystore_*`) and hardware-only key support (`WH_KEYTYPE_HW`, `WH_CLIENT_KEYID_MAKE_HW()`). Hardware-only keys are served on demand by a user-supplied callback and are usable only as keywrap KEKs; they never enter the key cache or NVM and are never exported. See [Hardware-Only Keys](5-Features.md#hardware-only-keys). |
 | `WOLFHSM_CFG_HWKEYSTORE_MAX_KEY_SIZE` | `32` | Maximum size, in bytes, of a key served by the hardware keystore backend; sizes the local buffer that holds a hardware KEK for the duration of a keywrap operation. Only consulted when `WOLFHSM_CFG_HWKEYSTORE` is defined. |
 | `WOLFHSM_CFG_GLOBAL_KEYS` | Undefined | If defined, enable the global-keys feature, allowing keys to be cached so that they are visible to every client rather than scoped to the caching client. See [Global Keys](5-Features.md#global-keys) for a full discussion of the API and security implications. |

src/wh_client_keywrap.c

@@ -26,7 +26,10 @@ int wh_Client_KeyWrapRequest(whClientContext*   ctx,
         return WH_ERROR_BADARGS;
     }
 
-    if (keySz == 0 || keySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE) {
+    /* Bound the whole wire length before copying into the comm data buffer */
+    if (keySz == 0 || keySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE ||
+        (size_t)sizeof(*req) + sizeof(*metadata) + keySz >
+            WOLFHSM_CFG_COMM_DATA_LEN) {
         return WH_ERROR_BADARGS;
     }
 
@@ -249,7 +252,9 @@ int wh_Client_KeyUnwrapAndExportRequest(whClientContext*   ctx,
         return WH_ERROR_BADARGS;
     }
 
-    if (wrappedKeySz == 0 || wrappedKeySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE) {
+    /* Bound the whole wire length before copying into the comm data buffer */
+    if (wrappedKeySz == 0 || wrappedKeySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE ||
+        (size_t)sizeof(*req) + wrappedKeySz > WOLFHSM_CFG_COMM_DATA_LEN) {
         return WH_ERROR_BADARGS;
     }
 
@@ -371,7 +376,9 @@ int wh_Client_KeyUnwrapAndCacheRequest(whClientContext*   ctx,
     if (ctx == NULL || wrappedKeyIn == NULL)
         return WH_ERROR_BADARGS;
 
-    if (wrappedKeySz == 0 || wrappedKeySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE) {
+    /* Bound the whole wire length before copying into the comm data buffer */
+    if (wrappedKeySz == 0 || wrappedKeySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE ||
+        (size_t)sizeof(*req) + wrappedKeySz > WOLFHSM_CFG_COMM_DATA_LEN) {
         return WH_ERROR_BADARGS;
     }
 
@@ -477,7 +484,9 @@ int wh_Client_DataWrapRequest(whClientContext*   ctx,
         return WH_ERROR_BADARGS;
     }
 
-    if (dataInSz == 0 || dataInSz > WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE) {
+    /* Bound the whole wire length before copying into the comm data buffer */
+    if (dataInSz == 0 || dataInSz > WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE ||
+        (size_t)sizeof(*req) + dataInSz > WOLFHSM_CFG_COMM_DATA_LEN) {
         return WH_ERROR_BADARGS;
     }
 
@@ -590,8 +599,10 @@ int wh_Client_DataUnwrapRequest(whClientContext*   ctx,
         return WH_ERROR_BADARGS;
     }
 
+    /* Bound the whole wire length before copying into the comm data buffer */
     if (wrappedDataInSz == 0 ||
-        wrappedDataInSz > WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE) {
+        wrappedDataInSz > WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE ||
+        (size_t)sizeof(*req) + wrappedDataInSz > WOLFHSM_CFG_COMM_DATA_LEN) {
         return WH_ERROR_BADARGS;
     }
 

test-refactor/client-server/wh_test_keywrap.c

@@ -31,6 +31,8 @@
  *                                    return WH_ERROR_BADARGS, not underflow
  *   _whTest_KeywrapDataUnwrapUnderflow - undersized wrapped-data blobs must
  *                                    return WH_ERROR_BADARGS, not underflow
+ *   _whTest_KeywrapOversizeRequest - payloads past the configured maximum must
+ *                                    be refused before any copy
  *
  * The positive wrap/unwrap-and-export round trip under a plain client KEK
  * lives in wh_test_crypto_keywrap.c. The trusted-KEK positive paths
@@ -56,6 +58,7 @@
 #include "wolfhsm/wh_keyid.h"
 #include "wolfhsm/wh_client.h"
 #include "wolfhsm/wh_client_crypto.h"
+#include "wolfhsm/wh_message_keystore.h"
 
 #include "wh_test_common.h"
 #include "wh_test_list.h"
@@ -65,6 +68,11 @@
     (WH_KEYWRAP_AES_GCM_HEADER_SIZE + WH_TEST_KW_KEYSIZE + \
      sizeof(whNvmMetadata))
 
+/* One byte past the configured maximum. The comm data buffer always has room
+ * for the maximum plus its header, so this is the binding limit */
+#define WH_TEST_KW_OVER_KEY (WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE + 1)
+#define WH_TEST_KW_OVER_DATA (WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE + 1)
+
 /* Distinct id range so nothing collides with other client-group suites; every
  * subtest cleans up its own keys */
 #define WH_TEST_KW_SWKEK_ID 0x60
@@ -414,12 +422,77 @@ static int _whTest_KeywrapDataUnwrapUnderflow(whClientContext* client)
     return ret;
 }
 
+/* Report whether an oversize request was rejected as expected */
+static int _CheckOversizeRejected(const char* what, int ret)
+{
+    if (ret != WH_ERROR_BADARGS) {
+        WH_ERROR_PRINT("%s oversize expected BADARGS, got %d\n", what, ret);
+        return WH_ERROR_ABORTED;
+    }
+    return WH_ERROR_OK;
+}
+
+/* A payload past the configured maximum must be refused before any copy. The
+ * input buffer stays small on purpose: a builder that copied first would read
+ * off the end of it and trip ASan */
+static int _whTest_KeywrapOversizeRequest(whClientContext* client)
+{
+    int           ret                             = WH_ERROR_OK;
+    whKeyId       kekId                           = WH_KEYID_ERASED;
+    whNvmMetadata meta                            = {0};
+    uint16_t      cachedId                        = WH_KEYID_ERASED;
+    uint8_t       in[WH_TEST_KW_KEYSIZE]          = {0};
+    uint8_t       out[WH_TEST_KW_WRAPPED_KEYSIZE] = {0};
+    uint16_t      keyOutSz                        = (uint16_t)sizeof(out);
+    uint32_t      dataOutSz                       = (uint32_t)sizeof(out);
+
+    WH_TEST_RETURN_ON_FAIL(_CacheSwKek(client, &kekId));
+
+    ret = _CheckOversizeRejected(
+        "KeyWrap", wh_Client_KeyWrap(client, WC_CIPHER_AES_GCM, kekId, in,
+                                     (uint16_t)WH_TEST_KW_OVER_KEY, &meta, out,
+                                     &keyOutSz));
+
+    if (ret == WH_ERROR_OK) {
+        ret = _CheckOversizeRejected(
+            "KeyUnwrapAndExport",
+            wh_Client_KeyUnwrapAndExport(client, WC_CIPHER_AES_GCM, kekId, in,
+                                         (uint16_t)WH_TEST_KW_OVER_KEY, &meta,
+                                         out, &keyOutSz));
+    }
+
+    if (ret == WH_ERROR_OK) {
+        ret = _CheckOversizeRejected(
+            "KeyUnwrapAndCache", wh_Client_KeyUnwrapAndCache(
+                                     client, WC_CIPHER_AES_GCM, kekId, in,
+                                     (uint16_t)WH_TEST_KW_OVER_KEY, &cachedId));
+    }
+
+    if (ret == WH_ERROR_OK) {
+        ret = _CheckOversizeRejected(
+            "DataWrap",
+            wh_Client_DataWrap(client, WC_CIPHER_AES_GCM, kekId, in,
+                               WH_TEST_KW_OVER_DATA, out, &dataOutSz));
+    }
+
+    if (ret == WH_ERROR_OK) {
+        ret = _CheckOversizeRejected(
+            "DataUnwrap",
+            wh_Client_DataUnwrap(client, WC_CIPHER_AES_GCM, kekId, in,
+                                 WH_TEST_KW_OVER_DATA, out, &dataOutSz));
+    }
+
+    (void)wh_Client_KeyEvict(client, kekId);
+    return ret;
+}
+
 int whTest_KeyWrap(whClientContext* ctx)
 {
     WH_TEST_RETURN_ON_FAIL(_whTest_KeywrapTrustedKekPolicy(ctx));
     WH_TEST_RETURN_ON_FAIL(_whTest_KeywrapDataWrapUsage(ctx));
     WH_TEST_RETURN_ON_FAIL(_whTest_KeywrapKeyUnwrapUnderflow(ctx));
     WH_TEST_RETURN_ON_FAIL(_whTest_KeywrapDataUnwrapUnderflow(ctx));
+    WH_TEST_RETURN_ON_FAIL(_whTest_KeywrapOversizeRequest(ctx));
 
     WH_TEST_PRINT("KEYWRAP POLICY SUCCESS\n");
     return 0;

wolfhsm/wh_message_keystore.h

@@ -31,6 +31,7 @@
 #include <stdint.h>
 
 #include "wolfhsm/wh_common.h"
+#include "wolfhsm/wh_utils.h"
 
 /* Key Cache Request */
 typedef struct {
@@ -512,4 +513,45 @@ int wh_MessageKeystore_TranslateDataUnwrapResponse(
     uint16_t magic, const whMessageKeystore_DataUnwrapResponse* src,
     whMessageKeystore_DataUnwrapResponse* dest);
 
+#if defined(WOLFHSM_CFG_KEYWRAP)
+/* A maximum-sized keywrap payload plus its header must fit the comm data
+ * buffer. On failure, raise COMM_DATA_LEN or lower the keywrap maximum */
+WH_UTILS_STATIC_ASSERT(
+    (uint32_t)sizeof(whMessageKeystore_KeyWrapRequest) +
+            (uint32_t)sizeof(whNvmMetadata) +
+            (uint32_t)WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE <=
+        (uint32_t)WOLFHSM_CFG_COMM_DATA_LEN,
+    "WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE too large for WOLFHSM_CFG_COMM_DATA_LEN");
+
+WH_UTILS_STATIC_ASSERT(
+    (uint32_t)sizeof(whMessageKeystore_KeyUnwrapAndExportRequest) +
+            (uint32_t)WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE <=
+        (uint32_t)WOLFHSM_CFG_COMM_DATA_LEN,
+    "WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE too large for WOLFHSM_CFG_COMM_DATA_LEN");
+
+WH_UTILS_STATIC_ASSERT(
+    (uint32_t)sizeof(whMessageKeystore_KeyUnwrapAndCacheRequest) +
+            (uint32_t)WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE <=
+        (uint32_t)WOLFHSM_CFG_COMM_DATA_LEN,
+    "WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE too large for WOLFHSM_CFG_COMM_DATA_LEN");
+
+WH_UTILS_STATIC_ASSERT((uint32_t)sizeof(whMessageKeystore_DataWrapRequest) +
+                               (uint32_t)WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE <=
+                           (uint32_t)WOLFHSM_CFG_COMM_DATA_LEN,
+                       "WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE too large for "
+                       "WOLFHSM_CFG_COMM_DATA_LEN");
+
+WH_UTILS_STATIC_ASSERT((uint32_t)sizeof(whMessageKeystore_DataUnwrapRequest) +
+                               (uint32_t)WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE <=
+                           (uint32_t)WOLFHSM_CFG_COMM_DATA_LEN,
+                       "WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE too large for "
+                       "WOLFHSM_CFG_COMM_DATA_LEN");
+
+/* The defaults derive from this overhead, so it must cover the widest header */
+WH_UTILS_STATIC_ASSERT((uint32_t)sizeof(whMessageKeystore_KeyWrapRequest) +
+                               (uint32_t)sizeof(whNvmMetadata) <=
+                           (uint32_t)WH_KEYWRAP_MAX_REQ_OVERHEAD,
+                       "WH_KEYWRAP_MAX_REQ_OVERHEAD is too small");
+#endif /* WOLFHSM_CFG_KEYWRAP */
+
 #endif /* !WOLFHSM_WH_MESSAGE_KEYSTORE_H_ */

wolfhsm/wh_settings.h

@@ -51,8 +51,14 @@
  *      Default: Not defined
  *
  *  WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE - The maximum size (in bytes) of a key that
- *  can be wrapped
- *      Default: 512
+ *  can be wrapped. Together with the request header it must fit within
+ *  WOLFHSM_CFG_COMM_DATA_LEN, which is checked at compile time
+ *      Default: 2000, or what WOLFHSM_CFG_COMM_DATA_LEN leaves when smaller
+ *
+ *  WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE - The maximum size (in bytes) of a data
+ *  payload that can be wrapped, bounded by WOLFHSM_CFG_COMM_DATA_LEN the same
+ *  way as WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE
+ *      Default: 2000, or what WOLFHSM_CFG_COMM_DATA_LEN leaves when smaller
  *
  *  WOLFHSM_CFG_HWKEYSTORE - If defined, include the hardware keystore
  *  front-end module and hardware-only key (WH_KEYTYPE_HW) support
@@ -233,6 +239,37 @@
 #define WOLFHSM_CFG_COMM_DATA_LEN 1280
 #endif
 
+/* Maximum keywrap key and data sizes, defaulted to fit the comm data buffer
+ * after the largest request header (asserted in the message header) */
+#if defined(WOLFHSM_CFG_KEYWRAP)
+
+/* Widest header is 38 bytes, rounded up to the next 8-byte boundary */
+#define WH_KEYWRAP_MAX_REQ_OVERHEAD 40
+
+#if WOLFHSM_CFG_COMM_DATA_LEN <= WH_KEYWRAP_MAX_REQ_OVERHEAD
+#error "WOLFHSM_CFG_COMM_DATA_LEN is too small to carry a keywrap request"
+#endif
+
+#ifndef WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE
+#if (WOLFHSM_CFG_COMM_DATA_LEN - WH_KEYWRAP_MAX_REQ_OVERHEAD) < 2000
+#define WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE \
+    (WOLFHSM_CFG_COMM_DATA_LEN - WH_KEYWRAP_MAX_REQ_OVERHEAD)
+#else
+#define WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE 2000
+#endif
+#endif
+
+#ifndef WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE
+#if (WOLFHSM_CFG_COMM_DATA_LEN - WH_KEYWRAP_MAX_REQ_OVERHEAD) < 2000
+#define WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE \
+    (WOLFHSM_CFG_COMM_DATA_LEN - WH_KEYWRAP_MAX_REQ_OVERHEAD)
+#else
+#define WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE 2000
+#endif
+#endif
+
+#endif /* WOLFHSM_CFG_KEYWRAP */
+
 /** Default server resource configurations */
 /* Reported version string */
 #ifndef WOLFHSM_CFG_INFOVERSION
@@ -462,14 +499,6 @@
 
 #if defined(WOLFHSM_CFG_KEYWRAP)
 
-#ifndef WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE
-#define WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE 2000
-#endif
-
-#ifndef WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE
-#define WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE 2000
-#endif
-
 #if defined(NO_AES) || !defined(HAVE_AESGCM)
 #error \
     "WOLFHSM_CFG_KEYWRAP requires NO_AES to be undefined and HAVE_AESGCM to be defined"