From a34b7a9ece87fac9fc98de0a1f071ada251ff093 Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Mon, 30 Oct 2023 09:21:07 +0100 Subject: [PATCH 1/9] Use Builder Pattern for HsmKeyParams --- src/hsm.cpp | 6 ++-- src/mococrw/hsm.h | 38 +++++++++++++++++++--- tests/integration/hsm-integration-test.cpp | 8 +++-- tests/unit/test_hsm.cpp | 3 +- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/hsm.cpp b/src/hsm.cpp index 286ce9da..23f213cb 100644 --- a/src/hsm.cpp +++ b/src/hsm.cpp @@ -188,7 +188,8 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const RSASpec &spec, const std::string &keyLabel, const std::vector &keyID) { - HsmKeyParams hsmKeyParams; + HsmKeyParams hsmKeyParams = + HsmKeyParams::Builder().setCkaExtractable(false).setCkaSensitive(true).build(); return generateKey(spec, keyLabel, keyID, hsmKeyParams); } @@ -233,7 +234,8 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const ECCSpec &spec, const std::string &keyLabel, const std::vector &keyID) { - HsmKeyParams hsmKeyParams; + HsmKeyParams hsmKeyParams = + HsmKeyParams::Builder().setCkaExtractable(false).setCkaSensitive(true).build(); return generateKey(spec, keyLabel, keyID, hsmKeyParams); } diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index 79d7f78a..3c665612 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -26,17 +26,47 @@ class ECCSpec; class RSASpec; /** - * This struct currently contains PKCS#11 attributes which are changeable on key creation. + * This class currently contains PKCS#11 attributes which are changeable on key creation. * In the future also parameters for other keystorage interfaces can be added. */ -struct HsmKeyParams +class HsmKeyParams { +public: + class Builder; + +private: + bool cka_extractable; + bool cka_sensitive; + /* Default is that the key cannot be extracted and is marked as sensitive. * Check https://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html * for more details. */ - bool cka_extractable = false; - bool cka_sensitive = true; + HsmKeyParams() : cka_extractable(false), cka_sensitive(true) {} +}; + +class HsmKeyParams::Builder +{ +public: + Builder() {} + Builder &setCkaExtractable(bool extractable) + { + params_.cka_extractable = extractable; + return *this; + } + + Builder &setCkaSensitive(bool sensitive) + { + params_.cka_sensitive = sensitive; + return *this; + } + + HsmKeyParams build() { + return params_; + } + +private: + HsmKeyParams params_; }; /** diff --git a/tests/integration/hsm-integration-test.cpp b/tests/integration/hsm-integration-test.cpp index a5700094..3ddf5156 100644 --- a/tests/integration/hsm-integration-test.cpp +++ b/tests/integration/hsm-integration-test.cpp @@ -452,9 +452,11 @@ int main(void) /** * Generate extractable and non-extractable keys for ECC and RSA */ - HsmKeyParams hsmKeyParamsExtract = {/*.cka_extractable =*/true, - /* .cka_sensitive = */ false}; - HsmKeyParams hsmKeyParamsDefault; + HsmKeyParams hsmKeyParamsExtract = + HsmKeyParams::Builder().setCkaExtractable(true).setCkaSensitive(false).build(); + + HsmKeyParams hsmKeyParamsDefault = + HsmKeyParams::Builder().build(); /* We need a new token otherwise the keys generated before litter the slot */ diff --git a/tests/unit/test_hsm.cpp b/tests/unit/test_hsm.cpp index e25175ae..58e29815 100644 --- a/tests/unit/test_hsm.cpp +++ b/tests/unit/test_hsm.cpp @@ -153,7 +153,8 @@ TEST_F(HSMTest, testHSMKeygenWithParams) auto hsm = initialiseEngine(); std::string keyLabel{"key-label"}; std::vector keyId{0x12}; - HsmKeyParams params{true, false}; + HsmKeyParams params = + HsmKeyParams::Builder().setCkaExtractable(true).setCkaSensitive(false).build(); EXPECT_CALL(_mock(), SSL_ENGINE_ctrl_cmd_string( engine, StrEq("PIN"), StrEq(pin.c_str()), 0 /*non-optional*/)) From 319ac8c826c8a7093df7b5ef9b980d438bf46c90 Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:20:53 +0100 Subject: [PATCH 2/9] fixup! Use Builder Pattern for HsmKeyParams --- src/hsm.cpp | 8 ++++---- src/mococrw/hsm.h | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/hsm.cpp b/src/hsm.cpp index 23f213cb..5ed87d02 100644 --- a/src/hsm.cpp +++ b/src/hsm.cpp @@ -216,8 +216,8 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const RSASpec &spec, pkcs11RSASpec.bits = spec.numberOfBits(); PKCS11_params _params; - _params.extractable = static_cast(params.cka_extractable); - _params.sensitive = static_cast(params.cka_sensitive); + _params.extractable = static_cast(params.isExtractable()); + _params.sensitive = static_cast(params.isSensitive()); PKCS11_KGEN_ATTRS pkcs11RSAKeygen; pkcs11RSAKeygen.type = EVP_PKEY_RSA; @@ -263,8 +263,8 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const ECCSpec &spec, pkcs11ECCSpec.curve = curve.c_str(); PKCS11_params _params; - _params.extractable = static_cast(params.cka_extractable); - _params.sensitive = static_cast(params.cka_sensitive); + _params.extractable = static_cast(params.isExtractable()); + _params.sensitive = static_cast(params.isSensitive()); PKCS11_KGEN_ATTRS pkcs11ECCKeygen; pkcs11ECCKeygen.type = EVP_PKEY_EC; diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index 3c665612..1839a05e 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -34,6 +34,14 @@ class HsmKeyParams public: class Builder; + bool isExtractable() { + return cka_extractable; + } + + bool isSensitive() { + return cka_sensitive; + } + private: bool cka_extractable; bool cka_sensitive; From 7c7b5afa819a59c549cdcd520073c34c3385d76d Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:30:12 +0100 Subject: [PATCH 3/9] fixup! fixup! Use Builder Pattern for HsmKeyParams --- src/mococrw/hsm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index 1839a05e..eb93d6ba 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -34,11 +34,11 @@ class HsmKeyParams public: class Builder; - bool isExtractable() { + bool isExtractable() const { return cka_extractable; } - bool isSensitive() { + bool isSensitive() const { return cka_sensitive; } From c8078e40f588e964502b6b7d43015f88511f62ae Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Mon, 30 Oct 2023 11:19:52 +0100 Subject: [PATCH 4/9] fixup! Use Builder Pattern for HsmKeyParams --- src/mococrw/hsm.h | 12 +++--------- tests/integration/hsm-integration-test.cpp | 3 +-- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index eb93d6ba..3ff325e6 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -34,13 +34,9 @@ class HsmKeyParams public: class Builder; - bool isExtractable() const { - return cka_extractable; - } + bool isExtractable() const { return cka_extractable; } - bool isSensitive() const { - return cka_sensitive; - } + bool isSensitive() const { return cka_sensitive; } private: bool cka_extractable; @@ -69,9 +65,7 @@ class HsmKeyParams::Builder return *this; } - HsmKeyParams build() { - return params_; - } + HsmKeyParams build() { return params_; } private: HsmKeyParams params_; diff --git a/tests/integration/hsm-integration-test.cpp b/tests/integration/hsm-integration-test.cpp index 3ddf5156..967e95f3 100644 --- a/tests/integration/hsm-integration-test.cpp +++ b/tests/integration/hsm-integration-test.cpp @@ -455,8 +455,7 @@ int main(void) HsmKeyParams hsmKeyParamsExtract = HsmKeyParams::Builder().setCkaExtractable(true).setCkaSensitive(false).build(); - HsmKeyParams hsmKeyParamsDefault = - HsmKeyParams::Builder().build(); + HsmKeyParams hsmKeyParamsDefault = HsmKeyParams::Builder().build(); /* We need a new token otherwise the keys generated before litter the slot */ From 12c147b646308bbc6af935054d7da969b92916eb Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:35:41 +0100 Subject: [PATCH 5/9] fixup! Use Builder Pattern for HsmKeyParams --- src/hsm.cpp | 9 +++++---- src/mococrw/hsm.h | 17 ++++------------- tests/integration/hsm-integration-test.cpp | 4 ++-- tests/unit/test_hsm.cpp | 2 +- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/hsm.cpp b/src/hsm.cpp index 5ed87d02..b018c7e8 100644 --- a/src/hsm.cpp +++ b/src/hsm.cpp @@ -189,7 +189,7 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const RSASpec &spec, const std::vector &keyID) { HsmKeyParams hsmKeyParams = - HsmKeyParams::Builder().setCkaExtractable(false).setCkaSensitive(true).build(); + HsmKeyParams::Builder{}.setExtractable(false).build(); return generateKey(spec, keyLabel, keyID, hsmKeyParams); } @@ -217,7 +217,7 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const RSASpec &spec, PKCS11_params _params; _params.extractable = static_cast(params.isExtractable()); - _params.sensitive = static_cast(params.isSensitive()); + _params.sensitive = static_cast(!params.isExtractable()); PKCS11_KGEN_ATTRS pkcs11RSAKeygen; pkcs11RSAKeygen.type = EVP_PKEY_RSA; @@ -235,7 +235,7 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const ECCSpec &spec, const std::vector &keyID) { HsmKeyParams hsmKeyParams = - HsmKeyParams::Builder().setCkaExtractable(false).setCkaSensitive(true).build(); + HsmKeyParams::Builder{}.setExtractable(false).build(); return generateKey(spec, keyLabel, keyID, hsmKeyParams); } @@ -263,8 +263,9 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const ECCSpec &spec, pkcs11ECCSpec.curve = curve.c_str(); PKCS11_params _params; + // If the key is extractable it shouldn't be sensitive and vice versa _params.extractable = static_cast(params.isExtractable()); - _params.sensitive = static_cast(params.isSensitive()); + _params.sensitive = static_cast(!params.isExtractable()); PKCS11_KGEN_ATTRS pkcs11ECCKeygen; pkcs11ECCKeygen.type = EVP_PKEY_EC; diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index 3ff325e6..004a013f 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -36,32 +36,23 @@ class HsmKeyParams bool isExtractable() const { return cka_extractable; } - bool isSensitive() const { return cka_sensitive; } - private: - bool cka_extractable; - bool cka_sensitive; + bool extractable; /* Default is that the key cannot be extracted and is marked as sensitive. * Check https://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html * for more details. */ - HsmKeyParams() : cka_extractable(false), cka_sensitive(true) {} + HsmKeyParams() : cka_extractable(false) {} }; class HsmKeyParams::Builder { public: Builder() {} - Builder &setCkaExtractable(bool extractable) - { - params_.cka_extractable = extractable; - return *this; - } - - Builder &setCkaSensitive(bool sensitive) + Builder &setExtractable(bool extractable) { - params_.cka_sensitive = sensitive; + params_.extractable = extractable; return *this; } diff --git a/tests/integration/hsm-integration-test.cpp b/tests/integration/hsm-integration-test.cpp index 967e95f3..f04fb781 100644 --- a/tests/integration/hsm-integration-test.cpp +++ b/tests/integration/hsm-integration-test.cpp @@ -453,9 +453,9 @@ int main(void) * Generate extractable and non-extractable keys for ECC and RSA */ HsmKeyParams hsmKeyParamsExtract = - HsmKeyParams::Builder().setCkaExtractable(true).setCkaSensitive(false).build(); + HsmKeyParams::Builder{}.setExtractable(true).build(); - HsmKeyParams hsmKeyParamsDefault = HsmKeyParams::Builder().build(); + HsmKeyParams hsmKeyParamsDefault = HsmKeyParams::Builder{}.build(); /* We need a new token otherwise the keys generated before litter the slot */ diff --git a/tests/unit/test_hsm.cpp b/tests/unit/test_hsm.cpp index 58e29815..df37bc25 100644 --- a/tests/unit/test_hsm.cpp +++ b/tests/unit/test_hsm.cpp @@ -154,7 +154,7 @@ TEST_F(HSMTest, testHSMKeygenWithParams) std::string keyLabel{"key-label"}; std::vector keyId{0x12}; HsmKeyParams params = - HsmKeyParams::Builder().setCkaExtractable(true).setCkaSensitive(false).build(); + HsmKeyParams::Builder{}.setExtractable(true).build(); EXPECT_CALL(_mock(), SSL_ENGINE_ctrl_cmd_string( engine, StrEq("PIN"), StrEq(pin.c_str()), 0 /*non-optional*/)) From 64c10c80326d4d61e36289f708aa0c730d4f7a3d Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:49:22 +0100 Subject: [PATCH 6/9] fixup! Use Builder Pattern for HsmKeyParams --- src/mococrw/hsm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index 004a013f..f223b37b 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -34,7 +34,7 @@ class HsmKeyParams public: class Builder; - bool isExtractable() const { return cka_extractable; } + bool isExtractable() const { return extractable; } private: bool extractable; From 3ac296ce19cb4d621dbb40bf7d741ce41f751048 Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:56:52 +0100 Subject: [PATCH 7/9] fixup! Use Builder Pattern for HsmKeyParams --- src/mococrw/hsm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index f223b37b..16b3bd41 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -43,7 +43,7 @@ class HsmKeyParams * Check https://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html * for more details. */ - HsmKeyParams() : cka_extractable(false) {} + HsmKeyParams() : extractable(false) {} }; class HsmKeyParams::Builder From 3cbd4dac6fe812d1203334ba1e1dbfd645c3045b Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Tue, 31 Oct 2023 16:22:52 +0100 Subject: [PATCH 8/9] clang format --- src/hsm.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hsm.cpp b/src/hsm.cpp index b018c7e8..aaa2dabd 100644 --- a/src/hsm.cpp +++ b/src/hsm.cpp @@ -188,8 +188,7 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const RSASpec &spec, const std::string &keyLabel, const std::vector &keyID) { - HsmKeyParams hsmKeyParams = - HsmKeyParams::Builder{}.setExtractable(false).build(); + HsmKeyParams hsmKeyParams = HsmKeyParams::Builder{}.setExtractable(false).build(); return generateKey(spec, keyLabel, keyID, hsmKeyParams); } @@ -234,8 +233,7 @@ openssl::SSL_EVP_PKEY_Ptr HsmEngine::generateKey(const ECCSpec &spec, const std::string &keyLabel, const std::vector &keyID) { - HsmKeyParams hsmKeyParams = - HsmKeyParams::Builder{}.setExtractable(false).build(); + HsmKeyParams hsmKeyParams = HsmKeyParams::Builder{}.setExtractable(false).build(); return generateKey(spec, keyLabel, keyID, hsmKeyParams); } From 896f77676227f76f400ebd73697598ea78c89b4d Mon Sep 17 00:00:00 2001 From: cps-b <136316734+cps-b@users.noreply.github.com> Date: Fri, 3 Nov 2023 09:33:36 +0100 Subject: [PATCH 9/9] fixup! Use Builder Pattern for HsmKeyParams --- src/mococrw/hsm.h | 8 ++++---- tests/integration/hsm-integration-test.cpp | 3 +-- tests/unit/test_hsm.cpp | 3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/mococrw/hsm.h b/src/mococrw/hsm.h index 16b3bd41..0b28dd03 100644 --- a/src/mococrw/hsm.h +++ b/src/mococrw/hsm.h @@ -34,16 +34,16 @@ class HsmKeyParams public: class Builder; - bool isExtractable() const { return extractable; } + bool isExtractable() const { return _extractable; } private: - bool extractable; + bool _extractable; /* Default is that the key cannot be extracted and is marked as sensitive. * Check https://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html * for more details. */ - HsmKeyParams() : extractable(false) {} + HsmKeyParams() : _extractable(false) {} }; class HsmKeyParams::Builder @@ -52,7 +52,7 @@ class HsmKeyParams::Builder Builder() {} Builder &setExtractable(bool extractable) { - params_.extractable = extractable; + params_._extractable = extractable; return *this; } diff --git a/tests/integration/hsm-integration-test.cpp b/tests/integration/hsm-integration-test.cpp index f04fb781..7a07ff2d 100644 --- a/tests/integration/hsm-integration-test.cpp +++ b/tests/integration/hsm-integration-test.cpp @@ -452,8 +452,7 @@ int main(void) /** * Generate extractable and non-extractable keys for ECC and RSA */ - HsmKeyParams hsmKeyParamsExtract = - HsmKeyParams::Builder{}.setExtractable(true).build(); + HsmKeyParams hsmKeyParamsExtract = HsmKeyParams::Builder{}.setExtractable(true).build(); HsmKeyParams hsmKeyParamsDefault = HsmKeyParams::Builder{}.build(); diff --git a/tests/unit/test_hsm.cpp b/tests/unit/test_hsm.cpp index df37bc25..7e8dcd92 100644 --- a/tests/unit/test_hsm.cpp +++ b/tests/unit/test_hsm.cpp @@ -153,8 +153,7 @@ TEST_F(HSMTest, testHSMKeygenWithParams) auto hsm = initialiseEngine(); std::string keyLabel{"key-label"}; std::vector keyId{0x12}; - HsmKeyParams params = - HsmKeyParams::Builder{}.setExtractable(true).build(); + HsmKeyParams params = HsmKeyParams::Builder{}.setExtractable(true).build(); EXPECT_CALL(_mock(), SSL_ENGINE_ctrl_cmd_string( engine, StrEq("PIN"), StrEq(pin.c_str()), 0 /*non-optional*/))