Fix line spacing in `rndc secroots`

Before this change, there was a missing blank line between the
negative trust anchors for one view, and the heading line for the next
view. This is because dns_ntatable_totext() omits the last newline.
There is an example of the incorrect output below; the fixed output
has a blank line before "Start view auth".

secure roots as of 21-Oct-2019 12:03:23.500:

 Start view rec
   Secure roots:

./RSASHA256/20326 ; managed

   Negative trust anchors:

example.com: expiry 21-Oct-2019 13:03:15.000
 Start view auth
   Secure roots:

./RSASHA256/20326 ; managed

   Negative trust anchors:

example.com: expiry 21-Oct-2019 13:03:07.000

diff --git a/CHANGES b/CHANGES
index 2d733a3cb22fdc679ca04071cbfb1eaba54ce6fe..1434df9a5b06d1a3dbd9eb2d75538df8c7693bc3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5338.  [bug]           Fix line spacing in `rndc secroots`.
+                       Thanks to Tony Finch. [GL !2478]
+
 5337.  [func]          'named -V' now reports maxminddb and protobuf-c
                        versions. [GL !2686]
 

diff --git a/bin/named/server.c b/bin/named/server.c
index 2eb6865f25b3343a67bf5b0f6f03b6cbf0f1149a..2bfc1a44b055ed29aaca3d3ff74d319b9ef5568d 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -11118,17 +11118,20 @@ named_server_dumpsecroots(named_server_t *server, isc_lex_t *lex,
        FILE *fp = NULL;
        isc_time_t now;
        char tbuf[64];
+       unsigned int used = isc_buffer_usedlength(*text);
+       bool first = true;
 
        /* Skip the command name. */
        ptr = next_token(lex, text);
-       if (ptr == NULL)
+       if (ptr == NULL) {
                return (ISC_R_UNEXPECTEDEND);
+       }
 
        /* "-" here means print the output instead of dumping to file */
        ptr = next_token(lex, text);
-       if (ptr != NULL && strcmp(ptr, "-") == 0)
+       if (ptr != NULL && strcmp(ptr, "-") == 0) {
                ptr = next_token(lex, text);
-       else {
+       } else {
                result = isc_stdio_open(server->secrootsfile, "w", &fp);
                if (result != ISC_R_SUCCESS) {
                        (void) putstr(text, "could not open ");
@@ -11143,66 +11146,85 @@ named_server_dumpsecroots(named_server_t *server, isc_lex_t *lex,
        CHECK(putstr(text, "secure roots as of "));
        CHECK(putstr(text, tbuf));
        CHECK(putstr(text, ":\n"));
+       used = isc_buffer_usedlength(*text);
 
        do {
                for (view = ISC_LIST_HEAD(server->viewlist);
                     view != NULL;
                     view = ISC_LIST_NEXT(view, link))
                {
-                       if (ptr != NULL && strcmp(view->name, ptr) != 0)
+                       if (ptr != NULL && strcmp(view->name, ptr) != 0) {
                                continue;
-                       if (secroots != NULL)
+                       }
+                       if (secroots != NULL) {
                                dns_keytable_detach(&secroots);
+                       }
                        result = dns_view_getsecroots(view, &secroots);
                        if (result == ISC_R_NOTFOUND) {
                                result = ISC_R_SUCCESS;
                                continue;
                        }
-                       CHECK(putstr(text, "\n Start view "));
+                       if (first || used != isc_buffer_usedlength(*text)) {
+                               CHECK(putstr(text, "\n"));
+                               first = false;
+                       }
+                       CHECK(putstr(text, " Start view "));
                        CHECK(putstr(text, view->name));
                        CHECK(putstr(text, "\n   Secure roots:\n\n"));
+                       used = isc_buffer_usedlength(*text);
                        CHECK(dns_keytable_totext(secroots, text));
 
-                       if (ntatable != NULL)
+                       if (ntatable != NULL) {
                                dns_ntatable_detach(&ntatable);
+                       }
                        result = dns_view_getntatable(view, &ntatable);
                        if (result == ISC_R_NOTFOUND) {
                                result = ISC_R_SUCCESS;
                                continue;
                        }
-                       CHECK(putstr(text, "\n   Negative trust anchors:\n\n"));
+                       if (used != isc_buffer_usedlength(*text)) {
+                               CHECK(putstr(text, "\n"));
+                       }
+                       CHECK(putstr(text, "   Negative trust anchors:\n\n"));
+                       used = isc_buffer_usedlength(*text);
                        CHECK(dns_ntatable_totext(ntatable, NULL, text));
                }
-               if (ptr != NULL)
+
+               if (ptr != NULL) {
                        ptr = next_token(lex, text);
+               }
        } while (ptr != NULL);
 
  cleanup:
-       if (isc_buffer_usedlength(*text) > 0) {
-               if (fp != NULL)
-                       (void)putstr(text, "\n");
-               else
-                       (void)putnull(text);
-       }
-       if (secroots != NULL)
+       if (secroots != NULL) {
                dns_keytable_detach(&secroots);
-       if (ntatable != NULL)
+       }
+       if (ntatable != NULL) {
                dns_ntatable_detach(&ntatable);
+       }
+
        if (fp != NULL) {
+               if (used != isc_buffer_usedlength(*text)) {
+                       (void)putstr(text, "\n");
+               }
                fprintf(fp, "%.*s", (int) isc_buffer_usedlength(*text),
                        (char *) isc_buffer_base(*text));
                isc_buffer_clear(*text);
                (void)isc_stdio_close(fp);
+       } else if (isc_buffer_usedlength(*text) > 0) {
+               (void)putnull(text);
        }
-       if (result == ISC_R_SUCCESS)
+
+       if (result == ISC_R_SUCCESS) {
                isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
                              NAMED_LOGMODULE_SERVER, ISC_LOG_INFO,
                              "dumpsecroots complete");
-       else
+       } else {
                isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
                              NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
                              "dumpsecroots failed: %s",
                              dns_result_totext(result));
+       }
        return (result);
 }
 

diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index ccf2bf039bce8de0c409170fb18b4d5e86af2ec1..e959412961b3ecd88c1ef0f3c9728d79b670caab 100644 (file)
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -101,6 +101,21 @@ stripns () {
     awk '($4 == "NS") || ($4 == "RRSIG" && $5 == "NS") { next} { print }' "$1"
 }
 
+#
+# Ensure there is not multiple consecutive blank lines.
+# Ensure there is a blank line before "Start view" and
+# "Negative trust anchors:".
+# Ensure there is not a blank line before "Secure roots:".
+#
+check_secroots_layout () {
+       awk '$0 == "" { if (empty) exit(1); empty=1; next }
+            /Start view/ { if (!empty) exit(1) }
+            /Secure roots:/ { if (empty) exit(1) }
+            /Negative trust anchors:/ { if (!empty) exit(1) }
+            { empty=0 }' $1
+       return $?
+}
+
 # Check that for a query against a validating resolver where the
 # authoritative zone is unsigned (insecure delegation), glue is returned
 # in the additional section
@@ -1703,13 +1718,14 @@ status=$((status+ret))
 # Test that "rndc secroots" is able to dump trusted keys
 echo_i "checking rndc secroots ($n)"
 ret=0
-rndccmd 10.53.0.4 secroots 2>&1 | sed 's/^/ns4 /' | cat_i
 keyid=$(cat ns1/managed.key.id)
+rndccmd 10.53.0.4 secroots 2>&1 | sed 's/^/ns4 /' | cat_i
 cp ns4/named.secroots named.secroots.test$n
+check_secroots_layout named.secroots.test$n || ret=1
 linecount=$(grep -c "./${DEFAULT_ALGORITHM}/$keyid ; static" named.secroots.test$n || true)
 [ "$linecount" -eq 1 ] || ret=1
 linecount=$(< named.secroots.test$n wc -l)
-[ "$linecount" -eq 10 ] || ret=1
+[ "$linecount" -eq 9 ] || ret=1
 n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
@@ -1860,10 +1876,12 @@ dig_with_opts a.fakenode.secure.example. a @10.53.0.4 > dig.out.ns4.test$n.7 ||
 grep "flags:[^;]* ad[^;]*;" dig.out.ns4.test$n.7 > /dev/null && ret=1
 echo_i "dumping secroots"
 rndccmd 10.53.0.4 secroots | sed 's/^/ns4 /' | cat_i
-grep "bogus.example: expiry" ns4/named.secroots > /dev/null || ret=1
-grep "badds.example: expiry" ns4/named.secroots > /dev/null || ret=1
-grep "secure.example: expiry" ns4/named.secroots > /dev/null || ret=1
-grep "fakenode.secure.example: expiry" ns4/named.secroots > /dev/null || ret=1
+cp ns4/named.secroots named.secroots.test$n
+check_secroots_layout named.secroots.test$n || ret=1
+grep "bogus.example: expiry" named.secroots.test$n > /dev/null || ret=1
+grep "badds.example: expiry" named.secroots.test$n > /dev/null || ret=1
+grep "secure.example: expiry" named.secroots.test$n > /dev/null || ret=1
+grep "fakenode.secure.example: expiry" named.secroots.test$n > /dev/null || ret=1
 
 if [ "$ret" -ne 0 ]; then echo_i "failed - with NTA's in place failed"; fi
 status=$((status+ret))
@@ -4051,5 +4069,13 @@ do
   status=$((status+ret))
 done
 
+echo_i "checking secroots output with multiple views ($n)"
+rndccmd 10.53.0.4 secroots 2>&1 | sed 's/^/ns4 /' | cat_i
+cp ns4/named.secroots named.secroots.test$n
+check_secroots_layout named.secroots.test$n || ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1