Use default OpenSSL provider for internal ML-DSA key reconstruction#881
Use default OpenSSL provider for internal ML-DSA key reconstruction#881olszomal wants to merge 1 commit into
Conversation
Signed-off-by: olszomal <Malgorzata.Olszowka@stunnel.org>
📝 WalkthroughWalkthroughBoth ML-DSA key classes now explicitly specify the default provider when constructing OpenSSL EVP_PKEY contexts, replacing NULL with ChangesML-DSA Provider Specification
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/lib/crypto/OSSLMLDSAPublicKey.cppsrc/lib/crypto/OSSLMLDSAPublicKey.cpp:7:10: fatal error: 'config.h' file not found ... [truncated 1082 characters] ... b/clang/18/include" src/lib/crypto/OSSLMLDSAPrivateKey.cppsrc/lib/crypto/OSSLMLDSAPrivateKey.cpp:7:10: fatal error: 'config.h' file not found ... [truncated 1085 characters] ... /clang/18/include" 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.
🧹 Nitpick comments (1)
src/lib/crypto/OSSLMLDSAPublicKey.cpp (1)
114-127: ⚡ Quick winConsider adding NULL check for
nameto match the private key implementation.
OSSLMLDSAPrivateKey::createOSSLKey()validates thatnameis non-NULL before proceeding (lines 185-189), but this function does not. IfmldsaParameterSet2Name()returns NULL for an unknown parameter set, passing NULL toEVP_PKEY_CTX_new_from_namemay produce unclear errors or undefined behavior.♻️ Suggested fix
const char* name = OSSL::mldsaParameterSet2Name(getParameterSet()); + if (name == NULL) + { + ERROR_MSG("Unknown ML-DSA parameter set (value length: %zu)", localValue.size()); + return; + } int selection = 0;🤖 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/OSSLMLDSAPublicKey.cpp` around lines 114 - 127, The code calls mldsaParameterSet2Name(getParameterSet()) into the variable name and immediately passes it to EVP_PKEY_CTX_new_from_name without verifying it is non-NULL; add the same NULL-check used in OSSLMLDSAPrivateKey::createOSSLKey(): if name is NULL, log/return an error (or clean up and return failure) before calling EVP_PKEY_CTX_new_from_name, ensuring any allocated resources (e.g., params/ctx) are handled consistently and avoid passing NULL into EVP_PKEY_CTX_new_from_name.
🤖 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.
Nitpick comments:
In `@src/lib/crypto/OSSLMLDSAPublicKey.cpp`:
- Around line 114-127: The code calls mldsaParameterSet2Name(getParameterSet())
into the variable name and immediately passes it to EVP_PKEY_CTX_new_from_name
without verifying it is non-NULL; add the same NULL-check used in
OSSLMLDSAPrivateKey::createOSSLKey(): if name is NULL, log/return an error (or
clean up and return failure) before calling EVP_PKEY_CTX_new_from_name, ensuring
any allocated resources (e.g., params/ctx) are handled consistently and avoid
passing NULL into EVP_PKEY_CTX_new_from_name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d167bef-7d86-4f10-afb6-9d0ff51313f8
📒 Files selected for processing (2)
src/lib/crypto/OSSLMLDSAPrivateKey.cppsrc/lib/crypto/OSSLMLDSAPublicKey.cpp
1 participant
Current Behavior:
Relying on implicit provider selection can make
ML-DSApublic/private key reconstruction fail, causing operations such asC_Signto returnCKR_GENERAL_ERROR.Scope of Changes:
This change explicitly uses
provider=defaultwhen creating internalML-DSAkey contexts.Testing:
Verified
ML-DSAsigning with SoftHSM, OpenSSL 3.6.2, and a development branch of thepkcs11provprovider.Before this change, signing failed because SoftHSM could not obtain the internal OpenSSL private key. After the change, signing succeeds.
Related Pull Requests:
#823, #862, #874
Summary by CodeRabbit