Skip to content

Commit eae465e

Browse files
dgarskedgarske
committed
Fix peer review findings
1 parent 90096b9 commit eae465e

File tree

13 files changed

+112
-46
lines changed

13 files changed

+112
-46
lines changed

‎.github/workflows/fwtpm-test.yml‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,11 @@ jobs:
191191
CONFIGURE_ARGS="${{ matrix.wolfssl_config }}"
192192
EXTRA_CFLAGS="${{ matrix.extra_cflags }}"
193193
EXTRA_LDFLAGS="${{ matrix.extra_ldflags }}"
194+
WOLFSSL_CFLAGS="-DWC_RSA_NO_PADDING"
194195
if [ -n "$EXTRA_CFLAGS" ]; then
195-
CONFIGURE_ARGS="$CONFIGURE_ARGS CFLAGS=\"$EXTRA_CFLAGS\""
196+
WOLFSSL_CFLAGS="$WOLFSSL_CFLAGS $EXTRA_CFLAGS"
196197
fi
198+
CONFIGURE_ARGS="$CONFIGURE_ARGS CFLAGS=\"$WOLFSSL_CFLAGS\""
197199
if [ -n "$EXTRA_LDFLAGS" ]; then
198200
CONFIGURE_ARGS="$CONFIGURE_ARGS LDFLAGS=\"$EXTRA_LDFLAGS\""
199201
fi
@@ -355,13 +357,15 @@ jobs:
355357
./autogen.sh
356358
./configure --enable-wolftpm --enable-pkcallbacks --enable-keygen
357359
make
358-
sudo make install
359-
sudo ldconfig
360+
make install
361+
ldconfig
360362
361363
- name: Copy wolfSSL to /tmp/wolfssl-fwtpm
362364
run: cp -a wolfssl /tmp/wolfssl-fwtpm
363365

364366
- name: Run fwTPM emulator test (non-TZ)
367+
env:
368+
WOLFSSL_DIR: /tmp/wolfssl-fwtpm
365369
run: scripts/fwtpm_emu_test.sh
366370

367371
- name: Upload failure logs

‎scripts/fwtpm_build_test.sh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ check_wolfssl_options() {
5858
[ -f "$opts_file" ] || return 1
5959
grep -q "HAVE_PK_CALLBACKS" "$opts_file" && \
6060
grep -q "WOLFSSL_KEY_GEN" "$opts_file" && \
61-
grep -q "WOLFSSL_PUBLIC_MP" "$opts_file"
61+
grep -q "WOLFSSL_PUBLIC_MP" "$opts_file" && \
62+
grep -q "WC_RSA_NO_PADDING" "$opts_file"
6263
}
6364

