From 4d0ae0d87bbbdb260671c59348113bfaf6242599 Mon Sep 17 00:00:00 2001 From: Maksim Kazantsev Date: Wed, 15 Jul 2026 09:32:42 +0000 Subject: [PATCH] Pull request 2707: AGDNS-4231-dont-allow-empty-hostnames-in-static-leases Squashed commit of the following: commit a5f75312023ce3f90f17a6833add37902e7d670a Merge: 05724001c 4da215221 Author: Maksim Kazantsev Date: Wed Jul 15 12:21:15 2026 +0300 Merge branch 'master' into AGDNS-4231-dont-allow-empty-hostnames-in-static-leases commit 05724001c53f7053134ab141e3ca2da0dad06dd9 Author: Maksim Kazantsev Date: Wed Jul 15 12:19:09 2026 +0300 dhcpd: fix duplicated test case; commit 92d557b4bb91afddd36aa7dc936d0f2d5bf1a349 Author: Maksim Kazantsev Date: Tue Jul 14 15:43:06 2026 +0300 all: upd chlog; commit e44ef6c28208191a86138535339d22771af5dd31 Merge: 40e755b57 a8a958a77 Author: Maksim Kazantsev Date: Tue Jul 14 15:40:51 2026 +0300 Merge branch 'master' into AGDNS-4231-dont-allow-empty-hostnames-in-static-leases commit 40e755b5720b1ce4e02a44883fe56471d45ef4d9 Author: Maksim Kazantsev Date: Tue Jul 14 15:37:50 2026 +0300 all: upd chlog; commit f81ef4ec5b2817900e980c46e7e9357137044f68 Author: Maksim Kazantsev Date: Tue Jul 14 13:17:43 2026 +0300 dhcpd: imp tests; imp code; commit c5b3f19b414ad8841a5572ec53bc10b4b124cb5d Author: Maksim Kazantsev Date: Tue Jul 14 12:55:34 2026 +0300 dhcpd: allow empty hostnames; add tests; commit 554615617fe24940ca589a8e8082da9b03b65642 Author: Maksim Kazantsev Date: Mon Jul 13 17:42:37 2026 +0300 dhcpd: don't allow empty hostnames in static leases; --- CHANGELOG.md | 4 ++ internal/dhcpd/http_unix_internal_test.go | 12 +++- internal/dhcpd/v4_unix.go | 38 +++++------- internal/dhcpd/v4_unix_internal_test.go | 71 ++++++++++++++++++++++- 4 files changed, 99 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0377da170..9c673067b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ See also the [v0.107.79 GitHub milestone][ms-v0.107.79]. NOTE: Add new changes BELOW THIS COMMENT. --> +### Added + +- The user is able to remove the static lease's hostname via the HTTP API. + ### Fixed - Blocked requests without an EDNS(0) OPT record ([#8183]). diff --git a/internal/dhcpd/http_unix_internal_test.go b/internal/dhcpd/http_unix_internal_test.go index 059e40ac9..ed5fe2401 100644 --- a/internal/dhcpd/http_unix_internal_test.go +++ b/internal/dhcpd/http_unix_internal_test.go @@ -209,6 +209,14 @@ func TestServer_HandleUpdateStaticLease(t *testing.T) { IP: leaseV4IP, Hostname: "updated-client-v4", }, + }, { + name: "update_v4_empty_hostname", + pos: leaseV4Pos, + lease: &leaseStatic{ + HWAddr: leaseV4MAC, + IP: leaseV4IP, + Hostname: "", + }, }, { name: "update_v4_ip", pos: leaseV4Pos, @@ -306,7 +314,7 @@ func TestServer_HandleUpdateStaticLease_validation(t *testing.T) { IP: anotherV4IP, Hostname: leaseV4Name, }, - want: "dhcpv4: updating static lease: ip address is not unique\n", + want: "dhcpv4: updating static lease: ip address: duplicated value\n", }, { name: "update_v4_same_name", lease: &leaseStatic{ @@ -314,7 +322,7 @@ func TestServer_HandleUpdateStaticLease_validation(t *testing.T) { IP: leaseV4IP, Hostname: anotherV4Name, }, - want: "dhcpv4: updating static lease: hostname is not unique\n", + want: "dhcpv4: updating static lease: hostname: duplicated value\n", }} for _, tc := range testCases { diff --git a/internal/dhcpd/v4_unix.go b/internal/dhcpd/v4_unix.go index 0350d7f3b..e5814012e 100644 --- a/internal/dhcpd/v4_unix.go +++ b/internal/dhcpd/v4_unix.go @@ -313,16 +313,6 @@ func (s *v4Server) rmDynamicLease(lease *dhcpsvc.Lease) (err error) { return nil } -const ( - // ErrDupHostname is returned by addLease, validateStaticLease when the - // modified lease has a not empty non-unique hostname. - ErrDupHostname = errors.Error("hostname is not unique") - - // ErrDupIP is returned by addLease, validateStaticLease when the modified - // lease has a non-unique IP address. - ErrDupIP = errors.Error("ip address is not unique") -) - // addLease adds a dynamic or static lease. func (s *v4Server) addLease(l *dhcpsvc.Lease) (err error) { r := s.conf.ipRange @@ -342,7 +332,7 @@ func (s *v4Server) addLease(l *dhcpsvc.Lease) (err error) { // TODO(e.burkov): l must have a valid hostname here, investigate. if l.Hostname != "" { if _, ok := s.hostsIndex[l.Hostname]; ok { - return ErrDupHostname + return fmt.Errorf("hostname: %w", errors.ErrDuplicated) } s.hostsIndex[l.Hostname] = l @@ -485,23 +475,25 @@ func (s *v4Server) validateStaticLease(l *dhcpsvc.Lease) (err error) { return err } - err = netutil.ValidateHostname(hostname) - if err != nil { - return fmt.Errorf("validating hostname: %w", err) - } + if hostname != "" { + err = netutil.ValidateHostname(hostname) + if err != nil { + return fmt.Errorf("hostname: %w", err) + } - dup, ok := s.hostsIndex[hostname] - if ok && !bytes.Equal(dup.HWAddr, l.HWAddr) { - return ErrDupHostname - } - - dup, ok = s.ipIndex[l.IP] - if ok && !bytes.Equal(dup.HWAddr, l.HWAddr) { - return ErrDupIP + dup, ok := s.hostsIndex[hostname] + if ok && !bytes.Equal(dup.HWAddr, l.HWAddr) { + return fmt.Errorf("hostname: %w", errors.ErrDuplicated) + } } l.Hostname = hostname + dup, ok := s.ipIndex[l.IP] + if ok && !bytes.Equal(dup.HWAddr, l.HWAddr) { + return fmt.Errorf("ip address: %w", errors.ErrDuplicated) + } + if gwIP := s.conf.GatewayIP; gwIP == l.IP { return fmt.Errorf("can't assign the gateway IP %q to the lease", gwIP) } diff --git a/internal/dhcpd/v4_unix_internal_test.go b/internal/dhcpd/v4_unix_internal_test.go index ae80b5bdc..9d09687f2 100644 --- a/internal/dhcpd/v4_unix_internal_test.go +++ b/internal/dhcpd/v4_unix_internal_test.go @@ -12,6 +12,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc" + "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/stringutil" "github.com/AdguardTeam/golibs/testutil" @@ -83,7 +84,7 @@ func TestV4Server_leasing(t *testing.T) { IP: anotherIP, IsStatic: true, }) - assert.ErrorIs(t, err, ErrDupHostname) + assert.ErrorIs(t, err, errors.ErrDuplicated) }) t.Run("same_mac", func(t *testing.T) { @@ -914,3 +915,71 @@ func TestV4Server_handleRelease(t *testing.T) { require.Equal(t, wantResp, resp) } + +func TestV4Server_validateStaticLease_emptyHostname(t *testing.T) { + t.Parallel() + + const ( + existingHostname = "existing-client" + ) + + existingIP := netip.MustParseAddr("192.168.10.150") + nonExistingIP := existingIP.Next() + existingMAC := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA} + otherMAC := net.HardwareAddr{0xBB, 0xBB, 0xBB, 0xBB, 0xBB, 0xBB} + + s := defaultSrv(t) + + s4, ok := s.(*v4Server) + require.True(t, ok) + + existingLease := &dhcpsvc.Lease{ + Hostname: existingHostname, + HWAddr: existingMAC, + IP: existingIP, + IsStatic: true, + } + err := s4.addLease(existingLease) + require.NoError(t, err) + + testCases := []struct { + name string + wantErr string + lease *dhcpsvc.Lease + }{{ + name: "empty_hostname_allowed", + lease: &dhcpsvc.Lease{ + Hostname: "", + HWAddr: otherMAC, + IP: nonExistingIP, + IsStatic: true, + }, + }, { + name: "non_empty_hostname_duplicate_hostname", + wantErr: "hostname: duplicated value", + lease: &dhcpsvc.Lease{ + Hostname: existingHostname, + HWAddr: otherMAC, + IP: nonExistingIP, + IsStatic: true, + }, + }, { + name: "non_empty_hostname_duplicate_ip", + wantErr: "ip address: duplicated value", + lease: &dhcpsvc.Lease{ + Hostname: "new-client", + HWAddr: otherMAC, + IP: existingIP, + IsStatic: true, + }, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Don't run subtests in parallel because they modify the server's + // leases. + err = s4.validateStaticLease(tc.lease) + testutil.AssertErrorMsg(t, tc.wantErr, err) + }) + } +}