Discussion:
[Dnsmasq-discuss] [PATCH] Nack requests for unknown leases.
Alin Nastac
2017-04-20 09:34:56 UTC
Permalink
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.
---
src/rfc2131.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/rfc2131.c b/src/rfc2131.c
index 6a8f0db..9987745 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1141,10 +1141,12 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
else
{
/* INIT-REBOOT */
- if (!lease && !option_bool(OPT_AUTHORITATIVE))
- return 0;
-
- if (lease && lease->addr.s_addr != mess->yiaddr.s_addr)
+ if (!lease)
+ {
+ if (!option_bool(OPT_AUTHORITATIVE))
+ message = _("lease not found");
+ }
+ else if (lease->addr.s_addr != mess->yiaddr.s_addr)
message = _("wrong address");
}
}
--
2.7.4
Simon Kelley
2017-04-23 15:46:14 UTC
Permalink
Post by Alin Nastac
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.

Cheers,


Simon.
Post by Alin Nastac
---
src/rfc2131.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/rfc2131.c b/src/rfc2131.c
index 6a8f0db..9987745 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1141,10 +1141,12 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
else
{
/* INIT-REBOOT */
- if (!lease && !option_bool(OPT_AUTHORITATIVE))
- return 0;
-
- if (lease && lease->addr.s_addr != mess->yiaddr.s_addr)
+ if (!lease)
+ {
+ if (!option_bool(OPT_AUTHORITATIVE))
+ message = _("lease not found");
+ }
+ else if (lease->addr.s_addr != mess->yiaddr.s_addr)
message = _("wrong address");
}
}
Alin Năstac
2017-04-24 09:16:57 UTC
Permalink
Post by Simon Kelley
Post by Alin Nastac
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.
w***@gmail.com
2017-04-24 13:43:30 UTC
Permalink
Post by Simon Kelley
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.
Alin Năstac
2017-04-25 07:08:34 UTC
Permalink
Post by w***@gmail.com
Post by Simon Kelley
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...
I'm talking about second case, the "static" one. The use case is this:
1) Client A using ISC DHCP client gets a lease from a different LAN called X
2) Client A gets disconnected from LAN X and connected to LAN Y where
dnsmasq DHCP server runs in a non-authoritative mode.
3) Client A is connected to LAN Y (where dnsmasq serve as DHCP server)
and sends a DHCP requests asking for the same IP address used in LAN X
4) dnsmasq does not have a lease for that IP address so it ignores the requests
5) After a couple of seconds client A sends a DHCP discovery carrying
the same option-50 as the DHCP requests at step 3
6) dnsmasq will happily lease the requested IP address without
checking if there is another host that use it; unfortunately there is
another statically configured host B that use the same address.

What I did to fix it was to send a NACK to the initial DHCP request,
which luckily convinced the ISC DHCP client to stop asking for the
same IP address in the following DHCP discovery. However, NACK will
not quarantee all DHCP clients will do the same, so the case where
DHCP discovery is carrying a conflicting option-50 should also be
fixed IMO.
Post by w***@gmail.com
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...
Then why does dnsmasq use ICMP echo requests to verify that IP address
is about to lease is not already used in the network?
Roy Marples
2017-04-25 09:11:46 UTC
Permalink
Post by Alin Năstac
1) Client A using ISC DHCP client gets a lease from a different LAN called X
2) Client A gets disconnected from LAN X and connected to LAN Y where
dnsmasq DHCP server runs in a non-authoritative mode.
3) Client A is connected to LAN Y (where dnsmasq serve as DHCP server)
and sends a DHCP requests asking for the same IP address used in LAN X
4) dnsmasq does not have a lease for that IP address so it ignores the requests
5) After a couple of seconds client A sends a DHCP discovery carrying
the same option-50 as the DHCP requests at step 3
6) dnsmasq will happily lease the requested IP address without
checking if there is another host that use it; unfortunately there is
another statically configured host B that use the same address.
Irregardless of dnsmasq, ISC dhclient *should* ARP probe to check the
offered address isn't in use. If changing to another DHCP client which
does do this (like say dhcpcd) or fixing dhclient then consider using an
OS which enforces ARP address validation like say NetBSD or Solaris -
not that dhclient will actually do anything about the invalidated
address on these OS's, but that's another topic.

This is important, because dnsmasq could be being a DHCP relay and may
not be able to ICMP ping the requested IP address - hence both sides
need to validate.

Roy
Simon Kelley
2017-04-25 20:39:31 UTC
Permalink
Post by Alin Năstac
What I did to fix it was to send a NACK to the initial DHCP request,
which luckily convinced the ISC DHCP client to stop asking for the
same IP address in the following DHCP discovery. However, NACK will
not quarantee all DHCP clients will do the same, so the case where
DHCP discovery is carrying a conflicting option-50 should also be
fixed IMO.
It also violates RFC 2131, para 4.3.2

"If the DHCP
server has no record of this client, then it MUST remain silent,
and MAY output a warning to the network administrator. This
behavior is necessary for peaceful coexistence of non-
communicating DHCP servers on the same wire."
Post by Alin Năstac
Unfortunately dnsmasq does not send ICMP echo requests when DHCP
discovery packet carries an OPTION_REQUESTED_IP, see DHCPDISCOVER case
...
else if (opt && address_available(context, addr,
tagif_netid) && !lease_find_by_addr(addr) &&
!config_find_by_address(daemon->dhcp_conf, addr))
mess->yiaddr = addr;
That's possibly a bug. Let me think about if there's a reason for that
(This is 10-year old code.....)

However, doing that doesn't defend against the case that a client with a
statically-configured address turns up on a network _after_ the address
got leased to another machine. The only way to do that is not to give
static addresses in the range that you told your DHCP was available for
DHCP leases.


Cheers,

Simon.
Simon Kelley
2017-04-24 20:51:13 UTC
Permalink
Post by Alin Năstac
Post by Simon Kelley
Post by Alin Nastac
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.
Alin Năstac
2017-04-25 07:41:20 UTC
Permalink
Post by Simon Kelley
Post by Alin Năstac
Post by Simon Kelley
Post by Alin Nastac
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.
Unfortunately dnsmasq does not send ICMP echo requests when DHCP
discovery packet carries an OPTION_REQUESTED_IP, see DHCPDISCOVER case
in file rfc2131.c starting from line 990:
...
else if (opt && address_available(context, addr,
tagif_netid) && !lease_find_by_addr(addr) &&
!config_find_by_address(daemon->dhcp_conf, addr))
mess->yiaddr = addr;
Simon Kelley
2017-04-28 21:20:53 UTC
Permalink
Post by Alin Năstac
Post by Simon Kelley
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.
Unfortunately dnsmasq does not send ICMP echo requests when DHCP
discovery packet carries an OPTION_REQUESTED_IP, see DHCPDISCOVER case
...
else if (opt && address_available(context, addr,
tagif_netid) && !lease_find_by_addr(addr) &&
!config_find_by_address(daemon->dhcp_conf, addr))
mess->yiaddr = addr;
That's the bug here, I think. I was worried that a client sending a
DHCPDISCOVER when it thinks it knows that address, might respond to ICMP
pings, but at least for ISC dhclient on Linux, that's not the case.

Patch is here, and was much more trouble than it should have been: the
code really didn't consider this case.

http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=5ce3e76fbf89e942e8c54ef3e3389facf0d9067a

It's still the case that addresses used by statically configured host on
a network should not be in the dhcp-range configured into that network's
DHCP server.


Cheers,

Simon.
Alin Năstac
2017-05-02 09:32:58 UTC
Permalink
Post by Simon Kelley
That's the bug here, I think. I was worried that a client sending a
DHCPDISCOVER when it thinks it knows that address, might respond to ICMP
pings, but at least for ISC dhclient on Linux, that's not the case.
Patch is here, and was much more trouble than it should have been: the
code really didn't consider this case.
http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=5ce3e76fbf89e942e8c54ef3e3389facf0d9067a
I've tested the patch, it fixes the scenario I described earlier.

Thanks!

Kevin Darbyshire-Bryant
2017-04-29 09:49:46 UTC
Permalink
Post by Simon Kelley
That's the bug here, I think. I was worried that a client sending a
DHCPDISCOVER when it thinks it knows that address, might respond to ICMP
pings, but at least for ISC dhclient on Linux, that's not the case.
Patch is here, and was much more trouble than it should have been: the
code really didn't consider this case.
http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=5ce3e76fbf89e942e8c54ef3e3389facf0d9067a
It's still the case that addresses used by statically configured host on
a network should not be in the dhcp-range configured into that network's
DHCP server.
I've just subtly renumbered my own home network to make sure the above
condition is true...those hosts manually statically configured are now
out of the 'statically allocated' dhcp and true dynamic DHCP ranges :-)
Not that I got bitten by this feature 'cos everyone seems to use
192.168.0/1....... and I don't :-)

Simon, is there any chance of a 'test5' bundling all the latest tweaks
into a tarball? It's much easier to get the LEDE guys to accept a test
release tarball than it is loads of patches....and it means the code
would get tested by a wider community.


Thanks,

Kevin
Simon Kelley
2017-04-29 21:55:24 UTC
Permalink
Post by Kevin Darbyshire-Bryant
Simon, is there any chance of a 'test5' bundling all the latest tweaks
into a tarball? It's much easier to get the LEDE guys to accept a test
release tarball than it is loads of patches....and it means the code
would get tested by a wider community.
Done.

As soon as we reach a final conclusion on the dhcp-script logging
patches, I plan to start on a 2.77 release.


Cheers,

Simon.
Loading...