fwtpm: peer-review batch 3 (design changes)

dgarske · dgarske · commit b0c0bbabb57e · 2026-04-12T14:53:28.000-07:00
ContextSave/ContextLoad integrity + confidentiality:
- Add per-boot ctxProtectKey (volatile, 32-byte random) in FWTPM_CTX
- FwWrapContextBlob: AES-CFB encrypt then HMAC-SHA256 session blob
- FwUnwrapContextBlob: verify HMAC then AES-CFB decrypt, reject on
  integrity failure with TPM_RC_INTEGRITY
- ContextLoad validates restored session fields (sessionType, authHash)
- ContextSave uses TPM2_ForceZero instead of XMEMSET for session clear
- Addresses CWE-502 (deserialization) and CWE-311 (missing encryption)

Auth framework:
- Policy session validation now checks hierarchy handles (owner,
  endorsement, platform, lockout) against SetPrimaryPolicy authPolicy
- PolicyRestart resets all policy flags (isPasswordPolicy,
  isAuthValuePolicy, isPPRequired, cpHashA, nameHash)
- wolfTPM2_SetSessionHandle: return BUFFER_E instead of truncating
diff --git a/src/fwtpm/fwtpm.c b/src/fwtpm/fwtpm.c
@@ -91,6 +91,19 @@ int FWTPM_Init(FWTPM_CTX* ctx)
         return rc;
     }
 
+    /* Generate per-boot context protection key (volatile only) for
+     * ContextSave/ContextLoad HMAC + AES-CFB session blob protection. */
+    rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->ctxProtectKey,
+        sizeof(ctx->ctxProtectKey));
+    if (rc == 0) {
+        ctx->ctxProtectKeyValid = 1;
+    }
+    else {
+        wc_FreeRng(&ctx->rng);
+        wolfCrypt_Cleanup();
+        return rc;
+    }
+
     /* Initialize NV storage - loads existing state or creates fresh seeds */
 #ifndef FWTPM_NO_NV
     rc = FWTPM_NV_Init(ctx);
diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c
@@ -2580,17 +2580,26 @@ static TPM_RC FwCmd_ContextSave(FWTPM_CTX* ctx, TPM2_Packet* cmd,
         TPM2_Packet_AppendU32(rsp, hierarchy);
 
         if (isSession && sess != NULL) {
-            /* Session: serialize full FWTPM_Session into blob and free slot.
-             * Format: magic(4) | version(4) | FWTPM_Session (raw struct) */
-            blobSz = 4 + 4 + (UINT16)sizeof(FWTPM_Session);
-            TPM2_Packet_AppendU16(rsp, blobSz);
-            tmp32 = TPM2_Packet_SwapU32(FWTPM_CTX_MAGIC);
-            TPM2_Packet_AppendBytes(rsp, (byte*)&tmp32, 4);
-            tmp32 = TPM2_Packet_SwapU32(FWTPM_CTX_VER);
-            TPM2_Packet_AppendBytes(rsp, (byte*)&tmp32, 4);
-            TPM2_Packet_AppendBytes(rsp, (byte*)sess, sizeof(FWTPM_Session));
-            /* Free the session slot — ContextLoad will restore it */
-            XMEMSET(sess, 0, sizeof(FWTPM_Session));
+            /* Session: HMAC + AES-CFB protected blob per TPM 2.0 §30.
+             * Format: magic(4) | version(4) | wrappedBlob(iv+cipher+hmac) */
+            byte wrappedBuf[AES_BLOCK_SIZE + sizeof(FWTPM_Session) +
+                WC_SHA256_DIGEST_SIZE];
+            int wrappedSz = 0;
+            rc = FwWrapContextBlob(ctx, (const byte*)sess,
+                (int)sizeof(FWTPM_Session),
+                wrappedBuf, (int)sizeof(wrappedBuf), &wrappedSz);
+            if (rc == 0) {
+                blobSz = 4 + 4 + (UINT16)wrappedSz;
+                TPM2_Packet_AppendU16(rsp, blobSz);
+                tmp32 = TPM2_Packet_SwapU32(FWTPM_CTX_MAGIC);
+                TPM2_Packet_AppendBytes(rsp, (byte*)&tmp32, 4);
+                tmp32 = TPM2_Packet_SwapU32(FWTPM_CTX_VER);
+                TPM2_Packet_AppendBytes(rsp, (byte*)&tmp32, 4);
+                TPM2_Packet_AppendBytes(rsp, wrappedBuf, wrappedSz);
+            }
+            TPM2_ForceZero(wrappedBuf, sizeof(wrappedBuf));
+            /* Free the session slot */
+            TPM2_ForceZero(sess, sizeof(FWTPM_Session));
         }
         else {
             /* Object: opaque handle reference (object stays in slot).
@@ -2660,14 +2669,44 @@ static TPM_RC FwCmd_ContextLoad(FWTPM_CTX* ctx, TPM2_Packet* cmd,
 
         if ((savedHandle & 0xFF000000) == HMAC_SESSION_FIRST ||
             (savedHandle & 0xFF000000) == POLICY_SESSION_FIRST) {
-            /* Session context: restore FWTPM_Session from blob into free slot */
-            if (dataLen != (UINT16)sizeof(FWTPM_Session)) {
+            /* Session context: verify integrity + decrypt, then restore.
+             * Wrapped blob: iv(16) + ciphertext(sizeof(FWTPM_Session)) + hmac(32) */
+            int expectedWrapSz = AES_BLOCK_SIZE +
+                (int)sizeof(FWTPM_Session) + WC_SHA256_DIGEST_SIZE;
+            FWTPM_Session restored;
+            int restoredSz = 0;
+            int si;
+            int found = 0;
+
+            if ((int)dataLen != expectedWrapSz) {
                 rc = TPM_RC_SIZE;
             }
             if (rc == 0) {
-                /* Restore session to any free slot */
-                int si;
-                int found = 0;
+                byte wrappedBuf[AES_BLOCK_SIZE + sizeof(FWTPM_Session) +
+                    WC_SHA256_DIGEST_SIZE];
+                TPM2_Packet_ParseBytes(cmd, wrappedBuf, (int)dataLen);
+                rc = FwUnwrapContextBlob(ctx, wrappedBuf, (int)dataLen,
+                    (byte*)&restored, (int)sizeof(restored), &restoredSz);
+                TPM2_ForceZero(wrappedBuf, sizeof(wrappedBuf));
+                if (rc != 0) {
+                    /* HMAC mismatch or decrypt failure */
+                    if (rc != TPM_RC_INTEGRITY)
+                        rc = TPM_RC_INTEGRITY;
+                }
+            }
+            /* Validate deserialized session fields */
+            if (rc == 0) {
+                if (restored.sessionType != TPM_SE_HMAC &&
+                    restored.sessionType != TPM_SE_POLICY &&
+                    restored.sessionType != TPM_SE_TRIAL) {
+                    rc = TPM_RC_VALUE;
+                }
+                if (TPM2_GetHashDigestSize(restored.authHash) <= 0) {
+                    rc = TPM_RC_VALUE;
+                }
+            }
+            if (rc == 0) {
+                /* Find a free session slot */
                 for (si = 0; si < FWTPM_MAX_SESSIONS; si++) {
                     if (!ctx->sessions[si].used) {
                         found = 1;
@@ -2677,14 +2716,15 @@ static TPM_RC FwCmd_ContextLoad(FWTPM_CTX* ctx, TPM2_Packet* cmd,
                 if (!found) {
                     rc = TPM_RC_SESSION_MEMORY;
                 }
-                if (rc == 0) {
-                    TPM2_Packet_ParseBytes(cmd, (byte*)&ctx->sessions[si],
-                        sizeof(FWTPM_Session));
-                    /* Keep the original handle — cpHash includes session
-                     * handle as entity name, so it must match what ESYS
-                     * computed. FwFindSession searches by handle value. */
-                }
             }
+            if (rc == 0) {
+                XMEMCPY(&ctx->sessions[si], &restored,
+                    sizeof(FWTPM_Session));
+                /* Keep the original handle — cpHash includes session
+                 * handle as entity name, so it must match what ESYS
+                 * computed. FwFindSession searches by handle value. */
+            }
+            TPM2_ForceZero(&restored, sizeof(restored));
         }
         else if ((savedHandle & 0xFF000000) == TRANSIENT_FIRST) {
             /* Object context: verify object still in slot (opaque handle) */
@@ -7294,6 +7334,15 @@ static TPM_RC FwCmd_PolicyRestart(FWTPM_CTX* ctx, TPM2_Packet* cmd,
         /* Reset policy digest to all zeros */
         XMEMSET(sess->policyDigest.buffer, 0, sess->policyDigest.size);
 
+        /* Reset all policy-related session flags so stale assertions from
+         * prior policy evaluations don't affect future HMAC computation
+         * or authorization checks. */
+        sess->isPasswordPolicy = 0;
+        sess->isAuthValuePolicy = 0;
+        sess->isPPRequired = 0;
+        sess->cpHashA.size = 0;
+        sess->nameHash.size = 0;
+
         FwRspFinalize(rsp, TPM_ST_NO_SESSIONS, TPM_RC_SUCCESS);
     }
 
@@ -12477,6 +12526,19 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
                     authPolicy = &objEnt->pub.authPolicy;
                 }
             }
+            /* Hierarchy handles: check SetPrimaryPolicy-assigned policy */
+            else if (entityH == TPM_RH_OWNER) {
+                authPolicy = &ctx->ownerPolicy;
+            }
+            else if (entityH == TPM_RH_ENDORSEMENT) {
+                authPolicy = &ctx->endorsementPolicy;
+            }
+            else if (entityH == TPM_RH_PLATFORM) {
+                authPolicy = &ctx->platformPolicy;
+            }
+            else if (entityH == TPM_RH_LOCKOUT) {
+                authPolicy = &ctx->lockoutPolicy;
+            }
 
             /* If entity has a non-empty authPolicy, it must match */
             if (authPolicy != NULL && authPolicy->size > 0) {
diff --git a/src/fwtpm/fwtpm_crypto.c b/src/fwtpm/fwtpm_crypto.c
@@ -1208,6 +1208,163 @@ int FwUnwrapPrivate(FWTPM_Object* parent,
     return rc;
 }
 
+/* ================================================================== */
+/* Context blob wrap/unwrap (ContextSave/ContextLoad)                  */
+/* ================================================================== */
+
+/* Encrypt-then-MAC context blob protection using the per-boot key.
+ * Layout: iv(16) | ciphertext(plainSz) | hmac(32)
+ * Returns 0 on success, sets *outSz. */
+int FwWrapContextBlob(FWTPM_CTX* ctx,
+    const byte* plain, int plainSz,
+    byte* out, int outBufSz, int* outSz)
+{
+    int rc = TPM_RC_SUCCESS;
+    byte iv[AES_BLOCK_SIZE];
+    FWTPM_DECLARE_VAR(aes, Aes);
+    FWTPM_DECLARE_VAR(hmac, Hmac);
+    int aesInit = 0;
+    int totalSz = AES_BLOCK_SIZE + plainSz + WC_SHA256_DIGEST_SIZE;
+
+    *outSz = 0;
+    if (!ctx->ctxProtectKeyValid || totalSz > outBufSz) {
+        return TPM_RC_FAILURE;
+    }
+
+    FWTPM_ALLOC_VAR(aes, Aes);
+    FWTPM_ALLOC_VAR(hmac, Hmac);
+
+    /* Generate random IV */
+    rc = wc_RNG_GenerateBlock(&ctx->rng, iv, AES_BLOCK_SIZE);
+
+    /* AES-CFB encrypt */
+    if (rc == 0) {
+        rc = wc_AesInit(aes, NULL, INVALID_DEVID);
+        if (rc == 0) {
+            aesInit = 1;
+            rc = wc_AesSetKey(aes, ctx->ctxProtectKey, 32, iv, AES_ENCRYPTION);
+        }
+    }
+    if (rc == 0) {
+        XMEMCPY(out, iv, AES_BLOCK_SIZE);
+        rc = wc_AesCfbEncrypt(aes, out + AES_BLOCK_SIZE, plain, plainSz);
+    }
+    if (aesInit) {
+        wc_AesFree(aes);
+    }
+
+    /* HMAC-SHA256 over iv || ciphertext */
+    if (rc == 0) {
+        rc = wc_HmacInit(hmac, NULL, INVALID_DEVID);
+    }
+    if (rc == 0) {
+        rc = wc_HmacSetKey(hmac, WC_SHA256,
+            ctx->ctxProtectKey, sizeof(ctx->ctxProtectKey));
+    }
+    if (rc == 0) {
+        rc = wc_HmacUpdate(hmac, out, AES_BLOCK_SIZE + plainSz);
+    }
+    if (rc == 0) {
+        rc = wc_HmacFinal(hmac, out + AES_BLOCK_SIZE + plainSz);
+    }
+    wc_HmacFree(hmac);
+
+    if (rc == 0) {
+        *outSz = totalSz;
+    }
+    else {
+        TPM2_ForceZero(out, totalSz);
+        rc = TPM_RC_FAILURE;
+    }
+
+    TPM2_ForceZero(iv, sizeof(iv));
+    FWTPM_FREE_VAR(aes);
+    FWTPM_FREE_VAR(hmac);
+    return rc;
+}
+
+/* Verify-then-decrypt context blob. Returns 0 on success, sets *outSz. */
+int FwUnwrapContextBlob(FWTPM_CTX* ctx,
+    const byte* in, int inSz,
+    byte* out, int outBufSz, int* outSz)
+{
+    int rc = TPM_RC_SUCCESS;
+    byte computedHmac[WC_SHA256_DIGEST_SIZE];
+    FWTPM_DECLARE_VAR(aes, Aes);
+    FWTPM_DECLARE_VAR(hmac, Hmac);
+    int aesInit = 0;
+    int cipherSz;
+
+    *outSz = 0;
+    /* Minimum: iv(16) + at least 1 byte ciphertext + hmac(32) */
+    if (!ctx->ctxProtectKeyValid ||
+        inSz < AES_BLOCK_SIZE + 1 + WC_SHA256_DIGEST_SIZE) {
+        return TPM_RC_FAILURE;
+    }
+
+    cipherSz = inSz - AES_BLOCK_SIZE - WC_SHA256_DIGEST_SIZE;
+    if (cipherSz > outBufSz) {
+        return TPM_RC_SIZE;
+    }
+
+    FWTPM_ALLOC_VAR(aes, Aes);
+    FWTPM_ALLOC_VAR(hmac, Hmac);
+
+    /* Verify HMAC over iv || ciphertext */
+    if (rc == 0) {
+        rc = wc_HmacInit(hmac, NULL, INVALID_DEVID);
+    }
+    if (rc == 0) {
+        rc = wc_HmacSetKey(hmac, WC_SHA256,
+            ctx->ctxProtectKey, sizeof(ctx->ctxProtectKey));
+    }
+    if (rc == 0) {
+        rc = wc_HmacUpdate(hmac, in, AES_BLOCK_SIZE + cipherSz);
+    }
+    if (rc == 0) {
+        rc = wc_HmacFinal(hmac, computedHmac);
+    }
+    wc_HmacFree(hmac);
+
+    if (rc == 0) {
+        if (TPM2_ConstantCompare(computedHmac,
+                in + AES_BLOCK_SIZE + cipherSz,
+                WC_SHA256_DIGEST_SIZE) != 0) {
+            rc = TPM_RC_INTEGRITY;
+        }
+    }
+
+    /* AES-CFB decrypt using IV from blob.
+     * CFB mode uses the encrypt direction internally for both
+     * encryption and decryption (encrypts IV, XORs with data). */
+    if (rc == 0) {
+        rc = wc_AesInit(aes, NULL, INVALID_DEVID);
+        if (rc == 0) {
+            aesInit = 1;
+            rc = wc_AesSetKey(aes, ctx->ctxProtectKey, 32, in, AES_ENCRYPTION);
+        }
+    }
+    if (rc == 0) {
+        rc = wc_AesCfbDecrypt(aes, out, in + AES_BLOCK_SIZE, cipherSz);
+    }
+    if (aesInit) {
+        wc_AesFree(aes);
+    }
+
+    if (rc == 0) {
+        *outSz = cipherSz;
+    }
+    else if (rc != TPM_RC_INTEGRITY) {
+        TPM2_ForceZero(out, cipherSz);
+        rc = TPM_RC_FAILURE;
+    }
+
+    TPM2_ForceZero(computedHmac, sizeof(computedHmac));
+    FWTPM_FREE_VAR(aes);
+    FWTPM_FREE_VAR(hmac);
+    return rc;
+}
+
 /* ================================================================== */
 /* Seed encrypt/decrypt                                                */
 /* ================================================================== */
diff --git a/src/tpm2_wrap.c b/src/tpm2_wrap.c
@@ -1866,7 +1866,7 @@ int wolfTPM2_SetSessionHandle(WOLFTPM2_DEV* dev, int index,
 
         session->auth.size = tpmSession->handle.auth.size;
         if (session->auth.size > sizeof(session->auth.buffer)) {
-            session->auth.size = sizeof(session->auth.buffer); /* truncate */
+            return BUFFER_E;
         }
         XMEMCPY(session->auth.buffer, tpmSession->handle.auth.buffer,
             session->auth.size);
diff --git a/wolftpm/fwtpm/fwtpm.h b/wolftpm/fwtpm/fwtpm.h
@@ -425,6 +425,12 @@ typedef struct FWTPM_CTX {
     TPM2B_DIGEST lockoutPolicy;
     TPMI_ALG_HASH lockoutPolicyAlg;
 
+    /* Per-boot context protection key (volatile only, never persisted).
+     * Used by ContextSave/ContextLoad for HMAC + AES-CFB protection of
+     * session context blobs per TPM 2.0 Part 1 §30. */
+    byte ctxProtectKey[32];
+    int  ctxProtectKeyValid;
+
     /* TIS transport state (when not using sockets) */
 #ifdef WOLFTPM_FWTPM_TIS
     FWTPM_TIS_HAL  tisHal;      /* Transport HAL callbacks */
diff --git a/wolftpm/fwtpm/fwtpm_crypto.h b/wolftpm/fwtpm/fwtpm_crypto.h
@@ -162,6 +162,15 @@ int FwUnwrapPrivate(FWTPM_Object* parent,
     UINT16* sensitiveType, TPM2B_AUTH* auth,
     byte* privKeyDer, int* privKeyDerSz);
 
+/* --- Context blob wrap/unwrap (ContextSave/Load) --- */
+
+int FwWrapContextBlob(FWTPM_CTX* ctx,
+    const byte* plain, int plainSz,
+    byte* out, int outBufSz, int* outSz);
+int FwUnwrapContextBlob(FWTPM_CTX* ctx,
+    const byte* in, int inSz,
+    byte* out, int outBufSz, int* outSz);
+
 /* --- Seed encrypt/decrypt --- */
 
 TPM_RC FwDecryptSeed(FWTPM_CTX* ctx,

Original file line number	Diff line number	Diff line change
`@@ -1866,7 +1866,7 @@ int wolfTPM2_SetSessionHandle(WOLFTPM2_DEV* dev, int index,`
`1866`	`1866`
`1867`	`1867`	`session->auth.size = tpmSession->handle.auth.size;`
`1868`	`1868`	`if (session->auth.size > sizeof(session->auth.buffer)) {`
`1869`		`- session->auth.size = sizeof(session->auth.buffer); /* truncate */`
	`1869`	`+ return BUFFER_E;`
`1870`	`1870`	`}`
`1871`	`1871`	`XMEMCPY(session->auth.buffer, tpmSession->handle.auth.buffer,`
`1872`	`1872`	`session->auth.size);`