From: Fred Morcos Date: Thu, 9 Feb 2023 16:09:08 +0000 (+0100) Subject: Avoid raw pointers in checkKey(s) routines X-Git-Tag: dnsdist-1.8.0-rc1~36^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F12527%2Fhead;p=thirdparty%2Fpdns.git Avoid raw pointers in checkKey(s) routines --- diff --git a/pdns/dbdnsseckeeper.cc b/pdns/dbdnsseckeeper.cc index 20b51a71e6..ab24a07c0e 100644 --- a/pdns/dbdnsseckeeper.cc +++ b/pdns/dbdnsseckeeper.cc @@ -614,7 +614,7 @@ DNSSECKeeper::keyset_t DNSSECKeeper::getKeys(const DNSName& zone, bool useCache) return retkeyset; } -bool DNSSECKeeper::checkKeys(const DNSName& zone, vector* errorMessages) +bool DNSSECKeeper::checkKeys(const DNSName& zone, std::optional>> errorMessages) { vector dbkeyset; d_keymetadb->getDomainKeys(zone, dbkeyset); diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc index 8e5f0cd1a3..d379257706 100644 --- a/pdns/dnssecinfra.cc +++ b/pdns/dnssecinfra.cc @@ -22,6 +22,7 @@ #ifdef HAVE_CONFIG_H #include "config.h" #endif +#include #include "dnsparser.hh" #include "sstuff.hh" #include "misc.hh" @@ -63,11 +64,11 @@ std::unique_ptr DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRe fp.reset(); auto dke = makeFromISCString(drc, isc); - vector checkKeyErrors; + auto checkKeyErrors = std::vector{}; - if(!dke->checkKey(&checkKeyErrors)) { + if(!dke->checkKey(checkKeyErrors)) { string reason; - if(checkKeyErrors.size()) { + if(!checkKeyErrors.empty()) { reason = " ("+boost::algorithm::join(checkKeyErrors, ", ")+")"; } throw runtime_error("Invalid DNS Private Key in file '"+string(fname)+"'"+reason); diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh index e8f98a8715..61d718341c 100644 --- a/pdns/dnssecinfra.hh +++ b/pdns/dnssecinfra.hh @@ -24,6 +24,7 @@ #include #include +#include #include #include "misc.hh" @@ -75,7 +76,7 @@ class DNSCryptoKeyEngine virtual void fromISCMap(DNSKEYRecordContent& drc, stormap_t& stormap) = 0; virtual void fromPublicKeyString(const std::string& content) = 0; - virtual bool checkKey(vector* errorMessages = nullptr) const + [[nodiscard]] virtual bool checkKey(std::optional>> /* errorMessages */ = std::nullopt) const { return true; } diff --git a/pdns/dnsseckeeper.hh b/pdns/dnsseckeeper.hh index e753e8e746..ccad4c9838 100644 --- a/pdns/dnsseckeeper.hh +++ b/pdns/dnsseckeeper.hh @@ -202,7 +202,7 @@ public: bool deactivateKey(const DNSName& zname, unsigned int id); bool publishKey(const DNSName& zname, unsigned int id); bool unpublishKey(const DNSName& zname, unsigned int id); - bool checkKeys(const DNSName& zname, vector* errorMessages = nullptr); + bool checkKeys(const DNSName& zname, std::optional>> errorMessages); bool getNSEC3PARAM(const DNSName& zname, NSEC3PARAMRecordContent* n3p=nullptr, bool* narrow=nullptr, bool useCache=true); bool checkNSEC3PARAM(const NSEC3PARAMRecordContent& ns3p, string& msg); diff --git a/pdns/opensslsigners.cc b/pdns/opensslsigners.cc index fddfeba3b3..94b96ff3e7 100644 --- a/pdns/opensslsigners.cc +++ b/pdns/opensslsigners.cc @@ -251,7 +251,7 @@ public: void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; void fromPublicKeyString(const std::string& content) override; - bool checkKey(vector* errorMessages) const override; + [[nodiscard]] bool checkKey(std::optional>> errorMessages) const override; static std::unique_ptr maker(unsigned int algorithm) { @@ -872,20 +872,20 @@ void OpenSSLRSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::map #endif } -bool OpenSSLRSADNSCryptoKeyEngine::checkKey(vector* errorMessages) const +bool OpenSSLRSADNSCryptoKeyEngine::checkKey(std::optional>> errorMessages) const { bool retval = true; // When changing the bitsizes, also edit them in ::create if ((d_algorithm == DNSSECKeeper::RSASHA1 || d_algorithm == DNSSECKeeper::RSASHA1NSEC3SHA1 || d_algorithm == DNSSECKeeper::RSASHA256) && (getBits() < 512 || getBits() > 4096)) { retval = false; - if (errorMessages != nullptr) { - errorMessages->push_back("key is " + std::to_string(getBits()) + " bytes, should be between 512 and 4096"); + if (errorMessages.has_value()) { + errorMessages->get().push_back("key is " + std::to_string(getBits()) + " bytes, should be between 512 and 4096"); } } if (d_algorithm == DNSSECKeeper::RSASHA512 && (getBits() < 1024 || getBits() > 4096)) { retval = false; - if (errorMessages != nullptr) { - errorMessages->push_back("key is " + std::to_string(getBits()) + " bytes, should be between 1024 and 4096"); + if (errorMessages.has_value()) { + errorMessages->get().push_back("key is " + std::to_string(getBits()) + " bytes, should be between 1024 and 4096"); } } @@ -900,12 +900,12 @@ bool OpenSSLRSADNSCryptoKeyEngine::checkKey(vector* errorMessages) const if (RSA_check_key(d_key.get()) != 1) { #endif retval = false; - if (errorMessages != nullptr) { + if (errorMessages.has_value()) { const auto* errmsg = ERR_error_string(ERR_get_error(), nullptr); if (errmsg == nullptr) { errmsg = "Unknown OpenSSL error"; } - errorMessages->push_back(errmsg); + errorMessages->get().emplace_back(errmsg); } } return retval; @@ -1034,7 +1034,7 @@ public: [[nodiscard]] std::string getPublicKeyString() const override; void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; void fromPublicKeyString(const std::string& content) override; - bool checkKey(vector* errorMessages) const override; + [[nodiscard]] bool checkKey(std::optional>> errorMessages) const override; // TODO Fred: hashSize() and hasher() can probably be completely removed along with // hash(). See #12464. @@ -1635,7 +1635,7 @@ void OpenSSLECDSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::m #endif } -bool OpenSSLECDSADNSCryptoKeyEngine::checkKey(vector* errorMessages) const +bool OpenSSLECDSADNSCryptoKeyEngine::checkKey(std::optional>> errorMessages) const { #if OPENSSL_VERSION_MAJOR >= 3 auto ctx = KeyContext{EVP_PKEY_CTX_new_from_pkey(nullptr, d_eckey.get(), nullptr), EVP_PKEY_CTX_free}; @@ -1650,13 +1650,13 @@ bool OpenSSLECDSADNSCryptoKeyEngine::checkKey(vector* errorMessages) con if (errorCode != 1 && errorCode != -2) { retval = false; - if (errorMessages != nullptr) { + if (errorMessages.has_value()) { const auto* errorMessage = ERR_reason_error_string(ERR_get_error()); if (errorMessage == nullptr) { - errorMessages->push_back(defaultErrorMessage); + errorMessages->get().push_back(defaultErrorMessage); } else { - errorMessages->push_back(errorMessage); + errorMessages->get().emplace_back(errorMessage); } } } @@ -1672,12 +1672,12 @@ bool OpenSSLECDSADNSCryptoKeyEngine::checkKey(vector* errorMessages) con bool retval = true; if (EC_KEY_check_key(d_eckey.get()) != 1) { retval = false; - if (errorMessages != nullptr) { + if (errorMessages.has_value()) { const auto* errmsg = ERR_reason_error_string(ERR_get_error()); if (errmsg == nullptr) { errmsg = "Unknown OpenSSL error"; } - errorMessages->push_back(errmsg); + errorMessages->get().push_back(errmsg); } } return retval; @@ -1788,7 +1788,7 @@ public: [[nodiscard]] std::string getPublicKeyString() const override; void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; void fromPublicKeyString(const std::string& content) override; - bool checkKey(vector* errorMessages) const override; + [[nodiscard]] bool checkKey(std::optional>> errorMessages) const override; static std::unique_ptr maker(unsigned int algorithm) { @@ -1837,7 +1837,7 @@ int OpenSSLEDDSADNSCryptoKeyEngine::getBits() const return (int)d_len << 3; } -bool OpenSSLEDDSADNSCryptoKeyEngine::checkKey(vector* errorMessages) const +bool OpenSSLEDDSADNSCryptoKeyEngine::checkKey(std::optional>> errorMessages) const { #if OPENSSL_VERSION_MAJOR >= 3 auto ctx = KeyContext{EVP_PKEY_CTX_new_from_pkey(nullptr, d_edkey.get(), nullptr), EVP_PKEY_CTX_free}; @@ -1852,13 +1852,13 @@ bool OpenSSLEDDSADNSCryptoKeyEngine::checkKey(vector* errorMessages) con if (errorCode != 1 && errorCode != -2) { retval = false; - if (errorMessages != nullptr) { + if (errorMessages.has_value()) { const auto* errorMessage = ERR_reason_error_string(ERR_get_error()); if (errorMessage == nullptr) { - errorMessages->push_back(defaultErrorMessage); + errorMessages->get().push_back(defaultErrorMessage); } else { - errorMessages->push_back(errorMessage); + errorMessages->get().emplace_back(errorMessage); } } } diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index 14dc785a72..b32052ea37 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -299,7 +299,7 @@ static int checkZone(DNSSECKeeper &dk, UeberBackend &B, const DNSName& zone, con bool isSecure=dk.isSecuredZone(zone); bool presigned=dk.isPresigned(zone); vector checkKeyErrors; - bool validKeys=dk.checkKeys(zone, &checkKeyErrors); + bool validKeys=dk.checkKeys(zone, checkKeyErrors); if (haveNSEC3) { if(isSecure && zone.wirelength() > 222) { diff --git a/pdns/test-signers.cc b/pdns/test-signers.cc index 0ba6f5914e..062398522c 100644 --- a/pdns/test-signers.cc +++ b/pdns/test-signers.cc @@ -390,7 +390,7 @@ static void test_generic_signer(std::shared_ptr dcke, DNSKEY BOOST_CHECK_EQUAL(dcke->getBits(), signer.bits); vector errorMessages{}; - BOOST_CHECK_EQUAL(dcke->checkKey(&errorMessages), true); + BOOST_CHECK_EQUAL(dcke->checkKey(errorMessages), true); if (!errorMessages.empty()) { BOOST_TEST_MESSAGE("Errors from " + dcke->getName() + " checkKey()"); for (auto& errorMessage : errorMessages) {