ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Do not set qctx->result to DNS_R_SERVFAIL unless necessary

Browse Source 
In some cases, setting qctx->result to DNS_R_SERVFAIL causes the value
of a 'result' variable containing a more specific failure reason to be
effectively discarded.  This may cause certain query error log messages
to lack specificity despite a more accurate problem cause being
determined during query processing.

In other cases, qctx->result is set to DNS_R_SERVFAIL even though a more
specific error (e.g. ISC_R_NOMEMORY) could be explicitly indicated.

Since the response message's RCODE is derived from qctx->result using
dns_result_torcode(), which handles a number of possible isc_result_t
values and returns SERVFAIL for anything not explicitly listed, it is
fine to set qctx->result to something more specific than DNS_R_SERVFAIL
(in fact, this is already being done in a few cases).  Modify most
QUERY_ERROR() calls so that qctx->result is set to a more specific error
code when possible.  Adjust query_error() so that statistics are still
calculated properly.  Remove the RECURSE_ERROR() macro which was
introduced exactly because qctx->result could be set to DNS_R_SERVFAIL
instead of DNS_R_DUPLICATE or DNS_R_DROP, which need special handling.
Modify dns_sdlz_putrr() so that it returns DNS_R_SERVFAIL when a DLZ
driver returns invalid RDATA, in order to prevent setting RCODE to
FORMERR (which is what dns_result_torcode() translates e.g. DNS_R_SYNTAX
to) while responding authoritatively.
This commit is contained in:
Michał Kępień
2018-10-08 12:47:28 +02:00
parent b3cd868c4b
commit ba91243542
2 changed files with 28 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
lib/ns/query.c
View File

