fwtpm: peer-review batch 2 (medium hardening)

Security/bounds:
- FlushContext: reject persistent handles (use EvictControl)
- PCR_Extend: reject unknown hash alg (dSize=0 desync fix)
- HierarchyChangeAuth: ForceZero old auth buffer before overwrite
- FwDecryptSeed ECC: bounds check before ySz FwLoadU16BE (OOB read)
- FwImportReconstructKey: verify q divides n (mp_div remainder check)
- FwWrapPrivate: validate outPriv buffer size before packing
- TIS FIFO: snapshot shared-mem positions to locals (TOCTOU)
- NV writePos: validate against hal->maxSize on journal load
- NV compaction: only reset writePos after successful erase

Zeroization:
- FwWrapPrivate: zero sensBuf, hmacDigest before free
- FwDeriveEccPrimaryKey: constant-time zero check (OR accumulator)
- FwCredentialDeriveKeys: zero symKey on second KDFa failure
- FwCredentialUnwrap: zero computedHmac, integrityHmac
- FwEncryptSeed: zero seedBuf on RSA/ECC error paths
- FwImportVerifyAndDecrypt: zero plainSens on decrypt failure
- NV save functions: ForceZero marshal buffers before XFREE
- KDFa/KDFe: zero output key on mid-loop hash error

Author: dgarske

src/fwtpm/fwtpm_command.c

