Fixed EDDSA context/pre-hash support#892
Conversation
📝 WalkthroughWalkthroughAdds EdDSA "context data" and pre-hash (Ed25519ph/Ed448ph) support via a new ChangesEdDSA Context and Pre-hash Support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant SoftHSM
participant Backend as BotanEDDSA/OSSLEDDSA
Application->>SoftHSM: C_SignInit(CKM_EDDSA, CK_EDDSA_PARAMS)
SoftHSM->>SoftHSM: validate pParameter/ulParameterLen, parse phFlag/contextData
SoftHSM->>Backend: signInit(mechanismParam)
Application->>SoftHSM: C_Sign(data)
SoftHSM->>Backend: sign(data, mechanismParam)
Backend->>Backend: parse EDDSAMechanismParam(preHash, contextData)
alt preHash true
Backend->>Backend: select Ed25519ph/Ed448ph
else contextData non-empty
Backend-->>SoftHSM: error (context requires pre-hash)
end
Backend-->>SoftHSM: signature
SoftHSM-->>Application: signature
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
antoinelochet
force-pushed
the
bugfix/issue_873
branch
4 times, most recently
from
ddf46b5 to
569b85d
Compare
antoinelochet
force-pushed
the
bugfix/issue_873
branch
from
569b85d to
a3cf255
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/lib/crypto/BotanEDDSA.cpp (1)
124-131: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRemove the unreachable
Ed448phfallbackBotanEDDSA.cpponly acceptsBotan::Ed25519_{Private,Public}Key, and bothBotanEDPrivateKey::getOrderLength()andBotanEDPublicKey::getOrderLength()return32, so this branch can never run. The same pattern exists inverify(), soEd25519phis the only pre-hash EMSA used here.🤖 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/BotanEDDSA.cpp` around lines 124 - 131, Remove the unreachable Ed448ph fallback in BotanEDDSA by keeping Ed25519ph as the only EMSA chosen from pk->getOrderLength() in both the signing and verify paths. Since BotanEDDSA only works with Botan::Ed25519_{Private,Public}Key and BotanEDPrivateKey::getOrderLength()/BotanEDPublicKey::getOrderLength() always return 32, update the logic in the relevant functions to eliminate the else branch and any duplicate Ed448ph handling.src/lib/crypto/EDDSAMechanismParam.h (1)
20-41: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePass
contextDataby const reference.
EDDSAMechanismParam(bool flag, ByteString contextData)takescontextDataby value, causing an extra copy on every call;const ByteString&avoids it.♻️ Proposed fix
- EDDSAMechanismParam(bool flag, ByteString contextData); + EDDSAMechanismParam(bool flag, const ByteString& contextData);🤖 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/EDDSAMechanismParam.h` around lines 20 - 41, EDDSAMechanismParam’s constructor currently copies contextData unnecessarily by taking it by value. Update the EDDSAMechanismParam(bool flag, ByteString contextData) declaration and corresponding definition to take contextData as a const ByteString& instead, and keep the member initialization in EDDSAMechanismParam consistent with that change.src/lib/SoftHSM.cpp (1)
4609-4622: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing upper-bound check on EdDSA context length.
Per RFC 8032, EdDSA context strings must be at most 255 octets, yet
ulContextDataLenis accepted unbounded here. The siblingCKM_ML_DSAcase in this same file validatesulContextLen > 255; consider applying the same guard forCKM_EDDSA.🛡️ Proposed fix
CK_EDDSA_PARAMS* ckEddsaParams = (CK_EDDSA_PARAMS*) pMechanism->pParameter; eddsaParam.flag = (ckEddsaParams->phFlag != 0x00); if (ckEddsaParams->ulContextDataLen > 0) { if (ckEddsaParams->pContextData == NULL_PTR) { ERROR_MSG("Invalid parameters"); return CKR_ARGUMENTS_BAD; } + if (ckEddsaParams->ulContextDataLen > 255) + { + ERROR_MSG("Invalid parameters"); + return CKR_ARGUMENTS_BAD; + } eddsaParam.contextData = ByteString(ckEddsaParams->pContextData, ckEddsaParams->ulContextDataLen);Also applies to: 5730-5743
🤖 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/SoftHSM.cpp` around lines 4609 - 4622, The CKM_EDDSA handling in SoftHSM::signMechanism accepts an unbounded context length, so add an upper-bound check before building eddsaParam from CK_EDDSA_PARAMS. Validate that ulContextDataLen is not greater than 255, mirroring the CKM_ML_DSA guard in this file, and return CKR_ARGUMENTS_BAD with an error message when the limit is exceeded; keep the existing NULL_PTR check and ByteString assignment unchanged.
🤖 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.
Inline comments:
In `@src/lib/crypto/BotanEDDSA.cpp`:
- Around line 101-132: The Botan EDDSA path is ignoring caller-provided
contextData when preHash is true, which can produce signatures that do not match
context-aware verification. In BotanEDDSA::sign or the mechanism-parameter
handling block, either explicitly reject any non-empty eddsaParam->contextData
for both preHash and non-preHash cases, or wire it through only if Botan
supports it; do not silently discard it. Also remove the unused local ByteString
contextData or assign and validate it consistently alongside
EDDSAMechanismParam.
In `@src/lib/crypto/OSSLEDDSA.cpp`:
- Around line 115-138: The instance selection in OSSLEDDSA::sign is dropping
valid Ed448 context-only requests and misclassifying Ed25519 when both context
and pre-hash are requested. Update the branching on pk->getOrderLength() so
Ed448 sets the native Ed448 instance whenever contextData is present (not only
when preHash is true), and ensure the Ed25519 path explicitly handles the ph+ctx
combination instead of letting contextData.size() win first. Apply the same
selection rules in OSSLEDDSA::verify so signing and verification stay aligned.
- Around line 161-170: The OpenSSL error logging in OSSLEDDSA.cpp is fetching
the error queue twice per message, so the numeric code and text can mismatch.
Update the sign and verify error paths in the EDDSA helpers to call
ERR_get_error() once, store the result in a local variable, and use that same
value for both the formatted code and ERR_error_string() output. Apply the same
fix in the EVP_DigestSignInit, EVP_DigestSign, and corresponding verification
error logs.
- Around line 140-158: The EdDSA instance/context parameters are being applied
before EVP_DigestSignInit/EVP_DigestVerifyInit, so EVP_MD_CTX_get_pkey_ctx(ctx)
can be NULL and the params never reach OpenSSL. Update OSSLEDDSA.cpp to move the
OSSL_PARAM setup in the EdDSA sign/verify flow to the pctx obtained after init,
or switch to the _ex APIs, and ensure the same logic is used in both the signing
and verification paths so Ed25519ctx/ph and Ed448ph are actually selected.
- Around line 147-151: The `instance` parameter is being built with the wrong
OpenSSL param constructor in `OSSLEDDSA` because it is a `const char *`, not a
writable string buffer. Update the `OSSL_PARAM` construction in the signing path
and the matching verify path in `OSSLEDDSA` to use `OSSL_PARAM_utf8_ptr` for
`OSSL_SIGNATURE_PARAM_INSTANCE`, passing the pointer variable itself and the
correct string length, while leaving the context string param unchanged.
In `@src/lib/test/SignVerifyTests.cpp`:
- Around line 847-1029: The new EdDSA context tests in
testEdSignVerifyWithContext and testEdSignVerifyWithContextPreHashed only cover
successful same-context verification, so add negative assertions that a
signature created with one CK_EDDSA_PARAMS cannot be verified with a different
context or with the other phFlag mode. Reuse the existing signVerifySingle flow
or the lower-level sign/verify helpers around generateED and CK_EDDSA_PARAMS to
produce one signature and then verify it with mismatched parameters for each key
pair type, ensuring the test explicitly checks cross-context and
plain-vs-prehash failures.
---
Nitpick comments:
In `@src/lib/crypto/BotanEDDSA.cpp`:
- Around line 124-131: Remove the unreachable Ed448ph fallback in BotanEDDSA by
keeping Ed25519ph as the only EMSA chosen from pk->getOrderLength() in both the
signing and verify paths. Since BotanEDDSA only works with
Botan::Ed25519_{Private,Public}Key and
BotanEDPrivateKey::getOrderLength()/BotanEDPublicKey::getOrderLength() always
return 32, update the logic in the relevant functions to eliminate the else
branch and any duplicate Ed448ph handling.
In `@src/lib/crypto/EDDSAMechanismParam.h`:
- Around line 20-41: EDDSAMechanismParam’s constructor currently copies
contextData unnecessarily by taking it by value. Update the
EDDSAMechanismParam(bool flag, ByteString contextData) declaration and
corresponding definition to take contextData as a const ByteString& instead, and
keep the member initialization in EDDSAMechanismParam consistent with that
change.
In `@src/lib/SoftHSM.cpp`:
- Around line 4609-4622: The CKM_EDDSA handling in SoftHSM::signMechanism
accepts an unbounded context length, so add an upper-bound check before building
eddsaParam from CK_EDDSA_PARAMS. Validate that ulContextDataLen is not greater
than 255, mirroring the CKM_ML_DSA guard in this file, and return
CKR_ARGUMENTS_BAD with an error message when the limit is exceeded; keep the
existing NULL_PTR check and ByteString assignment unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7e7c86e-e220-428d-ade4-244efe719345
📒 Files selected for processing (9)
src/lib/SoftHSM.cppsrc/lib/crypto/BotanEDDSA.cppsrc/lib/crypto/CMakeLists.txtsrc/lib/crypto/EDDSAMechanismParam.cppsrc/lib/crypto/EDDSAMechanismParam.hsrc/lib/crypto/Makefile.amsrc/lib/crypto/OSSLEDDSA.cppsrc/lib/test/SignVerifyTests.cppsrc/lib/test/SignVerifyTests.h
| // Extract context parameters if provided | ||
| bool preHash = false; | ||
| ByteString contextData; | ||
| if (mechanismParam != NULL && mechanismParam->isOfType(EDDSAMechanismParam::type)) | ||
| { | ||
| EDDSAMechanismParam* eddsaParam = (EDDSAMechanismParam*) mechanismParam; | ||
| preHash = eddsaParam->flag; | ||
| if (!preHash && eddsaParam->contextData.size() > 0) | ||
| { | ||
| ERROR_MSG("Context data is not supported in Botan EDDSA implementation"); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| if (preHash) | ||
| { | ||
| unsigned long len = pk->getOrderLength(); | ||
| if (len == 0) | ||
| { | ||
| ERROR_MSG("EDDSA: Could not get the order length"); | ||
| return false; | ||
| } | ||
|
|
||
| if (pk->getOrderLength() == 32) | ||
| { | ||
| emsa = "Ed25519ph"; | ||
| } | ||
| else | ||
| { | ||
| emsa = "Ed448ph"; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Context data is silently discarded when preHash is true.
The guard only rejects context when preHash == false. When preHash == true, eddsaParam->contextData is never passed to Botan — Ed25519ph/Ed448ph are constructed with no context, so a caller-supplied contextData (a valid PKCS#11 CK_EDDSA_PARAMS combination) is silently ignored and the resulting signature will not verify against a context-aware verifier. Either reject non-empty contextData outright in the Botan backend (as done for the pure case) or fail loudly, rather than dropping it.
Also note ByteString contextData; (Line 103) is declared but never assigned or used.
🤖 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/BotanEDDSA.cpp` around lines 101 - 132, The Botan EDDSA path
is ignoring caller-provided contextData when preHash is true, which can produce
signatures that do not match context-aware verification. In BotanEDDSA::sign or
the mechanism-parameter handling block, either explicitly reject any non-empty
eddsaParam->contextData for both preHash and non-preHash cases, or wire it
through only if Botan supports it; do not silently discard it. Also remove the
unused local ByteString contextData or assign and validate it consistently
alongside EDDSAMechanismParam.
| if (pk->getOrderLength() == 32) | ||
| { | ||
| if (contextData.size() > 0) | ||
| { | ||
| instance = "Ed25519ctx"; | ||
| } | ||
| else if (preHash) | ||
| { | ||
| instance = "Ed25519ph"; | ||
| } | ||
| } | ||
| else if (pk->getOrderLength() == 57) | ||
| { | ||
| if (preHash) | ||
| { | ||
| instance = "Ed448ph"; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| ERROR_MSG("EDDSA: Unknown EdDSA key size: %zu", pk->getOrderLength()); | ||
| EVP_MD_CTX_free(ctx); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Instance selection drops valid context/pre-hash combinations.
Two gaps in the selection logic:
- Ed448 with context but no pre-hash: for
orderLength == 57,instanceis only set whenpreHashis true. A caller supplying context data without pre-hash leavesinstance == NULL, so the context params block is skipped and the context is silently dropped from the Ed448 signature. - Ed25519 with both pre-hash and context:
contextData.size() > 0is checked first and selects"Ed25519ctx", so aphFlag=1+ context request loses its pre-hash semantics.
Consider handling context on Ed448 (native "Ed448" accepts a context) and disambiguating the ph+ctx case for Ed25519. The same logic is mirrored in verify (Line 265–288).
🤖 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/OSSLEDDSA.cpp` around lines 115 - 138, The instance selection
in OSSLEDDSA::sign is dropping valid Ed448 context-only requests and
misclassifying Ed25519 when both context and pre-hash are requested. Update the
branching on pk->getOrderLength() so Ed448 sets the native Ed448 instance
whenever contextData is present (not only when preHash is true), and ensure the
Ed25519 path explicitly handles the ph+ctx combination instead of letting
contextData.size() win first. Apply the same selection rules in
OSSLEDDSA::verify so signing and verification stay aligned.
There was a problem hiding this comment.
@antoinelochet Here's a patch that fixes both issues — it lets Ed448 use context even without pre-hash (native "Ed448" mode always accepts a context string), and disambiguates the Ed25519 ph+ctx case by prioritizing preHash over context-size when selecting the instance (OpenSSL's "Ed25519ph" mode also accepts a context string via OSSL_SIGNATURE_PARAM_CONTEXT_STRING, so nothing is lost).
Apply the same change to both sign() (Line ~115-138) and verify() (Line ~265-288):
`#if` OPENSSL_VERSION_NUMBER >= 0x30200000L
const char* instance = NULL;
if (pk->getOrderLength() == 32)
{
- if (contextData.size() > 0)
+ if (preHash)
{
- instance = "Ed25519ctx";
- }
- else if (preHash)
- {
instance = "Ed25519ph";
}
+ else if (contextData.size() > 0)
+ {
+ instance = "Ed25519ctx";
+ }
}
else if (pk->getOrderLength() == 57)
{
if (preHash)
{
instance = "Ed448ph";
}
+ else
+ {
+ instance = "Ed448";
+ }
}
else
{
ERROR_MSG("EDDSA: Unknown EdDSA key size: %zu", pk->getOrderLength());
EVP_MD_CTX_free(ctx);
return false;
}Notes:
- For Ed25519 with
preHash == true, the instance is now always"Ed25519ph"regardless of context length, and the existing context-string param setting below still applies (empty context is fine when none is supplied). - For Ed448 with
preHash == false, the instance is now explicitly set to"Ed448"so the context params block below actually executes and the context isn't silently dropped. When there's no context either, this is equivalent to the previous default behavior (empty context), so it's safe to always take this branch. - Please verify against a real OpenSSL 3.2+ build/test (e.g., the new
testEdSignVerifyWithContext/testEdSignVerifyWithContextPreHashedtests) since I can't execute the build in this sandbox — in particular, double-check that OpenSSL's"Ed25519ph"provider implementation indeed honors a non-default context string set viaOSSL_SIGNATURE_PARAM_CONTEXT_STRINGin your OpenSSL version.
(ノ◕ヮ◕)ノ*:・゚✧
| // Set context if present (OpenSSL 3.0+) | ||
| if (instance != NULL) | ||
| { | ||
| INFO_MSG("EDDSA: Setting context for signature with instance: %s", instance); | ||
| EVP_PKEY_CTX* pctx = EVP_MD_CTX_get_pkey_ctx(ctx); | ||
| if (pctx != NULL) | ||
| { | ||
| OSSL_PARAM params[] = { | ||
| OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_INSTANCE, &instance, sizeof(instance)), | ||
| OSSL_PARAM_octet_string(OSSL_SIGNATURE_PARAM_CONTEXT_STRING, (unsigned char*)contextData.const_byte_str(), contextData.size()), | ||
| OSSL_PARAM_END | ||
| }; | ||
| if (!EVP_PKEY_CTX_set_params(pctx, params)) | ||
| { | ||
| DEBUG_MSG("EDDSA: Could not set EdDSA context (OpenSSL version may not support context)"); | ||
| } | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
EVP_MD_CTX_get_pkey_ctx returns NULL before EVP_DigestSignInit OpenSSL
💡 Result:
The function EVP_MD_CTX_get_pkey_ctx returns NULL before EVP_DigestSignInit because the EVP_PKEY_CTX is not initialized or assigned to the EVP_MD_CTX until that initialization function is called [1][2]. In the OpenSSL EVP workflow, the EVP_MD_CTX acts as a container for digest and signing operations [1]. When you call EVP_MD_CTX_new, you create an empty context that does not yet contain a PKEY context [1][2]. The EVP_PKEY_CTX is created and associated with the EVP_MD_CTX only when you perform the initialization step using EVP_DigestSignInit or EVP_DigestSignInit_ex [1][2]. If you need to customize the PKEY context (for example, to set padding or salt length) before the actual signing operation begins, you have two primary options: 1. Use the pctx pointer: When calling EVP_DigestSignInit, you can provide a pointer to an EVP_PKEY_CTX* variable [1][2]. Upon successful execution, OpenSSL will set this pointer to the newly created PKEY context, which you can then use to configure the operation [1][2]. 2. Use EVP_MD_CTX_set_pkey_ctx: If you must configure the context before initialization, you can manually create an EVP_PKEY_CTX (using EVP_PKEY_CTX_new_from_pkey or similar) and assign it to your EVP_MD_CTX using EVP_MD_CTX_set_pkey_ctx [3][4]. When EVP_DigestSignInit is subsequently called, it will use the provided context instead of creating a new one [1][5]. Note that in some older or specific legacy documentation contexts, EVP_MD_CTX_get_pkey_ctx might be referenced in ways that suggest limited usage or specific backward compatibility behaviors [6], but generally, its purpose is to retrieve a context that has already been established by an initialization routine or explicit assignment [3][4].
Citations:
- 1: https://docs.openssl.org/4.0/man3/EVP_DigestSignInit/
- 2: https://man.archlinux.org/man/core/openssl/EVP_DigestSignInit.3ssl.en
- 3: https://manpages.opensuse.org/Leap-15.6/openssl-3-doc/EVP_MD_CTX_set_pkey_ctx.33ssl.en.html
- 4: https://docs.openssl.org/3.4/man3/EVP_DigestInit/
- 5: https://docs.openssl.org/1.1.1/man3/EVP_DigestSignInit/
- 6: https://manpages.debian.org/experimental/libssl-doc/EVP_DigestInit_ex.3ssl.en.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='src/lib/crypto/OSSLEDDSA.cpp'
wc -l "$file"
sed -n '110,190p' "$file"
printf '\n---- VERIFY PATH ----\n'
sed -n '260,330p' "$file"Repository: softhsm/SoftHSMv2
Length of output: 4205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='src/lib/crypto/OSSLEDDSA.cpp'
nl -ba "$file" | sed -n '120,180p'
printf '\n----\n'
nl -ba "$file" | sed -n '270,320p'Repository: softhsm/SoftHSMv2
Length of output: 195
Apply the EdDSA params after init The instance/context params are set before EVP_DigestSignInit/EVP_DigestVerifyInit, so EVP_MD_CTX_get_pkey_ctx(ctx) is NULL and EVP_PKEY_CTX_set_params() never runs. Move this setup to the pctx returned by init (or use the _ex APIs) so Ed25519ctx/ph and Ed448ph are actually selected in both sign and verify paths.
🤖 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/OSSLEDDSA.cpp` around lines 140 - 158, The EdDSA
instance/context parameters are being applied before
EVP_DigestSignInit/EVP_DigestVerifyInit, so EVP_MD_CTX_get_pkey_ctx(ctx) can be
NULL and the params never reach OpenSSL. Update OSSLEDDSA.cpp to move the
OSSL_PARAM setup in the EdDSA sign/verify flow to the pctx obtained after init,
or switch to the _ex APIs, and ensure the same logic is used in both the signing
and verification paths so Ed25519ctx/ph and Ed448ph are actually selected.
| OSSL_PARAM params[] = { | ||
| OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_INSTANCE, &instance, sizeof(instance)), | ||
| OSSL_PARAM_octet_string(OSSL_SIGNATURE_PARAM_CONTEXT_STRING, (unsigned char*)contextData.const_byte_str(), contextData.size()), | ||
| OSSL_PARAM_END | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
OSSL_PARAM_utf8_string macro arguments buffer size OpenSSL
💡 Result:
The OSSL_PARAM_utf8_string macro in OpenSSL is used to define an OSSL_PARAM structure for a UTF8 string, taking three arguments: key (the parameter name), address (the buffer pointer), and size (the size of the buffer) [1][2]. The required buffer size depends on whether you are setting or requesting a parameter: When requesting (retrieving) parameters: The size argument must be set to the size of the buffer you have allocated [3][4]. Crucially, this size must include enough space for a terminating NUL byte [3][4]. If the buffer is not large enough to accommodate the string and the terminating NUL byte, the retrieval operation will fail [1][2]. When setting parameters: The size argument should be set to the length of the string, not counting the terminating NUL byte [3]. When using OSSL_PARAM_set_utf8_string specifically: If the parameter's data field is not NULL, the data_size must indicate that the buffer is large enough to hold the string, not including the terminating NUL byte [5]. A terminating NUL byte is only added if the buffer is longer than the string itself [5]. General usage summary: - OSSL_PARAM_utf8_string(key, address, size): Defines the parameter structure with the specified storage and size [1][2]. - For constant strings (where no copy is needed), consider using OSSL_PARAM_utf8_ptr, where the size argument refers to the data size, not the pointer size [3]. - When in doubt, OSSL_PARAM_UTF8_STRING is typically the correct type to use for printable strings [3].
Citations:
- 1: https://docs.openssl.org/3.5/man3/OSSL_PARAM_int/
- 2: https://manpages.opensuse.org/Leap-16.0/openssl-3-doc/OSSL_PARAM_utf8_string.33ssl.en.html
- 3: https://docs.openssl.org/3.5/man3/OSSL_PARAM/
- 4: https://docs.openssl.org/4.0/man3/OSSL_PARAM/
- 5: https://manpages.opensuse.org/Tumbleweed/openssl-3-doc/OSSL_PARAM_set_utf8_string.33ssl.en.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/crypto/OSSLEDDSA.cpp"
echo "== line count =="
wc -l "$file"
echo
echo "== around lines 130-170 =="
sed -n '130,170p' "$file" | cat -n
echo
echo "== around lines 280-310 =="
sed -n '280,310p' "$file" | cat -n
echo
echo "== search for instance declaration/usages =="
rg -n "\binstance\b" "$file"Repository: softhsm/SoftHSMv2
Length of output: 3659
🌐 Web query:
OpenSSL OSSL_PARAM_utf8_string vs OSSL_PARAM_utf8_ptr const char* instance set_params
💡 Result:
In OpenSSL's OSSL_PARAM API, OSSL_PARAM_UTF8_STRING and OSSL_PARAM_UTF8_PTR serve different purposes regarding data handling, memory, and const-correctness [1][2]. OSSL_PARAM_UTF8_STRING This type indicates that the parameter's data field points directly to the string buffer itself [1][2]. When setting parameters, you must provide a mutable buffer (not a const char*), as the API requires direct access to the storage [3][4]. The data_size should be the length of the string, excluding the terminating NUL byte [1][2]. If there is uncertainty regarding which type to use, OSSL_PARAM_UTF8_STRING is generally the correct choice [1][2]. OSSL_PARAM_UTF8_PTR This type indicates that the parameter's data field points to a pointer variable, which in turn points to the actual string [1][2]. This is specifically designed for constant data where you want to avoid copying the string [1][2]. By passing a pointer to your const char* variable, you do not need to copy the underlying string data [1][2]. When using OSSL_PARAM_UTF8_PTR, the data_size must be set to the size of the actual string data, not the size of the pointer variable [1][2]. Comparison Summary Feature: OSSL_PARAM_UTF8_STRING | OSSL_PARAM_UTF8_PTR Data pointing: Direct (points to buffer) | Indirect (points to a pointer) Purpose: Standard string buffer | Constant data (avoids copying) const char* compatibility: Incompatible (requires non-const) | Compatible (stores pointer to const) For set_params implementations, if you have a const char* and wish to avoid warnings or modification, OSSL_PARAM_UTF8_PTR is the intended mechanism [1][2]. If you use OSSL_PARAM_UTF8_STRING with a literal or const char*, you will likely encounter compiler warnings about discarding qualifiers, as the API expects to potentially write into the provided buffer [3][4].
Citations:
- 1: https://docs.openssl.org/3.4/man3/OSSL_PARAM/
- 2: https://docs.openssl.org/3.5/man3/OSSL_PARAM/
- 3: https://docs.openssl.org/3.5/man3/OSSL_PARAM_int/
- 4: OSSL_PARAM_utf8_string doesn't allow
const char*to be passed for parameters openssl/openssl#20956
🌐 Web query:
OpenSSL OSSL_PARAM_utf8_ptr macro signature set_params example const char**
💡 Result:
The OSSL_PARAM_utf8_ptr macro in OpenSSL is used to define an OSSL_PARAM entry where the parameter data is a pointer to a printable UTF-8 string, rather than the string data itself [1][2]. This is useful when you want to pass a constant string without copying it [1][3]. Macro Signature The macro is defined in <openssl/params.h> as [4]: #define OSSL_PARAM_utf8_ptr(key, addr, sz) OSSL_PARAM_DEFN((key), OSSL_PARAM_UTF8_PTR, (addr), sz) Parameters: key: The name of the parameter. addr: A pointer to the char* variable that holds the string pointer (i.e., a char). sz: The size of the string data (excluding the NUL terminator). Important usage note: Unlike some other OSSL_PARAM macros that take a direct address, this macro expects the address of the pointer variable that will point to your string [1][5]. Example: Setting Parameters To set parameters using this macro, you typically populate an OSSL_PARAM array [2][6]. #include <openssl/core.h> #include <string.h> const char foo = "some string"; size_t foo_l = strlen(foo); const OSSL_PARAM set[] = { OSSL_PARAM_utf8_ptr("foo", &foo, foo_l), OSSL_PARAM_END }; In a responder function (a function receiving these parameters), you can use OSSL_PARAM_set_utf8_ptr to update the value [6][7]: / Assuming 'params' is the OSSL_PARAM array received */ OSSL_PARAM p; if ((p = OSSL_PARAM_locate(params, "foo"))!= NULL) { OSSL_PARAM_set_utf8_ptr(p, "new string value"); } Key Distinctions OSSL_PARAM_UTF8_PTR vs OSSL_PARAM_UTF8_STRING: OSSL_PARAM_UTF8_STRING points directly to the string buffer and typically requires a copy [1][2]. OSSL_PARAM_UTF8_PTR points to a char variable, which allows the parameter to reference existing memory without copying [1][3]. If there is uncertainty about which to use, OSSL_PARAM_UTF8_STRING is generally recommended [1][3].
Citations:
- 1: https://docs.openssl.org/3.5/man3/OSSL_PARAM/
- 2: https://manpages.debian.org/trixie/libssl-doc/OSSL_PARAM.3ssl.en.html
- 3: https://docs.openssl.org/3.0/man3/OSSL_PARAM/
- 4: https://github.com/openssl/openssl/blob/master/include/openssl/params.h
- 5: OSSL_PARAM_utf8_ptr() API is confusing openssl/openssl#13446
- 6: https://github.com/openssl/openssl/blob/8782e9a7/doc/man3/OSSL_PARAM_int.pod
- 7: https://manpages.opensuse.org/Tumbleweed/openssl-3-doc/OSSL_PARAM_set_utf8_ptr.33ssl.en.html
Use OSSL_PARAM_utf8_ptr for instance
instance is a const char *, so OSSL_PARAM_utf8_string is the wrong constructor here. Pass the pointer variable with OSSL_PARAM_utf8_ptr(OSSL_SIGNATURE_PARAM_INSTANCE, &instance, strlen(instance)) here and in the matching verify path at line 298.
🤖 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/OSSLEDDSA.cpp` around lines 147 - 151, The `instance`
parameter is being built with the wrong OpenSSL param constructor in `OSSLEDDSA`
because it is a `const char *`, not a writable string buffer. Update the
`OSSL_PARAM` construction in the signing path and the matching verify path in
`OSSLEDDSA` to use `OSSL_PARAM_utf8_ptr` for `OSSL_SIGNATURE_PARAM_INSTANCE`,
passing the pointer variable itself and the correct string length, while leaving
the context string param unchanged.
| if (!EVP_DigestSignInit(ctx, NULL, NULL, NULL, pkey)) | ||
| { | ||
| ERROR_MSG("EDDSA sign init failed (0x%08X)", ERR_get_error()); | ||
| ERROR_MSG("EDDSA sign init failed (0x%08X): %s", ERR_get_error(), ERR_error_string(ERR_get_error(), NULL)); | ||
| EVP_MD_CTX_free(ctx); | ||
| return false; | ||
| } | ||
|
|
||
| if (!EVP_DigestSign(ctx, &signature[0], &len, dataToSign.const_byte_str(), dataToSign.size())) | ||
| { | ||
| ERROR_MSG("EDDSA sign failed (0x%08X)", ERR_get_error()); | ||
| ERROR_MSG("EDDSA sign failed (0x%08X): %s", ERR_get_error(), ERR_error_string(ERR_get_error(), NULL)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
ERR_get_error() is called twice per log line, yielding a mismatched code and message.
ERR_get_error() pops an entry from the error queue. Calling it once for 0x%08X and again inside ERR_error_string() returns two different queue entries (the second is often 0), so the printed code and text won't correspond. Argument evaluation order is also unspecified in C++. Capture the code once:
🐛 Proposed fix
- ERROR_MSG("EDDSA sign init failed (0x%08X): %s", ERR_get_error(), ERR_error_string(ERR_get_error(), NULL));
+ unsigned long e = ERR_get_error();
+ ERROR_MSG("EDDSA sign init failed (0x%08lX): %s", e, ERR_error_string(e, NULL));Same pattern at Line 170 (sign) and Line 312/322 (verify).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!EVP_DigestSignInit(ctx, NULL, NULL, NULL, pkey)) | |
| { | |
| ERROR_MSG("EDDSA sign init failed (0x%08X)", ERR_get_error()); | |
| ERROR_MSG("EDDSA sign init failed (0x%08X): %s", ERR_get_error(), ERR_error_string(ERR_get_error(), NULL)); | |
| EVP_MD_CTX_free(ctx); | |
| return false; | |
| } | |
| if (!EVP_DigestSign(ctx, &signature[0], &len, dataToSign.const_byte_str(), dataToSign.size())) | |
| { | |
| ERROR_MSG("EDDSA sign failed (0x%08X)", ERR_get_error()); | |
| ERROR_MSG("EDDSA sign failed (0x%08X): %s", ERR_get_error(), ERR_error_string(ERR_get_error(), NULL)); | |
| if (!EVP_DigestSignInit(ctx, NULL, NULL, NULL, pkey)) | |
| { | |
| unsigned long e = ERR_get_error(); | |
| ERROR_MSG("EDDSA sign init failed (0x%08lX): %s", e, ERR_error_string(e, NULL)); | |
| EVP_MD_CTX_free(ctx); | |
| return false; | |
| } | |
| if (!EVP_DigestSign(ctx, &signature[0], &len, dataToSign.const_byte_str(), dataToSign.size())) | |
| { | |
| ERROR_MSG("EDDSA sign failed (0x%08X): %s", ERR_get_error(), ERR_error_string(ERR_get_error(), NULL)); |
🤖 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/OSSLEDDSA.cpp` around lines 161 - 170, The OpenSSL error
logging in OSSLEDDSA.cpp is fetching the error queue twice per message, so the
numeric code and text can mismatch. Update the sign and verify error paths in
the EDDSA helpers to call ERR_get_error() once, store the result in a local
variable, and use that same value for both the formatted code and
ERR_error_string() output. Apply the same fix in the EVP_DigestSignInit,
EVP_DigestSign, and corresponding verification error logs.
|
|
||
| void SignVerifyTests::testEdSignVerifyWithContext(const char* curve) | ||
| { | ||
| #ifdef WITH_BOTAN | ||
| // Botan 2.X does not support EdDSA with context, so we skip this test for now. | ||
| return; | ||
| #endif | ||
| #if OPENSSL_VERSION_NUMBER < 0x30200000L | ||
| // OpenSSL 3.2.0 or later is required for EdDSA with pre-hash support | ||
| return; | ||
| #endif | ||
|
|
||
| CK_RV rv; | ||
| CK_SESSION_HANDLE hSessionRO; | ||
| CK_SESSION_HANDLE hSessionRW; | ||
|
|
||
| // Just make sure that we finalize any previous tests | ||
| CRYPTOKI_F_PTR( C_Finalize(NULL_PTR) ); | ||
|
|
||
| // Open read-only session on when the token is not initialized should fail | ||
| rv = CRYPTOKI_F_PTR( C_OpenSession(m_initializedTokenSlotID, CKF_SERIAL_SESSION, NULL_PTR, NULL_PTR, &hSessionRO) ); | ||
| CPPUNIT_ASSERT(rv == CKR_CRYPTOKI_NOT_INITIALIZED); | ||
|
|
||
| // Initialize the library and start the test. | ||
| rv = CRYPTOKI_F_PTR( C_Initialize(NULL_PTR) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
|
|
||
| // Open read-only session | ||
| rv = CRYPTOKI_F_PTR( C_OpenSession(m_initializedTokenSlotID, CKF_SERIAL_SESSION, NULL_PTR, NULL_PTR, &hSessionRO) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
|
|
||
| // Open read-write session | ||
| rv = CRYPTOKI_F_PTR( C_OpenSession(m_initializedTokenSlotID, CKF_SERIAL_SESSION | CKF_RW_SESSION, NULL_PTR, NULL_PTR, &hSessionRW) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
|
|
||
| // Login USER into the sessions so we can create a private objects | ||
| rv = CRYPTOKI_F_PTR( C_Login(hSessionRO,CKU_USER,m_userPin1,m_userPin1Length) ); | ||
| CPPUNIT_ASSERT(rv==CKR_OK); | ||
|
|
||
| CK_OBJECT_HANDLE hPuk = CK_INVALID_HANDLE; | ||
| CK_OBJECT_HANDLE hPrk = CK_INVALID_HANDLE; | ||
|
|
||
| // Test EdDSA signature with context data | ||
| // Create EdDSA parameters with context | ||
| CK_BYTE contextData[] = { 0x4a, 0x65, 0x73, 0x73, 0x69, 0x63, 0x61 }; // "Jessica" | ||
| CK_EDDSA_PARAMS eddsaParams = { | ||
| 0x00, // phFlag = 0 (no pre-hash) | ||
| sizeof(contextData), // context_data_len | ||
| contextData // context_data | ||
| }; | ||
|
|
||
| // Public Session keys with context | ||
| rv = generateED(curve, hSessionRW,IN_SESSION,IS_PUBLIC,IN_SESSION,IS_PUBLIC,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Private Session Keys with context | ||
| rv = generateED(curve, hSessionRW,IN_SESSION,IS_PRIVATE,IN_SESSION,IS_PRIVATE,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Test with different context data | ||
| CK_BYTE anotherContext[] = { 0x42, 0x6f, 0x62 }; // "Bob" | ||
| CK_EDDSA_PARAMS eddsaParams2 = { | ||
| 0x00, // phFlag = 0 | ||
| sizeof(anotherContext), // context_data_len | ||
| anotherContext // context_data | ||
| }; | ||
|
|
||
| // Public Token Keys with different context | ||
| rv = generateED(curve, hSessionRW,ON_TOKEN,IS_PUBLIC,ON_TOKEN,IS_PUBLIC,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams2, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Private Token Keys with different context | ||
| rv = generateED(curve, hSessionRW,ON_TOKEN,IS_PRIVATE,ON_TOKEN,IS_PRIVATE,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams2, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Test with empty context | ||
| CK_EDDSA_PARAMS eddsaParamsNoCtx = { | ||
| 0x00, // phFlag = 0 | ||
| 0, // context_data_len = 0 | ||
| NULL // context_data = NULL | ||
| }; | ||
|
|
||
| // Generate new keys for empty context test | ||
| rv = generateED(curve, hSessionRW,IN_SESSION,IS_PUBLIC,IN_SESSION,IS_PUBLIC,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParamsNoCtx, sizeof(CK_EDDSA_PARAMS)); | ||
| } | ||
|
|
||
| void SignVerifyTests::testEdSignVerifyWithContextPreHashed(const char* curve) | ||
| { | ||
| #ifdef WITH_BOTAN | ||
| // Botan 2.X does not support Ed448 curve, so we skip this test for now. | ||
| if (strcmp(curve, "Ed448") == 0) | ||
| return; | ||
| #endif | ||
| #if OPENSSL_VERSION_NUMBER < 0x30200000L | ||
| // OpenSSL 3.2.0 or later is required for EdDSA with pre-hash support | ||
| return; | ||
| #endif | ||
|
|
||
| CK_RV rv; | ||
| CK_SESSION_HANDLE hSessionRO; | ||
| CK_SESSION_HANDLE hSessionRW; | ||
|
|
||
| // Just make sure that we finalize any previous tests | ||
| CRYPTOKI_F_PTR( C_Finalize(NULL_PTR) ); | ||
|
|
||
| // Open read-only session on when the token is not initialized should fail | ||
| rv = CRYPTOKI_F_PTR( C_OpenSession(m_initializedTokenSlotID, CKF_SERIAL_SESSION, NULL_PTR, NULL_PTR, &hSessionRO) ); | ||
| CPPUNIT_ASSERT(rv == CKR_CRYPTOKI_NOT_INITIALIZED); | ||
|
|
||
| // Initialize the library and start the test. | ||
| rv = CRYPTOKI_F_PTR( C_Initialize(NULL_PTR) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
|
|
||
| // Open read-only session | ||
| rv = CRYPTOKI_F_PTR( C_OpenSession(m_initializedTokenSlotID, CKF_SERIAL_SESSION, NULL_PTR, NULL_PTR, &hSessionRO) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
|
|
||
| // Open read-write session | ||
| rv = CRYPTOKI_F_PTR( C_OpenSession(m_initializedTokenSlotID, CKF_SERIAL_SESSION | CKF_RW_SESSION, NULL_PTR, NULL_PTR, &hSessionRW) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
|
|
||
| // Login USER into the sessions so we can create a private objects | ||
| rv = CRYPTOKI_F_PTR( C_Login(hSessionRO,CKU_USER,m_userPin1,m_userPin1Length) ); | ||
| CPPUNIT_ASSERT(rv==CKR_OK); | ||
|
|
||
| CK_OBJECT_HANDLE hPuk = CK_INVALID_HANDLE; | ||
| CK_OBJECT_HANDLE hPrk = CK_INVALID_HANDLE; | ||
|
|
||
| // Test EdDSA signature with context data | ||
| // Create EdDSA parameters with context | ||
| CK_BYTE contextData[] = { 0x4a, 0x65, 0x73, 0x73, 0x69, 0x63, 0x61 }; // "Jessica" | ||
| CK_EDDSA_PARAMS eddsaParams = { | ||
| 0x01, // phFlag = 1 (pre-hash) | ||
| sizeof(contextData), // context_data_len | ||
| contextData // context_data | ||
| }; | ||
|
|
||
| // Public Session keys with context | ||
| rv = generateED(curve, hSessionRW,IN_SESSION,IS_PUBLIC,IN_SESSION,IS_PUBLIC,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Private Session Keys with context | ||
| rv = generateED(curve, hSessionRW,IN_SESSION,IS_PRIVATE,IN_SESSION,IS_PRIVATE,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Test with different context data | ||
| CK_BYTE anotherContext[] = { 0x42, 0x6f, 0x62 }; // "Bob" | ||
| CK_EDDSA_PARAMS eddsaParams2 = { | ||
| 0x01, // phFlag = 1 (pre-hash) | ||
| sizeof(anotherContext), // context_data_len | ||
| anotherContext // context_data | ||
| }; | ||
|
|
||
| // Public Token Keys with different context | ||
| rv = generateED(curve, hSessionRW,ON_TOKEN,IS_PUBLIC,ON_TOKEN,IS_PUBLIC,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams2, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Private Token Keys with different context | ||
| rv = generateED(curve, hSessionRW,ON_TOKEN,IS_PRIVATE,ON_TOKEN,IS_PRIVATE,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams2, sizeof(CK_EDDSA_PARAMS)); | ||
|
|
||
| // Test with empty context | ||
| CK_EDDSA_PARAMS eddsaParamsNoCtx = { | ||
| 0x01, // phFlag = 1 (pre-hash) | ||
| 0, // context_data_len = 0 | ||
| NULL // context_data = NULL | ||
| }; | ||
|
|
||
| // Generate new keys for empty context test | ||
| rv = generateED(curve, hSessionRW,IN_SESSION,IS_PUBLIC,IN_SESSION,IS_PUBLIC,hPuk,hPrk); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParamsNoCtx, sizeof(CK_EDDSA_PARAMS)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing negative cross-context/cross-mode verification coverage.
Both new tests only sign and verify using the same CK_EDDSA_PARAMS for a given key pair; they never assert that a signature produced with one context (or with pre-hash) fails to verify under a different context (or under the plain/pre-hash mismatch). Domain separation is the entire point of context data — this is the property most worth testing and it's currently unverified.
🧪 Example additional check to add per key-pair
// Public Session keys with context
rv = generateED(curve, hSessionRW,IN_SESSION,IS_PUBLIC,IN_SESSION,IS_PUBLIC,hPuk,hPrk);
CPPUNIT_ASSERT(rv == CKR_OK);
signVerifySingle(CKM_EDDSA, hSessionRO, hPuk, hPrk, &eddsaParams, sizeof(CK_EDDSA_PARAMS));
+
+ // Signature produced under context A must not verify under context B
+ CK_MECHANISM signMech = { CKM_EDDSA, &eddsaParams, sizeof(CK_EDDSA_PARAMS) };
+ CK_MECHANISM verifyMechOther = { CKM_EDDSA, &eddsaParams2, sizeof(CK_EDDSA_PARAMS) };
+ CK_BYTE data[] = { 0x00, 0x01, 0x02 };
+ CK_BYTE signature[64 * 1024];
+ CK_ULONG ulSignatureLen = sizeof(signature);
+ rv = CRYPTOKI_F_PTR( C_SignInit(hSessionRO, &signMech, hPrk) );
+ CPPUNIT_ASSERT(rv == CKR_OK);
+ rv = CRYPTOKI_F_PTR( C_Sign(hSessionRO, data, sizeof(data), signature, &ulSignatureLen) );
+ CPPUNIT_ASSERT(rv == CKR_OK);
+ rv = CRYPTOKI_F_PTR( C_VerifyInit(hSessionRO, &verifyMechOther, hPuk) );
+ CPPUNIT_ASSERT(rv == CKR_OK);
+ rv = CRYPTOKI_F_PTR( C_Verify(hSessionRO, data, sizeof(data), signature, ulSignatureLen) );
+ CPPUNIT_ASSERT(rv == CKR_SIGNATURE_INVALID);🤖 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/test/SignVerifyTests.cpp` around lines 847 - 1029, The new EdDSA
context tests in testEdSignVerifyWithContext and
testEdSignVerifyWithContextPreHashed only cover successful same-context
verification, so add negative assertions that a signature created with one
CK_EDDSA_PARAMS cannot be verified with a different context or with the other
phFlag mode. Reuse the existing signVerifySingle flow or the lower-level
sign/verify helpers around generateED and CK_EDDSA_PARAMS to produce one
signature and then verify it with mismatched parameters for each key pair type,
ensuring the test explicitly checks cross-context and plain-vs-prehash failures.
1 participant
Tentative fix for #873
Not sure about the Botan implementation.
Summary by CodeRabbit
New Features
Bug Fixes
Tests