Improve crypto daemon portability and provider resilience#84
Merged
Conversation
antonkri
commented
Jul 10, 2026
antonkri
commented
Contributor
- config: replace std::filesystem::exists with POSIX ::stat to avoid libstdc++fs dependency on the target/QNX toolchain
- flatbuffer_config_parser: fix signed/unsigned comparison in file-size guard by casting std::streamsize to its unsigned equivalent
- provider_manager: make provider creation best-effort — continue when a non-critical factory (e.g. PKCS#11/SoftHSM) fails and return false instead of throwing when no provider registers
- openssl_hmac_handler: validate bound key against the context's provider_id instead of a hardcoded 0, with clearer error logging
- tests/demo: set init_params.provider_id from the bound key in MakeMacHandler to mirror the mediator and fix Demo2/Demo3 MAC tests
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
ChansAlive
approved these changes
Jul 14, 2026
ChansAlive
left a comment
ChansAlive
left a comment
Contributor
There was a problem hiding this comment.
Changes looks good and relevant.
There was a problem hiding this comment.
Pull request overview
This PR improves the crypto daemon’s portability (notably for QNX toolchains) and strengthens multi-provider resilience by relaxing provider factory failure handling, while also aligning MAC handler initialization with provider-aware key binding.
Changes:
- Replace
std::filesystem::existswith POSIX::statin config discovery to avoid filesystem library/toolchain dependencies. - Make provider factory initialization best-effort and adjust provider registration success criteria (with a correctness issue noted below).
- Tighten OpenSSL HMAC handler bound-key validation to compare against the context’s
provider_id, and update demo tests to mirror mediator behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/demo/mac_multi_provider_demo.cpp | Sets init_params.provider_id from the bound key so MAC demo tests match mediator/provider-id validation behavior. |
| score/crypto/daemon/provider/src/provider_manager.cpp | Changes initialization to return false instead of throwing and alters factory/provider creation flow for best-effort behavior. |
| score/crypto/daemon/provider/score_provider/openssl/operations/mac/openssl_hmac_handler.cpp | Validates bound key provider id against context provider id and improves mismatch logging. |
| score/crypto/daemon/config/src/flatbuffer_config_parser.cpp | Adjusts file-size guard to avoid signed/unsigned comparison warnings by casting to an unsigned equivalent. |
| score/crypto/daemon/config/src/config.cpp | Switches config existence checks from std::filesystem::exists to ::stat for portability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+133
to
137
| bool any_registered = false; | ||
| for (auto& factory : m_factories) | ||
| { | ||
| if (!factory->CreateAndRegister(*this)) | ||
| if (factory->CreateAndRegister(*this)) | ||
| { |
Comment on lines
+90
to
94
| struct stat st_cfg; | ||
| if (::stat(config_file_path, &st_cfg) != 0) | ||
| { | ||
| score::mw::log::LogError() << "[CONFIG] Configuration file does not exist:" << config_file_path; | ||
| return false; |
Comment on lines
+168
to
+172
| if (init_params.bound_key_handler->GetProviderId() != init_params.provider_id) | ||
| { | ||
| score::mw::log::LogError() << LOG_PREFIX << "InitializeContext: bound key is not an OpenSSL key handler"; | ||
| score::mw::log::LogError() << LOG_PREFIX << "InitializeContext: bound key is not an OpenSSL key handler" | ||
| << " (key provider_id=" << init_params.bound_key_handler->GetProviderId() | ||
| << ", expected=" << init_params.provider_id << ")"; |
Contributor
Author
|
/recheck |
Contributor
Author
|
recheck |
- config: replace std::filesystem::exists with POSIX ::stat to avoid libstdc++fs dependency on the target/QNX toolchain - flatbuffer_config_parser: fix signed/unsigned comparison in file-size guard by casting std::streamsize to its unsigned equivalent - provider_manager: make provider creation best-effort — continue when a non-critical factory (e.g. PKCS#11/SoftHSM) fails and return false instead of throwing when no provider registers - openssl_hmac_handler: validate bound key against the context's provider_id instead of a hardcoded 0, with clearer error logging - tests/demo: set init_params.provider_id from the bound key in MakeMacHandler to mirror the mediator and fix Demo2/Demo3 MAC tests
antonkri
force-pushed
the
ugmu_improve_portability
branch
from
b9bef43 to
4b8ef8c
Compare
nradakovic
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.