]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
selection: fix DNSSEC_BOGUS/NSNXAttack mitigation interaction
authorŠtěpán Balážik <stepan.balazik@nic.cz>
Thu, 14 Jan 2021 23:49:38 +0000 (00:49 +0100)
committerŠtěpán Balážik <stepan.balazik@nic.cz>
Mon, 25 Jan 2021 14:42:55 +0000 (15:42 +0100)
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

index e9870c13d56c2a0d3b9c06c7ecbbf72c2103be56..f7727c62082ce6879d274f47ea227f801fd12057 100644 (file)
@@ -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)