6465
ensure_wolfssl() {
@@ -103,6 +104,7 @@ ensure_wolfssl() {
103104
./autogen.sh > /dev/null 2>&1 && \
104105
./configure \
105106
--enable-wolftpm --enable-pkcallbacks --enable-keygen \
107+
CFLAGS="-DWC_RSA_NO_PADDING" \
106108
> /tmp/wolfssl-fwtpm-configure.log 2>&1 && \
107109
make -j"$(nproc)" > /tmp/wolfssl-fwtpm-build.log 2>&1 && \
108110
sudo make install > /tmp/wolfssl-fwtpm-install.log 2>&1 && \

‎scripts/fwtpm_emu_test.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ echo " TZEN: $TZEN"
6262
if [ $DO_BUILD -eq 1 ]; then
6363
echo "Building fwTPM STM32 (TZEN=$TZEN, SELFTEST=1)..."
6464
make -C "$PORT_DIR" clean > /dev/null 2>&1
65-
if ! make -C "$PORT_DIR" TZEN=$TZEN SELFTEST=1 > /tmp/fwtpm_emu_build.log 2>&1; then
65+
if ! make -C "$PORT_DIR" ${WOLFSSL_DIR:+WOLFSSL_DIR="$WOLFSSL_DIR"} TZEN=$TZEN SELFTEST=1 > /tmp/fwtpm_emu_build.log 2>&1; then
6666
echo "FAIL: Build failed"
6767
tail -20 /tmp/fwtpm_emu_build.log
6868
exit 1

‎src/fwtpm/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ SPI/I2C for bare-metal integration. Implements 103 of 113 TPM 2.0 v1.38 commands
1111

1212
## Building
1313

14-
wolfSSL must be built with `--enable-keygen`:
14+
wolfSSL must be built with `--enable-keygen` and `WC_RSA_NO_PADDING`:
1515

1616
```bash
1717
cd wolfssl
18-
./configure --enable-wolftpm --enable-pkcallbacks --enable-keygen
18+
./configure --enable-wolftpm --enable-pkcallbacks --enable-keygen CFLAGS="-DWC_RSA_NO_PADDING"
1919
make && make install
2020
```
2121

‎src/fwtpm/fwtpm_command.c‎

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5457,9 +5457,13 @@ static TPM_RC FwCmd_RSA_Encrypt(FWTPM_CTX* ctx, TPM2_Packet* cmd,
54575457
}
54585458
else if (encScheme == TPM_ALG_NULL) {
54595459
/* Raw RSA (no padding) */
5460+
#ifdef WC_RSA_NO_PADDING
54605461
outSz = wc_RsaPublicEncrypt_ex(message.buffer, message.size,
54615462
outBuf, (word32)FWTPM_MAX_PUB_BUF, rsaKey, &ctx->rng,
54625463
WC_RSA_NO_PAD, WC_HASH_TYPE_NONE, 0, NULL, 0);
5464+
#else
5465+
rc = TPM_RC_SCHEME;
5466+
#endif
54635467
}
54645468
else {
54655469
/* RSAES PKCS1 v1.5 */
@@ -5602,9 +5606,13 @@ static TPM_RC FwCmd_RSA_Decrypt(FWTPM_CTX* ctx, TPM2_Packet* cmd,
56025606
}
56035607
else if (decScheme == TPM_ALG_NULL) {
56045608
/* Raw RSA (no padding) */
5609+
#ifdef WC_RSA_NO_PADDING
56055610
outSz = wc_RsaPrivateDecrypt_ex(cipherText.buffer, cipherText.size,
56065611
outBuf, (word32)FWTPM_MAX_PUB_BUF, rsaKey,
56075612
WC_RSA_NO_PAD, WC_HASH_TYPE_NONE, 0, NULL, 0);
5613+
#else
5614+
rc = TPM_RC_SCHEME;
5615+
#endif
56085616
}
56095617
else {
56105618
/* RSAES PKCS1 v1.5 */
@@ -10964,7 +10972,10 @@ static TPM_RC FwCmd_MakeCredential(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1096410972
XMEMCPY(oaepLabel, "IDENTITY", 8);
1096510973
oaepLabelSz = 8;
1096610974
oaepLabel[oaepLabelSz++] = 0x00;
10967-
if (objectName.size + oaepLabelSz <= (int)sizeof(oaepLabel)) {
10975+
if (objectName.size + oaepLabelSz > (int)sizeof(oaepLabel)) {
10976+
rc = TPM_RC_SIZE;
10977+
}
10978+
else {
1096810979
XMEMCPY(oaepLabel + oaepLabelSz, objectName.name,
1096910980
objectName.size);
1097010981
oaepLabelSz += objectName.size;
@@ -11016,14 +11027,20 @@ static TPM_RC FwCmd_MakeCredential(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1101611027
TPM2_Packet_AppendBytes(rsp, encCred, (int)encCredSz);
1101711028
/* patch blob size */
1101811029
blobSz = rsp->pos - blobStart;
11019-
savedPos = rsp->pos;
11020-
rsp->pos = blobSzPos;
11021-
TPM2_Packet_AppendU16(rsp, (UINT16)blobSz);
11022-
rsp->pos = savedPos;
11023-
/* secret = TPM2B_ENCRYPTED_SECRET */
11024-
TPM2_Packet_AppendU16(rsp, (UINT16)encSeedSz);
11025-
TPM2_Packet_AppendBytes(rsp, encSeed, encSeedSz);
11026-
FwRspParamsEnd(rsp, cmdTag, paramSzPos, paramStart);
11030+
if (blobSz < 0 || blobSz > 0xFFFF ||
11031+
encSeedSz < 0 || encSeedSz > 0xFFFF) {
11032+
rc = TPM_RC_SIZE;
11033+
}
11034+
if (rc == 0) {
11035+
savedPos = rsp->pos;
11036+
rsp->pos = blobSzPos;
11037+
TPM2_Packet_AppendU16(rsp, (UINT16)blobSz);
11038+
rsp->pos = savedPos;
11039+
/* secret = TPM2B_ENCRYPTED_SECRET */
11040+
TPM2_Packet_AppendU16(rsp, (UINT16)encSeedSz);
11041+
TPM2_Packet_AppendBytes(rsp, encSeed, encSeedSz);
11042+
FwRspParamsEnd(rsp, cmdTag, paramSzPos, paramStart);
11043+
}
1102711044
}
1102811045

1102911046
TPM2_ForceZero(seed, sizeof(seed));
@@ -11151,7 +11168,10 @@ static TPM_RC FwCmd_ActivateCredential(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1115111168
XMEMCPY(oaepLabel, "IDENTITY", 8);
1115211169
oaepLabelSz = 8;
1115311170
oaepLabel[oaepLabelSz++] = 0x00;
11154-
if (objName->size + oaepLabelSz <= (int)sizeof(oaepLabel)) {
11171+
if (objName->size + oaepLabelSz > (int)sizeof(oaepLabel)) {
11172+
rc = TPM_RC_SIZE;
11173+
}
11174+
else {
1115511175
XMEMCPY(oaepLabel + oaepLabelSz, objName->name, objName->size);
1115611176
oaepLabelSz += objName->size;
1115711177
}
@@ -12293,6 +12313,10 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
1229312313
int rspParamEnd;
1229412314
#endif
1229512315
int j;
12316+
int rngRc;
12317+
byte rpHash[TPM_MAX_DIGEST_SIZE];
12318+
int rpHashSz = 0;
12319+
TPMI_ALG_HASH rpHashAlg = TPM_ALG_SHA256;
1229612320

1229712321
/* Read parameterSize from response buffer */
1229812322
if (rspHandleEnd + 4 <= rspPkt.pos) {
@@ -12314,9 +12338,14 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
1231412338
FWTPM_Session* sess = cmdAuths[j].sess;
1231512339
int digestSz = TPM2_GetHashDigestSize(sess->authHash);
1231612340
if (digestSz > 0) {
12317-
sess->nonceTPM.size = digestSz;
12318-
wc_RNG_GenerateBlock(&ctx->rng, sess->nonceTPM.buffer,
12319-
digestSz);
12341+
rngRc = wc_RNG_GenerateBlock(&ctx->rng,
12342+
sess->nonceTPM.buffer, digestSz);
12343+
if (rngRc == 0) {
12344+
sess->nonceTPM.size = digestSz;
12345+
}
12346+
else {
12347+
sess->nonceTPM.size = 0;
12348+
}
1232012349
}
1232112350
}
1232212351
}
@@ -12340,8 +12369,6 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
1234012369
#endif /* !FWTPM_NO_PARAM_ENC */
1234112370

1234212371
/* Compute rpHash on (possibly encrypted) response parameters */
12343-
byte rpHash[TPM_MAX_DIGEST_SIZE];
12344-
int rpHashSz = 0;
1234512372
{
1234612373
const byte* rpBytes = NULL;
1234712374
int rpBytesSz = 0;
@@ -12350,7 +12377,6 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
1235012377
rpBytesSz = (int)rspParamSzVal;
1235112378
}
1235212379
/* Use first session's hashAlg for rpHash (or SHA-256 default) */
12353-
TPMI_ALG_HASH rpHashAlg = TPM_ALG_SHA256;
1235412380
for (j = 0; j < cmdAuthCnt; j++) {
1235512381
if (cmdAuths[j].sess != NULL) {
1235612382
rpHashAlg = cmdAuths[j].sess->authHash;

‎src/fwtpm/fwtpm_crypto.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,15 @@ int FwComputeUniqueHash(TPMI_ALG_HASH nameAlg, const byte* keyData,
8484
FWTPM_DECLARE_VAR(hCtx, wc_HashAlg);
8585
FWTPM_ALLOC_VAR(hCtx, wc_HashAlg);
8686
if (rc == 0 && wc_HashInit(hCtx, wcHash) == 0) {
87-
wc_HashUpdate(hCtx, wcHash, keyData, (word32)keyDataSz);
88-
wc_HashFinal(hCtx, wcHash, outBuf);
87+
rc = wc_HashUpdate(hCtx, wcHash, keyData, (word32)keyDataSz);
88+
if (rc == 0) {
89+
rc = wc_HashFinal(hCtx, wcHash, outBuf);
90+
}
8991
wc_HashFree(hCtx, wcHash);
90-
FWTPM_FREE_VAR(hCtx);
91-
return hSz;
92+
if (rc == 0) {
93+
FWTPM_FREE_VAR(hCtx);
94+
return hSz;
95+
}
9296
}
9397
FWTPM_FREE_VAR(hCtx);
9498
}
@@ -263,10 +267,14 @@ int FwAppendCreationHashAndTicket(FWTPM_CTX* ctx, TPM2_Packet* rsp,
263267
byte ticketData[TPM_MAX_DIGEST_SIZE + sizeof(TPM2B_NAME)];
264268
int chSz = TPM2_GetHashDigestSize(nameAlg);
265269
int ticketDataSz = 0;
270+
int hashRc = 0;
266271

267272
if (chSz > 0) {
268-
wc_Hash(FwGetWcHashType(nameAlg),
273+
hashRc = wc_Hash(FwGetWcHashType(nameAlg),
269274
rsp->buf + cdStart, cdSize, creationHash, chSz);
275+
if (hashRc != 0) {
276+
chSz = 0;
277+
}
270278
}
271279
TPM2_Packet_AppendU16(rsp, (UINT16)chSz);
272280
if (chSz > 0) {

‎src/fwtpm/fwtpm_io.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,7 @@ static int HandleMssimSignal(FWTPM_CTX* ctx, int clientFd, UINT32 tssCmd)
261261
printf("fwTPM: Cmd-port signal %u (ack)\n", tssCmd);
262262
#endif
263263
netVal = FwTpmSwapU32(0);
264-
SocketSend(clientFd, &netVal, 4);
265-
return TPM_RC_SUCCESS;
264+
return SocketSend(clientFd, &netVal, 4);
266265
}
267266

268267
/* --- Process and send TPM command response --- */

‎src/fwtpm/fwtpm_nv.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,10 @@ static int FwNvFileWrite(void* ctx, word32 offset, const byte* buf,
104104
byte zero = 0;
105105
long i;
106106
for (i = fileSize; i < (long)offset; i++) {
107-
fwrite(&zero, 1, 1, f);
107+
if (fwrite(&zero, 1, 1, f) != 1) {
108+
fclose(f);
109+
return TPM_RC_FAILURE;
110+
}
108111
}
109112
}
110113

@@ -860,6 +863,9 @@ static int FwNvProcessEntry(FWTPM_CTX* ctx, UINT16 tag,
860863
XMEMCPY(&ctx->nvIndices[slot], &nv,
861864
sizeof(FWTPM_NvIndex));
862865
}
866+
else {
867+
WOLFSSL_MSG("fwTPM NV: no free NV slot, entry dropped");
868+
}
863869
}
864870
break;
865871
}

‎src/fwtpm/fwtpm_tis.c‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,12 @@ static void TisHandleRegAccess(FWTPM_CTX* ctx, FWTPM_TIS_REGS* regs)
159159
case FWTPM_TIS_XDATA_FIFO: {
160160
/* Write command data to FIFO */
161161
UINT32 i;
162-
UINT32 space = FWTPM_TIS_FIFO_SIZE - regs->fifo_write_pos;
162+
UINT32 space;
163+
/* Clamp len to reg_data buffer size */
164+
if (len > sizeof(regs->reg_data)) {
165+
len = (UINT32)sizeof(regs->reg_data);
166+
}
167+
space = FWTPM_TIS_FIFO_SIZE - regs->fifo_write_pos;
163168
if (len > space) {
164169
len = space;
165170
}
@@ -178,6 +183,16 @@ static void TisHandleRegAccess(FWTPM_CTX* ctx, FWTPM_TIS_REGS* regs)
178183
* is complete based on the size field in bytes [2..5] */
179184
if (regs->cmd_len >= TPM2_HEADER_SIZE) {
180185
UINT32 cmdTotalSz = FwLoadU32BE(regs->cmd_buf + 2);
186+
if (cmdTotalSz < TPM2_HEADER_SIZE ||
187+
cmdTotalSz > FWTPM_TIS_FIFO_SIZE) {
188+
/* Invalid command size, reset FIFO */
189+
regs->cmd_len = 0;
190+
regs->fifo_write_pos = 0;
191+
regs->sts = TisBuildSts(
192+
FWTPM_STS_VALID | FWTPM_STS_COMMAND_READY,
193+
FWTPM_TIS_BURST_COUNT);
194+
break;
195+
}
181196
if (regs->cmd_len >= cmdTotalSz) {
182197
/* Full command received, clear EXPECT */
183198
regs->sts = TisBuildSts(

‎src/fwtpm/fwtpm_tis_shm.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,9 @@ static int TisShmWaitRequest(void* ctx)
148148
static int TisShmSignalResponse(void* ctx)
149149
{
150150
FWTPM_TIS_SHM_CTX* shm = (FWTPM_TIS_SHM_CTX*)ctx;
151-
sem_post(shm->semRsp);
151+
if (sem_post(shm->semRsp) != 0) {
152+
return TPM_RC_FAILURE;
153+
}
152154
return 0;
153155
}
154156

0 commit comments

Comments
 (0)