AdguardTeam / AdGuardHome

Network-wide ads & trackers blocking DNS server
https://adguard.com/adguard-home.html
GNU General Public License v3.0
24.62k stars 1.78k forks source link

DHCPv4 gives a duplicated IP address if a already existing fixed IP address in the pool doesn't ping #6166

Open antoninchadima opened 1 year ago

antoninchadima commented 1 year ago

Prerequisites

Platform (OS and CPU architecture)

Linux, AMD64 (aka x86_64)

Installation

Docker

Setup

On one machine

AdGuard Home version

v0.107.36

Action

setup a DHCP server in Adguard Home have some devices with IP addresses already in the DHCP pool (with DHCP leases from the previous DHCP server or with fixed IP) these devices should not respond to ping (some routers, switches, wifi ap, or computers with firewall and so on) so those devices are only detectable by an ARP scan

Expected result

Adguard Home should not only use ping for detecting colliding IP addresses but an proper ARP scan on the network on Linux for example provided by arp-scan

Actual result

devices with a new DHCP lease from Adguard can get a duplicated IP address resulting in Windows in disconnecting from the network and on other systems in traffic jam with colliding IP addresses

Additional information and/or screenshots

a regular ARP scanning could be used to provide further info in the DHCP screen

  1. all devices on your network in the DHCP pool
  2. obtained mac addresses could be used to determine vendors (from mac vendor, ieee oui and ieee iab)

please consider having a separate DHCP and network scan / clients page having DHCP leases in config is not the best way this overview should show: IP, MAC, Vendor (by MAC overridden by DHCP request option), Host name, Lease Start, Lease End, Active lease? Alive ping? Alive ARP? Send DNS request? (are request in request log - already in PiHole!), Actions (delete, add to static etc. for example edit of lease start end end), DHCP Options send in the request and maybe reverse DNS resolve of the client IP and the overview should show all leases in leases.json - not only the active ones and it should contain even devices scanned/discovered (in a configurable period of time) to add them to static leases etc.

this enhancement would make a great tool from Adguard Home with an overview of devices on the actual subnet or DHCP pool

antoninchadima commented 1 year ago

this bug can be very easily reproduced: (i'm administering 5 subnets of 254 ip and it happens every day)

  1. have adguard home dhcpv4 running and giving leases for example in the range 192.168.0.1-254
  2. start a device with a fixed ip in the range of the dhcp pool for example 192.168.0.2 and disable ping responses on the device (at the moment of starting this device - this ip is unique there is no lease for 192.168.0.2 in this moment)
  3. send a dhcp request from any device (the next free ip / lease for adguard home should be at this moment 192.168.0.2)
  4. adguard home checks the network if 192.168.0.2 is available but only over ping - nor arp scan
  5. adguard home doesn't get a ping response and thinks that 192.168.0.2 is available
  6. adguard home sends a dhcp response with a 192.168.0.2 lease and at this moment everything breaks

result - client cannot connect (client is more smart then adguard home and using arp scan detects duplicate ip) the client sends the request over and over and has no network acces after a while you have duplicated records, nul mac addresses and wrong hostnames in leases.json

antoninchadima commented 1 year ago

at the same moment (the problem is 147.231.80.109)

adguard home Screenshot 2023-09-05 16 15 50

ping scan Screenshot 2023-09-05 16 16 47

arp scan Screenshot 2023-09-05 16 17 56

EugeneOne1 commented 1 year ago

@antoninchadima, hello and thanks for thorough report. Indeed, the RFC 2131 makes this clear:

As a consistency check, the allocating server SHOULD probe the reused address before allocating the address, e.g., with an ICMP echo request, and the client SHOULD probe the newly received address, e.g., with ARP.

We're going to enhance the DHCP behavior in v0.108.0 release cycle, thanks again.

antoninchadima commented 1 year ago

The RFC 2131 says:

  1. The client receives the DHCPACK message with configuration parameters. The client SHOULD perform a final check on the parameters (e.g., ARP for allocated network address), and notes the duration of the lease specified in the DHCPACK message. At this point, the client is configured. If the client detects that the address is already in use (e.g., through the use of ARP), the client MUST send a DHCPDECLINE message to the server and restarts the configuration process. The client SHOULD wait a minimum of ten seconds before restarting the configuration process to avoid excessive network traffic in case of looping.

And the Windows OS client did send a DHCPDECLINE message. But Adguard Home was sending the not accepted IP again and again.

antoninchadima commented 1 year ago

it is more of a bug

image

A DHCP Decline message is sent by a DHCP client to notify the DHCP server that the allocated IP address conflicts with another IP address. The DHCP client then applies to the DHCP server for another IP address. The DHCP server notes the declined address as in use and avoids this address.

the ICMP echo on the server side and ARP probe on the client side is because of dhcp clients on other subnets... (yes this is possible with a dhcp relay / proxy), so the server uses layer 3 and the client layer 2...

but adguard home is mostly intended in small nets and home deployments - so i would assume that the dhcp server and client are on the same net...

so you should implement dhcpdecline the correct way (as a bug request) or scan the subnet and keep a track of the subnet as an enhancement request (this could be used for tracking a scanning clients on the network which does not use adguard dns server etc...)