Skip to content

Commit b7253ef

Browse files
committed
fwtpm: peer-review batch 5 + CI --disable-fwtpm
CI: Add --disable-fwtpm to non-fwtpm CI jobs that auto-enable fwtpm on Linux x86_64 (autodetect, pedantic, clang-asan, symmetric, swecdhe, old-wolfssl, multi-compiler, sanitizer). Security (High): - Session flush: free sessions when continueSession bit is not set in command attributes (TPM 2.0 Part 1 §19.6.4) - Import inner integrity: verify Hash(decryptedSens || objectName) after AES-CFB inner decrypt, reject TPM_RC_INTEGRITY on mismatch - FwImportReconstructKey: reject sensType != objectPublic.type - KDFa derivation counter: start at 1 per TPM 2.0 spec (was 0) - ECDH_ZGen/ZGen_2Phase: wc_ecc_check_key on peer points - ZGen_2Phase: ForceZero ephemeral key after use Security (Medium): - ParamEnc/ParamDec: return TPM_RC_FAILURE for unknown symmetric alg - FwComputeSessionHmac: bounds check sessionKey.size - FwImportParseSensitive: validate totalSensSize against buffer - FwDerivePrime: 28 Miller-Rabin rounds per FIPS 186-4 (was 8) - SwTpmTransmit: reject negative bufSz
1 parent 262be34 commit b7253ef

File tree

5 files changed

+94
-22
lines changed

5 files changed

+94
-22
lines changed

.github/workflows/make-test-swtpm.yml

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,38 +140,33 @@ jobs:
140140

141141
# Autodetect (default configure, /dev/tpm0 + SPI dual support)
142142
- name: autodetect
143-
wolftpm_config: ""
143+
wolftpm_config: "--disable-fwtpm"
144144
needs_swtpm: false
145145

146146
# Autodetect with debug
147147
- name: autodetect-debug
148-
wolftpm_config: --enable-debug
148+
wolftpm_config: "--enable-debug --disable-fwtpm"
149149
needs_swtpm: false
150150

151-
# Clang ASAN
152-
- name: clang-asan
153-
wolftpm_cflags: "-fsanitize=address -fno-omit-frame-pointer -g"
154-
wolftpm_cc: clang
155-
test_command: "make check && ASAN_OPTIONS=detect_leaks=1:abort_on_error=1 WOLFSSL_PATH=./wolfssl ./examples/run_examples.sh"
156-
157151
# Pedantic
158152
- name: pedantic
159-
wolftpm_config: ""
160153
wolftpm_cflags: "-Wpedantic"
161154
needs_swtpm: false
162155

163-
# Not provisioning
156+
# No provisioning
164157
- name: no-provisioning
165158
wolftpm_config: --disable-provisioning
166159
needs_swtpm: false
167160

168161
# Symmetric encryption
169162
- name: symmetric
163+
wolftpm_config: "--enable-swtpm --disable-fwtpm"
170164
wolftpm_cflags: "-DWOLFTPM_USE_SYMMETRIC"
171165
test_command: "make check && WOLFSSL_PATH=./wolfssl ./examples/run_examples.sh"
172166

173167
# Software ECDHE
174168
- name: swecdhe
169+
wolftpm_config: "--enable-swtpm --disable-fwtpm"
175170
wolftpm_cflags: "-DWOLFTPM2_USE_SW_ECDHE"
176171
test_command: "make check && WOLFSSL_PATH=./wolfssl ./examples/run_examples.sh"
177172

@@ -204,6 +199,7 @@ jobs:
204199
# Builds latest wolfSSL for examples/client/client and examples/server/server
205200
# Builds old wolfSSL (v4.7.0) for linking wolfTPM against older shared library
206201
- name: old-wolfssl
202+
wolftpm_config: "--enable-swtpm --disable-fwtpm"
207203
test_command: "make check && WOLFSSL_PATH=./wolfssl NO_PUBASPRIV=1 ./examples/run_examples.sh"
208204
needs_install: true
209205

