Add wc_SignCert_cb API for external signing callbacks

[jackctj117]

This pull request adds support for signing certificates and CSRs using a user-provided callback function, enabling integration with external signing devices (such as TPMs or HSMs) without relying on the crypto callback infrastructure. This is particularly useful for FIPS-compliant applications and scenarios where offloading cryptographic operations is required. The changes include new API definitions, documentation, internal implementation, and tests for the callback-based signing mechanism.

New Callback-Based Certificate Signing API

• Introduced the wc_SignCert_cb function and the wc_SignCertCb callback type, allowing certificates and CSRs to be signed via an external callback for flexible integration with devices like TPMs/HSMs. [1] [2] [3]

Internal Implementation

• Added the internal MakeSignatureCb function to handle hashing, digest encoding, and invoking the user-provided signing callback, supporting both RSA and ECC key types.

Testing
Setup:
TPM simulator: swtpm running on port 2321
Built wolfSSL with: --enable-certgen --enable-certreq --enable-certext --enable-cryptocb
Built wolfTPM with: --enable-swtpm --enable-certgen --enable-debug
Tests Run:
Generated RSA and ECC test keys in TPM
Created CSRs using ./examples/csr/csr
Validated CSRs with openssl req -text -noout
Results:
wc_SignCert_cb compiled into wolfSSL
wolfTPM2_SignCertCb and CSR_MakeAndSign_Cb compiled into wolfTPM
Generated valid RSA (1228 bytes) and ECC (696 bytes) CSRs
CSRs verified successfully with OpenSSL

[dgarske]

Jenkins retest this please. History lost.

[Copilot]

Pull request overview

This pull request adds support for signing certificates and CSRs using a user-provided callback function, enabling integration with external signing devices (TPMs/HSMs) without relying on the crypto callback infrastructure. This is particularly useful for FIPS-compliant applications where offloading cryptographic operations is not acceptable.

Changes:

• Introduced new wc_SignCert_cb API and wc_SignCertCb callback type for external certificate/CSR signing
• Refactored internal MakeSignature function to use new MakeSignatureCb internally for RSA and ECC, eliminating code duplication
• Added configure option --enable-certsigncb to enable the feature

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
wolfssl/wolfcrypt/asn_public.h	Added public API declarations for the callback-based certificate signing, including typedef for wc_SignCertCb and function declaration for wc_SignCert_cb
wolfcrypt/src/asn.c	Implemented internal MakeSignatureCb function and refactored MakeSignature to use callback path for RSA/ECC; added wc_SignCert_cb implementation
tests/api.c	Added test case test_wc_SignCert_cb with mock callback to verify the new API functionality
configure.ac	Added configuration option --enable-certsigncb to control compilation of the new feature

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jackctj117]

Jenkins retest this

[jackctj117]

Jenkins retest this please.

[dgarske]

[check-source-text] [2 of 7] [wolfssl]
    autogen.sh wolfssl...   real 0m17.507s  user 0m15.557s  sys 0m0.446s
    configure...   real 0m12.829s  user 0m10.049s  sys 0m2.680s
