More fenrir/skoll review feedback

Author: dgarske

Makefile.am

@@ -28,7 +28,10 @@ DISTCLEANFILES+= aminclude.am
 # make sure we pass the correct flags to distcheck
 # SWTPM_PORT can be set via --with-swtpm-port during configure
 # Use @SWTPM_PORT@ substitution from configure.ac
-AM_DISTCHECK_CONFIGURE_FLAGS = --enable-swtpm @DISTCHECK_SWTPM_PORT_FLAG@
+# Explicitly disable fwtpm during distcheck — fwtpm_server requires
+# optional wolfSSL features (WOLFSSL_KEY_GEN, WC_RSA_NO_PADDING) that
+# distcheck runners can't guarantee. Restores pre-auto-enable behavior.
+AM_DISTCHECK_CONFIGURE_FLAGS = --enable-swtpm --disable-fwtpm @DISTCHECK_SWTPM_PORT_FLAG@
 
 exampledir = $(docdir)/example
 dist_example_DATA=

configure.ac

@@ -311,6 +311,31 @@ then
     then
         AC_MSG_ERROR([fwTPM requires wolfCrypt. Do not use --disable-wolfcrypt with --enable-fwtpm.])
     fi
+    # Probe wolfSSL for optional RSA features required by fwTPM's RSA key
+    # paths. These aren't hard errors — ECC-only fwTPM works without them —
+    # so emit warnings rather than failing configure.
+    AC_MSG_CHECKING([wolfSSL for WOLFSSL_KEY_GEN (needed for RSA CreatePrimary)])
+    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+        #include <wolfssl/options.h>
+        #ifndef WOLFSSL_KEY_GEN
+        #error WOLFSSL_KEY_GEN not defined
+        #endif
+        int main(void){return 0;}
+    ]])],
+    [AC_MSG_RESULT([yes])],
+    [AC_MSG_RESULT([no])
+     AC_MSG_WARN([fwTPM: wolfSSL lacks WOLFSSL_KEY_GEN — RSA CreatePrimary will fail at runtime. Rebuild wolfSSL with --enable-keygen or use --disable-fwtpm.])])
+    AC_MSG_CHECKING([wolfSSL for WC_RSA_NO_PADDING (needed for RSA_Encrypt/Decrypt)])
+    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+        #include <wolfssl/options.h>
+        #ifndef WC_RSA_NO_PADDING
+        #error WC_RSA_NO_PADDING not defined
+        #endif
+        int main(void){return 0;}
+    ]])],
+    [AC_MSG_RESULT([yes])],
+    [AC_MSG_RESULT([no])
+     AC_MSG_WARN([fwTPM: wolfSSL lacks WC_RSA_NO_PADDING — raw RSA operations will return TPM_RC_SCHEME. Rebuild wolfSSL with CFLAGS="-DWC_RSA_NO_PADDING".])])
     # WOLFTPM_FWTPM is added to options.h (via OPTION_FLAGS) but NOT to AM_CFLAGS.
     # It gates server-specific code in tpm2_packet.c/tpm2_param_enc.c and is set
     # as a compile flag only for the fwtpm_server target in src/fwtpm/include.am.

src/tpm2_crypto.c

@@ -329,7 +329,7 @@ int TPM2_KDFe(
 
 #ifndef WOLFTPM2_NO_WOLFCRYPT
 
-#ifndef NO_AES
+#if !defined(NO_AES) && defined(WOLFSSL_AES_CFB)
 
 int TPM2_AesCfbEncrypt(
     const byte* key, int keySz,
@@ -359,6 +359,8 @@ int TPM2_AesCfbEncrypt(
             rc = wc_AesCfbEncrypt(&aes, data, data, dataSz);
         }
         wc_AesFree(&aes);
+        /* Wipe residual key material from the stack Aes object. */
+        TPM2_ForceZero(&aes, sizeof(aes));
     }
     return rc;
 }
@@ -391,11 +393,13 @@ int TPM2_AesCfbDecrypt(
             rc = wc_AesCfbDecrypt(&aes, data, data, dataSz);
         }
         wc_AesFree(&aes);
