Merge pull request #491 from aidangarske/fenrir-fixes-9

fwTPM/SPDM/src corrections and unit test additions

Author: dgarske

examples/firmware/ifx_fw_extract.c

@@ -63,7 +63,7 @@
 #define LOG(t) { printf(__FILE__":%i: %s\n", __LINE__, t); }
 
 #define READ_BE16(dest, buf, size, off) { \
-    if (off + sizeof(dest) >= size) { \
+    if (off + sizeof(dest) > size) { \
         LOG("FW file too short"); \
         return -1; \
     } \
@@ -73,7 +73,7 @@
 }
 
 #define READ_BE32(dest, buf, size, off) { \
-    if (off + sizeof(dest) >= size) { \
+    if (off + sizeof(dest) > size) { \
         LOG("FW file too short"); \
         return -1; \
     } \
@@ -172,7 +172,7 @@ static int extractFW(
     }
 
     READ_BE32(size32, fw, fw_size, offset);
-    if (offset + size32 >= fw_size) {
+    if (offset + size32 > fw_size) {
         LOG("FW file too short");
         return -1;
     }

examples/firmware/ifx_fw_update.c

@@ -170,7 +170,7 @@ int TPM2_IFX_Firmware_Update(void* userCtx, int argc, char *argv[])
         else {
             printf("Success: Please reset or power cycle TPM\n");
         }
-        return rc;
+        goto exit;
     }
 
     if (manifest_file == NULL || firmware_file == NULL) {

examples/firmware/st33_fw_update.c

@@ -264,7 +264,7 @@ int TPM2_ST33_Firmware_Update(void* userCtx, int argc, char *argv[])
         else {
             printf("Success: Please reset or power cycle TPM\n");
         }
-        return rc;
+        goto exit;
     }
 
     if (fi_file == NULL) {

examples/pcr/quote.c

@@ -98,12 +98,17 @@ int TPM2_PCR_Quote_Test(void* userCtx, int argc, char *argv[])
 
         /* Advanced usage */
         if (argv[1][0] != '-') {
-            if (pcrIndex < 0 || pcrIndex > 23 || *argv[1] < '0' || *argv[1] > '9') {
+            if (*argv[1] < '0' || *argv[1] > '9') {
                 printf("PCR index is out of range (0-23)\n");
                 usage();
                 return 0;
             }
             pcrIndex = XATOI(argv[1]);
+            if (pcrIndex < 0 || pcrIndex > 23) {
+                printf("PCR index is out of range (0-23)\n");
+                usage();
+                return 0;
+            }
         }
         if (argc >= 3 && argv[2][0] != '-')
             outputFile = argv[2];

examples/pkcs7/pkcs7.c

@@ -115,10 +115,14 @@ static int PKCS7_SignVerifyEx(WOLFTPM2_DEV* dev, int tpmDevId,
 
     XMEMSET(&pkcs7, 0, sizeof(pkcs7));
 
-    hashSz = wc_HashGetDigestSize(hashType);
-    if (hashSz <= 0) {
-        return hashSz;
+    rc = wc_HashGetDigestSize(hashType);
+    if (rc <= 0) {
+        /* Preserve the wolfCrypt error on negatives; for a 0 return
+         * (not currently produced by wolfCrypt), report BAD_FUNC_ARG
+         * rather than masquerading as success. */
+        return (rc < 0) ? rc : BAD_FUNC_ARG;
     }
+    hashSz = (word32)rc;
 
     /* calculate hash for content */
     rc = wc_HashInit(&hash, hashType);

hal/tpm_io_infineon.c

@@ -186,7 +186,7 @@
     #include <Qspi/SpiMaster/IfxQspi_SpiMaster.h>
 
     /* externally declared SPI master channel */
-    extern IfxQspi_SpiMaster_Channel spiMasterChannel
+    extern IfxQspi_SpiMaster_Channel spiMasterChannel;
 
     static int TPM2_IoCb_Infineon_TriCore_SPI(TPM2_CTX* ctx, const byte* txBuf,
         byte* rxBuf, word16 xferSz, void* userCtx)

hal/tpm_io_microchip.c

@@ -223,6 +223,8 @@
                 return -1;
             }
 
+            busy_retry = TPM_I2C_TRIES;
+
             while (I2C_BB_IsBusy() && --busy_retry > 0) {
                 microchip_wait(250);
             }

src/fwtpm/fwtpm_command.c

@@ -429,6 +429,46 @@ static void FwLookupEntityAuth(FWTPM_CTX* ctx, TPM_HANDLE handle,
     }
 }
 
+/* Constant-time password vs authValue comparison.
+ * Iterates a fixed upper bound (TPM_MAX_DIGEST_SIZE) with bitwise masks so
+ * neither the trip count nor per-iteration work depends on the secret
+ * authValSz. Trailing zeros on either side are treated as insignificant
+ * (matches TCG reference for authValues padded to nameAlg digest size).
+ * Returns 1 on mismatch, 0 on match. Precondition: pwSz and avSz must
+ * each be <= TPM_MAX_DIGEST_SIZE; out-of-range inputs fail closed. */
+static int FwCtAuthCompare(const byte* password, int pwSz,
+    const byte* authVal, int avSz)
+{
+    byte zeroAuth[TPM_MAX_DIGEST_SIZE];
+    const byte* avPtr;
+    volatile byte diff = 0;
+    int ci;
+
+    if (pwSz < 0 || avSz < 0 ||
+            pwSz > TPM_MAX_DIGEST_SIZE || avSz > TPM_MAX_DIGEST_SIZE) {
+        return 1;
+    }
+
+    XMEMSET(zeroAuth, 0, sizeof(zeroAuth));
+    avPtr = (authVal != NULL) ? authVal : zeroAuth;
+
+    for (ci = 0; ci < TPM_MAX_DIGEST_SIZE; ci++) {
+        /* 0xFF if ci < bound, else 0x00. Use UINT32 (guaranteed 32-bit
+         * wolfTPM typedef) so the >> 31 shift is always well-defined. */
+        byte pwMask = (byte)-((UINT32)(ci - pwSz) >> 31);
+        byte avMask = (byte)-((UINT32)(ci - avSz) >> 31);
+        byte overlap = (byte)(pwMask & avMask);
+        /* Overlap region: bytes must match */
+        diff |= (byte)((password[ci] ^ avPtr[ci]) & overlap);
+        /* Trailing bytes of pw past avSz must be zero */
+        diff |= (byte)(password[ci] & (pwMask & (byte)~avMask));
+        /* Trailing bytes of av past pwSz must be zero */
+        diff |= (byte)(avPtr[ci] & (avMask & (byte)~pwMask));
+    }
+
+    return ((int)diff != 0) ? 1 : 0;
+}
+
 /* Compute cpHash = H(commandCode || name1 || ... || cpBuffer)
  * Per TPM 2.0 Part 1 Section 18.7 */
 static int FwComputeCpHash(TPMI_ALG_HASH hashAlg, TPM_CC cmdCode,
@@ -9975,6 +10015,7 @@ static TPM_RC FwCmd_NV_DefineSpace(FWTPM_CTX* ctx, TPM2_Packet* cmd,
         FwRspNoParams(rsp, cmdTag);
     }
 
+    TPM2_ForceZero(&auth, sizeof(auth));
     return rc;
 }
 
@@ -12719,9 +12760,7 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
     int doEncRsp = 0;               /* Encrypt outgoing response param */
 #endif
     int pj, hj;                     /* Loop indices for auth validation */
-    int pwSz, avSz, maxSz, minSz, authFail; /* Password comparison */
-    volatile byte diff;
-    int ci;
+    int authFail;                   /* Password comparison result */
 
     if (ctx == NULL || cmdBuf == NULL || rspBuf == NULL || rspSize == NULL) {
         return BAD_FUNC_ARG;
@@ -13085,29 +13124,8 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
 
             FwLookupEntityAuth(ctx, entityH, &authVal, &authValSz);
 
-            /* Compare password with authValue in constant time.
-             * Per TCG reference implementation, trailing zeros are
-             * insignificant (handles authValues padded to nameAlg
-             * digest size). We compare up to the max of both sizes
-             * and verify trailing bytes are zero, all in constant
-             * time to avoid leaking the effective auth length. */
-            diff = 0;
-            pwSz = (int)cmdAuths[pj].passwordSize;
-            avSz = authValSz;
-            maxSz = (pwSz > avSz) ? pwSz : avSz;
-            minSz = (pwSz < avSz) ? pwSz : avSz;
-            /* Compare overlapping portion */
-            for (ci = 0; ci < minSz; ci++) {
-                diff |= cmdAuths[pj].password[ci] ^ authVal[ci];
-            }
-            /* Verify trailing bytes of the longer buffer are zero */
-            for (ci = minSz; ci < maxSz; ci++) {
-                if (ci < pwSz)
-                    diff |= cmdAuths[pj].password[ci];
-                if (ci < avSz)
-                    diff |= authVal[ci];
-            }
-            authFail = ((int)diff != 0);
+            authFail = FwCtAuthCompare(cmdAuths[pj].password,
+                (int)cmdAuths[pj].passwordSize, authVal, authValSz);
             if (authFail) {
             #ifdef DEBUG_WOLFTPM
                 printf("fwTPM: Password auth failed for handle "