From ce229628b22039d36e158ef7098a221fb8b16282 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 10 Dec 2025 15:10:41 -0800 Subject: [PATCH 1/2] Fix for wolfTPM2_SignHash to return padded r/s. Improves handling of ECDSA with P521. Adds tests for ECDSA with crypto callbacks. ZD20777 --- src/tpm2.c | 6 +++ src/tpm2_cryptocb.c | 2 - src/tpm2_wrap.c | 29 ++++++------ tests/unit_tests.c | 108 +++++++++++++++++++++++++++++++------------- 4 files changed, 99 insertions(+), 46 deletions(-) diff --git a/src/tpm2.c b/src/tpm2.c index 994f8f7e..1e998278 100644 --- a/src/tpm2.c +++ b/src/tpm2.c @@ -6111,6 +6111,12 @@ const char* TPM2_GetAlgName(TPM_ALG_ID alg) return "SHA384"; case TPM_ALG_SHA512: return "SHA512"; + case TPM_ALG_SHA3_256: + return "SHA3_256"; + case TPM_ALG_SHA3_384: + return "SHA3_384"; + case TPM_ALG_SHA3_512: + return "SHA3_512"; case TPM_ALG_NULL: return "NULL"; case TPM_ALG_SM3_256: diff --git a/src/tpm2_cryptocb.c b/src/tpm2_cryptocb.c index 510b0d1e..7ae57fcc 100644 --- a/src/tpm2_cryptocb.c +++ b/src/tpm2_cryptocb.c @@ -328,8 +328,6 @@ int wolfTPM2_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx) rLen = sLen = rsLen / 2; r = &sigRS[0]; s = &sigRS[rLen]; - r = TPM2_ASN_TrimZeros(r, &rLen); - s = TPM2_ASN_TrimZeros(s, &sLen); /* Encode ECDSA Header */ rc = wc_ecc_rs_raw_to_sig(r, rLen, s, sLen, diff --git a/src/tpm2_wrap.c b/src/tpm2_wrap.c index fc8c6c33..14df3cef 100644 --- a/src/tpm2_wrap.c +++ b/src/tpm2_wrap.c @@ -4080,32 +4080,35 @@ int wolfTPM2_SignHashScheme(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* key, } if (key->pub.publicArea.type == TPM_ALG_ECC) { + TPMS_SIGNATURE_ECDSA* ecdsa = &signOut.signature.signature.ecdsa; + /* Assemble R and S into signature (R then S) */ - sigOutSz = signOut.signature.signature.ecdsa.signatureR.size + - signOut.signature.signature.ecdsa.signatureS.size; + sigOutSz = curveSize * 2; if (sigOutSz > *sigSz) { #ifdef DEBUG_WOLFTPM - printf("TPM2_Sign: ECC result truncated %d -> %d\n", + printf("TPM2_Sign: ECC result buffer too small %d -> %d\n", sigOutSz, *sigSz); #endif - sigOutSz = *sigSz; + return BUFFER_E; } - XMEMCPY(sig, - signOut.signature.signature.ecdsa.signatureR.buffer, - signOut.signature.signature.ecdsa.signatureR.size); - XMEMCPY(sig + signOut.signature.signature.ecdsa.signatureR.size, - signOut.signature.signature.ecdsa.signatureS.buffer, - signOut.signature.signature.ecdsa.signatureS.size); + XMEMCPY(sig, ecdsa->signatureR.buffer, + ecdsa->signatureR.size); + XMEMSET(sig + ecdsa->signatureR.size, 0, + curveSize - ecdsa->signatureR.size); + XMEMCPY(sig + curveSize, ecdsa->signatureS.buffer, + ecdsa->signatureS.size); + XMEMSET(sig + curveSize + ecdsa->signatureS.size, 0, + curveSize - ecdsa->signatureS.size); } else if (key->pub.publicArea.type == TPM_ALG_RSA) { /* RSA signature size and buffer (with padding depending on scheme) */ sigOutSz = signOut.signature.signature.rsassa.sig.size; if (sigOutSz > *sigSz) { #ifdef DEBUG_WOLFTPM - printf("TPM2_Sign: RSA result truncated %d -> %d\n", + printf("TPM2_Sign: RSA result buffer too small %d -> %d\n", sigOutSz, *sigSz); #endif - sigOutSz = *sigSz; + return BUFFER_E; } XMEMCPY(sig, signOut.signature.signature.rsassa.sig.buffer, sigOutSz); } @@ -7368,7 +7371,7 @@ static int CSR_MakeAndSign(WOLFTPM2_DEV* dev, WOLFTPM2_CSR* csr, CSRKey* key, } #else #ifdef DEBUG_WOLFTPM - printf("CSR_MakeAndSign PEM not supported\n") + printf("CSR_MakeAndSign PEM not supported\n"); #endif rc = NOT_COMPILED_IN; #endif diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 08ddcfd4..2d764e43 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -409,9 +409,12 @@ static void test_wolfTPM2_CSR(void) #if !defined(WOLFTPM2_NO_WOLFCRYPT) && defined(HAVE_ECC) && \ !defined(WOLFTPM2_NO_ASN) +#define FLAGS_USE_WOLFCRYPT (1 << 0) +#define FLAGS_USE_CRYPTO_CB (1 << 1) +#define FLAGS_USE_PK_CB (1 << 2) /* requires TLS layer to test */ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* storageKey, const byte* digest, int digestSz, - TPM_ECC_CURVE curve, TPMI_ALG_HASH hashAlg) + TPM_ECC_CURVE curve, TPMI_ALG_HASH hashAlg, int flags) { int rc; int verifyRes = 0; @@ -425,6 +428,21 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, word32 rLen, sLen; ecc_key wolfKey; int curveSize = TPM2_GetCurveSize(curve); +#ifdef WOLF_CRYPTO_CB + int tpmDevId = INVALID_DEVID; + TpmCryptoDevCtx tpmCtx; + + XMEMSET(&tpmCtx, 0, sizeof(tpmCtx)); + tpmCtx.dev = dev; + tpmCtx.eccKey = &eccKey; + tpmCtx.storageKey = storageKey; + + if (flags & FLAGS_USE_CRYPTO_CB) { + rc = wolfTPM2_SetCryptoDevCb(dev, wolfTPM2_CryptoDevCb, &tpmCtx, + &tpmDevId); + AssertIntEQ(rc, 0); + } +#endif /* -- Use TPM key to sign and verify with wolfCrypt -- */ /* Create ECC key for signing */ @@ -439,6 +457,10 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, printf("Hash type not supported... Skipping\n"); return; } + if ((rc & TPM_RC_CURVE) == TPM_RC_CURVE) { + printf("Curve not supported... Skipping\n"); + return; + } AssertIntEQ(rc, 0); /* Sign with TPM */ @@ -450,8 +472,6 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, rLen = sLen = sigRsSz / 2; r = &sigRs[0]; s = &sigRs[rLen]; - r = TPM2_ASN_TrimZeros(r, &rLen); - s = TPM2_ASN_TrimZeros(s, &sLen); /* Encode ECDSA Header */ sigSz = (word32)sizeof(sig); @@ -459,7 +479,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, AssertIntEQ(rc, 0); /* Initialize wolfCrypt ECC key */ - rc = wc_ecc_init(&wolfKey); + rc = wc_ecc_init_ex(&wolfKey, NULL, tpmDevId); AssertIntEQ(rc, 0); /* Convert TPM key to wolfCrypt key for verification */ @@ -478,7 +498,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, /* -- Use wolfCrypt key to sign and verify with TPM -- */ /* Initialize new wolfCrypt ECC key */ - rc = wc_ecc_init(&wolfKey); + rc = wc_ecc_init_ex(&wolfKey, NULL, tpmDevId); AssertIntEQ(rc, 0); /* Generate new ECC key with wolfCrypt */ @@ -490,6 +510,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, rc = wc_ecc_sign_hash(digest, digestSz, sig, &sigSz, wolfTPM2_GetRng(dev), &wolfKey); AssertIntEQ(rc, 0); + wolfTPM2_UnloadHandle(dev, &eccKey.handle); /* Decode ECDSA Header */ r = sigRs; @@ -519,18 +540,63 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, wolfTPM2_UnloadHandle(dev, &eccKey.handle); printf("Test TPM Wrapper:\t" - "Sign/Verify (DigSz=%d, CurveSz=%d, Hash=%s):" + "Sign/Verify (DigSz=%d, CurveSz=%d, Hash=%s, Flags=%s):" "\t%s\n", digestSz, TPM2_GetCurveSize(curve), TPM2_GetAlgName(hashAlg), + (flags & FLAGS_USE_CRYPTO_CB) ? "Crypto CB" : "", rc == 0 ? "Passed" : "Failed"); + + if (flags & FLAGS_USE_CRYPTO_CB) { + wolfTPM2_ClearCryptoDevCb(dev, tpmDevId); + } +} + +static void test_wolfTPM2_EccSignVerify_All(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* storageKey, int flags) +{ + int i; + byte digest[TPM_MAX_DIGEST_SIZE]; + + for (i = 0; i < (int)sizeof(digest); i++) { + digest[i] = (byte)i; + } + + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 20, + TPM_ECC_NIST_P256, TPM_ALG_SHA256, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 32, + TPM_ECC_NIST_P256, TPM_ALG_SHA256, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 48, + TPM_ECC_NIST_P256, TPM_ALG_SHA256, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 64, + TPM_ECC_NIST_P256, TPM_ALG_SHA256, flags); + +#if (defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 384 + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 20, + TPM_ECC_NIST_P384, TPM_ALG_SHA384, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 32, + TPM_ECC_NIST_P384, TPM_ALG_SHA384, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 48, + TPM_ECC_NIST_P384, TPM_ALG_SHA384, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 64, + TPM_ECC_NIST_P384, TPM_ALG_SHA384, flags); +#endif + +#if (defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 521 + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 20, + TPM_ECC_NIST_P521, TPM_ALG_SHA512, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 32, + TPM_ECC_NIST_P521, TPM_ALG_SHA512, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 48, + TPM_ECC_NIST_P521, TPM_ALG_SHA512, flags); + test_wolfTPM2_EccSignVerifyDig(dev, storageKey, digest, 64, + TPM_ECC_NIST_P521, TPM_ALG_SHA512, flags); +#endif } /* Test with smaller, same and larger digest sizes using different ECC curves. * Interop sign and verify with wolfCrypt and TPM */ static void test_wolfTPM2_EccSignVerify(void) { - int rc, i; - byte digest[TPM_MAX_DIGEST_SIZE]; + int rc; WOLFTPM2_DEV dev; WOLFTPM2_KEY storageKey; @@ -543,29 +609,9 @@ static void test_wolfTPM2_EccSignVerify(void) (byte*)gStorageKeyAuth, sizeof(gStorageKeyAuth)-1); AssertIntEQ(rc, 0); - - for (i = 0; i < (int)sizeof(digest); i++) { - digest[i] = (byte)i; - } - - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 20, - TPM_ECC_NIST_P256, TPM_ALG_SHA256); - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 32, - TPM_ECC_NIST_P256, TPM_ALG_SHA256); - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 48, - TPM_ECC_NIST_P256, TPM_ALG_SHA256); - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 64, - TPM_ECC_NIST_P256, TPM_ALG_SHA256); - -#if (defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 384 - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 20, - TPM_ECC_NIST_P384, TPM_ALG_SHA384); - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 32, - TPM_ECC_NIST_P384, TPM_ALG_SHA384); - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 48, - TPM_ECC_NIST_P384, TPM_ALG_SHA384); - test_wolfTPM2_EccSignVerifyDig(&dev, &storageKey, digest, 64, - TPM_ECC_NIST_P384, TPM_ALG_SHA384); + test_wolfTPM2_EccSignVerify_All(&dev, &storageKey, 0); +#ifdef WOLF_CRYPTO_CB + test_wolfTPM2_EccSignVerify_All(&dev, &storageKey, FLAGS_USE_CRYPTO_CB); #endif wolfTPM2_UnloadHandle(&dev, &storageKey.handle); From 8c5c6cbcc8d23427315d73535ffafa71fd728b79 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 10 Dec 2025 15:22:27 -0800 Subject: [PATCH 2/2] Fix AddressSanitizer warning with overlapping memcpy (need to use memmove). Fix test case with `WOLFTPM2_USE_SW_ECDHE`. --- src/tpm2_cryptocb.c | 4 ++-- src/tpm2_wrap.c | 4 +++- tests/unit_tests.c | 32 ++++++++++++++++++++------------ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/tpm2_cryptocb.c b/src/tpm2_cryptocb.c index 7ae57fcc..a99ffc02 100644 --- a/src/tpm2_cryptocb.c +++ b/src/tpm2_cryptocb.c @@ -353,9 +353,9 @@ int wolfTPM2_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx) if (rc == 0) { /* combine R and S at key size (zero pad leading) */ word32 keySz = wc_ecc_size(info->pk.eccverify.key); - XMEMCPY(&sigRS[keySz-rLen], r, rLen); + XMEMMOVE(&sigRS[keySz-rLen], r, rLen); XMEMSET(&sigRS[0], 0, keySz-rLen); - XMEMCPY(&sigRS[keySz + (keySz-sLen)], s, sLen); + XMEMMOVE(&sigRS[keySz + (keySz-sLen)], s, sLen); XMEMSET(&sigRS[keySz], 0, keySz-sLen); rc = wolfTPM2_VerifyHash(tlsCtx->dev, &eccPub, sigRS, keySz*2, diff --git a/src/tpm2_wrap.c b/src/tpm2_wrap.c index 14df3cef..ed0d7b51 100644 --- a/src/tpm2_wrap.c +++ b/src/tpm2_wrap.c @@ -4084,7 +4084,9 @@ int wolfTPM2_SignHashScheme(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* key, /* Assemble R and S into signature (R then S) */ sigOutSz = curveSize * 2; - if (sigOutSz > *sigSz) { + if (sigOutSz > *sigSz || + curveSize > ecdsa->signatureR.size || + curveSize > ecdsa->signatureS.size) { #ifdef DEBUG_WOLFTPM printf("TPM2_Sign: ECC result buffer too small %d -> %d\n", sigOutSz, *sigSz); diff --git a/tests/unit_tests.c b/tests/unit_tests.c index 2d764e43..afbfceb2 100644 --- a/tests/unit_tests.c +++ b/tests/unit_tests.c @@ -418,7 +418,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, { int rc; int verifyRes = 0; - WOLFTPM2_KEY eccKey; + WOLFTPM2_KEYBLOB eccKey; TPMT_PUBLIC publicTemplate; byte sigRs[MAX_ECC_BYTES*2]; word32 sigRsSz = (word32)sizeof(sigRs); @@ -434,7 +434,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, XMEMSET(&tpmCtx, 0, sizeof(tpmCtx)); tpmCtx.dev = dev; - tpmCtx.eccKey = &eccKey; + tpmCtx.ecdsaKey = &eccKey; tpmCtx.storageKey = storageKey; if (flags & FLAGS_USE_CRYPTO_CB) { @@ -451,8 +451,14 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, TPMA_OBJECT_sign | TPMA_OBJECT_noDA), curve, TPM_ALG_ECDSA, hashAlg); AssertIntEQ(rc, 0); - rc = wolfTPM2_CreateAndLoadKey(dev, &eccKey, &storageKey->handle, + + /* Use create key and load key directly instead to make + * sure the private portion is populated */ + rc = wolfTPM2_CreateKey(dev, &eccKey, &storageKey->handle, &publicTemplate, (byte*)gKeyAuth, sizeof(gKeyAuth)-1); + if (rc == TPM_RC_SUCCESS) { + rc = wolfTPM2_LoadKey(dev, &eccKey, &storageKey->handle); + } if ((rc & TPM_RC_HASH) == TPM_RC_HASH) { printf("Hash type not supported... Skipping\n"); return; @@ -464,7 +470,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, AssertIntEQ(rc, 0); /* Sign with TPM */ - rc = wolfTPM2_SignHashScheme(dev, &eccKey, digest, digestSz, + rc = wolfTPM2_SignHashScheme(dev, (WOLFTPM2_KEY*)&eccKey, digest, digestSz, sigRs, (int*)&sigRsSz, TPM_ALG_ECDSA, hashAlg); AssertIntEQ(rc, 0); @@ -483,7 +489,7 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, AssertIntEQ(rc, 0); /* Convert TPM key to wolfCrypt key for verification */ - rc = wolfTPM2_EccKey_TpmToWolf(dev, &eccKey, &wolfKey); + rc = wolfTPM2_EccKey_TpmToWolf(dev, (WOLFTPM2_KEY*)&eccKey, &wolfKey); AssertIntEQ(rc, 0); /* Verify TPM signature with wolfCrypt */ @@ -494,7 +500,9 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, /* Cleanup first wolfCrypt key */ wc_ecc_free(&wolfKey); wolfTPM2_UnloadHandle(dev, &eccKey.handle); - +#ifdef WOLF_CRYPTO_CB + tpmCtx.ecdsaKey = NULL; /* create new one */ +#endif /* -- Use wolfCrypt key to sign and verify with TPM -- */ /* Initialize new wolfCrypt ECC key */ @@ -516,12 +524,11 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, r = sigRs; s = &sigRs[MAX_ECC_BYTES]; rLen = sLen = MAX_ECC_BYTES; - rc = wc_ecc_sig_to_rs(sig, - sigSz, r, &rLen, s, &sLen); + rc = wc_ecc_sig_to_rs(sig, sigSz, r, &rLen, s, &sLen); AssertIntEQ(rc, 0); /* Convert wolfCrypt key to TPM key for verification */ - rc = wolfTPM2_EccKey_WolfToTpm(dev, &wolfKey, &eccKey); + rc = wolfTPM2_EccKey_WolfToTpm(dev, &wolfKey, (WOLFTPM2_KEY*)&eccKey); AssertIntEQ(rc, 0); /* combine R and S at key size (zero pad leading) */ @@ -531,8 +538,8 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, XMEMSET(&sigRs[curveSize], 0, curveSize-sLen); /* Verify wolfCrypt signature with TPM */ - rc = wolfTPM2_VerifyHashScheme(dev, &eccKey, sigRs, curveSize*2, - digest, digestSz, TPM_ALG_ECDSA, hashAlg); + rc = wolfTPM2_VerifyHashScheme(dev, (WOLFTPM2_KEY*)&eccKey, sigRs, + curveSize*2, digest, digestSz, TPM_ALG_ECDSA, hashAlg); AssertIntEQ(rc, 0); /* Cleanup */ @@ -551,7 +558,8 @@ static void test_wolfTPM2_EccSignVerifyDig(WOLFTPM2_DEV* dev, } } -static void test_wolfTPM2_EccSignVerify_All(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* storageKey, int flags) +static void test_wolfTPM2_EccSignVerify_All(WOLFTPM2_DEV* dev, + WOLFTPM2_KEY* storageKey, int flags) { int i; byte digest[TPM_MAX_DIGEST_SIZE];