AdguardTeam / AdguardTeam/AdGuardHome
DHCP: a DISCOVER without a REQUEST persists a lease with a zero Expiry, which is never cleaned up
- Lingua principale
- TypeScript
- Stelle
- 36.9k
- Fork
- 2.5k
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Descrizione
### Prerequisites
- [X] I have checked the Wiki and Discussions and found no answer
- [X] I have searched other issues and found no duplicates
- [X] I want to report a bug and not ask a question
- [X] I have set up AdGuard Home correctly and configured clients to use it
### Platform (OS and CPU architecture)
Linux, AMD64 (aka x86_64)
### Installation
Custom (see the description below)
### Setup
On one machine
### AdGuard Home version
v0.107.78
### Action
Send a single `DHCPDISCOVER` to the AdGuard Home DHCP server and never follow it
with a `DHCPREQUEST` — i.e. exactly what a client does when it walks away
mid-negotiation (roaming between APs, MAC rotation, device going to sleep).
Minimal reproduction, using a made-up MAC so it is easy to spot afterwards:
```python
import socket, struct, random
xid = random.randbytes(4)
mac = bytes.fromhex('020000abcdef')
p = b'\x01\x01\x06\x00' + xid + b'\x00\x00\x80\x00'
p += b'\x00'*16 + mac + b'\x00'*10 + b'\x00'*192
p += b'\x63\x82\x53\x63' + b'\x35\x01\x01' + b'\x37\x03\x01\x03\x06' + b'\xff'
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
s.setsockopt(socket.SOL_SOCKET, socket.SO_BROADCAST, 1)
s.bind(('0.0.0.0', 68))
s.sendto(p, ('255.255.255.255', 67)) # DISCOVER only — no REQUEST
```
Then look at `data/leases.json`.
### Expected result
A `DHCPDISCOVER` alone should not create a persistent lease. The address may be
reserved in memory while the offer is pending, but nothing should be written to
`leases.json` until the client confirms with a `DHCPREQUEST`.
### Actual result
A lease is written to `leases.json` immediately, with a **zero `Expiry`**:
```json
{"expires": "0001-01-01T00:00", "ip": "192.168.0.12",
"hostname": "", "mac": "02:00:00:ab:cd:ef", "static": false}
```
Because the expiry is the Go zero value rather than a real timestamp, the entry
is never treated as expired and never cleaned up. It stays in `leases.json`
indefinitely.
Looking at `internal/dhcpd/v4_unix.go`, this seems to be why:
```go
func (s *v4Server) handleDiscover(req, resp *dhcpv4.DHCPv4) (l *dhcpsvc.Lease, err error) {
mac := req.ClientHWAddr
defer s.conf.notify(LeaseChangedDBStore) // persists the lease DB
...
l, err = s.allocateLease(mac) // allocates, does not set Expiry
```
while `Expiry` is only ever set when the client actually confirms:
```go
func (s *v4Server) commitLease(l *dhcpsvc.Lease, hostname string) {
l.Expiry = time.Now().Add(s.conf.leaseTime)
```
So `handleDiscover` both allocates the lease *and* triggers a write of the lease
database, before the client has accepted anything.
### Additional information and/or screenshots
**Why this matters in practice.** On a network with a small DHCP pool these
entries accumulate until the pool is exhausted. On my setup the range is
`192.168.0.10`–`192.168.0.99` (90 addresses) with roughly 20 real devices. Every
phone that roams between two access points, and every iOS/Android private-MAC
rotation, leaves another zero-expiry lease behind.
Twice within 24 hours the pool reached **2 free addresses out of 90**. The
symptoms were not obvious:
- clients could no longer complete DHCP, and retried continuously
(~15 requests/second sustained)
- each request caused a rewrite of the whole lease file (~9 writes/second)
- **DNS resolution on the same process stopped answering entirely** — including
on `127.0.0.1` — because the server was busy serializing leases
So the visible failure was "DNS is down", several layers away from the actual
cause. Cleaning `leases.json` by hand and restarting resolved it both times.
**There is currently no supported way to clean this up.** Static leases can be
removed through the UI/API, but dynamic ones cannot be removed individually, and
"Reset all leases" also deletes static ones. This has been requested for years:
- #2164 — *Option to delete dynamic DHCP leases* (2020)
- #6804 — *Separate "Reset leases" button for non-static leases* (2024)
- #7901 — *Reset All Lease deletes static leases* (2025)
I suspect several of those requests are downstream of this same behaviour:
people accumulate leases they never really handed out, and ask for a button to
clear them. Users with a `/24`-sized pool would likely never notice, since the
junk would take years to fill the range.
**Possible fixes**, in what I would guess is increasing order of effort:
1. Do not persist the lease database from `handleDiscover` — only write on
commit.
2. Give allocated-but-unconfirmed leases a short expiry (e.g. the offer timeout)
so they age out on their own instead of living forever.
3. Skip entries whose `Expiry` is the zero value when loading `leases.json`, so
existing installations self-heal on restart.
Happy to test a patch or provide more data from a live instance.
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.