From 74745c760c8ac4462aceb2fa6e55bc545621c66d Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 25 Aug 2014 18:01:26 -0700 Subject: [PATCH 1/2] [master] "rndc nta -r" could hang 3930. [bug] "rndc nta -r" could cause a server hang if the NTA was not found. [RT #36909] --- CHANGES | 3 +++ bin/named/server.c | 7 +++++-- bin/tests/system/dnssec/tests.sh | 36 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 0529e5439a..60eb22ebab 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +3930. [bug] "rndc nta -r" could cause a server hang if the + NTA was not found. [RT #36909] + 3929. [bug] 'host -a' needed to clear idnoptions. [RT #36963] 3928. [test] Improve rndc system test. [RT #36898] diff --git a/bin/named/server.c b/bin/named/server.c index 2b96567cd1..ce858854d8 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9870,7 +9870,7 @@ ns_server_nta(ns_server_t *server, char *args, isc_buffer_t *text) { dns_fixedname_t fn; dns_name_t *ntaname; dns_ttl_t ntattl; - isc_boolean_t ttlset = ISC_FALSE; + isc_boolean_t ttlset = ISC_FALSE, excl = ISC_FALSE; UNUSED(force); @@ -9969,6 +9969,7 @@ ns_server_nta(ns_server_t *server, char *args, isc_buffer_t *text) { result = isc_task_beginexclusive(server->task); RUNTIME_CHECK(result == ISC_R_SUCCESS); + excl = ISC_TRUE; for (view = ISC_LIST_HEAD(server->viewlist); view != NULL; view = ISC_LIST_NEXT(view, link)) @@ -10037,11 +10038,13 @@ ns_server_nta(ns_server_t *server, char *args, isc_buffer_t *text) { isc_buffer_putuint8(text, 0); } - isc_task_endexclusive(server->task); if (msg != NULL) (void) putstr(text, msg); + cleanup: + if (excl) + isc_task_endexclusive(server->task); if (ntatable != NULL) dns_ntatable_detach(&ntatable); return (result); diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index bd742663af..1736fe2079 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1766,6 +1766,42 @@ n=`expr $n + 1` if [ $ret != 0 ]; then echo "I:failed - that all nta's have been lifted"; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo "I: testing NTA removals ($n)" +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta badds.example 2>&1 | sed 's/^/I:ns4 /' +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -d > rndc.out.ns4.test$n.1 +grep "badds.example: 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 "^a.badds.example." dig.out.ns4.test$n.1 > /dev/null || ret=1 +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 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 +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -d > rndc.out.ns4.test$n.3 +grep "badds.example: 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 +echo "I: remove non-existent NTA three times" +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -r foo > rndc.out.ns4.test$n.4 2>&1 +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -remove foo > rndc.out.ns4.test$n.5 2>&1 +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 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 +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + +n=`expr $n + 1` +echo "I: testing NTA with bogus lifetimes ($n)" +echo "I:check with no nta lifetime specified" +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -l "" foo > rndc.out.ns4.test$n.1 2>&1 +grep "'nta' failed: bad ttl" rndc.out.ns4.test$n.1 > /dev/null || ret=1 +echo "I:check with bad nta lifetime" +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -l garbage foo > rndc.out.ns4.test$n.2 2>&1 +grep "'nta' failed: bad ttl" rndc.out.ns4.test$n.2 > /dev/null || ret=1 +echo "I:check with too long nta lifetime" +$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 nta -l 5d23h foo > rndc.out.ns4.test$n.3 2>&1 +grep "'nta' failed: out of range" rndc.out.ns4.test$n.3 > /dev/null || ret=1 +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + + # Run a minimal update test if possible. This is really just # a regression test for RT #2399; more tests should be added. From 11649973111d83027faf08ed4fb36a2b3c29c875 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 26 Aug 2014 15:01:29 +1000 Subject: [PATCH 2/2] 3931. [cleanup] Cleanup how dlz grammer is defined. [RT #36879] --- CHANGES | 2 ++ bin/named/server.c | 11 ++++------ lib/isccfg/namedconf.c | 48 ++++++++++++------------------------------ 3 files changed, 19 insertions(+), 42 deletions(-) diff --git a/CHANGES b/CHANGES index 60eb22ebab..1ab9f8dff7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +3931. [cleanup] Cleanup how dlz grammer is defined. [RT #36879] + 3930. [bug] "rndc nta -r" could cause a server hang if the NTA was not found. [RT #36909] diff --git a/bin/named/server.c b/bin/named/server.c index ce858854d8..72bbdd285f 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2562,13 +2562,10 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, element != NULL; element = cfg_list_next(element)) { - const cfg_obj_t *dlzopts; - obj = NULL; + dlz = cfg_listelt_value(element); obj = NULL; - dlz = cfg_listelt_value(element); - dlzopts = cfg_tuple_get(dlz, "options"); - (void)cfg_map_get(dlzopts, "database", &obj); + (void)cfg_map_get(dlz, "database", &obj); if (obj != NULL) { dns_dlzdb_t *dlzdb = NULL; const cfg_obj_t *name, *search = NULL; @@ -2585,7 +2582,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, goto cleanup; } - name = cfg_tuple_get(dlz, "name"); + name = cfg_map_getname(dlz); result = dns_dlzcreate(mctx, cfg_obj_asstring(name), dlzargv[0], dlzargc, dlzargv, &dlzdb); @@ -2600,7 +2597,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, * method now. If not searchable, we'll take * care of it when we process the zone statement. */ - (void)cfg_map_get(dlzopts, "search", &search); + (void)cfg_map_get(dlz, "search", &search); if (search == NULL || cfg_obj_asboolean(search)) { dlzdb->search = ISC_TRUE; result = dns_dlzconfigure(view, dlzdb, diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index d9e5a8cef7..f8b3bad316 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -141,35 +141,8 @@ static cfg_type_t cfg_type_view; static cfg_type_t cfg_type_viewopts; static cfg_type_t cfg_type_zone; static cfg_type_t cfg_type_zoneopts; -static cfg_type_t cfg_type_dynamically_loadable_zones; -static cfg_type_t cfg_type_dynamically_loadable_zones_opts; static cfg_type_t cfg_type_filter_aaaa; - -/* - * Clauses that can be found in a 'dynamically loadable zones' statement - */ -static cfg_clausedef_t -dynamically_loadable_zones_clauses[] = { - { "database", &cfg_type_astring, 0 }, - { "search", &cfg_type_boolean, 0 }, - { NULL, NULL, 0 } -}; - -/* - * A dynamically loadable zones statement. - */ -static cfg_tuplefielddef_t dynamically_loadable_zones_fields[] = { - { "name", &cfg_type_astring, 0 }, - { "options", &cfg_type_dynamically_loadable_zones_opts, 0 }, - { NULL, NULL, 0 } -}; - -static cfg_type_t cfg_type_dynamically_loadable_zones = { - "dlz", cfg_parse_tuple, cfg_print_tuple, cfg_doc_tuple, - &cfg_rep_tuple, - dynamically_loadable_zones_fields - }; - +static cfg_type_t cfg_type_dlz; /*% tkey-dhkey */ @@ -936,7 +909,7 @@ static cfg_clausedef_t namedconf_or_view_clauses[] = { { "key", &cfg_type_key, CFG_CLAUSEFLAG_MULTI }, { "zone", &cfg_type_zone, CFG_CLAUSEFLAG_MULTI }, - { "dlz", &cfg_type_dynamically_loadable_zones, CFG_CLAUSEFLAG_MULTI }, + { "dlz", &cfg_type_dlz, CFG_CLAUSEFLAG_MULTI }, { "server", &cfg_type_server, CFG_CLAUSEFLAG_MULTI }, { "trusted-keys", &cfg_type_dnsseckeys, CFG_CLAUSEFLAG_MULTI }, { "managed-keys", &cfg_type_managedkeys, CFG_CLAUSEFLAG_MULTI }, @@ -1803,15 +1776,20 @@ static cfg_type_t cfg_type_zoneopts = { /*% The "dynamically loadable zones" statement syntax. */ +static cfg_clausedef_t +dlz_clauses[] = { + { "database", &cfg_type_astring, 0 }, + { "search", &cfg_type_boolean, 0 }, + { NULL, NULL, 0 } +}; static cfg_clausedef_t * -dynamically_loadable_zones_clausesets[] = { - dynamically_loadable_zones_clauses, +dlz_clausesets[] = { + dlz_clauses, NULL }; -static cfg_type_t cfg_type_dynamically_loadable_zones_opts = { - "dynamically_loadable_zones_opts", cfg_parse_map, - cfg_print_map, cfg_doc_map, &cfg_rep_map, - dynamically_loadable_zones_clausesets +static cfg_type_t cfg_type_dlz = { + "dlz", cfg_parse_named_map, cfg_print_map, cfg_doc_map, + &cfg_rep_map, dlz_clausesets }; /*%