@@ -94,14 +94,6 @@ do { \
qctx->line = __LINE__; \
} while (0)
#define RECURSE_ERROR(qctx, r) \
do { \
if ((r) == DNS_R_DUPLICATE || (r) == DNS_R_DROP) \
QUERY_ERROR(qctx, r); \
else \
QUERY_ERROR(qctx, DNS_R_SERVFAIL); \
} while (0)
/*% Partial answer? */
#define PARTIALANSWER(c) (((c)->query.attributes & \
NS_QUERYATTR_PARTIALANSWER) != 0)
@@ -522,12 +514,12 @@ static void
query_error(ns_client_t *client, isc_result_t result, int line) {
int loglevel = ISC_LOG_DEBUG(3);
switch (result) {
case DNS_R_SERVFAIL:
switch (dns_result_torcode(result)) {
case dns_rcode_servfail:
loglevel = ISC_LOG_DEBUG(1);
inc_stats(client, ns_statscounter_servfail);
break;
case DNS_R_FORMERR:
case dns_rcode_formerr:
inc_stats(client, ns_statscounter_formerr);
break;
default:
@@ -5376,7 +5368,7 @@ ns__query_start(query_ctx_t *qctx) {
} else {
CCTRACE(ISC_LOG_ERROR,
"ns__query_start: query_getdb failed");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, result);
}
return (query_done(qctx));
}
@@ -5458,7 +5450,7 @@ query_lookup(query_ctx_t *qctx) {
if (ISC_UNLIKELY(qctx->dbuf == NULL)) {
CCTRACE(ISC_LOG_ERROR,
"query_lookup: query_getnamebuf failed (2)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));
}
@@ -5468,7 +5460,7 @@ query_lookup(query_ctx_t *qctx) {
if (ISC_UNLIKELY(qctx->fname == NULL || qctx->rdataset == NULL)) {
CCTRACE(ISC_LOG_ERROR,
"query_lookup: query_newname failed (2)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));
}
@@ -5479,7 +5471,7 @@ query_lookup(query_ctx_t *qctx) {
if (qctx->sigrdataset == NULL) {
CCTRACE(ISC_LOG_ERROR,
"query_lookup: query_newrdataset failed (2)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));
}
}
@@ -6008,7 +6000,7 @@ query_resume(query_ctx_t *qctx) {
if (qctx->dbuf == NULL) {
CCTRACE(ISC_LOG_ERROR,
"query_resume: query_getnamebuf failed (1)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));
}
@@ -6016,7 +6008,7 @@ query_resume(query_ctx_t *qctx) {
if (qctx->fname == NULL) {
CCTRACE(ISC_LOG_ERROR,
"query_resume: query_newname failed (1)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));
}
@@ -6033,7 +6025,7 @@ query_resume(query_ctx_t *qctx) {
if (result != ISC_R_SUCCESS) {
CCTRACE(ISC_LOG_ERROR,
"query_resume: dns_name_copy failed");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, result);
return (query_done(qctx));
}
@@ -6317,8 +6309,8 @@ query_checkrpz(query_ctx_t *qctx, isc_result_t result) {
qctx->client->query.attributes |= NS_QUERYATTR_RECURSING;
return (ISC_R_COMPLETE);
default:
RECURSE_ERROR(qctx, rresult);
return (ISC_R_COMPLETE);;
QUERY_ERROR(qctx, rresult);
return (ISC_R_COMPLETE);
}
if (qctx->rpz_st->m.policy != DNS_RPZ_POLICY_MISS) {
@@ -6760,7 +6752,7 @@ query_gotanswer(query_ctx_t *qctx, isc_result_t result) {
*/
return (query_lookup(qctx));
}
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, result);
return (query_done(qctx));
}
}
@@ -6866,7 +6858,7 @@ query_respond_any(query_ctx_t *qctx) {
if (result != ISC_R_SUCCESS) {
CCTRACE(ISC_LOG_ERROR,
"query_respond_any: allrdatasets failed");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, result);
return (query_done(qctx));
}
@@ -7075,7 +7067,7 @@ query_respond_any(query_ctx_t *qctx) {
if (result != ISC_R_NOMORE) {
CCTRACE(ISC_LOG_ERROR,
"query_respond_any: dns_rdatasetiter_destroy failed");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, result);
} else {
query_addauth(qctx);
}
@@ -7261,7 +7253,7 @@ query_respond(query_ctx_t *qctx) {
qctx->client->query.attributes |=
NS_QUERYATTR_DNS64EXCLUDE;
} else {
RECURSE_ERROR(qctx, result);
QUERY_ERROR(qctx, result);
}
return (query_done(qctx));
@@ -7778,14 +7770,15 @@ query_notfound(query_ctx_t *qctx) {
if (qctx->dns64_exclude)
qctx->client->query.attributes |=
NS_QUERYATTR_DNS64EXCLUDE;
} else
RECURSE_ERROR(qctx, result);
} else {
QUERY_ERROR(qctx, result);
}
return (query_done(qctx));
} else {
/* Unable to give root server referral. */
CCTRACE(ISC_LOG_ERROR,
"unable to give root server referral");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, result);
return (query_done(qctx));
}
}
@@ -8041,10 +8034,8 @@ query_delegation(query_ctx_t *qctx) {
if (qctx->dns64_exclude)
qctx->client->query.attributes |=
NS_QUERYATTR_DNS64EXCLUDE;
} else if (result == DNS_R_DUPLICATE || result == DNS_R_DROP) {
QUERY_ERROR(qctx, result);
} else {
RECURSE_ERROR(qctx, result);
QUERY_ERROR(qctx, result);
}
return (query_done(qctx));
@@ -8211,7 +8202,7 @@ query_nodata(query_ctx_t *qctx, isc_result_t result) {
CCTRACE(ISC_LOG_ERROR,
"query_nodata: "
"query_getnamebuf failed (3)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));;
}
qctx->fname = query_newname(qctx->client,
@@ -8220,7 +8211,7 @@ query_nodata(query_ctx_t *qctx, isc_result_t result) {
CCTRACE(ISC_LOG_ERROR,
"query_nodata: "
"query_newname failed (3)");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));;
}
}
@@ -8366,7 +8357,7 @@ query_sign_nodata(query_ctx_t *qctx) {
"query_sign_nodata: "
"failure getting "
"closest encloser");
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
QUERY_ERROR(qctx, ISC_R_NOMEMORY);
return (query_done(qctx));
}
/*
@@ -9427,7 +9418,7 @@ query_cname(query_ctx_t *qctx) {
qctx->client->query.attributes |=
NS_QUERYATTR_DNS64EXCLUDE;
} else {
RECURSE_ERROR(qctx, result);
QUERY_ERROR(qctx, result);
}
return (query_done(qctx));