Skip to content

Commit

Permalink
Merge branch 'master' into AG-32341-client-duplicate-uids
Browse files Browse the repository at this point in the history
  • Loading branch information
schzhn committed Apr 26, 2024
2 parents 672a30c + b9d5e5b commit 2fea9c0
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 26 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ NOTE: Add new changes BELOW THIS COMMENT.

- Acceptance of duplicate UIDs for persistent clients at startup. See also the
section on client settings on the [Wiki page][wiki-config].
- Support for link-local subnets, i.e. `fe80::/16`, as client identifiers
([#6312]).
- Issues with QUIC and HTTP/3 upstreams on older Linux kernel versions
([#6422]).
- YouTube restricted mode is not enforced by HTTPS queries on Firefox.
Expand All @@ -59,6 +61,7 @@ NOTE: Add new changes BELOW THIS COMMENT.
[#5345]: https://github.com/AdguardTeam/AdGuardHome/issues/5345
[#5812]: https://github.com/AdguardTeam/AdGuardHome/issues/5812
[#6192]: https://github.com/AdguardTeam/AdGuardHome/issues/6192
[#6312]: https://github.com/AdguardTeam/AdGuardHome/issues/6312
[#6422]: https://github.com/AdguardTeam/AdGuardHome/issues/6422
[#6854]: https://github.com/AdguardTeam/AdGuardHome/issues/6854
[#6875]: https://github.com/AdguardTeam/AdGuardHome/issues/6875
Expand Down
4 changes: 2 additions & 2 deletions client/src/__locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@
"fallback_dns_desc": "List of fallback DNS servers used when upstream DNS servers are not responding. The syntax is the same as in the main upstreams field above.",
"fallback_dns_placeholder": "Enter one fallback DNS server per line",
"local_ptr_title": "Private reverse DNS servers",
"local_ptr_desc": "The DNS servers that AdGuard Home uses for private PTR, SOA, and NS queries. The request is considered private if it asks for ARPA domain containing a subnet within private IP ranges, for example \"192.168.12.34\", and came from a client with private address. If not set, AdGuard Home uses the addresses of the default DNS resolvers of your OS except for the addresses of AdGuard Home itself.",
"local_ptr_desc": "DNS servers used by AdGuard Home for private PTR, SOA, and NS requests. A request is considered private if it asks for an ARPA domain containing a subnet within private IP ranges (such as \"192.168.12.34\") and comes from a client with a private IP address. If not set, the default DNS resolvers of your OS will be used, except for the AdGuard Home IP addresses.",
"local_ptr_default_resolver": "By default, AdGuard Home uses the following reverse DNS resolvers: {{ip}}.",
"local_ptr_no_default_resolver": "AdGuard Home could not determine suitable private reverse DNS resolvers for this system.",
"local_ptr_placeholder": "Enter one IP address per line",
"resolve_clients_title": "Enable reverse resolving of clients' IP addresses",
"resolve_clients_desc": "Reversely resolve clients' IP addresses into their hostnames by sending PTR queries to corresponding resolvers (private DNS servers for local clients, upstream servers for clients with public IP addresses).",
"use_private_ptr_resolvers_title": "Use private reverse DNS resolvers",
"use_private_ptr_resolvers_desc": "Resolve PTR, SOA, and NS requests for ARPA domains containing private addresses using private upstream servers, DHCP, /etc/hosts, and so on. If disabled, AdGuard Home responds with NXDOMAIN to all such queries.",
"use_private_ptr_resolvers_desc": "Resolve PTR, SOA, and NS requests for ARPA domains containing private IP addresses through private upstream servers, DHCP, /etc/hosts, etc. If disabled, AdGuard Home will respond to all such requests with NXDOMAIN.",
"check_dhcp_servers": "Check for DHCP servers",
"save_config": "Save configuration",
"enabled_dhcp": "DHCP server enabled",
Expand Down
24 changes: 23 additions & 1 deletion internal/client/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,10 @@ func (ci *Index) findByIP(ip netip.Addr) (c *Persistent, found bool) {
return ci.uidToClient[uid], true
}

ipWithoutZone := ip.WithZone("")
ci.subnetToUID.Range(func(pref netip.Prefix, id UID) (cont bool) {
if pref.Contains(ip) {
// Remove zone before checking because prefixes strip zones.
if pref.Contains(ipWithoutZone) {
uid, found = id, true

return false
Expand All @@ -225,6 +227,26 @@ func (ci *Index) findByIP(ip netip.Addr) (c *Persistent, found bool) {
return nil, false
}

// FindByIPWithoutZone finds a persistent client by IP address without zone. It
// strips the IPv6 zone index from the stored IP addresses before comparing,
// because querylog entries don't have it. See TODO on [querylog.logEntry.IP].
//
// Note that multiple clients can have the same IP address with different zones.
// Therefore, the result of this method is indeterminate.
func (ci *Index) FindByIPWithoutZone(ip netip.Addr) (c *Persistent) {
if (ip == netip.Addr{}) {
return nil
}

for addr, uid := range ci.ipToUID {
if addr.WithZone("") == ip {
return ci.uidToClient[uid]
}
}

return nil
}

// find finds persistent client by MAC.
func (ci *Index) findByMAC(mac net.HardwareAddr) (c *Persistent, found bool) {
k := macToKey(mac)
Expand Down
119 changes: 97 additions & 22 deletions internal/client/index_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,49 @@ func TestClientIndex(t *testing.T) {

cliID = "client-id"
cliMAC = "11:11:11:11:11:11"

linkLocalIP = "fe80::abcd:abcd:abcd:ab%eth0"
linkLocalSubnet = "fe80::/16"
)

clients := []*Persistent{{
Name: "client1",
IPs: []netip.Addr{
netip.MustParseAddr(cliIP1),
netip.MustParseAddr(cliIPv6),
},
}, {
Name: "client2",
IPs: []netip.Addr{netip.MustParseAddr(cliIP2)},
Subnets: []netip.Prefix{netip.MustParsePrefix(cliSubnet)},
}, {
Name: "client_with_mac",
MACs: []net.HardwareAddr{mustParseMAC(cliMAC)},
}, {
Name: "client_with_id",
ClientIDs: []string{cliID},
}}
var (
clientWithBothFams = &Persistent{
Name: "client1",
IPs: []netip.Addr{
netip.MustParseAddr(cliIP1),
netip.MustParseAddr(cliIPv6),
},
}

ci := newIDIndex(clients)
clientWithSubnet = &Persistent{
Name: "client2",
IPs: []netip.Addr{netip.MustParseAddr(cliIP2)},
Subnets: []netip.Prefix{netip.MustParsePrefix(cliSubnet)},
}

clientWithMAC = &Persistent{
Name: "client_with_mac",
MACs: []net.HardwareAddr{mustParseMAC(cliMAC)},
}

clientWithID = &Persistent{
Name: "client_with_id",
ClientIDs: []string{cliID},
}

clientLinkLocal = &Persistent{
Name: "client_link_local",
Subnets: []netip.Prefix{netip.MustParsePrefix(linkLocalSubnet)},
}
)

ci := newIDIndex([]*Persistent{
clientWithBothFams,
clientWithSubnet,
clientWithMAC,
clientWithID,
clientLinkLocal,
})

testCases := []struct {
want *Persistent
Expand All @@ -64,19 +86,23 @@ func TestClientIndex(t *testing.T) {
}{{
name: "ipv4_ipv6",
ids: []string{cliIP1, cliIPv6},
want: clients[0],
want: clientWithBothFams,
}, {
name: "ipv4_subnet",
ids: []string{cliIP2, cliSubnetIP},
want: clients[1],
want: clientWithSubnet,
}, {
name: "mac",
ids: []string{cliMAC},
want: clients[2],
want: clientWithMAC,
}, {
name: "client_id",
ids: []string{cliID},
want: clients[3],
want: clientWithID,
}, {
name: "client_link_local_subnet",
ids: []string{linkLocalIP},
want: clientLinkLocal,
}}

for _, tc := range testCases {
Expand Down Expand Up @@ -221,3 +247,52 @@ func TestMACToKey(t *testing.T) {
_ = macToKey(mac)
})
}

func TestIndex_FindByIPWithoutZone(t *testing.T) {
var (
ip = netip.MustParseAddr("fe80::a098:7654:32ef:ff1")
ipWithZone = netip.MustParseAddr("fe80::1ff:fe23:4567:890a%eth2")
)

var (
clientNoZone = &Persistent{
Name: "client",
IPs: []netip.Addr{ip},
}

clientWithZone = &Persistent{
Name: "client_with_zone",
IPs: []netip.Addr{ipWithZone},
}
)

ci := newIDIndex([]*Persistent{
clientNoZone,
clientWithZone,
})

testCases := []struct {
ip netip.Addr
want *Persistent
name string
}{{
name: "without_zone",
ip: ip,
want: clientNoZone,
}, {
name: "with_zone",
ip: ipWithZone,
want: clientWithZone,
}, {
name: "zero_address",
ip: netip.Addr{},
want: nil,
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
c := ci.FindByIPWithoutZone(tc.ip.WithZone(""))
require.Equal(t, tc.want, c)
})
}
}
6 changes: 5 additions & 1 deletion internal/home/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,11 @@ func (clients *clientsContainer) clientOrArtificial(
}()

cli, ok := clients.find(id)
if ok {
if !ok {
cli = clients.clientIndex.FindByIPWithoutZone(ip)
}

if cli != nil {
return &querylog.Client{
Name: cli.Name,
IgnoreQueryLog: cli.IgnoreQueryLog,
Expand Down
1 change: 1 addition & 0 deletions internal/querylog/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type logEntry struct {
Answer []byte `json:",omitempty"`
OrigAnswer []byte `json:",omitempty"`

// TODO(s.chzhen): Use netip.Addr.
IP net.IP `json:"IP"`

Result filtering.Result
Expand Down

0 comments on commit 2fea9c0

Please sign in to comment.