From 873b9f51eaced03edcda78705bf1f4692ed9fe88 Mon Sep 17 00:00:00 2001 From: AKG451 <217809809+AKG451@users.noreply.github.com> Date: Tue, 3 Nov 2026 04:05:09 +0000 Subject: [PATCH 1/6] Cleanse secret keys before replacement Secret keys were moved into wrapper objects before algorithm validation, so constructor exceptions could leave caller-provided key material uncleansed. Existing keys were also overwritten during assignment and key regeneration without cleansing the old bytes first.\n\nRetain constructor ownership until validation and metadata initialization complete, cleanse keys before copy/move assignment and regeneration, and add lifecycle coverage for both KEM and signature wrappers. Signed-off-by: AKG451 <217809809+AKG451@users.noreply.github.com> --- include/oqs_cpp.hpp | 147 +++++++++++++++++++++++----------- unit_tests/tests/test_kem.cpp | 23 ++++++ unit_tests/tests/test_sig.cpp | 23 ++++++ 3 files changed, 145 insertions(+), 48 deletions(-) diff --git a/include/oqs_cpp.hpp b/include/oqs_cpp.hpp index 6927967..61bbb60 100644 --- a/include/oqs_cpp.hpp +++ b/include/oqs_cpp.hpp @@ -168,6 +168,12 @@ class KeyEncapsulation { C::OQS_KEM_free(p); }}; ///< liboqs smart pointer to C::OQS_KEM bytes secret_key_{}; ///< secret key + + static void cleanse_secret_key(bytes& secret_key) { + if (!secret_key.empty()) + C::OQS_MEM_cleanse(secret_key.data(), secret_key.size()); + } + public: /** * \brief KEM algorithm details @@ -194,27 +200,34 @@ class KeyEncapsulation { */ explicit KeyEncapsulation(const std::string& alg_name, bytes secret_key = {}) - : secret_key_{std::move(secret_key)} { - // KEM not enabled - if (!KEMs::is_KEM_enabled(alg_name)) { - // Perhaps it's supported - if (KEMs::is_KEM_supported(alg_name)) - throw MechanismNotEnabledError(alg_name); - else - throw MechanismNotSupportedError(alg_name); + : secret_key_{} { + try { + // KEM not enabled + if (!KEMs::is_KEM_enabled(alg_name)) { + // Perhaps it's supported + if (KEMs::is_KEM_supported(alg_name)) + throw MechanismNotEnabledError(alg_name); + else + throw MechanismNotSupportedError(alg_name); + } + + kem_.reset(C::OQS_KEM_new(alg_name.c_str()), + [](C::OQS_KEM* p) { C::OQS_KEM_free(p); }); + + alg_details_.name = kem_->method_name; + alg_details_.version = kem_->alg_version; + alg_details_.claimed_nist_level = kem_->claimed_nist_level; + alg_details_.is_ind_cca = kem_->ind_cca; + alg_details_.length_public_key = kem_->length_public_key; + 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_ = std::move(secret_key); + } catch (...) { + cleanse_secret_key(secret_key); + throw; } - - kem_.reset(C::OQS_KEM_new(alg_name.c_str()), - [](C::OQS_KEM* p) { C::OQS_KEM_free(p); }); - - alg_details_.name = kem_->method_name; - alg_details_.version = kem_->alg_version; - alg_details_.claimed_nist_level = kem_->claimed_nist_level; - alg_details_.is_ind_cca = kem_->ind_cca; - alg_details_.length_public_key = kem_->length_public_key; - 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; } /** @@ -223,10 +236,19 @@ 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) { + bytes copied_secret_key = rhs.secret_key_; + alg_details_ = rhs.alg_details_; + kem_ = rhs.kem_; + cleanse_secret_key(secret_key_); + secret_key_ = std::move(copied_secret_key); + } + return *this; + } /** * \brief Move constructor, guarantees that the rvalue secret key is always @@ -249,6 +271,10 @@ class KeyEncapsulation { * \return Reference to the current instance */ KeyEncapsulation& operator=(KeyEncapsulation&& rhs) noexcept { + if (this == &rhs) + return *this; + + cleanse_secret_key(secret_key_); kem_ = std::move(rhs.kem_); alg_details_ = std::move(rhs.alg_details_); @@ -266,8 +292,7 @@ class KeyEncapsulation { * \brief Virtual default destructor */ virtual ~KeyEncapsulation() { - if (!secret_key_.empty()) - C::OQS_MEM_cleanse(secret_key_.data(), secret_key_.size()); + cleanse_secret_key(secret_key_); } /** @@ -288,6 +313,7 @@ class KeyEncapsulation { */ bytes generate_keypair() { bytes public_key(alg_details_.length_public_key, 0); + cleanse_secret_key(secret_key_); secret_key_ = bytes(alg_details_.length_secret_key, 0); OQS_STATUS rv_ = C::OQS_KEM_keypair(kem_.get(), public_key.data(), @@ -486,6 +512,11 @@ class Signature { }}; ///< liboqs smart pointer to C::OQS_SIG bytes secret_key_{}; ///< secret key + static void cleanse_secret_key(bytes& secret_key) { + if (!secret_key.empty()) + C::OQS_MEM_cleanse(secret_key.data(), secret_key.size()); + } + public: /** * \brief Signature algorithm details @@ -511,27 +542,34 @@ class Signature { * \param secret_key Secret key (optional) */ explicit Signature(const std::string& alg_name, bytes secret_key = {}) - : secret_key_{std::move(secret_key)} { - // signature not enabled - if (!Sigs::is_sig_enabled(alg_name)) { - // perhaps it's supported - if (Sigs::is_sig_supported(alg_name)) - throw MechanismNotEnabledError(alg_name); - else - throw MechanismNotSupportedError(alg_name); + : secret_key_{} { + try { + // signature not enabled + if (!Sigs::is_sig_enabled(alg_name)) { + // perhaps it's supported + if (Sigs::is_sig_supported(alg_name)) + throw MechanismNotEnabledError(alg_name); + else + throw MechanismNotSupportedError(alg_name); + } + + sig_.reset(C::OQS_SIG_new(alg_name.c_str()), + [](C::OQS_SIG* p) { C::OQS_SIG_free(p); }); + + alg_details_.name = sig_->method_name; + alg_details_.version = sig_->alg_version; + alg_details_.claimed_nist_level = sig_->claimed_nist_level; + alg_details_.is_euf_cma = sig_->euf_cma; + alg_details_.sig_with_ctx_support = sig_->sig_with_ctx_support; + 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_ = std::move(secret_key); + } catch (...) { + cleanse_secret_key(secret_key); + throw; } - - sig_.reset(C::OQS_SIG_new(alg_name.c_str()), - [](C::OQS_SIG* p) { C::OQS_SIG_free(p); }); - - alg_details_.name = sig_->method_name; - alg_details_.version = sig_->alg_version; - alg_details_.claimed_nist_level = sig_->claimed_nist_level; - alg_details_.is_euf_cma = sig_->euf_cma; - alg_details_.sig_with_ctx_support = sig_->sig_with_ctx_support; - 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; } /** @@ -540,10 +578,19 @@ 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) { + bytes copied_secret_key = rhs.secret_key_; + alg_details_ = rhs.alg_details_; + sig_ = rhs.sig_; + cleanse_secret_key(secret_key_); + secret_key_ = std::move(copied_secret_key); + } + return *this; + } /** * \brief Move constructor, guarantees that the rvalue secret key is always @@ -566,6 +613,10 @@ class Signature { * \return Reference to the current instance */ Signature& operator=(Signature&& rhs) noexcept { + if (this == &rhs) + return *this; + + cleanse_secret_key(secret_key_); sig_ = std::move(rhs.sig_); alg_details_ = std::move(rhs.alg_details_); @@ -583,8 +634,7 @@ class Signature { * \brief Virtual default destructor */ virtual ~Signature() { - if (!secret_key_.empty()) - C::OQS_MEM_cleanse(secret_key_.data(), secret_key_.size()); + cleanse_secret_key(secret_key_); } /** @@ -605,6 +655,7 @@ class Signature { */ bytes generate_keypair() { bytes public_key(get_details().length_public_key, 0); + cleanse_secret_key(secret_key_); secret_key_ = bytes(alg_details_.length_secret_key, 0); OQS_STATUS rv_ = C::OQS_SIG_keypair(sig_.get(), public_key.data(), diff --git a/unit_tests/tests/test_kem.cpp b/unit_tests/tests/test_kem.cpp index 4b3c028..11a2f1c 100644 --- a/unit_tests/tests/test_kem.cpp +++ b/unit_tests/tests/test_kem.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -132,3 +133,25 @@ TEST(oqs_KeyEncapsulation, NotSupported) { EXPECT_THROW(oqs::KeyEncapsulation{"unsupported_kem"}, oqs::MechanismNotSupportedError); } + +TEST(oqs_KeyEncapsulation, SecretKeyLifecycle) { + const auto enabled_kems = oqs::KEMs::get_enabled_KEMs(); + if (enabled_kems.empty()) + return; + + oqs::KeyEncapsulation source{enabled_kems.front()}; + source.generate_keypair(); + oqs::KeyEncapsulation target{enabled_kems.front()}; + target.generate_keypair(); + + target = source; + EXPECT_EQ(target.export_secret_key(), source.export_secret_key()); + + target = std::move(source); + EXPECT_EQ(target.export_secret_key().size(), + target.get_details().length_secret_key); + + target.generate_keypair(); + EXPECT_EQ(target.export_secret_key().size(), + target.get_details().length_secret_key); +} diff --git a/unit_tests/tests/test_sig.cpp b/unit_tests/tests/test_sig.cpp index 966587c..f4d40a1 100644 --- a/unit_tests/tests/test_sig.cpp +++ b/unit_tests/tests/test_sig.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -247,3 +248,25 @@ TEST(oqs_Signature, NotSupported) { EXPECT_THROW(oqs::Signature{"unsupported_sig"}, oqs::MechanismNotSupportedError); } + +TEST(oqs_Signature, SecretKeyLifecycle) { + const auto enabled_sigs = oqs::Sigs::get_enabled_sigs(); + if (enabled_sigs.empty()) + return; + + oqs::Signature source{enabled_sigs.front()}; + source.generate_keypair(); + oqs::Signature target{enabled_sigs.front()}; + target.generate_keypair(); + + target = source; + EXPECT_EQ(target.export_secret_key(), source.export_secret_key()); + + target = std::move(source); + EXPECT_EQ(target.export_secret_key().size(), + target.get_details().length_secret_key); + + target.generate_keypair(); + EXPECT_EQ(target.export_secret_key().size(), + target.get_details().length_secret_key); +} From 0e80fcd896a13c5b348c098baf4910221341a0ac Mon Sep 17 00:00:00 2001 From: "Aryan K. Gupta" Date: Sat, 7 Nov 2026 03:30:25 +0530 Subject: [PATCH 2/6] Refactor secret key handling in KeyEncapsulation and Signature Refactor secret key management for KeyEncapsulation and Signature classes. Introduce SecretKeyGuard for automatic cleansing of secret keys upon destruction. Signed-off-by: Aryan K. Gupta --- include/oqs_cpp.hpp | 281 ++++++++++++++++++++++---------------------- 1 file changed, 142 insertions(+), 139 deletions(-) diff --git a/include/oqs_cpp.hpp b/include/oqs_cpp.hpp index 61bbb60..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,12 +181,9 @@ 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) { + static void cleanse_secret_key(bytes& secret_key) noexcept { if (!secret_key.empty()) C::OQS_MEM_cleanse(secret_key.data(), secret_key.size()); } @@ -191,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: /** @@ -200,34 +224,34 @@ class KeyEncapsulation { */ explicit KeyEncapsulation(const std::string& alg_name, bytes secret_key = {}) - : secret_key_{} { - try { - // KEM not enabled - if (!KEMs::is_KEM_enabled(alg_name)) { - // Perhaps it's supported - if (KEMs::is_KEM_supported(alg_name)) - throw MechanismNotEnabledError(alg_name); - else - throw MechanismNotSupportedError(alg_name); - } - - kem_.reset(C::OQS_KEM_new(alg_name.c_str()), - [](C::OQS_KEM* p) { C::OQS_KEM_free(p); }); - - alg_details_.name = kem_->method_name; - alg_details_.version = kem_->alg_version; - alg_details_.claimed_nist_level = kem_->claimed_nist_level; - alg_details_.is_ind_cca = kem_->ind_cca; - alg_details_.length_public_key = kem_->length_public_key; - 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_ = std::move(secret_key); - } catch (...) { - cleanse_secret_key(secret_key); - throw; + try : secret_key_{} { + // KEM not enabled + if (!KEMs::is_KEM_enabled(alg_name)) { + // Perhaps it's supported + if (KEMs::is_KEM_supported(alg_name)) + throw MechanismNotEnabledError(alg_name); + else + throw MechanismNotSupportedError(alg_name); } + + 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; + alg_details_.claimed_nist_level = kem_->claimed_nist_level; + alg_details_.is_ind_cca = kem_->ind_cca; + alg_details_.length_public_key = kem_->length_public_key; + 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; } /** @@ -241,55 +265,39 @@ class KeyEncapsulation { */ KeyEncapsulation& operator=(const KeyEncapsulation& rhs) { if (this != &rhs) { - bytes copied_secret_key = rhs.secret_key_; - alg_details_ = rhs.alg_details_; - kem_ = rhs.kem_; - cleanse_secret_key(secret_key_); - secret_key_ = std::move(copied_secret_key); + 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 { - if (this == &rhs) - return *this; - - cleanse_secret_key(secret_key_); - 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() { cleanse_secret_key(secret_key_); @@ -313,14 +321,16 @@ class KeyEncapsulation { */ bytes generate_keypair() { bytes public_key(alg_details_.length_public_key, 0); - cleanse_secret_key(secret_key_); - 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; } @@ -507,12 +517,9 @@ 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) { + static void cleanse_secret_key(bytes& secret_key) noexcept { if (!secret_key.empty()) C::OQS_MEM_cleanse(secret_key.data(), secret_key.size()); } @@ -534,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: /** @@ -542,34 +559,34 @@ class Signature { * \param secret_key Secret key (optional) */ explicit Signature(const std::string& alg_name, bytes secret_key = {}) - : secret_key_{} { - try { - // signature not enabled - if (!Sigs::is_sig_enabled(alg_name)) { - // perhaps it's supported - if (Sigs::is_sig_supported(alg_name)) - throw MechanismNotEnabledError(alg_name); - else - throw MechanismNotSupportedError(alg_name); - } - - sig_.reset(C::OQS_SIG_new(alg_name.c_str()), - [](C::OQS_SIG* p) { C::OQS_SIG_free(p); }); - - alg_details_.name = sig_->method_name; - alg_details_.version = sig_->alg_version; - alg_details_.claimed_nist_level = sig_->claimed_nist_level; - alg_details_.is_euf_cma = sig_->euf_cma; - alg_details_.sig_with_ctx_support = sig_->sig_with_ctx_support; - 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_ = std::move(secret_key); - } catch (...) { - cleanse_secret_key(secret_key); - throw; + try : secret_key_{} { + // signature not enabled + if (!Sigs::is_sig_enabled(alg_name)) { + // perhaps it's supported + if (Sigs::is_sig_supported(alg_name)) + throw MechanismNotEnabledError(alg_name); + else + throw MechanismNotSupportedError(alg_name); } + + 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; + alg_details_.claimed_nist_level = sig_->claimed_nist_level; + alg_details_.is_euf_cma = sig_->euf_cma; + alg_details_.sig_with_ctx_support = sig_->sig_with_ctx_support; + 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; } /** @@ -583,55 +600,39 @@ class Signature { */ Signature& operator=(const Signature& rhs) { if (this != &rhs) { - bytes copied_secret_key = rhs.secret_key_; - alg_details_ = rhs.alg_details_; - sig_ = rhs.sig_; - cleanse_secret_key(secret_key_); - secret_key_ = std::move(copied_secret_key); + 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 { - if (this == &rhs) - return *this; - - cleanse_secret_key(secret_key_); - 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() { cleanse_secret_key(secret_key_); @@ -654,15 +655,17 @@ class Signature { * \return Public key */ bytes generate_keypair() { - bytes public_key(get_details().length_public_key, 0); - cleanse_secret_key(secret_key_); - 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; } From fbc376ea66a01f162ab831e9cf54b5f8bd75dd63 Mon Sep 17 00:00:00 2001 From: "Aryan K. Gupta" Date: Sat, 7 Nov 2026 03:34:00 +0530 Subject: [PATCH 3/6] Refactor KEM unit tests for better memory handling Refactor KEM tests to use TestSecret for memory management and improve thread safety. Signed-off-by: Aryan K. Gupta --- unit_tests/tests/test_kem.cpp | 254 +++++++++++++++++++++++++--------- 1 file changed, 185 insertions(+), 69 deletions(-) diff --git a/unit_tests/tests/test_kem.cpp b/unit_tests/tests/test_kem.cpp index 11a2f1c..fa7b7fd 100644 --- a/unit_tests/tests/test_kem.cpp +++ b/unit_tests/tests/test_kem.cpp @@ -1,5 +1,7 @@ // Unit testing oqs::KeyEncapsulation +#include +#include #include #include #include @@ -12,14 +14,31 @@ #include "oqs_cpp.hpp" #include "rand/rand.hpp" -// no_thread_KEM_patterns lists KEM patterns that have issues running in a -// separate thread +// KEMs that need the main thread's larger stack on some platforms. static std::vector no_thread_KEM_patterns{"Classic-McEliece", - "HQC-256"}; - -// used for thread-safe console output + "HQC-256"}; static std::mutex mu; +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 + void test_kem_correctness(const std::string& kem_name) { { std::lock_guard lg{mu}; @@ -31,12 +50,9 @@ void test_kem_correctness(const std::string& kem_name) { oqs::bytes ciphertext, shared_secret_server; std::tie(ciphertext, shared_secret_server) = server.encap_secret(client_public_key); - oqs::bytes shared_secret_client = client.decap_secret(ciphertext); - bool is_valid = (shared_secret_client == shared_secret_server); - if (!is_valid) - std::cerr << kem_name << ": shared secrets do not coincide" - << std::endl; - EXPECT_TRUE(is_valid); + TestSecret expected{std::move(shared_secret_server)}; + TestSecret actual{client.decap_secret(ciphertext)}; + EXPECT_EQ(actual.get(), expected.get()); } void test_kem_wrong_ciphertext(const std::string& kem_name) { @@ -50,108 +66,208 @@ void test_kem_wrong_ciphertext(const std::string& kem_name) { oqs::bytes ciphertext, shared_secret_server; std::tie(ciphertext, shared_secret_server) = server.encap_secret(client_public_key); + TestSecret expected{std::move(shared_secret_server)}; oqs::bytes wrong_ciphertext = oqs::rand::randombytes(ciphertext.size()); - oqs::bytes shared_secret_client; try { - // this line should throw - shared_secret_client = client.decap_secret(wrong_ciphertext); - // if not, test should fail - bool is_valid = (shared_secret_client == shared_secret_server); - if (is_valid) - std::cerr << kem_name << ": shared secrets should not coincide" - << std::endl; - EXPECT_FALSE(is_valid); - } catch (std::exception& e) { - if (e.what() == std::string{"Can not decapsulate secret"}) - return; - else - throw; // this is another un-expected exception + // Accept either a decapsulation error or a different shared secret. + TestSecret actual{client.decap_secret(wrong_ciphertext)}; + EXPECT_NE(actual.get(), expected.get()); + } catch (const std::exception& e) { + if (std::string{e.what()} != "Can not decapsulate secret") + throw; + } +} + +// An exception escaping a worker would terminate the entire test process. +void test_kem_in_thread(void (*test)(const std::string&), + const std::string& kem_name) { + try { + test(kem_name); + } catch (const std::exception& e) { + ADD_FAILURE() << kem_name << ": unexpected exception: " << e.what(); + } catch (...) { + ADD_FAILURE() << kem_name << ": unexpected exception"; } } TEST(oqs_KeyEncapsulation, Correctness) { + const auto& enabled = oqs::KEMs::get_enabled_KEMs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled KEM algorithms"; + std::vector thread_pool; - std::vector enabled_KEMs = oqs::KEMs::get_enabled_KEMs(); - // first test KEMs that belong to no_thread_KEM_patterns[] in the main - // thread (stack size is 8Mb on macOS), due to issues with stack size being - // too small in macOS (512Kb for threads) - for (auto&& kem_name : enabled_KEMs) { - for (auto&& no_thread_kem : no_thread_KEM_patterns) { - if (kem_name.find(no_thread_kem) != std::string::npos) { - test_kem_correctness(kem_name); + // Run algorithms requiring a larger stack on the main thread first. + for (const auto& alg : enabled) { + for (const auto& pattern : no_thread_KEM_patterns) { + if (alg.find(pattern) != std::string::npos) { + test_kem_correctness(alg); + break; } } } - // test the remaining KEMs in separate threads - for (auto&& kem_name : enabled_KEMs) { + for (const auto& alg : enabled) { bool test_in_thread = true; - for (auto&& no_thread_kem : no_thread_KEM_patterns) { - if (kem_name.find(no_thread_kem) != std::string::npos) { + for (const auto& pattern : no_thread_KEM_patterns) { + if (alg.find(pattern) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) - thread_pool.emplace_back(test_kem_correctness, kem_name); + thread_pool.emplace_back(test_kem_in_thread, + test_kem_correctness, alg); } - // join the rest of the threads - for (auto&& elem : thread_pool) - elem.join(); + for (auto& thread : thread_pool) + thread.join(); } TEST(oqs_KeyEncapsulation, WrongCiphertext) { + const auto& enabled = oqs::KEMs::get_enabled_KEMs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled KEM algorithms"; + std::vector thread_pool; - std::vector enabled_KEMs = oqs::KEMs::get_enabled_KEMs(); - // first test KEMs that belong to no_thread_KEM_patterns[] in the main - // thread (stack size is 8Mb on macOS), due to issues with stack size being - // too small in macOS (512Kb for threads) - for (auto&& kem_name : enabled_KEMs) { - for (auto&& no_thread_kem : no_thread_KEM_patterns) { - if (kem_name.find(no_thread_kem) != std::string::npos) { - test_kem_wrong_ciphertext(kem_name); + // Run algorithms requiring a larger stack on the main thread first. + for (const auto& alg : enabled) { + for (const auto& pattern : no_thread_KEM_patterns) { + if (alg.find(pattern) != std::string::npos) { + test_kem_wrong_ciphertext(alg); + break; } } } - // test the remaining KEMs in separate threads - for (auto&& kem_name : enabled_KEMs) { + for (const auto& alg : enabled) { bool test_in_thread = true; - for (auto&& no_thread_kem : no_thread_KEM_patterns) { - if (kem_name.find(no_thread_kem) != std::string::npos) { + for (const auto& pattern : no_thread_KEM_patterns) { + if (alg.find(pattern) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) - thread_pool.emplace_back(test_kem_wrong_ciphertext, kem_name); + thread_pool.emplace_back(test_kem_in_thread, + test_kem_wrong_ciphertext, alg); } - // join the rest of the threads - for (auto&& elem : thread_pool) - elem.join(); + for (auto& thread : thread_pool) + thread.join(); } +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, NotSupported) { EXPECT_THROW(oqs::KeyEncapsulation{"unsupported_kem"}, oqs::MechanismNotSupportedError); } +TEST(oqs_KeyEncapsulation, NotSupportedWithSecretKey) { + // This checks the exception and preservation of a caller-owned lvalue. + // Cleansing on failure is tested separately by the instrumented harness. + 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_kems = oqs::KEMs::get_enabled_KEMs(); - if (enabled_kems.empty()) - return; + const auto& enabled = oqs::KEMs::get_enabled_KEMs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled KEM algorithms"; - oqs::KeyEncapsulation source{enabled_kems.front()}; - source.generate_keypair(); - oqs::KeyEncapsulation target{enabled_kems.front()}; - target.generate_keypair(); + // 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(target.export_secret_key(), source.export_secret_key()); + 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); - target = std::move(source); - EXPECT_EQ(target.export_secret_key().size(), - target.get_details().length_secret_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(); - EXPECT_EQ(target.export_secret_key().size(), + 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); } From 783be1b9a484afc1d7a065301d4604cb07bfac8d Mon Sep 17 00:00:00 2001 From: "Aryan K. Gupta" Date: Sat, 7 Nov 2026 03:36:06 +0530 Subject: [PATCH 4/6] Refactor signature tests for clarity and efficiency Signed-off-by: Aryan K. Gupta --- unit_tests/tests/test_sig.cpp | 311 ++++++++++++++++++++++------------ 1 file changed, 199 insertions(+), 112 deletions(-) diff --git a/unit_tests/tests/test_sig.cpp b/unit_tests/tests/test_sig.cpp index f4d40a1..165f7a5 100644 --- a/unit_tests/tests/test_sig.cpp +++ b/unit_tests/tests/test_sig.cpp @@ -1,5 +1,7 @@ // Unit testing oqs::Signature +#include +#include #include #include #include @@ -11,16 +13,32 @@ #include "oqs_cpp.hpp" #include "rand/rand.hpp" -// no_thread_sig_patterns lists sig patterns that have issues running in a -// separate thread +// Signatures that need the main thread's larger stack on some platforms. static std::vector no_thread_sig_patterns{ "cross-rsdp", "cross-rsdpg", "SPHINCS+", "Falcon", "MAYO", "SNOVA"}; - -// used for thread-safe console output static std::mutex mu; -// the ML-DSA-*-extmu variants sign an externally computed mu, which must be -// exactly 64 bytes, rather than an arbitrary message +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 + +// The -extmu variants accept exactly 64 bytes of externally computed mu. oqs::bytes test_message(const std::string& sig_name) { if (sig_name.find("-extmu") != std::string::npos) return oqs::rand::randombytes(64); @@ -36,18 +54,14 @@ void test_sig_correctness(const std::string& sig_name, const oqs::bytes& msg) { oqs::bytes signer_public_key = signer.generate_keypair(); oqs::bytes signature = signer.sign(msg); oqs::Signature verifier{sig_name}; - bool is_valid = verifier.verify(msg, signature, signer_public_key); - if (!is_valid) - std::cerr << sig_name << ": signature verification failed" << std::endl; - EXPECT_TRUE(is_valid); + EXPECT_TRUE(verifier.verify(msg, signature, signer_public_key)); } void test_sig_correctness_with_ctx_str(const std::string& sig_name, const oqs::bytes& msg) { oqs::Signature signer{sig_name}; - if (!signer.get_details().sig_with_ctx_support) { + if (!signer.get_details().sig_with_ctx_support) return; - } { std::lock_guard lg{mu}; std::cout << "Correctness with context string - " << sig_name @@ -57,13 +71,8 @@ void test_sig_correctness_with_ctx_str(const std::string& sig_name, oqs::bytes signer_public_key = signer.generate_keypair(); oqs::bytes signature = signer.sign_with_ctx_str(msg, context_str); oqs::Signature verifier{sig_name}; - bool is_valid = verifier.verify_with_ctx_str(msg, signature, context_str, - signer_public_key); - if (!is_valid) - std::cerr << sig_name - << ": signature with context string verification failed" - << std::endl; - EXPECT_TRUE(is_valid); + EXPECT_TRUE(verifier.verify_with_ctx_str(msg, signature, context_str, + signer_public_key)); } void test_sig_wrong_signature(const std::string& sig_name, @@ -77,11 +86,7 @@ void test_sig_wrong_signature(const std::string& sig_name, oqs::bytes signature = signer.sign(msg); oqs::bytes wrong_signature = oqs::rand::randombytes(signature.size()); oqs::Signature verifier{sig_name}; - bool is_valid = verifier.verify(msg, wrong_signature, signer_public_key); - if (is_valid) - std::cerr << sig_name << ": signature verification should have failed" - << std::endl; - EXPECT_FALSE(is_valid); + EXPECT_FALSE(verifier.verify(msg, wrong_signature, signer_public_key)); } void test_sig_wrong_public_key(const std::string& sig_name, @@ -96,15 +101,9 @@ void test_sig_wrong_public_key(const std::string& sig_name, oqs::rand::randombytes(signer_public_key.size()); oqs::bytes signature = signer.sign(msg); oqs::Signature verifier{sig_name}; - bool is_valid = verifier.verify(msg, signature, wrong_public_key); - if (is_valid) - std::cerr << sig_name << ": signature verification should have failed" - << std::endl; - EXPECT_FALSE(is_valid); + EXPECT_FALSE(verifier.verify(msg, signature, wrong_public_key)); } -// runs a test in a separate thread; an exception escaping a std::thread calls -// std::terminate() and aborts the whole test run, so report it as a failure void test_sig_in_thread(void (*test)(const std::string&, const oqs::bytes&), const std::string& sig_name, const oqs::bytes& msg) { try { @@ -117,156 +116,244 @@ void test_sig_in_thread(void (*test)(const std::string&, const oqs::bytes&), } TEST(oqs_Signature, Correctness) { + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled SIG algorithms"; + std::vector thread_pool; - std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); - // first test sigs that belong to no_thread_sig_patterns[] in the main - // thread (stack size is 8Mb on macOS), due to issues with stack size being - // too small in macOS (512Kb for threads) - for (auto&& sig_name : enabled_sigs) { - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { - test_sig_correctness(sig_name, test_message(sig_name)); + // Run algorithms requiring a larger stack on the main thread first. + for (const auto& alg : enabled) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { + test_sig_correctness(alg, test_message(alg)); + break; } } } - // test the remaining sigs in separate threads - for (auto&& sig_name : enabled_sigs) { + for (const auto& alg : enabled) { bool test_in_thread = true; - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) - thread_pool.emplace_back(test_sig_in_thread, test_sig_correctness, - sig_name, test_message(sig_name)); + thread_pool.emplace_back(test_sig_in_thread, + test_sig_correctness, alg, test_message(alg)); } - // join the rest of the threads - for (auto&& elem : thread_pool) - elem.join(); + for (auto& thread : thread_pool) + thread.join(); } TEST(oqs_Signature, CorrectnessWithContextString) { + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled SIG algorithms"; + std::vector thread_pool; - std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); - // first test sigs that belong to no_thread_sig_patterns[] in the main - // thread (stack size is 8Mb on macOS), due to issues with stack size being - // too small in macOS (512Kb for threads) - for (auto&& sig_name : enabled_sigs) { - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { - test_sig_correctness_with_ctx_str(sig_name, - test_message(sig_name)); + // Run algorithms requiring a larger stack on the main thread first. + for (const auto& alg : enabled) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { + test_sig_correctness_with_ctx_str(alg, test_message(alg)); + break; } } } - // test the remaining sigs in separate threads - for (auto&& sig_name : enabled_sigs) { + for (const auto& alg : enabled) { bool test_in_thread = true; - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) thread_pool.emplace_back(test_sig_in_thread, - test_sig_correctness_with_ctx_str, - sig_name, test_message(sig_name)); + test_sig_correctness_with_ctx_str, alg, test_message(alg)); } - // join the rest of the threads - for (auto&& elem : thread_pool) - elem.join(); + for (auto& thread : thread_pool) + thread.join(); } TEST(oqs_Signature, WrongSignature) { + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled SIG algorithms"; + std::vector thread_pool; - std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); - // first test sigs that belong to no_thread_sig_patterns[] in the main - // thread (stack size is 8Mb on macOS), due to issues with stack size being - // too small in macOS (512Kb for threads) - for (auto&& sig_name : enabled_sigs) { - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { - test_sig_wrong_signature(sig_name, test_message(sig_name)); + // Run algorithms requiring a larger stack on the main thread first. + for (const auto& alg : enabled) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { + test_sig_wrong_signature(alg, test_message(alg)); + break; } } } - // test the remaining sigs in separate threads - for (auto&& sig_name : enabled_sigs) { + for (const auto& alg : enabled) { bool test_in_thread = true; - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) thread_pool.emplace_back(test_sig_in_thread, - test_sig_wrong_signature, sig_name, - test_message(sig_name)); + test_sig_wrong_signature, alg, test_message(alg)); } - // join the rest of the threads - for (auto&& elem : thread_pool) - elem.join(); + for (auto& thread : thread_pool) + thread.join(); } TEST(oqs_Signature, WrongPublicKey) { + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled SIG algorithms"; + std::vector thread_pool; - std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); - // first test sigs that belong to no_thread_sig_patterns[] in the main - // thread (stack size is 8Mb on macOS), due to issues with stack size being - // too small in macOS (512Kb for threads) - for (auto&& sig_name : enabled_sigs) { - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { - test_sig_wrong_public_key(sig_name, test_message(sig_name)); + // Run algorithms requiring a larger stack on the main thread first. + for (const auto& alg : enabled) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { + test_sig_wrong_public_key(alg, test_message(alg)); + break; } } } - // test the remaining sigs in separate threads - for (auto&& sig_name : enabled_sigs) { + for (const auto& alg : enabled) { bool test_in_thread = true; - for (auto&& no_thread_sig : no_thread_sig_patterns) { - if (sig_name.find(no_thread_sig) != std::string::npos) { + for (const auto& pattern : no_thread_sig_patterns) { + if (alg.find(pattern) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) thread_pool.emplace_back(test_sig_in_thread, - test_sig_wrong_public_key, sig_name, - test_message(sig_name)); + test_sig_wrong_public_key, alg, test_message(alg)); } - // join the rest of the threads - for (auto&& elem : thread_pool) - elem.join(); + for (auto& thread : thread_pool) + thread.join(); +} + +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, NotSupported) { EXPECT_THROW(oqs::Signature{"unsupported_sig"}, oqs::MechanismNotSupportedError); } +TEST(oqs_Signature, NotSupportedWithSecretKey) { + // This checks the exception and preservation of a caller-owned lvalue. + // Cleansing on failure is tested separately by the instrumented harness. + 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_sigs = oqs::Sigs::get_enabled_sigs(); - if (enabled_sigs.empty()) - return; + const auto& enabled = oqs::Sigs::get_enabled_sigs(); + if (enabled.empty()) + GTEST_SKIP() << "No enabled SIG algorithms"; - oqs::Signature source{enabled_sigs.front()}; - source.generate_keypair(); - oqs::Signature target{enabled_sigs.front()}; - target.generate_keypair(); + // 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(target.export_secret_key(), source.export_secret_key()); + 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); - target = std::move(source); - EXPECT_EQ(target.export_secret_key().size(), - target.get_details().length_secret_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(); - EXPECT_EQ(target.export_secret_key().size(), + 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 SIG 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); } From cfdc4c8596cd7401cedefd008e1bb2eaaaed15cd Mon Sep 17 00:00:00 2001 From: "Aryan K. Gupta" Date: Sat, 7 Nov 2026 03:46:44 +0530 Subject: [PATCH 5/6] Refactor KEM tests and add unsupported KEM case Refactor KEM tests to improve clarity and correctness. Removed TestSecret class usage in favor of direct comparison of shared secrets. Added NotSupported test case for unsupported KEMs. Signed-off-by: Aryan K. Gupta --- unit_tests/tests/test_kem.cpp | 168 +++++++++++++++++----------------- 1 file changed, 83 insertions(+), 85 deletions(-) diff --git a/unit_tests/tests/test_kem.cpp b/unit_tests/tests/test_kem.cpp index fa7b7fd..f21f0f8 100644 --- a/unit_tests/tests/test_kem.cpp +++ b/unit_tests/tests/test_kem.cpp @@ -1,7 +1,6 @@ // Unit testing oqs::KeyEncapsulation #include -#include #include #include #include @@ -14,30 +13,13 @@ #include "oqs_cpp.hpp" #include "rand/rand.hpp" -// KEMs that need the main thread's larger stack on some platforms. +// no_thread_KEM_patterns lists KEM patterns that have issues running in a +// separate thread static std::vector no_thread_KEM_patterns{"Classic-McEliece", - "HQC-256"}; -static std::mutex mu; + "HQC-256"}; -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 +// used for thread-safe console output +static std::mutex mu; void test_kem_correctness(const std::string& kem_name) { { @@ -50,9 +32,12 @@ void test_kem_correctness(const std::string& kem_name) { oqs::bytes ciphertext, shared_secret_server; std::tie(ciphertext, shared_secret_server) = server.encap_secret(client_public_key); - TestSecret expected{std::move(shared_secret_server)}; - TestSecret actual{client.decap_secret(ciphertext)}; - EXPECT_EQ(actual.get(), expected.get()); + oqs::bytes shared_secret_client = client.decap_secret(ciphertext); + bool is_valid = (shared_secret_client == shared_secret_server); + if (!is_valid) + std::cerr << kem_name << ": shared secrets do not coincide" + << std::endl; + EXPECT_TRUE(is_valid); } void test_kem_wrong_ciphertext(const std::string& kem_name) { @@ -66,92 +51,110 @@ void test_kem_wrong_ciphertext(const std::string& kem_name) { oqs::bytes ciphertext, shared_secret_server; std::tie(ciphertext, shared_secret_server) = server.encap_secret(client_public_key); - TestSecret expected{std::move(shared_secret_server)}; oqs::bytes wrong_ciphertext = oqs::rand::randombytes(ciphertext.size()); + oqs::bytes shared_secret_client; try { - // Accept either a decapsulation error or a different shared secret. - TestSecret actual{client.decap_secret(wrong_ciphertext)}; - EXPECT_NE(actual.get(), expected.get()); - } catch (const std::exception& e) { - if (std::string{e.what()} != "Can not decapsulate secret") - throw; - } -} - -// An exception escaping a worker would terminate the entire test process. -void test_kem_in_thread(void (*test)(const std::string&), - const std::string& kem_name) { - try { - test(kem_name); - } catch (const std::exception& e) { - ADD_FAILURE() << kem_name << ": unexpected exception: " << e.what(); - } catch (...) { - ADD_FAILURE() << kem_name << ": unexpected exception"; + // this line should throw + shared_secret_client = client.decap_secret(wrong_ciphertext); + // if not, test should fail + bool is_valid = (shared_secret_client == shared_secret_server); + if (is_valid) + std::cerr << kem_name << ": shared secrets should not coincide" + << std::endl; + EXPECT_FALSE(is_valid); + } catch (std::exception& e) { + if (e.what() == std::string{"Can not decapsulate secret"}) + return; + else + throw; // this is another un-expected exception } } TEST(oqs_KeyEncapsulation, Correctness) { - const auto& enabled = oqs::KEMs::get_enabled_KEMs(); - if (enabled.empty()) - GTEST_SKIP() << "No enabled KEM algorithms"; - std::vector thread_pool; - // Run algorithms requiring a larger stack on the main thread first. - for (const auto& alg : enabled) { - for (const auto& pattern : no_thread_KEM_patterns) { - if (alg.find(pattern) != std::string::npos) { - test_kem_correctness(alg); - break; + std::vector enabled_KEMs = oqs::KEMs::get_enabled_KEMs(); + // first test KEMs that belong to no_thread_KEM_patterns[] in the main + // thread (stack size is 8Mb on macOS), due to issues with stack size being + // too small in macOS (512Kb for threads) + for (auto&& kem_name : enabled_KEMs) { + for (auto&& no_thread_kem : no_thread_KEM_patterns) { + if (kem_name.find(no_thread_kem) != std::string::npos) { + test_kem_correctness(kem_name); } } } - for (const auto& alg : enabled) { + // test the remaining KEMs in separate threads + for (auto&& kem_name : enabled_KEMs) { bool test_in_thread = true; - for (const auto& pattern : no_thread_KEM_patterns) { - if (alg.find(pattern) != std::string::npos) { + for (auto&& no_thread_kem : no_thread_KEM_patterns) { + if (kem_name.find(no_thread_kem) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) - thread_pool.emplace_back(test_kem_in_thread, - test_kem_correctness, alg); + thread_pool.emplace_back(test_kem_correctness, kem_name); } - for (auto& thread : thread_pool) - thread.join(); + // join the rest of the threads + for (auto&& elem : thread_pool) + elem.join(); } TEST(oqs_KeyEncapsulation, WrongCiphertext) { - const auto& enabled = oqs::KEMs::get_enabled_KEMs(); - if (enabled.empty()) - GTEST_SKIP() << "No enabled KEM algorithms"; - std::vector thread_pool; - // Run algorithms requiring a larger stack on the main thread first. - for (const auto& alg : enabled) { - for (const auto& pattern : no_thread_KEM_patterns) { - if (alg.find(pattern) != std::string::npos) { - test_kem_wrong_ciphertext(alg); - break; + std::vector enabled_KEMs = oqs::KEMs::get_enabled_KEMs(); + // first test KEMs that belong to no_thread_KEM_patterns[] in the main + // thread (stack size is 8Mb on macOS), due to issues with stack size being + // too small in macOS (512Kb for threads) + for (auto&& kem_name : enabled_KEMs) { + for (auto&& no_thread_kem : no_thread_KEM_patterns) { + if (kem_name.find(no_thread_kem) != std::string::npos) { + test_kem_wrong_ciphertext(kem_name); } } } - for (const auto& alg : enabled) { + // test the remaining KEMs in separate threads + for (auto&& kem_name : enabled_KEMs) { bool test_in_thread = true; - for (const auto& pattern : no_thread_KEM_patterns) { - if (alg.find(pattern) != std::string::npos) { + for (auto&& no_thread_kem : no_thread_KEM_patterns) { + if (kem_name.find(no_thread_kem) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) - thread_pool.emplace_back(test_kem_in_thread, - test_kem_wrong_ciphertext, alg); + thread_pool.emplace_back(test_kem_wrong_ciphertext, kem_name); } - for (auto& thread : thread_pool) - thread.join(); + // join the rest of the threads + for (auto&& elem : thread_pool) + elem.join(); +} + +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) { @@ -163,14 +166,9 @@ void expect_kem_key_works(const oqs::KeyEncapsulation& key, } } // namespace -TEST(oqs_KeyEncapsulation, NotSupported) { - EXPECT_THROW(oqs::KeyEncapsulation{"unsupported_kem"}, - oqs::MechanismNotSupportedError); -} - TEST(oqs_KeyEncapsulation, NotSupportedWithSecretKey) { // This checks the exception and preservation of a caller-owned lvalue. - // Cleansing on failure is tested separately by the instrumented harness. + // 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); From 4f724c6af1addd1e6e6d1ccd0df6db17431671b6 Mon Sep 17 00:00:00 2001 From: "Aryan K. Gupta" Date: Sat, 7 Nov 2026 03:47:50 +0530 Subject: [PATCH 6/6] Refactor signature tests for clarity and error logging Refactor signature tests to improve readability and error handling. Update verification checks to log failures and ensure proper thread handling. Signed-off-by: Aryan K. Gupta --- unit_tests/tests/test_sig.cpp | 230 +++++++++++++++++++--------------- 1 file changed, 128 insertions(+), 102 deletions(-) diff --git a/unit_tests/tests/test_sig.cpp b/unit_tests/tests/test_sig.cpp index 165f7a5..5a77eb7 100644 --- a/unit_tests/tests/test_sig.cpp +++ b/unit_tests/tests/test_sig.cpp @@ -1,7 +1,6 @@ // Unit testing oqs::Signature #include -#include #include #include #include @@ -13,32 +12,16 @@ #include "oqs_cpp.hpp" #include "rand/rand.hpp" -// Signatures that need the main thread's larger stack on some platforms. +// no_thread_sig_patterns lists sig patterns that have issues running in a +// separate thread static std::vector no_thread_sig_patterns{ "cross-rsdp", "cross-rsdpg", "SPHINCS+", "Falcon", "MAYO", "SNOVA"}; -static std::mutex mu; -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 +// used for thread-safe console output +static std::mutex mu; -// The -extmu variants accept exactly 64 bytes of externally computed mu. +// the ML-DSA-*-extmu variants sign an externally computed mu, which must be +// exactly 64 bytes, rather than an arbitrary message oqs::bytes test_message(const std::string& sig_name) { if (sig_name.find("-extmu") != std::string::npos) return oqs::rand::randombytes(64); @@ -54,14 +37,18 @@ void test_sig_correctness(const std::string& sig_name, const oqs::bytes& msg) { oqs::bytes signer_public_key = signer.generate_keypair(); oqs::bytes signature = signer.sign(msg); oqs::Signature verifier{sig_name}; - EXPECT_TRUE(verifier.verify(msg, signature, signer_public_key)); + bool is_valid = verifier.verify(msg, signature, signer_public_key); + if (!is_valid) + std::cerr << sig_name << ": signature verification failed" << std::endl; + EXPECT_TRUE(is_valid); } void test_sig_correctness_with_ctx_str(const std::string& sig_name, const oqs::bytes& msg) { oqs::Signature signer{sig_name}; - if (!signer.get_details().sig_with_ctx_support) + if (!signer.get_details().sig_with_ctx_support) { return; + } { std::lock_guard lg{mu}; std::cout << "Correctness with context string - " << sig_name @@ -71,8 +58,13 @@ void test_sig_correctness_with_ctx_str(const std::string& sig_name, oqs::bytes signer_public_key = signer.generate_keypair(); oqs::bytes signature = signer.sign_with_ctx_str(msg, context_str); oqs::Signature verifier{sig_name}; - EXPECT_TRUE(verifier.verify_with_ctx_str(msg, signature, context_str, - signer_public_key)); + bool is_valid = verifier.verify_with_ctx_str(msg, signature, context_str, + signer_public_key); + if (!is_valid) + std::cerr << sig_name + << ": signature with context string verification failed" + << std::endl; + EXPECT_TRUE(is_valid); } void test_sig_wrong_signature(const std::string& sig_name, @@ -86,7 +78,11 @@ void test_sig_wrong_signature(const std::string& sig_name, oqs::bytes signature = signer.sign(msg); oqs::bytes wrong_signature = oqs::rand::randombytes(signature.size()); oqs::Signature verifier{sig_name}; - EXPECT_FALSE(verifier.verify(msg, wrong_signature, signer_public_key)); + bool is_valid = verifier.verify(msg, wrong_signature, signer_public_key); + if (is_valid) + std::cerr << sig_name << ": signature verification should have failed" + << std::endl; + EXPECT_FALSE(is_valid); } void test_sig_wrong_public_key(const std::string& sig_name, @@ -101,9 +97,15 @@ void test_sig_wrong_public_key(const std::string& sig_name, oqs::rand::randombytes(signer_public_key.size()); oqs::bytes signature = signer.sign(msg); oqs::Signature verifier{sig_name}; - EXPECT_FALSE(verifier.verify(msg, signature, wrong_public_key)); + bool is_valid = verifier.verify(msg, signature, wrong_public_key); + if (is_valid) + std::cerr << sig_name << ": signature verification should have failed" + << std::endl; + EXPECT_FALSE(is_valid); } +// runs a test in a separate thread; an exception escaping a std::thread calls +// std::terminate() and aborts the whole test run, so report it as a failure void test_sig_in_thread(void (*test)(const std::string&, const oqs::bytes&), const std::string& sig_name, const oqs::bytes& msg) { try { @@ -116,129 +118,158 @@ void test_sig_in_thread(void (*test)(const std::string&, const oqs::bytes&), } TEST(oqs_Signature, Correctness) { - const auto& enabled = oqs::Sigs::get_enabled_sigs(); - if (enabled.empty()) - GTEST_SKIP() << "No enabled SIG algorithms"; - std::vector thread_pool; - // Run algorithms requiring a larger stack on the main thread first. - for (const auto& alg : enabled) { - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { - test_sig_correctness(alg, test_message(alg)); - break; + std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); + // first test sigs that belong to no_thread_sig_patterns[] in the main + // thread (stack size is 8Mb on macOS), due to issues with stack size being + // too small in macOS (512Kb for threads) + for (auto&& sig_name : enabled_sigs) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { + test_sig_correctness(sig_name, test_message(sig_name)); } } } - for (const auto& alg : enabled) { + // test the remaining sigs in separate threads + for (auto&& sig_name : enabled_sigs) { bool test_in_thread = true; - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) - thread_pool.emplace_back(test_sig_in_thread, - test_sig_correctness, alg, test_message(alg)); + thread_pool.emplace_back(test_sig_in_thread, test_sig_correctness, + sig_name, test_message(sig_name)); } - for (auto& thread : thread_pool) - thread.join(); + // join the rest of the threads + for (auto&& elem : thread_pool) + elem.join(); } TEST(oqs_Signature, CorrectnessWithContextString) { - const auto& enabled = oqs::Sigs::get_enabled_sigs(); - if (enabled.empty()) - GTEST_SKIP() << "No enabled SIG algorithms"; - std::vector thread_pool; - // Run algorithms requiring a larger stack on the main thread first. - for (const auto& alg : enabled) { - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { - test_sig_correctness_with_ctx_str(alg, test_message(alg)); - break; + std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); + // first test sigs that belong to no_thread_sig_patterns[] in the main + // thread (stack size is 8Mb on macOS), due to issues with stack size being + // too small in macOS (512Kb for threads) + for (auto&& sig_name : enabled_sigs) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { + test_sig_correctness_with_ctx_str(sig_name, + test_message(sig_name)); } } } - for (const auto& alg : enabled) { + // test the remaining sigs in separate threads + for (auto&& sig_name : enabled_sigs) { bool test_in_thread = true; - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) thread_pool.emplace_back(test_sig_in_thread, - test_sig_correctness_with_ctx_str, alg, test_message(alg)); + test_sig_correctness_with_ctx_str, + sig_name, test_message(sig_name)); } - for (auto& thread : thread_pool) - thread.join(); + // join the rest of the threads + for (auto&& elem : thread_pool) + elem.join(); } TEST(oqs_Signature, WrongSignature) { - const auto& enabled = oqs::Sigs::get_enabled_sigs(); - if (enabled.empty()) - GTEST_SKIP() << "No enabled SIG algorithms"; - std::vector thread_pool; - // Run algorithms requiring a larger stack on the main thread first. - for (const auto& alg : enabled) { - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { - test_sig_wrong_signature(alg, test_message(alg)); - break; + std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); + // first test sigs that belong to no_thread_sig_patterns[] in the main + // thread (stack size is 8Mb on macOS), due to issues with stack size being + // too small in macOS (512Kb for threads) + for (auto&& sig_name : enabled_sigs) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { + test_sig_wrong_signature(sig_name, test_message(sig_name)); } } } - for (const auto& alg : enabled) { + // test the remaining sigs in separate threads + for (auto&& sig_name : enabled_sigs) { bool test_in_thread = true; - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) thread_pool.emplace_back(test_sig_in_thread, - test_sig_wrong_signature, alg, test_message(alg)); + test_sig_wrong_signature, sig_name, + test_message(sig_name)); } - for (auto& thread : thread_pool) - thread.join(); + // join the rest of the threads + for (auto&& elem : thread_pool) + elem.join(); } TEST(oqs_Signature, WrongPublicKey) { - const auto& enabled = oqs::Sigs::get_enabled_sigs(); - if (enabled.empty()) - GTEST_SKIP() << "No enabled SIG algorithms"; - std::vector thread_pool; - // Run algorithms requiring a larger stack on the main thread first. - for (const auto& alg : enabled) { - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { - test_sig_wrong_public_key(alg, test_message(alg)); - break; + std::vector enabled_sigs = oqs::Sigs::get_enabled_sigs(); + // first test sigs that belong to no_thread_sig_patterns[] in the main + // thread (stack size is 8Mb on macOS), due to issues with stack size being + // too small in macOS (512Kb for threads) + for (auto&& sig_name : enabled_sigs) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { + test_sig_wrong_public_key(sig_name, test_message(sig_name)); } } } - for (const auto& alg : enabled) { + // test the remaining sigs in separate threads + for (auto&& sig_name : enabled_sigs) { bool test_in_thread = true; - for (const auto& pattern : no_thread_sig_patterns) { - if (alg.find(pattern) != std::string::npos) { + for (auto&& no_thread_sig : no_thread_sig_patterns) { + if (sig_name.find(no_thread_sig) != std::string::npos) { test_in_thread = false; break; } } if (test_in_thread) thread_pool.emplace_back(test_sig_in_thread, - test_sig_wrong_public_key, alg, test_message(alg)); + test_sig_wrong_public_key, sig_name, + test_message(sig_name)); } - for (auto& thread : thread_pool) - thread.join(); + // join the rest of the threads + for (auto&& elem : thread_pool) + elem.join(); +} + +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) { @@ -249,14 +280,9 @@ void expect_sig_key_works(const oqs::Signature& key, } } // namespace -TEST(oqs_Signature, NotSupported) { - EXPECT_THROW(oqs::Signature{"unsupported_sig"}, - oqs::MechanismNotSupportedError); -} - TEST(oqs_Signature, NotSupportedWithSecretKey) { // This checks the exception and preservation of a caller-owned lvalue. - // Cleansing on failure is tested separately by the instrumented harness. + // 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); @@ -267,7 +293,7 @@ TEST(oqs_Signature, NotSupportedWithSecretKey) { TEST(oqs_Signature, SecretKeyLifecycle) { const auto& enabled = oqs::Sigs::get_enabled_sigs(); if (enabled.empty()) - GTEST_SKIP() << "No enabled SIG algorithms"; + 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. @@ -334,7 +360,7 @@ TEST(oqs_Signature, SecretKeyLifecycle) { TEST(oqs_Signature, EmptySecretKeyAssignment) { const auto& enabled = oqs::Sigs::get_enabled_sigs(); if (enabled.empty()) - GTEST_SKIP() << "No enabled SIG algorithms"; + 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();