Skip to content

Commit 262be34

Browse files
dgarskedgarske
committed
Cleanups from peer-review
1 parent 6deabcd commit 262be34

8 files changed

Lines changed: 83 additions & 94 deletions

File tree

‎src/fwtpm/fwtpm.c‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -82,39 +82,29 @@ int FWTPM_Init(FWTPM_CTX* ctx)
8282

8383
/* Initialize wolfCrypt RNG */
8484
rc = wolfCrypt_Init();
85-
if (rc != 0) {
86-
return rc;
87-
}
88-
rc = wc_InitRng(&ctx->rng);
89-
if (rc != 0) {
90-
wolfCrypt_Cleanup();
91-
return rc;
85+
if (rc == 0) {
86+
rc = wc_InitRng(&ctx->rng);
9287
}
9388

9489
/* Generate per-boot context protection key (volatile only) for
9590
* ContextSave/ContextLoad HMAC + AES-CFB session blob protection. */
96-
rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->ctxProtectKey,
97-
sizeof(ctx->ctxProtectKey));
9891
if (rc == 0) {
99-
ctx->ctxProtectKeyValid = 1;
100-
}
101-
else {
102-
wc_FreeRng(&ctx->rng);
103-
wolfCrypt_Cleanup();
104-
return rc;
92+
rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->ctxProtectKey,
93+
sizeof(ctx->ctxProtectKey));
94+
if (rc == 0) {
95+
ctx->ctxProtectKeyValid = 1;
96+
}
10597
}
10698

10799
/* Initialize NV storage - loads existing state or creates fresh seeds */
108100
#ifndef FWTPM_NO_NV
109-
rc = FWTPM_NV_Init(ctx);
110-
if (rc != 0) {
111-
wc_FreeRng(&ctx->rng);
112-
wolfCrypt_Cleanup();
113-
return rc;
101+
if (rc == 0) {
102+
rc = FWTPM_NV_Init(ctx);
114103
}
115104
#else
116105
/* No NV: generate ephemeral seeds (lost on reset) */
117-
rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->ownerSeed, FWTPM_SEED_SIZE);
106+
if (rc == 0)
107+
rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->ownerSeed, FWTPM_SEED_SIZE);
118108
if (rc == 0)
119109
rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->endorsementSeed,
120110
FWTPM_SEED_SIZE);
@@ -124,13 +114,15 @@ int FWTPM_Init(FWTPM_CTX* ctx)
124114
if (rc == 0)
125115
rc = wc_RNG_GenerateBlock(&ctx->rng, ctx->nullSeed,
126116
FWTPM_SEED_SIZE);
117+
if (rc == 0) {
118+
ctx->pcrAllocatedBanks = FWTPM_PCR_ALLOC_DEFAULT;
119+
}
120+
#endif
121+
127122
if (rc != 0) {
128123
wc_FreeRng(&ctx->rng);
129124
wolfCrypt_Cleanup();
130-
return TPM_RC_FAILURE;
131125
}
132-
ctx->pcrAllocatedBanks = FWTPM_PCR_ALLOC_DEFAULT;
133-
#endif
134126

135127
return rc;
136128
}

