Add legacy RSA fallback for Engine-based applications#882
Conversation
Use the default provider for RSA key reconstruction with OpenSSL 3.x. If the provider-based RSA key creation path cannot be initialized, fall back to the legacy RSA API for Engine-based applications. Treat EVP_PKEY_fromdata_init() and EVP_PKEY_fromdata() failures as key reconstruction errors. Signed-off-by: olszomal <[email protected]>
📝 WalkthroughWalkthroughBoth RSA key files update reconstruction logic for OpenSSL 3.x+ compatibility. They introduce provider-based context methods to rebuild ChangesOpenSSL 3.x+ RSA Key Reconstruction Compatibility
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/crypto/OSSLRSAPrivateKey.cpp (1)
385-427:⚠️ Potential issue | 🟠 MajorRestrict legacy RSA fallback to provider-context/init failures only.
- In
src/lib/crypto/OSSLRSAPrivateKey.cppandsrc/lib/crypto/OSSLRSAPublicKey.cpp, whenOPENSSL_VERSION_NUMBER < 0x40000000L, the code clears the error queue and proceeds to the legacy RSA builder for any provider-based reconstruction failure (includingOSSL_PARAM_BLD_to_param()/params == NULLandEVP_PKEY_fromdata()failures, even afterEVP_PKEY_fromdata_init()succeeded).- Split the failure handling so only
EVP_PKEY_CTX_new_from_name(...)/EVP_PKEY_fromdata_init(...)failures trigger the legacy fallback; keepEVP_PKEY_fromdata(...)and param-building failures as hard reconstruction errors (log and return withrsa == NULL) to avoid masking malformed/inconsistent RSA components.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/crypto/OSSLRSAPrivateKey.cpp` around lines 385 - 427, The current logic falls back to the legacy RSA API for any provider-path failure; change it so only failures to obtain/init the provider context trigger the legacy fallback: check the results of EVP_PKEY_CTX_new_from_name(...) and EVP_PKEY_fromdata_init(ctx) first and if either fails then clear errors and jump to the existing legacy fallback path; treat OSSL_PARAM_BLD_to_param()/params == NULL and EVP_PKEY_fromdata(ctx, &rsa, ...) failures as hard errors — log ("Could not create RSA key object" or similar), free bn_* and params/ctx as needed, leave rsa NULL and return immediately (no legacy fallback). Update the same pattern in both OSSLRSAPrivateKey.cpp and OSSLRSAPublicKey.cpp, referencing EVP_PKEY_CTX_new_from_name, EVP_PKEY_fromdata_init, OSSL_PARAM_BLD_to_param, params, EVP_PKEY_fromdata and rsa to locate the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/crypto/OSSLRSAPrivateKey.cpp`:
- Around line 385-427: The current logic falls back to the legacy RSA API for
any provider-path failure; change it so only failures to obtain/init the
provider context trigger the legacy fallback: check the results of
EVP_PKEY_CTX_new_from_name(...) and EVP_PKEY_fromdata_init(ctx) first and if
either fails then clear errors and jump to the existing legacy fallback path;
treat OSSL_PARAM_BLD_to_param()/params == NULL and EVP_PKEY_fromdata(ctx, &rsa,
...) failures as hard errors — log ("Could not create RSA key object" or
similar), free bn_* and params/ctx as needed, leave rsa NULL and return
immediately (no legacy fallback). Update the same pattern in both
OSSLRSAPrivateKey.cpp and OSSLRSAPublicKey.cpp, referencing
EVP_PKEY_CTX_new_from_name, EVP_PKEY_fromdata_init, OSSL_PARAM_BLD_to_param,
params, EVP_PKEY_fromdata and rsa to locate the code to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f363725-b39b-4a37-b442-4980e248fcae
📒 Files selected for processing (2)
src/lib/crypto/OSSLRSAPrivateKey.cppsrc/lib/crypto/OSSLRSAPublicKey.cpp
Current Behavior:
With OpenSSL 3.x, RSA key reconstruction uses the provider-based EVP_PKEY_fromdata() path.
In Engine-based applications, a suitable provider context may not always be available for this operation. In that case, EVP_PKEY_fromdata_init() can fail, which prevents SoftHSM from reconstructing the internal RSA key object.
Scope of Changes:
This change explicitly uses the OpenSSL default provider for internal RSA key reconstruction.
If the provider-based RSA reconstruction cannot be initialized, the code falls back to the legacy RSA API while it is still available. This fallback is intended for Engine-based applications and preserves compatibility with OpenSSL 3.x.
The fallback is only used when the provider key creation context cannot be initialized. Failures from EVP_PKEY_fromdata() itself are still treated as RSA key reconstruction errors.
Testing:
Verified RSA signing with SoftHSM, OpenSSL 3.6.2, and the libp11 Engine.
Before this change, RSA signing through the Engine failed because SoftHSM could not initialize the provider-based RSA key reconstruction path. After this change, SoftHSM falls back to the legacy RSA API and signing succeeds.
Summary by CodeRabbit