]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Avoid raw pointers in checkKey(s) routines 12527/head
authorFred Morcos <fred.morcos@open-xchange.com>
Thu, 9 Feb 2023 16:09:08 +0000 (17:09 +0100)
committerFred Morcos <fred.morcos@open-xchange.com>
Fri, 10 Feb 2023 13:12:51 +0000 (14:12 +0100)
pdns/dbdnsseckeeper.cc
pdns/dnssecinfra.cc
pdns/dnssecinfra.hh
pdns/dnsseckeeper.hh
pdns/opensslsigners.cc
pdns/pdnsutil.cc
pdns/test-signers.cc

index 20b51a71e6566fe31dfb25b1ea0e2cda52ebc31c..ab24a07c0e1a4f8e8316c2ef6c12615730641427 100644 (file)
@@ -614,7 +614,7 @@ DNSSECKeeper::keyset_t DNSSECKeeper::getKeys(const DNSName& zone, bool useCache)
   return retkeyset;
 }
 
-bool DNSSECKeeper::checkKeys(const DNSName& zone, vector<string>* errorMessages)
+bool DNSSECKeeper::checkKeys(const DNSName& zone, std::optional<std::reference_wrapper<std::vector<std::string>>> errorMessages)
 {
   vector<DNSBackend::KeyData> dbkeyset;
   d_keymetadb->getDomainKeys(zone, dbkeyset);
index 8e5f0cd1a3074eb4660cc5b52e7c4f1d62507ee3..d379257706feab543d9adb4e9cbceb223c4775d8 100644 (file)
@@ -22,6 +22,7 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
+#include <functional>
 #include "dnsparser.hh"
 #include "sstuff.hh"
 #include "misc.hh"
@@ -63,11 +64,11 @@ std::unique_ptr<DNSCryptoKeyEngine> DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRe
   fp.reset();
 
   auto dke = makeFromISCString(drc, isc);
-  vector<string> checkKeyErrors;
+  auto checkKeyErrors = std::vector<std::string>{};
 
-  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);
index e8f98a871509dbb9626f0a28346f56171a08b4e3..61d718341c947ada6b9034aa859f6d51f52c2498 100644 (file)
@@ -24,6 +24,7 @@
 
 #include <string>
 #include <vector>
+#include <optional>
 #include <map>
 #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<string>* errorMessages = nullptr) const
+    [[nodiscard]] virtual bool checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> /* errorMessages */ = std::nullopt) const
     {
       return true;
     }
index e753e8e746f3cd7aab675d8b90065ae80d8d15aa..ccad4c98385b3b7873a14727385790b5d2431e0e 100644 (file)
@@ -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<string>* errorMessages = nullptr);
+  bool checkKeys(const DNSName& zname, std::optional<std::reference_wrapper<std::vector<std::string>>> errorMessages);
 
   bool getNSEC3PARAM(const DNSName& zname, NSEC3PARAMRecordContent* n3p=nullptr, bool* narrow=nullptr, bool useCache=true);
   bool checkNSEC3PARAM(const NSEC3PARAMRecordContent& ns3p, string& msg);
index fddfeba3b3e6df445c18d9776bfffcb529b362bb..94b96ff3e7e305b37855bc0b75750b5171d87e94 100644 (file)
@@ -251,7 +251,7 @@ public:
 
   void fromISCMap(DNSKEYRecordContent& drc, std::map<std::string, std::string>& stormap) override;
   void fromPublicKeyString(const std::string& content) override;
-  bool checkKey(vector<string>* errorMessages) const override;
+  [[nodiscard]] bool checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> errorMessages) const override;
 
   static std::unique_ptr<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
@@ -872,20 +872,20 @@ void OpenSSLRSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::map
 #endif
 }
 
-bool OpenSSLRSADNSCryptoKeyEngine::checkKey(vector<string>* errorMessages) const
+bool OpenSSLRSADNSCryptoKeyEngine::checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> 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<string>* 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<std::string, std::string>& stormap) override;
   void fromPublicKeyString(const std::string& content) override;
-  bool checkKey(vector<string>* errorMessages) const override;
+  [[nodiscard]] bool checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> 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<string>* errorMessages) const
+bool OpenSSLECDSADNSCryptoKeyEngine::checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> 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<string>* 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<string>* 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<std::string, std::string>& stormap) override;
   void fromPublicKeyString(const std::string& content) override;
-  bool checkKey(vector<string>* errorMessages) const override;
+  [[nodiscard]] bool checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> errorMessages) const override;
 
   static std::unique_ptr<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
@@ -1837,7 +1837,7 @@ int OpenSSLEDDSADNSCryptoKeyEngine::getBits() const
   return (int)d_len << 3;
 }
 
-bool OpenSSLEDDSADNSCryptoKeyEngine::checkKey(vector<string>* errorMessages) const
+bool OpenSSLEDDSADNSCryptoKeyEngine::checkKey(std::optional<std::reference_wrapper<std::vector<std::string>>> 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<string>* 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);
         }
       }
     }
index 14dc785a7274beba2162e3a9e53cf5f1c6c649fb..b32052ea377bc5331c62e00b3359a220602ae051 100644 (file)
@@ -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<string> checkKeyErrors;
-  bool validKeys=dk.checkKeys(zone, &checkKeyErrors);
+  bool validKeys=dk.checkKeys(zone, checkKeyErrors);
 
   if (haveNSEC3) {
     if(isSecure && zone.wirelength() > 222) {
index 0ba6f5914ec7d9c8ccb6c46f5391d825a06aeedc..062398522c812c25c3e2d1daf45d25ab7d581319 100644 (file)
@@ -390,7 +390,7 @@ static void test_generic_signer(std::shared_ptr<DNSCryptoKeyEngine> dcke, DNSKEY
   BOOST_CHECK_EQUAL(dcke->getBits(), signer.bits);
 
   vector<string> 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) {