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 <Malgorzata.Olszowka@stunnel.org>
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
|
I don't really like this as we we would apply this kind of logic to all algs - note that this is likely already broken for DH. I'm not keen on this sort of deprecation suppression as it's quite ugly hack. What I would be willing to accept is to completely separate old api to the new backend that would be primarily for LibreSSL could support 1.1.1 and legacy engine code in 3.x. So if you want to go with this, you will need to copy all algs so we will have some duplicate code for a while but it would make the separation easier when we migrate other algs. So I think that's the path. |
2 participants
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