[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.


-- 
Best Regards and Many Thanks
Bear Mi
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Re: [Dnsmasq-discuss] [PATCH] Nack requests for unknown leases.

[Simon Kelley]

On 24/04/17 10:16, Alin Năstac wrote:
> On Sun, Apr 23, 2017 at 5:46 PM, Simon Kelley  wrote:
>> On 20/04/17 10:34, Alin Nastac wrote:
>>> Hosts that migrate from one network to another could request their
>>> old IP address which might be already in use by another statically
>>> configured host. Currently non-authoritative dnsmasq servers will
>>> ignore such requests, but ISC DHCP client will send discovery packets
>>> next carrying the same requested IP address and dnsmasq will end up
>>> allocating a new lease for it without checking first if is already
>>> used by another host.
>>
>>
>> When the client sends the discovery packet, dnsmasq will notice that the
>> requested address is in use by another client, and offer a different
>> address instead.
> 
> You did not understood the scenario. The host that already use the
> requested IP address is statically configured to use it (in other
> words dnsmasq does not have a lease for the given IP address).
> 
> While at it, you might consider fixing the scenario in which a client
> fills a DHCP discovery message with an option-50 containing an IP
> address that is already used by another statically configured host.
> 

At the DHCPDISCOVER stage, both the server and the client are supposed
to check if an address in in use. The server sends an ICMP echo request
and checks there's no answer. The client sends an ARP who-has request.
These checks should be enough to avoid address-stealing, but it's also
best not to overlap address ranges configured for DHCP allocation with
addresses of non-DHCP configured hosts.

Cheers,

Simon.


___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Re: [Dnsmasq-discuss] [PATCH] Logging of dhcp_script output

[Petr Mensik]

Thank you for accepting that patches. I agree that some garbage is far more 
likely to appear in dhcp-script mode.
I would myself welcome error log from wrong formatted lease file as well. If I 
understand it well, that file will be overwritten after the first lease 
created. If it contains wrong data, just log an error, but do not terminate.

It should not surprise administrator that just disabled IPv6 support. Error 
message would be logged once at startup until the first lease is created. Then 
file will be rewritten without any IPv6 leases, because they were skipped 
during the reading. I would accept one error message as notification some 
leases are gone forever.

I will have to ask whether failure to start on database corruption is 
considered a problem. It was silently ignoring all problems before. Now it 
fails to start the service completely if any error occurs. I think it relied on 
auto recovery with empty leases in the same way as with plain file. I think 
there should be a way to override default behavior. I will check for more 
opinions and get back with results.

Thank you Simon.

Cheers,
Petr


--
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com  PGP: 65C6C973


- Original Message -
From: "Simon Kelley" 
To: "Petr Mensik" 
Cc: dnsmasq-discuss@lists.thekelleys.org.uk
Sent: Sunday, April 23, 2017 3:14:08 PM
Subject: Re: [Dnsmasq-discuss] [PATCH] Logging of dhcp_script output

OK, there's a little subtlety here that needs to be taken into account.


The lease file format was not originally designed to be extensible, so
the code takes advantage (sort of) of the fact that lines which are
associated with IPv6 leases make no sense to older versions of dnsmasq
or even current versions compiled without IPv6 support.

The order of lines when the file is written is always

IPv4 leases
duid 
IPv6 leases

(I think duid may not get written in some cases - doesn't matter for the
sake of this argument.)

Therefore the behaviour of silently giving up when an unrecognisable
line is encountered is a feature, not a bug, as it allows old or
non-IPv6 dnsmasq binaries to work with newer lease files - they get the
IPv4 leases and ignore the rest.

Since the main problem you're trying to solve is unexpected error
messages from the script, the solution to this is to keep the old
behaviour when reading from a file, but the new behaviour when running
the script init command.

Given that we're only checking when using the init script, I think
die()ing is the best thing to do when parsing fails.

I've moved your patch on to do that, and to check ferror() on the
stream, to catch IO-errors.

Again, any new  bugs are mine :)

Cheers,

Simon.