trailing whitespace:
./wolfcrypt/src/asn.c:32089:/* Make signature from buffer (sz), write to sig (sigSz)·
./wolfcrypt/src/asn.c:32100:····
./wolfcrypt/src/asn.c:32119:········
./wolfcrypt/src/asn.c:32125:········
./wolfcrypt/src/asn.c:32137:········
C++-style comments:
./wolfssl/wolfcrypt/asn_public.h:269:        // Perform signing using external device/HSM
./wolfssl/wolfcrypt/asn_public.h:601:    // Initialize cert and set subject, etc.
./wolfssl/wolfcrypt/asn_public.h:603:    // ... set cert fields ...
./wolfssl/wolfcrypt/asn_public.h:605:    // Make certificate body
./wolfssl/wolfcrypt/asn_public.h:608:    // Sign using callback
unescaped error code operands (missing WC_NO_ERR_TRACE()):
wolfcrypt/src/asn.c:32016:    int ret = ALGO_ID_E;

[lealem47]

Jenkins retest this please.

[lealem47]

Thanks Jack!

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test function test_wc_SignCert_cb only tests with ECC keys (CTC_SHA256wECDSA). Since the new API explicitly supports both RSA and ECC key types, and the mockSignCb callback implementation includes RSA support (lines 20011-20023), there should be test coverage for RSA as well to ensure the RSA code path works correctly. Consider adding an RSA test case to verify that RSA signing through the callback works properly.

[dgarske]

Jenkins retest this please

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated 11 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Potential memory leak in wc_SignCert_cb when WOLFSSL_ASYNC_CRYPT is enabled. When WC_PENDING_E is returned at line 34452, the function returns without freeing certSignCtx->sig (correctly, as noted in the comment). However, there is no mechanism to track this allocation across async calls, and no way for the caller to free this memory later.

In the SignCert function (lines 34103-34175), the certSignCtx is stored in the key structure (rsaKey->certSignCtx or eccKey->certSignCtx) when WOLFSSL_ASYNC_CRYPT is enabled, allowing the allocation to be tracked across async calls. But in wc_SignCert_cb, certSignCtx is a local variable that goes out of scope, so the allocated certSignCtx->sig will be lost.

Either wc_SignCert_cb needs a way to persist the certSignCtx (similar to how SignCert uses the key's embedded certSignCtx), or the API needs to document that it cannot be used with async crypto operations.

[Copilot]

MakeSignatureCb doesn't check for or handle WC_PENDING_E return value from the user callback (signCb). At lines 34061-34070, the callback is invoked but the return value is only checked for success (ret == 0) or error (ret < 0), without any special handling for WC_PENDING_E.

If a user's callback returns WC_PENDING_E (e.g., when using async hardware), the function will not properly handle the pending state. It should check for WC_PENDING_E and return it without cleaning up state or resetting certSignCtx->state, similar to how the original MakeSignature function handled async operations.

The original implementation commented "set next state, since WC_PENDING_E reentry for these are not 'call again'" (formerly around the HashForSignature call), which suggests careful consideration was given to async state transitions. This logic needs to be preserved when the callback returns WC_PENDING_E.

[Copilot]

The documentation for wc_SignCert_cb does not mention support limitations for key types. The implementation in MakeSignatureCb (lines 34003-34098) only handles RSA_TYPE and ECC_TYPE, but the documentation at lines 261 and 253 says "keyType: Type of key used for signing (RSA_TYPE, ECC_TYPE, etc.)" with "etc." implying support for other types.

However, the function will silently fail or behave incorrectly for other key types like ED25519_TYPE, ED448_TYPE, or post-quantum types, since MakeSignatureCb only has conditional logic for RSA (line 34038) and falls through to ECC for everything else (line 34066-34070).

The documentation should explicitly state which key types are supported (RSA_TYPE and ECC_TYPE only) and explain that other algorithms like Ed25519, Ed448, and post-quantum schemes cannot use this callback-based API because they sign messages directly rather than hashes.

[Copilot]

Limited error case testing for wc_SignCert_cb. The tests only verify:

1. NULL callback returns BAD_FUNC_ARG (lines 20101-20102, 20160-20161)

Missing error case tests include:

1. NULL buf parameter
2. Invalid keyType (e.g., unsupported key type like ED25519_TYPE)
3. Buffer too small (buffSz smaller than needed)
4. Negative requestSz
5. Callback returning an error
6. Callback returning insufficient signature data in outLen

These additional tests would help ensure the API handles error conditions correctly and returns appropriate error codes to callers.

[Copilot]

The refactored MakeSignature function no longer handles state management for async operations (CertSignCtx state machine), which is inconsistent with the original implementation. The original MakeSignature had a state machine (CERTSIGN_STATE_BEGIN, CERTSIGN_STATE_DIGEST, CERTSIGN_STATE_ENCODE, CERTSIGN_STATE_DO) that was critical for async crypto operations.

When WOLFSSL_ASYNC_CRYPT is enabled, the CertSignCtx state needs to be preserved across calls when WC_PENDING_E is returned. The new implementation delegates to MakeSignatureCb, which does have state management, but MakeSignature itself no longer handles state transitions properly for RSA/ECC cases.

The original MakeSignature code handled the full state machine in the main function body, allowing it to return WC_PENDING_E at different stages. Now the RSA/ECC path jumps straight to MakeSignatureCb and exits, which may break async operations if they are in progress.

[Copilot]

Insufficient parameter validation in wc_SignCert_cb. The function validates that signCb and buf are not NULL (line 34426), but doesn't validate other critical parameters:

1. buffSz is never checked - if buffSz is 0 or very small, buffer overflow could occur when calling AddSignature at line 34461.
2. keyType is never validated - invalid/unsupported key types will silently be treated as ECC in MakeSignatureCb (line 34066-34070), potentially causing incorrect signature generation.
3. signCtx is allowed to be NULL without validation, but the callback might expect it to be non-NULL (though this may be intentional for stateless callbacks).

The function should validate these parameters and return BAD_FUNC_ARG if they are invalid. For consistency, look at wc_SignCert_ex (line 34340) which doesn't validate keyType either, but that function has explicit casting logic for each supported key type.

[Copilot]

MakeSignatureCb always resets the certSignCtx state and frees memory at exit_ms (lines 34078-34091), even when an error occurs in the middle of processing. This is problematic for async operations when combined with wc_SignCert_cb.

When using WOLFSSL_ASYNC_CRYPT, if the callback returns WC_PENDING_E, the function should preserve the state (not reset it to CERTSIGN_STATE_BEGIN) and not free the digest/encSig buffers, as they will be needed when the operation resumes. Currently, any error path (including WC_PENDING_E) will cause state reset and memory deallocation.

The cleanup at exit_ms should be conditional: only reset state and free memory if the operation completed (successfully or with a final error), not if it's pending. The original MakeSignature function structure avoided this by only having the cleanup code outside the main logic, but here it's in a shared exit path.

[Copilot]

The buffer size check in wc_SignCert_cb (line 34457) may be insufficient. The check requestSz + MAX_SEQ_SZ * 2 + sigSz > (int)buffSz is an approximation that may underestimate the actual space needed.

Looking at AddSignature (line 32368), it adds:

1. Algorithm identifier (SetAlgoID)
2. Bit string header (SetBitString)
3. Signature data
4. Outer sequence header (SetSequence)

The check uses MAX_SEQ_SZ * 2 to account for headers, but AddSignature also calls SetAlgoID which can vary in size depending on the algorithm. For some algorithms, the OID encoding plus any parameters could exceed the space allocated by this estimate.

A more accurate approach would be to either:

1. Call AddSignature with a NULL buffer first to get the exact size needed (as the function supports this), or
2. Use a more conservative buffer size estimate that accounts for algorithm-specific overhead

Compare this to SignCert (line 34162) which uses the same check, suggesting this is an existing pattern in the codebase, but it may still be worth documenting the buffer size requirements more clearly.

[Copilot]

The test for wc_SignCert_cb doesn't verify the signature is valid or that the certificate structure is correctly formed. The test only checks that the function returns a positive value (lines 20094-20098 for ECC and 20153-20157 for RSA) but doesn't:

1. Parse and verify the signed certificate structure
2. Validate the signature against the certificate body
3. Check that the signature algorithm matches expectations
4. Verify the certificate can be parsed by wc_ParseCert

Consider adding verification similar to test_MakeCertWithCaFalse (lines 19977-19979) which uses wc_InitDecodedCert, wc_ParseCert, and validates the parsed certificate contents. This would provide stronger assurance that the callback-based signing produces correct certificates.

[Copilot]

Potential use of uninitialized variable in MakeSignatureCb. The variable digestSz is declared at line 34007 but is only set by HashForSignature at line 34027. However, if the state machine enters at CERTSIGN_STATE_ENCODE (line 34035) or CERTSIGN_STATE_DO (line 34053) on a retry (e.g., after WC_PENDING_E from HashForSignature), the digestSz variable will be uninitialized but is used at line 34068.

This is a problem because:

1. The state machine supports re-entry at any state
2. When re-entering at CERTSIGN_STATE_ENCODE or CERTSIGN_STATE_DO, digestSz is not recalculated
3. The ECC signing path at line 34068 uses digestSz: signCb(certSignCtx->digest, (word32)digestSz, ...)

The digestSz should either be:

1. Stored in certSignCtx structure to persist across calls, or
2. Recalculated from the digest buffer size

Looking at the original code structure, this wasn't an issue because the original MakeSignature didn't store digestSz in the context, but it also didn't have separate states for each phase in the same way.