peer review fixes

pull/531/head
David Garske 2026-06-25 15:17:07 -07:00
parent ae88ab8812
commit ec93a9b733
9 changed files with 81 additions and 29 deletions

6
.gitignore vendored
View File

@ -41,6 +41,9 @@ build/
examples/wrap/wrap_test
examples/wrap/caps
examples/wrap/hmac
examples/wrap/getrandom
examples/wrap/hash
examples/wrap/encrypt_decrypt
examples/native/native_test
examples/bench/bench
examples/csr/csr
@ -56,6 +59,7 @@ examples/pcr/reset
examples/timestamp/clock_set
examples/management/flush
examples/management/tpmclear
examples/management/da_check
pkcs7tpmsigned.p7s
pkcs7tpmsignedex.p7s
examples/tls/tls_server
@ -70,6 +74,7 @@ examples/keygen/keyload
examples/keygen/keygen
examples/keygen/keyimport
examples/keygen/external_import
examples/keygen/ecdh
examples/pqc/mldsa_sign
examples/pqc/mlkem_encap
examples/pqc/pqc_mssim_e2e
@ -207,6 +212,7 @@ examples/firmware/*.MANIFESTHASH
src/fwtpm/fwtpm_server
fwtpm_nv.bin
fwtpm_test_nv.bin
fwtpm_test_nv.bin.key
# Fuzz artifacts (corpus generated at runtime by gen_corpus.py)
tests/fuzz/corpus/

View File

@ -90,9 +90,20 @@ static void usage(void)
#ifdef WOLFTPM_MLDSA
static int mldsaParamSet(const char* optVal, TPMI_MLDSA_PARAMETER_SET* ps)
{
int n = XATOI(optVal);
int n;
const char* p;
if (optVal[0] == '\0') { /* missing or empty suffix, use default */
*ps = TPM_MLDSA_65;
return TPM_RC_SUCCESS;
}
/* reject non-digit input (e.g. -mldsa=abc) before XATOI */
for (p = optVal; *p != '\0'; p++) {
if (*p < '0' || *p > '9')
return TPM_RC_FAILURE;
}
n = XATOI(optVal);
switch (n) {
case 0: /* missing or empty suffix, use default */
case 65: *ps = TPM_MLDSA_65; return TPM_RC_SUCCESS;
case 44: *ps = TPM_MLDSA_44; return TPM_RC_SUCCESS;
case 87: *ps = TPM_MLDSA_87; return TPM_RC_SUCCESS;
@ -147,7 +158,7 @@ int TPM2_CreatePrimaryKey_Example(void* userCtx, int argc, char *argv[])
argv[argc-1] + 7 : "";
if (mldsaParamSet(optVal, &mldsaPs) != TPM_RC_SUCCESS) {
usage();
return 0;
return -1;
}
alg = TPM_ALG_MLDSA;
}

View File

@ -74,6 +74,7 @@ int TPM2_NVRAM_Counter_Example(void* userCtx, int argc, char *argv[])
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
TPM_ALG_ID pqcParamEncAlg = TPM_ALG_NULL;
int pqcParamSet = 0;
int pqcRc = 0;
WOLFTPM2_KEY pqcKey;
#endif
word32 nvIndex = TPM2_DEMO_NV_COUNTER_INDEX;
@ -121,13 +122,15 @@ int TPM2_NVRAM_Counter_Example(void* userCtx, int argc, char *argv[])
paramEncAlg = TPM_ALG_XOR;
}
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
else if (parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet) != 0) {
/* PQC option; an unsupported parameter set (alg left NULL) is a
* fatal typo, not a silent drop of parameter encryption. */
if (pqcParamEncAlg == TPM_ALG_NULL) {
else if ((pqcRc = parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet)) != 0) {
/* An unsupported parameter set (parser returns -1) is a fatal typo,
* not a silent drop of parameter encryption. Branch on the parser
* result directly: pqcParamEncAlg may already hold a valid alg from
* an earlier PQC option (argv is parsed right-to-left). */
if (pqcRc < 0) {
usage();
return 0;
return -1;
}
}
#endif

View File

@ -82,6 +82,7 @@ int TPM2_NVRAM_Store_Example(void* userCtx, int argc, char *argv[])
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
TPM_ALG_ID pqcParamEncAlg = TPM_ALG_NULL;
int pqcParamSet = 0;
int pqcRc = 0;
WOLFTPM2_KEY pqcKey;
#endif
int partialStore = 0;
@ -135,13 +136,15 @@ int TPM2_NVRAM_Store_Example(void* userCtx, int argc, char *argv[])
partialStore = PUBLIC_PART_ONLY;
}
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
else if (parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet) != 0) {
/* PQC option; an unsupported parameter set (alg left NULL) is a
* fatal typo, not a silent drop of parameter encryption. */
if (pqcParamEncAlg == TPM_ALG_NULL) {
else if ((pqcRc = parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet)) != 0) {
/* An unsupported parameter set (parser returns -1) is a fatal typo,
* not a silent drop of parameter encryption. Branch on the parser
* result directly: pqcParamEncAlg may already hold a valid alg from
* an earlier PQC option (argv is parsed right-to-left). */
if (pqcRc < 0) {
usage();
return 0;
return -1;
}
}
#endif

View File

@ -91,6 +91,7 @@ int TPM2_PCR_Quote_Test(void* userCtx, int argc, char *argv[])
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
TPM_ALG_ID pqcParamEncAlg = TPM_ALG_NULL;
int pqcParamSet = 0;
int pqcRc = 0;
WOLFTPM2_KEY pqcKey;
#endif
#if !defined(NO_FILESYSTEM) && !defined(NO_WRITE_TEMP_FILES)
@ -140,13 +141,15 @@ int TPM2_PCR_Quote_Test(void* userCtx, int argc, char *argv[])
paramEncAlg = TPM_ALG_XOR;
}
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
else if (parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet) != 0) {
/* PQC option; an unsupported parameter set (alg left NULL) is a
* fatal typo, not a silent drop of parameter encryption. */
if (pqcParamEncAlg == TPM_ALG_NULL) {
else if ((pqcRc = parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet)) != 0) {
/* An unsupported parameter set (parser returns -1) is a fatal typo,
* not a silent drop of parameter encryption. Branch on the parser
* result directly: pqcParamEncAlg may already hold a valid alg from
* an earlier PQC option (argv is parsed right-to-left). */
if (pqcRc < 0) {
usage();
return 0;
return -1;
}
}
#endif

View File

@ -383,6 +383,20 @@ if [ $ENABLE_V185 -eq 1 ]; then
[ $RESULT -ne 0 ] && echo -e "create_primary mldsa=$PS failed! $RESULT" && exit 1
done
echo -e "PQC usage-error checks (invalid parameter sets must be rejected)"
# These return before touching the TPM; a zero exit means an invalid
# parameter set was silently accepted as the default.
./examples/keygen/create_primary -mldsa=0 -oh >> $TPMPWD/run.out 2>&1
[ $? -eq 0 ] && echo -e "create_primary -mldsa=0 should fail!" && exit 1
./examples/keygen/create_primary -mldsa=abc -oh >> $TPMPWD/run.out 2>&1
[ $? -eq 0 ] && echo -e "create_primary -mldsa=abc should fail!" && exit 1
# An invalid PQC option must stay fatal even after a valid one (argv is
# parsed right-to-left), in either argument order.
./examples/wrap/wrap_test -aes -mldsa=999 -mlkem=768 >> $TPMPWD/run.out 2>&1
[ $? -eq 0 ] && echo -e "wrap_test -mldsa=999 -mlkem=768 should fail!" && exit 1
./examples/wrap/wrap_test -aes -mlkem=768 -mldsa=999 >> $TPMPWD/run.out 2>&1
[ $? -eq 0 ] && echo -e "wrap_test -mlkem=768 -mldsa=999 should fail!" && exit 1
echo -e "PQC parameter encryption (ML-KEM salt / ML-DSA bind)"
# ML-KEM as the param-enc session salt, ML-DSA as the param-enc session
# bind; exercise AES-CFB and XOR across child-create, attestation and NV.

View File

@ -138,6 +138,7 @@ int TPM2_Wrapper_TestArgs(void* userCtx, int argc, char *argv[])
* (ML-KEM) or bind (ML-DSA) key. */
TPM_ALG_ID pqcParamEncAlg = TPM_ALG_NULL;
int pqcParamSet = 0;
int pqcRc = 0;
WOLFTPM2_KEY pqcKey;
#endif
@ -178,13 +179,15 @@ int TPM2_Wrapper_TestArgs(void* userCtx, int argc, char *argv[])
paramEncAlg = TPM_ALG_XOR;
}
#if defined(WOLFTPM_MLKEM) || defined(WOLFTPM_MLDSA)
else if (parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet) != 0) {
/* PQC option; an unsupported parameter set (alg left NULL) is a
* fatal typo, not a silent drop of parameter encryption. */
if (pqcParamEncAlg == TPM_ALG_NULL) {
else if ((pqcRc = parsePqcParamEncArg(argv[argc-1], &pqcParamEncAlg,
&pqcParamSet)) != 0) {
/* An unsupported parameter set (parser returns -1) is a fatal typo,
* not a silent drop of parameter encryption. Branch on the parser
* result directly: pqcParamEncAlg may already hold a valid alg from
* an earlier PQC option (argv is parsed right-to-left). */
if (pqcRc < 0) {
usage();
return 0;
return -1;
}
}
#endif

View File

@ -224,7 +224,7 @@ static int FwBuildParamEncKey(FWTPM_Session* sess, TPM_HANDLE authHandle,
static int FwParamDecryptCmd(FWTPM_CTX* ctx, FWTPM_Session* sess,
TPM_HANDLE authHandle, byte* paramData, UINT32 paramSz)
{
int rc;
int rc = TPM_RC_FAILURE;
byte keyBuf[TPM_MAX_DIGEST_SIZE * 2];
int keyBufSz = 0;
@ -247,6 +247,9 @@ static int FwParamDecryptCmd(FWTPM_CTX* ctx, FWTPM_Session* sess,
sess->nonceTPM.buffer, sess->nonceTPM.size,
paramData, paramSz, 0); /* decrypt */
}
else {
rc = TPM_RC_SYMMETRIC; /* unsupported param-enc cipher */
}
TPM2_ForceZero(keyBuf, sizeof(keyBuf));
(void)ctx;
@ -257,7 +260,7 @@ static int FwParamDecryptCmd(FWTPM_CTX* ctx, FWTPM_Session* sess,
static int FwParamEncryptRsp(FWTPM_CTX* ctx, FWTPM_Session* sess,
TPM_HANDLE authHandle, byte* paramData, UINT32 paramSz)
{
int rc;
int rc = TPM_RC_FAILURE;
byte keyBuf[TPM_MAX_DIGEST_SIZE * 2];
int keyBufSz = 0;
@ -281,6 +284,9 @@ static int FwParamEncryptRsp(FWTPM_CTX* ctx, FWTPM_Session* sess,
sess->nonceCaller.buffer, sess->nonceCaller.size,
paramData, paramSz, 1); /* encrypt */
}
else {
rc = TPM_RC_SYMMETRIC; /* unsupported param-enc cipher */
}
TPM2_ForceZero(keyBuf, sizeof(keyBuf));
(void)ctx;

View File

@ -4,6 +4,9 @@ project(wolftpm_wrap_test)
# Include source code for wrap test
target_sources(app PRIVATE ${ZEPHYR_WOLFTPM_MODULE_DIR}/examples/wrap/wrap_test.c)
# wrap_test calls PQC param-enc helpers from examples/tpm_test_keys.c under
# WOLFTPM_MLKEM / WOLFTPM_MLDSA; compile it so those references resolve.
target_sources(app PRIVATE ${ZEPHYR_WOLFTPM_MODULE_DIR}/examples/tpm_test_keys.c)
target_sources(app PRIVATE ${app_sources})
# Include header files