Skip to content

Commit 11d0cdc

Browse files
aidangarskeaidangarske
committed
Fix skoll review
1 parent 6177a5d commit 11d0cdc

7 files changed

Lines changed: 49 additions & 37 deletions

File tree

‎examples/seal/seal.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,6 @@ int TPM2_Seal_Example(void* userCtx, int argc, char *argv[])
128128
}
129129

130130
wolfTPM2_GetKeyTemplate_KeySeal(&publicTemplate, TPM_ALG_SHA256);
131-
/* Allow password based unsealing */
132-
publicTemplate.objectAttributes |= TPMA_OBJECT_userWithAuth;
133131

134132
/* set session for authorization key */
135133
auth.size = (int)sizeof(gKeyAuth)-1;

‎src/spdm/spdm_psk.c‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -356,19 +356,21 @@ int wolfSPDM_ConnectPsk(WOLFSPDM_CTX* ctx)
356356
wolfSPDM_TranscriptReset(ctx);
357357

358358
/* Step 1: GET_VERSION */
359-
SPDM_CONNECT_STEP(ctx, "PSK Step 1: GET_VERSION\n",
360-
wolfSPDM_GetVersion(ctx));
359+
wolfSPDM_DebugPrint(ctx, "PSK Step 1: GET_VERSION\n");
360+
rc = wolfSPDM_GetVersion(ctx);
361361

362362
/* Steps 2-3: GET_CAPABILITIES + NEGOTIATE_ALGORITHMS
363363
* Not mandatory for PSK mode per TCG PC Client PSK spec.
364364
* NS350 supports direct GET_VERSION -> PSK_EXCHANGE. */
365365

366366
/* Step 2: PSK_EXCHANGE / PSK_EXCHANGE_RSP */
367-
txSz = sizeof(txBuf);
368-
rxSz = sizeof(rxBuf);
367+
if (rc == WOLFSPDM_SUCCESS) {
368+
txSz = sizeof(txBuf);
369+
rxSz = sizeof(rxBuf);
369370

370-
wolfSPDM_DebugPrint(ctx, "PSK Step 4: PSK_EXCHANGE\n");
371-
rc = wolfSPDM_BuildPskExchange(ctx, txBuf, &txSz);
371+
wolfSPDM_DebugPrint(ctx, "PSK Step 4: PSK_EXCHANGE\n");
372+
rc = wolfSPDM_BuildPskExchange(ctx, txBuf, &txSz);
373+
}
372374
if (rc == WOLFSPDM_SUCCESS) {
373375
rc = wolfSPDM_TranscriptAdd(ctx, txBuf, txSz);
374376
}
@@ -417,6 +419,8 @@ int wolfSPDM_ConnectPsk(WOLFSPDM_CTX* ctx)
417419
}
418420

419421
/* Always zero sensitive stack buffers */
422+
wc_ForceZero(txBuf, sizeof(txBuf));
423+
wc_ForceZero(rxBuf, sizeof(rxBuf));
420424
wc_ForceZero(finBuf, sizeof(finBuf));
421425
wc_ForceZero(encBuf, sizeof(encBuf));
422426
wc_ForceZero(decBuf, sizeof(decBuf));

‎src/spdm/spdm_tcg.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ int wolfSPDM_TCG_VendorCmdSecured(WOLFSPDM_CTX* ctx, const char* vdCode,
8989
byte spdmMsg[WOLFSPDM_VENDOR_BUF_SZ];
9090
int spdmMsgSz;
9191
byte decBuf[WOLFSPDM_VENDOR_BUF_SZ];
92-
word32 decSz;
92+
word32 decSz = 0;
9393
int rc;
9494
byte ver;
9595

‎src/tpm2.c‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3175,8 +3175,7 @@ TPM_RC TPM2_Certify(Certify_In* in, Certify_Out* out)
31753175
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
31763176
in->qualifyingData.size);
31773177

3178-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3179-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3178+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
31803179

31813180
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS, TPM_CC_Certify);
31823181

@@ -3238,8 +3237,7 @@ TPM_RC TPM2_CertifyCreation(CertifyCreation_In* in, CertifyCreation_Out* out)
32383237
TPM2_Packet_AppendBytes(&packet, in->creationHash.buffer,
32393238
in->creationHash.size);
32403239

3241-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3242-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3240+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
32433241

32443242
TPM2_Packet_AppendU16(&packet, in->creationTicket.tag);
32453243
TPM2_Packet_AppendU32(&packet, in->creationTicket.hierarchy);
@@ -3303,8 +3301,7 @@ TPM_RC TPM2_Quote(Quote_In* in, Quote_Out* out)
33033301
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
33043302
in->qualifyingData.size);
33053303

3306-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3307-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3304+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
33083305

33093306
TPM2_Packet_AppendPCR(&packet, &in->PCRselect);
33103307

@@ -3367,8 +3364,7 @@ TPM_RC TPM2_GetSessionAuditDigest(GetSessionAuditDigest_In* in,
33673364
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
33683365
in->qualifyingData.size);
33693366

3370-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3371-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3367+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
33723368

33733369
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS,
33743370
TPM_CC_GetSessionAuditDigest);
@@ -3429,8 +3425,7 @@ TPM_RC TPM2_GetCommandAuditDigest(GetCommandAuditDigest_In* in,
34293425
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
34303426
in->qualifyingData.size);
34313427

3432-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3433-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3428+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
34343429

34353430
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS,
34363431
TPM_CC_GetCommandAuditDigest);
@@ -3490,8 +3485,7 @@ TPM_RC TPM2_GetTime(GetTime_In* in, GetTime_Out* out)
34903485
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
34913486
in->qualifyingData.size);
34923487

3493-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3494-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3488+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
34953489

34963490
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS, TPM_CC_GetTime);
34973491

@@ -3686,14 +3680,7 @@ TPM_RC TPM2_Sign(Sign_In* in, Sign_Out* out)
36863680
TPM2_Packet_AppendU16(&packet, in->digest.size);
36873681
TPM2_Packet_AppendBytes(&packet, in->digest.buffer, in->digest.size);
36883682

3689-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3690-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3691-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3692-
if (in->inScheme.scheme == TPM_ALG_ECDAA) {
3693-
TPM2_Packet_AppendU16(&packet,
3694-
in->inScheme.details.ecdaa.count);
3695-
}
3696-
}
3683+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
36973684

36983685
TPM2_Packet_AppendU16(&packet, in->validation.tag);
36993686
TPM2_Packet_AppendU32(&packet, in->validation.hierarchy);
@@ -5931,8 +5918,7 @@ TPM_RC TPM2_NV_Certify(NV_Certify_In* in, NV_Certify_Out* out)
59315918
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
59325919
in->qualifyingData.size);
59335920

5934-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
5935-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
5921+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
59365922

59375923
TPM2_Packet_AppendU16(&packet, in->size);
59385924
TPM2_Packet_AppendU16(&packet, in->offset);

