From 9b7a612889f72c2ce7fccc4953682cc15913a21b Mon Sep 17 00:00:00 2001
From: Miod Vallat <miod.vallat@open-xchange.com>
Date: Thu, 9 Jan 2025 15:05:55 +0100
Subject: [PATCH 1/3] Move add-zone-key logic to its own routine. NFC

---
 pdns/pdnsutil.cc | 154 ++++++++++++++++++++++++-----------------------
 1 file changed, 80 insertions(+), 74 deletions(-)

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index ed19662297cd..b0f71c63d919 100644
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -2515,6 +2515,85 @@ static int addOrSetMeta(const DNSName& zone, const string& kind, const vector<st
   return 0;
 }
 
+static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk)
+{
+  if(cmds.size() < 3 ) {
+    cerr << "Syntax: pdnsutil add-zone-key ZONE [zsk|ksk] [BITS] [active|inactive] [rsasha1|rsasha1-nsec3-sha1|rsasha256|rsasha512|ecdsa256|ecdsa384";
+#if defined(HAVE_LIBSODIUM) || defined(HAVE_LIBCRYPTO_ED25519)
+    cerr << "|ed25519";
+#endif
+#if defined(HAVE_LIBCRYPTO_ED448)
+    cerr << "|ed448";
+#endif
+    cerr << "]"<<endl;
+    cerr << endl;
+    cerr << "If zsk|ksk is omitted, add-zone-key makes a key with flags 256 (a 'ZSK')."<<endl;
+    return 0;
+  }
+  DNSName zone(cmds.at(1));
+
+  UeberBackend B("default");
+  DomainInfo di;
+
+  if (!B.getDomainInfo(zone, di)){
+    cerr << "No such zone in the database" << endl;
+    return 0;
+  }
+
+  // need to get algorithm, bits & ksk or zsk from commandline
+  bool keyOrZone=false;
+  int tmp_algo=0;
+  int bits=0;
+  int algorithm=DNSSECKeeper::ECDSA256;
+  bool active=false;
+  bool published=true;
+  for(unsigned int n=2; n < cmds.size(); ++n) {
+    if (pdns_iequals(cmds.at(n), "zsk"))
+      keyOrZone = false;
+    else if (pdns_iequals(cmds.at(n), "ksk"))
+      keyOrZone = true;
+    else if ((tmp_algo = DNSSECKeeper::shorthand2algorithm(cmds.at(n))) > 0) {
+      algorithm = tmp_algo;
+    }
+    else if (pdns_iequals(cmds.at(n), "active")) {
+      active=true;
+    }
+    else if (pdns_iequals(cmds.at(n), "inactive") || pdns_iequals(cmds.at(n), "passive")) { // 'passive' eventually needs to be removed
+      active=false;
+    }
+    else if (pdns_iequals(cmds.at(n), "published")) {
+      published = true;
+    }
+    else if (pdns_iequals(cmds.at(n), "unpublished")) {
+      published = false;
+    }
+    else if (pdns::checked_stoi<int>(cmds.at(n)) != 0) {
+      pdns::checked_stoi_into(bits, cmds.at(n));
+    }
+    else {
+      cerr << "Unknown algorithm, key flag or size '" << cmds.at(n) << "'" << endl;
+      return EXIT_FAILURE;
+    }
+  }
+  int64_t id{-1};
+  if (!dk.addKey(zone, keyOrZone, algorithm, id, bits, active, published)) {
+    cerr<<"Adding key failed, perhaps DNSSEC not enabled in configuration?"<<endl;
+    return 1;
+  } else {
+    cerr<<"Added a " << (keyOrZone ? "KSK" : "ZSK")<<" with algorithm = "<<algorithm<<", active="<<active<<endl;
+    if (bits)
+      cerr<<"Requested specific key size of "<<bits<<" bits"<<endl;
+    if (id == -1) {
+      cerr<<std::to_string(id)<<": Key was added, but backend does not support returning of key id"<<endl;
+    } else if (id < -1) {
+      cerr<<std::to_string(id)<<": Key was added, but there was a failure while returning the key id"<<endl;
+    } else {
+      cout<<std::to_string(id)<<endl;
+    }
+  }
+  return 0;
+}
+
 // NOLINTNEXTLINE(readability-function-cognitive-complexity): TODO Clean this function up.
 int main(int argc, char** argv)
 try
