Bug 5342 - (patch) res_query silently rejects responses against T_ANY DNS questions
Summary: (patch) res_query silently rejects responses against T_ANY DNS questions
Status: NEW
Alias: None
Product: uClibc
Classification: Unclassified
Component: Networking (show other bugs)
Version: 0.9.33.2
Hardware: All All
: P5 minor
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-08 03:21 UTC by Chris Luke
Modified: 2012-08-06 23:00 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:


Attachments
resolv-any.patch (736 bytes, patch)
2012-07-08 03:21 UTC, Chris Luke
Details
code to test the issue (2.73 KB, text/x-csrc)
2012-07-08 14:38 UTC, Chris Luke
Details
code to test the issue, v2 (5.63 KB, text/x-csrc)
2012-08-06 18:22 UTC, Bernhard Reutner-Fischer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Luke 2012-07-08 03:21:24 UTC
Created attachment 4394 [details]
resolv-any.patch

res_query() performs a DNS query using the internal __dns_lookup function. That much works perfectly. However, it rejects the response if the type of the first answer returned from the DNS server does not exactly match the original question type.

Also, when rejecting the response it still returns the length of the packet received (instead of -1) without filling out the contents of the user-supplied packet buffer. Without a rejection indication, parsing of the results naturally fails - in my case, with ns_initparse() being fed an uninitialized buffer of rubbish.

This is problematic since, when one performs a T_ANY query, you're explicitly saying "I want anything that matches this domain name, regardless of type". Additionally, when one does a T_A or T_AAAA query you can end up with a T_CNAME answer, and it will generally be the first answer.

The attached patch resolves both the unduly strict answer type matching and the failure to indicate rejection.

If the question type was T_ANY then it allows any answer type. If the question type was T_A or T_AAAA then it allows a T_CNAME answer.

If it rejects an answer for not satisfying this condition, it returns -1, as you would expect.

This patch has been tested on OpenWrt (trunk) against uClibc 0.9.33.2.
Comment 1 Chris Luke 2012-07-08 14:36:57 UTC
I've confirmed this using uClibc buildroot on i386, too.  See attached nsquery.c which does a simple T_ANY query, but pre-initializes the packet buffer to 0xAA55 and compares the contents after the query.

On glibc, the output is this:

$ ./nsquery-glibc www.google.com
resCount: 1
query=www.google.com answer=www.l.google.com type=CNAME ttl=41905
resCount: 7
query=www.l.google.com answer=2001:4860:800a::67 type=AAAA ttl=244
query=www.l.google.com answer=173.194.73.105 type=A ttl=75
query=www.l.google.com answer=173.194.73.106 type=A ttl=75
query=www.l.google.com answer=173.194.73.147 type=A ttl=75
query=www.l.google.com answer=173.194.73.99 type=A ttl=75
query=www.l.google.com answer=173.194.73.103 type=A ttl=75
query=www.l.google.com answer=173.194.73.104 type=A ttl=75


On unpatched uClibc:

$ ./nsquery-uclib www.google.com
DNS query for "www.google.com" failed: res_query claimed to return 188 bytes, but the buffer was unchanged.

Patched uClibc:

$ ./nsquery-uclibc www.google.com
resCount: 1
query=www.google.com answer=www.l.google.com type=CNAME ttl=41900
resCount: 7
query=www.l.google.com answer=2001:4860:800a::67 type=AAAA ttl=240
query=www.l.google.com answer=173.194.73.105 type=A ttl=70
query=www.l.google.com answer=173.194.73.106 type=A ttl=70
query=www.l.google.com answer=173.194.73.147 type=A ttl=70
query=www.l.google.com answer=173.194.73.99 type=A ttl=70
query=www.l.google.com answer=173.194.73.103 type=A ttl=70
query=www.l.google.com answer=173.194.73.104 type=A ttl=70
Comment 2 Chris Luke 2012-07-08 14:38:32 UTC
Created attachment 4400 [details]
code to test the issue

execute with a DNS entry to query on the command line. It will perform a T_ANY query and chase down any CNAME responses.
Comment 3 Bernhard Reutner-Fischer 2012-08-06 18:22:45 UTC
Created attachment 4478 [details]
code to test the issue, v2

slightly improved test-code
Comment 4 Bernhard Reutner-Fischer 2012-08-06 18:26:30 UTC
I think the proper thing to do is remove the type-filtering from res_query altogether. __dns_lookup is supposed to return the proper stuff that you asked for (and only that), so i suppose it should be ok to just

@@ -3730,12 +3737,9 @@ int res_query(const char *dname, int class, int type,
        }
 
        free(a.dotted);
-
-       if (a.atype == type) { /* CNAME */
-               if (i > anslen)
-                       i = anslen;
-               memcpy(answer, packet, i);
-       }
+       if (i > anslen)
+               i = anslen;
+       memcpy(answer, packet, i);
        free(packet);
        return i;
 }
Comment 5 Chris Luke 2012-08-06 23:00:46 UTC
I agree that it shouldn't be filtering the results at all. I wasn't sure which would be least surprising a change for anything that expects this behaviour!

I like your improved nsquery.c too :)