diff --git a/src/fwtpm/fwtpm_crypto.c b/src/fwtpm/fwtpm_crypto.c index 215ce863..cf8f285e 100644 --- a/src/fwtpm/fwtpm_crypto.c +++ b/src/fwtpm/fwtpm_crypto.c @@ -418,31 +418,36 @@ int FwAppendCreationHashAndTicket(FWTPM_CTX* ctx, TPM2_Packet* rsp, int FwGetWcCurveId(UINT16 tpmCurve) { int curveIdx; - int keyBits; int wcCurve; switch (tpmCurve) { case TPM_ECC_NIST_P256: - keyBits = 256; + #if ECC_MIN_KEY_SZ > 256 + return -1; + #else wcCurve = ECC_SECP256R1; break; + #endif case TPM_ECC_NIST_P384: - keyBits = 384; + #if ECC_MIN_KEY_SZ > 384 + return -1; + #else wcCurve = ECC_SECP384R1; break; + #endif #ifdef FWTPM_HAVE_ECC521 case TPM_ECC_NIST_P521: - keyBits = 521; + #if ECC_MIN_KEY_SZ > 521 + return -1; + #else wcCurve = ECC_SECP521R1; break; + #endif #endif default: return -1; } - if (keyBits < ECC_MIN_KEY_SZ) { - return -1; - } curveIdx = wc_ecc_get_curve_idx(wcCurve); if (curveIdx < 0 || wc_ecc_get_curve_params(curveIdx) == NULL) { return -1; diff --git a/src/tpm2.c b/src/tpm2.c index 21e518b1..e3d1ec67 100644 --- a/src/tpm2.c +++ b/src/tpm2.c @@ -88,6 +88,11 @@ static THREAD_LS_T TPM2_CTX* gActiveTPM; #define TPM2_LOCALITY_UNINITIALIZED (-1) +#if !defined(WOLFTPM2_NO_WOLFCRYPT) && !defined(WOLFTPM_NO_LOCK) && \ + !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) +#define TPM2_DYNAMIC_HW_LOCK +#endif + /******************************************************************************/ /* --- Local Functions -- */ /******************************************************************************/ @@ -1019,9 +1024,11 @@ TPM_RC TPM2_Init(TPM2_CTX* ctx, TPM2HalIoCb ioCb, void* userCtx) TPM_RC TPM2_Cleanup(TPM2_CTX* ctx) { TPM_RC rc; - int lockCtx = 1; #ifndef WOLFTPM2_NO_WOLFCRYPT int wolfCryptInit; + #ifdef TPM2_DYNAMIC_HW_LOCK + int lockCtx; + #endif #endif if (ctx == NULL) @@ -1029,23 +1036,32 @@ TPM_RC TPM2_Cleanup(TPM2_CTX* ctx) #ifndef WOLFTPM2_NO_WOLFCRYPT wolfCryptInit = (ctx->locality != TPM2_LOCALITY_UNINITIALIZED); - #if !defined(WOLFTPM_NO_LOCK) && !defined(SINGLE_THREADED) && \ - !defined(WOLFSSL_MUTEX_INITIALIZER) + #ifdef TPM2_DYNAMIC_HW_LOCK lockCtx = wolfCryptInit; #endif #endif /* clear global */ - if (lockCtx) + #ifdef TPM2_DYNAMIC_HW_LOCK + if (lockCtx) { rc = TPM2_AcquireLock(ctx); - else + } + else { rc = TPM_RC_SUCCESS; + } + #else + rc = TPM2_AcquireLock(ctx); + #endif if (rc == TPM_RC_SUCCESS) { if (TPM2_GetActiveCtx() == ctx) { + #ifdef TPM2_DYNAMIC_HW_LOCK if (lockCtx) { TPM2_INTERNAL_CLEANUP(ctx); } + #else + TPM2_INTERNAL_CLEANUP(ctx); + #endif /* set non-active */ TPM2_SetActiveCtx(NULL); } @@ -1054,8 +1070,13 @@ TPM_RC TPM2_Cleanup(TPM2_CTX* ctx) * auth values, decrypted parameters) */ TPM2_ForceZero(ctx->cmdBuf, sizeof(ctx->cmdBuf)); - if (lockCtx) + #ifdef TPM2_DYNAMIC_HW_LOCK + if (lockCtx) { TPM2_ReleaseLock(ctx); + } + #else + TPM2_ReleaseLock(ctx); + #endif } else { TPM2_ForceZero(ctx->cmdBuf, sizeof(ctx->cmdBuf)); diff --git a/src/tpm2_wrap.c b/src/tpm2_wrap.c index 2fe73d79..96999072 100644 --- a/src/tpm2_wrap.c +++ b/src/tpm2_wrap.c @@ -2065,13 +2065,26 @@ int wolfTPM2_SetAuthHandle(WOLFTPM2_DEV* dev, int index, if (handle->policyAuth) { TPM2_AUTH_SESSION* session = &dev->session[index]; int authDigestSz = TPM2_GetHashDigestSize(session->authHash); + word32 authSz; + + /* Validate bounds before reading caller data or mutating the + * session so it isn't left inconsistent on BUFFER_E. */ + if (authDigestSz <= 0 || + (word32)authDigestSz > + (word32)sizeof(session->auth.buffer) || + handle->auth.size > (word32)sizeof(session->auth.buffer) - + (word32)authDigestSz || + handle->name.size > sizeof(session->name.name)) { + return BUFFER_E; + } + authSz = (word32)authDigestSz + handle->auth.size; #ifdef WOLFTPM_DEBUG_VERBOSE printf("Session %d: Edit (PolicyAuth)\n", index); printf("\tHandle 0x%x (not touching)\n", session->sessionHandle); printf("\tPolicyAuth %d->%d\n", session->policyAuth, handle->policyAuth); - printf("\tAuth Sz %d -> %d\n", session->auth.size, - authDigestSz + handle->auth.size); + printf("\tAuth Sz %d -> %u\n", session->auth.size, + (unsigned int)authSz); #ifdef WOLFTPM_DEBUG_SECRETS TPM2_PrintBin(session->auth.buffer, session->auth.size); TPM2_PrintBin(handle->auth.buffer, handle->auth.size); @@ -2080,22 +2093,13 @@ int wolfTPM2_SetAuthHandle(WOLFTPM2_DEV* dev, int index, TPM2_PrintBin(session->name.name, session->name.size); TPM2_PrintBin(handle->name.name, handle->name.size); #endif - /* Validate bounds before any session-state mutation so the - * session isn't left inconsistent on BUFFER_E. */ - if (authDigestSz <= 0 || - (handle->auth.size + authDigestSz) > - (int)sizeof(session->auth.buffer) || - handle->name.size > sizeof(session->name.name)) { - return BUFFER_E; - } session->policyAuth = handle->policyAuth; - session->auth.size = authDigestSz + handle->auth.size; + session->auth.size = (word16)authSz; XMEMMOVE(&session->auth.buffer[authDigestSz], handle->auth.buffer, handle->auth.size); - if (session->auth.size < sizeof(session->auth.buffer)) { - TPM2_ForceZero(&session->auth.buffer[session->auth.size], - (word32)sizeof(session->auth.buffer) - - session->auth.size); + if (authSz < sizeof(session->auth.buffer)) { + TPM2_ForceZero(&session->auth.buffer[authSz], + (word32)sizeof(session->auth.buffer) - authSz); } session->name.size = handle->name.size; XMEMCPY(session->name.name, handle->name.name, session->name.size); @@ -11441,11 +11445,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.overflow || packet.pos > packet.size || - pcrDigestSz > (word32)(packet.size - packet.pos)) { + if (packet.overflow || packet.pos > packet.size) { return BUFFER_E; } if (pcrDigestSz > 0) { + if (packet.pos >= packet.size || + pcrDigestSz > (word32)(packet.size - packet.pos)) { + return BUFFER_E; + } XMEMCPY(buf + packet.pos, pcrDigest, pcrDigestSz); packet.pos += (int)pcrDigestSz; } @@ -12384,8 +12391,7 @@ int wolfTPM2_ST33_GetFwUpgradeCommands(const WOLFTPM2_CAPS* caps, stdImpl); } #endif - return wolfTPM2_ST33_FwUpgradeCommands( - (caps != NULL) ? caps->fwVerMinor : 0, (caps != NULL), + return wolfTPM2_ST33_FwUpgradeCommands(caps->fwVerMinor, 1, manifestMajor, ccStart, ccData); } diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 43b3c82f..25611a26 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -11176,8 +11176,8 @@ static void test_fwtpm_pcr_properties_capability(void) } tag = GetU32BE(gRsp + p); p += 4; wireSz = gRsp[p]; p += 1; - if (wireSz <= 0 || p > rspSize || wireSz > rspSize - p) { - AssertTrue(wireSz > 0 && p <= rspSize && + if (wireSz <= 0 || p >= rspSize || wireSz > rspSize - p) { + AssertTrue(wireSz > 0 && p < rspSize && wireSz <= rspSize - p); FWTPM_Cleanup(&ctx); return; diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 21d15721..2eee812a 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -1604,11 +1604,14 @@ static void test_wolfTPM2_SetAuthHandle_PolicyAuthOffset(void) int rc; WOLFTPM2_DEV dev; WOLFTPM2_HANDLE handle; + TPM2_AUTH_SESSION sessionBefore; int authDigestSz; + int maxAuthSz; int i; XMEMSET(&dev, 0, sizeof(dev)); XMEMSET(&handle, 0, sizeof(handle)); + XMEMSET(&sessionBefore, 0, sizeof(sessionBefore)); (void)wolfTPM2_Init(&dev, TPM2_IoCb, NULL); @@ -1669,6 +1672,51 @@ static void test_wolfTPM2_SetAuthHandle_PolicyAuthOffset(void) AssertIntEQ(XMEMCMP(&dev.session[0].auth.buffer[authDigestSz], handle.auth.buffer, handle.auth.size), 0); + /* Verify the largest combined digest and authorization fits exactly. */ + maxAuthSz = (int)sizeof(dev.session[0].auth.buffer) - authDigestSz; + AssertIntGT(maxAuthSz, 0); + handle.auth.size = (word16)maxAuthSz; + XMEMSET(handle.auth.buffer, 0x5A, handle.auth.size); + handle.name.size = (word16)sizeof(handle.name.name); + XMEMSET(handle.name.name, 0xA5, handle.name.size); + + rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(dev.session[0].auth.size, + (int)sizeof(dev.session[0].auth.buffer)); + AssertIntEQ(XMEMCMP(&dev.session[0].auth.buffer[authDigestSz], + handle.auth.buffer, handle.auth.size), 0); + AssertIntEQ(dev.session[0].name.size, + (int)sizeof(dev.session[0].name.name)); + AssertIntEQ(XMEMCMP(dev.session[0].name.name, handle.name.name, + handle.name.size), 0); + + /* Reject one byte beyond the remaining auth capacity without mutation. */ + handle.auth.size = (word16)(maxAuthSz + 1); + XMEMCPY(&sessionBefore, &dev.session[0], sizeof(sessionBefore)); + rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle); + AssertIntEQ(rc, BUFFER_E); + AssertIntEQ(XMEMCMP(&dev.session[0], &sessionBefore, + sizeof(sessionBefore)), 0); + + /* Reject an oversized name without mutating the session. */ + handle.auth.size = 4; + handle.name.size = (word16)(sizeof(handle.name.name) + 1U); + XMEMCPY(&sessionBefore, &dev.session[0], sizeof(sessionBefore)); + rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle); + AssertIntEQ(rc, BUFFER_E); + AssertIntEQ(XMEMCMP(&dev.session[0], &sessionBefore, + sizeof(sessionBefore)), 0); + + /* Reject a non-hash session algorithm without mutating the session. */ + handle.name.size = 2; + dev.session[0].authHash = TPM_ALG_NULL; + XMEMCPY(&sessionBefore, &dev.session[0], sizeof(sessionBefore)); + rc = wolfTPM2_SetAuthHandle(&dev, 0, &handle); + AssertIntEQ(rc, BUFFER_E); + AssertIntEQ(XMEMCMP(&dev.session[0], &sessionBefore, + sizeof(sessionBefore)), 0); + wolfTPM2_Cleanup(&dev); printf("Test TPM Wrapper: %-40s Passed\n", "SetAuthHandle PolicyAuth:");