+        /* Wipe residual key material from the stack Aes object. */
+        TPM2_ForceZero(&aes, sizeof(aes));
     }
     return rc;
 }
 
-#endif /* !NO_AES */
+#endif /* !NO_AES && WOLFSSL_AES_CFB */
 
 #ifndef NO_HMAC
 
@@ -441,6 +445,8 @@ int TPM2_HmacCompute(
             rc = wc_HmacFinal(&hmac, digest);
         }
         wc_HmacFree(&hmac);
+        /* Wipe residual key material from the stack Hmac object. */
+        TPM2_ForceZero(&hmac, sizeof(hmac));
     }
     if (rc == 0 && digestSz != NULL) {
         *digestSz = (word32)dSz;

src/tpm2_param_enc.c

@@ -57,9 +57,11 @@
 /* --- Param Enc/Dec Functions -- */
 /******************************************************************************/
 
-/* Maximum XOR mask size: RSA 2048 private key blob is ~1250 bytes */
+/* Maximum XOR mask size. RSA-2048 inSensitive parameter blobs on Create can
+ * exceed MAX_DIGEST_BUFFER (1024), so leave headroom to ~1250 bytes. Keep
+ * stack usage bounded by switching to heap under WOLFTPM_SMALL_STACK. */
 #ifndef TPM2_XOR_MASK_MAX
-#define TPM2_XOR_MASK_MAX 1500
+#define TPM2_XOR_MASK_MAX 1280
 #endif
 
 /* XOR parameter encryption/decryption (shared by client and fwTPM).
@@ -73,14 +75,23 @@ int TPM2_ParamEnc_XOR(
     BYTE *paramData, UINT32 paramSz)
 {
     int rc;
-    BYTE mask[TPM2_XOR_MASK_MAX];
     UINT32 i;
+#ifdef WOLFTPM_SMALL_STACK
+    BYTE *mask = (BYTE*)XMALLOC(TPM2_XOR_MASK_MAX, NULL,
+        DYNAMIC_TYPE_TMP_BUFFER);
+    if (mask == NULL) {
+        return MEMORY_E;
+    }
+#else
+    BYTE mask[TPM2_XOR_MASK_MAX];
+#endif
 
-    if (paramSz > sizeof(mask)) {
-        return BUFFER_E;
+    if (paramSz > TPM2_XOR_MASK_MAX) {
+        rc = BUFFER_E;
+        goto out;
     }
 
-    XMEMSET(mask, 0, sizeof(mask));
+    XMEMSET(mask, 0, TPM2_XOR_MASK_MAX);
     rc = TPM2_KDFa_ex(authHash, keyIn, keyInSz, "XOR",
         nonceA, nonceASz, nonceB, nonceBSz, mask, paramSz);
     if ((UINT32)rc == paramSz) {
@@ -96,7 +107,11 @@ int TPM2_ParamEnc_XOR(
         rc = TPM_RC_FAILURE;
     }
 
-    TPM2_ForceZero(mask, sizeof(mask));
+    TPM2_ForceZero(mask, TPM2_XOR_MASK_MAX);
+out:
+#ifdef WOLFTPM_SMALL_STACK
+    XFREE(mask, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+#endif
     return rc;
 }
 

src/tpm2_swtpm.c

@@ -65,12 +65,18 @@
 #include <fcntl.h>
 #include <termios.h>
 #include <sys/stat.h>
+#include <time.h>
 #ifndef O_CLOEXEC
 #define O_CLOEXEC 0
 #endif
 #ifndef O_NOFOLLOW
 #define O_NOFOLLOW 0
 #endif
+/* Cumulative wall-clock timeout for SwTpmReceive. Defends against trickle
+ * senders that repeatedly reset the per-read VMIN/VTIME timer. */
+#ifndef WOLFTPM_SWTPM_UART_RX_TIMEOUT_SECS
+#define WOLFTPM_SWTPM_UART_RX_TIMEOUT_SECS 30
+#endif
 #endif
 
 #include <wolftpm/tpm2_socket.h>
@@ -133,6 +139,13 @@ static TPM_RC SwTpmReceive(TPM2_CTX* ctx, void* buffer, size_t rxSz)
     ssize_t wrc = 0;
     size_t bytes_remaining = rxSz;
     char* ptr = (char*)buffer;
+#if defined(WOLFTPM_SWTPM_UART) && defined(CLOCK_MONOTONIC)
+    struct timespec start, now;
+    int haveStart = 0;
+    if (clock_gettime(CLOCK_MONOTONIC, &start) == 0) {
+        haveStart = 1;
+    }
+#endif
 
     if (ctx == NULL || ctx->tcpCtx.fd < 0 || buffer == NULL) {
         return BAD_FUNC_ARG;
@@ -161,6 +174,23 @@ static TPM_RC SwTpmReceive(TPM2_CTX* ctx, void* buffer, size_t rxSz)
         printf("TPM socket received %zd waiting for %zu more\n",
                wrc, bytes_remaining);
         #endif
+
+#if defined(WOLFTPM_SWTPM_UART) && defined(CLOCK_MONOTONIC)
+        /* Enforce cumulative timeout so a trickle sender cannot reset
+         * VMIN/VTIME indefinitely. */
+        if (haveStart && bytes_remaining > 0 &&
+                clock_gettime(CLOCK_MONOTONIC, &now) == 0 &&
+                (now.tv_sec - start.tv_sec) >
+                    WOLFTPM_SWTPM_UART_RX_TIMEOUT_SECS) {
+        #ifdef DEBUG_WOLFTPM
+            printf("SwTpmReceive: cumulative UART timeout after %lds "
+                   "(%zu bytes remaining)\n",
+                   (long)(now.tv_sec - start.tv_sec), bytes_remaining);
+        #endif
+            rc = TPM_RC_FAILURE;
+            break;
+        }
+#endif
     }
 
     return rc;
@@ -233,8 +263,14 @@ static TPM_RC SwTpmConnect(TPM2_CTX* ctx, const char* host, const char* port)
     #endif
         default:     baud = B115200; break;
     }
-    cfsetospeed(&tty, baud);
-    cfsetispeed(&tty, baud);
+    if (cfsetospeed(&tty, baud) != 0 || cfsetispeed(&tty, baud) != 0) {
+    #ifdef DEBUG_WOLFTPM
+        printf("Failed to set UART baud rate %d: %s\n",
+            baudInt, strerror(errno));
+    #endif
+        close(fd);
+        return TPM_RC_FAILURE;
+    }
 
     /* 8N1: 8 data bits, no parity, 1 stop bit */
     tty.c_cflag = (tty.c_cflag & ~CSIZE) | CS8;
@@ -381,10 +417,15 @@ static TPM_RC SwTpmDisconnect(TPM2_CTX* ctx)
     #endif
 
 #ifdef WOLFTPM_SWTPM_UART
-    /* UART: keep the port open for the next command.
+    /* UART: on success, keep the port open for the next command.
      * The SESSION_END tells the server the command sequence is done.
-     * Final cleanup of the UART FD is handled in TPM2_SwtpmCloseUART. */
-    (void)ctx;
+     * Final cleanup of the UART FD is handled in TPM2_SwtpmCloseUART.
+     * On SESSION_END write failure, close and reset the fd so the next
+     * command reconnects instead of reusing a broken connection. */
+    if (rc != TPM_RC_SUCCESS) {
+        close(ctx->tcpCtx.fd);
+        ctx->tcpCtx.fd = -1;
+    }
 #else
     if (0 != close(ctx->tcpCtx.fd)) {
         rc = TPM_RC_FAILURE;

src/tpm2_wrap.c

@@ -1723,12 +1723,14 @@ int wolfTPM2_SetAuthHandle(WOLFTPM2_DEV* dev, int index,
             TPM2_PrintBin(session->name.name, session->name.size);
             TPM2_PrintBin(handle->name.name, handle->name.size);
         #endif
-            session->policyAuth = handle->policyAuth;
+            /* Validate bounds before any session-state mutation so the
+             * session isn't left inconsistent on BUFFER_E. */
             if (authDigestSz <= 0 ||
                 (handle->auth.size + authDigestSz) >
                     (int)sizeof(session->auth.buffer)) {
                 return BUFFER_E;
             }
+            session->policyAuth = handle->policyAuth;
             session->auth.size = authDigestSz + handle->auth.size;
             XMEMCPY(&session->auth.buffer[authDigestSz], handle->auth.buffer,
                 handle->auth.size);
@@ -1862,16 +1864,19 @@ int wolfTPM2_SetSessionHandle(WOLFTPM2_DEV* dev, int index,
 
     /* Set password handle unless TPM session is available */
     if (tpmSession) {
-        session->sessionHandle = tpmSession->handle.hndl;
-
-        session->auth.size = tpmSession->handle.auth.size;
-        if (session->auth.size > sizeof(session->auth.buffer)) {
+        /* Validate bounds before mutating session state so the session
+         * isn't left inconsistent on BUFFER_E. */
+        if (tpmSession->handle.auth.size > sizeof(session->auth.buffer)) {
         #ifdef DEBUG_WOLFTPM
             printf("SetSessionHandle: auth size %d exceeds buffer %d\n",
-                session->auth.size, (int)sizeof(session->auth.buffer));
+                tpmSession->handle.auth.size,
+                (int)sizeof(session->auth.buffer));
         #endif
             return BUFFER_E;
         }
+
+        session->sessionHandle = tpmSession->handle.hndl;
+        session->auth.size = tpmSession->handle.auth.size;
         XMEMCPY(session->auth.buffer, tpmSession->handle.auth.buffer,
             session->auth.size);
 
@@ -9249,14 +9254,20 @@ int wolfTPM2_SetIdentityAuth(WOLFTPM2_DEV* dev, WOLFTPM2_HANDLE* handle,
         wc_HashFree(&hash_ctx, hashType);
     }
 
-    /* Hash Final truncate to 16 bytes */
-    /* Use 16-byte for auth when accessing key */
-    handle->auth.size = 16;
-    XMEMCPY(handle->auth.buffer, &digest[16], 16);
-#ifdef DEBUG_WOLFTPM
-    printf("Handle 0x%x, Auth %d\n", handle->hndl, handle->auth.size);
-    TPM2_PrintBin(handle->auth.buffer, handle->auth.size);
-#endif
+    /* Only copy digest to handle auth when hashing succeeded — otherwise
+     * `digest` contains uninitialized stack data. */
+    if (rc == 0) {
+        /* Hash Final truncate to 16 bytes — use 16-byte auth for key access */
+        handle->auth.size = 16;
+        XMEMCPY(handle->auth.buffer, &digest[16], 16);
+    #ifdef DEBUG_WOLFTPM
+        printf("Handle 0x%x, Auth %d\n", handle->hndl, handle->auth.size);
+        TPM2_PrintBin(handle->auth.buffer, handle->auth.size);
+    #endif
+    }
+    else {
+        handle->auth.size = 0;
+    }
 
     TPM2_ForceZero(digest, sizeof(digest));
     TPM2_ForceZero(serialNum, sizeof(serialNum));

wolftpm/tpm2_crypto.h

@@ -144,7 +144,9 @@ WOLFTPM_API int TPM2_KDFe(
 
 /* --- Crypto Primitive Wrappers --- */
 
-#ifndef NO_AES
+#ifndef WOLFTPM2_NO_WOLFCRYPT
+
+#if !defined(NO_AES) && defined(WOLFSSL_AES_CFB)
 /*!
     \ingroup TPM2_Crypto
     \brief AES-CFB one-shot encrypt (in-place).
@@ -184,7 +186,7 @@ WOLFTPM_API int TPM2_AesCfbDecrypt(
     const byte* key, int keySz,
     const byte* iv,
     byte* data, word32 dataSz);
-#endif /* !NO_AES */
+#endif /* !NO_AES && WOLFSSL_AES_CFB */
 
 #ifndef NO_HMAC
 /*!
@@ -264,6 +266,8 @@ WOLFTPM_API int TPM2_HashCompute(
     const byte* data, word32 dataSz,
     byte* digest, word32* digestSz);
 
+#endif /* !WOLFTPM2_NO_WOLFCRYPT */
+
 #ifdef __cplusplus
     }  /* extern "C" */
 #endif