‎src/fwtpm/fwtpm_crypto.c‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,7 @@ TPM_RC FwDeriveEccPrimaryKey(TPMI_ALG_HASH nameAlg,
557557
int valid = 0;
558558
int i;
559559
int allZero;
560+
volatile byte orAccum;
560561

561562
FWTPM_ALLOC_VAR(eccKey, ecc_key);
562563

@@ -577,13 +578,11 @@ TPM_RC FwDeriveEccPrimaryKey(TPMI_ALG_HASH nameAlg,
577578
break;
578579
}
579580
/* Constant-time check d != 0 (all zeros) */
580-
{
581-
volatile byte orAccum = 0;
582-
for (i = 0; i < keySz; i++) {
583-
orAccum |= dBuf[i];
584-
}
585-
allZero = (orAccum == 0);
581+
orAccum = 0;
582+
for (i = 0; i < keySz; i++) {
583+
orAccum |= dBuf[i];
586584
}
585+
allZero = (orAccum == 0);
587586
if (!allZero) {
588587
valid = 1; /* Accept — range check done by import */
589588
}
@@ -1248,7 +1247,7 @@ int FwWrapContextBlob(FWTPM_CTX* ctx,
12481247
rc = wc_AesInit(aes, NULL, INVALID_DEVID);
12491248
if (rc == 0) {
12501249
aesInit = 1;
1251-
rc = wc_AesSetKey(aes, ctx->ctxProtectKey, 32, iv, AES_ENCRYPTION);
1250+
rc = wc_AesSetKey(aes, ctx->ctxProtectKey, AES_256_KEY_SIZE, iv, AES_ENCRYPTION);
12521251
}
12531252
}
12541253
if (rc == 0) {
@@ -1347,7 +1346,7 @@ int FwUnwrapContextBlob(FWTPM_CTX* ctx,
13471346
rc = wc_AesInit(aes, NULL, INVALID_DEVID);
13481347
if (rc == 0) {
13491348
aesInit = 1;
1350-
rc = wc_AesSetKey(aes, ctx->ctxProtectKey, 32, in, AES_ENCRYPTION);
1349+
rc = wc_AesSetKey(aes, ctx->ctxProtectKey, AES_256_KEY_SIZE, in, AES_ENCRYPTION);
13511350
}
13521351
}
13531352
if (rc == 0) {

‎src/fwtpm/fwtpm_io.c‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,11 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
522522
int maxFd;
523523
int cmdFd = -1; /* active command client fd */
524524
int platFd = -1; /* active platform client fd */
525+
struct timeval tv;
526+
int selRc;
527+
#ifndef _WIN32
528+
struct sigaction sa;
529+
#endif
525530
#endif
526531

527532
if (ctx == NULL) {
@@ -536,13 +541,10 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
536541
#ifndef _WIN32
537542
/* Ignore SIGPIPE so write to closed socket returns error instead
538543
* of killing the process */
539-
{
540-
struct sigaction sa;
541-
sa.sa_handler = SIG_IGN;
542-
sigemptyset(&sa.sa_mask);
543-
sa.sa_flags = 0;
544-
sigaction(SIGPIPE, &sa, NULL);
545-
}
544+
sa.sa_handler = SIG_IGN;
545+
sigemptyset(&sa.sa_mask);
546+
sa.sa_flags = 0;
547+
sigaction(SIGPIPE, &sa, NULL);
546548
#endif
547549

548550
/* Multiplexed select loop: handles listen sockets AND active client fds
@@ -568,25 +570,21 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
568570
if (platFd > maxFd) maxFd = platFd;
569571
}
570572

571-
{
572-
struct timeval tv;
573-
int selRc;
574-
tv.tv_sec = 30;
575-
tv.tv_usec = 0;
576-
selRc = select(maxFd + 1, &readFds, NULL, NULL, &tv);
577-
if (selRc < 0) {
578-
if (errno == EINTR) {
579-
continue;
580-
}
581-
#ifdef DEBUG_WOLFTPM
582-
printf("fwTPM: select error %d (%s)\n", errno, strerror(errno));
583-
#endif
584-
rc = TPM_RC_FAILURE;
585-
break;
586-
}
587-
if (selRc == 0) {
588-
continue; /* timeout — recheck ctx->running */
573+
tv.tv_sec = 30;
574+
tv.tv_usec = 0;
575+
selRc = select(maxFd + 1, &readFds, NULL, NULL, &tv);
576+
if (selRc < 0) {
577+
if (errno == EINTR) {
578+
continue;
589579
}
580+
#ifdef DEBUG_WOLFTPM
581+
printf("fwTPM: select error %d (%s)\n", errno, strerror(errno));
582+
#endif
583+
rc = TPM_RC_FAILURE;
584+
break;
585+
}
586+
if (selRc == 0) {
587+
continue; /* timeout — recheck ctx->running */
590588
}
591589

592590
/* Accept new platform connection */

‎src/fwtpm/fwtpm_main.c‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ int main(int argc, char* argv[])
7575
static FWTPM_CTX ctx;
7676
int i;
7777
int clearNv = 0;
78+
struct sigaction sa;
7879

7980
/* Zero context before init (required so HAL save/restore works) */
8081
XMEMSET(&ctx, 0, sizeof(ctx));
@@ -159,14 +160,11 @@ int main(int argc, char* argv[])
159160

160161
/* Install signal handler for graceful shutdown with NV save */
161162
g_ctx = &ctx;
162-
{
163-
struct sigaction sa;
164-
sa.sa_handler = sigterm_handler;
165-
sigemptyset(&sa.sa_mask);
166-
sa.sa_flags = 0;
167-
sigaction(SIGTERM, &sa, NULL);
168-
sigaction(SIGINT, &sa, NULL);
169-
}
163+
sa.sa_handler = sigterm_handler;
164+
sigemptyset(&sa.sa_mask);
165+
sa.sa_flags = 0;
166+
sigaction(SIGTERM, &sa, NULL);
167+
sigaction(SIGINT, &sa, NULL);
170168

171169
/* Initialize socket transport */
172170
rc = FWTPM_IO_Init(&ctx);

‎src/fwtpm/fwtpm_tis.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,9 @@ int FWTPM_TIS_ServerLoop(FWTPM_CTX* ctx)
406406
FWTPM_TIS_HAL* hal;
407407
FWTPM_TIS_REGS* regs;
408408
int rc;
409+
#ifndef _WIN32
410+
struct sigaction sa;
411+
#endif
409412

410413
if (ctx == NULL || ctx->tisRegs == NULL ||
411414
ctx->tisHal.wait_request == NULL) {
@@ -417,13 +420,10 @@ int FWTPM_TIS_ServerLoop(FWTPM_CTX* ctx)
417420
ctx->running = 1;
418421

419422
#ifndef _WIN32
420-
{
421-
struct sigaction sa;
422-
sa.sa_handler = SIG_IGN;
423-
sigemptyset(&sa.sa_mask);
424-
sa.sa_flags = 0;
425-
sigaction(SIGPIPE, &sa, NULL);
426-
}
423+
sa.sa_handler = SIG_IGN;
424+
sigemptyset(&sa.sa_mask);
425+
sa.sa_flags = 0;
426+
sigaction(SIGPIPE, &sa, NULL);
427427
#endif
428428

429429
printf("fwTPM TIS: Server ready, waiting for register accesses...\n");

‎src/tpm2_swtpm.c‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -95,27 +95,27 @@ static TPM_RC SwTpmTransmit(TPM2_CTX* ctx, const void* buffer, ssize_t bufSz)
9595
{
9696
TPM_RC rc = TPM_RC_SUCCESS;
9797
ssize_t wrc = 0;
98+
const char* ptr;
99+
int remaining;
98100

99101
if (ctx == NULL || ctx->tcpCtx.fd < 0 || buffer == NULL) {
100102
return BAD_FUNC_ARG;
101103
}
102104

103-
{
104-
const char* ptr = (const char*)buffer;
105-
int remaining = bufSz;
106-
while (remaining > 0) {
107-
wrc = write(ctx->tcpCtx.fd, ptr, remaining);
108-
if (wrc <= 0) {
109-
#ifdef WOLFTPM_DEBUG_VERBOSE
110-
printf("Failed to send the TPM command to fd %d, got errno %d ="
111-
"%s\n", ctx->tcpCtx.fd, errno, strerror(errno));
112-
#endif
113-
rc = TPM_RC_FAILURE;
114-
break;
115-
}
116-
remaining -= (int)wrc;
117-
ptr += wrc;
105+
ptr = (const char*)buffer;
106+
remaining = bufSz;
107+
while (remaining > 0) {
108+
wrc = write(ctx->tcpCtx.fd, ptr, remaining);
109+
if (wrc <= 0) {
110+
#ifdef WOLFTPM_DEBUG_VERBOSE
111+
printf("Failed to send the TPM command to fd %d, got errno %d ="
112+
"%s\n", ctx->tcpCtx.fd, errno, strerror(errno));
113+
#endif
114+
rc = TPM_RC_FAILURE;
115+
break;
118116
}
117+
remaining -= (int)wrc;
118+
ptr += wrc;
119119
}
120120

121121
return rc;
@@ -170,6 +170,7 @@ static TPM_RC SwTpmConnect(TPM2_CTX* ctx, const char* host, const char* port)
170170
struct termios tty;
171171
speed_t baud;
172172
int baudInt;
173+
struct stat devStat;
173174

174175
if (ctx == NULL) {
175176
return BAD_FUNC_ARG;
@@ -185,12 +186,9 @@ static TPM_RC SwTpmConnect(TPM2_CTX* ctx, const char* host, const char* port)
185186
return TPM_RC_FAILURE;
186187
}
187188
/* Verify the opened path is a character device */
188-
{
189-
struct stat st;
190-
if (fstat(fd, &st) != 0 || !S_ISCHR(st.st_mode)) {
191-
close(fd);
192-
return TPM_RC_FAILURE;
193-
}
189+
if (fstat(fd, &devStat) != 0 || !S_ISCHR(devStat.st_mode)) {
190+
close(fd);
191+
return TPM_RC_FAILURE;
194192
}
195193

196194
/* Configure serial port: 8N1, raw mode, no flow control */

‎src/tpm2_wrap.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1866,6 +1866,10 @@ int wolfTPM2_SetSessionHandle(WOLFTPM2_DEV* dev, int index,
18661866

18671867
session->auth.size = tpmSession->handle.auth.size;
18681868
if (session->auth.size > sizeof(session->auth.buffer)) {
1869+
#ifdef DEBUG_WOLFTPM
1870+
printf("SetSessionHandle: auth size %d exceeds buffer %d\n",
1871+
session->auth.size, (int)sizeof(session->auth.buffer));
1872+
#endif
18691873
return BUFFER_E;
18701874
}
18711875
XMEMCPY(session->auth.buffer, tpmSession->handle.auth.buffer,

‎wolftpm/fwtpm/fwtpm.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ typedef struct FWTPM_CTX {
428428
/* Per-boot context protection key (volatile only, never persisted).
429429
* Used by ContextSave/ContextLoad for HMAC + AES-CFB protection of
430430
* session context blobs per TPM 2.0 Part 1 §30. */
431-
byte ctxProtectKey[32];
431+
byte ctxProtectKey[AES_256_KEY_SIZE];
432432
int ctxProtectKeyValid;
433433

434434
/* TIS transport state (when not using sockets) */

0 commit comments

Comments
 (0)