Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ repos:
files: ^\.github/workflows/
- id: generate-compile-commands
name: bazel-compile-commands (for clang-tidy)
entry: bazel-compile-commands --compiler /usr/bin/clang -R "-fno-canonical-system-headers=" //...
entry: bazel-compile-commands --compiler /usr/bin/clang -R "-fno-canonical-system-headers=" -b '--define=pkcs11_lib=/dummy/path/to/make/pkcs11_lib_path/gen_rule/happy' //...
language: system
files: \.(c|cpp|h|hpp)$
pass_filenames: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#include "score/crypto/src/daemon/provider/i_provider_factory.hpp"
#include "score/crypto/src/daemon/provider/pkcs11/pkcs11_token_config.hpp"

#include <vector>

namespace score::crypto::daemon::provider::pkcs11
{

Expand Down Expand Up @@ -47,7 +45,7 @@ namespace score::crypto::daemon::provider::pkcs11
class Pkcs11ProviderFactory final : public IProviderFactory
{
public:
explicit Pkcs11ProviderFactory(Pkcs11ProviderFactoryConfig config);
explicit Pkcs11ProviderFactory(const Pkcs11ProviderFactoryConfig& config);

~Pkcs11ProviderFactory() override = default;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,22 @@
#include "score/crypto/src/daemon/provider/pkcs11/pkcs11_provider_factory.hpp"

#include <memory>
#include <vector>

#include <pkcs11.h>

#include "score/crypto/src/daemon/common/daemon_error.hpp"
#include "score/crypto/src/daemon/common/types.hpp"
#include "score/crypto/src/daemon/provider/i_provider_factory.hpp"
#include "score/crypto/src/daemon/provider/pkcs11/pkcs11_module.hpp"
#include "score/crypto/src/daemon/provider/pkcs11/pkcs11_provider.hpp"
#include "score/crypto/src/daemon/provider/pkcs11/pkcs11_token_config.hpp"
#include "score/crypto/src/daemon/provider/provider_manager.hpp"

namespace score::crypto::daemon::provider::pkcs11
{

Pkcs11ProviderFactory::Pkcs11ProviderFactory(Pkcs11ProviderFactoryConfig config) : m_config{std::move(config)} {}
Pkcs11ProviderFactory::Pkcs11ProviderFactory(const Pkcs11ProviderFactoryConfig& config) : m_config{config} {}

ProviderFactoryResult Pkcs11ProviderFactory::CreateAndRegister(ProviderManager& manager)
{
Expand Down Expand Up @@ -60,7 +65,11 @@ ProviderFactoryResult Pkcs11ProviderFactory::CreateAndRegister(ProviderManager&
// so that C_Initialize is called exactly once and C_Finalize is deferred
// until the very last provider (and therefore all its sessions) is destroyed.
auto pkcs11Module = std::make_shared<Pkcs11Module>();
const auto initResult = pkcs11Module->Init();

CK_C_INITIALIZE_ARGS init_args{};
// Set CKF_OS_LOCKING_OK since the daemon will use it in a multi-threaded manner.
init_args.flags = CKF_OS_LOCKING_OK;
const auto initResult = pkcs11Module->Init(&init_args);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShoroukRamzy & @schreibwsag I would like your feedback on this point.

The daemon intents to use the pkcs11 module concurrently from multiple threads. Previously, we did not indicate this during C_Initialize().

We now set CKF_OS_LOCKING_OK to inform the pkcs11 module that we use it concurrently and it may use the OS locking mechanisms.

Please check whether the pkcs11 module(s) in use support this mode and whether you have any concerns.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm interested in the pkcs11 module pkcs11-hse from NXP.
Looking at the source code, CKF_OS_LOCKING_OK seems to be supported. I will check with NXP, but my
assumption is that is supported.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @se-lo, Yes, The rust cryptoki lib supports CKF_OS_LOCKING_OK via internal parking_lot / RwLock primitives.
SoftHSM supports it as well.

if (!initResult.has_value())
{
result.failures.push_back(ProviderFailure{
Expand Down
3 changes: 1 addition & 2 deletions third_party/openssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ configure_make(
}) + [
"shared", # Build shared libraries (libcrypto.so, libssl.so)
"no-asm", # Disable assembly optimizations to avoid GCC inline assembly issues
"no-threads", # Disable threading to avoid pthread requirement on QNX
"no-makedepend", # Disable dependency generation that causes QCC issues
"no-sock", # Disable socket/network BIO (not needed for a crypto library; avoids missing socket APIs in QNX sysroot)
# Note: ENGINE API kept enabled for SoftHSM compatibility (ENGINE is deprecated in OpenSSL 3.x but still functional)
Expand Down Expand Up @@ -81,7 +80,7 @@ configure_make(
"no-afalgeng", # Disable Linux AF_ALG engine (requires linux/version.h, not available in QNX sysroot)
"no-padlockeng", # Disable VIA Padlock engine (x86-only, irrelevant for aarch64/QNX targets)
"no-legacy", # Disable legacy provider (only default provider used)
"no-async", # Disable async dispatch (unused, especially with no-threads)
"no-async", # Disable async dispatch (unused)
"-DUSE_TIMEGM", # QNX doesn't provide the 'timezone' global variable
"--prefix=$${INSTALLDIR}",
"--openssldir=$${INSTALLDIR}",
Expand Down
Loading