From 4bbd2bb51121ab12dbfa3110c2a4a7bbbe7e232d Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Tue, 6 Dec 2016 14:32:47 +0000 Subject: [PATCH] Fixes for `rndc nta` user interface Tell the user explicitly about their mistakes: * Unknown options, e.g. -list instead of -dump or -delete instead of -remove. * Unknown view names. * Excess arguments. Include the view name in `rndc nta -dump` output, for consistency with the NTA add and remove actions. When removing an NTA from all views, do not abort with an error if the NTA was not found in one of the views. --- bin/named/server.c | 66 +++++++++++++++++++++++++------- bin/rndc/rndc.docbook | 6 +++ bin/tests/system/dnssec/tests.sh | 12 +++--- lib/dns/include/dns/nta.h | 5 ++- lib/dns/nta.c | 10 +++-- 5 files changed, 75 insertions(+), 24 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index c1839f98fe..e9773fba55 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -10877,7 +10877,7 @@ named_server_dumpsecroots(named_server_t *server, isc_lex_t *lex, continue; } CHECK(putstr(text, "\n Negative trust anchors:\n\n")); - CHECK(dns_ntatable_totext(ntatable, text)); + CHECK(dns_ntatable_totext(ntatable, NULL, text)); } if (ptr != NULL) ptr = next_token(lex, text); @@ -14314,6 +14314,7 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, isc_result_t result = ISC_R_SUCCESS; char *ptr, *nametext = NULL, *viewname; char namebuf[DNS_NAME_FORMATSIZE]; + char viewbuf[DNS_NAME_FORMATSIZE]; isc_stdtime_t now, when; isc_time_t t; char tbuf[64]; @@ -14323,8 +14324,9 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, const dns_name_t *ntaname; dns_name_t *fname; dns_ttl_t ntattl; - bool ttlset = false, excl = false; + bool ttlset = false, excl = false, viewfound = false; dns_rdataclass_t rdclass = dns_rdataclass_in; + bool first = true; UNUSED(force); @@ -14337,18 +14339,24 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, } for (;;) { + bool opts = true; + /* Check for options */ ptr = next_token(lex, text); if (ptr == NULL) { return (ISC_R_UNEXPECTEDEND); } - if (argcheck(ptr, "dump")) { + if (!opts) { + nametext = ptr; + } else if (strcmp(ptr, "--") == 0) { + opts = false; + } else if (argcheck(ptr, "dump")) { dump = true; } else if (argcheck(ptr, "remove")) { ntattl = 0; ttlset = true; - } else if (argcheck(ptr, "force")) { + } else if (opts && argcheck(ptr, "force")) { force = true; continue; } else if (argcheck(ptr, "lifetime")) { @@ -14388,6 +14396,9 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, tr.length = strlen(ptr); CHECK(dns_rdataclass_fromtext(&rdclass, &tr)); continue; + } else if (ptr[0] == '-') { + msg = "Unknown option"; + CHECK(DNS_R_SYNTAX); } else { nametext = ptr; } @@ -14410,7 +14421,8 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, if (result == ISC_R_NOTFOUND) { continue; } - CHECK(dns_ntatable_totext(ntatable, text)); + + CHECK(dns_ntatable_totext(ntatable, view->name, text)); } CHECK(putnull(text)); @@ -14448,6 +14460,14 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, /* Look for the view name. */ viewname = next_token(lex, text); + if (viewname != NULL) { + strlcpy(viewbuf, viewname, DNS_NAME_FORMATSIZE); + viewname = viewbuf; + } + + if (next_token(lex, text) != NULL) { + CHECK(DNS_R_SYNTAX); + } isc_stdtime_get(&now); @@ -14458,11 +14478,10 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, view != NULL; view = ISC_LIST_NEXT(view, link)) { - static bool first = true; - if (viewname != NULL && strcmp(view->name, viewname) != 0) { continue; } + viewfound = true; if (view->rdclass != rdclass && rdclass != dns_rdataclass_any) { continue; @@ -14518,22 +14537,37 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, "added NTA '%s' (%d sec) in view '%s'", namebuf, ntattl, view->name); } else { - CHECK(dns_ntatable_delete(ntatable, ntaname)); + bool removed; + + result = dns_ntatable_delete(ntatable, ntaname); + if (result == ISC_R_SUCCESS) { + removed = true; + } else if (result == ISC_R_NOTFOUND) { + removed = false; + } else { + goto cleanup; + } if (!first) { CHECK(putstr(text, "\n")); } first = false; - CHECK(putstr(text, "Negative trust anchor removed: ")); + CHECK(putstr(text, "Negative trust anchor ")); + CHECK(putstr(text, removed ? "removed: " + : "not found: ")); CHECK(putstr(text, namebuf)); CHECK(putstr(text, "/")); CHECK(putstr(text, view->name)); - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "removed NTA '%s' in view %s", - namebuf, view->name); + if (removed) { + isc_log_write(named_g_lctx, + NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, + ISC_LOG_INFO, + "removed NTA '%s' in view %s", + namebuf, view->name); + } } result = dns_view_saventa(view); @@ -14546,6 +14580,11 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, } } + if (!viewfound) { + msg = "No such view"; + CHECK(ISC_R_NOTFOUND); + } + CHECK(putnull(text)); cleanup: @@ -14553,6 +14592,7 @@ named_server_nta(named_server_t *server, isc_lex_t *lex, (void) putstr(text, msg); (void) putnull(text); } + if (excl) { isc_task_endexclusive(server->task); } diff --git a/bin/rndc/rndc.docbook b/bin/rndc/rndc.docbook index 047e5c0199..999d408943 100644 --- a/bin/rndc/rndc.docbook +++ b/bin/rndc/rndc.docbook @@ -650,6 +650,12 @@ , , , , and . + + Unrecognized options are treated as errors. To reference + a domain or view name that begins with a hyphen, + use a double-hyphen on the command line to indicate the + end of options. + diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index f3490027a3..6e2369f696 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1873,8 +1873,8 @@ $PERL -e 'my $delay = '$start' + 13 - time(); select(undef, undef, undef, $delay $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n._11 lines=`grep " expiry " rndc.out.ns4.test$n._11 | wc -l` [ "$lines" -le 2 ] || ret=1 -grep "bogus.example: expiry" rndc.out.ns4.test$n._11 > /dev/null || ret=1 -grep "badds.example: expiry" rndc.out.ns4.test$n._11 > /dev/null && ret=1 +grep "bogus.example/_default: expiry" rndc.out.ns4.test$n._11 > /dev/null || ret=1 +grep "badds.example/_default: expiry" rndc.out.ns4.test$n._11 > /dev/null && ret=1 $DIG $DIGOPTS b.bogus.example. a @10.53.0.4 > dig.out.ns4.test$n.11 || ret=1 grep "status: SERVFAIL" dig.out.ns4.test$n.11 > /dev/null && ret=1 $DIG $DIGOPTS a.badds.example. a @10.53.0.4 > dig.out.ns4.test$n.12 || ret=1 @@ -1910,14 +1910,14 @@ ret=0 echo_i "testing NTA removals ($n)" $RNDCCMD 10.53.0.4 nta badds.example 2>&1 | sed 's/^/ns4 /' | cat_i $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n.1 -grep "badds.example: expiry" rndc.out.ns4.test$n.1 > /dev/null || ret=1 +grep "badds.example/_default: expiry" rndc.out.ns4.test$n.1 > /dev/null || ret=1 $DIG $DIGOPTS a.badds.example. a @10.53.0.4 > dig.out.ns4.test$n.1 || ret=1 grep "status: SERVFAIL" dig.out.ns4.test$n.1 > /dev/null && ret=1 grep "^a.badds.example." dig.out.ns4.test$n.1 > /dev/null || ret=1 $RNDCCMD 10.53.0.4 nta -remove badds.example > rndc.out.ns4.test$n.2 grep "Negative trust anchor removed: badds.example/_default" rndc.out.ns4.test$n.2 > /dev/null || ret=1 $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n.3 -grep "badds.example: expiry" rndc.out.ns4.test$n.3 > /dev/null && ret=1 +grep "badds.example/_default: expiry" rndc.out.ns4.test$n.3 > /dev/null && ret=1 $DIG $DIGOPTS a.badds.example. a @10.53.0.4 > dig.out.ns4.test$n.2 || ret=1 grep "status: SERVFAIL" dig.out.ns4.test$n.2 > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi @@ -1928,7 +1928,7 @@ echo_i "remove non-existent NTA three times" $RNDCCMD 10.53.0.4 nta -r foo > rndc.out.ns4.test$n.4 2>&1 $RNDCCMD 10.53.0.4 nta -remove foo > rndc.out.ns4.test$n.5 2>&1 $RNDCCMD 10.53.0.4 nta -r foo > rndc.out.ns4.test$n.6 2>&1 -grep "'nta' failed: not found" rndc.out.ns4.test$n.6 > /dev/null || ret=1 +grep "not found" rndc.out.ns4.test$n.6 > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` ret=0 @@ -2009,7 +2009,7 @@ sleep 4 $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n.3 lines=`wc -l < rndc.out.ns4.test$n.3` [ "$lines" -eq 1 ] || ret=1 -grep "bogus.example: expiry" rndc.out.ns4.test$n.3 > /dev/null || ret=1 +grep "bogus.example/_default: expiry" rndc.out.ns4.test$n.3 > /dev/null || ret=1 $DIG $DIGOPTS b.bogus.example. a @10.53.0.4 > dig.out.ns4.test$n.4 || ret=1 grep "status: SERVFAIL" dig.out.ns4.test$n.4 > /dev/null && ret=1 grep "flags:[^;]* ad[^;]*;" dig.out.ns4.test$n.4 > /dev/null && ret=1 diff --git a/lib/dns/include/dns/nta.h b/lib/dns/include/dns/nta.h index 21af6499c3..cce47b0a44 100644 --- a/lib/dns/include/dns/nta.h +++ b/lib/dns/include/dns/nta.h @@ -182,9 +182,10 @@ dns_ntatable_covered(dns_ntatable_t *ntatable, isc_stdtime_t now, */ isc_result_t -dns_ntatable_totext(dns_ntatable_t *ntatable, isc_buffer_t **buf); +dns_ntatable_totext(dns_ntatable_t *ntatable, const char *view, + isc_buffer_t **buf); /*%< - * Dump the NTA table to buffer at 'buf' + * Dump the NTA table to buffer at 'buf', with view names * * Requires: * \li "ntatable" is a valid table. diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 194688f865..73674b3002 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -509,7 +509,9 @@ putstr(isc_buffer_t **b, const char *str) { } isc_result_t -dns_ntatable_totext(dns_ntatable_t *ntatable, isc_buffer_t **buf) { +dns_ntatable_totext(dns_ntatable_t *ntatable, const char *view, + isc_buffer_t **buf) +{ isc_result_t result; dns_rbtnode_t *node; dns_rbtnodechain_t chain; @@ -552,8 +554,10 @@ dns_ntatable_totext(dns_ntatable_t *ntatable, isc_buffer_t **buf) { isc_time_formattimestamp(&t, tbuf, sizeof(tbuf)); - snprintf(obuf, sizeof(obuf), "%s%s: %s %s", + snprintf(obuf, sizeof(obuf), "%s%s%s%s: %s %s", first ? "" : "\n", nbuf, + view != NULL ? "/" : "", + view != NULL ? view : "", n->expiry <= now ? "expired" : "expiry", @@ -588,7 +592,7 @@ dns_ntatable_dump(dns_ntatable_t *ntatable, FILE *fp) { if (result != ISC_R_SUCCESS) return (result); - result = dns_ntatable_totext(ntatable, &text); + result = dns_ntatable_totext(ntatable, NULL, &text); if (isc_buffer_usedlength(text) != 0) { (void) putstr(&text, "\n");