checkKey: allow returning error messages

diff --git a/pdns/dbdnsseckeeper.cc b/pdns/dbdnsseckeeper.cc
index b34d23164bdfc2242fe58a4b566efd3b088a5f63..2f63af582483931ffd4ad9e3c65b11b55464b848 100644 (file)
--- a/pdns/dbdnsseckeeper.cc
+++ b/pdns/dbdnsseckeeper.cc
@@ -521,20 +521,19 @@ DNSSECKeeper::keyset_t DNSSECKeeper::getKeys(const DNSName& zone, bool useCache)
   return retkeyset;
 }
 
-bool DNSSECKeeper::checkKeys(const DNSName& zone)
+bool DNSSECKeeper::checkKeys(const DNSName& zone, vector<string>* errorMessages)
 {
   vector<DNSBackend::KeyData> dbkeyset;
   d_keymetadb->getDomainKeys(zone, dbkeyset);
+  bool retval = true;
 
   for(const DNSBackend::KeyData &keydata : dbkeyset) {
     DNSKEYRecordContent dkrc;
     shared_ptr<DNSCryptoKeyEngine> dke(DNSCryptoKeyEngine::makeFromISCString(dkrc, keydata.content));
-    if (!dke->checkKey()) {
-      return false;
-    }
+    retval = dke->checkKey(errorMessages) && retval;
   }
 
-  return true;
+  return retval;
 }
 
 bool DNSSECKeeper::getPreRRSIGs(UeberBackend& db, const DNSName& signer, const DNSName& qname,

diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh
index 3cf6dd95a976bd3c72f06aa5ac8dd5ca64433ec2..76f16b22ce70cac1f8c2f28c654dee42a51f9b91 100644 (file)
--- a/pdns/dnssecinfra.hh
+++ b/pdns/dnssecinfra.hh
@@ -67,7 +67,7 @@ class DNSCryptoKeyEngine
       throw std::runtime_error("Can't import from PEM string");
     }
     virtual void fromPublicKeyString(const std::string& content) = 0;
-    virtual bool checkKey() const
+    virtual bool checkKey(vector<string> *errorMessages = nullptr) const
     {
       return true;
     }

diff --git a/pdns/dnsseckeeper.hh b/pdns/dnsseckeeper.hh
index d1fd12962279c1700171dc8ed7256f622fc48524..69dde0430eb32e02e4d57c7091ff193e1c5099d8 100644 (file)
--- a/pdns/dnsseckeeper.hh
+++ b/pdns/dnsseckeeper.hh
@@ -192,7 +192,7 @@ public:
   bool removeKey(const DNSName& zname, unsigned int id);
   bool activateKey(const DNSName& zname, unsigned int id);
   bool deactivateKey(const DNSName& zname, unsigned int id);
-  bool checkKeys(const DNSName& zname);
+  bool checkKeys(const DNSName& zname, vector<string>* errorMessages = nullptr);
 
   bool getNSEC3PARAM(const DNSName& zname, NSEC3PARAMRecordContent* n3p=0, bool* narrow=0);
   bool checkNSEC3PARAM(const NSEC3PARAMRecordContent& ns3p, string& msg);

diff --git a/pdns/opensslsigners.cc b/pdns/opensslsigners.cc
index 452025398c37d559347f8553c63998cc49253459..afa55b87cc41333396577775b55dd56ae12e976b 100644 (file)
--- a/pdns/opensslsigners.cc
+++ b/pdns/opensslsigners.cc
@@ -30,6 +30,7 @@
 #include <openssl/rand.h>
 #include <openssl/rsa.h>
 #include <openssl/opensslv.h>
+#include <openssl/err.h>
 #include "opensslsigners.hh"
 #include "dnssecinfra.hh"
 #include "dnsseckeeper.hh"
@@ -200,7 +201,7 @@ public:
   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() const override;
+  bool checkKey(vector<string> *errorMessages) const override;
 
   static std::shared_ptr<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
