Merge pull request #563 from aidangarske/fenrir-fixes-7606-7610

Reuse transport connections and reduce NV write and hash cache overhead

Author: dgarske

docs/SWTPM.md

@@ -51,7 +51,7 @@ Build Options:
 * `TPM2_SWTPM_HOST`: The serial device path (default is `/dev/ttyACM0` on Linux, `/dev/cu.usbmodem` on macOS). Can be overridden at runtime via the `TPM2_SWTPM_HOST` environment variable.
 * `TPM2_SWTPM_PORT`: The baud rate (default is 115200)
 
-The UART transport uses the same mssim protocol as the socket transport. The serial port is configured as 8N1 raw mode with no flow control. Unlike the socket transport, the serial port file descriptor is kept open across commands (no reconnect per command).
+The UART transport uses the same mssim protocol as the socket transport. The serial port is configured as 8N1 raw mode with no flow control. Like the socket transport, the serial port file descriptor is kept open across commands (no reconnect per command). Both transports close the connection during `wolfTPM2_Cleanup`. On the socket transport any transmit/receive failure also closes the connection so the next command reconnects; the UART transport closes only when the per-command `TPM_SESSION_END` write fails.
 
 #### Security note: environment variable override
 

examples/tpm_test.h

@@ -45,6 +45,7 @@
 #define TPM2_DEMO_NV_TEST_INDEX         0x01800200
 #define TPM2_DEMO_NV_TEST_AUTH_INDEX    0x01800201
 #define TPM2_DEMO_NVRAM_STORE_INDEX     0x01800202
+#define TPM2_DEMO_NV_TEST_CHUNKED_INDEX 0x01800205
 #define TPM2_DEMO_NVRAM_EXTEND_INDEX    0x01000200
 #define TPM2_DEMO_NV_TEST_SIZE          MAX_DIGEST_BUFFER /* max size on Infineon SLB9670 is 1664 */
 #define TPM2_DEMO_NV_COUNTER_INDEX      0x01800300

hal/tpm_io_linux.c

@@ -76,6 +76,7 @@
         #define TPM2_I2C_DEV  "/dev/i2c-1"
         #define TPM2_I2C_HZ   400000 /* 400kHz */
         static int i2cOpenFailed = 0;
+        static int i2cDevFd = -1;
     #else
         /* SPI */
         #ifndef TPM2_SPI_DEV_CS
@@ -112,6 +113,7 @@
             #define TPM2_SPI_DEV TPM2_SPI_DEV_PATH TPM2_SPI_DEV_CS
             static int spiOpenFailed = 0;
         #endif
+        static int spiDevFd = -1;
     #endif
 #endif
 
@@ -195,14 +197,19 @@
         word16 size, void* userCtx)
     {
         int ret = TPM_RC_FAILURE;
-        int i2cDev = open(TPM2_I2C_DEV, O_RDWR);
-        if (i2cDev >= 0) {
+        if (i2cDevFd < 0) {
+            i2cDevFd = open(TPM2_I2C_DEV, O_RDWR | O_CLOEXEC);
+        }
+        if (i2cDevFd >= 0) {
             if (isRead)
-                ret = i2c_read(i2cDev, addr, buf, size);
+                ret = i2c_read(i2cDevFd, addr, buf, size);
             else
-                ret = i2c_write(i2cDev, addr, buf, size);
+                ret = i2c_write(i2cDevFd, addr, buf, size);
 
-            close(i2cDev);
+            if (ret != TPM_RC_SUCCESS) {
+                close(i2cDevFd);
+                i2cDevFd = -1;
+            }
         }
         else if (!i2cOpenFailed) {
             i2cOpenFailed = 1;
@@ -264,7 +271,6 @@
         word16 xferSz, void* userCtx)
     {
         int ret;
-        int spiDev;
     #ifdef WOLFTPM_CHECK_WAIT_STATE
         int timeout;
     #endif
@@ -274,7 +280,6 @@
 
         /* Note: PI has issue with 5-10Mhz on packets sized over 130 bytes */
         unsigned int maxSpeed = TPM2_SPI_HZ;
-        int mode = 0; /* Mode 0 (CPOL=0, CPHA=0) */
         int bits_per_word = 8; /* 8-bits */
 
     #ifdef WOLFTPM_AUTODETECT
@@ -290,16 +295,20 @@
     #ifdef WOLFTPM_CHECK_WAIT_STATE
         timeout = TPM_SPI_WAIT_RETRY;
     #endif
-        spiDev = open(TPM2_SPI_DEV, O_RDWR);
-        if (spiDev >= 0) {
+        if (spiDevFd < 0) {
+            spiDevFd = open(TPM2_SPI_DEV, O_RDWR | O_CLOEXEC);
+            if (spiDevFd >= 0) {
+                int mode = 0; /* Mode 0 (CPOL=0, CPHA=0) */
+                ioctl(spiDevFd, SPI_IOC_WR_MODE, &mode);
+            }
+        }
+        if (spiDevFd >= 0) {
             struct spi_ioc_transfer spi;
             size_t size;
 
-            ioctl(spiDev, SPI_IOC_WR_MODE, &mode);
-            ioctl(spiDev, SPI_IOC_WR_MAX_SPEED_HZ, &maxSpeed);
-            ioctl(spiDev, SPI_IOC_WR_BITS_PER_WORD, &bits_per_word);
-
             XMEMSET(&spi, 0, sizeof(spi));
+            spi.speed_hz = maxSpeed;
+            spi.bits_per_word = bits_per_word;
 
     #ifdef WOLFTPM_CHECK_WAIT_STATE
             /* Keep CS asserted for header and flow control transfers */
@@ -309,7 +318,7 @@
             spi.tx_buf   = (unsigned long)txBuf;
             spi.rx_buf   = (unsigned long)rxBuf;
             spi.len      = TPM_TIS_HEADER_SZ;
-            size = ioctl(spiDev, SPI_IOC_MESSAGE(1), &spi);
+            size = ioctl(spiDevFd, SPI_IOC_MESSAGE(1), &spi);
             if (size != TPM_TIS_HEADER_SZ) {
                 ret =  TPM_RC_FAILURE;
             }
@@ -322,7 +331,7 @@
                 spi.len = 1;
                 do {
                     /* Check for SPI ready */
-                    size = ioctl(spiDev, SPI_IOC_MESSAGE(1), &spi);
+                    size = ioctl(spiDevFd, SPI_IOC_MESSAGE(1), &spi);
                 } while (
                     (size == 1) &&
                     ((rxBuf[TPM_TIS_HEADER_SZ-1] & TPM_TIS_READY_MASK) == 0) &&
@@ -342,7 +351,7 @@
                 spi.tx_buf   = (unsigned long)&txBuf[TPM_TIS_HEADER_SZ];
                 spi.rx_buf   = (unsigned long)&rxBuf[TPM_TIS_HEADER_SZ];
                 spi.len      = xferSz - TPM_TIS_HEADER_SZ;
-                size = ioctl(spiDev, SPI_IOC_MESSAGE(1), &spi);
+                size = ioctl(spiDevFd, SPI_IOC_MESSAGE(1), &spi);
                 if (size != (size_t)xferSz - TPM_TIS_HEADER_SZ)
                     ret = TPM_RC_FAILURE;
             }
@@ -351,20 +360,23 @@
             if (spi.cs_change == 1) {
                 spi.cs_change = 0;
                 spi.len = 1;
-                size = ioctl(spiDev, SPI_IOC_MESSAGE(1), &spi);
+                size = ioctl(spiDevFd, SPI_IOC_MESSAGE(1), &spi);
                 (void)size;  /* Ignore result */
             }
     #else
             /* Send Entire Message - no wait states */
             spi.tx_buf   = (unsigned long)txBuf;
             spi.rx_buf   = (unsigned long)rxBuf;
             spi.len      = xferSz;
-            size = ioctl(spiDev, SPI_IOC_MESSAGE(1), &spi);
+            size = ioctl(spiDevFd, SPI_IOC_MESSAGE(1), &spi);
             if (size != (size_t)xferSz)
                 ret = TPM_RC_FAILURE;
     #endif /* WOLFTPM_CHECK_WAIT_STATE */
 
-            close(spiDev);
+            if (ret != TPM_RC_SUCCESS) {
+                close(spiDevFd);
+                spiDevFd = -1;
+            }
         }
         else {
             /* Failed to open device */
@@ -398,6 +410,10 @@
                 foundSpiDev = 1;
             }
             else {
+                if (spiDevFd >= 0) {
+                    close(spiDevFd);
+                    spiDevFd = -1;
+                }
                 devLen = (int)XSTRLEN(TPM2_SPI_DEV);
                 /* tries spidev0.[0-4] */
                 if (TPM2_SPI_DEV[devLen-1] < MAX_SPI_DEV_CS) {

src/fwtpm/fwtpm_io.c

@@ -84,6 +84,8 @@ int FWTPM_IO_IsStopRequested(void)
 #ifndef WOLFTPM_FWTPM_TIS
 /* --- Low-level socket helpers --- */
 
+#define FWTPM_MAX_COMMAND_CLIENTS 8
+
 static int SocketSend(SOCKET_T fd, const void* buf, int sz)
 {
     const char* ptr = (const char*)buf;
@@ -650,10 +652,11 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
     int rc = TPM_RC_SUCCESS;
     fd_set readFds;
     int maxFd;
-    SOCKET_T cmdFd = FWTPM_INVALID_FD;   /* active command client fd */
+    SOCKET_T cmdFds[FWTPM_MAX_COMMAND_CLIENTS];
     SOCKET_T platFd = FWTPM_INVALID_FD;  /* active platform client fd */
     struct timeval tv;
     int selRc;
+    int i;
 #ifndef _WIN32
     struct sigaction sa;
 #endif
@@ -667,6 +670,9 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
     return FWTPM_TIS_ServerLoop(ctx);
 #else
     ctx->running = 1;
+    for (i = 0; i < FWTPM_MAX_COMMAND_CLIENTS; i++) {
+        cmdFds[i] = FWTPM_INVALID_FD;
+    }
 
 #ifndef _WIN32
     /* Ignore SIGPIPE so write to closed socket returns error instead
@@ -697,9 +703,13 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
             maxFd = ctx->io.platListenFd;
 
         /* Watch active client connections for incoming data */
-        if (cmdFd != FWTPM_INVALID_FD) {
-            FD_SET(cmdFd, &readFds);
-            if (cmdFd > maxFd) maxFd = cmdFd;
+        for (i = 0; i < FWTPM_MAX_COMMAND_CLIENTS; i++) {
+            if (cmdFds[i] != FWTPM_INVALID_FD) {
+                FD_SET(cmdFds[i], &readFds);
+                if (cmdFds[i] > maxFd) {
+                    maxFd = cmdFds[i];
+                }
+            }
         }
         if (platFd != FWTPM_INVALID_FD) {
             FD_SET(platFd, &readFds);
@@ -739,48 +749,54 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
             }
         }
 
+        /* Handle one message from active platform client */
+        if (platFd != FWTPM_INVALID_FD && FD_ISSET(platFd, &readFds)) {
+            if (HandlePlatformCommand(ctx, platFd) != TPM_RC_SUCCESS) {
+                CloseSocket(platFd);
+                platFd = FWTPM_INVALID_FD;
+            }
+        }
+
+        /* Handle one message from each ready command client */
+        for (i = 0; i < FWTPM_MAX_COMMAND_CLIENTS; i++) {
+            if (cmdFds[i] != FWTPM_INVALID_FD &&
+                    FD_ISSET(cmdFds[i], &readFds)) {
+                if (HandleCommandConnection(ctx, cmdFds[i]) !=
+                        TPM_RC_SUCCESS) {
+                    CloseSocket(cmdFds[i]);
+                    cmdFds[i] = FWTPM_INVALID_FD;
+                }
+            }
+        }
+
         /* Accept new command connection */
         if (FD_ISSET(ctx->io.listenFd, &readFds)) {
             SOCKET_T newFd = accept(ctx->io.listenFd, NULL, NULL);
             if (newFd != FWTPM_INVALID_FD) {
-                if (cmdFd != FWTPM_INVALID_FD) {
-                    /* Consume any select-confirmed in-flight command on the
-                     * old connection before dropping it, so a pending
-                     * request is not silently lost. */
-                    if (FD_ISSET(cmdFd, &readFds)) {
-                        HandleCommandConnection(ctx, cmdFd);
+                for (i = 0; i < FWTPM_MAX_COMMAND_CLIENTS; i++) {
+                    if (cmdFds[i] == FWTPM_INVALID_FD) {
+                        cmdFds[i] = newFd;
+                        break;
                     }
+                }
+                if (i == FWTPM_MAX_COMMAND_CLIENTS) {
                 #ifdef DEBUG_WOLFTPM
-                    printf("fwTPM: command connection replaced\n");
+                    printf("fwTPM: too many command connections\n");
                 #endif
-                    CloseSocket(cmdFd);
+                    CloseSocket(newFd);
                 }
-                cmdFd = newFd;
-            }
-        }
-
-        /* Handle one message from active platform client */
-        if (platFd != FWTPM_INVALID_FD && FD_ISSET(platFd, &readFds)) {
-            if (HandlePlatformCommand(ctx, platFd) != TPM_RC_SUCCESS) {
-                CloseSocket(platFd);
-                platFd = FWTPM_INVALID_FD;
             }
         }
+    }
 
-        /* Handle one message from active command client */
-        if (cmdFd != FWTPM_INVALID_FD && FD_ISSET(cmdFd, &readFds)) {
-            if (HandleCommandConnection(ctx, cmdFd) != TPM_RC_SUCCESS) {
-                CloseSocket(cmdFd);
-                cmdFd = FWTPM_INVALID_FD;
-                /* Transient state persists across command connections: the
-                 * mssim transport reconnects per command for one logical TPM,
-                 * so a clean disconnect must not flush handles. */
-            }
+    for (i = 0; i < FWTPM_MAX_COMMAND_CLIENTS; i++) {
+        if (cmdFds[i] != FWTPM_INVALID_FD) {
+            CloseSocket(cmdFds[i]);
         }
     }
-
-    if (cmdFd != FWTPM_INVALID_FD)  CloseSocket(cmdFd);
-    if (platFd != FWTPM_INVALID_FD) CloseSocket(platFd);
+    if (platFd != FWTPM_INVALID_FD) {
+        CloseSocket(platFd);
+    }
 
     return rc;
 #endif /* !WOLFTPM_FWTPM_TIS */

src/tpm2.c

@@ -854,6 +854,11 @@ TPM_RC TPM2_Init_ex(TPM2_CTX* ctx, TPM2HalIoCb ioCb, void* userCtx,
 
     XMEMSET(ctx, 0, sizeof(TPM2_CTX));
 
+#if defined(WOLFTPM_SWTPM)
+    /* set before any early return so cleanup cannot act on fd 0 */
+    ctx->tcpCtx.fd = -1;
+#endif
+
 #ifndef WOLFTPM_NO_RETRY
     ctx->retries = WOLFTPM_MAX_RETRIES;
 #endif
@@ -864,10 +869,6 @@ TPM_RC TPM2_Init_ex(TPM2_CTX* ctx, TPM2HalIoCb ioCb, void* userCtx,
         return rc;
 #endif
 
-#if defined(WOLFTPM_SWTPM)
-    ctx->tcpCtx.fd = -1;
-#endif
-
 #if defined(WOLFTPM_LINUX_DEV) || defined(WOLFTPM_SWTPM) || \
     defined(WOLFTPM_WINAPI)
     if (ioCb != NULL || userCtx != NULL) {
@@ -976,9 +977,8 @@ TPM_RC TPM2_Cleanup(TPM2_CTX* ctx)
         close(ctx->fd);
 #endif
 
-#ifdef WOLFTPM_SWTPM_UART
-    /* Close the persistent UART connection */
-    TPM2_SwtpmCloseUART(ctx);
+#ifdef WOLFTPM_SWTPM
+    TPM2_SwtpmClose(ctx);
 #endif
 
     return TPM_RC_SUCCESS;

src/tpm2_cryptocb.c

@@ -817,13 +817,19 @@ static int wolfTPM2_HashUpdateCache(WOLFTPM2_HASHCTX* hashCtx,
     else if ((hashCtx->cacheSz + inSz) > hashCtx->cacheBufSz) {
         byte* oldIn = hashCtx->cacheBuf;
         word32 oldBufSz = hashCtx->cacheBufSz;
+        word32 newSz;
         /* check for overflow, including the block round-up below */
         if (hashCtx->cacheSz + inSz < hashCtx->cacheSz ||
             hashCtx->cacheSz + inSz >
                 0xFFFFFFFFU - (WOLFTPM2_HASH_BLOCK_SZ - 1)) {
             return BUFFER_E;
         }
-        hashCtx->cacheBufSz = (hashCtx->cacheSz + inSz +
+        newSz = hashCtx->cacheSz + inSz;
+        /* Block alignment keeps the round-up safe after doubling. */
+        if (oldBufSz <= 0xFFFFFFFFU / 2 && (oldBufSz * 2) > newSz) {
+            newSz = oldBufSz * 2;
+        }
+        hashCtx->cacheBufSz = (newSz +
             WOLFTPM2_HASH_BLOCK_SZ - 1) & ~(WOLFTPM2_HASH_BLOCK_SZ - 1);
         hashCtx->cacheBuf = (byte*)XMALLOC(hashCtx->cacheBufSz,
             NULL, DYNAMIC_TYPE_TMP_BUFFER);