Skip to content

Commit

Permalink
dhcpd: imp normalization, validation
Browse files Browse the repository at this point in the history
  • Loading branch information
ainar-g committed May 5, 2021
1 parent b3cd33d commit c6ea471
Show file tree
Hide file tree
Showing 8 changed files with 265 additions and 245 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ and this project adheres to
## [v0.106.2] - 2021-05-17 (APPROX.)
-->

### Fixed

- Uniqueness validation for dynamic DHCP leases ([#3056]).

[#3056]: https://github.com/AdguardTeam/AdGuardHome/issues/3056



## [v0.106.1] - 2021-04-30

### Fixed
Expand Down
4 changes: 3 additions & 1 deletion internal/aghnet/addr.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ func ValidateDomainNameLabel(label string) (err error) {
// TODO(a.garipov): After making sure that this works correctly, port this into
// module golibs.
func ValidateDomainName(name string) (err error) {
defer agherr.Annotate("validating %q: %w", &err, name)

name, err = idna.ToASCII(name)
if err != nil {
return err
Expand All @@ -96,7 +98,7 @@ func ValidateDomainName(name string) (err error) {
if l == 0 {
return agherr.Error("domain name is empty")
} else if l > maxDomainNameLen {
return fmt.Errorf("%q is too long, max: %d", name, maxDomainNameLen)
return fmt.Errorf("too long, max: %d", maxDomainNameLen)
}

labels := strings.Split(name, ".")
Expand Down
22 changes: 12 additions & 10 deletions internal/aghnet/addr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,39 +95,41 @@ func TestValidateDomainName(t *testing.T) {
}, {
name: "empty",
in: "",
wantErrMsg: `domain name is empty`,
wantErrMsg: `validating "": domain name is empty`,
}, {
name: "bad_symbol",
in: "!!!",
wantErrMsg: `invalid domain name label at index 0: ` +
wantErrMsg: `validating "!!!": invalid domain name label at index 0: ` +
`invalid char '!' at index 0 in "!!!"`,
}, {
name: "bad_length",
in: longDomainName,
wantErrMsg: `"` + longDomainName + `" is too long, max: 253`,
wantErrMsg: `validating "` + longDomainName + `": too long, max: 253`,
}, {
name: "bad_label_length",
in: longLabelDomainName,
wantErrMsg: `invalid domain name label at index 0: "` + longLabel +
wantErrMsg: `validating "` + longLabelDomainName + `": ` +
`invalid domain name label at index 0: "` + longLabel +
`" is too long, max: 63`,
}, {
name: "bad_label_empty",
in: "example..com",
wantErrMsg: `invalid domain name label at index 1: label is empty`,
name: "bad_label_empty",
in: "example..com",
wantErrMsg: `validating "example..com": ` +
`invalid domain name label at index 1: label is empty`,
}, {
name: "bad_label_first_symbol",
in: "example.-aa.com",
wantErrMsg: `invalid domain name label at index 1:` +
wantErrMsg: `validating "example.-aa.com": invalid domain name label at index 1:` +
` invalid char '-' at index 0 in "-aa"`,
}, {
name: "bad_label_last_symbol",
in: "example-.aa.com",
wantErrMsg: `invalid domain name label at index 0:` +
wantErrMsg: `validating "example-.aa.com": invalid domain name label at index 0:` +
` invalid char '-' at index 7 in "example-"`,
}, {
name: "bad_label_symbol",
in: "example.a!!!.com",
wantErrMsg: `invalid domain name label at index 1:` +
wantErrMsg: `validating "example.a!!!.com": invalid domain name label at index 1:` +
` invalid char '!' at index 1 in "a!!!"`,
}}

Expand Down
10 changes: 5 additions & 5 deletions internal/dhcpd/dhcpd.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ var webHandlersRegistered = false

// Lease contains the necessary information about a DHCP lease
type Lease struct {
// Expiry is the expiration time of the lease. The unix timestamp of
// 1 means that this is a static lease.
Expiry time.Time `json:"expires"`

Hostname string `json:"hostname"`
HWAddr net.HardwareAddr `json:"mac"`
IP net.IP `json:"ip"`
Hostname string `json:"hostname"`

// Lease expiration time
// 1: static lease
Expiry time.Time `json:"expires"`
}

// IsStatic returns true if the lease is static.
Expand Down
24 changes: 14 additions & 10 deletions internal/dhcpd/dhcpd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,34 @@ func TestDB(t *testing.T) {
SubnetMask: net.IP{255, 255, 255, 0},
notify: testNotify,
})
require.Nil(t, err)
require.NoError(t, err)

s.srv6, err = v6Create(V6ServerConf{})
require.Nil(t, err)
require.NoError(t, err)

leases := []Lease{{
IP: net.IP{192, 168, 10, 100},
HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
Expiry: time.Now().Add(time.Hour),
Expiry: time.Now().Add(time.Hour),
Hostname: "static-1.local",
HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
IP: net.IP{192, 168, 10, 100},
}, {
IP: net.IP{192, 168, 10, 101},
HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xBB},
Hostname: "static-2.local",
HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xBB},
IP: net.IP{192, 168, 10, 101},
}}

srv4, ok := s.srv4.(*v4Server)
require.True(t, ok)

err = srv4.addLease(&leases[0])
require.Nil(t, err)
require.Nil(t, s.srv4.AddStaticLease(leases[1]))
require.NoError(t, err)

err = s.srv4.AddStaticLease(leases[1])
require.NoError(t, err)

s.dbStore()
t.Cleanup(func() {
assert.Nil(t, os.Remove(dbFilename))
assert.NoError(t, os.Remove(dbFilename))
})
s.srv4.ResetLeases(nil)
s.dbLoad()
Expand Down
Loading

0 comments on commit c6ea471

Please sign in to comment.