Skip to content

Conversation

@jackctj117
Copy link
Contributor

This pull request adds support for registering and using a user-defined callback for ECC raw digest signing in PKCS7 operations, similar to existing RSA support. The changes include new callback types, struct members, API functions, and corresponding test coverage.

ECC Raw Digest Signing Callback Support:

  • Added a new callback type CallbackEccSignRawDigest for ECC raw digest signing, and a corresponding struct member eccSignRawDigestCb to wc_PKCS7 to allow user-supplied signing logic. [1] [2]
  • Introduced the public API function wc_PKCS7_SetEccSignRawDigestCb to register the ECC signing callback with a PKCS7 context. [1] [2]
  • Modified the PKCS7 signature building logic to invoke the ECC raw digest signing callback if set, enabling custom or hardware-backed ECC signing.

Testing Enhancements:

  • Added an example ECC sign raw digest callback implementation (eccSignRawDigestCb) and corresponding test logic in test_pkcs7.c to verify the callback mechanism when ECC is enabled and RSA is disabled. [1] [2]

@jackctj117 jackctj117 self-assigned this Jan 13, 2026
@jackctj117 jackctj117 marked this pull request as draft January 13, 2026 21:24
@jackctj117 jackctj117 marked this pull request as ready for review January 14, 2026 16:14
@devin-ai-integration
Copy link
Contributor

🛟 Devin Lifeguard found 2 likely issues in this PR

  • check-all-return-codes snippet: Check eccHashOID after wc_HashGetOID(esd->hashType) and, if it is negative, set ret to eccHashOID and break/return before invoking the callback.
  • limit-stack-usage snippet: Apply the WOLFSSL_SMALL_STACK pattern: replace the on-stack declaration “ecc_key ecc;” with a pointer allocated via XMALLOC in combination with wc_ecc_init_ex (e.g., “ecc_key* ecc = (ecc_key*)XMALLOC(sizeof(ecc_key), pkcs7->heap, DYNAMIC_TYPE_ECC);”) and free it with XFREE to keep stack usage below 100 bytes.

@jackctj117
please take a look at the above issues which Devin flagged. Devin will not fix these issues automatically.

@wolfSSL wolfSSL deleted a comment from devin-ai-integration bot Jan 14, 2026
@dgarske dgarske self-requested a review January 14, 2026 16:58
@dgarske dgarske requested review from lealem47 and removed request for wolfSSL-Bot January 14, 2026 16:58
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrecognized macros used:
HAVE_PKCS7_ECC_RAW_SIGN_CALLBACK
add well-formed but unknown macros to .wolfssl_known_macro_extras at top of source tree.

@dgarske dgarske removed their assignment Jan 14, 2026
#endif

#if defined(HAVE_PKCS7) && defined(HAVE_PKCS7_ECC_RAW_SIGN_CALLBACK) && \
defined(HAVE_ECC) && defined(NO_RSA) && !defined(NO_SHA256)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need the NO_RSA check here.

tests/api/test_pkcs7.c:457:12: error: unused function 'eccSignRawDigestCb' [-Werror,-Wunused-function]
  457 | static int eccSignRawDigestCb(PKCS7* pkcs7, byte* digest, word32 digestSz,
      |            ^~~~~~~~~~~~~~~~~~
1 error generated.
  CC       tests/api/unit_test-test_ossl_sk.o

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of the code looks great

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds support for ECC raw digest signing callbacks in PKCS7 operations, mirroring the existing RSA callback functionality. The implementation allows users to provide custom ECC signing logic, which is useful for hardware-backed or alternative signing implementations.

Changes:

  • Added ECC raw digest signing callback infrastructure (typedef, struct member, and API function)
  • Modified PKCS7 signature building logic to invoke the ECC callback when configured
  • Added comprehensive test coverage with example callback implementation

Reviewed changes

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

File Description
wolfssl/wolfcrypt/pkcs7.h Added ECC callback typedef, struct member, and API declaration
wolfcrypt/src/pkcs7.c Implemented callback invocation in signing logic and setter function
tests/api/test_pkcs7.c Added example callback and test case for ECC signing
.wolfssl_known_macro_extras Added HAVE_PKCS7_ECC_RAW_SIGN_CALLBACK macro definition

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

Comment on lines +357 to +359
#if defined(HAVE_PKCS7_ECC_RAW_SIGN_CALLBACK) && defined(HAVE_ECC)
CallbackEccSignRawDigest eccSignRawDigestCb;
#endif
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new struct member eccSignRawDigestCb should be added at the end of the wc_PKCS7 struct, after the aesKeyWrapUnwrapCb member (around line 389), not here. The comment at line 391 states "!! NEW DATA MEMBERS MUST BE ADDED AT END !!" to maintain backwards compatibility. Placing it in the middle of the struct could break ABI compatibility with existing code.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants