Skip to content

Fix incorrect call to isMechanismPermitted#835

Merged
bukka merged 1 commit intosofthsm:mainfrom
bukka:asym-encrypt-mechanism-permitted-build
Jan 10, 2026
Merged

Fix incorrect call to isMechanismPermitted#835
bukka merged 1 commit intosofthsm:mainfrom
bukka:asym-encrypt-mechanism-permitted-build

Conversation

@bukka
Copy link
Copy Markdown
Member

@bukka bukka commented Jan 10, 2026

This fixes build failure introduced in f3c0156

Summary by CodeRabbit

  • Bug Fixes
    • Fixed incorrect mechanism validation in asymmetric encryption initialization, ensuring proper parameter handling for cryptographic operations.

✏️ Tip: You can customize this high-level summary in your review settings.

@bukka bukka requested a review from a team as a code owner January 10, 2026 10:12
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 10, 2026

📝 Walkthrough

Walkthrough

A single-line correction in AsymEncryptInit modifies the isMechanismPermitted function call to pass the mechanism type (pMechanism->mechanism) directly instead of the full mechanism pointer, aligning the parameter with the function's expected signature.

Changes

Cohort / File(s) Summary
Mechanism validation correction
src/lib/SoftHSM.cpp
Changed isMechanismPermitted call parameter from CK_MECHANISM_PTR (full pointer) to CK_MECHANISM_TYPE (mechanism field only) in AsymEncryptInit

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Poem

🐰 A pointer was passed, but type didn't match,
Just the mechanism type is all that we catch,
One tiny fix makes the signature right,
Now AsymEncryptInit validates just right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and clearly describes the main change: fixing an incorrect function call to isMechanismPermitted with the correct parameter type.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f3c0156 and 304eaf2.

📒 Files selected for processing (1)
  • src/lib/SoftHSM.cpp
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: bukka
Repo: softhsm/SoftHSMv2 PR: 818
File: src/lib/SoftHSM.cpp:3238-3266
Timestamp: 2025-10-13T09:42:31.270Z
Learning: In src/lib/SoftHSM.cpp, SoftHSM::AsymDecryptInit should use MechParamCheckRSAPKCSOAEP for CKM_RSA_PKCS_OAEP to keep OAEP parameter validation identical to AsymEncryptInit, and avoid redundant manual checks for pMechanism->pParameter and MGF allowlists.
Learnt from: bukka
Repo: softhsm/SoftHSMv2 PR: 815
File: src/lib/SoftHSM.cpp:6543-6546
Timestamp: 2025-09-11T16:54:00.370Z
Learning: SoftHSM PR #815 (pkcs11.h 3.1.0 update, CK_RSA_AES_KEY_WRAP_PARAMS fix): Maintainer bukka prefers to keep scope minimal (header sync and param rename only); broader OAEP parameter enforcement or backend plumbing is out of scope for this PR and should be handled in a follow-up issue.
Learnt from: antoinelochet
Repo: softhsm/SoftHSMv2 PR: 809
File: src/lib/SoftHSM.cpp:5550-5596
Timestamp: 2025-10-27T16:44:57.865Z
Learning: SoftHSM ML‑DSA additional context: avoid transient malloc in SoftHSM::{AsymSignInit, AsymVerifyInit}; let Session::setParameters deep‑copy SIGN_ADDITIONAL_CONTEXT by allocating one buffer for struct + context bytes and fixing the stored pointer; Session::resetOp frees m_pParameters so both struct and context are released.
📚 Learning: 2025-10-13T09:42:31.270Z
Learnt from: bukka
Repo: softhsm/SoftHSMv2 PR: 818
File: src/lib/SoftHSM.cpp:3238-3266
Timestamp: 2025-10-13T09:42:31.270Z
Learning: In src/lib/SoftHSM.cpp, SoftHSM::AsymDecryptInit should use MechParamCheckRSAPKCSOAEP for CKM_RSA_PKCS_OAEP to keep OAEP parameter validation identical to AsymEncryptInit, and avoid redundant manual checks for pMechanism->pParameter and MGF allowlists.

Applied to files:

  • src/lib/SoftHSM.cpp