@@ -216,7 +217,7 @@ private:
 
 void OpenSSLRSADNSCryptoKeyEngine::create(unsigned int bits)
 {
-  // When changing the bitsizes, also edit them in ::checkKey and pdnsutil.cc
+  // When changing the bitsizes, also edit them in ::checkKey
   if ((d_algorithm == DNSSECKeeper::RSASHA1 || d_algorithm == DNSSECKeeper::RSASHA1NSEC3SHA1) && (bits < 512 || bits > 4096)) {
     /* RFC3110 */
     throw runtime_error(getName()+" RSASHA1 key generation failed for invalid bits size " + std::to_string(bits));
@@ -539,19 +540,29 @@ void OpenSSLRSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::map
   d_key = key;
 }
 
-bool OpenSSLRSADNSCryptoKeyEngine::checkKey() const
+bool OpenSSLRSADNSCryptoKeyEngine::checkKey(vector<string> *errorMessages) const
 {
-  // When changing the bitsizes, also edit them in ::create and pdnsutil.cc
-  if ((d_algorithm == DNSSECKeeper::RSASHA1 || d_algorithm == DNSSECKeeper::RSASHA1NSEC3SHA1) && (getBits() < 512 || getBits()> 4096)) {
-    return false;
-  }
-  if (d_algorithm == DNSSECKeeper::RSASHA256 && (getBits() < 512 || getBits() > 4096)) {
-    return false;
+  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 (d_algorithm == DNSSECKeeper::RSASHA512 && (getBits() < 1024 || getBits() > 4096)) {
-    return false;
+    retval = false;
+    if (errorMessages != nullptr) {
+      errorMessages->push_back("key is " + std::to_string(getBits()) + " bytes, should be between 1024 and 4096");
+    }
   }
-  return (RSA_check_key(d_key) == 1);
+  if (RSA_check_key(d_key) != 1) {
+    retval = false;
+    if (errorMessages != nullptr) {
+      errorMessages->push_back(ERR_reason_error_string(ERR_get_error()));
+    }
+  }
+  return retval;
 }
 
 void OpenSSLRSADNSCryptoKeyEngine::fromPublicKeyString(const std::string& input)
@@ -668,7 +679,7 @@ public:
   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() const override;
+  bool checkKey(vector<string> *errorMessages) const override;
 
   static std::shared_ptr<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
@@ -891,9 +902,16 @@ void OpenSSLECDSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::m
   EC_POINT_free(pub_key);
 }
 
-bool OpenSSLECDSADNSCryptoKeyEngine::checkKey() const
+bool OpenSSLECDSADNSCryptoKeyEngine::checkKey(vector<string> *errorMessages) const
 {
-  return (EC_KEY_check_key(d_eckey) == 1);
+  bool retval = true;
+  if (EC_KEY_check_key(d_eckey) != 1) {
+    retval = false;
+    if (errorMessages != nullptr) {
+      errorMessages->push_back(ERR_reason_error_string(ERR_get_error()));
+    }
+  }
+  return retval;
 }
 
 void OpenSSLECDSADNSCryptoKeyEngine::fromPublicKeyString(const std::string& input)

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index 5c98023ea5a1b4f7e1699abac6a69e305c2267b7..6d77355ec6f382720681fc2940d3b4ba0a8f80bc 100644 (file)
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -252,7 +252,8 @@ int checkZone(DNSSECKeeper &dk, UeberBackend &B, const DNSName& zone, const vect
 
   bool isSecure=dk.isSecuredZone(zone);
   bool presigned=dk.isPresigned(zone);
-  bool validKeys=dk.checkKeys(zone);
+  vector<string> checkKeyErrors;
+  bool validKeys=dk.checkKeys(zone, &checkKeyErrors);
 
   uint64_t numerrors=0, numwarnings=0;
 
@@ -279,25 +280,8 @@ int checkZone(DNSSECKeeper &dk, UeberBackend &B, const DNSName& zone, const vect
   if (!validKeys) {
     numerrors++;
     cout<<"[Error] zone '" << zone << "' has at least one invalid DNS Private Key." << endl;
-    vector<DNSBackend::KeyData> dbkeyset;
-    B.getDomainKeys(zone, dbkeyset);
-
-    for(const DNSBackend::KeyData &keydata : dbkeyset) {
-      DNSKEYRecordContent dkrc;
-      shared_ptr<DNSCryptoKeyEngine> dke(DNSCryptoKeyEngine::makeFromISCString(dkrc, keydata.content));
-      string msg;
-      if ((dke->getAlgorithm() == DNSSECKeeper::RSASHA1 || dke->getAlgorithm() == DNSSECKeeper::RSASHA1NSEC3SHA1) && (dke->getBits() < 512 || dke->getBits() > 4096)) {
-        msg = "512 and 4096";
-      }
-      if (dke->getAlgorithm() == DNSSECKeeper::RSASHA256 && (dke->getBits() < 512 || dke->getBits() > 4096)) {
-        msg = "512 and 4096";
-      }
-      if (dke->getAlgorithm() == DNSSECKeeper::RSASHA512 && (dke->getBits() < 1024 || dke->getBits() > 4096)) {
-        msg = "1024 and 4096";
-      }
-      if (!msg.empty()) {
-        cout<<"[Error] zone '" << zone << "' key with algorithm " << DNSSECKeeper::algorithm2name(dke->getAlgorithm()) << " has a keysize of " << dke->getBits() << ", which is not between " << msg << endl;
-      }
+    for (const auto &msg : checkKeyErrors) {
+      cout<<"\t"<<msg<<endl;
     }
   }
 

diff --git a/pdns/test-signers.cc b/pdns/test-signers.cc
index adeee553169390a1352ba6dccfc7e8e131e03fdf..774faa801237dd93263e04aef6a906a42f692522 100644 (file)
--- a/pdns/test-signers.cc
+++ b/pdns/test-signers.cc
@@ -168,7 +168,7 @@ BOOST_AUTO_TEST_CASE(test_generic_signers)
 
     BOOST_CHECK_EQUAL(dcke->getAlgorithm(), signer.algorithm);
     BOOST_CHECK_EQUAL(dcke->getBits(), signer.bits);
-    BOOST_CHECK_EQUAL(dcke->checkKey(), true);
+    BOOST_CHECK_EQUAL(dcke->checkKey(nullptr), true);
 
     BOOST_CHECK_EQUAL(drc.d_algorithm, signer.algorithm);