From 127810e51232458ddb1f6abcc8708b27d37e988f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 11 Jul 2018 08:27:10 +0200 Subject: [PATCH 1/3] Extract TAT QNAME preparation to a separate function Extract the part of dotat() reponsible for preparing the QNAME for a TAT query to a separate function in order to limit the number of local variables used by each function and improve code readability. Rename 'name' to 'origin' to better convey the purpose of that variable. Also mark it with the const qualifier. --- bin/named/server.c | 69 +++++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 22b483123e..3d0b9860ad 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6500,35 +6500,32 @@ struct dotat_arg { isc_task_t *task; }; -static void -dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { - isc_result_t result; +/*% + * Prepare the QNAME for the TAT query to be sent by processing the trust + * anchors present at 'keynode' of 'keytable'. Store the result in 'dst'. + * + * A maximum of 12 key IDs can be reported in a single TAT query due to the + * 63-octet length limit for any single label in a domain name. If there are + * more than 12 keys configured at 'keynode', only the first 12 will be + * reported in the TAT query. + */ +static isc_result_t +get_tat_qname(dns_name_t *dst, dns_keytable_t *keytable, + dns_keynode_t *keynode) +{ dns_keynode_t *firstnode = keynode; + const dns_name_t *origin; dns_keynode_t *nextnode; unsigned int i, n = 0; - char label[64], namebuf[DNS_NAME_FORMATSIZE]; - dns_fixedname_t fixed; - dns_name_t *tatname; - isc_uint16_t ids[12]; /* Only 12 id's will fit in a label. */ - int m; - ns_tat_t *tat; - dns_name_t *name = NULL; - struct dotat_arg *dotat_arg = arg; - dns_view_t *view; - isc_task_t *task; + isc_uint16_t ids[12]; isc_textregion_t r; - - REQUIRE(keytable != NULL); - REQUIRE(keynode != NULL); - REQUIRE(arg != NULL); - - view = dotat_arg->view; - task = dotat_arg->task; + char label[64]; + int m; do { dst_key_t *key = dns_keynode_key(keynode); if (key != NULL) { - name = dst_key_name(key); + origin = dst_key_name(key); if (n < (sizeof(ids)/sizeof(ids[0]))) { ids[n] = dst_key_id(key); n++; @@ -6543,7 +6540,7 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { } while (keynode != NULL); if (n == 0) { - return; + return (DNS_R_EMPTYNAME); } if (n > 1) { @@ -6559,18 +6556,40 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { r.length = sizeof(label); m = snprintf(r.base, r.length, "_ta"); if (m < 0 || (unsigned)m > r.length) { - return; + return (ISC_R_FAILURE); } isc_textregion_consume(&r, m); for (i = 0; i < n; i++) { m = snprintf(r.base, r.length, "-%04x", ids[i]); if (m < 0 || (unsigned)m > r.length) { - return; + return (ISC_R_FAILURE); } isc_textregion_consume(&r, m); } + + return (dns_name_fromstring2(dst, label, origin, 0, NULL)); +} + +static void +dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { + struct dotat_arg *dotat_arg = arg; + char namebuf[DNS_NAME_FORMATSIZE]; + dns_fixedname_t fixed; + dns_name_t *tatname; + isc_result_t result; + dns_view_t *view; + isc_task_t *task; + ns_tat_t *tat; + + REQUIRE(keytable != NULL); + REQUIRE(keynode != NULL); + REQUIRE(arg != NULL); + + view = dotat_arg->view; + task = dotat_arg->task; + tatname = dns_fixedname_initname(&fixed); - result = dns_name_fromstring2(tatname, label, name, 0, NULL); + result = get_tat_qname(tatname, keytable, keynode); if (result != ISC_R_SUCCESS) { return; } From a7657dc15076c93c1366776301063bb23b690f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 11 Jul 2018 08:27:10 +0200 Subject: [PATCH 2/3] Send upstream TAT queries for locally served zones Trying to resolve a trust anchor telemetry query for a locally served zone does not cause upstream queries to be sent as the response is determined just by consulting local data. Work around this issue by calling dns_view_findzonecut() first in order to determine the NS RRset for a given domain name and then passing the zone cut found to dns_resolver_createfetch(). Note that this change only applies to TAT queries generated by the resolver itself, not to ones received from downstream resolvers. --- bin/named/server.c | 74 ++++++++++++++++++----- bin/tests/system/mirror/ns3/named.args | 1 + bin/tests/system/mirror/ns3/named.conf.in | 1 + bin/tests/system/mirror/tests.sh | 8 +++ util/copyrights | 1 + 5 files changed, 71 insertions(+), 14 deletions(-) create mode 100644 bin/tests/system/mirror/ns3/named.args diff --git a/bin/named/server.c b/bin/named/server.c index 3d0b9860ad..a21ea70597 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6502,7 +6502,8 @@ struct dotat_arg { /*% * Prepare the QNAME for the TAT query to be sent by processing the trust - * anchors present at 'keynode' of 'keytable'. Store the result in 'dst'. + * anchors present at 'keynode' of 'keytable'. Store the result in 'dst' and + * the domain name which 'keynode' is associated with in 'origin'. * * A maximum of 12 key IDs can be reported in a single TAT query due to the * 63-octet length limit for any single label in a domain name. If there are @@ -6510,11 +6511,10 @@ struct dotat_arg { * reported in the TAT query. */ static isc_result_t -get_tat_qname(dns_name_t *dst, dns_keytable_t *keytable, - dns_keynode_t *keynode) +get_tat_qname(dns_name_t *dst, const dns_name_t **origin, + dns_keytable_t *keytable, dns_keynode_t *keynode) { dns_keynode_t *firstnode = keynode; - const dns_name_t *origin; dns_keynode_t *nextnode; unsigned int i, n = 0; isc_uint16_t ids[12]; @@ -6522,10 +6522,12 @@ get_tat_qname(dns_name_t *dst, dns_keytable_t *keytable, char label[64]; int m; + REQUIRE(origin != NULL && *origin == NULL); + do { dst_key_t *key = dns_keynode_key(keynode); if (key != NULL) { - origin = dst_key_name(key); + *origin = dst_key_name(key); if (n < (sizeof(ids)/sizeof(ids[0]))) { ids[n] = dst_key_id(key); n++; @@ -6567,15 +6569,17 @@ get_tat_qname(dns_name_t *dst, dns_keytable_t *keytable, isc_textregion_consume(&r, m); } - return (dns_name_fromstring2(dst, label, origin, 0, NULL)); + return (dns_name_fromstring2(dst, label, *origin, 0, NULL)); } static void dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { struct dotat_arg *dotat_arg = arg; char namebuf[DNS_NAME_FORMATSIZE]; - dns_fixedname_t fixed; - dns_name_t *tatname; + const dns_name_t *origin = NULL; + dns_fixedname_t fixed, fdomain; + dns_name_t *tatname, *domain; + dns_rdataset_t nameservers; isc_result_t result; dns_view_t *view; isc_task_t *task; @@ -6589,7 +6593,7 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { task = dotat_arg->task; tatname = dns_fixedname_initname(&fixed); - result = get_tat_qname(tatname, keytable, keynode); + result = get_tat_qname(tatname, &origin, keytable, keynode); if (result != ISC_R_SUCCESS) { return; } @@ -6613,12 +6617,54 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { isc_mem_attach(dotat_arg->view->mctx, &tat->mctx); isc_task_attach(task, &tat->task); - result = dns_resolver_createfetch(view->resolver, tatname, - dns_rdatatype_null, NULL, NULL, - NULL, NULL, 0, 0, 0, NULL, tat->task, - tat_done, tat, &tat->rdataset, - &tat->sigrdataset, &tat->fetch); + /* + * TAT queries should be sent to the authoritative servers for a given + * zone. If this function is called for a keytable node corresponding + * to a locally served zone, calling dns_resolver_createfetch() with + * NULL 'domain' and 'nameservers' arguments will cause 'tatname' to be + * resolved locally, without sending any TAT queries upstream. + * + * Work around this issue by calling dns_view_findzonecut() first. If + * the zone is served locally, the NS RRset for the given domain name + * will be retrieved from local data; if it is not, the deepest zone + * cut we have for it will be retrieved from cache. In either case, + * passing the results to dns_resolver_createfetch() will prevent it + * from returning NXDOMAIN for 'tatname' while still allowing it to + * chase down any potential delegations returned by upstream servers in + * order to eventually find the destination host to send the TAT query + * to. + * + * 'origin' holds the domain name at 'keynode', i.e. the domain name + * for which the trust anchors to be reported by this TAT query are + * defined. + * + * After the dns_view_findzonecut() call, 'domain' will hold the + * deepest zone cut we can find for 'origin' while 'nameservers' will + * hold the NS RRset at that zone cut. + */ + domain = dns_fixedname_initname(&fdomain); + dns_rdataset_init(&nameservers); + result = dns_view_findzonecut(view, origin, domain, 0, 0, ISC_TRUE, + ISC_TRUE, &nameservers, NULL); + if (result != ISC_R_SUCCESS) { + goto done; + } + result = dns_resolver_createfetch(view->resolver, tatname, + dns_rdatatype_null, domain, + &nameservers, NULL, NULL, 0, 0, 0, + NULL, tat->task, tat_done, tat, + &tat->rdataset, &tat->sigrdataset, + &tat->fetch); + + /* + * dns_resolver_createfetch() creates its own copies of 'domain' and + * 'nameservers'; clean up the latter (the former points into a + * dst_key_t structure and thus must not be freed). + */ + dns_rdataset_disassociate(&nameservers); + + done: if (result != ISC_R_SUCCESS) { isc_task_detach(&tat->task); isc_mem_putanddetach(&tat->mctx, tat, sizeof(*tat)); diff --git a/bin/tests/system/mirror/ns3/named.args b/bin/tests/system/mirror/ns3/named.args new file mode 100644 index 0000000000..f4038878e8 --- /dev/null +++ b/bin/tests/system/mirror/ns3/named.args @@ -0,0 +1 @@ +-X named.lock -m record,size,mctx -T clienttest -c named.conf -d 99 -g -U 4 -T tat=1 diff --git a/bin/tests/system/mirror/ns3/named.conf.in b/bin/tests/system/mirror/ns3/named.conf.in index 627b82f3db..a54cdf4900 100644 --- a/bin/tests/system/mirror/ns3/named.conf.in +++ b/bin/tests/system/mirror/ns3/named.conf.in @@ -28,6 +28,7 @@ options { listen-on-v6 { none; }; recursion yes; allow-query-cache { 10.53.0.1; }; + trust-anchor-telemetry yes; }; zone "." { diff --git a/bin/tests/system/mirror/tests.sh b/bin/tests/system/mirror/tests.sh index 95eb93f734..d4cff6ad84 100644 --- a/bin/tests/system/mirror/tests.sh +++ b/bin/tests/system/mirror/tests.sh @@ -404,5 +404,13 @@ nextpart ns3/named.run | grep "No correct RSASHA256 signature for verify-reconfi if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo_i "ensuring trust anchor telemetry queries are sent upstream for a mirror zone ($n)" +ret=0 +# ns3 is started with "-T tat=1", so TAT queries should have already been sent. +grep "_ta-[-0-9a-f]*/NULL" ns1/named.run > /dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/util/copyrights b/util/copyrights index 6c8af60bea..29797a94e4 100644 --- a/util/copyrights +++ b/util/copyrights @@ -1573,6 +1573,7 @@ ./bin/tests/system/mirror/ns2/sign.sh SH 2018 ./bin/tests/system/mirror/ns2/sub.example.db.in ZONE 2018 ./bin/tests/system/mirror/ns2/verify.db.in ZONE 2018 +./bin/tests/system/mirror/ns3/named.args X 2018 ./bin/tests/system/mirror/ns3/named.conf.in CONF-C 2018 ./bin/tests/system/mirror/setup.sh SH 2018 ./bin/tests/system/mirror/tests.sh SH 2018 From a64750e4285d2fcc71c3c9e3a61a0902ab4693ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 11 Jul 2018 08:27:10 +0200 Subject: [PATCH 3/3] Add CHANGES entry 4994. [bug] Trust anchor telemetry queries were not being sent upstream for locally served zones. [GL #392] --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 7aebc442e8..6c985e47c8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4994. [bug] Trust anchor telemetry queries were not being sent + upstream for locally served zones. [GL #392] + 4993. [cleanup] Remove support for silently ignoring 'no-change' deltas from BIND 8 when processing an IXFR stream. 'no-change' deltas will now trigger a fallback to AXFR as the