-
Notifications
You must be signed in to change notification settings - Fork 922
Add PKCS7 ECC raw sign callback support #9656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
🛟 Devin Lifeguard found 2 likely issues in this PR
@jackctj117 |
dgarske
left a comment
There was a problem hiding this 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.
There was a problem hiding this 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.
|
Jenkins retest please |
|
Jenkins retest this please |
23f4659 to
8e2e6d1
Compare
|
Jenkins retest this |
|
Jenkins retest this please |
1 similar comment
|
Jenkins retest this please |
dgarske
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also review the rsaSignRawDigestCb for same.
|
Jenkins retest this please |
2eae560 to
6d6d0ab
Compare
|
Jenkins retest this please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* privateKey may be NULL in HSM/secure element use case - we load it | ||
| * independently in this callback to simulate that scenario */ | ||
| (void)privateKey; | ||
| (void)privateKeySz; | ||
| (void)hashOID; | ||
|
|
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ECC raw-sign example callback comment says it assumes SHA-256, but the implementation explicitly ignores hashOID (and doesn’t validate digestSz for SHA-256). Either validate hashOID == SHA256h (and expected digest size) to match the documented assumption, or update the comment to indicate the callback is hash-agnostic.
| /* user signing plain digest */ | ||
| ret = pkcs7->eccSignRawDigestCb(pkcs7, | ||
| esd->contentAttribsDigest, hashSz, | ||
| esd->encContentDigest, sizeof(esd->encContentDigest), | ||
| pkcs7->privateKey, pkcs7->privateKeySz, pkcs7->devId, | ||
| eccHashOID); | ||
| break; | ||
| } |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ECC raw-sign callback return value is used as esd->encContentDigestSz without validating it against the provided output buffer size (sizeof(esd->encContentDigest)). If the callback mistakenly returns a size larger than outSz, later encoding will read past encContentDigest. Add a bounds check after the callback (e.g., treat ret > outSz as BUFFER_E) before accepting the size.
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:
CallbackEccSignRawDigestfor ECC raw digest signing, and a corresponding struct membereccSignRawDigestCbtowc_PKCS7to allow user-supplied signing logic. [1] [2]wc_PKCS7_SetEccSignRawDigestCbto register the ECC signing callback with a PKCS7 context. [1] [2]Testing Enhancements:
eccSignRawDigestCb) and corresponding test logic intest_pkcs7.cto verify the callback mechanism when ECC is enabled and RSA is disabled. [1] [2]