The Samba-Bugzilla – Bug 10471
Incorrect NXDOMAIN response when records of another type exist
Last modified: 2014-04-04 18:55:06 UTC
SAMBA DNS server incorrectly responds with NXDOMAIN when data for the domain name does exist but is not of the type requested.
The example most likely to arise in production is an IPv6 only entry in DNS, where a host will query for the A record, receive NXDOMAIN and not proceed any further because it believes the host does not exist.
Example wireshark output talking to a Samba server: (Using ping from a Windows client)
1 DNS Standard query A servername.XXX.XXX.org
2 DNS Standard query response, No such name
The same server from the same client, making an IPv6 specific request: (In this case, ping -6)
1 DNS Standard query AAAA servername.XXX.XXX.org
2 DNS Standard query response AAAA 2001:xxxx:xxxx:2::1:15
Correct behavior - note empty non-error response at packet 2.
1 DNS Standard query A ipv6.l.google.com
2 DNS Standard query response
3 DNS Standard query AAAA ipv6.l.google.com
4 DNS Standard query response AAAA 2a00:1450:4009:809::1004
Tests above performed against 4.0.7, but bug appears to be present in all versions.
The trouble seems to stem from handle_question in dns_query.c, which looks as if it returns NAME_ERROR if no records are to be returned.
that seems weird, I'm pretty sure the NXDOMAIN reply complies with the RFC. Could you attach the wireshark capture so I could have a look at that?
Created attachment 9724 [details]
Example of correct NODATA query response
Created attachment 9725 [details]
Bad DNS behavior
Note times - this is two separate attempts to ping the host, one with just "ping" which fails and the other with "ping -6".
I've attached two pcaps, one showing automatic query of the AAAA record using google, the other where it has to be done by hand due to the NXDOMAIN.
RFC1034 section 6.2.4 covers this particular case.
reading the rfc i think zoe is right. if a name in dns exists (with whatever record type) we should not return any DNS error but an emtpy answer instead. This is what other name servers also do. This is also why the "host" command against Samba 4 servers usually return even for existing A records two NXDOMAIN errors - which come from the MX and AAAA lookups that "host" does by default.
Thanks for the captures, and the RFC reference. You're clearly correct, this is a bug in the DNS server. I'll work on a patch.
Created attachment 9734 [details]
Fix the invalid NXDOMAIN return.
This patch should fix the observed issue.
Tested this patch and it resolves the issue. Will it make it into 4.1.6 and/or 4.0.16?
Once I've got this patch into master, I'll prepare the patches for 4.0.next and 4.1.next. The code won't have changed much, I don't expect any trouble.
(In reply to comment #9)
> Once I've got this patch into master, I'll prepare the patches for 4.0.next and
> 4.1.next. The code won't have changed much, I don't expect any trouble.
Kai, This is now in master. Do you need help getting this back-ported ?
Jeremy, I was on the road the past days, I'll try to make the time to look at this tomorrow. Should be straightforward, the DNS code didn't change much in that area.
Created attachment 9771 [details]
Patch for 4.1 and 4.0
This patch applies cleanly to both v4-0-test and v4-1-test.
Comment on attachment 9771 [details]
Patch for 4.1 and 4.0
LGTM for 4.0.x and 4.1.x.
Re-assigning to Karolin for inclusion in 4.1.next and 4.0.next.
Kai, one external note (nothing to do with this bug but I noticed in test compile). On compiling I get:
[2185/4132] Compiling source4/dns_server/dns_query.c
../source4/dns_server/dns_query.c: In function ‘handle_question’:
../source4/dns_server/dns_query.c:331:23: warning: comparison between ‘enum dns_record_type’ and ‘const enum dns_qtype’ [-Wenum-compare]
which looks incorrect. Any chance you can take a look !
These enums are coming from two different IDL files, but have identical numerical values. It'd be possible to add casts, but that'd make the code much harder to read. Not sure how to best fix this.
Thanks - no problem, let's let Metze's warnings patch take care of it. Doesn't affect Karolin merging this change.
(In reply to comment #17)
> Thanks - no problem, let's let Metze's warnings patch take care of it. Doesn't
> affect Karolin merging this change.
Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Pushed to v4-0-test and v4-1-test.
Closing out bug report.