From 7402615697072799fddd21156ecf7822c05321a2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 22 Apr 2019 17:19:23 -0700 Subject: [PATCH] force SERVFAIL response in the gotanswer failure case - named could return FORMERR if parsing iterative responses ended with a result code such as DNS_R_OPTERR. instead of computing a response code based on the result, in this case we now just force the response to be SERVFAIL. --- lib/ns/client.c | 32 +++++++++++++++++++++++--------- lib/ns/include/ns/client.h | 8 ++++++++ lib/ns/query.c | 11 +++++++---- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/lib/ns/client.c b/lib/ns/client.c index f008827f81..12fb45f474 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1427,7 +1427,12 @@ ns_client_error(ns_client_t *client, isc_result_t result) { CTRACE("error"); message = client->message; - rcode = dns_result_torcode(result); + + if (client->rcode_override == -1) { + rcode = dns_result_torcode(result); + } else { + rcode = (dns_rcode_t)(client->rcode_override & 0xfff); + } #if NS_CLIENT_DROPPORT /* @@ -1435,13 +1440,15 @@ ns_client_error(ns_client_t *client, isc_result_t result) { */ if (rcode == dns_rcode_formerr && ns_client_dropport(isc_sockaddr_getport(&client->peeraddr)) != - DROPPORT_NO) { + DROPPORT_NO) + { char buf[64]; isc_buffer_t b; isc_buffer_init(&b, buf, sizeof(buf) - 1); - if (dns_rcode_totext(rcode, &b) != ISC_R_SUCCESS) + if (dns_rcode_totext(rcode, &b) != ISC_R_SUCCESS) { isc_buffer_putstr(&b, "UNKNOWN RCODE"); + } ns_client_log(client, DNS_LOGCATEGORY_SECURITY, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(10), "dropped error (%.*s) response: suspicious port", @@ -1462,10 +1469,11 @@ ns_client_error(ns_client_t *client, isc_result_t result) { INSIST(rcode != dns_rcode_noerror && rcode != dns_rcode_nxdomain); - if ((client->sctx->options & NS_SERVER_LOGQUERIES) != 0) + if ((client->sctx->options & NS_SERVER_LOGQUERIES) != 0) { loglevel = DNS_RRL_LOG_DROP; - else + } else { loglevel = ISC_LOG_DEBUG(1); + } wouldlog = isc_log_wouldlog(ns_lctx, loglevel); rrl_result = dns_rrl(client->view, &client->peeraddr, TCP_CLIENT(client), @@ -1570,12 +1578,14 @@ ns_client_error(ns_client_t *client, isc_result_t result) { isc_interval_set(&i, client->view->fail_ttl, 0); result = isc_time_nowplusinterval(&expire, &i); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { dns_badcache_add(client->view->failcache, client->query.qname, client->query.qtype, true, flags, &expire); + } } + ns_client_send(client); } @@ -3058,6 +3068,7 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { ISC_QLINK_INIT(client, ilink); client->keytag = NULL; client->keytag_len = 0; + client->rcode_override = -1; /* not set */ /* * We call the init routines for the various kinds of client here, @@ -3519,12 +3530,13 @@ get_client(ns_clientmgr_t *manager, ns_interface_t *ifp, * if that fails, make a new one. */ client = NULL; - if ((manager->sctx->options & NS_SERVER_CLIENTTEST) == 0) + if ((manager->sctx->options & NS_SERVER_CLIENTTEST) == 0) { ISC_QUEUE_POP(manager->inactive, ilink, client); + } - if (client != NULL) + if (client != NULL) { MTRACE("recycle"); - else { + } else { MTRACE("create new"); LOCK(&manager->lock); @@ -3545,6 +3557,7 @@ get_client(ns_clientmgr_t *manager, ns_interface_t *ifp, INSIST(client->recursionquota == NULL); client->dscp = ifp->dscp; + client->rcode_override = -1; /* not set */ if (tcp) { client->attributes |= NS_CLIENTATTR_TCP; @@ -3615,6 +3628,7 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock) { client->pipelined = true; client->mortal = true; client->sendcb = NULL; + client->rcode_override = -1; /* not set */ isc_socket_attach(ifp->tcpsocket, &client->tcplistener); isc_socket_attach(sock, &client->tcpsocket); diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index cd6c9c8a47..f1c85d5fe9 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -170,6 +170,14 @@ struct ns_client { uint32_t expire; unsigned char *keytag; uint16_t keytag_len; + + /*% + * Used to override the DNS response code in ns_client_error(). + * If set to -1, the rcode is determined from the result code, + * but if set to any other value, the least significant 12 + * bits will be used as the rcode in the response message. + */ + int32_t rcode_override; }; typedef ISC_QUEUE(ns_client_t) client_queue_t; diff --git a/lib/ns/query.c b/lib/ns/query.c index 3b6b3dc3d1..6e9ed7aad7 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -6749,10 +6749,6 @@ query_gotanswer(query_ctx_t *qctx, isc_result_t res) { case DNS_R_DNAME: return (query_dname(qctx)); - case DNS_R_FORMERR: - QUERY_ERROR(qctx, DNS_R_SERVFAIL); - return (ns_query_done(qctx)); - default: /* * Something has gone wrong. @@ -6768,6 +6764,13 @@ query_gotanswer(query_ctx_t *qctx, isc_result_t res) { */ return (query_lookup(qctx)); } + + /* + * Regardless of the triggering result, we definitely + * want to return SERVFAIL from here. + */ + qctx->client->rcode_override = dns_rcode_servfail; + QUERY_ERROR(qctx, result); return (ns_query_done(qctx)); }