Patchwork resolv/res_query.c - do not append search paths if number of dots in query is greater or equals ndots

login
register
mail settings
Submitter Canto .
Date Nov. 20, 2019, 7:49 p.m.
Message ID <CADk=2vb4_d5RMqouVTi+EMoX3ENuZN8eUv_ZaBWbVkya2vDvhA@mail.gmail.com>
Download mbox | patch
Permalink /patch/36072/
State New
Headers show

Comments

Canto . - Nov. 20, 2019, 7:49 p.m.
Hi all,

When NXDomain response is received, resolver appends original query
with search path and retries. It happens even if original query has
more dots than ndots.
It's causing unnecessary and excessive traffic to DNS servers,
especially with environments based on DNS service discovery, where
users often query for non-existent fqdns to local DNS server.

According to resolv.conf man page, search option  -
http://man7.org/linux/man-pages/man5/resolv.conf.5.html -
"Resolver queries having fewer than ndots dots (default is 1) in them
will be attempted using each component of the search path in turn
until a match is found."
Which seems as an expected and logical behavior (at least to me, thus
I don't think it's a documentation bug), but it's not the case.
Original BIND documentation also mention that search paths should be
appended to queries without any dots (.) -
https://downloads.isc.org/isc/bind8/doc/bog/file.lst
"The _ s_ e_ a_ r_ c_ h_ -_ l_ i_ s_ t is a list of  domains  which  are
 tried, in order, as qualifying domains for query-names
 which do not contain a "." "

This could also be a security threat, as regular users would not
expect adding a search path to NXDomain responses.
Search path is often controlled by dhcp server.

Thanks,
Oskar Wycislak


PoC:

15:49:38.603795 IP X.60296 > Y.53: 39152+ A?
this.is.a.non.existing.fqdnnn.com. (51)
15:49:38.604072 IP X.53 > Y.60296: 39152 NXDomain 0/1/0 (124)
15:49:38.604197 IP X.41202 > Y.53: 5283+ A?
this.is.a.non.existing.fqdnnn.com.nodes.company.int. (69)
15:49:38.625739 IP X.53 > Y.41202: 5283 NXDomain 0/1/0 (126)
15:49:38.625883 IP X.36961 > Y.53: 216+ A?
this.is.a.non.existing.fqdnnn.com.services.company.int. (72)
15:49:38.647587 IP X.53 > Y.36961: 216 NXDomain 0/1/0 (129)

Config:

cat /etc/resolv.conf
search nodes.company.int services.company.int
nameserver Y
options ndots:1

Patch test results (truncated):

cat tests.sum  | grep -i resolv
PASS: elf/resolvfail
PASS: resolv/check-abi-libanl
PASS: resolv/check-abi-libnss_dns
PASS: resolv/check-abi-libresolv
PASS: resolv/check-installed-headers-c
PASS: resolv/check-installed-headers-cxx
PASS: resolv/check-obsolete-constructs
PASS: resolv/check-wrapper-headers
PASS: resolv/mtrace-tst-leaks
PASS: resolv/mtrace-tst-resolv-res_ninit
PASS: resolv/tst-aton
PASS: resolv/tst-bug18665
PASS: resolv/tst-bug18665-tcp
PASS: resolv/tst-inet_aton_exact
PASS: resolv/tst-inet_ntop
PASS: resolv/tst-inet_pton
PASS: resolv/tst-leaks
PASS: resolv/tst-ns_name
PASS: resolv/tst-ns_name_compress
PASS: resolv/tst-ns_name_pton
PASS: resolv/tst-p_secstodate
PASS: resolv/tst-res_hconf_reorder
PASS: resolv/tst-res_hnok
UNSUPPORTED: resolv/tst-resolv-ai_idn
UNSUPPORTED: resolv/tst-resolv-ai_idn-latin1
PASS: resolv/tst-resolv-ai_idn-nolibidn2
PASS: resolv/tst-resolv-basic
PASS: resolv/tst-resolv-binary
PASS: resolv/tst-resolv-canonname
PASS: resolv/tst-resolv-edns
PASS: resolv/tst-resolv-network
PASS: resolv/tst-resolv-nondecimal
PASS: resolv/tst-resolv-res_init
PASS: resolv/tst-resolv-res_init-multi
PASS: resolv/tst-resolv-res_init-thread
PASS: resolv/tst-resolv-res_ninit
PASS: resolv/tst-resolv-search
PASS: resolv/tst-resolv-threads
PASS: resolv/tst-resolv-trailing

Patch against master branch below

                for (size_t domain_index = 0; !done; ++domain_index) {
Canto . - Dec. 9, 2019, 7:09 p.m.
What say you?

śr., 20 lis 2019 o 20:49 Canto . <cantorek@gmail.com> napisał(a):
>
> Hi all,
>
> When NXDomain response is received, resolver appends original query
> with search path and retries. It happens even if original query has
> more dots than ndots.
> It's causing unnecessary and excessive traffic to DNS servers,
> especially with environments based on DNS service discovery, where
> users often query for non-existent fqdns to local DNS server.
>
> According to resolv.conf man page, search option  -
> http://man7.org/linux/man-pages/man5/resolv.conf.5.html -
> "Resolver queries having fewer than ndots dots (default is 1) in them
> will be attempted using each component of the search path in turn
> until a match is found."
> Which seems as an expected and logical behavior (at least to me, thus
> I don't think it's a documentation bug), but it's not the case.
> Original BIND documentation also mention that search paths should be
> appended to queries without any dots (.) -
> https://downloads.isc.org/isc/bind8/doc/bog/file.lst
> "The _ s_ e_ a_ r_ c_ h_ -_ l_ i_ s_ t is a list of  domains  which  are
>  tried, in order, as qualifying domains for query-names
>  which do not contain a "." "
>
> This could also be a security threat, as regular users would not
> expect adding a search path to NXDomain responses.
> Search path is often controlled by dhcp server.
>
> Thanks,
> Oskar Wycislak
>
>
> PoC:
>
> 15:49:38.603795 IP X.60296 > Y.53: 39152+ A?
> this.is.a.non.existing.fqdnnn.com. (51)
> 15:49:38.604072 IP X.53 > Y.60296: 39152 NXDomain 0/1/0 (124)
> 15:49:38.604197 IP X.41202 > Y.53: 5283+ A?
> this.is.a.non.existing.fqdnnn.com.nodes.company.int. (69)
> 15:49:38.625739 IP X.53 > Y.41202: 5283 NXDomain 0/1/0 (126)
> 15:49:38.625883 IP X.36961 > Y.53: 216+ A?
> this.is.a.non.existing.fqdnnn.com.services.company.int. (72)
> 15:49:38.647587 IP X.53 > Y.36961: 216 NXDomain 0/1/0 (129)
>
> Config:
>
> cat /etc/resolv.conf
> search nodes.company.int services.company.int
> nameserver Y
> options ndots:1
>
> Patch test results (truncated):
>
> cat tests.sum  | grep -i resolv
> PASS: elf/resolvfail
> PASS: resolv/check-abi-libanl
> PASS: resolv/check-abi-libnss_dns
> PASS: resolv/check-abi-libresolv
> PASS: resolv/check-installed-headers-c
> PASS: resolv/check-installed-headers-cxx
> PASS: resolv/check-obsolete-constructs
> PASS: resolv/check-wrapper-headers
> PASS: resolv/mtrace-tst-leaks
> PASS: resolv/mtrace-tst-resolv-res_ninit
> PASS: resolv/tst-aton
> PASS: resolv/tst-bug18665
> PASS: resolv/tst-bug18665-tcp
> PASS: resolv/tst-inet_aton_exact
> PASS: resolv/tst-inet_ntop
> PASS: resolv/tst-inet_pton
> PASS: resolv/tst-leaks
> PASS: resolv/tst-ns_name
> PASS: resolv/tst-ns_name_compress
> PASS: resolv/tst-ns_name_pton
> PASS: resolv/tst-p_secstodate
> PASS: resolv/tst-res_hconf_reorder
> PASS: resolv/tst-res_hnok
> UNSUPPORTED: resolv/tst-resolv-ai_idn
> UNSUPPORTED: resolv/tst-resolv-ai_idn-latin1
> PASS: resolv/tst-resolv-ai_idn-nolibidn2
> PASS: resolv/tst-resolv-basic
> PASS: resolv/tst-resolv-binary
> PASS: resolv/tst-resolv-canonname
> PASS: resolv/tst-resolv-edns
> PASS: resolv/tst-resolv-network
> PASS: resolv/tst-resolv-nondecimal
> PASS: resolv/tst-resolv-res_init
> PASS: resolv/tst-resolv-res_init-multi
> PASS: resolv/tst-resolv-res_init-thread
> PASS: resolv/tst-resolv-res_ninit
> PASS: resolv/tst-resolv-search
> PASS: resolv/tst-resolv-threads
> PASS: resolv/tst-resolv-trailing
>
> Patch against master branch below
>
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index ebbe5a6..75bb349 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -385,11 +385,11 @@ __res_context_search (struct resolv_context *ctx,
>         /*
>          * We do at least one level of search if
>          *      - there is no dot and RES_DEFNAME is set, or
> -        *      - there is at least one dot, there is no trailing dot,
> +        *      - there is less dots than ndots, there is no trailing dot,
>          *        and RES_DNSRCH is set.
>          */
>         if ((!dots && (statp->options & RES_DEFNAMES) != 0) ||
> -           (dots && !trailing_dot && (statp->options & RES_DNSRCH) != 0)) {
> +           ((dots < statp->ndots) && !trailing_dot && (statp->options
> & RES_DNSRCH) != 0)) {
>                 int done = 0;
>
>                 for (size_t domain_index = 0; !done; ++domain_index) {

Patch

diff --git a/resolv/res_query.c b/resolv/res_query.c
index ebbe5a6..75bb349 100644
--- a/resolv/res_query.c
+++ b/resolv/res_query.c
@@ -385,11 +385,11 @@  __res_context_search (struct resolv_context *ctx,
        /*
         * We do at least one level of search if
         *      - there is no dot and RES_DEFNAME is set, or
-        *      - there is at least one dot, there is no trailing dot,
+        *      - there is less dots than ndots, there is no trailing dot,
         *        and RES_DNSRCH is set.
         */
        if ((!dots && (statp->options & RES_DEFNAMES) != 0) ||
-           (dots && !trailing_dot && (statp->options & RES_DNSRCH) != 0)) {
+           ((dots < statp->ndots) && !trailing_dot && (statp->options
& RES_DNSRCH) != 0)) {
                int done = 0;