C_WrapKey: end the encrypt op after a length query - #220
Open
MarkAtwood wants to merge 1 commit into
Open
MarkAtwood wants to merge 1 commit into
MarkAtwood wants to merge 1 commit into
Conversation
C_WrapKey with an AES mechanism runs C_EncryptInit and C_Encrypt internally. A length query (pWrappedKey NULL) or a buffer that is too small leaves that encrypt operation active, so the caller's next C_WrapKey fails with CKR_OPERATION_ACTIVE. NSS queries the length first when exporting a key, so pk12util -o failed. C_WrapKey is single-part; end the operation after the internal C_Encrypt. Add tests/wrapkey_length_query_test.c, with an OpenSSL known answer.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new test needs a serialization-support guard to avoid failing the no-keystore CI configuration.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Fixes C_WrapKey retries after length queries or short buffers, supporting NSS PKCS #12 exports.
Changes:
- Clears the internal encryption operation after wrapping.
- Adds a known-answer regression test and registers it with Automake.
| File | Description |
|---|---|
| tests/wrapkey_length_query_test.c | Tests length-query and short-buffer retries. |
| tests/include.am | Registers and links the regression test. |
| src/crypto.c | Clears encryption state after internal C_Encrypt. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| #include "testdata.h" | ||
|
|
||
| #if !defined(NO_AES) && !defined(NO_AES_CBC) && defined(WOLFSSL_AES_256) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
C_WrapKey with a NULL output buffer is a length query. wolfPKCS11 returns the length but leaves the internal encrypt operation active, so the real call that follows fails with CKR_OPERATION_ACTIVE. Same after CKR_BUFFER_TOO_SMALL. That breaks
pk12util -othrough NSS.This ends the encrypt operation after the internal C_Encrypt, so a length query or a short buffer leaves no operation active.
Test:
wrapkey_length_query_test, checked against an OpenSSL known answer. Fails on master, passes with the fix.