On 19/04/17 19:14, Petr Mensik wrote:
> Hi Simon,
> 
> I like your changes. New shorter log is definitely helpful, separate log 
> section helps. The only bug I found is red white space highlight in the patch.
> 
> However I did yet another fix for remaining dhcp-script init action.
> It completely ignored any error in structure and silently skipped the rest of 
> database. If there was any message in stdin of init script, it just died 
> silently.
> The only thing that were visible was SIGPIPE from dhcp-script, because it did 
> not read whole database, if that signal was logged at all.
> My new patch handles garbage in leases database. If the line is wrong, it 
> logs part of wrong line and skips the rest of init.
> I thought about die in that case. I think it would be better for backward 
> compatibility to start with empty leases as before.
> 
> --
> Petr Menšík
> Software Engineer
> Red Hat, http://www.redhat.com/
> email: pemen...@redhat.com  PGP: 65C6C973
> 
> - Original Message -
> From: "Simon Kelley" 
> To: dnsmasq-discuss@lists.thekelleys.org.uk
> Sent: Sunday, April 16, 2017 9:29:21 PM
> Subject: Re: [Dnsmasq-discuss] [PATCH] Logging of dhcp_script output
> 
> I like this. Yes, I know you can do it with shell magic, but this is
> easier and what I would expect to happen.
> 
> I've changed the patch quite a lot:
> 
> 1) Don't go to large effort to report "never happen" errors from pipe(),
> just silently handle them in the same way as fork()
> 
> 2) Don't do any of this when the -d debug flag is in effect, as it's
> already defined that the script gets stdin, stdout and stderr from the
> dnsmasq process in that case.
> 
> 3) Expand the subject-based logging that already exists, DHCP stuff
> comes from dnsmasq-dhcp, script output comes from dhcp-script. That
> avoids the wordy preamble to every line otherwise.
> 
> 4) Pull the copy-to-log code out of the loop wait()ing for processes to
> die, it makes more sense to iterate until the descriptors close, then
> reap child processes.
> 
> 5) Rationalise conditional compilation stuff. There may be more of that
> in a subsequent commit.
> 
> 6) Update the man page to reflect new reality (!)
> 
> Any 

Re: [Dnsmasq-discuss] [PATCH] Nack requests for unknown leases.

[wkitty42]

On 04/24/2017 05:16 AM, Alin Năstac wrote:

On Sun, Apr 23, 2017 at 5:46 PM, Simon Kelley  wrote:

When the client sends the discovery packet, dnsmasq will notice that the
requested address is in use by another client, and offer a different
address instead.


You did not understood the scenario. The host that already use the requested
IP address is statically configured to use it (in other words dnsmasq does
not have a lease for the given IP address).

While at it, you might consider fixing the scenario in which a client fills a
DHCP discovery message with an option-50 containing an IP address that is
already used by another statically configured host.


in the above two paragraphs, you use the phrase "statically configured"... do 
you mean "pseudo-statically configured"?


"pseudo-static" where the DHCP gives the same IP to the same MAC all the time

versus

"static" where the machine is configured locally to use a specific IP address

in the first case, the system will be configured for DHCP and will have to ask 
for its address... in the second case, the system will never talk to the DHCP 
server...


something we found in a firewall product was that one must configure their 
dynamically assigned pool to exclude their static and pseudo-static IP address 
ranges otherwise there is the very real possibility that the DHCP server will 
hand out addresses already in use by other systems...



--
 NOTE: No off-list assistance is given without prior approval.
   *Please keep mailing list traffic on the list* unless
   private contact is specifically requested and granted.

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Re: [Dnsmasq-discuss] [PATCH] Nack requests for unknown leases.

[Alin Năstac]

On Sun, Apr 23, 2017 at 5:46 PM, Simon Kelley  wrote:
> On 20/04/17 10:34, Alin Nastac wrote:
>> Hosts that migrate from one network to another could request their
>> old IP address which might be already in use by another statically
>> configured host. Currently non-authoritative dnsmasq servers will
>> ignore such requests, but ISC DHCP client will send discovery packets
>> next carrying the same requested IP address and dnsmasq will end up
>> allocating a new lease for it without checking first if is already
>> used by another host.
>
>
> When the client sends the discovery packet, dnsmasq will notice that the
> requested address is in use by another client, and offer a different
> address instead.

You did not understood the scenario. The host that already use the
requested IP address is statically configured to use it (in other
words dnsmasq does not have a lease for the given IP address).

While at it, you might consider fixing the scenario in which a client
fills a DHCP discovery message with an option-50 containing an IP
address that is already used by another statically configured host.

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss