mirror of
https://github.com/AdguardTeam/AdGuardHome.git
synced 2026-08-04 15:28:58 +00:00
Pull request 2707: AGDNS-4231-dont-allow-empty-hostnames-in-static-leases
Squashed commit of the following: commit a5f75312023ce3f90f17a6833add37902e7d670a Merge: 05724001c4da215221Author: 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; commit92d557b4bbAuthor: Maksim Kazantsev <m.kazantsev@adguard.com> Date: Tue Jul 14 15:43:06 2026 +0300 all: upd chlog; commite44ef6c282Merge:40e755b57a8a958a77Author: 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 commit40e755b572Author: Maksim Kazantsev <m.kazantsev@adguard.com> Date: Tue Jul 14 15:37:50 2026 +0300 all: upd chlog; commitf81ef4ec5bAuthor: Maksim Kazantsev <m.kazantsev@adguard.com> Date: Tue Jul 14 13:17:43 2026 +0300 dhcpd: imp tests; imp code; commitc5b3f19b41Author: Maksim Kazantsev <m.kazantsev@adguard.com> Date: Tue Jul 14 12:55:34 2026 +0300 dhcpd: allow empty hostnames; add tests; commit554615617fAuthor: 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:
parent
4da215221a
commit
4d0ae0d87b
4 changed files with 99 additions and 26 deletions
|
|
@ -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]).
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue