Skip to content

Commit e62dc84

Browse files
aidangarskeaidangarske
committed
Fix skoll review
1 parent 373845a commit e62dc84

File tree

7 files changed

+59
-73
lines changed

7 files changed

+59
-73
lines changed

‎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: 18 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -233,11 +233,9 @@ static int TPM2_CommandProcess(TPM2_CTX* ctx, TPM2_Packet* packet,
233233
}
234234

235235
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && !defined(NO_HMAC)
236-
rc = TPM2_GetName(ctx, handleValue1, info->inHandleCnt, 0, &name1);
237-
if (rc == TPM_RC_SUCCESS)
238-
rc = TPM2_GetName(ctx, handleValue2, info->inHandleCnt, 1, &name2);
239-
if (rc == TPM_RC_SUCCESS)
240-
rc = TPM2_GetName(ctx, handleValue3, info->inHandleCnt, 2, &name3);
236+
rc = TPM2_GetName(ctx, handleValue1, info->inHandleCnt, 0, &name1);
237+
rc |= TPM2_GetName(ctx, handleValue2, info->inHandleCnt, 1, &name2);
238+
rc |= TPM2_GetName(ctx, handleValue3, info->inHandleCnt, 2, &name3);
241239
if (rc != TPM_RC_SUCCESS) {
242240
#ifdef DEBUG_WOLFTPM
243241
printf("Error getting names for cpHash!\n");
@@ -277,17 +275,18 @@ static int TPM2_CommandProcess(TPM2_CTX* ctx, TPM2_Packet* packet,
277275
/* Update the Auth Area total size in the command packet */
278276
i = TPM2_Packet_PlaceU32(packet, authTotalSzPos);
279277

278+
#ifdef DEBUG_WOLFTPM
280279
if ((int)authSz != i) {
281280
/* actual auth size did not match estimated size from
282281
* TPM2_Packet_AppendAuth */
283-
#ifdef DEBUG_WOLFTPM
284282
printf("Error: Calculated auth size %d did not match actual %d!\n",
285283
authSz, i);
286-
#endif
287284
return BUFFER_E;
288285
}
286+
#endif
289287

290288
(void)cmdCode;
289+
(void)i;
291290

292291
return rc;
293292
}
@@ -2144,8 +2143,9 @@ TPM_RC TPM2_Duplicate(Duplicate_In* in, Duplicate_Out* out)
21442143
TPM2_Packet_AppendBytes(&packet, in->encryptionKeyIn.buffer,
21452144
in->encryptionKeyIn.size);
21462145

2147-
TPM2_Packet_AppendSymmetric(&packet,
2148-
(TPMT_SYM_DEF*)&in->symmetricAlg);
2146+
TPM2_Packet_AppendU16(&packet, in->symmetricAlg.algorithm);
2147+
TPM2_Packet_AppendU16(&packet, in->symmetricAlg.keyBits.sym);
2148+
TPM2_Packet_AppendU16(&packet, in->symmetricAlg.mode.sym);
21492149

21502150
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS, TPM_CC_Duplicate);
21512151

@@ -3175,10 +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-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3180-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3181-
}
3178+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
31823179

31833180
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS, TPM_CC_Certify);
31843181

@@ -3240,10 +3237,7 @@ TPM_RC TPM2_CertifyCreation(CertifyCreation_In* in, CertifyCreation_Out* out)
32403237
TPM2_Packet_AppendBytes(&packet, in->creationHash.buffer,
32413238
in->creationHash.size);
32423239

3243-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3244-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3245-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3246-
}
3240+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
32473241

32483242
TPM2_Packet_AppendU16(&packet, in->creationTicket.tag);
32493243
TPM2_Packet_AppendU32(&packet, in->creationTicket.hierarchy);
@@ -3307,10 +3301,7 @@ TPM_RC TPM2_Quote(Quote_In* in, Quote_Out* out)
33073301
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
33083302
in->qualifyingData.size);
33093303

3310-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3311-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3312-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3313-
}
3304+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
33143305

33153306
TPM2_Packet_AppendPCR(&packet, &in->PCRselect);
33163307

@@ -3373,10 +3364,7 @@ TPM_RC TPM2_GetSessionAuditDigest(GetSessionAuditDigest_In* in,
33733364
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
33743365
in->qualifyingData.size);
33753366

3376-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3377-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3378-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3379-
}
3367+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
33803368

33813369
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS,
33823370
TPM_CC_GetSessionAuditDigest);
@@ -3437,10 +3425,7 @@ TPM_RC TPM2_GetCommandAuditDigest(GetCommandAuditDigest_In* in,
34373425
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
34383426
in->qualifyingData.size);
34393427

3440-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3441-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3442-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3443-
}
3428+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
34443429

34453430
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS,
34463431
TPM_CC_GetCommandAuditDigest);
@@ -3500,10 +3485,7 @@ TPM_RC TPM2_GetTime(GetTime_In* in, GetTime_Out* out)
35003485
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
35013486
in->qualifyingData.size);
35023487

3503-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3504-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3505-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3506-
}
3488+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
35073489

35083490
TPM2_Packet_Finalize(&packet, TPM_ST_SESSIONS, TPM_CC_GetTime);
35093491

@@ -3628,7 +3610,6 @@ TPM_RC TPM2_VerifySignature(VerifySignature_In* in,
36283610
TPM_RC rc;
36293611
TPM2_CTX* ctx = TPM2_GetActiveCtx();
36303612
TPM_ST st;
3631-
UINT16 wireSize = 0;
36323613

36333614
if (ctx == NULL || in == NULL || out == NULL)
36343615
return BAD_FUNC_ARG;
@@ -3664,20 +3645,10 @@ TPM_RC TPM2_VerifySignature(VerifySignature_In* in,
36643645

36653646
TPM2_Packet_ParseU16(&packet, &out->validation.tag);
36663647
TPM2_Packet_ParseU32(&packet, &out->validation.hierarchy);
3667-
3668-
TPM2_Packet_ParseU16(&packet, &wireSize);
3669-
out->validation.digest.size = wireSize;
3670-
if (out->validation.digest.size >
3671-
(UINT16)sizeof(out->validation.digest.buffer)) {
3672-
out->validation.digest.size =
3673-
(UINT16)sizeof(out->validation.digest.buffer);
3674-
}
3648+
TPM2_Packet_ParseU16(&packet, &out->validation.digest.size);
36753649
TPM2_Packet_ParseBytes(&packet,
36763650
out->validation.digest.buffer,
36773651
out->validation.digest.size);
3678-
if (wireSize > out->validation.digest.size)
3679-
TPM2_Packet_ParseBytes(&packet, NULL,
3680-
wireSize - out->validation.digest.size);
36813652
}
36823653

36833654
TPM2_ReleaseLock(ctx);
@@ -3709,14 +3680,7 @@ TPM_RC TPM2_Sign(Sign_In* in, Sign_Out* out)
37093680
TPM2_Packet_AppendU16(&packet, in->digest.size);
37103681
TPM2_Packet_AppendBytes(&packet, in->digest.buffer, in->digest.size);
37113682

3712-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
3713-
if (in->inScheme.scheme != TPM_ALG_NULL) {
3714-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
3715-
if (in->inScheme.scheme == TPM_ALG_ECDAA) {
3716-
TPM2_Packet_AppendU16(&packet,
3717-
in->inScheme.details.ecdaa.count);
3718-
}
3719-
}
3683+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
37203684

37213685
TPM2_Packet_AppendU16(&packet, in->validation.tag);
37223686
TPM2_Packet_AppendU32(&packet, in->validation.hierarchy);
@@ -5954,10 +5918,7 @@ TPM_RC TPM2_NV_Certify(NV_Certify_In* in, NV_Certify_Out* out)
59545918
TPM2_Packet_AppendBytes(&packet, in->qualifyingData.buffer,
59555919
in->qualifyingData.size);
59565920

5957-
TPM2_Packet_AppendU16(&packet, in->inScheme.scheme);
5958-
if (in->inScheme.scheme != TPM_ALG_NULL) {
5959-
TPM2_Packet_AppendU16(&packet, in->inScheme.details.any.hashAlg);
5960-
}
5921+
TPM2_Packet_AppendEccScheme(&packet, &in->inScheme);
59615922

59625923
TPM2_Packet_AppendU16(&packet, in->size);
59635924
TPM2_Packet_AppendU16(&packet, in->offset);

‎src/tpm2_wrap.c‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3884,8 +3884,13 @@ int wolfTPM2_DecodeRsaDer(const byte* der, word32 derSz,
38843884
pub->publicArea.objectAttributes = attributes;
38853885
rsa->keyBits = nSz * 8;
38863886
rsa->exponent = e;
3887+
/* if both sign and decrypt are set then must use NULL scheme */
38873888
rsa->scheme.scheme =
3888-
(attributes & TPMA_OBJECT_sign) ? TPM_ALG_RSASSA : TPM_ALG_NULL;
3889+
((attributes & TPMA_OBJECT_sign) &&
3890+
(attributes & TPMA_OBJECT_decrypt)) ?
3891+
TPM_ALG_NULL :
3892+
((attributes & TPMA_OBJECT_sign) ? TPM_ALG_RSASSA :
3893+
TPM_ALG_NULL);
38893894
rsa->scheme.details.anySig.hashAlg = WOLFTPM2_WRAP_DIGEST;
38903895
pub->publicArea.unique.rsa.size = nSz;
38913896
XMEMCPY(pub->publicArea.unique.rsa.buffer, n, nSz);
@@ -3996,8 +4001,13 @@ int wolfTPM2_DecodeEccDer(const byte* der, word32 derSz, TPM2B_PUBLIC* pub,
39964001
pub->publicArea.nameAlg = WOLFTPM2_WRAP_DIGEST;
39974002
pub->publicArea.objectAttributes = attributes;
39984003
ecc->symmetric.algorithm = TPM_ALG_NULL;
4004+
/* if both sign and decrypt are set then must use NULL scheme */
39994005
ecc->scheme.scheme =
4000-
(attributes & TPMA_OBJECT_sign) ? TPM_ALG_ECDSA : TPM_ALG_NULL;
4006+
((attributes & TPMA_OBJECT_sign) &&
4007+
(attributes & TPMA_OBJECT_decrypt)) ?
4008+
TPM_ALG_NULL :
4009+
((attributes & TPMA_OBJECT_sign) ? TPM_ALG_ECDSA :
4010+
TPM_ALG_NULL);
40014011
ecc->scheme.details.ecdsa.hashAlg = WOLFTPM2_WRAP_DIGEST;
40024012
ecc->curveID = curveId;
40034013
ecc->kdf.scheme = TPM_ALG_NULL;
@@ -9473,7 +9483,6 @@ int wolfTPM2_SetIdentityAuth(WOLFTPM2_DEV* dev, WOLFTPM2_HANDLE* handle,
94739483
TPM2_PrintBin(handle->auth.buffer, handle->auth.size);
94749484
#endif
94759485
}
9476-
wc_ForceZero(digest, sizeof(digest));
94779486

94789487
(void)dev;
94799488

‎tests/unit_tests.c‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,7 +1244,8 @@ static void test_TPM2_HashNvPublic(void)
12441244
/* Known-answer test for TPM2_KDFe (SP800-56A one-step KDF for ECC).
12451245
* Reference: independently computed SHA-256(counter || Z || label || U || V)
12461246
* with counter starting at 1. */
1247-
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
1247+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC) && \
1248+
!defined(WC_NO_RNG) && defined(WOLFSSL_PUBLIC_MP)
12481249
static void test_TPM2_KDFe(void)
12491250
{
12501251
int rc;
@@ -1419,7 +1420,9 @@ static void test_KeySealTemplate(void)
14191420
printf("Test TPM Wrapper:\tKeySealTemplate:\t\tPassed\n");
14201421
}
14211422

1422-
/* Test boundary validation for seal size and keyed hash key size */
1423+
/* Test boundary validation for seal size and keyed hash key size.
1424+
* Uses zero-initialized dev intentionally — only testing argument validation,
1425+
* not TPM operations. */
14231426
static void test_SealAndKeyedHash_Boundaries(void)
14241427
{
14251428
int rc;
@@ -2138,7 +2141,14 @@ static void test_wolfTPM2_DecodeDer_DefaultAttribs(void)
21382141

21392142
/* userWithAuth should be set */
21402143
AssertTrue(attrs & TPMA_OBJECT_userWithAuth);
2144+
2145+
/* When both sign and decrypt are set, scheme must be NULL */
2146+
AssertIntEQ(pub.publicArea.parameters.eccDetail.scheme.scheme,
2147+
TPM_ALG_NULL);
21412148
#endif
2149+
/* Note: DecodeRsaDer uses the same default attribute and scheme logic
2150+
* as DecodeEccDer — validated by the ECC test above. RSA DER key is
2151+
* too large (1217 bytes) to embed inline for a unit test. */
21422152

21432153
printf("Test TPM Wrapper:\tDecodeDer DefaultAttribs:\tPassed\n");
21442154
}
@@ -2222,8 +2232,11 @@ int unit_tests(int argc, char *argv[])
22222232
test_TPM2_ParamDec_AESCFB_Roundtrip();
22232233
test_TPM2_ParamEncDec_Dispatch_Roundtrip();
22242234
test_TPM2_HashNvPublic();
2225-
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
2235+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC) && \
2236+
!defined(WC_NO_RNG) && defined(WOLFSSL_PUBLIC_MP)
22262237
test_TPM2_KDFe();
2238+
#endif
2239+
#if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC)
22272240
test_wolfTPM2_ComputeName();
22282241
#endif
22292242
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)