diff --git a/include/oqs_cpp.hpp b/include/oqs_cpp.hpp index 6927967..35806a4 100644 --- a/include/oqs_cpp.hpp +++ b/include/oqs_cpp.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -35,6 +36,22 @@ extern "C" { } } // namespace C +namespace internal { +// Guards temporary secret storage, including partially generated keys. +class SecretKeyGuard { + bytes& key_; + + public: + explicit SecretKeyGuard(bytes& key) noexcept : key_{key} {} + SecretKeyGuard(const SecretKeyGuard&) = delete; + SecretKeyGuard& operator=(const SecretKeyGuard&) = delete; + ~SecretKeyGuard() noexcept { + if (!key_.empty()) + C::OQS_MEM_cleanse(key_.data(), key_.size()); + } +}; +} // namespace internal + /** * \class oqs::MechanismNotSupportedError * \brief Cryptographic scheme not supported @@ -164,10 +181,13 @@ class KEMs final : public internal::Singleton { * \brief Key encapsulation mechanisms */ class KeyEncapsulation { - std::shared_ptr kem_{nullptr, [](C::OQS_KEM* p) { - C::OQS_KEM_free(p); - }}; ///< liboqs smart pointer to C::OQS_KEM - bytes secret_key_{}; ///< secret key + std::shared_ptr kem_{}; ///< liboqs object + + static void cleanse_secret_key(bytes& secret_key) noexcept { + if (!secret_key.empty()) + C::OQS_MEM_cleanse(secret_key.data(), secret_key.size()); + } + public: /** * \brief KEM algorithm details @@ -185,6 +205,16 @@ class KeyEncapsulation { private: KeyEncapsulationDetails alg_details_{}; ///< KEM algorithm details + // Keep secret storage last: default copying must finish all potentially + // throwing metadata copies before copying any secret bytes. + bytes secret_key_{}; ///< secret key + + void swap(KeyEncapsulation& other) noexcept { + kem_.swap(other.kem_); + using std::swap; + swap(alg_details_, other.alg_details_); + secret_key_.swap(other.secret_key_); + } public: /** @@ -194,7 +224,7 @@ class KeyEncapsulation { */ explicit KeyEncapsulation(const std::string& alg_name, bytes secret_key = {}) - : secret_key_{std::move(secret_key)} { + try : secret_key_{} { // KEM not enabled if (!KEMs::is_KEM_enabled(alg_name)) { // Perhaps it's supported @@ -206,6 +236,8 @@ class KeyEncapsulation { kem_.reset(C::OQS_KEM_new(alg_name.c_str()), [](C::OQS_KEM* p) { C::OQS_KEM_free(p); }); + if (!kem_) + throw std::bad_alloc{}; alg_details_.name = kem_->method_name; alg_details_.version = kem_->alg_version; @@ -215,6 +247,11 @@ class KeyEncapsulation { alg_details_.length_secret_key = kem_->length_secret_key; alg_details_.length_ciphertext = kem_->length_ciphertext; alg_details_.length_shared_secret = kem_->length_shared_secret; + + secret_key_.swap(secret_key); + } catch (...) { + cleanse_secret_key(secret_key); + throw; } /** @@ -223,51 +260,47 @@ class KeyEncapsulation { KeyEncapsulation(const KeyEncapsulation&) = default; /** - * \brief Default copy assignment operator + * \brief Copy assignment operator * \return Reference to the current instance */ - KeyEncapsulation& operator=(const KeyEncapsulation&) = default; + KeyEncapsulation& operator=(const KeyEncapsulation& rhs) { + if (this != &rhs) { + KeyEncapsulation replacement{rhs}; + swap(replacement); + // replacement now owns and cleanses our previous secret key. + } + return *this; + } /** - * \brief Move constructor, guarantees that the rvalue secret key is always - * zeroed - * \param rhs oqs::KeyEncapsulation instance + * \brief Move constructor; transfers key ownership without allocation. + * The source no longer owns any secret bytes after the move. + * \param rhs KeyEncapsulation instance */ KeyEncapsulation(KeyEncapsulation&& rhs) noexcept - : kem_{std::move(rhs.kem_)}, alg_details_{std::move(rhs.alg_details_)} { - // Paranoid move via copy/clean/resize, see - // https://stackoverflow.com/questions/55054187/can-i-resize-a-vector-that-was-moved-from - secret_key_ = rhs.secret_key_; // copy - // Clean (zero) - C::OQS_MEM_cleanse(rhs.secret_key_.data(), rhs.secret_key_.size()); - rhs.secret_key_.resize(0); // resize + : kem_{std::move(rhs.kem_)}, + alg_details_{std::move(rhs.alg_details_)} { + secret_key_.swap(rhs.secret_key_); } + /** - * \brief Move assignment operator, guarantees that the rvalue secret key is - * always zeroed - * \param rhs oqs::KeyEncapsulation instance + * \brief Move assignment; cleanses the replaced key and transfers ownership. + * \param rhs KeyEncapsulation instance * \return Reference to the current instance */ KeyEncapsulation& operator=(KeyEncapsulation&& rhs) noexcept { - kem_ = std::move(rhs.kem_); - alg_details_ = std::move(rhs.alg_details_); - - // Paranoid move via copy/clean/resize, see - // https://stackoverflow.com/questions/55054187/can-i-resize-a-vector-that-was-moved-from - secret_key_ = rhs.secret_key_; // copy - // Clean (zero) - C::OQS_MEM_cleanse(rhs.secret_key_.data(), rhs.secret_key_.size()); - rhs.secret_key_.resize(0); // resize - + if (this != &rhs) { + KeyEncapsulation replacement{std::move(rhs)}; + swap(replacement); + } return *this; } /** - * \brief Virtual default destructor + * \brief Virtual destructor; cleanses the owned secret key */ virtual ~KeyEncapsulation() { - if (!secret_key_.empty()) - C::OQS_MEM_cleanse(secret_key_.data(), secret_key_.size()); + cleanse_secret_key(secret_key_); } /** @@ -288,13 +321,16 @@ class KeyEncapsulation { */ bytes generate_keypair() { bytes public_key(alg_details_.length_public_key, 0); - secret_key_ = bytes(alg_details_.length_secret_key, 0); + bytes new_secret_key(alg_details_.length_secret_key, 0); + internal::SecretKeyGuard guard{new_secret_key}; - OQS_STATUS rv_ = C::OQS_KEM_keypair(kem_.get(), public_key.data(), - secret_key_.data()); + OQS_STATUS rv_ = C::OQS_KEM_keypair( + kem_.get(), public_key.data(), new_secret_key.data()); if (rv_ != OQS_STATUS::OQS_SUCCESS) throw std::runtime_error("Can not generate keypair"); + // Commit only after success. The guard then cleanses the old key. + secret_key_.swap(new_secret_key); return public_key; } @@ -481,10 +517,12 @@ class Sigs final : public internal::Singleton { * \brief Signature mechanisms */ class Signature { - std::shared_ptr sig_{nullptr, [](C::OQS_SIG* p) { - C::OQS_SIG_free(p); - }}; ///< liboqs smart pointer to C::OQS_SIG - bytes secret_key_{}; ///< secret key + std::shared_ptr sig_{}; ///< liboqs object + + static void cleanse_secret_key(bytes& secret_key) noexcept { + if (!secret_key.empty()) + C::OQS_MEM_cleanse(secret_key.data(), secret_key.size()); + } public: /** @@ -503,6 +541,16 @@ class Signature { private: SignatureDetails alg_details_{}; ///< Signature algorithm details + // Keep secret storage last: default copying must finish all potentially + // throwing metadata copies before copying any secret bytes. + bytes secret_key_{}; ///< secret key + + void swap(Signature& other) noexcept { + sig_.swap(other.sig_); + using std::swap; + swap(alg_details_, other.alg_details_); + secret_key_.swap(other.secret_key_); + } public: /** @@ -511,7 +559,7 @@ class Signature { * \param secret_key Secret key (optional) */ explicit Signature(const std::string& alg_name, bytes secret_key = {}) - : secret_key_{std::move(secret_key)} { + try : secret_key_{} { // signature not enabled if (!Sigs::is_sig_enabled(alg_name)) { // perhaps it's supported @@ -523,6 +571,8 @@ class Signature { sig_.reset(C::OQS_SIG_new(alg_name.c_str()), [](C::OQS_SIG* p) { C::OQS_SIG_free(p); }); + if (!sig_) + throw std::bad_alloc{}; alg_details_.name = sig_->method_name; alg_details_.version = sig_->alg_version; @@ -532,6 +582,11 @@ class Signature { alg_details_.length_public_key = sig_->length_public_key; alg_details_.length_secret_key = sig_->length_secret_key; alg_details_.max_length_signature = sig_->length_signature; + + secret_key_.swap(secret_key); + } catch (...) { + cleanse_secret_key(secret_key); + throw; } /** @@ -540,51 +595,47 @@ class Signature { Signature(const Signature&) = default; /** - * \brief Default copy assignment operator + * \brief Copy assignment operator * \return Reference to the current instance */ - Signature& operator=(const Signature&) = default; + Signature& operator=(const Signature& rhs) { + if (this != &rhs) { + Signature replacement{rhs}; + swap(replacement); + // replacement now owns and cleanses our previous secret key. + } + return *this; + } /** - * \brief Move constructor, guarantees that the rvalue secret key is always - * zeroed - * \param rhs oqs::Signature instance + * \brief Move constructor; transfers key ownership without allocation. + * The source no longer owns any secret bytes after the move. + * \param rhs Signature instance */ Signature(Signature&& rhs) noexcept - : sig_{std::move(rhs.sig_)}, alg_details_{std::move(rhs.alg_details_)} { - // Paranoid move via copy/clean/resize, see - // https://stackoverflow.com/questions/55054187/can-i-resize-a-vector-that-was-moved-from - secret_key_ = rhs.secret_key_; // copy - // Clean (zero) - C::OQS_MEM_cleanse(rhs.secret_key_.data(), rhs.secret_key_.size()); - rhs.secret_key_.resize(0); // resize + : sig_{std::move(rhs.sig_)}, + alg_details_{std::move(rhs.alg_details_)} { + secret_key_.swap(rhs.secret_key_); } + /** - * \brief Move assignment operator, guarantees that the rvalue secret key is - * always zeroed - * \param rhs oqs::Signature instance + * \brief Move assignment; cleanses the replaced key and transfers ownership. + * \param rhs Signature instance * \return Reference to the current instance */ Signature& operator=(Signature&& rhs) noexcept { - sig_ = std::move(rhs.sig_); - alg_details_ = std::move(rhs.alg_details_); - - // Paranoid move via copy/clean/resize, see - // https://stackoverflow.com/questions/55054187/can-i-resize-a-vector-that-was-moved-from - secret_key_ = rhs.secret_key_; // copy - // Clean (zero) - C::OQS_MEM_cleanse(rhs.secret_key_.data(), rhs.secret_key_.size()); - rhs.secret_key_.resize(0); // resize - + if (this != &rhs) { + Signature replacement{std::move(rhs)}; + swap(replacement); + } return *this; } /** - * \brief Virtual default destructor + * \brief Virtual destructor; cleanses the owned secret key */ virtual ~Signature() { - if (!secret_key_.empty()) - C::OQS_MEM_cleanse(secret_key_.data(), secret_key_.size()); + cleanse_secret_key(secret_key_); } /** @@ -604,14 +655,17 @@ class Signature { * \return Public key */ bytes generate_keypair() { - bytes public_key(get_details().length_public_key, 0); - secret_key_ = bytes(alg_details_.length_secret_key, 0); + bytes public_key(alg_details_.length_public_key, 0); + bytes new_secret_key(alg_details_.length_secret_key, 0); + internal::SecretKeyGuard guard{new_secret_key}; - OQS_STATUS rv_ = C::OQS_SIG_keypair(sig_.get(), public_key.data(), - secret_key_.data()); + OQS_STATUS rv_ = C::OQS_SIG_keypair( + sig_.get(), public_key.data(), new_secret_key.data()); if (rv_ != OQS_STATUS::OQS_SUCCESS) throw std::runtime_error("Can not generate keypair"); + // Commit only after success. The guard then cleanses the old key. + secret_key_.swap(new_secret_key); return public_key; } diff --git a/unit_tests/tests/test_kem.cpp b/unit_tests/tests/test_kem.cpp index 4b3c028..f21f0f8 100644 --- a/unit_tests/tests/test_kem.cpp +++ b/unit_tests/tests/test_kem.cpp @@ -1,9 +1,11 @@ // Unit testing oqs::KeyEncapsulation +#include #include #include #include #include +#include #include #include @@ -132,3 +134,138 @@ TEST(oqs_KeyEncapsulation, NotSupported) { EXPECT_THROW(oqs::KeyEncapsulation{"unsupported_kem"}, oqs::MechanismNotSupportedError); } + +namespace { +// Exported key copies belong to the caller. Clean up these test-owned copies, +// including when an assertion returns early or an operation throws. +class TestSecret { + oqs::bytes value_; + + public: + explicit TestSecret(oqs::bytes value) noexcept { + value_.swap(value); + } + TestSecret(const TestSecret&) = delete; + TestSecret& operator=(const TestSecret&) = delete; + ~TestSecret() { + if (!value_.empty()) + oqs::C::OQS_MEM_cleanse(value_.data(), value_.size()); + } + const oqs::bytes& get() const { return value_; } +}; +} // namespace + +namespace { +void expect_kem_key_works(const oqs::KeyEncapsulation& key, + const oqs::bytes& public_key) { + oqs::KeyEncapsulation peer{key.get_details().name}; + auto encapsulated = peer.encap_secret(public_key); + TestSecret expected{std::move(encapsulated.second)}; + TestSecret actual{key.decap_secret(encapsulated.first)}; + EXPECT_EQ(actual.get(), expected.get()); +} +} // namespace + +TEST(oqs_KeyEncapsulation, NotSupportedWithSecretKey) { + // This checks the exception and preservation of a caller-owned lvalue. + // It does not inspect the constructor's discarded secret allocation. + const TestSecret caller_key{oqs::bytes(32, 0xa5)}; + EXPECT_THROW((oqs::KeyEncapsulation{"unsupported_kem", caller_key.get()}), + oqs::MechanismNotSupportedError); + EXPECT_TRUE(std::all_of(caller_key.get().begin(), caller_key.get().end(), + [](unsigned char byte) { return byte == 0xa5; })); +} + +TEST(oqs_KeyEncapsulation, SecretKeyLifecycle) { + const auto& enabled = oqs::KEMs::get_enabled_KEMs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled KEM algorithms"; + + // Prefer a fast standard algorithm for repeated lifecycle operations. + // Run this test on the main thread, including the fallback algorithm. + const auto preferred = std::find(enabled.begin(), enabled.end(), + "ML-KEM-768"); + const auto& alg = preferred != enabled.end() ? *preferred : enabled.front(); + SCOPED_TRACE(alg); + + oqs::KeyEncapsulation source{alg}; + const auto public_key = source.generate_keypair(); + const TestSecret original_key{source.export_secret_key()}; + ASSERT_EQ(original_key.get().size(), source.get_details().length_secret_key); + + // Copy construction preserves the source and yields a usable key. + oqs::KeyEncapsulation copied{source}; + EXPECT_EQ(TestSecret{copied.export_secret_key()}.get(), original_key.get()); + expect_kem_key_works(copied, public_key); + + // Exercise replacement across algorithms when the alternative is enabled. + const auto alternative = std::find(enabled.begin(), enabled.end(), + "ML-KEM-512"); + const auto& target_alg = alternative != enabled.end() ? *alternative : alg; + oqs::KeyEncapsulation target{target_alg}; + target.generate_keypair(); + target = source; + EXPECT_EQ(TestSecret{target.export_secret_key()}.get(), original_key.get()); + EXPECT_EQ(target.get_details().name, source.get_details().name); + expect_kem_key_works(target, public_key); + + // Use an alias to exercise self-assignment without self-move warnings. + auto& same_target = target; + target = same_target; + target = std::move(same_target); + EXPECT_EQ(TestSecret{target.export_secret_key()}.get(), original_key.get()); + expect_kem_key_works(target, public_key); + + oqs::KeyEncapsulation moved{std::move(copied)}; + EXPECT_TRUE(TestSecret{copied.export_secret_key()}.get().empty()); + EXPECT_EQ(TestSecret{moved.export_secret_key()}.get(), original_key.get()); + expect_kem_key_works(moved, public_key); + + // Replace a different, already initialized key via move assignment. + target = oqs::KeyEncapsulation{target_alg}; + target.generate_keypair(); + target = std::move(moved); + EXPECT_TRUE(TestSecret{moved.export_secret_key()}.get().empty()); + EXPECT_EQ(TestSecret{target.export_secret_key()}.get(), original_key.get()); + expect_kem_key_works(target, public_key); + + const auto regenerated_public_key = target.generate_keypair(); + EXPECT_EQ(TestSecret{target.export_secret_key()}.get().size(), + target.get_details().length_secret_key); + expect_kem_key_works(target, regenerated_public_key); + + // Regenerating the destination must not change the source's copied key. + EXPECT_EQ(TestSecret{source.export_secret_key()}.get(), original_key.get()); + expect_kem_key_works(source, public_key); + + // Moved-from wrappers remain assignable. + copied = source; + expect_kem_key_works(copied, public_key); +} + +TEST(oqs_KeyEncapsulation, EmptySecretKeyAssignment) { + const auto& enabled = oqs::KEMs::get_enabled_KEMs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled KEM algorithms"; + const auto preferred = std::find(enabled.begin(), enabled.end(), + "ML-KEM-768"); + const auto& alg = preferred != enabled.end() ? *preferred : enabled.front(); + SCOPED_TRACE(alg); + + oqs::KeyEncapsulation empty{alg}; + oqs::KeyEncapsulation copied_empty{empty}; + EXPECT_TRUE(TestSecret{copied_empty.export_secret_key()}.get().empty()); + + oqs::KeyEncapsulation target{alg}; + target.generate_keypair(); + target = empty; + EXPECT_TRUE(TestSecret{target.export_secret_key()}.get().empty()); + EXPECT_EQ(target.get_details().name, alg); + + target.generate_keypair(); + target = std::move(copied_empty); + EXPECT_TRUE(TestSecret{target.export_secret_key()}.get().empty()); + EXPECT_TRUE(TestSecret{copied_empty.export_secret_key()}.get().empty()); + const auto public_key = target.generate_keypair(); + expect_kem_key_works(target, public_key); +} diff --git a/unit_tests/tests/test_sig.cpp b/unit_tests/tests/test_sig.cpp index 966587c..5a77eb7 100644 --- a/unit_tests/tests/test_sig.cpp +++ b/unit_tests/tests/test_sig.cpp @@ -1,8 +1,10 @@ // Unit testing oqs::Signature +#include #include #include #include +#include #include #include @@ -247,3 +249,137 @@ TEST(oqs_Signature, NotSupported) { EXPECT_THROW(oqs::Signature{"unsupported_sig"}, oqs::MechanismNotSupportedError); } + +namespace { +// Exported key copies belong to the caller. Clean up these test-owned copies, +// including when an assertion returns early or an operation throws. +class TestSecret { + oqs::bytes value_; + + public: + explicit TestSecret(oqs::bytes value) noexcept { + value_.swap(value); + } + TestSecret(const TestSecret&) = delete; + TestSecret& operator=(const TestSecret&) = delete; + ~TestSecret() { + if (!value_.empty()) + oqs::C::OQS_MEM_cleanse(value_.data(), value_.size()); + } + const oqs::bytes& get() const { return value_; } +}; +} // namespace + +namespace { +void expect_sig_key_works(const oqs::Signature& key, + const oqs::bytes& public_key) { + const auto message = test_message(key.get_details().name); + const auto signature = key.sign(message); + oqs::Signature verifier{key.get_details().name}; + EXPECT_TRUE(verifier.verify(message, signature, public_key)); +} +} // namespace + +TEST(oqs_Signature, NotSupportedWithSecretKey) { + // This checks the exception and preservation of a caller-owned lvalue. + // It does not inspect the constructor's discarded secret allocation. + const TestSecret caller_key{oqs::bytes(32, 0xa5)}; + EXPECT_THROW((oqs::Signature{"unsupported_sig", caller_key.get()}), + oqs::MechanismNotSupportedError); + EXPECT_TRUE(std::all_of(caller_key.get().begin(), caller_key.get().end(), + [](unsigned char byte) { return byte == 0xa5; })); +} + +TEST(oqs_Signature, SecretKeyLifecycle) { + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled signature algorithms"; + + // Prefer a fast standard algorithm for repeated lifecycle operations. + // Run this test on the main thread, including the fallback algorithm. + const auto preferred = std::find(enabled.begin(), enabled.end(), + "ML-DSA-65"); + const auto& alg = preferred != enabled.end() ? *preferred : enabled.front(); + SCOPED_TRACE(alg); + + oqs::Signature source{alg}; + const auto public_key = source.generate_keypair(); + const TestSecret original_key{source.export_secret_key()}; + ASSERT_EQ(original_key.get().size(), source.get_details().length_secret_key); + + // Copy construction preserves the source and yields a usable key. + oqs::Signature copied{source}; + EXPECT_EQ(TestSecret{copied.export_secret_key()}.get(), original_key.get()); + expect_sig_key_works(copied, public_key); + + // Exercise replacement across algorithms when the alternative is enabled. + const auto alternative = std::find(enabled.begin(), enabled.end(), + "ML-DSA-44"); + const auto& target_alg = alternative != enabled.end() ? *alternative : alg; + oqs::Signature target{target_alg}; + target.generate_keypair(); + target = source; + EXPECT_EQ(TestSecret{target.export_secret_key()}.get(), original_key.get()); + EXPECT_EQ(target.get_details().name, source.get_details().name); + expect_sig_key_works(target, public_key); + + // Use an alias to exercise self-assignment without self-move warnings. + auto& same_target = target; + target = same_target; + target = std::move(same_target); + EXPECT_EQ(TestSecret{target.export_secret_key()}.get(), original_key.get()); + expect_sig_key_works(target, public_key); + + oqs::Signature moved{std::move(copied)}; + EXPECT_TRUE(TestSecret{copied.export_secret_key()}.get().empty()); + EXPECT_EQ(TestSecret{moved.export_secret_key()}.get(), original_key.get()); + expect_sig_key_works(moved, public_key); + + // Replace a different, already initialized key via move assignment. + target = oqs::Signature{target_alg}; + target.generate_keypair(); + target = std::move(moved); + EXPECT_TRUE(TestSecret{moved.export_secret_key()}.get().empty()); + EXPECT_EQ(TestSecret{target.export_secret_key()}.get(), original_key.get()); + expect_sig_key_works(target, public_key); + + const auto regenerated_public_key = target.generate_keypair(); + EXPECT_EQ(TestSecret{target.export_secret_key()}.get().size(), + target.get_details().length_secret_key); + expect_sig_key_works(target, regenerated_public_key); + + // Regenerating the destination must not change the source's copied key. + EXPECT_EQ(TestSecret{source.export_secret_key()}.get(), original_key.get()); + expect_sig_key_works(source, public_key); + + // Moved-from wrappers remain assignable. + copied = source; + expect_sig_key_works(copied, public_key); +} + +TEST(oqs_Signature, EmptySecretKeyAssignment) { + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled signature algorithms"; + const auto preferred = std::find(enabled.begin(), enabled.end(), + "ML-DSA-65"); + const auto& alg = preferred != enabled.end() ? *preferred : enabled.front(); + SCOPED_TRACE(alg); + + oqs::Signature empty{alg}; + oqs::Signature copied_empty{empty}; + EXPECT_TRUE(TestSecret{copied_empty.export_secret_key()}.get().empty()); + + oqs::Signature target{alg}; + target.generate_keypair(); + target = empty; + EXPECT_TRUE(TestSecret{target.export_secret_key()}.get().empty()); + EXPECT_EQ(target.get_details().name, alg); + + target.generate_keypair(); + target = std::move(copied_empty); + EXPECT_TRUE(TestSecret{target.export_secret_key()}.get().empty()); + EXPECT_TRUE(TestSecret{copied_empty.export_secret_key()}.get().empty()); + const auto public_key = target.generate_keypair(); + expect_sig_key_works(target, public_key); +}