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 51ae8652b2c460e0fa2657886035cdb67bda61d3..e99ceda4c010c3c2d64592bfb7685ea6f6818037 100644 (file)
--- 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<std::string, std::string>& 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<std::string, std::string>& 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<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
-    return new GOSTDNSCryptoKeyEngine(algorithm);
+    return std::make_shared<GOSTDNSCryptoKeyEngine>(algorithm);
   }
 
 private:

diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc
index 8a17cac035d0e509fa70999c3fe3299c23726326..d2654d1d91d8a07e56a285656f4dea37cbd17fed 100644 (file)
--- 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> 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<DNSCryptoKeyEngine> 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> 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<DNSCryptoKeyEngine> 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> DNSCryptoKeyEngine::make(unsigned int algo)
 {
   makers_t& makers = getMakers();
   makers_t::const_iterator iter = makers.find(algo);
@@ -308,20 +307,20 @@ pair<unsigned int, unsigned int> DNSCryptoKeyEngine::testMakers(unsigned int alg
   return make_pair(udiffSign, udiffVerify);
 }
 
-DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromPublicKeyString(unsigned int algorithm, const std::string& content)
+shared_ptr<DNSCryptoKeyEngine> DNSCryptoKeyEngine::makeFromPublicKeyString(unsigned int algorithm, const std::string& content)
 {
-  DNSCryptoKeyEngine* dpk=make(algorithm);
+  shared_ptr<DNSCryptoKeyEngine> dpk=make(algorithm);
   dpk->fromPublicKeyString(content);
   return dpk;
 }
 
 
-DNSCryptoKeyEngine* DNSCryptoKeyEngine::makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw)
+shared_ptr<DNSCryptoKeyEngine> DNSCryptoKeyEngine::makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw)
 {
   
   for(makers_t::value_type& val :  getMakers())
   {
-    DNSCryptoKeyEngine* ret=0;
+    shared_ptr<DNSCryptoKeyEngine> 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<DNSCryptoKeyEngine> pk, uint8_t algorithm, uint16_t flags)
 {
   DNSKEYRecordContent drc;
 

diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh
index 149cc3aa5caa769a2c25468967fa9f6128a1a7d4..3e07642b9f73d6ea81d238b5849589f6009c14e2 100644 (file)
--- 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<DNSCryptoKeyEngine> makeFromISCFile(DNSKEYRecordContent& drc, const char* fname);
+    static shared_ptr<DNSCryptoKeyEngine> makeFromISCString(DNSKEYRecordContent& drc, const std::string& content);
+    static shared_ptr<DNSCryptoKeyEngine> makeFromPEMString(DNSKEYRecordContent& drc, const std::string& raw);
+    static shared_ptr<DNSCryptoKeyEngine> makeFromPublicKeyString(unsigned int algorithm, const std::string& raw);
+    static shared_ptr<DNSCryptoKeyEngine> make(unsigned int algorithm);
     
-    typedef DNSCryptoKeyEngine* maker_t(unsigned int algorithm);
+    typedef shared_ptr<DNSCryptoKeyEngine> maker_t(unsigned int algorithm);
     
     static void report(unsigned int algorithm, maker_t* maker, bool fallback=false);
     static std::pair<unsigned int, unsigned int> 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<DNSCryptoKeyEngine> getKey() const
   {
-    return d_key.get();
+    return d_key;
   }
   
   void setKey(const shared_ptr<DNSCryptoKeyEngine> key)

diff --git a/pdns/dnssecsigner.cc b/pdns/dnssecsigner.cc
index 8beca0f82d6c470d17bb69c51fe6100d5be18717..724da8c95c1e38470e68a59e25e1626497b7fd10 100644 (file)
--- 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<DNSCryptoKeyEngine> 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 acf8f08a677592f3c2726b94abb9602daba02f99..97bab5eb56a9fcb9c3e8b6e13d5cca6b6cbe4321 100644 (file)
--- 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<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
-    return new OpenSSLRSADNSCryptoKeyEngine(algorithm);
+    return std::make_shared<OpenSSLRSADNSCryptoKeyEngine>(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<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
-    return new OpenSSLECDSADNSCryptoKeyEngine(algorithm);
+    return std::make_shared<OpenSSLECDSADNSCryptoKeyEngine>(algorithm);
   }
 
 private:

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index 5a1bd4dd819a77832e8d9e8d899e97bfcc6c443f..7e58164e45981d171111d067f19d8a9ce61728d9 100644 (file)
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -1433,7 +1433,7 @@ void verifyCrypto(const string& zone)
     cerr<<"Original DS:   "<<apex.toString()<<" IN DS "<<dsrc.getZoneRepresentation()<<endl;
   }
 #if 0
-  DNSCryptoKeyEngine*key=DNSCryptoKeyEngine::makeFromISCString(drc, "Private-key-format: v1.2\n"
+  std::shared_ptr<DNSCryptoKeyEngine> 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<DNSCryptoKeyEngine> engine(DNSCryptoKeyEngine::makeFromPublicKeyString(key.d_algorithm, key.d_key)); // throws on unknown algo or bad key
+        std::shared_ptr<DNSCryptoKeyEngine> 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<string>());
         return 1;
       } 
 
-      DNSCryptoKeyEngine *dke = NULL;
+      std::shared_ptr<DNSCryptoKeyEngine> 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 5a98b7470ebefaa269fdbabcd0b4c6204af060fc..7b9ff813972c0a76edcfabc1c25fe34bb513fdb6 100644 (file)
--- 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<DNSCryptoKeyEngine> PKCS11DNSCryptoKeyEngine::maker(unsigned int algorithm)
 {
-  return new PKCS11DNSCryptoKeyEngine(algorithm);
+  return std::make_shared<PKCS11DNSCryptoKeyEngine>(algorithm);
 }
 
 // this is called during program startup

diff --git a/pdns/pkcs11signers.hh b/pdns/pkcs11signers.hh
index 90a8735096c808f71f008f00e7a5a24cfa3b0e38..bf9d480110a74ea947e447d8ae6180e98fb52d07 100644 (file)
--- 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<DNSCryptoKeyEngine> 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 a937484aa4ca4cf2f0f26f3ad3152c685853c4c5..f5bdc6ffc67133597355bc2bdcadaaf7b911977a 100644 (file)
--- 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<std::string, std::string>& 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<std::string, std::string>& 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<DNSCryptoKeyEngine> maker(unsigned int algorithm)
   {
-    return new SodiumED25519DNSCryptoKeyEngine(algorithm);
+    return std::make_shared<SodiumED25519DNSCryptoKeyEngine>(algorithm);
   }
 
 private: