From: Remi Gacogne Date: Thu, 27 Apr 2017 20:41:33 +0000 (+0200) Subject: Always wrap DNSCryptoKeyEngine objects in a shared pointer X-Git-Tag: rec-4.0.5-rc1~1^2~5^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ed4610b33a280e3eec796954fc1f8e9e76860cb5;p=pdns Always wrap DNSCryptoKeyEngine objects in a shared pointer It's done almost everywhere, but not quite, and some of the paths where it's not could leak if an exception is raised. Also mark the overridden virtual methods with `override` to prevent future mistakes. (cherry picked from commit e69c2dac28d798813dd8e4a986c5045c63806ef0) --- diff --git a/pdns/botan110signers.cc b/pdns/botan110signers.cc index 51ae8652b..e99ceda4c 100644 --- a/pdns/botan110signers.cc +++ b/pdns/botan110signers.cc @@ -45,23 +45,23 @@ public: explicit GOSTDNSCryptoKeyEngine(unsigned int algorithm) : DNSCryptoKeyEngine(algorithm) {} // XXX FIXME NEEDS COPY CONSTRUCTOR SO WE DON'T SHARE KEYS ~GOSTDNSCryptoKeyEngine(){} - void create(unsigned int bits); - string getName() const { return "Botan 1.10 GOST"; } - storvector_t convertToISCVector() const; - std::string getPubKeyHash() const; - std::string sign(const std::string& hash) const; - std::string hash(const std::string& hash) const; - bool verify(const std::string& hash, const std::string& signature) const; - std::string getPublicKeyString() const; - int getBits() const; - void fromISCMap(DNSKEYRecordContent& drc, std::map& content); - void fromPublicKeyString(const std::string& content); - void fromPEMString(DNSKEYRecordContent& drc, const std::string& raw) + void create(unsigned int bits) override; + string getName() const override { return "Botan 1.10 GOST"; } + storvector_t convertToISCVector() const override; + std::string getPubKeyHash() const override; + std::string sign(const std::string& hash) const override; + std::string hash(const std::string& hash) const override; + bool verify(const std::string& hash, const std::string& signature) const override; + std::string getPublicKeyString() const override; + int getBits() const override; + void fromISCMap(DNSKEYRecordContent& drc, std::map& content) override; + void fromPublicKeyString(const std::string& content) override; + void fromPEMString(DNSKEYRecordContent& drc, const std::string& raw) override {} - static DNSCryptoKeyEngine* maker(unsigned int algorithm) + static std::shared_ptr maker(unsigned int algorithm) { - return new GOSTDNSCryptoKeyEngine(algorithm); + return std::make_shared(algorithm); } private: diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc index 8a17cac03..d2654d1d9 100644 --- a/pdns/dnssecinfra.cc +++ b/pdns/dnssecinfra.cc @@ -49,7 +49,7 @@ using namespace boost::assign; -DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRecordContent& drc, const char* fname) +shared_ptr DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRecordContent& drc, const char* fname) { string sline, isc; FILE *fp=fopen(fname, "r"); @@ -61,15 +61,14 @@ DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRecordContent& drc isc += sline; } fclose(fp); - DNSCryptoKeyEngine* dke = makeFromISCString(drc, isc); + shared_ptr dke = makeFromISCString(drc, isc); if(!dke->checkKey()) { - delete dke; throw runtime_error("Invalid DNS Private Key in file '"+string(fname)); } return dke; } -DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromISCString(DNSKEYRecordContent& drc, const std::string& content) +shared_ptr DNSCryptoKeyEngine::makeFromISCString(DNSKEYRecordContent& drc, const std::string& content) { bool pkcs11=false; int algorithm = 0; @@ -104,7 +103,7 @@ DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromISCString(DNSKEYRecordContent& d B64Decode(value, raw); stormap[toLower(key)]=raw; } - DNSCryptoKeyEngine* dpk; + shared_ptr dpk; if (pkcs11) { #ifdef HAVE_P11KIT1 @@ -140,7 +139,7 @@ std::string DNSCryptoKeyEngine::convertToISC() const return ret.str(); } -DNSCryptoKeyEngine* DNSCryptoKeyEngine::make(unsigned int algo) +shared_ptr DNSCryptoKeyEngine::make(unsigned int algo) { makers_t& makers = getMakers(); makers_t::const_iterator iter = makers.find(algo); @@ -308,20 +307,20 @@ pair DNSCryptoKeyEngine::testMakers(unsigned int alg return make_pair(udiffSign, udiffVerify); } -DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromPublicKeyString(unsigned int algorithm, const std::string& content) +shared_ptr DNSCryptoKeyEngine::makeFromPublicKeyString(unsigned int algorithm, const std::string& content) { - DNSCryptoKeyEngine* dpk=make(algorithm); + shared_ptr dpk=make(algorithm); dpk->fromPublicKeyString(content); return dpk; } -DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw) +shared_ptr DNSCryptoKeyEngine::makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw) { for(makers_t::value_type& val : getMakers()) { - DNSCryptoKeyEngine* ret=0; + shared_ptr ret=nullptr; try { ret = val.second(val.first); ret->fromPEMString(drc, raw); @@ -329,7 +328,6 @@ DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromPEMString(DNSKEYRecordContent& d } catch(...) { - delete ret; // fine if 0 } } return 0; @@ -432,7 +430,7 @@ DSRecordContent makeDSFromDNSKey(const DNSName& qname, const DNSKEYRecordContent } -DNSKEYRecordContent makeDNSKEYFromDNSCryptoKeyEngine(const DNSCryptoKeyEngine* pk, uint8_t algorithm, uint16_t flags) +DNSKEYRecordContent makeDNSKEYFromDNSCryptoKeyEngine(const std::shared_ptr pk, uint8_t algorithm, uint16_t flags) { DNSKEYRecordContent drc; diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh index 149cc3aa5..3e07642b9 100644 --- a/pdns/dnssecinfra.hh +++ b/pdns/dnssecinfra.hh @@ -63,13 +63,13 @@ class DNSCryptoKeyEngine { return true; } - static DNSCryptoKeyEngine* makeFromISCFile(DNSKEYRecordContent& drc, const char* fname); - static DNSCryptoKeyEngine* makeFromISCString(DNSKEYRecordContent& drc, const std::string& content); - static DNSCryptoKeyEngine* makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw); - static DNSCryptoKeyEngine* makeFromPublicKeyString(unsigned int algorithm, const std::string& raw); - static DNSCryptoKeyEngine* make(unsigned int algorithm); + static shared_ptr makeFromISCFile(DNSKEYRecordContent& drc, const char* fname); + static shared_ptr makeFromISCString(DNSKEYRecordContent& drc, const std::string& content); + static shared_ptr makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw); + static shared_ptr makeFromPublicKeyString(unsigned int algorithm, const std::string& raw); + static shared_ptr make(unsigned int algorithm); - typedef DNSCryptoKeyEngine* maker_t(unsigned int algorithm); + typedef shared_ptr maker_t(unsigned int algorithm); static void report(unsigned int algorithm, maker_t* maker, bool fallback=false); static std::pair testMakers(unsigned int algorithm, maker_t* creator, maker_t* signer, maker_t* verifier); @@ -98,9 +98,9 @@ struct DNSSECPrivateKey { uint16_t getTag(); - const DNSCryptoKeyEngine* getKey() const + const shared_ptr getKey() const { - return d_key.get(); + return d_key; } void setKey(const shared_ptr key) diff --git a/pdns/dnssecsigner.cc b/pdns/dnssecsigner.cc index 8beca0f82..724da8c95 100644 --- a/pdns/dnssecsigner.cc +++ b/pdns/dnssecsigner.cc @@ -123,7 +123,7 @@ void fillOutRRSIG(DNSSECPrivateKey& dpk, const DNSName& signQName, RRSIGRecordCo g_signatureCount = S.getPointer("signatures"); DNSKEYRecordContent drc = dpk.getDNSKEY(); - const DNSCryptoKeyEngine* rc = dpk.getKey(); + const std::shared_ptr rc = dpk.getKey(); rrc.d_tag = drc.getTag(); rrc.d_algorithm = drc.d_algorithm; diff --git a/pdns/opensslsigners.cc b/pdns/opensslsigners.cc index acf8f08a6..97bab5eb5 100644 --- a/pdns/opensslsigners.cc +++ b/pdns/opensslsigners.cc @@ -197,9 +197,9 @@ public: void fromPublicKeyString(const std::string& content) override; bool checkKey() const override; - static DNSCryptoKeyEngine* maker(unsigned int algorithm) + static std::shared_ptr maker(unsigned int algorithm) { - return new OpenSSLRSADNSCryptoKeyEngine(algorithm); + return std::make_shared(algorithm); } private: @@ -630,9 +630,9 @@ public: void fromPublicKeyString(const std::string& content) override; bool checkKey() const override; - static DNSCryptoKeyEngine* maker(unsigned int algorithm) + static std::shared_ptr maker(unsigned int algorithm) { - return new OpenSSLECDSADNSCryptoKeyEngine(algorithm); + return std::make_shared(algorithm); } private: diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index 5a1bd4dd8..7e58164e4 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -1433,7 +1433,7 @@ void verifyCrypto(const string& zone) cerr<<"Original DS: "< key=DNSCryptoKeyEngine::makeFromISCString(drc, "Private-key-format: v1.2\n" "Algorithm: 12 (ECC-GOST)\n" "GostAsn1: MEUCAQAwHAYGKoUDAgITMBIGByqFAwICIwEGByqFAwICHgEEIgQg/9MiXtXKg9FDXDN/R9CmVhJDyuzRAIgh4tPwCu4NHIs=\n"); string resign=key->sign(hash); @@ -1598,7 +1598,7 @@ bool showZone(DNSSECKeeper& dk, const DNSName& zone) int bits = -1; try { - std::unique_ptr engine(DNSCryptoKeyEngine::makeFromPublicKeyString(key.d_algorithm, key.d_key)); // throws on unknown algo or bad key + std::shared_ptr engine(DNSCryptoKeyEngine::makeFromPublicKeyString(key.d_algorithm, key.d_key)); // throws on unknown algo or bad key bits=engine->getBits(); } catch(std::exception& e) { @@ -3026,7 +3026,7 @@ loadMainConfig(g_vm["config-dir"].as()); return 1; } - DNSCryptoKeyEngine *dke = NULL; + std::shared_ptr dke = nullptr; // lookup correct key for(DNSBackend::KeyData &kd : keys) { if (kd.id == id) { diff --git a/pdns/pkcs11signers.cc b/pdns/pkcs11signers.cc index 5a98b7470..7b9ff8139 100644 --- a/pdns/pkcs11signers.cc +++ b/pdns/pkcs11signers.cc @@ -944,9 +944,9 @@ void PKCS11DNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, stormap_t& s throw PDNSException("Could not log in to token (PIN wrong?)"); }; -DNSCryptoKeyEngine* PKCS11DNSCryptoKeyEngine::maker(unsigned int algorithm) +std::shared_ptr PKCS11DNSCryptoKeyEngine::maker(unsigned int algorithm) { - return new PKCS11DNSCryptoKeyEngine(algorithm); + return std::make_shared(algorithm); } // this is called during program startup diff --git a/pdns/pkcs11signers.hh b/pdns/pkcs11signers.hh index 90a873509..bf9d48011 100644 --- a/pdns/pkcs11signers.hh +++ b/pdns/pkcs11signers.hh @@ -40,29 +40,29 @@ class PKCS11DNSCryptoKeyEngine : public DNSCryptoKeyEngine } PKCS11DNSCryptoKeyEngine(const PKCS11DNSCryptoKeyEngine& orig); - string getName() const { return "P11 Kit PKCS#11"; }; + string getName() const override { return "P11 Kit PKCS#11"; }; - void create(unsigned int bits); + void create(unsigned int bits) override; - storvector_t convertToISCVector() const; + storvector_t convertToISCVector() const override; - std::string sign(const std::string& msg) const; + std::string sign(const std::string& msg) const override; - std::string hash(const std::string& msg) const; + std::string hash(const std::string& msg) const override; - bool verify(const std::string& msg, const std::string& signature) const; + bool verify(const std::string& msg, const std::string& signature) const override; - std::string getPubKeyHash() const; + std::string getPubKeyHash() const override; - std::string getPublicKeyString() const; - int getBits() const; + std::string getPublicKeyString() const override; + int getBits() const override; - void fromISCMap(DNSKEYRecordContent& drc, stormap_t& stormap); + void fromISCMap(DNSKEYRecordContent& drc, stormap_t& stormap) override; - void fromPEMString(DNSKEYRecordContent& drc, const std::string& raw) { throw "Unimplemented"; }; - void fromPublicKeyString(const std::string& content) { throw "Unimplemented"; }; + void fromPEMString(DNSKEYRecordContent& drc, const std::string& raw) override { throw "Unimplemented"; }; + void fromPublicKeyString(const std::string& content) override { throw "Unimplemented"; }; - static DNSCryptoKeyEngine* maker(unsigned int algorithm); + static std::shared_ptr maker(unsigned int algorithm); }; bool PKCS11ModuleSlotLogin(const std::string& module, const string& tokenId, const std::string& pin); diff --git a/pdns/sodiumsigners.cc b/pdns/sodiumsigners.cc index a937484aa..f5bdc6ffc 100644 --- a/pdns/sodiumsigners.cc +++ b/pdns/sodiumsigners.cc @@ -11,23 +11,23 @@ class SodiumED25519DNSCryptoKeyEngine : public DNSCryptoKeyEngine public: explicit SodiumED25519DNSCryptoKeyEngine(unsigned int algo) : DNSCryptoKeyEngine(algo) {} - string getName() const { return "Sodium ED25519"; } - void create(unsigned int bits); - storvector_t convertToISCVector() const; - std::string getPubKeyHash() const; - std::string sign(const std::string& hash) const; - std::string hash(const std::string& hash) const; - bool verify(const std::string& msg, const std::string& signature) const; - std::string getPublicKeyString() const; - int getBits() const; - void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap); - void fromPublicKeyString(const std::string& content); - void fromPEMString(DNSKEYRecordContent& drc, const std::string& raw) + string getName() const override { return "Sodium ED25519"; } + void create(unsigned int bits) override; + storvector_t convertToISCVector() const override; + std::string getPubKeyHash() const override; + std::string sign(const std::string& hash) const override; + std::string hash(const std::string& hash) const override; + bool verify(const std::string& msg, const std::string& signature) const override; + std::string getPublicKeyString() const override; + int getBits() const override; + void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; + void fromPublicKeyString(const std::string& content) override; + void fromPEMString(DNSKEYRecordContent& drc, const std::string& raw) override {} - static DNSCryptoKeyEngine* maker(unsigned int algorithm) + static std::shared_ptr maker(unsigned int algorithm) { - return new SodiumED25519DNSCryptoKeyEngine(algorithm); + return std::make_shared(algorithm); } private: