From fb807b8c2d30472838067de43ed5473870bd0107 Mon Sep 17 00:00:00 2001 From: =?utf8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Fri, 15 Jan 2021 00:49:38 +0100 Subject: [PATCH] selection: fix DNSSEC_BOGUS/NSNXAttack mitigation interaction When cancelling a query due to NSNXAttack mitigation when validator was also in BOGUS state, records wouldn't be stripped from the answer. --- lib/selection_iter.c | 68 +++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/lib/selection_iter.c b/lib/selection_iter.c index e9870c13d..f7727c620 100644 --- a/lib/selection_iter.c +++ b/lib/selection_iter.c @@ -249,49 +249,40 @@ void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport int choices_len = get_valid_addresses(local_state, choices); int resolvable_len = get_resolvable_names(local_state, resolvable, qry); - if (choices_len || resolvable_len) { - bool tcp = qry->flags.TCP || qry->server_selection.local_state->truncated; - *transport = select_transport( - choices, choices_len, resolvable, resolvable_len, - qry->server_selection.local_state->timeouts, mempool, - tcp, NULL); - if (*transport) { - switch ((*transport)->protocol) { - case KR_TRANSPORT_RESOLVE_A: - case KR_TRANSPORT_RESOLVE_AAAA: - /* Note that we tried resolving this name to not try it again. */ - update_name_state((*transport)->ns_name, - (*transport)->protocol, - local_state->names); - break; - case KR_TRANSPORT_TLS: - case KR_TRANSPORT_TCP: - /* We need to propagate this to flags since it's used in - * other parts of the resolver. */ - qry->flags.TCP = true; - break; - default: + bool tcp = qry->flags.TCP || qry->server_selection.local_state->truncated; + *transport = select_transport(choices, choices_len, resolvable, resolvable_len, + qry->server_selection.local_state->timeouts, + mempool, tcp, NULL); + bool nxnsattack_mitigation = false; + + if (*transport) { + switch ((*transport)->protocol) { + case KR_TRANSPORT_RESOLVE_A: + case KR_TRANSPORT_RESOLVE_AAAA: + if (++local_state->no_ns_addr_count > KR_COUNT_NO_NSADDR_LIMIT) { + *transport = NULL; + nxnsattack_mitigation = true; break; } - } - } else { - *transport = NULL; - /* Last selected server had broken DNSSEC and now we have no more - * servers to ask. We signal this to the rest of resolver by - * setting DNSSEC_BOGUS flag. */ - if (local_state->last_error == KR_SELECTION_DNSSEC_ERROR) { - qry->flags.DNSSEC_BOGUS = true; + /* Note that we tried resolving this name to not try it again. */ + update_name_state((*transport)->ns_name, (*transport)->protocol, local_state->names); + break; + case KR_TRANSPORT_TLS: + case KR_TRANSPORT_TCP: + /* We need to propagate this to flags since it's used in + * other parts of the resolver. */ + qry->flags.TCP = true; + break; + default: + break; } } - bool nxnsattack_mitigation = false; - const enum kr_transport_protocol proto = - *transport ? (*transport)->protocol : -1; - if (proto == KR_TRANSPORT_RESOLVE_A || proto == KR_TRANSPORT_RESOLVE_AAAA) { - if (++local_state->no_ns_addr_count > KR_COUNT_NO_NSADDR_LIMIT) { - *transport = NULL; - nxnsattack_mitigation = true; - } + if (*transport == NULL && local_state->last_error == KR_SELECTION_DNSSEC_ERROR) { + /* Last selected server had broken DNSSEC and now we have no more + * servers to ask. We signal this to the rest of resolver by + * setting DNSSEC_BOGUS flag. */ + qry->flags.DNSSEC_BOGUS = true; } WITH_VERBOSE(qry) @@ -299,6 +290,7 @@ void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport KR_DNAME_GET_STR(zonecut_str, qry->zone_cut.name); if (*transport) { KR_DNAME_GET_STR(ns_name, (*transport)->ns_name); + const enum kr_transport_protocol proto = *transport ? (*transport)->protocol : -1; const char *ns_str = kr_straddr(&(*transport)->address.ip); const char *ip_version; switch (proto) -- 2.47.2