diff --git a/CHANGES b/CHANGES index a25b896a8c..869fb98810 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +3548. [bug] The NSID request code in resolver.c was broken + resulting in invalid EDNS options being sent. + [RT #33153] + 3547. [bug] Some malformed unknown rdata records were not properly detected and rejected. [RT #33129] diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index d8edf96c5a..d73ceda1ce 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -251,6 +251,12 @@ struct dns_message { const void * order_arg; }; +struct dns_ednsopt { + isc_uint16_t code; + isc_uint16_t length; + unsigned char *value; +}; + /*** *** Functions ***/ @@ -1359,6 +1365,24 @@ dns_message_logpacket(dns_message_t *message, const char *description, * normally end with a newline. */ +isc_result_t +dns_message_buildopt(dns_message_t *msg, dns_rdataset_t **opt, + unsigned int version, isc_uint16_t udpsize, + unsigned int flags, dns_ednsopt_t *ednsopts, size_t count); +/*%< + * Built a opt record. + * + * Requires: + * \li msg be a valid message. + * \li opt to be a non NULL and *opt to be NULL. + * + * Returns: + * \li ISC_R_SUCCESS on success. + * \li ISC_R_NOMEMORY + * \li ISC_R_NOSPACE + * \li other. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index 894dc17b0d..7324a973a2 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -77,6 +77,7 @@ typedef struct dns_dnsseckey dns_dnsseckey_t; typedef ISC_LIST(dns_dnsseckey_t) dns_dnsseckeylist_t; typedef isc_uint8_t dns_dsdigest_t; typedef struct dns_dumpctx dns_dumpctx_t; +typedef struct dns_ednsopt dns_ednsopt_t; typedef struct dns_fetch dns_fetch_t; typedef struct dns_fixedname dns_fixedname_t; typedef struct dns_forwarders dns_forwarders_t; diff --git a/lib/dns/message.c b/lib/dns/message.c index 7a0f6a0534..a492a6a9c9 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -2642,9 +2642,9 @@ dns_message_setopt(dns_message_t *msg, dns_rdataset_t *opt) { return (ISC_R_SUCCESS); cleanup: + dns_rdataset_disassociate(opt); dns_message_puttemprdataset(msg, &opt); return (result); - } dns_rdataset_t * @@ -3497,3 +3497,95 @@ dns_message_logpacket(dns_message_t *message, const char *description, if (buf != NULL) isc_mem_put(mctx, buf, len); } + +isc_result_t +dns_message_buildopt(dns_message_t *message, dns_rdataset_t **rdatasetp, + unsigned int version, isc_uint16_t udpsize, + unsigned int flags, dns_ednsopt_t *ednsopts, size_t count) +{ + dns_rdataset_t *rdataset = NULL; + dns_rdatalist_t *rdatalist = NULL; + dns_rdata_t *rdata = NULL; + isc_result_t result; + size_t len = 0, i; + + REQUIRE(DNS_MESSAGE_VALID(message)); + REQUIRE(rdatasetp != NULL && *rdatasetp == NULL); + + result = dns_message_gettemprdatalist(message, &rdatalist); + if (result != ISC_R_SUCCESS) + return (result); + result = dns_message_gettemprdata(message, &rdata); + if (result != ISC_R_SUCCESS) + goto cleanup; + result = dns_message_gettemprdataset(message, &rdataset); + if (result != ISC_R_SUCCESS) + goto cleanup; + dns_rdataset_init(rdataset); + + rdatalist->type = dns_rdatatype_opt; + rdatalist->covers = 0; + + /* + * Set Maximum UDP buffer size. + */ + rdatalist->rdclass = udpsize; + + /* + * Set EXTENDED-RCODE and Z to 0. + */ + rdatalist->ttl = (version << 16); + rdatalist->ttl |= (flags & 0xffff); + + /* + * Set EDNS options if applicable + */ + if (count != 0) { + isc_buffer_t *buf = NULL; + for (i = 0; i < count; i++) + len += ednsopts[i].length + 4; + + if (len > 0xffffU) { + result = ISC_R_NOSPACE; + goto cleanup; + } + + result = isc_buffer_allocate(message->mctx, &buf, len); + if (result != ISC_R_SUCCESS) + goto cleanup; + + for (i = 0; i < count; i++) { + isc_buffer_putuint16(buf, ednsopts[i].code); + isc_buffer_putuint16(buf, ednsopts[i].length); + isc_buffer_putmem(buf, ednsopts[i].value, + ednsopts[i].length); + } + rdata->data = isc_buffer_base(buf); + rdata->length = len; + dns_message_takebuffer(message, &buf); + } else { + rdata->data = NULL; + rdata->length = 0; + } + + rdata->rdclass = rdatalist->rdclass; + rdata->type = rdatalist->type; + rdata->flags = 0; + + ISC_LIST_INIT(rdatalist->rdata); + ISC_LIST_APPEND(rdatalist->rdata, rdata, link); + result = dns_rdatalist_tordataset(rdatalist, rdataset); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + + *rdatasetp = rdataset; + return (ISC_R_SUCCESS); + + cleanup: + if (rdata != NULL) + dns_message_puttemprdata(message, &rdata); + if (rdataset != NULL) + dns_message_puttemprdataset(message, &rdataset); + if (rdatalist != NULL) + dns_message_puttemprdatalist(message, &rdatalist); + return (result); +} diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 90cf907828..625170acee 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -134,6 +134,7 @@ * Maximum EDNS0 input packet size. */ #define RECV_BUFFER_SIZE 4096 /* XXXRTH Constant. */ +#define EDNSOPTS 2 /*% * This defines the maximum number of timeouts we will permit before we @@ -1288,67 +1289,15 @@ resquery_senddone(isc_task_t *task, isc_event_t *event) { static inline isc_result_t fctx_addopt(dns_message_t *message, unsigned int version, - isc_uint16_t udpsize, isc_boolean_t request_nsid) + isc_uint16_t udpsize, dns_ednsopt_t *ednsopts, size_t count) { - dns_rdataset_t *rdataset; - dns_rdatalist_t *rdatalist; - dns_rdata_t *rdata; + dns_rdataset_t *rdataset = NULL; isc_result_t result; - rdatalist = NULL; - result = dns_message_gettemprdatalist(message, &rdatalist); + result = dns_message_buildopt(message, &rdataset, version, udpsize, + DNS_MESSAGEEXTFLAG_DO, ednsopts, count); if (result != ISC_R_SUCCESS) return (result); - rdata = NULL; - result = dns_message_gettemprdata(message, &rdata); - if (result != ISC_R_SUCCESS) - return (result); - rdataset = NULL; - result = dns_message_gettemprdataset(message, &rdataset); - if (result != ISC_R_SUCCESS) - return (result); - dns_rdataset_init(rdataset); - - rdatalist->type = dns_rdatatype_opt; - rdatalist->covers = 0; - - /* - * Set Maximum UDP buffer size. - */ - rdatalist->rdclass = udpsize; - - /* - * Set EXTENDED-RCODE and Z to 0, DO to 1. - */ - rdatalist->ttl = (version << 16); - rdatalist->ttl |= DNS_MESSAGEEXTFLAG_DO; - - /* - * Set EDNS options if applicable - */ - if (request_nsid) { - /* Send empty NSID option (RFC5001) */ - unsigned char data[4]; - isc_buffer_t buf; - - isc_buffer_init(&buf, data, sizeof(data)); - isc_buffer_putuint16(&buf, DNS_OPT_NSID); - isc_buffer_putuint16(&buf, 0); - rdata->data = data; - rdata->length = sizeof(data); - } else { - rdata->data = NULL; - rdata->length = 0; - } - - rdata->rdclass = rdatalist->rdclass; - rdata->type = rdatalist->type; - rdata->flags = 0; - - ISC_LIST_INIT(rdatalist->rdata); - ISC_LIST_APPEND(rdatalist->rdata, rdata, link); - RUNTIME_CHECK(dns_rdatalist_tordataset(rdatalist, rdataset) == ISC_R_SUCCESS); - return (dns_message_setopt(message, rdataset)); } @@ -1748,6 +1697,8 @@ resquery_send(resquery_t *query) { isc_boolean_t cleanup_cctx = ISC_FALSE; isc_boolean_t secure_domain; isc_boolean_t connecting = ISC_FALSE; + dns_ednsopt_t ednsopts[EDNSOPTS]; + unsigned ednsopt = 0; fctx = query->fctx; QTRACE("send"); @@ -1930,8 +1881,15 @@ resquery_send(resquery_t *query) { /* request NSID for current view or peer? */ if (peer != NULL) (void) dns_peer_getrequestnsid(peer, &reqnsid); + if (reqnsid) { + INSIST(ednsopt < EDNSOPTS); + ednsopts[ednsopt].code = DNS_OPT_NSID; + ednsopts[ednsopt].length = 0; + ednsopts[ednsopt].value = NULL; + ednsopt++; + } result = fctx_addopt(fctx->qmessage, version, - udpsize, reqnsid); + udpsize, ednsopts, ednsopt); if (reqnsid && result == ISC_R_SUCCESS) { query->options |= DNS_FETCHOPT_WANTNSID; } else if (result != ISC_R_SUCCESS) { @@ -6711,44 +6669,24 @@ checknames(dns_message_t *message) { /* * Log server NSID at log level 'level' */ -static isc_result_t -log_nsid(dns_rdataset_t *opt, resquery_t *query, int level, isc_mem_t *mctx) +static void +log_nsid(isc_buffer_t *opt, size_t nsid_len, resquery_t *query, + int level, isc_mem_t *mctx) { static const char hex[17] = "0123456789abcdef"; char addrbuf[ISC_SOCKADDR_FORMATSIZE]; - isc_uint16_t optcode, nsid_len, buflen, i; - isc_result_t result; - isc_buffer_t nsidbuf; - dns_rdata_t rdata; + isc_uint16_t buflen, i; unsigned char *p, *buf, *nsid; - /* Extract rdata from OPT rdataset */ - result = dns_rdataset_first(opt); - if (result != ISC_R_SUCCESS) - return (ISC_R_FAILURE); - - dns_rdata_init(&rdata); - dns_rdataset_current(opt, &rdata); - if (rdata.length < 4) - return (ISC_R_FAILURE); - - /* Check for NSID */ - isc_buffer_init(&nsidbuf, rdata.data, rdata.length); - isc_buffer_add(&nsidbuf, rdata.length); - optcode = isc_buffer_getuint16(&nsidbuf); - nsid_len = isc_buffer_getuint16(&nsidbuf); - if (optcode != DNS_OPT_NSID || nsid_len == 0) - return (ISC_R_FAILURE); - /* Allocate buffer for storing hex version of the NSID */ buflen = nsid_len * 2 + 1; buf = isc_mem_get(mctx, buflen); if (buf == NULL) - return (ISC_R_NOSPACE); + return; /* Convert to hex */ p = buf; - nsid = rdata.data + 4; + nsid = isc_buffer_current(opt); for (i = 0; i < nsid_len; i++) { *p++ = hex[(nsid[0] >> 4) & 0xf]; *p++ = hex[nsid[0] & 0xf]; @@ -6764,7 +6702,7 @@ log_nsid(dns_rdataset_t *opt, resquery_t *query, int level, isc_mem_t *mctx) /* Clean up */ isc_mem_put(mctx, buf, buflen); - return (ISC_R_SUCCESS); + return; } static isc_boolean_t @@ -6800,6 +6738,41 @@ betterreferral(fetchctx_t *fctx) { return (ISC_FALSE); } +static void +process_opt(resquery_t *query, dns_rdataset_t *opt) { + dns_rdata_t rdata; + isc_buffer_t optbuf; + isc_result_t result; + isc_uint16_t optcode; + isc_uint16_t optlen; + + result = dns_rdataset_first(opt); + if (result == ISC_R_SUCCESS) { + dns_rdata_init(&rdata); + dns_rdataset_current(opt, &rdata); + isc_buffer_init(&optbuf, rdata.data, rdata.length); + isc_buffer_add(&optbuf, rdata.length); + while (isc_buffer_remaininglength(&optbuf) >= 4) { + optcode = isc_buffer_getuint16(&optbuf); + optlen = isc_buffer_getuint16(&optbuf); + INSIST(optlen <= isc_buffer_remaininglength(&optbuf)); + switch (optcode) { + case DNS_OPT_NSID: + if (query->options & DNS_FETCHOPT_WANTNSID) + log_nsid(&optbuf, optlen, query, + ISC_LOG_INFO, + query->fctx->res->mctx); + isc_buffer_forward(&optbuf, optlen); + break; + default: + isc_buffer_forward(&optbuf, optlen); + break; + } + } + INSIST(isc_buffer_remaininglength(&optbuf) == 0U); + } +} + static void resquery_response(isc_task_t *task, isc_event_t *event) { isc_result_t result = ISC_R_SUCCESS; @@ -6991,12 +6964,11 @@ resquery_response(isc_task_t *task, isc_event_t *event) { ISC_LOG_DEBUG(10), fctx->res->mctx); /* - * Did we request NSID? If so, and if the response contains - * NSID data, log it at INFO level. + * Process receive opt record. */ opt = dns_message_getopt(message); - if (opt != NULL && (query->options & DNS_FETCHOPT_WANTNSID) != 0) - log_nsid(opt, query, ISC_LOG_INFO, fctx->res->mctx); + if (opt != NULL) + process_opt(query, opt); /* * If the message is signed, check the signature. If not, this