From 4fdd248f5312ae4cdd72c8f07b0056e731356889 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. (cherry picked from commit 2e7dd0d61f66412fbf79a76f0748ca668117bbed) --- bin/named/server.c | 81 +++++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 29 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 59a8998f2c..c4f90bac50 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6089,35 +6089,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; 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]; + dns_name_t *origin; 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++; @@ -6130,8 +6127,9 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { keynode = nextnode; } while (keynode != NULL); - if (n == 0) - return; + if (n == 0) { + return (DNS_R_EMPTYNAME); + } if (n > 1) qsort(ids, n, sizeof(ids[0]), cid); @@ -6144,19 +6142,44 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) { r.base = label; r.length = sizeof(label);; m = snprintf(r.base, r.length, "_ta"); - if (m < 0 || (unsigned)m > r.length) - return; + if (m < 0 || (unsigned)m > r.length) { + 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; + if (m < 0 || (unsigned)m > r.length) { + 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); - if (result != ISC_R_SUCCESS) + result = get_tat_qname(tatname, keytable, keynode); + if (result != ISC_R_SUCCESS) { return; + } dns_name_format(tatname, namebuf, sizeof(namebuf)); isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_SERVER, From d4a6cb321bc23898a936c7b58581c3f4646df3f7 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. (cherry picked from commit 873c091408b492755fbfc0e509e77d7606a0f8ea) --- bin/named/server.c | 71 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 13 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index c4f90bac50..a39fdea6d7 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6091,7 +6091,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 @@ -6099,22 +6100,23 @@ struct dotat_arg { * reported in the TAT query. */ static isc_result_t -get_tat_qname(dns_name_t *dst, dns_keytable_t *keytable, +get_tat_qname(dns_name_t *dst, dns_name_t **origin, dns_keytable_t *keytable, dns_keynode_t *keynode) { dns_keynode_t *firstnode = keynode; dns_keynode_t *nextnode; unsigned int i, n = 0; isc_uint16_t ids[12]; - dns_name_t *origin; isc_textregion_t r; 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++; @@ -6154,15 +6156,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; + dns_fixedname_t fixed, fdomain; + dns_name_t *tatname, *domain; + dns_rdataset_t nameservers; + dns_name_t *origin = NULL; isc_result_t result; dns_view_t *view; isc_task_t *task; @@ -6176,7 +6180,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; } @@ -6199,12 +6203,53 @@ 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, 0, 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, + &nameservers, NULL); + if (result != ISC_R_SUCCESS) { + goto done; + } + result = dns_resolver_createfetch(view->resolver, tatname, + dns_rdatatype_null, domain, + &nameservers, NULL, 0, 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)); From 3c710a851cb90a9c420f127e9a4eca08f7e157ef 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] (cherry picked from commit e54cddc0c113c06fbcb589d3b75d64b06e0e9d99) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 3ff6131262..a81c2d846c 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] + 4992. [bug] The wrong address was being logged for trust anchor telemetry queries. [GL #379]