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 <m.kazantsev@adguard.com>
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 <m.kazantsev@adguard.com>
Date:   Wed Jul 15 12:19:09 2026 +0300

    dhcpd: fix duplicated test case;

commit 92d557b4bb
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Tue Jul 14 15:43:06 2026 +0300

    all: upd chlog;

commit e44ef6c282
Merge: 40e755b57 a8a958a77
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Tue Jul 14 15:40:51 2026 +0300

    Merge branch 'master' into AGDNS-4231-dont-allow-empty-hostnames-in-static-leases

commit 40e755b572
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Tue Jul 14 15:37:50 2026 +0300

    all: upd chlog;

commit f81ef4ec5b
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Tue Jul 14 13:17:43 2026 +0300

    dhcpd: imp tests; imp code;

commit c5b3f19b41
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Tue Jul 14 12:55:34 2026 +0300

    dhcpd: allow empty hostnames; add tests;

commit 554615617f
Author: Maksim Kazantsev <m.kazantsev@adguard.com>
Date:   Mon Jul 13 17:42:37 2026 +0300

    dhcpd: don't allow empty hostnames in static leases;
This commit is contained in:
Maksim Kazantsev 2026-07-15 09:32:42 +00:00
parent 4da215221a
commit 4d0ae0d87b
4 changed files with 99 additions and 26 deletions

View file

@ -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]).

View file

@ -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 {

View file

@ -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)
}

View file

@ -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)
})
}
}