📚 Learning: 2025-09-11T16:54:00.370Z
Learnt from: bukka
Repo: softhsm/SoftHSMv2 PR: 815
File: src/lib/SoftHSM.cpp:6543-6546
Timestamp: 2025-09-11T16:54:00.370Z
Learning: SoftHSM PR #815 (pkcs11.h 3.1.0 update, CK_RSA_AES_KEY_WRAP_PARAMS fix): Maintainer bukka prefers to keep scope minimal (header sync and param rename only); broader OAEP parameter enforcement or backend plumbing is out of scope for this PR and should be handled in a follow-up issue.

Applied to files:

  • src/lib/SoftHSM.cpp
📚 Learning: 2025-11-11T19:42:46.886Z
Learnt from: JoaoHenrique12
Repo: softhsm/SoftHSMv2 PR: 823
File: src/lib/SoftHSM.cpp:58-58
Timestamp: 2025-11-11T19:42:46.886Z
Learning: SoftHSM include policy: Algorithm key/parameter headers (e.g., SLHParameters, SLH*Key) are included unconditionally in SoftHSM.cpp/SoftHSM.h; feature gating is handled in mechanism tables and CryptoFactory (WITH_*). Avoid wrapping such includes in WITH_* guards.

Applied to files:

  • src/lib/SoftHSM.cpp
📚 Learning: 2025-10-27T16:44:57.865Z
Learnt from: antoinelochet
Repo: softhsm/SoftHSMv2 PR: 809
File: src/lib/SoftHSM.cpp:5550-5596
Timestamp: 2025-10-27T16:44:57.865Z
Learning: SoftHSM ML‑DSA additional context: avoid transient malloc in SoftHSM::{AsymSignInit, AsymVerifyInit}; let Session::setParameters deep‑copy SIGN_ADDITIONAL_CONTEXT by allocating one buffer for struct + context bytes and fixing the stored pointer; Session::resetOp frees m_pParameters so both struct and context are released.

Applied to files:

  • src/lib/SoftHSM.cpp
📚 Learning: 2025-08-18T19:46:46.659Z
Learnt from: antoinelochet
Repo: softhsm/SoftHSMv2 PR: 809
File: src/lib/SoftHSM.cpp:10044-10055
Timestamp: 2025-08-18T19:46:46.659Z
Learning: In SoftHSM::generateMLDSA (src/lib/SoftHSM.cpp), the project prefers using CKP_ML_DSA_* constants for parameter set checks and logs "Unsupported parameter set: %lu". The code currently returns CKR_PARAMETER_SET_NOT_SUPPORTED for unsupported sets.

Applied to files:

  • src/lib/SoftHSM.cpp
📚 Learning: 2025-08-18T17:59:30.693Z
Learnt from: antoinelochet
Repo: softhsm/SoftHSMv2 PR: 809
File: src/lib/crypto/MLDSAParameters.h:42-44
Timestamp: 2025-08-18T17:59:30.693Z
Learning: In MLDSAParameters.h, the author prefers to keep PKCS#11 constant references (CKP_ML_DSA_*) rather than explicit numeric values for better code clarity and maintainability.

Applied to files:

  • src/lib/SoftHSM.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Windows (x64, openssl)
  • GitHub Check: Linux with OpenSSL 3.0
  • GitHub Check: Windows (x64, botan)
  • GitHub Check: Windows (x86, openssl)
  • GitHub Check: Linux with Botan
  • GitHub Check: macOS (openssl)
  • GitHub Check: macOS (botan)
  • GitHub Check: Linux with OpenSSL 1.1.1
🔇 Additional comments (1)
src/lib/SoftHSM.cpp (1)

2457-2459: Correct argument type passed to isMechanismPermitted (fixes mechanism validation).
This now matches SoftHSM::isMechanismPermitted(OSObject*, CK_MECHANISM_TYPE) and is consistent with other call sites.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bukka
Copy link
Copy Markdown
Member Author

bukka commented Jan 10, 2026

This is just a correction of the previous merge so merging to fix quickly build.

@bukka bukka merged commit eba7ac6 into softhsm:main Jan 10, 2026
9 checks passed
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.

1 participant