Fix Fenrir PKCS#11 findings#189
Open
LinuxJedi wants to merge 3 commits into
Open
Conversation
Address the batch of Fenrir findings: - F-4260 (AES-CBC-PAD): guard the size-query underflow in C_Decrypt and enforce a minimum one-block ciphertext; add real output-capacity bounds checks inside WP11_AesCbcPad_DecryptUpdate/DecryptFinal that return BUFFER_E (mapped to CKR_BUFFER_TOO_SMALL) instead of overflowing the caller buffer. - F-3832 (GetEcPoint): size check now accounts for the full encoded length and reports the required size on BUFFER_E so callers can resize. - F-4314 (WP11_Object_Find): hold the token lock (and, on NSS, the slot lock in the established slot-then-token order) across the session object walk so a concurrent remove can't free a node mid-traversal. - F-4690 (WP11_Slot_CloseSession): re-validate the session is still linked in the slot under the RW lock before freeing, closing a double-close race. - F-4731 (WP11_Slot_CloseSessions): hoist the slot lock above the free loop. - F-4730 (WP11_Tls12_Master_Key_Derive): reject client+server random length sums that wrap or exceed word32. - F-4732 (WP11_Session_UpdateData / C_SignUpdate): guard the cumulative word32 length against wrap and reject out-of-range CK_ULONG part lengths. - F-4798/F-4799 (library init/final): serialize Init/Final/IsInitialized with a statically-initialized libraryInitLock so concurrent C_Initialize/C_Finalize can't race on globalLock's lifetime. - F-4313 (PKCS12 PBE key gen): reject password/salt/iteration lengths that don't fit the int-typed wc_PKCS12_PBKDF parameters. - F-4633/F-4634 (C_EncapsulateKey/C_DecapsulateKey): reject creation of a CKA_PRIVATE=TRUE object on a public, non-empty-PIN session. - F-4632 (C_InitToken): use WP11_Slot_SOLogin so the SO failed-login counter and lockout apply on this path.
Three build/test breakages surfaced by the wider CI matrix: - pkcs11v32 / mldsa / scan-build (V3.2 without PQC): CheckPrivateObjectLogin was defined under WOLFPKCS11_PKCS11_V3_2 but only called from the WOLFPKCS11_MLKEM-guarded C_EncapsulateKey/C_DecapsulateKey bodies, tripping -Werror=unused-function. Guard the definition with WOLFPKCS11_MLKEM to match its only call sites. - wolfssl_v5_6_6: libraryInitLock used WOLFSSL_MUTEX_INITIALIZER(lockname), but older wolfSSL exposes only the object-like WOLFSSL_MUTEX_INITIALIZER with no lockname argument, causing a parse error. Switch to the WOLFSSL_MUTEX_INITIALIZER_CLAUSE wrapper and gate on its presence so older wolfSSL falls back to the non-static-lock path instead of failing to build. - stm32h5-pkcs11-single-boot: C_InitToken used WP11_Slot_SOLogin to apply the SO failed-login lockout, but SOLogin also sets loginState and rejects open read-only sessions, which is wrong for InitToken and broke the secure-mode (SINGLE_THREADED, custom store, NO_TIME) provisioning flow. Add WP11_Slot_CheckSOPinLockout, which enforces the same lockout without logging in; under WOLFPKCS11_NO_TIME it collapses to a plain CheckSOPin, matching the historical behaviour exactly.
- F-4313 (PBKDF2 key gen): apply the same int-range guards used on the NSS PKCS#12 PBE path to the PBKDF2 branch. pwLen, ulSaltSourceDataLen, iterations and derivedKeyLen are all caller-controlled CK_ULONGs cast to the int parameters of WP11_PBKDF2, so reject values that would truncate to word32 or go negative on the cast. - F-4260 (AES-CBC-PAD decrypt): on a too-small output buffer the BUFFER_E paths now report the required size and leave the operation active, matching the PKCS#11 CKR_BUFFER_TOO_SMALL contract. WP11_AesCbcPad_DecryptFinal no longer frees the AES context / clears session->init on this path (that pre-empted any retry), WP11_AesCbcPad_DecryptUpdate writes the needed size into *decSz, the one-shot WP11_AesCbcPad_Decrypt propagates the required total, and C_Decrypt / C_DecryptUpdate / C_DecryptFinal write it back into the caller's length output before returning CKR_BUFFER_TOO_SMALL.
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.
Address the batch of Fenrir findings: