From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 4 Sep 2020 09:42:52 +0000 (+0200)
Subject: Use explicit result codes for 'rndc dnssec' cmd
X-Git-Tag: v9.17.6~15^2~1
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=70d1ec432fba09e2125a34bcd35cfd4113555ee2;p=thirdparty%2Fbind9.git

Use explicit result codes for 'rndc dnssec' cmd

It is better to add new result codes than to overload existing codes.
---

diff --git a/bin/named/server.c b/bin/named/server.c
index c2c12693718..b25d258befe 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -14766,6 +14766,7 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex,
 		char whenbuf[80];
 		isc_time_set(&timewhen, when, 0);
 		isc_time_formattimestamp(&timewhen, whenbuf, sizeof(whenbuf));
+		isc_result_t ret;
 
 		LOCK(&kasp->lock);
 		if (use_keyid) {
@@ -14796,16 +14797,16 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex,
 			CHECK(putstr(text, "since "));
 			CHECK(putstr(text, whenbuf));
 			break;
-		case ISC_R_NOTFOUND:
-			CHECK(putstr(text, "No matching KSK found"));
-			break;
-		case ISC_R_FAILURE:
+		case DNS_R_TOOMANYKEYS:
 			CHECK(putstr(text,
-				     "Error: multiple possible KSKs found, "
+				     "Error: multiple possible keys found, "
 				     "retry command with -key id"));
 			break;
 		default:
-			CHECK(putstr(text, "Error executing checkds command"));
+			ret = result;
+			CHECK(putstr(text,
+				     "Error executing checkds command: "));
+			CHECK(putstr(text, isc_result_totext(ret)));
 			break;
 		}
 	} else if (rollover) {
@@ -14815,6 +14816,7 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex,
 		char whenbuf[80];
 		isc_time_set(&timewhen, when, 0);
 		isc_time_formattimestamp(&timewhen, whenbuf, sizeof(whenbuf));
+		isc_result_t ret;
 
 		LOCK(&kasp->lock);
 		result = dns_keymgr_rollover(kasp, &keys, dir, now, when, keyid,
@@ -14833,21 +14835,16 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex,
 			CHECK(putstr(text, "Rollover scheduled on "));
 			CHECK(putstr(text, whenbuf));
 			break;
-		case ISC_R_NOTFOUND:
-			CHECK(putstr(text, "No matching keyfound"));
-			break;
-		case ISC_R_FAILURE:
+		case DNS_R_TOOMANYKEYS:
 			CHECK(putstr(text,
 				     "Error: multiple possible keys found, "
 				     "retry command with -alg algorithm"));
 			break;
-		case ISC_R_UNEXPECTED:
-			CHECK(putstr(text,
-				     "Error: key is not active and cannot "
-				     "be rolled at this time"));
-			break;
 		default:
-			CHECK(putstr(text, "Error executing rollover command"));
+			ret = result;
+			CHECK(putstr(text,
+				     "Error executing rollover command: "));
+			CHECK(putstr(text, isc_result_totext(ret)));
 			break;
 		}
 	}
diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index 19665deaebc..aed268f78b7 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -2814,7 +2814,7 @@ n=$((n+1))
 echo_i "check that rndc dnssec -rollover fails if key is inactive ($n)"
 ret=0
 rndccmd "$SERVER" dnssec -rollover -key $(key_get KEY4 ID) "$ZONE" > rndc.dnssec.rollover.out.$ZONE.$n
-grep "key is not active and cannot be rolled" rndc.dnssec.rollover.out.$ZONE.$n > /dev/null || log_error "bad error message"
+grep "key is not actively signing" rndc.dnssec.rollover.out.$ZONE.$n > /dev/null || log_error "bad error message"
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
diff --git a/lib/dns/include/dns/keymgr.h b/lib/dns/include/dns/keymgr.h
index 14ee5c980d2..6c7e17ceee9 100644
--- a/lib/dns/include/dns/keymgr.h
+++ b/lib/dns/include/dns/keymgr.h
@@ -74,8 +74,8 @@ dns_keymgr_checkds_id(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
  *
  *	Returns:
  *\li		#ISC_R_SUCCESS (No error).
- *\li		#ISC_R_FAILURE (More than one matching KSK found).
- *\li		#ISC_R_NOTFOUND (No matching KSK found).
+ *\li		#DNS_R_NOKEYMATCH (No matching keys found).
+ *\li		#DNS_R_TOOMANYKEYS (More than one matching keys found).
  *
  */
 
@@ -104,9 +104,9 @@ dns_keymgr_rollover(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
  *
  *	Returns:
  *\li		#ISC_R_SUCCESS (No error).
- *\li		#ISC_R_FAILURE (More than one matching keys found).
- *\li		#ISC_R_NOTFOUND (No matching keys found).
- *\li		#ISC_R_UNEXPECTED (Key is not active).
+ *\li		#DNS_R_NOKEYMATCH (No matching keys found).
+ *\li		#DNS_R_TOOMANYKEYS (More than one matching keys found).
+ *\li		#DNS_R_KEYNOTACTIVE (Key is not active).
  *
  */
 
diff --git a/lib/dns/include/dns/result.h b/lib/dns/include/dns/result.h
index c2e0a68a580..b7f178c7af9 100644
--- a/lib/dns/include/dns/result.h
+++ b/lib/dns/include/dns/result.h
@@ -155,8 +155,11 @@
 #define DNS_R_TOOMANYRECORDS	(ISC_RESULTCLASS_DNS + 117)
 #define DNS_R_VERIFYFAILURE	(ISC_RESULTCLASS_DNS + 118)
 #define DNS_R_ATZONETOP		(ISC_RESULTCLASS_DNS + 119)
+#define DNS_R_NOKEYMATCH	(ISC_RESULTCLASS_DNS + 120)
+#define DNS_R_TOOMANYKEYS	(ISC_RESULTCLASS_DNS + 121)
+#define DNS_R_KEYNOTACTIVE	(ISC_RESULTCLASS_DNS + 122)
 
-#define DNS_R_NRESULTS 120 /*%< Number of results */
+#define DNS_R_NRESULTS 123 /*%< Number of results */
 
 /*
  * DNS wire format rcodes.
diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index 2d1edc97cb5..2518b487390 100644
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -1894,7 +1894,7 @@ keymgr_checkds(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
 				/*
 				 * Only checkds for one key at a time.
 				 */
-				return (ISC_R_FAILURE);
+				return (DNS_R_TOOMANYKEYS);
 			}
 
 			ksk_key = dkey;
@@ -1902,7 +1902,7 @@ keymgr_checkds(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
 	}
 
 	if (ksk_key == NULL) {
-		return (ISC_R_NOTFOUND);
+		return (DNS_R_NOKEYMATCH);
 	}
 
 	if (dspublish) {
@@ -1918,7 +1918,7 @@ keymgr_checkds(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
 	}
 	result = isc_dir_open(&dir, directory);
 	if (result != ISC_R_SUCCESS) {
-		return result;
+		return (result);
 	}
 
 	dns_dnssec_get_hints(ksk_key, now);
@@ -2174,18 +2174,18 @@ dns_keymgr_rollover(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
 			/*
 			 * Only rollover for one key at a time.
 			 */
-			return (ISC_R_FAILURE);
+			return (DNS_R_TOOMANYKEYS);
 		}
 		key = dkey;
 	}
 
 	if (key == NULL) {
-		return (ISC_R_NOTFOUND);
+		return (DNS_R_NOKEYMATCH);
 	}
 
 	result = dst_key_gettime(key->key, DST_TIME_ACTIVATE, &active);
 	if (result != ISC_R_SUCCESS || active > now) {
-		return (ISC_R_UNEXPECTED);
+		return (DNS_R_KEYNOTACTIVE);
 	}
 
 	result = dst_key_gettime(key->key, DST_TIME_INACTIVE, &retire);
@@ -2218,7 +2218,7 @@ dns_keymgr_rollover(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
 	}
 	result = isc_dir_open(&dir, directory);
 	if (result != ISC_R_SUCCESS) {
-		return result;
+		return (result);
 	}
 
 	dns_dnssec_get_hints(key, now);
diff --git a/lib/dns/result.c b/lib/dns/result.c
index a69419c6741..465f72f7839 100644
--- a/lib/dns/result.c
+++ b/lib/dns/result.c
@@ -165,6 +165,10 @@ static const char *text[DNS_R_NRESULTS] = {
 	"too many records",	    /*%< 117 DNS_R_TOOMANYRECORDS */
 	"verify failure",	    /*%< 118 DNS_R_VERIFYFAILURE */
 	"at top of zone",	    /*%< 119 DNS_R_ATZONETOP */
+
+	"no matching key found",       /*%< 120 DNS_R_NOKEYMATCH */
+	"too many keys matching",      /*%< 121 DNS_R_TOOMANYKEYS */
+	"key is not actively signing", /*%< 122 DNS_R_KEYNOTACTIVE */
 };
 
 static const char *ids[DNS_R_NRESULTS] = {
@@ -292,6 +296,9 @@ static const char *ids[DNS_R_NRESULTS] = {
 	"DNS_R_TOOMANYRECORDS",
 	"DNS_R_VERIFYFAILURE",
 	"DNS_R_ATZONETOP",
+	"DNS_R_NOKEYMATCH",
+	"DNS_R_TOOMANYKEYS",
+	"DNS_R_KEYNOTACTIVE",
 };
 
 static const char *rcode_text[DNS_R_NRCODERESULTS] = {