From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 18987 invoked from network); 23 Oct 2022 15:32:46 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 23 Oct 2022 15:32:46 -0000 Received: (qmail 23926 invoked by uid 550); 23 Oct 2022 15:32:43 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 23888 invoked from network); 23 Oct 2022 15:32:42 -0000 Date: Sun, 23 Oct 2022 11:32:32 -0400 From: Rich Felker To: Qingtao Cao Cc: musl@lists.openwall.com, Qingtao Cao Message-ID: <20221023153229.GR29905@brightrain.aerifal.cx> References: <20221023111801.515290-1-qingtao.cao@digi.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221023111801.515290-1-qingtao.cao@digi.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH 1/1] musl/resolver: serialise querying name servers On Sun, Oct 23, 2022 at 09:18:01PM +1000, Qingtao Cao wrote: > Querying all configured DNS servers in parallel boost performance but makes > it impossible to prefer private DNS servers to public ones. Serialise DNS > servers access from the top of the nameserver specified in resolv.conf and > only move on to the next, or less preferred DNS server when the current one > either timed out or replied NOERROR and NXDOMAIN for all queries > > When querying the next or less perferred DNS servers, only unsuccessfully > resolved host names were queried. When replies are received, they won't replace > a perviously successful answer. This ensures successful answers from higher > prioritised DNS servers won't be overridden > > This eliminates a racy condition when different DNS servers replies differently > to a query, the last received one wins > > Last but not least, using a local array to save the currently received reply to > simplify the logic to re-shuffle answers[next] to the corresponding answers[i] > > Signed-off-by: Qingtao Cao This patch is being submitted as one to change a very intentional and longstanding behavior (and with that impose a new contract) and is not going to be accepted. A basic premise of musl's resolver, that's not just arbitrary but a consequence of a "fallback" model, is that all the nameservers provided present a consistent view of a global dns namespace. Some people mistake "fallback" for "search" because it kinda appears to work that way if you don't actually question what happens in the corner cases. But if you're using fallback as search, failure of the primary server to answer in time will *change the results* of the query. This is not how fallback is supposed to work. It would have been a lot more productive to start from a standpoint of what you're trying to do. I assume that's "unioning" where you have a local nameserver serving fake names that don't actually exist in the real DNS namespace, and a real nameserver that's serving the real DNS namespace. There are at least 2 valid ways to achieve this that don't break in the above manner. 1. Run a unioning nameserver locally. The unioning nameserver handles all the complex logic of handling clashes and which upstream source takes precedence, querying both and only answering if it gets sufficient answers to draw a conclusive result about which result it can use (and ServFail otherwise). 2. Put your local names under an actual DNS domain (or subdomain zone) you actually own. Set the public DNS delegations to point to nameservers that don't exist. Now the public DNS will always ServFail. Set your private local nameserver to only return results for this local zone, and ServFail otherwise. As a result, neither is providing conflicting answers for the part of the namespace the other is responsible for. There are likely other ways too. > --- > src/network/res_msend.c | 100 +++++++++++++++++++++++++++------------- > 1 file changed, 68 insertions(+), 32 deletions(-) > > diff --git a/src/network/res_msend.c b/src/network/res_msend.c > index 9adaea13..130ac4fe 100644 > --- a/src/network/res_msend.c > +++ b/src/network/res_msend.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include "stdio_impl.h" > #include "syscall.h" > #include "lookup.h" > @@ -41,11 +42,12 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries, > int nns = 0; > int family = AF_INET; > int rlen; > - int next; > - int i, j; > + int i; > int cs; > struct pollfd pfd; > unsigned long t0, t1, t2; > + int ns_index, rcode, all_done, unanswered; > + unsigned char answer[512]; > > pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); > > @@ -115,19 +117,27 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries, > pfd.fd = fd; > pfd.events = POLLIN; > retry_interval = timeout / attempts; > - next = 0; > + > + ns_index = 0; /* Start from the top name server */ > + > +next_ns: > t0 = t2 = mtime(); > t1 = t2 - retry_interval; > > + unanswered = 0; > + > for (; t2-t0 < timeout; t2=mtime()) { > if (t2-t1 >= retry_interval) { > - /* Query all configured namservers in parallel */ > - for (i=0; i - if (!alens[i]) > - for (j=0; j - sendto(fd, queries[i], > - qlens[i], MSG_NOSIGNAL, > - (void *)&ns[j], sl); > + for (i=0; i + /* Only query unresolved or negative resolved host names */ > + rcode = answers[i][3] & 15; > + if (!alens[i] || rcode != NOERROR) { > + sendto(fd, queries[i], > + qlens[i], MSG_NOSIGNAL, > + (void *)&ns[ns_index], sl); > + unanswered++; > + } > + } > t1 = t2; > servfail_retry = 2 * nqueries; > } > @@ -135,51 +145,77 @@ int __res_msend_rc(int nqueries, const unsigned char *const *queries, > /* Wait for a response, or until time to retry */ > if (poll(&pfd, 1, t1+retry_interval-t2) <= 0) continue; > > - while ((rlen = recvfrom(fd, answers[next], asize, 0, > + while ((rlen = recvfrom(fd, answer, 512, 0, > (void *)&sa, (socklen_t[1]){sl})) >= 0) { > > /* Ignore non-identifiable packets */ > if (rlen < 4) continue; > > /* Ignore replies from addresses we didn't send to */ > - for (j=0; j - if (j==nns) continue; > + if (memcmp(ns+ns_index, &sa, sl)) > + continue; > > /* Find which query this answer goes with, if any */ > - for (i=next; i - answers[next][0] != queries[i][0] || > - answers[next][1] != queries[i][1] ); i++); > + for (i=0; i + answer[0] != queries[i][0] || > + answer[1] != queries[i][1] ); i++); > if (i==nqueries) continue; > - if (alens[i]) continue; > + > + rcode = answers[i][3] & 15; > + if (alens[i] && rcode == NOERROR) { > + /* Do not override successful answers from a more prioritied server */ > + continue; > + } > > /* Only accept positive or negative responses; > * retry immediately on server failure, and ignore > * all other codes such as refusal. */ > - switch (answers[next][3] & 15) { > - case 0: > - case 3: > - break; > - case 2: > + rcode = answer[3] & 15; > + switch (rcode) { > + case NOERROR: > + case NXDOMAIN: > + alens[i] = rlen; > + memcpy(answers[i], answer, rlen); > + unanswered--; > + continue; > + case SERVFAIL: > if (servfail_retry && servfail_retry--) > sendto(fd, queries[i], > qlens[i], MSG_NOSIGNAL, > - (void *)&ns[j], sl); > + (void *)&ns[ns_index], sl); > default: > continue; > } > + } > > - /* Store answer in the right slot, or update next > - * available temp slot if it's already in place. */ > - alens[i] = rlen; > - if (i == next) > - for (; next - else > - memcpy(answers[i], answers[next], rlen); > + /* Check if all sent queries have got NOERROR or NXDOMAIN replies from this server, > + * no need to retry it anymore even when not timed out yet. When the unanswered counter > + * drops to zero, any remaining NXDOMAIN replies were sure sent from this server */ > + if (!unanswered) { > + all_done = 1; > + for (i = 0; i < nqueries; i++) { > + rcode = answers[i][3] & 15; > + if (!(alens[i] && ((rcode == NOERROR) || (rcode == NXDOMAIN)))) { > + all_done = 0; > + break; > + } > + } > > - if (next == nqueries) goto out; > + if (all_done) > + break; > } > } > -out: > + > + /* Check if any queries is unresolved or unsuccessful (regardless of failure code) > + * try the next DNS server if available */ > + if (++ns_index < conf->nns) { > + for (i = 0; i < nqueries; i++) { > + rcode = answers[i][3] & 15; > + if (!alens[i] || rcode != NOERROR) > + goto next_ns; > + } > + } > + > pthread_cleanup_pop(1); > > return 0; > -- > 2.34.1