From: Pieter Lexis Date: Mon, 1 Oct 2018 15:30:18 +0000 (+0200) Subject: checkKey: allow returning error messages X-Git-Tag: auth-4.2.0-alpha1~34^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ac3f3893666c39f7ce6f0213538ba2db07bdb47f;p=thirdparty%2Fpdns.git checkKey: allow returning error messages --- diff --git a/pdns/dbdnsseckeeper.cc b/pdns/dbdnsseckeeper.cc index b34d23164b..2f63af5824 100644 --- 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* errorMessages) { vector dbkeyset; d_keymetadb->getDomainKeys(zone, dbkeyset); + bool retval = true; for(const DNSBackend::KeyData &keydata : dbkeyset) { DNSKEYRecordContent dkrc; shared_ptr 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 3cf6dd95a9..76f16b22ce 100644 --- 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 *errorMessages = nullptr) const { return true; } diff --git a/pdns/dnsseckeeper.hh b/pdns/dnsseckeeper.hh index d1fd129622..69dde0430e 100644 --- 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* 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 452025398c..afa55b87cc 100644 --- a/pdns/opensslsigners.cc +++ b/pdns/opensslsigners.cc @@ -30,6 +30,7 @@ #include #include #include +#include #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& stormap) override; void fromPublicKeyString(const std::string& content) override; - bool checkKey() const override; + bool checkKey(vector *errorMessages) const override; static std::shared_ptr 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 *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& stormap) override; void fromPublicKeyString(const std::string& content) override; - bool checkKey() const override; + bool checkKey(vector *errorMessages) const override; static std::shared_ptr 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 *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 5c98023ea5..6d77355ec6 100644 --- 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 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 dbkeyset; - B.getDomainKeys(zone, dbkeyset); - - for(const DNSBackend::KeyData &keydata : dbkeyset) { - DNSKEYRecordContent dkrc; - shared_ptr 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"<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);