‎src/tpm2_wrap.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3818,8 +3818,13 @@ int wolfTPM2_DecodeRsaDer(const byte* der, word32 derSz,
38183818
pub->publicArea.objectAttributes = attributes;
38193819
rsa->keyBits = nSz * 8;
38203820
rsa->exponent = e;
3821+
/* if both sign and decrypt are set then must use NULL scheme */
38213822
rsa->scheme.scheme =
3822-
(attributes & TPMA_OBJECT_sign) ? TPM_ALG_RSASSA : TPM_ALG_NULL;
3823+
((attributes & TPMA_OBJECT_sign) &&
3824+
(attributes & TPMA_OBJECT_decrypt)) ?
3825+
TPM_ALG_NULL :
3826+
((attributes & TPMA_OBJECT_sign) ? TPM_ALG_RSASSA :
3827+
TPM_ALG_NULL);
38233828
rsa->scheme.details.anySig.hashAlg = WOLFTPM2_WRAP_DIGEST;
38243829
pub->publicArea.unique.rsa.size = nSz;
38253830
XMEMCPY(pub->publicArea.unique.rsa.buffer, n, nSz);
@@ -3930,8 +3935,13 @@ int wolfTPM2_DecodeEccDer(const byte* der, word32 derSz, TPM2B_PUBLIC* pub,
39303935
pub->publicArea.nameAlg = WOLFTPM2_WRAP_DIGEST;
39313936
pub->publicArea.objectAttributes = attributes;
39323937
ecc->symmetric.algorithm = TPM_ALG_NULL;
3938+
/* if both sign and decrypt are set then must use NULL scheme */
39333939
ecc->scheme.scheme =
3934-
(attributes & TPMA_OBJECT_sign) ? TPM_ALG_ECDSA : TPM_ALG_NULL;
3940+
((attributes & TPMA_OBJECT_sign) &&
3941+
(attributes & TPMA_OBJECT_decrypt)) ?
3942+
TPM_ALG_NULL :
3943+
((attributes & TPMA_OBJECT_sign) ? TPM_ALG_ECDSA :
3944+
TPM_ALG_NULL);
39353945
ecc->scheme.details.ecdsa.hashAlg = WOLFTPM2_WRAP_DIGEST;
39363946
ecc->curveID = curveId;
39373947
ecc->kdf.scheme = TPM_ALG_NULL;

‎tests/unit_tests.c‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -935,7 +935,8 @@ static void test_TPM2_HashNvPublic(void)
935935
/* Known-answer test for TPM2_KDFe (SP800-56A one-step KDF for ECC).
936936
* Reference: independently computed SHA-256(counter || Z || label || U || V)
937937
* with counter starting at 1. */
938-
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
938+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC) && \
939+
!defined(WC_NO_RNG) && defined(WOLFSSL_PUBLIC_MP)
939940
static void test_TPM2_KDFe(void)
940941
{
941942
int rc;
@@ -1110,7 +1111,9 @@ static void test_KeySealTemplate(void)
11101111
printf("Test TPM Wrapper:\tKeySealTemplate:\t\tPassed\n");
11111112
}
11121113

1113-
/* Test boundary validation for seal size and keyed hash key size */
1114+
/* Test boundary validation for seal size and keyed hash key size.
1115+
* Uses zero-initialized dev intentionally — only testing argument validation,
1116+
* not TPM operations. */
11141117
static void test_SealAndKeyedHash_Boundaries(void)
11151118
{
11161119
int rc;
@@ -1829,7 +1832,14 @@ static void test_wolfTPM2_DecodeDer_DefaultAttribs(void)
18291832

18301833
/* userWithAuth should be set */
18311834
AssertTrue(attrs & TPMA_OBJECT_userWithAuth);
1835+
1836+
/* When both sign and decrypt are set, scheme must be NULL */
1837+
AssertIntEQ(pub.publicArea.parameters.eccDetail.scheme.scheme,
1838+
TPM_ALG_NULL);
18321839
#endif
1840+
/* Note: DecodeRsaDer uses the same default attribute and scheme logic
1841+
* as DecodeEccDer — validated by the ECC test above. RSA DER key is
1842+
* too large (1217 bytes) to embed inline for a unit test. */
18331843

18341844
printf("Test TPM Wrapper:\tDecodeDer DefaultAttribs:\tPassed\n");
18351845
}
@@ -1908,8 +1918,11 @@ int unit_tests(int argc, char *argv[])
19081918
test_TPM2_ParamDec_AESCFB_Roundtrip();
19091919
test_TPM2_ParamEncDec_Dispatch_Roundtrip();
19101920
test_TPM2_HashNvPublic();
1911-
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
1921+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC) && \
1922+
!defined(WC_NO_RNG) && defined(WOLFSSL_PUBLIC_MP)
19121923
test_TPM2_KDFe();
1924+
#endif
1925+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
19131926
test_wolfTPM2_ComputeName();
19141927
#endif
19151928
test_TPM2_SchemeSerialize();

‎wolftpm/tpm2_wrap.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4523,7 +4523,8 @@ WOLFTPM_API int wolfTPM2_FirmwareUpgradeCancel(WOLFTPM2_DEV* dev);
45234523

45244524

45254525
/* KDFe - exposed for unit testing */
4526-
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
4526+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC) && \
4527+
!defined(WC_NO_RNG) && defined(WOLFSSL_PUBLIC_MP)
45274528
WOLFTPM_TEST_API int TPM2_KDFe(
45284529
TPM_ALG_ID hashAlg, const TPM2B_DATA *Z, const char *label,
45294530
const TPM2B_DATA *partyUInfo, const TPM2B_DATA *partyVInfo,

0 commit comments

Comments
 (0)