@@ -1403,7 +1403,14 @@ static TPM_RC FwCmd_PCR_Extend(FWTPM_CTX* ctx, TPM2_Packet* cmd, int cmdSize,
         bank = FwGetPcrBankIndex(hashAlg);
         dSize = TPM2_GetHashDigestSize(hashAlg);
 
-        if (bank < 0 || dSize == 0) {
+        if (dSize == 0) {
+            /* Unknown hash algorithm — cannot determine digest size to skip,
+             * so reject rather than desync the command buffer parser. */
+            rc = TPM_RC_HASH;
+            break;
+        }
+        if (bank < 0) {
+            /* Known algorithm but unsupported bank — skip the digest bytes */
             cmd->pos += dSize;
             continue;
         }
@@ -2481,6 +2488,11 @@ static TPM_RC FwCmd_FlushContext(FWTPM_CTX* ctx, TPM2_Packet* cmd,
                 FwFreeSession(sess);
             }
         }
+        else if ((flushHandle & 0xFF000000) ==
+                 (PERSISTENT_FIRST & 0xFF000000)) {
+            /* Persistent objects cannot be flushed — use EvictControl */
+            rc = TPM_RC_HANDLE;
+        }
         else {
             /* Transient object handle */
             FWTPM_Object* obj = FwFindObject(ctx, flushHandle);
@@ -3064,25 +3076,33 @@ static TPM_RC FwCmd_HierarchyChangeAuth(FWTPM_CTX* ctx, TPM2_Packet* cmd,
     if (rc == 0) {
         switch (authHandle) {
             case TPM_RH_OWNER:
+                TPM2_ForceZero(ctx->ownerAuth.buffer,
+                    sizeof(ctx->ownerAuth.buffer));
                 ctx->ownerAuth.size = newAuthSize;
                 if (newAuthSize > 0) {
                     XMEMCPY(ctx->ownerAuth.buffer, newAuthBuf, newAuthSize);
                 }
                 break;
             case TPM_RH_ENDORSEMENT:
+                TPM2_ForceZero(ctx->endorsementAuth.buffer,
+                    sizeof(ctx->endorsementAuth.buffer));
                 ctx->endorsementAuth.size = newAuthSize;
                 if (newAuthSize > 0) {
                     XMEMCPY(ctx->endorsementAuth.buffer, newAuthBuf,
                         newAuthSize);
                 }
                 break;
             case TPM_RH_PLATFORM:
+                TPM2_ForceZero(ctx->platformAuth.buffer,
+                    sizeof(ctx->platformAuth.buffer));
                 ctx->platformAuth.size = newAuthSize;
                 if (newAuthSize > 0) {
                     XMEMCPY(ctx->platformAuth.buffer, newAuthBuf, newAuthSize);
                 }
                 break;
             case TPM_RH_LOCKOUT:
+                TPM2_ForceZero(ctx->lockoutAuth.buffer,
+                    sizeof(ctx->lockoutAuth.buffer));
                 ctx->lockoutAuth.size = newAuthSize;
                 if (newAuthSize > 0) {
                     XMEMCPY(ctx->lockoutAuth.buffer, newAuthBuf, newAuthSize);

src/fwtpm/fwtpm_crypto.c

@@ -573,13 +573,13 @@ TPM_RC FwDeriveEccPrimaryKey(TPMI_ALG_HASH nameAlg,
             rc = TPM_RC_FAILURE;
             break;
         }
-        /* Check d != 0 (all zeros) */
-        allZero = 1;
-        for (i = 0; i < keySz; i++) {
-            if (dBuf[i] != 0) {
-                allZero = 0;
-                break;
+        /* Constant-time check d != 0 (all zeros) */
+        {
+            volatile byte orAccum = 0;
+            for (i = 0; i < keySz; i++) {
+                orAccum |= dBuf[i];
             }
+            allZero = (orAccum == 0);
         }
         if (!allZero) {
             valid = 1; /* Accept — range check done by import */
@@ -1060,6 +1060,12 @@ int FwWrapPrivate(FWTPM_Object* parent,
     wc_HmacFree(hmac);
 
     /* Pack into TPM2B_PRIVATE */
+    if (rc == 0) {
+        int totalSz = 2 + WC_SHA256_DIGEST_SIZE + 2 + sensSz;
+        if (totalSz > (int)sizeof(outPriv->buffer)) {
+            rc = TPM_RC_SIZE;
+        }
+    }
     if (rc == 0) {
         /* integritySize(2) + integrity(32) + encSensSize(2) + encSens(N) */
         outPriv->buffer[pos++] = 0;
@@ -1079,6 +1085,8 @@ int FwWrapPrivate(FWTPM_Object* parent,
 
     TPM2_ForceZero(aesKey, sizeof(aesKey));
     TPM2_ForceZero(aesIV, sizeof(aesIV));
+    TPM2_ForceZero(hmacDigest, sizeof(hmacDigest));
+    TPM2_ForceZero(sensBuf, FWTPM_MAX_PRIVKEY_DER + 128);
     FWTPM_FREE_BUF(sensBuf);
     FWTPM_FREE_VAR(aes);
     FWTPM_FREE_VAR(hmac);
@@ -1284,6 +1292,11 @@ TPM_RC FwDecryptSeed(FWTPM_CTX* ctx,
         if (rc == 0) {
             XMEMCPY(xBuf, encSeedBuf + p, xSz);
             p += xSz;
+            if (p + 2 > encSeedSz) {
+                rc = TPM_RC_SIZE;
+            }
+        }
+        if (rc == 0) {
             ySz = FwLoadU16BE(encSeedBuf + p);
             p += 2;
             if (ySz > MAX_ECC_BYTES || p + ySz > encSeedSz) {
@@ -1433,6 +1446,9 @@ TPM_RC FwEncryptSeed(FWTPM_CTX* ctx,
             wc_FreeRsaKey(rsaKey);
         }
         FWTPM_FREE_VAR(rsaKey);
+        if (rc != 0) {
+            TPM2_ForceZero(seedBuf, seedBufSz);
+        }
     }
     else
 #endif /* !NO_RSA */
@@ -1551,6 +1567,9 @@ TPM_RC FwEncryptSeed(FWTPM_CTX* ctx,
         TPM2_ForceZero(sharedZ, sizeof(sharedZ));
         FWTPM_FREE_VAR(parentPub);
         FWTPM_FREE_VAR(ephemKey);
+        if (rc != 0) {
+            TPM2_ForceZero(seedBuf, seedBufSz);
+        }
     }
     else
 #endif /* HAVE_ECC */
@@ -1677,6 +1696,11 @@ TPM_RC FwImportVerifyAndDecrypt(
     if (rc == 0) {
         *plainSensSzOut = encSensSz;
     }
+    else {
+        /* Zero caller's output buffer on any failure — it may contain
+         * partially decrypted sensitive data. */
+        TPM2_ForceZero(plainSens, plainSensBufSz);
+    }
 
     TPM2_ForceZero(hmacCalc, sizeof(hmacCalc));
     FWTPM_FREE_VAR(aesObj);
@@ -1856,7 +1880,15 @@ TPM_RC FwImportReconstructKey(
                 primeBuf, (word32)primeSz);
         }
         if (rc == 0) {
-            rc = mp_div(&rsaKey->n, &rsaKey->q, &rsaKey->p, NULL);
+            mp_int rem;
+            rc = mp_init(&rem);
+            if (rc == 0) {
+                rc = mp_div(&rsaKey->n, &rsaKey->q, &rsaKey->p, &rem);
+                if (rc == 0 && !mp_iszero(&rem)) {
+                    rc = TPM_RC_BINDING; /* q does not divide n evenly */
+                }
+                mp_forcezero(&rem);
+            }
         }
         if (rc == 0) {
             rc = FwRsaComputeCRT(rsaKey);
@@ -2509,6 +2541,7 @@ TPM_RC FwCredentialDeriveKeys(
     kdfRc = TPM2_KDFa_ex(TPM_ALG_SHA256, seed, seedSz,
         "INTEGRITY", NULL, 0, NULL, 0, hmacKey, hmacKeySz);
     if (kdfRc != hmacKeySz) {
+        TPM2_ForceZero(symKey, symKeySz);
         return TPM_RC_FAILURE;
     }
     return TPM_RC_SUCCESS;
@@ -2676,6 +2709,8 @@ TPM_RC FwCredentialUnwrap(
     }
 
     TPM2_ForceZero(decBuf, FWTPM_MAX_NV_DATA + 2);
+    TPM2_ForceZero(computedHmac, sizeof(computedHmac));
+    TPM2_ForceZero(integrityHmac, sizeof(integrityHmac));
     FWTPM_FREE_BUF(decBuf);
     FWTPM_FREE_VAR(aes);
     FWTPM_FREE_VAR(hmac);

src/fwtpm/fwtpm_nv.c

@@ -1017,6 +1017,16 @@ int FWTPM_NV_Init(FWTPM_CTX* ctx)
         hdr.maxSize  = FwLoadU32LE(hdrBuf + 12);
     }
 
+    if (rc == TPM_RC_SUCCESS &&
+        hdr.magic == FWTPM_NV_MAGIC &&
+        hdr.version == FWTPM_NV_VERSION) {
+        /* Validate writePos against HAL bounds before using it */
+        if (hdr.writePos < sizeof(FWTPM_NV_HEADER) ||
+            hdr.writePos > hal->maxSize) {
+            /* Corrupt — treat as uninitialized, will generate fresh state */
+            rc = TPM_RC_NV_UNINITIALIZED;
+        }
+    }
     if (rc == TPM_RC_SUCCESS &&
         hdr.magic == FWTPM_NV_MAGIC &&
         hdr.version == FWTPM_NV_VERSION) {
@@ -1170,8 +1180,10 @@ int FWTPM_NV_Save(FWTPM_CTX* ctx)
         rc = hal->erase(hal->ctx, 0, hal->maxSize);
     }
 
-    /* Reset write position to after header */
-    ctx->nvWritePos = (word32)sizeof(FWTPM_NV_HEADER);
+    /* Reset write position to after header (only if erase succeeded) */
+    if (rc == 0) {
+        ctx->nvWritePos = (word32)sizeof(FWTPM_NV_HEADER);
+    }
 
     /* Write header (will be updated at end with final writePos) */
     if (rc == 0) {
@@ -1425,6 +1437,7 @@ int FWTPM_NV_Save(FWTPM_CTX* ctx)
 #endif
 
     ctx->nvCompacting = 0;
+    TPM2_ForceZero(buf, bufSz);
     XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     return rc;
 }
@@ -1664,6 +1677,7 @@ int FWTPM_NV_SaveNvIndex(FWTPM_CTX* ctx, int slot)
             buf, (UINT16)pos);
     }
 
+    TPM2_ForceZero(buf, bufSz);
     XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     return rc;
 }
@@ -1713,6 +1727,7 @@ int FWTPM_NV_SavePersistent(FWTPM_CTX* ctx, int slot)
             buf, (UINT16)pos);
     }
 
+    TPM2_ForceZero(buf, bufSz);
     XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     return rc;
 }
@@ -1761,6 +1776,7 @@ int FWTPM_NV_SavePrimaryCache(FWTPM_CTX* ctx, int slot)
             buf, (UINT16)pos);
     }
 
+    TPM2_ForceZero(buf, bufSz);
     XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     return rc;
 }

src/fwtpm/fwtpm_tis.c

@@ -168,18 +168,25 @@ static void TisHandleRegAccess(FWTPM_CTX* ctx, FWTPM_TIS_REGS* regs)
                 /* Write command data to FIFO */
                 UINT32 i;
                 UINT32 space;
+                /* Snapshot write position to local for TOCTOU safety */
+                UINT32 wpos = regs->fifo_write_pos;
+                if (wpos >= FWTPM_TIS_FIFO_SIZE) {
+                    regs->fifo_write_pos = 0;
+                    break;
+                }
                 /* Clamp len to reg_data buffer size */
                 if (len > sizeof(regs->reg_data)) {
                     len = (UINT32)sizeof(regs->reg_data);
                 }
-                space = FWTPM_TIS_FIFO_SIZE - regs->fifo_write_pos;
+                space = FWTPM_TIS_FIFO_SIZE - wpos;
                 if (len > space) {
                     len = space;
                 }
                 for (i = 0; i < len; i++) {
-                    regs->cmd_buf[regs->fifo_write_pos++] = regs->reg_data[i];
+                    regs->cmd_buf[wpos++] = regs->reg_data[i];
                 }
-                regs->cmd_len = regs->fifo_write_pos;
+                regs->fifo_write_pos = wpos;
+                regs->cmd_len = wpos;
 
                 /* Set EXPECT while we still expect more data */
                 regs->sts = TisBuildSts(
@@ -252,12 +259,14 @@ static void TisHandleRegAccess(FWTPM_CTX* ctx, FWTPM_TIS_REGS* regs)
                 /* Read response data from FIFO */
                 UINT32 i;
                 UINT32 avail;
-                if (regs->fifo_read_pos > regs->rsp_len ||
-                        regs->fifo_read_pos >= sizeof(regs->rsp_buf)) {
+                /* Snapshot read state to locals for TOCTOU safety */
+                UINT32 rpos = regs->fifo_read_pos;
+                UINT32 rlen = regs->rsp_len;
+                if (rpos > rlen || rpos >= sizeof(regs->rsp_buf)) {
                     avail = 0;
                 }
                 else {
-                    avail = regs->rsp_len - regs->fifo_read_pos;
+                    avail = rlen - rpos;
                 }
                 if (len > avail) {
                     len = avail;
@@ -266,10 +275,11 @@ static void TisHandleRegAccess(FWTPM_CTX* ctx, FWTPM_TIS_REGS* regs)
                     len = (UINT32)sizeof(regs->reg_data);
                 }
                 for (i = 0; i < len; i++) {
-                    regs->reg_data[i] = regs->rsp_buf[regs->fifo_read_pos++];
+                    regs->reg_data[i] = regs->rsp_buf[rpos++];
                 }
+                regs->fifo_read_pos = rpos;
                 /* Update data availability */
-                if (regs->fifo_read_pos >= regs->rsp_len) {
+                if (rpos >= rlen) {
                     /* All response bytes read */
                     regs->sts = TisBuildSts(
                         FWTPM_STS_VALID | FWTPM_STS_COMMAND_READY,

src/tpm2_crypto.c

@@ -146,6 +146,10 @@ int TPM2_KDFa_ex(
     if (ret == 0) {
         ret = (int)keySz;
     }
+    else {
+        /* Zero partial key material on mid-loop hash error */
+        TPM2_ForceZero(key, keySz);
+    }
     return ret;
 #else
     (void)hashAlg; (void)keyIn; (void)keyInSz; (void)label;
@@ -271,6 +275,10 @@ int TPM2_KDFe_ex(
     if (ret == 0) {
         ret = (int)keySz;
     }
+    else {
+        /* Zero partial key material on mid-loop hash error */
+        TPM2_ForceZero(key, keySz);
+    }
     return ret;
 #else
     (void)hashAlg; (void)Z; (void)ZSz; (void)label;