src/fwtpm/fwtpm_command.c

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,8 @@ static int FwComputeSessionHmac(FWTPM_Session* sess,
465465
*hmacOutSz = dSize;
466466

467467
/* Build HMAC key = sessionKey || authValue */
468-
if (sess->sessionKey.size > 0) {
468+
if (sess->sessionKey.size > 0 &&
469+
sess->sessionKey.size <= TPM_MAX_DIGEST_SIZE) {
469470
XMEMCPY(hmacKey, sess->sessionKey.buffer, sess->sessionKey.size);
470471
hmacKeySz = sess->sessionKey.size;
471472
}
@@ -4208,6 +4209,7 @@ static TPM_RC FwCmd_Import(FWTPM_CTX* ctx, TPM2_Packet* cmd,
42084209
int plainSensSz = 0;
42094210
UINT16 sensType = 0;
42104211
UINT16 primeSz = 0;
4212+
FWTPM_DECLARE_VAR(innerHashCtx, wc_HashAlg);
42114213
FWTPM_DECLARE_BUF(primeBuf, FWTPM_MAX_DER_SIG_BUF);
42124214
TPM2B_AUTH importedAuth;
42134215
TPM_RC rc = TPM_RC_SUCCESS;
@@ -4218,6 +4220,7 @@ static TPM_RC FwCmd_Import(FWTPM_CTX* ctx, TPM2_Packet* cmd,
42184220
FWTPM_ALLOC_BUF(pubAreaBuf, FWTPM_MAX_PUB_BUF);
42194221
FWTPM_ALLOC_BUF(plainSens, FWTPM_MAX_SENSITIVE_SIZE);
42204222
FWTPM_ALLOC_BUF(primeBuf, FWTPM_MAX_DER_SIG_BUF);
4223+
FWTPM_ALLOC_VAR(innerHashCtx, wc_HashAlg);
42214224

42224225
if (cmdSize < TPM2_HEADER_SIZE + 4) {
42234226
rc = TPM_RC_COMMAND_SIZE;
@@ -4440,6 +4443,8 @@ static TPM_RC FwCmd_Import(FWTPM_CTX* ctx, TPM2_Packet* cmd,
44404443
int innerStart;
44414444
byte* encInner;
44424445
int encInnerSz;
4446+
byte innerHash[TPM_MAX_DIGEST_SIZE];
4447+
int ihRc;
44434448

44444449
/* Parse inner integrity size */
44454450
innerIntegSz = FwLoadU16BE(plainSens);
@@ -4465,6 +4470,38 @@ static TPM_RC FwCmd_Import(FWTPM_CTX* ctx, TPM2_Packet* cmd,
44654470
}
44664471
}
44674472

4473+
/* Verify inner integrity: Hash(decryptedSensitive || objectName)
4474+
* Per TPM 2.0 Part 1 §23.4 */
4475+
if (rc == 0 && objDigestSz > 0 &&
4476+
(UINT16)objDigestSz != innerIntegSz) {
4477+
rc = TPM_RC_INTEGRITY;
4478+
}
4479+
if (rc == 0) {
4480+
ihRc = wc_HashInit(innerHashCtx, objWcHash);
4481+
if (ihRc == 0) {
4482+
ihRc = wc_HashUpdate(innerHashCtx, objWcHash,
4483+
encInner, (word32)encInnerSz);
4484+
}
4485+
if (ihRc == 0) {
4486+
ihRc = wc_HashUpdate(innerHashCtx, objWcHash,
4487+
nameBuf, (word32)nameSz);
4488+
}
4489+
if (ihRc == 0) {
4490+
ihRc = wc_HashFinal(innerHashCtx, objWcHash, innerHash);
4491+
}
4492+
wc_HashFree(innerHashCtx, objWcHash);
4493+
if (ihRc != 0) {
4494+
rc = TPM_RC_FAILURE;
4495+
}
4496+
if (rc == 0) {
4497+
if (TPM2_ConstantCompare(innerHash, plainSens + 2,
4498+
innerIntegSz) != 0) {
4499+
rc = TPM_RC_INTEGRITY;
4500+
}
4501+
}
4502+
TPM2_ForceZero(innerHash, sizeof(innerHash));
4503+
}
4504+
44684505
/* Shift decrypted sensitive to beginning of plainSens buffer */
44694506
if (rc == 0) {
44704507
XMEMMOVE(plainSens, plainSens + innerStart, encInnerSz);
@@ -4526,6 +4563,7 @@ static TPM_RC FwCmd_Import(FWTPM_CTX* ctx, TPM2_Packet* cmd,
45264563
FWTPM_FREE_BUF(plainSens);
45274564
TPM2_ForceZero(primeBuf, FWTPM_MAX_DER_SIG_BUF);
45284565
FWTPM_FREE_BUF(primeBuf);
4566+
FWTPM_FREE_VAR(innerHashCtx);
45294567
return rc;
45304568
}
45314569

@@ -6983,6 +7021,13 @@ static TPM_RC FwCmd_ECDH_ZGen(FWTPM_CTX* ctx, TPM2_Packet* cmd,
69837021
rc = TPM_RC_FAILURE;
69847022
}
69857023
}
7024+
/* Validate peer point is on the expected curve (invalid curve attack) */
7025+
if (rc == 0) {
7026+
rc = wc_ecc_check_key(peerPub);
7027+
if (rc != 0) {
7028+
rc = TPM_RC_ECC_POINT;
7029+
}
7030+
}
69867031

69877032
/* Compute shared secret */
69887033
if (rc == 0) {
@@ -11968,6 +12013,10 @@ static TPM_RC FwCmd_ZGen_2Phase(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1196812013
rc = wc_ecc_import_unsigned(peerPub,
1196912014
inQsB.point.x.buffer, inQsB.point.y.buffer, NULL, wcCurve);
1197012015
}
12016+
if (rc == 0) {
12017+
rc = wc_ecc_check_key(peerPub);
12018+
if (rc != 0) rc = TPM_RC_ECC_POINT;
12019+
}
1197112020
if (rc == 0) {
1197212021
z1Sz = (word32)sizeof(z1Buf);
1197312022
rc = wc_ecc_shared_secret(privKeyA, peerPub, z1Buf, &z1Sz);
@@ -12000,6 +12049,10 @@ static TPM_RC FwCmd_ZGen_2Phase(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1200012049
rc = wc_ecc_import_unsigned(peerPub,
1200112050
inQeB.point.x.buffer, inQeB.point.y.buffer, NULL, wcCurve);
1200212051
}
12052+
if (rc == 0) {
12053+
rc = wc_ecc_check_key(peerPub);
12054+
if (rc != 0) rc = TPM_RC_ECC_POINT;
12055+
}
1200312056
if (rc == 0) {
1200412057
z2Sz = (word32)sizeof(z2Buf);
1200512058
rc = wc_ecc_shared_secret(privEph, peerPub, z2Buf, &z2Sz);
@@ -12030,6 +12083,9 @@ static TPM_RC FwCmd_ZGen_2Phase(FWTPM_CTX* ctx, TPM2_Packet* cmd,
1203012083
/* Cleanup */
1203112084
TPM2_ForceZero(z1Buf, sizeof(z1Buf));
1203212085
TPM2_ForceZero(z2Buf, sizeof(z2Buf));
12086+
/* Zero ephemeral key — it was consumed and must not be reused */
12087+
TPM2_ForceZero(ctx->ecEphemeralKey, sizeof(ctx->ecEphemeralKey));
12088+
ctx->ecEphemeralKeySz = 0;
1203312089
if (privAInit) wc_ecc_free(privKeyA);
1203412090
if (ephInit) wc_ecc_free(privEph);
1203512091
if (peerInit) wc_ecc_free(peerPub);
@@ -12929,12 +12985,17 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx,
1292912985
*rspSize = rspPkt.pos;
1293012986
}
1293112987

12932-
/* Deferred clear: flush transient objects after response auth is built.
12933-
* Sessions are NOT flushed here — the calling tool's ESYS layer flushes
12934-
* its own auth session handle after the command response, and flushing it
12935-
* early would cause TPM_RC_HANDLE (0x8B). Stale sessions from prior
12936-
* commands are naturally invalidated when their auth values change, and
12937-
* the session slots are reused on next StartAuthSession. */
12988+
/* Per TPM 2.0 spec Part 1 §19.6.4: flush sessions where the caller
12989+
* did NOT set the continueSession bit in the command attributes.
12990+
* This must happen AFTER the response auth area is built. */
12991+
for (pj = 0; pj < cmdAuthCnt; pj++) {
12992+
if (cmdAuths[pj].sess != NULL &&
12993+
!(cmdAuths[pj].attributes & TPMA_SESSION_continueSession)) {
12994+
FwFreeSession(cmdAuths[pj].sess);
12995+
}
12996+
}
12997+
12998+
/* Deferred clear: flush transient objects after response auth is built. */
1293812999
if (ctx->pendingClear) {
1293913000
int nvRc;
1294013001
ctx->pendingClear = 0;

src/fwtpm/fwtpm_crypto.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ TPM_RC FwDeriveEccPrimaryKey(TPMI_ALG_HASH nameAlg,
552552
word32 xSz, ySz;
553553
byte dBuf[MAX_ECC_BYTES];
554554
byte counterBuf[4];
555-
UINT32 counter = 0;
555+
UINT32 counter = 1;
556556
int kdfRet;
557557
int valid = 0;
558558
int i;
@@ -657,7 +657,7 @@ static TPM_RC FwDerivePrime(TPMI_ALG_HASH nameAlg,
657657
TPM_RC rc = TPM_RC_SUCCESS;
658658
byte primeBuf[256]; /* max RSA-4096 half = 256 bytes */
659659
byte counterBuf[4];
660-
UINT32 counter = 0;
660+
UINT32 counter = 1;
661661
int isPrime = 0;
662662
int kdfRet;
663663

@@ -686,7 +686,8 @@ static TPM_RC FwDerivePrime(TPMI_ALG_HASH nameAlg,
686686
break;
687687
}
688688

689-
rc = mp_prime_is_prime_ex(outPrime, 8, &isPrime, rng);
689+
/* FIPS 186-4 Table C.3: 28 rounds for RSA-2048 */
690+
rc = mp_prime_is_prime_ex(outPrime, 28, &isPrime, rng);
690691
if (rc != 0) {
691692
rc = TPM_RC_FAILURE;
692693
break;
@@ -1886,7 +1887,9 @@ TPM_RC FwImportParseSensitive(
18861887
if (rc == 0) {
18871888
totalSensSize = FwLoadU16BE(plainSens + sp);
18881889
sp += 2;
1889-
(void)totalSensSize;
1890+
if (totalSensSize + 2 > (UINT16)plainSensSz) {
1891+
return TPM_RC_SIZE;
1892+
}
18901893
if (sp + 2 > plainSensSz) {
18911894
rc = TPM_RC_FAILURE;
18921895
}
@@ -1968,6 +1971,12 @@ TPM_RC FwImportReconstructKey(
19681971
{
19691972
TPM_RC rc = TPM_RC_SUCCESS;
19701973

1974+
/* Validate that sensitive type matches the public area type to prevent
1975+
* type confusion (e.g., ECC scalar used with RSA public params) */
1976+
if (sensType != objectPublic->publicArea.type) {
1977+
return TPM_RC_TYPE;
1978+
}
1979+
19711980
#ifdef HAVE_ECC
19721981
if (sensType == TPM_ALG_ECC && primeSz > 0) {
19731982
FWTPM_DECLARE_VAR(eccKey, ecc_key);

src/tpm2_param_enc.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ TPM_RC TPM2_ParamEnc_CmdRequest(TPM2_AUTH_SESSION *session,
229229
session->nonceTPM.buffer, session->nonceTPM.size,
230230
paramData, paramSz, 1);
231231
}
232+
else {
233+
rc = TPM_RC_FAILURE;
234+
}
232235

233236
TPM2_ForceZero(keyBuf, sizeof(keyBuf));
234237
return rc;
@@ -275,6 +278,9 @@ TPM_RC TPM2_ParamDec_CmdResponse(TPM2_AUTH_SESSION *session,
275278
session->nonceCaller.buffer, session->nonceCaller.size,
276279
paramData, paramSz, 0);
277280
}
281+
else {
282+
rc = TPM_RC_FAILURE;
283+
}
278284

279285
TPM2_ForceZero(keyBuf, sizeof(keyBuf));
280286
return rc;

src/tpm2_swtpm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ static TPM_RC SwTpmTransmit(TPM2_CTX* ctx, const void* buffer, ssize_t bufSz)
9898
const char* ptr;
9999
int remaining;
100100

101-
if (ctx == NULL || ctx->tcpCtx.fd < 0 || buffer == NULL) {
101+
if (ctx == NULL || ctx->tcpCtx.fd < 0 || buffer == NULL || bufSz <= 0) {
102102
return BAD_FUNC_ARG;
103103
}
104104

0 commit comments

Comments
 (0)