Support RSA PSS for import in softhsm2-util #816
Conversation
WalkthroughAdds recognition of OpenSSL Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as softhsm2-util
participant OSSL as OpenSSL EVP
participant Token as Token Store
User->>CLI: softhsm2-util --import <key.pem>
CLI->>OSSL: parse PEM -> EVP_PKEY*
OSSL-->>CLI: EVP_PKEY*
CLI->>CLI: switch(EVP_PKEY_base_id)
alt RSA or RSA‑PSS
note right of CLI `#lightblue`: RSA‑PSS now mapped to RSA path
CLI->>OSSL: EVP_PKEY_get1_RSA()
OSSL-->>CLI: RSA*
CLI->>CLI: build PKCS#11 key objects
else DSA/ECC/EDDSA/...
CLI->>CLI: existing handlers (unchanged)
end
CLI->>Token: store key pair and attributes
Token-->>CLI: OK / Error
CLI-->>User: result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-09-11T16:54:00.370ZApplied to files:
📚 Learning: 2025-10-13T09:42:31.270ZApplied to files:
📚 Learning: 2025-10-13T09:43:45.885ZApplied to files:
📚 Learning: 2025-11-11T19:42:46.886ZApplied to files:
📚 Learning: 2025-10-27T16:44:57.865ZApplied to files:
📚 Learning: 2025-08-11T16:42:23.012ZApplied to files:
📚 Learning: 2025-08-18T19:46:46.659ZApplied to files:
🪛 Cppcheck (2.18.0)src/bin/util/softhsm2-util-ossl.cpp[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches. (normalCheckLevelMaxBranches) ⏰ 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)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/bin/util/softhsm2-util-ossl.cpp (4)
156-164: Add compile-time guard for EVP_PKEY_RSA_PSS to preserve older OpenSSL builds.Some downstreams still build against older OpenSSL where EVP_PKEY_RSA_PSS may be undefined. Guard it to avoid compile errors.
Apply:
- case EVP_PKEY_RSA: - case EVP_PKEY_RSA_PSS: + case EVP_PKEY_RSA: +#ifdef EVP_PKEY_RSA_PSS + case EVP_PKEY_RSA_PSS: +#endif rsa = EVP_PKEY_get1_RSA(pkey); break;
156-156: Prefer EVP_PKEY_base_id(pkey) over EVP_PKEY_type(EVP_PKEY_id(pkey)).This simplifies the logic and is robust across OpenSSL variants/providers; it directly returns the base type (e.g., RSA for RSA-PSS).
Apply:
- switch (EVP_PKEY_type(EVP_PKEY_id(pkey))) + switch (EVP_PKEY_base_id(pkey))(Keeping the RSA_PSS case is harmless but becomes redundant.)
189-193: RSA-PSS semantics: consider restricting key usage on import.Mapping RSA-PSS to generic RSA sets CKA_DECRYPT/UNWRAP/ENCRYPT/WRAP to TRUE in crypto_save_rsa(), which may allow non-PSS operations (e.g., RSAES-PKCS1-v1_5). For PSS-only keys, consider:
- setting CKA_ENCRYPT/WRAP/DECRYPT/UNWRAP to FALSE, and
- optionally setting CKA_ALLOWED_MECHANISMS to PSS-only mechanisms.
Given the PR’s minimal scope, this can be a follow-up issue.
I can draft a follow-up PR that toggles these attributes when the source key was RSA_PSS and, if supported, sets CKA_ALLOWED_MECHANISMS.
179-183: Improve diagnostics for unsupported algorithms.Include the algorithm name to aid debugging (e.g., OBJ_nid2sn(EVP_PKEY_id(pkey)) or EVP_PKEY_get0_type_name in OpenSSL 3).
Example:
- fprintf(stderr, "ERROR: Cannot handle this algorithm.\n"); + const char* alg = +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + EVP_PKEY_get0_type_name(pkey); +#else + OBJ_nid2sn(EVP_PKEY_id(pkey)); +#endif + fprintf(stderr, "ERROR: Cannot handle this algorithm: %s.\n", alg ? alg : "unknown");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/bin/util/softhsm2-util-ossl.cpp(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bukka
PR: softhsm/SoftHSMv2#815
File: src/lib/SoftHSM.cpp:6543-6546
Timestamp: 2025-09-11T16:54:00.347Z
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.
📚 Learning: 2025-09-11T16:54:00.347Z
Learnt from: bukka
PR: softhsm/SoftHSMv2#815
File: src/lib/SoftHSM.cpp:6543-6546
Timestamp: 2025-09-11T16:54:00.347Z
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/bin/util/softhsm2-util-ossl.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 (x86, openssl)
- GitHub Check: Linux with OpenSSL 1.1.1
- GitHub Check: Linux with OpenSSL 3.0
- GitHub Check: Windows (x64, openssl)
- GitHub Check: Windows (x64, botan)
- GitHub Check: macOS (openssl)
- GitHub Check: macOS (botan)
- GitHub Check: Linux with Botan
🔇 Additional comments (2)
src/bin/util/softhsm2-util-ossl.cpp (2)
158-161: LGTM: RSA-PSS now recognized and routed through the RSA import path.This unblocks importing RSA-PSS PKCS#8 keys and matches the minimal scope of the PR.
156-164: Run RSA‑PSS import smoke test locally — softhsm2-util not available here.Couldn't run the provided script: softhsm2-util: command not found. Please run the original 3-step check on a machine with softhsm2 installed and confirm it exits 0 and prints "The key pair has been imported."
bukka
force-pushed
the
util-import-rsa-pss
branch
from
15a4860 to
7944e7d
Compare
1 participant
This should fix #721 . The type is differnt from RSA PSS so it needs to be added there.
I tested the change manual and the import went through after this. I don't see any tests for util so it comes wihtout a test.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.