@@ -3023,80 +3102,7 @@ try
   }
 
   else if (cmds.at(0) == "add-zone-key") {
-    if(cmds.size() < 3 ) {
-      cerr << "Syntax: pdnsutil add-zone-key ZONE [zsk|ksk] [BITS] [active|inactive] [rsasha1|rsasha1-nsec3-sha1|rsasha256|rsasha512|ecdsa256|ecdsa384";
-#if defined(HAVE_LIBSODIUM) || defined(HAVE_LIBCRYPTO_ED25519)
-      cerr << "|ed25519";
-#endif
-#if defined(HAVE_LIBCRYPTO_ED448)
-      cerr << "|ed448";
-#endif
-      cerr << "]"<<endl;
-      cerr << endl;
-      cerr << "If zsk|ksk is omitted, add-zone-key makes a key with flags 256 (a 'ZSK')."<<endl;
-      return 0;
-    }
-    DNSName zone(cmds.at(1));
-
-    UeberBackend B("default");
-    DomainInfo di;
-
-    if (!B.getDomainInfo(zone, di)){
-      cerr << "No such zone in the database" << endl;
-      return 0;
-    }
-
-    // need to get algorithm, bits & ksk or zsk from commandline
-    bool keyOrZone=false;
-    int tmp_algo=0;
-    int bits=0;
-    int algorithm=DNSSECKeeper::ECDSA256;
-    bool active=false;
-    bool published=true;
-    for(unsigned int n=2; n < cmds.size(); ++n) {
-      if (pdns_iequals(cmds.at(n), "zsk"))
-        keyOrZone = false;
-      else if (pdns_iequals(cmds.at(n), "ksk"))
-        keyOrZone = true;
-      else if ((tmp_algo = DNSSECKeeper::shorthand2algorithm(cmds.at(n))) > 0) {
-        algorithm = tmp_algo;
-      }
-      else if (pdns_iequals(cmds.at(n), "active")) {
-        active=true;
-      }
-      else if (pdns_iequals(cmds.at(n), "inactive") || pdns_iequals(cmds.at(n), "passive")) { // 'passive' eventually needs to be removed
-        active=false;
-      }
-      else if (pdns_iequals(cmds.at(n), "published")) {
-        published = true;
-      }
-      else if (pdns_iequals(cmds.at(n), "unpublished")) {
-        published = false;
-      }
-      else if (pdns::checked_stoi<int>(cmds.at(n)) != 0) {
-        pdns::checked_stoi_into(bits, cmds.at(n));
-      }
-      else {
-        cerr << "Unknown algorithm, key flag or size '" << cmds.at(n) << "'" << endl;
-        return EXIT_FAILURE;
-      }
-    }
-    int64_t id{-1};
-    if (!dk.addKey(zone, keyOrZone, algorithm, id, bits, active, published)) {
-      cerr<<"Adding key failed, perhaps DNSSEC not enabled in configuration?"<<endl;
-      return 1;
-    } else {
-      cerr<<"Added a " << (keyOrZone ? "KSK" : "ZSK")<<" with algorithm = "<<algorithm<<", active="<<active<<endl;
-      if (bits)
-        cerr<<"Requested specific key size of "<<bits<<" bits"<<endl;
-      if (id == -1) {
-        cerr<<std::to_string(id)<<": Key was added, but backend does not support returning of key id"<<endl;
-      } else if (id < -1) {
-        cerr<<std::to_string(id)<<": Key was added, but there was a failure while returning the key id"<<endl;
-      } else {
-        cout<<std::to_string(id)<<endl;
-      }
-    }
+    return addZoneKey(cmds, dk);
   }
   else if (cmds.at(0) == "remove-zone-key") {
     if(cmds.size() < 3) {

From ddc19815605c0586ca4fd03601ca0281a1ed6c90 Mon Sep 17 00:00:00 2001
From: Miod Vallat <miod.vallat@open-xchange.com>
Date: Thu, 9 Jan 2025 15:14:29 +0100
Subject: [PATCH 2/3] Silence clang-tidy

---
 pdns/pdnsutil.cc | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index b0f71c63d919..a808f4711e73 100644
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -2515,7 +2515,7 @@ static int addOrSetMeta(const DNSName& zone, const string& kind, const vector<st
   return 0;
 }
 
-static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk)
+static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk) //NOLINT(readability-identifier-length)
 {
   if(cmds.size() < 3 ) {
     cerr << "Syntax: pdnsutil add-zone-key ZONE [zsk|ksk] [BITS] [active|inactive] [rsasha1|rsasha1-nsec3-sha1|rsasha256|rsasha512|ecdsa256|ecdsa384";
@@ -2532,8 +2532,8 @@ static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk)
   }
   DNSName zone(cmds.at(1));
 
-  UeberBackend B("default");
-  DomainInfo di;
+  UeberBackend B("default"); //NOLINT(readability-identifier-length)
+  DomainInfo di; //NOLINT(readability-identifier-length)
 
   if (!B.getDomainInfo(zone, di)){
     cerr << "No such zone in the database" << endl;
@@ -2547,11 +2547,13 @@ static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk)
   int algorithm=DNSSECKeeper::ECDSA256;
   bool active=false;
   bool published=true;
-  for(unsigned int n=2; n < cmds.size(); ++n) {
-    if (pdns_iequals(cmds.at(n), "zsk"))
+  for(unsigned int n=2; n < cmds.size(); ++n) { //NOLINT(readability-identifier-length)
+    if (pdns_iequals(cmds.at(n), "zsk")) {
       keyOrZone = false;
-    else if (pdns_iequals(cmds.at(n), "ksk"))
+    }
+    else if (pdns_iequals(cmds.at(n), "ksk")) {
       keyOrZone = true;
+    }
     else if ((tmp_algo = DNSSECKeeper::shorthand2algorithm(cmds.at(n))) > 0) {
       algorithm = tmp_algo;
     }
@@ -2575,21 +2577,21 @@ static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk)
       return EXIT_FAILURE;
     }
   }
-  int64_t id{-1};
+  int64_t id{-1}; //NOLINT(readability-identifier-length)
   if (!dk.addKey(zone, keyOrZone, algorithm, id, bits, active, published)) {
     cerr<<"Adding key failed, perhaps DNSSEC not enabled in configuration?"<<endl;
     return 1;
+  }
+  cerr<<"Added a " << (keyOrZone ? "KSK" : "ZSK")<<" with algorithm = "<<algorithm<<", active="<<active<<endl;
+  if (bits != 0) {
+    cerr<<"Requested specific key size of "<<bits<<" bits"<<endl;
+  }
+  if (id == -1) {
+    cerr<<std::to_string(id)<<": Key was added, but backend does not support returning of key id"<<endl;
+  } else if (id < -1) {
+    cerr<<std::to_string(id)<<": Key was added, but there was a failure while returning the key id"<<endl;
   } else {
-    cerr<<"Added a " << (keyOrZone ? "KSK" : "ZSK")<<" with algorithm = "<<algorithm<<", active="<<active<<endl;
-    if (bits)
-      cerr<<"Requested specific key size of "<<bits<<" bits"<<endl;
-    if (id == -1) {
-      cerr<<std::to_string(id)<<": Key was added, but backend does not support returning of key id"<<endl;
-    } else if (id < -1) {
-      cerr<<std::to_string(id)<<": Key was added, but there was a failure while returning the key id"<<endl;
-    } else {
-      cout<<std::to_string(id)<<endl;
-    }
+    cout<<std::to_string(id)<<endl;
   }
   return 0;
 }

From 3e01a3952307e4fc58020d4d61ce3d7b82aa067c Mon Sep 17 00:00:00 2001
From: Miod Vallat <miod.vallat@open-xchange.com>
Date: Thu, 9 Jan 2025 15:55:16 +0100
Subject: [PATCH 3/3] Make add-zone-key try to retrieve it back to confirm
 operation.

---
 pdns/pdnsutil.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index a808f4711e73..866a442d534b 100644
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -2590,8 +2590,15 @@ static int addZoneKey(vector<string>& cmds, DNSSECKeeper& dk) //NOLINT(readabili
     cerr<<std::to_string(id)<<": Key was added, but backend does not support returning of key id"<<endl;
   } else if (id < -1) {
     cerr<<std::to_string(id)<<": Key was added, but there was a failure while returning the key id"<<endl;
+    return 1;
   } else {
-    cout<<std::to_string(id)<<endl;
+    try {
+      dk.getKeyById(zone, id);
+      cout<<std::to_string(id)<<endl;
+    } catch (std::exception& e) {
+      cerr<<std::to_string(id)<<": Key was added, but there was a failure while reading it back: " <<e.what()<<endl;
+      return 1;
+    }
   }
   return 0;
 }