[Dnsmasq-discuss] [dnsmasq][dns query]dns query failed if the first server replis REFUSE

[Mi Bear]

Hello Everyone,

I found an issue about DNS query. In my test scenario, there are two DNS
servers, and the first one will always return REFUSE, and the second one
can work properly. And the strict order option is on.

In this case, I expect the a domain name can be resolved correctly by the
second DNS server.

But I saw a DNS query packet was sent to the first server, and received a
REFUSE from it, and I got REFUSED as the the final result at the LAN side
PC. I did not see the DNS query packet sent to the second DNS server.


I checked the source code, I think the following part of code is hard to be
understood.

---------------------
I copied it here from dnsmasq-2.76

Line 788,function reply_query, in forward.c:

/* Note: if we send extra options in the EDNS0 header, we can't recreate
the query from the reply. */
if (RCODE(header) == REFUSED &&
*!*option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */
{

The meaning of this part code is, for broken servers, attempt to send to
another one, if:
1. strict order is *NOT* set
2. REFUSED got from a server
3. forwardall is 0
4. some conditions else

according to my understanding, if the option strict order is *set*, I think
dnsmasq will forward the DNS query packet to DNS servers one by one in the
list. If the first refused the query, dnsmasq should forward the query to
the second one.

But in this part of code, if the option strict order is *NOT* set and got
refused, (also with some other conditions), dnsmasq would try to send to
another one. It's different from my understanding.

--------------------
Also in the source code of function forward_query, I can see, if option
strict order is *NOT *set, forwardall would be set as* 1*.

So the condition 1(strict order is* not *set) and 3(forwardall is* 0*) in
function reply_query would never be matched together, and no dns query
would be sent to the second DNS server in my test case, just as what I saw.


I think the "!" in the condition 1 in function reply_query should be
removed as below. It's more reasonable. I tested the modified source code,
and it worked fine in my test case.


/* Note: if we send extra options in the EDNS0 header, we can't recreate
the query from the reply. */
if (RCODE(header) == REFUSED &&
option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */
{


I beg your help or comments on this issue.

[Baptiste Jonglez]

Hi,

Post by Mi Bear
Hello Everyone,
I found an issue about DNS query. In my test scenario, there are two DNS
servers, and the first one will always return REFUSE, and the second one
can work properly. And the strict order option is on.
In this case, I expect the a domain name can be resolved correctly by the
second DNS server.
But I saw a DNS query packet was sent to the first server, and received a
REFUSE from it, and I got REFUSED as the the final result at the LAN side
PC. I did not see the DNS query packet sent to the second DNS server.

You're right, it's a bug, introduced in 2.76. It has been fixed in
v2.77test2, but unfortunately the final version of 2.77 has apparently not
been released yet.

More details here: http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=68f6312d4bae30b78daafcd6f51dc441b8685b1e

Post by Mi Bear
I checked the source code, I think the following part of code is hard to be
understood.
---------------------
I copied it here from dnsmasq-2.76
/* Note: if we send extra options in the EDNS0 header, we can't recreate
the query from the reply. */
if (RCODE(header) == REFUSED &&
*!*option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */
{
The meaning of this part code is, for broken servers, attempt to send to
1. strict order is *NOT* set
2. REFUSED got from a server
3. forwardall is 0
4. some conditions else
according to my understanding, if the option strict order is *set*, I think
dnsmasq will forward the DNS query packet to DNS servers one by one in the
list. If the first refused the query, dnsmasq should forward the query to
the second one.
But in this part of code, if the option strict order is *NOT* set and got
refused, (also with some other conditions), dnsmasq would try to send to
another one. It's different from my understanding.
--------------------
Also in the source code of function forward_query, I can see, if option
strict order is *NOT *set, forwardall would be set as* 1*.
So the condition 1(strict order is* not *set) and 3(forwardall is* 0*) in
function reply_query would never be matched together, and no dns query
would be sent to the second DNS server in my test case, just as what I saw.
I think the "!" in the condition 1 in function reply_query should be
removed as below. It's more reasonable. I tested the modified source code,
and it worked fine in my test case.
/* Note: if we send extra options in the EDNS0 header, we can't recreate
the query from the reply. */
if (RCODE(header) == REFUSED &&
option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */
{
I beg your help or comments on this issue.
_______________________________________________
Dnsmasq-discuss mailing list
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

[Mi Bear]

Hi, Baptiste and all,

Thank you for your reply, and sorry for my late reply. :b

I made a test build based on 2.76, with this diff in the link in your mail,
but it did not fixed my issue.

Let me give more detail information of my case:
1. option strictorder (OPT_ORDER in source code) is on, that dnsmasq should
send query to upstream DNS servers one by one.

2. there are two upstream DNS servers, and the first one would always
return REFUSED, and the second one can work well


-----------------------------
But with following diff, it can work well, and got domain name resolved
correctly.


diff --git a/forward.c b/forward.c


index 726b5df..7fcf960 100644
--- a/forward.c
+++ b/forward.c
@@ -788,7 +788,7 @@ void reply_query(int fd, int family, time_t now)
/* Note: if we send extra options in the EDNS0 header, we can't recreate
the query from the reply. */
if (RCODE(header) == REFUSED &&
- !option_bool(OPT_ORDER) &&
+ option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */


I think it's a logical error, that from this in-line comment "*/* for
broken servers, attempt to send to another one. */*", it's obviously a
strict order scenario (to send to upstream server one by one).

But the condition selector is the strict order option (OPT_ORDER) should
NOT be set.

So I think it's a logical error. For more source code analyzing, please see
my previous mail.


Please help me to check this issue.


Thanks a lot
Mi Feng

Post by Baptiste Jonglez
Hi,

Post by Mi Bear
Hello Everyone,
I found an issue about DNS query. In my test scenario, there are two DNS
servers, and the first one will always return REFUSE, and the second one
can work properly. And the strict order option is on.
In this case, I expect the a domain name can be resolved correctly by the
second DNS server.
But I saw a DNS query packet was sent to the first server, and received a
REFUSE from it, and I got REFUSED as the the final result at the LAN side
PC. I did not see the DNS query packet sent to the second DNS server.

You're right, it's a bug, introduced in 2.76. It has been fixed in
v2.77test2, but unfortunately the final version of 2.77 has apparently not
been released yet.
More details here: http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=
commit;h=68f6312d4bae30b78daafcd6f51dc441b8685b1e

Post by Mi Bear
I checked the source code, I think the following part of code is hard to

be

Post by Mi Bear
understood.
---------------------
I copied it here from dnsmasq-2.76
/* Note: if we send extra options in the EDNS0 header, we can't

recreate

Post by Mi Bear
the query from the reply. */
if (RCODE(header) == REFUSED &&
*!*option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */
{
The meaning of this part code is, for broken servers, attempt to send to
1. strict order is *NOT* set
2. REFUSED got from a server
3. forwardall is 0
4. some conditions else
according to my understanding, if the option strict order is *set*, I

think

Post by Mi Bear
dnsmasq will forward the DNS query packet to DNS servers one by one in

the

Post by Mi Bear
list. If the first refused the query, dnsmasq should forward the query to
the second one.
But in this part of code, if the option strict order is *NOT* set and got
refused, (also with some other conditions), dnsmasq would try to send to
another one. It's different from my understanding.
--------------------
Also in the source code of function forward_query, I can see, if option
strict order is *NOT *set, forwardall would be set as* 1*.
So the condition 1(strict order is* not *set) and 3(forwardall is* 0*) in
function reply_query would never be matched together, and no dns query
would be sent to the second DNS server in my test case, just as what I

saw.

Post by Mi Bear
I think the "!" in the condition 1 in function reply_query should be
removed as below. It's more reasonable. I tested the modified source

code,

Post by Mi Bear
and it worked fine in my test case.
/* Note: if we send extra options in the EDNS0 header, we can't

recreate

Post by Mi Bear
the query from the reply. */
if (RCODE(header) == REFUSED &&
option_bool(OPT_ORDER) &&
forward->forwardall == 0 &&
!(forward->flags & FREC_HAS_EXTRADATA))
/* for broken servers, attempt to send to another one. */
{
I beg your help or comments on this issue.
_______________________________________________
Dnsmasq-discuss mailing list
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss