diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index edb785dbc..1df4efa90 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -4396,13 +4396,20 @@ static TPM_RC FwCmd_ClearControl(FWTPM_CTX* ctx, TPM2_Packet* cmd, } if (rc == 0) { + int oldDisableClear = ctx->disableClear; + #ifdef DEBUG_WOLFTPM printf("fwTPM: ClearControl(auth=0x%x, disable=%d)\n", authHandle, disable); #endif ctx->disableClear = (int)disable; - FWTPM_NV_SaveFlags(ctx); - FwRspNoParams(rsp, cmdTag); + rc = FWTPM_NV_SaveFlags(ctx); + if (rc != 0) { + ctx->disableClear = oldDisableClear; + } + else { + FwRspNoParams(rsp, cmdTag); + } } return rc; diff --git a/src/tpm2_wrap.c b/src/tpm2_wrap.c index 4828e612e..63dd56eeb 100644 --- a/src/tpm2_wrap.c +++ b/src/tpm2_wrap.c @@ -11069,14 +11069,22 @@ int wolfTPM2_PolicyPCRMake(TPM_ALG_ID pcrAlg, byte* pcrArray, word32 pcrArraySz, const byte* pcrDigest, word32 pcrDigestSz, byte* digest, word32* digestSz) { int rc; + int hashSz; TPM2_Packet packet; byte buf[sizeof(TPML_PCR_SELECTION)+WC_MAX_DIGEST_SIZE]; TPML_PCR_SELECTION pcr; if (digest == NULL || digestSz == NULL || pcrArray == NULL || - pcrArraySz == 0) { + pcrArraySz == 0 || (pcrDigest == NULL && pcrDigestSz > 0)) { + return BAD_FUNC_ARG; + } + hashSz = TPM2_GetHashDigestSize(pcrAlg); + if (hashSz <= 0) { return BAD_FUNC_ARG; } + if (*digestSz < (word32)hashSz) { + return BUFFER_E; + } /* Build PCRS (PCR Count and PCR Selection) */ XMEMSET(&pcr, 0, sizeof(pcr)); @@ -11087,12 +11095,14 @@ int wolfTPM2_PolicyPCRMake(TPM_ALG_ID pcrAlg, byte* pcrArray, word32 pcrArraySz, TPM2_Packet_AppendPCR(&packet, &pcr); /* Copy the pcrDigest to the end of buffer */ - if (packet.pos < 0 || (word32)packet.pos > (word32)sizeof(buf) || - pcrDigestSz > (word32)sizeof(buf) - (word32)packet.pos) { + if (packet.overflow || packet.pos > packet.size || + pcrDigestSz > (word32)(packet.size - packet.pos)) { return BUFFER_E; } - XMEMCPY(buf + packet.pos, pcrDigest, pcrDigestSz); - packet.pos += pcrDigestSz; + if (pcrDigestSz > 0) { + XMEMCPY(buf + packet.pos, pcrDigest, pcrDigestSz); + packet.pos += (int)pcrDigestSz; + } rc = wolfTPM2_PolicyHash(pcrAlg, digest, digestSz, TPM_CC_PolicyPCR, buf, packet.pos); diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index dd60a64a6..ff5d86eff 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -9494,7 +9494,13 @@ static void test_fwtpm_pcr_properties_capability(void) rspSize = 0; FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); AssertIntEQ(GetRspRC(gRsp), TPM_RC_SUCCESS); - AssertTrue(rspSize > 0 && rspSize <= (int)sizeof(gRsp)); + if (rspSize < TPM2_HEADER_SIZE + 9 || + rspSize > (int)sizeof(gRsp)) { + AssertTrue(rspSize >= TPM2_HEADER_SIZE + 9 && + rspSize <= (int)sizeof(gRsp)); + FWTPM_Cleanup(&ctx); + return; + } /* header(10) + moreData(1) + capability(4) + count(4) + properties */ p = TPM2_HEADER_SIZE + 1; @@ -9502,28 +9508,39 @@ static void test_fwtpm_pcr_properties_capability(void) AssertIntEQ(cap, TPM_CAP_PCR_PROPERTIES); count = GetU32BE(gRsp + p); p += 4; AssertIntGT((int)count, 0); - AssertTrue((int)count <= 32); /* bounded by the 32 records requested above */ + if (count == 0 || count > 32) { + AssertTrue(count > 0 && count <= 32); + FWTPM_Cleanup(&ctx); + return; + } for (i = 0; i < (int)count; i++) { - AssertTrue(p + 5 <= rspSize); /* room for tag(4)+size(1) */ + if (p > rspSize || rspSize - p < 5) { + AssertTrue(p <= rspSize && rspSize - p >= 5); + FWTPM_Cleanup(&ctx); + return; + } tag = GetU32BE(gRsp + p); p += 4; wireSz = gRsp[p]; p += 1; + if (wireSz <= 0 || p > rspSize || wireSz > rspSize - p) { + AssertTrue(wireSz > 0 && p <= rspSize && + wireSz <= rspSize - p); + FWTPM_Cleanup(&ctx); + return; + } selSz = (wireSz > 8) ? 8 : wireSz; - AssertTrue(selSz > 0); /* select bytes present; keeps p inside gRsp */ - AssertTrue(p + selSz <= rspSize); if (tag == TPM_PT_PCR_RESET_L0) { - memcpy(resetL0, gRsp + p, selSz); gotResetL0 = 1; + XMEMCPY(resetL0, gRsp + p, selSz); gotResetL0 = 1; } else if (tag == TPM_PT_PCR_RESET_L4) { - memcpy(resetL4, gRsp + p, selSz); gotResetL4 = 1; + XMEMCPY(resetL4, gRsp + p, selSz); gotResetL4 = 1; } else if (tag == TPM_PT_PCR_EXTEND_L0) { - memcpy(extendL0, gRsp + p, selSz); gotExtendL0 = 1; + XMEMCPY(extendL0, gRsp + p, selSz); gotExtendL0 = 1; } else if (tag == TPM_PT_PCR_DRTM_RESET) { - memcpy(drtm, gRsp + p, selSz); gotDrtm = 1; + XMEMCPY(drtm, gRsp + p, selSz); gotDrtm = 1; } - AssertTrue(p + wireSz <= rspSize); /* full record present on the wire */ p += wireSz; /* advance past the select bytes */ } @@ -9669,6 +9686,54 @@ static void test_fwtpm_clear(void) fwtpm_pass("Clear(LOCKOUT):", 0); } +#ifndef FWTPM_NO_NV +static int fail_nv_write(void* ctx, word32 offset, const byte* buf, + word32 size) +{ + (void)ctx; + (void)offset; + (void)buf; + (void)size; + return TPM_RC_FAILURE; +} + +/* ClearControl must not change disableClear or return success when its NV + * update fails. */ +static void test_fwtpm_clear_control_nv_failure(void) +{ + FWTPM_CTX ctx; + FWTPM_NV_HAL oldHal, failHal; + int rc, rspSize, pos; + + memset(&ctx, 0, sizeof(ctx)); + AssertIntEQ(fwtpm_test_startup(&ctx), 0); + AssertIntEQ(ctx.disableClear, 0); + + oldHal = ctx.nvHal; + failHal = oldHal; + failHal.write = fail_nv_write; + AssertIntEQ(FWTPM_NV_SetHAL(&ctx, &failHal), TPM_RC_SUCCESS); + + pos = 0; + PutU16BE(gCmd + pos, TPM_ST_SESSIONS); pos += 2; + PutU32BE(gCmd + pos, 0); pos += 4; + PutU32BE(gCmd + pos, TPM_CC_ClearControl); pos += 4; + PutU32BE(gCmd + pos, TPM_RH_PLATFORM); pos += 4; + pos = AppendPwAuth(gCmd, pos, NULL, 0); + gCmd[pos++] = 1; + PutU32BE(gCmd + 2, (UINT32)pos); + rspSize = 0; + rc = FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(GetRspRC(gRsp), TPM_RC_FAILURE); + AssertIntEQ(ctx.disableClear, 0); + + AssertIntEQ(FWTPM_NV_SetHAL(&ctx, &oldHal), TPM_RC_SUCCESS); + FWTPM_Cleanup(&ctx); + fwtpm_pass("ClearControl NV failure rollback:", 0); +} +#endif /* !FWTPM_NO_NV */ + /* Per Part 3 Sec.24.6 Table 134, TPM2_Clear has Auth Index 1, Auth Role USER * on @authHandle (TPM_RH_LOCKOUT or TPM_RH_PLATFORM). NO_SESSIONS leaves * cmdAuthCnt at 0, skipping every auth enforcement loop in @@ -12285,6 +12350,9 @@ int fwtpm_unit_tests(int argc, char *argv[]) test_fwtpm_sessions_short_authcount_rejected(); #endif /* !FWTPM_NO_HASH_CMDS */ test_fwtpm_sessions_missing_authsize_command_size(); +#ifndef FWTPM_NO_NV + test_fwtpm_clear_control_nv_failure(); +#endif /* !FWTPM_NO_NV */ test_fwtpm_clear(); printf("\nAll fwTPM unit tests passed!\n"); diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 46fcc8b1a..826d11c6c 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -1164,6 +1164,18 @@ static void test_TPM2_PCRSel(void) static void test_TPM2_Policy_NULL_Args(void) { int rc; + #ifndef WOLFTPM2_NO_WOLFCRYPT + const byte expectedDigest[TPM_SHA256_DIGEST_SIZE] = { + 0x4e, 0x35, 0x3a, 0xbb, 0x5b, 0x73, 0xa0, 0x8b, + 0x9c, 0x1c, 0x53, 0x1e, 0x02, 0x27, 0x9a, 0xa9, + 0x39, 0xb6, 0xb5, 0x61, 0x2d, 0xe3, 0x59, 0x6d, + 0x74, 0xfe, 0xd8, 0x99, 0x9b, 0xef, 0x13, 0xdf + }; + byte pcrArray[1] = {0}; + byte pcrDigest[sizeof(TPML_PCR_SELECTION) + WC_MAX_DIGEST_SIZE + 1] = {0}; + byte digest[TPM_SHA256_DIGEST_SIZE]; + word32 digestSz = (word32)sizeof(digest); + #endif /* Test NULL input handling for policy commands */ rc = TPM2_PolicyPhysicalPresence(NULL); @@ -1175,6 +1187,55 @@ static void test_TPM2_Policy_NULL_Args(void) rc = TPM2_PolicyPassword(NULL); AssertIntEQ(rc, BAD_FUNC_ARG); + #ifndef WOLFTPM2_NO_WOLFCRYPT + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), NULL, 0, NULL, &digestSz); + AssertIntEQ(rc, BAD_FUNC_ARG); + + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), NULL, 0, digest, NULL); + AssertIntEQ(rc, BAD_FUNC_ARG); + + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, NULL, + (word32)sizeof(pcrArray), NULL, 0, digest, &digestSz); + AssertIntEQ(rc, BAD_FUNC_ARG); + + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, 0, + NULL, 0, digest, &digestSz); + AssertIntEQ(rc, BAD_FUNC_ARG); + + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_NULL, pcrArray, + (word32)sizeof(pcrArray), NULL, 0, digest, &digestSz); + AssertIntEQ(rc, BAD_FUNC_ARG); + + /* A nonzero PCR digest size requires backing digest data. */ + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), NULL, 1, digest, &digestSz); + AssertIntEQ(rc, BAD_FUNC_ARG); + + /* An empty PCR digest is valid. */ + XMEMSET(digest, 0, sizeof(digest)); + digestSz = (word32)sizeof(digest); + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), NULL, 0, digest, &digestSz); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(digestSz, (word32)sizeof(expectedDigest)); + AssertIntEQ(XMEMCMP(digest, expectedDigest, sizeof(expectedDigest)), 0); + + /* Reject a caller-declared output capacity below the hash size. */ + digestSz = (word32)sizeof(digest) - 1; + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), NULL, 0, digest, &digestSz); + AssertIntEQ(rc, BUFFER_E); + + /* Reject a PCR digest that cannot fit in the assembly buffer. */ + digestSz = (word32)sizeof(digest); + rc = wolfTPM2_PolicyPCRMake(TPM_ALG_SHA256, pcrArray, + (word32)sizeof(pcrArray), pcrDigest, (word32)sizeof(pcrDigest), digest, + &digestSz); + AssertIntEQ(rc, BUFFER_E); + #endif + printf("Test TPM2: %-40s Passed\n", "Policy NULL Args:"); } diff --git a/wolftpm/tpm2_wrap.h b/wolftpm/tpm2_wrap.h index 681c261cb..c83822370 100644 --- a/wolftpm/tpm2_wrap.h +++ b/wolftpm/tpm2_wrap.h @@ -4897,17 +4897,24 @@ WOLFTPM_API int wolfTPM2_PolicyRefMake(TPM_ALG_ID pcrAlg, byte* digest, word32* \brief Utility for generating a policy PCR digest. + \note To start a fresh policy chain, zero the digest buffer and set + digestSz to the selected hash size before calling. + \return TPM_RC_SUCCESS: successful - \return INPUT_SIZE_E: policyDigestSz is too small to hold the returned digest + \return BUFFER_E: digest is too small for the selected hash or the PCR + policy input exceeds the internal assembly buffer \return BAD_FUNC_ARG: check the provided arguments - \param pcrAlg the hash algorithm to use with pcr policy - \param pcrArray optional array of pcrs to be used when creating the tpm object - \param pcrArraySz length of the pcrArray - \param pcrDigest digest for the PCR(s) collected (can get using wolfTPM2_PCRGetDigest) + \param pcrAlg the supported hash algorithm to use with the PCR policy + \param pcrArray non-NULL array of PCRs to include in the policy + \param pcrArraySz number of entries in pcrArray; must be greater than zero + \param pcrDigest digest for the PCR(s) collected (can get using + wolfTPM2_PCRGetDigest); required when pcrDigestSz is nonzero \param pcrDigestSz size of the PCR digest - \param digest input/out digest - \param digestSz input/out digest size + \param digest input/output policy digest buffer + \param digestSz input/output: current digest size and buffer capacity on + input, which must be at least the selected hash size; selected hash + size on output \sa wolfTPM2_PolicyPCRMake \sa wolfTPM2_PolicyAuthorizeMake