Skip to content

Commit d774825

Browse files
jackctj117jackctj117
committed
Address copilot feedback
1 parent cb2b7ad commit d774825

File tree

5 files changed

+180
-19
lines changed

5 files changed

+180
-19
lines changed

‎doc/dox_comments/header_files/asn.h‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,18 +253,29 @@ int wc_DhPublicKeyDecode(const byte* input, word32* inOutIdx, DhKey* key,
253253
3. Calls the user-provided callback to perform the actual signing
254254
4. Encodes the signature into the certificate/CSR DER structure
255255
256+
NOTE: Only RSA and ECC key types are supported. Ed25519, Ed448, and
257+
post-quantum algorithms (Falcon, Dilithium, SPHINCS+) sign messages
258+
directly rather than hashes, so they cannot use this callback-based API.
259+
Use wc_SignCert_ex for those algorithms.
260+
261+
NOTE: This function does NOT support async crypto (WOLFSSL_ASYNC_CRYPT).
262+
The internal context is local to this function and cannot persist across
263+
async re-entry.
264+
256265
\param requestSz Size of the certificate body to sign (from Cert.bodySz).
257266
\param sType Signature algorithm type (e.g., CTC_SHA256wRSA,
258267
CTC_SHA256wECDSA).
259268
\param buf Buffer containing the certificate/CSR DER data to sign.
260269
\param buffSz Total size of the buffer (must be large enough for signature).
261-
\param keyType Type of key used for signing (RSA_TYPE, ECC_TYPE, etc.).
270+
\param keyType Type of key used for signing. Only RSA_TYPE and ECC_TYPE
271+
are supported.
262272
\param signCb User-provided signing callback function.
263273
\param signCtx Context pointer passed to the signing callback.
264274
\param rng Random number generator (may be NULL if not needed).
265275
266276
\return Size of the signed certificate/CSR on success.
267-
\return BAD_FUNC_ARG if signCb is NULL or other parameters are invalid.
277+
\return BAD_FUNC_ARG if signCb or buf is NULL, buffSz is 0, or keyType
278+
is not RSA_TYPE or ECC_TYPE.
268279
\return BUFFER_E if the buffer is too small for the signed certificate.
269280
\return MEMORY_E if memory allocation fails.
270281
\return Negative error code on other failures.

‎tests/api.c‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20041,6 +20041,20 @@ static int mockSignCb(const byte* in, word32 inLen, byte* out, word32* outLen,
2004120041

2004220042
return ret;
2004320043
}
20044+
20045+
/* Mock callback that always returns an error for testing */
20046+
static int mockSignCbError(const byte* in, word32 inLen, byte* out,
20047+
word32* outLen, int sigAlgo, int keyType, void* ctx)
20048+
{
20049+
(void)in;
20050+
(void)inLen;
20051+
(void)out;
20052+
(void)outLen;
20053+
(void)sigAlgo;
20054+
(void)keyType;
20055+
(void)ctx;
20056+
return BAD_STATE_E; /* Return an error */
20057+
}
2004420058
#endif
2004520059

2004620060
#ifdef WOLFSSL_CERT_SIGN_CB
@@ -20058,12 +20072,14 @@ static int test_wc_SignCert_cb(void)
2005820072
WC_RNG rng;
2005920073
ecc_key key;
2006020074
MockSignCtx signCtx;
20075+
DecodedCert decodedCert;
2006120076
int ret;
2006220077

2006320078
XMEMSET(&rng, 0, sizeof(WC_RNG));
2006420079
XMEMSET(&key, 0, sizeof(ecc_key));
2006520080
XMEMSET(&cert, 0, sizeof(Cert));
2006620081
XMEMSET(&signCtx, 0, sizeof(MockSignCtx));
20082+
XMEMSET(&decodedCert, 0, sizeof(DecodedCert));
2006720083

2006820084
ExpectIntEQ(wc_InitRng(&rng), 0);
2006920085
ExpectIntEQ(wc_ecc_init(&key), 0);
@@ -20097,9 +20113,37 @@ static int test_wc_SignCert_cb(void)
2009720113
/* Verify the certificate was created properly */
2009820114
ExpectIntGT(derSize, 0);
2009920115

20116+
/* Parse the certificate and verify it's well-formed */
20117+
if (EXPECT_SUCCESS()) {
20118+
wc_InitDecodedCert(&decodedCert, der, (word32)derSize, NULL);
20119+
ExpectIntEQ(wc_ParseCert(&decodedCert, CERT_TYPE, NO_VERIFY, NULL),
20120+
0);
20121+
/* Verify signature algorithm matches what we set */
20122+
ExpectIntEQ(decodedCert.signatureOID, CTC_SHA256wECDSA);
20123+
wc_FreeDecodedCert(&decodedCert);
20124+
}
20125+
2010020126
/* Test error cases */
20127+
/* NULL callback */
2010120128
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
2010220129
FOURK_BUF, ECC_TYPE, NULL, &signCtx, &rng), BAD_FUNC_ARG);
20130+
/* NULL buffer */
20131+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, NULL,
20132+
FOURK_BUF, ECC_TYPE, mockSignCb, &signCtx, &rng), BAD_FUNC_ARG);
20133+
/* Zero buffer size */
20134+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
20135+
0, ECC_TYPE, mockSignCb, &signCtx, &rng), BAD_FUNC_ARG);
20136+
/* Negative requestSz - should return the negative value */
20137+
ExpectIntLT(wc_SignCert_cb(-1, cert.sigType, der,
20138+
FOURK_BUF, ECC_TYPE, mockSignCb, &signCtx, &rng), 0);
20139+
/* Callback returning error */
20140+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
20141+
FOURK_BUF, ECC_TYPE, mockSignCbError, &signCtx, &rng), BAD_STATE_E);
20142+
#ifndef NO_RSA
20143+
/* Invalid keyType for ECC signature */
20144+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
20145+
FOURK_BUF, ED25519_TYPE, mockSignCb, &signCtx, &rng), BAD_FUNC_ARG);
20146+
#endif
2010320147

2010420148
ret = wc_ecc_free(&key);
2010520149
ExpectIntEQ(ret, 0);
@@ -20117,12 +20161,14 @@ static int test_wc_SignCert_cb(void)
2011720161
WC_RNG rng;
2011820162
RsaKey key;
2011920163
MockSignCtx signCtx;
20164+
DecodedCert decodedCert;
2012020165
int ret;
2012120166

2012220167
XMEMSET(&rng, 0, sizeof(WC_RNG));
2012320168
XMEMSET(&key, 0, sizeof(RsaKey));
2012420169
XMEMSET(&cert, 0, sizeof(Cert));
2012520170
XMEMSET(&signCtx, 0, sizeof(MockSignCtx));
20171+
XMEMSET(&decodedCert, 0, sizeof(DecodedCert));
2012620172

2012720173
ExpectIntEQ(wc_InitRng(&rng), 0);
2012820174
ExpectIntEQ(wc_InitRsaKey(&key, NULL), 0);
@@ -20156,9 +20202,34 @@ static int test_wc_SignCert_cb(void)
2015620202
/* Verify the certificate was created properly */
2015720203
ExpectIntGT(derSize, 0);
2015820204

20159-
/* Test error case - NULL callback */
20205+
/* Parse the certificate and verify it's well-formed */
20206+
if (EXPECT_SUCCESS()) {
20207+
wc_InitDecodedCert(&decodedCert, der, (word32)derSize, NULL);
20208+
ExpectIntEQ(wc_ParseCert(&decodedCert, CERT_TYPE, NO_VERIFY, NULL),
20209+
0);
20210+
/* Verify signature algorithm matches what we set */
20211+
ExpectIntEQ(decodedCert.signatureOID, CTC_SHA256wRSA);
20212+
wc_FreeDecodedCert(&decodedCert);
20213+
}
20214+
20215+
/* Test error cases */
20216+
/* NULL callback */
2016020217
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
2016120218
FOURK_BUF, RSA_TYPE, NULL, &signCtx, &rng), BAD_FUNC_ARG);
20219+
/* NULL buffer */
20220+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, NULL,
20221+
FOURK_BUF, RSA_TYPE, mockSignCb, &signCtx, &rng), BAD_FUNC_ARG);
20222+
/* Zero buffer size */
20223+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
20224+
0, RSA_TYPE, mockSignCb, &signCtx, &rng), BAD_FUNC_ARG);
20225+
/* Callback returning error */
20226+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
20227+
FOURK_BUF, RSA_TYPE, mockSignCbError, &signCtx, &rng), BAD_STATE_E);
20228+
#ifdef HAVE_ECC
20229+
/* Invalid keyType */
20230+
ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der,
20231+
FOURK_BUF, ED448_TYPE, mockSignCb, &signCtx, &rng), BAD_FUNC_ARG);
20232+
#endif
2016220233

