Repository navigation
Keep active operations intact on stray and single-part calls - #283
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the operation-state handling and AES-CBC-PAD retry changes.
No open issues 🎉
What changed in this PR
This PR tightens wolfPKCS11 operation-state handling and fixes AES-CBC-PAD short-buffer retries, addressing #275 and #277.
Changes:
- Preserve active operations when unrelated calls are refused, and reject single-part calls after multipart updates.
- Track update status in saved digest state and size CBC-PAD output without advancing the decryption state.
- Add regression coverage for operation lifecycle, restored digests, buffer retries, and cleanup.
| File | Description |
|---|---|
wolfpkcs11/internal.h |
Declare operation-update tracking helpers. |
tests/single_part_op_state_test.c |
Add operation-state and CBC-PAD retry regression tests. |
tests/session_op_lifecycle_test.c |
Check GCM buffer cleanup after a refused single-part call. |
tests/include.am |
Register and link the new regression test. |
src/internal.c |
Track and serialize update status; preserve CBC state during output sizing. |
src/crypto.c |
Enforce operation-kind and single-part checks; abort failed operations. |
Review effort: Balanced
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #283
Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Not reviewed (no scan target covers them): tests/include.am
Coverage: 4 of 5 in-scope changed file(s) opened by the reviewer; not opened: wolfpkcs11/internal.h
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
Frauschi
force-pushed
the
single-part-op-state
branch
from
November 6, 2026 10:02
08bbb7d to
c7a09a7
Compare
C_Sign, C_Verify, C_Decrypt, C_VerifyRecover and the Update and Final calls checked only that the session had a key, then switched on the mechanism of the last keyed init. After an operation of another kind, finished or still active, they hit the default case, returned CKR_MECHANISM_INVALID and, for sign and verify, aborted the operation that was actually active. Each call now returns CKR_OPERATION_NOT_INITIALIZED unless an operation of its own category is active. Verify-recover shares the verify category but has no Update or Final, so C_Verify, C_VerifyUpdate and C_VerifyFinal treat it as not initialized, and C_VerifyRecover refuses any other verify operation. A single-part call after an Update of the same operation either restarted it (C_Digest) or finished it (C_Encrypt, C_Decrypt, C_Sign, C_Verify). The session now records that an update ran, and the single-part call fails with CKR_FUNCTION_FAILED and ends the operation, as CKM_TLS_MAC already did, so the TLS MAC specific checks go. C_GetOperationState saves the flag in one extra byte and C_SetOperationState restores it, so C_Digest still finishes a state saved straight after C_DigestInit. A state from an earlier build has no such byte and counts as updated. session_op_lifecycle_test drove Update then C_Encrypt for AES-GCM; it now expects the refusal and checks that the abort scrubs the buffered data.
Single-part CKM_AES_CBC_PAD C_Decrypt ran update and final into the caller's buffer. When the leading blocks fit but the plaintext did not, it had already decrypted them, moved the IV and kept the last block before returning CKR_BUFFER_TOO_SMALL, so the retry decrypted on top of that state and returned CKR_OK with garbage. A smaller buffer was told the ciphertext length minus a block, which is less than the plaintext needs. WP11_AesCbcPad_Decrypt now sizes a buffer that may be short from the last block alone: it is decrypted with the previous ciphertext block, or the IV saved at init, as chaining value, the padding is checked and the IV is restored. The caller gets the exact length and an untouched operation; bad padding still ends it with CKR_ENCRYPTED_DATA_INVALID. The padding check moves to a helper shared with C_DecryptFinal. A failed C_Encrypt, C_Decrypt, C_Sign or C_Verify now aborts the operation, so state a helper did not release is freed and scrubbed rather than left behind with the operation cleared.
Frauschi
force-pushed
the
single-part-op-state
branch
from
November 6, 2026 11:35
c7a09a7 to
946d0e2
Compare
LinuxJedi
approved these changes
Nov 6, 2026
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.
A call refused for a missing init no longer tears down the session's active operation, a single-part call can no longer finish a multi-part one, and a
CKM_AES_CBC_PADC_Decryptretried afterCKR_BUFFER_TOO_SMALLnow returns the right plaintext.Fixes #275, #277.
Changes
CKR_OPERATION_NOT_INITIALIZEDunless an operation of their own kind is active; verify-recover counts only forC_VerifyRecoverC_Encrypt,C_Decrypt,C_Digest,C_SignandC_Verifyafter an Update of the same operation returnCKR_FUNCTION_FAILEDand end it, asCKM_TLS_MACalready didC_GetOperationStateadds one byte recording whether an update ran, so a restored digest is refused byC_Digestonly when one didTesting
tests/single_part_op_state_testcovers stray calls, verify-recover, single-part calls after an Update, restored digest states and CBC-PAD short-buffer retriessession_op_lifecycle_testexpects the refusedC_Encryptafter an Update and checks that the abort scrubs buffered GCM data