2016320234
ret = wc_FreeRsaKey(&key);
2016420235
ExpectIntEQ(ret, 0);

‎wolfcrypt/src/asn.c‎

Lines changed: 92 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32112,46 +32112,46 @@ static int InternalSignCb(const byte* in, word32 inLen,
3211232112
ret = 0;
3211332113
}
3211432114
}
32115+
else
3211532116
#endif /* !NO_RSA && !WOLFSSL_RSA_PUBLIC_ONLY && !WOLFSSL_RSA_VERIFY_ONLY */
32116-
3211732117
#if defined(HAVE_ECC) && defined(HAVE_ECC_SIGN)
3211832118
if (keyType == ECC_TYPE && signCtx->key) {
3211932119
/* For ECC, input is the raw hash */
3212032120
ret = wc_ecc_sign_hash(in, inLen, out, outLen,
3212132121
signCtx->rng, (ecc_key*)signCtx->key);
3212232122
}
32123+
else
3212332124
#endif /* HAVE_ECC && HAVE_ECC_SIGN */
32124-
3212532125
#if defined(HAVE_ED25519) && defined(HAVE_ED25519_SIGN)
3212632126
if (keyType == ED25519_TYPE && signCtx->key) {
3212732127
/* Ed25519 signs messages, not hashes - cannot use callback path */
3212832128
ret = SIG_TYPE_E;
3212932129
}
32130+
else
3213032131
#endif /* HAVE_ED25519 && HAVE_ED25519_SIGN */
32131-
3213232132
#if defined(HAVE_ED448) && defined(HAVE_ED448_SIGN)
3213332133
if (keyType == ED448_TYPE && signCtx->key) {
3213432134
/* Ed448 signs messages, not hashes - cannot use callback path */
3213532135
ret = SIG_TYPE_E;
3213632136
}
32137+
else
3213732138
#endif /* HAVE_ED448 && HAVE_ED448_SIGN */
32138-
3213932139
#if defined(HAVE_FALCON)
3214032140
if ((keyType == FALCON_LEVEL1_TYPE || keyType == FALCON_LEVEL5_TYPE) &&
3214132141
signCtx->key) {
3214232142
/* Falcon signs messages, not hashes - cannot use callback path */
3214332143
ret = SIG_TYPE_E;
3214432144
}
32145+
else
3214532146
#endif /* HAVE_FALCON */
32146-
3214732147
#if defined(HAVE_DILITHIUM) && !defined(WOLFSSL_DILITHIUM_NO_SIGN)
3214832148
if ((keyType == DILITHIUM_LEVEL2_TYPE || keyType == DILITHIUM_LEVEL3_TYPE ||
3214932149
keyType == DILITHIUM_LEVEL5_TYPE) && signCtx->key) {
3215032150
/* Dilithium signs messages, not hashes - cannot use callback path */
3215132151
ret = SIG_TYPE_E;
3215232152
}
32153+
else
3215332154
#endif /* HAVE_DILITHIUM && !WOLFSSL_DILITHIUM_NO_SIGN */
32154-
3215532155
#if defined(HAVE_SPHINCS)
3215632156
if ((keyType == SPHINCS_FAST_LEVEL1_TYPE || keyType == SPHINCS_FAST_LEVEL3_TYPE ||
3215732157
keyType == SPHINCS_FAST_LEVEL5_TYPE || keyType == SPHINCS_SMALL_LEVEL1_TYPE ||
@@ -32160,7 +32160,17 @@ static int InternalSignCb(const byte* in, word32 inLen,
3216032160
/* Sphincs signs messages, not hashes - cannot use callback path */
3216132161
ret = SIG_TYPE_E;
3216232162
}
32163+
else
3216332164
#endif /* HAVE_SPHINCS */
32165+
{
32166+
/* Unhandled key type */
32167+
(void)in;
32168+
(void)inLen;
32169+
(void)out;
32170+
(void)outLen;
32171+
(void)keyType;
32172+
(void)signCtx;
32173+
}
3216432174

3216532175
return ret;
3216632176
}
@@ -34004,14 +34014,34 @@ static int MakeSignatureCb(CertSignCtx* certSignCtx, const byte* buf,
3400434014
word32 sz, byte* sig, word32 sigSz, int sigAlgoType, int keyType,
3400534015
wc_SignCertCb signCb, void* signCtx, WC_RNG* rng, void* heap)
3400634016
{
34007-
int digestSz = 0, typeH = 0, ret = 0;
34017+
int ret = 0;
3400834018
word32 outLen;
3400934019

3401034020
(void)rng;
3401134021
#ifdef WOLFSSL_NO_MALLOC
3401234022
(void)heap;
3401334023
#endif
3401434024

34025+
/* Validate keyType - only RSA and ECC are supported for callback signing.
34026+
* Ed25519, Ed448, and post-quantum algorithms sign messages directly,
34027+
* not hashes, so they cannot use the callback path. */
34028+
#if !defined(NO_RSA) && defined(HAVE_ECC)
34029+
if (keyType != RSA_TYPE && keyType != ECC_TYPE) {
34030+
return BAD_FUNC_ARG;
34031+
}
34032+
#elif !defined(NO_RSA)
34033+
if (keyType != RSA_TYPE) {
34034+
return BAD_FUNC_ARG;
34035+
}
34036+
#elif defined(HAVE_ECC)
34037+
if (keyType != ECC_TYPE) {
34038+
return BAD_FUNC_ARG;
34039+
}
34040+
#else
34041+
(void)keyType;
34042+
return NOT_COMPILED_IN;
34043+
#endif
34044+
3401534045
switch (certSignCtx->state) {
3401634046
case CERTSIGN_STATE_BEGIN:
3401734047
case CERTSIGN_STATE_DIGEST:
@@ -34025,7 +34055,8 @@ static int MakeSignatureCb(CertSignCtx* certSignCtx, const byte* buf,
3402534055
}
3402634056
#endif
3402734057
ret = HashForSignature(buf, sz, (word32)sigAlgoType, certSignCtx->digest,
34028-
&typeH, &digestSz, 0, NULL, INVALID_DEVID);
34058+
&certSignCtx->typeH, &certSignCtx->digestSz, 0,
34059+
NULL, INVALID_DEVID);
3402934060
certSignCtx->state = CERTSIGN_STATE_ENCODE;
3403034061
if (ret != 0) {
3403134062
goto exit_ms;
@@ -34044,8 +34075,10 @@ static int MakeSignatureCb(CertSignCtx* certSignCtx, const byte* buf,
3404434075
goto exit_ms;
3404534076
}
3404634077
#endif
34078+
/* typeH was stored in certSignCtx by HashForSignature */
3404734079
certSignCtx->encSigSz = (int)wc_EncodeSignature(certSignCtx->encSig,
34048-
certSignCtx->digest, (word32)digestSz, typeH);
34080+
certSignCtx->digest, (word32)certSignCtx->digestSz,
34081+
certSignCtx->typeH);
3404934082
}
3405034083
#endif /* !NO_RSA */
3405134084
FALL_THROUGH;
@@ -34065,10 +34098,17 @@ static int MakeSignatureCb(CertSignCtx* certSignCtx, const byte* buf,
3406534098
#endif /* !NO_RSA */
3406634099
{
3406734100
/* ECC: pass raw hash */
34068-
ret = signCb(certSignCtx->digest, (word32)digestSz,
34101+
ret = signCb(certSignCtx->digest, (word32)certSignCtx->digestSz,
3406934102
sig, &outLen, sigAlgoType, keyType, signCtx);
3407034103
}
3407134104

34105+
#ifdef WOLFSSL_ASYNC_CRYPT
34106+
/* If callback returns WC_PENDING_E, preserve state for re-entry */
34107+
if (ret == WC_NO_ERR_TRACE(WC_PENDING_E)) {
34108+
return ret;
34109+
}
34110+
#endif
34111+
3407234112
if (ret == 0) {
3407334113
ret = (int)outLen;
3407434114
}
@@ -34089,6 +34129,8 @@ static int MakeSignatureCb(CertSignCtx* certSignCtx, const byte* buf,
3408934129

3409034130
/* reset state */
3409134131
certSignCtx->state = CERTSIGN_STATE_BEGIN;
34132+
certSignCtx->digestSz = 0;
34133+
certSignCtx->typeH = 0;
3409234134

3409334135
if (ret < 0) {
3409434136
WOLFSSL_ERROR_VERBOSE(ret);
@@ -34402,16 +34444,22 @@ int wc_SignCert(int requestSz, int sType, byte* buf, word32 buffSz,
3440234444
* This allows external signing implementations (e.g., TPM, HSM)
3440334445
* without requiring the crypto callback infrastructure.
3440434446
*
34447+
* NOTE: This function does NOT support async crypto (WOLFSSL_ASYNC_CRYPT).
34448+
* The certSignCtx is local to this function and cannot persist across
34449+
* async re-entry. Use wc_SignCert or wc_SignCert_ex for async operations.
34450+
*
3440534451
* @param [in] requestSz Size of certificate body to sign.
3440634452
* @param [in] sType The signature type.
3440734453
* @param [in,out] buf Der buffer to sign.
3440834454
* @param [in] buffSz Der buffer size.
34409-
* @param [in] keyType The type of key.
34455+
* @param [in] keyType The type of key (RSA_TYPE or ECC_TYPE only).
3441034456
* @param [in] signCb User signing callback.
3441134457
* @param [in] signCtx Context passed to callback.
3441234458
* @param [in] rng Random number generator (may be NULL).
3441334459
*
3441434460
* @return Size of signature on success.
34461+
* @return BAD_FUNC_ARG if signCb or buf is NULL, buffSz is 0, or invalid
34462+
* keyType.
3441534463
* @return < 0 on error
3441634464
*/
3441734465
#ifdef WOLFSSL_CERT_SIGN_CB
@@ -34423,10 +34471,29 @@ int wc_SignCert_cb(int requestSz, int sType, byte* buf, word32 buffSz,
3442334471
CertSignCtx certSignCtx_lcl;
3442434472
CertSignCtx* certSignCtx = &certSignCtx_lcl;
3442534473

34426-
if (signCb == NULL || buf == NULL) {
34474+
/* Validate parameters */
34475+
if (signCb == NULL || buf == NULL || buffSz == 0) {
3442734476
return BAD_FUNC_ARG;
3442834477
}
3442934478

34479+
/* Validate keyType - only RSA and ECC supported */
34480+
#if !defined(NO_RSA) && defined(HAVE_ECC)
34481+
if (keyType != RSA_TYPE && keyType != ECC_TYPE) {
34482+
return BAD_FUNC_ARG;
34483+
}
34484+
#elif !defined(NO_RSA)
34485+
if (keyType != RSA_TYPE) {
34486+
return BAD_FUNC_ARG;
34487+
}
34488+
#elif defined(HAVE_ECC)
34489+
if (keyType != ECC_TYPE) {
34490+
return BAD_FUNC_ARG;
34491+
}
34492+
#else
34493+
(void)keyType;
34494+
return NOT_COMPILED_IN;
34495+
#endif
34496+
3443034497
XMEMSET(certSignCtx, 0, sizeof(*certSignCtx));
3443134498

3443234499
if (requestSz < 0) {
@@ -34447,13 +34514,23 @@ int wc_SignCert_cb(int requestSz, int sType, byte* buf, word32 buffSz,
3444734514

3444834515
#ifdef WOLFSSL_ASYNC_CRYPT
3444934516
if (sigSz == WC_NO_ERR_TRACE(WC_PENDING_E)) {
34450-
/* Not free'ing certSignCtx->sig here because it could still be in use
34451-
* with async operations. */
34452-
return sigSz;
34517+
/* Async crypto not supported with wc_SignCert_cb because certSignCtx
34518+
* is local and cannot persist across re-entry. Clean up and return
34519+
* error. */
34520+
#ifndef WOLFSSL_NO_MALLOC
34521+
XFREE(certSignCtx->sig, NULL, DYNAMIC_TYPE_TMP_BUFFER);
34522+
certSignCtx->sig = NULL;
34523+
#endif
34524+
return NOT_COMPILED_IN;
3445334525
}
3445434526
#endif
3445534527

3445634528
if (sigSz >= 0) {
34529+
/* Check buffer has room for signature structure. This is an estimate
34530+
* using MAX_SEQ_SZ * 2 to account for sequence headers and algorithm
34531+
* identifier overhead. For precise sizing, call AddSignature with
34532+
* NULL buffer first, but this estimate matches the existing pattern
34533+
* used in SignCert. */
3445734534
if (requestSz + MAX_SEQ_SZ * 2 + sigSz > (int)buffSz) {
3445834535
sigSz = BUFFER_E;
3445934536
}

0 commit comments

Comments
 (0)