mirrors
/
AdGuardHome
mirror of https://github.com/AdguardTeam/AdGuardHome.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Pull request: 4939 Client update 

Merge in DNS/adguard-home from 4939-client-upd to master

Updates #4939.

Squashed commit of the following:

commit 34f35822af
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Feb 10 14:01:57 2023 +0300

    all: imp code, docs

commit 1cd8767a38f6494c92fb5ceff26abe228fcca638
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Feb 9 17:20:56 2023 +0300

    all: different ttls

commit 66d951ba3dd72cb698b89b432cbbbdd65cb421a2
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Feb 9 14:24:47 2023 +0300

    all: imp code

commit 3fb8d08310296dad90783f13ba46a1d0ea11da2e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Feb 8 19:35:29 2023 +0300

    home: fix rdns check logic
This commit is contained in:
Eugene Burkov 2023-02-10 16:40:36 +03:00
parent b89105e3b5
commit ec19a85ed0
7 changed files with 119 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -62,6 +62,7 @@ In this release, the schema version has changed from 14 to 15.
### Fixed
- Client names resolved via reverse DNS not being updated ([#4939]).
- The icon for League Of Legends on the Blocked services page ([#5433]).
### Removed
@ -70,6 +71,7 @@ In this release, the schema version has changed from 14 to 15.
[#1717]: https://github.com/AdguardTeam/AdGuardHome/issues/1717
[#4299]: https://github.com/AdguardTeam/AdGuardHome/issues/4299
[#4939]: https://github.com/AdguardTeam/AdGuardHome/issues/4939
[#5433]: https://github.com/AdguardTeam/AdGuardHome/issues/5433
<!--

35
internal/dnsforward/dnsforward.go
View File

@ -253,8 +253,8 @@ func (s *Server) Resolve(host string) ([]net.IPAddr, error) {
// RDNSExchanger is a resolver for clients' addresses.
type RDNSExchanger interface {
// Exchange tries to resolve the ip in a suitable way, e.g. either as
// local or as external.
// Exchange tries to resolve the ip in a suitable way, i.e. either as local
// or as external.
Exchange(ip net.IP) (host string, err error)
// ResolvesPrivatePTR returns true if the RDNSExchanger is able to
@ -263,13 +263,13 @@ type RDNSExchanger interface {
}
const (
// rDNSEmptyAnswerErr is returned by Exchange method when the answer
// section of respond is empty.
rDNSEmptyAnswerErr errors.Error = "the answer section is empty"
// ErrRDNSNoData is returned by [RDNSExchanger.Exchange] when the answer
// section of response is either NODATA or has no PTR records.
ErrRDNSNoData errors.Error = "no ptr data in response"
// rDNSNotPTRErr is returned by Exchange method when the response is not
// of PTR type.
rDNSNotPTRErr errors.Error = "the response is not a ptr"
// ErrRDNSFailed is returned by [RDNSExchanger.Exchange] if the received
// response is not a NOERROR or NXDOMAIN.
ErrRDNSFailed errors.Error = "failed to resolve ptr"
)
// type check
@ -324,17 +324,24 @@ func (s *Server) Exchange(ip net.IP) (host string, err error) {
return "", err
}
// Distinguish between NODATA response and a failed request.
resp := ctx.Res
if len(resp.Answer) == 0 {
return "", fmt.Errorf("lookup for %q: %w", arpa, rDNSEmptyAnswerErr)
if resp.Rcode != dns.RcodeSuccess && resp.Rcode != dns.RcodeNameError {
return "", fmt.Errorf(
"received %s response: %w",
dns.RcodeToString[resp.Rcode],
ErrRDNSFailed,
)
}
ptr, ok := resp.Answer[0].(*dns.PTR)
if !ok {
return "", fmt.Errorf("type checking: %w", rDNSNotPTRErr)
for _, ans := range resp.Answer {
ptr, ok := ans.(*dns.PTR)
if ok {
return strings.TrimSuffix(ptr.Ptr, "."), nil
}
}
return strings.TrimSuffix(ptr.Ptr, "."), nil
return "", ErrRDNSNoData
}
// ResolvesPrivatePTR implements the RDNSExchanger interface for *Server.

15
internal/dnsforward/dnsforward_test.go
View File

@ -1221,6 +1221,9 @@ func TestServer_Exchange(t *testing.T) {
errUpstream := aghtest.NewErrorUpstream()
nonPtrUpstream := aghtest.NewBlockUpstream("some-host", true)
refusingUpstream := aghtest.NewUpstreamMock(func(req *dns.Msg) (resp *dns.Msg, err error) {
return new(dns.Msg).SetRcode(req, dns.RcodeRefused), nil
})
srv := &Server{
recDetector: newRecursionDetector(0, 1),
@ -1265,15 +1268,21 @@ func TestServer_Exchange(t *testing.T) {
}, {
name: "empty_answer_error",
want: "",
wantErr: rDNSEmptyAnswerErr,
wantErr: ErrRDNSNoData,
locUpstream: locUpstream,
req: net.IP{192, 168, 1, 2},
}, {
name: "not_ptr_error",
name: "invalid_answer",
want: "",
wantErr: rDNSNotPTRErr,
wantErr: ErrRDNSNoData,
locUpstream: nonPtrUpstream,
req: localIP,
}, {
name: "refused",
want: "",
wantErr: ErrRDNSFailed,
locUpstream: refusingUpstream,
req: localIP,
}}
for _, tc := range testCases {

23
internal/home/clients.go
View File

@ -67,15 +67,18 @@ func (c *Client) closeUpstreams() (err error) {
type clientSource uint
// Client sources. The order determines the priority.
// Clients information sources. The order determines the priority.
const (
ClientSourceWHOIS clientSource = iota
ClientSourceNone clientSource = iota
ClientSourceWHOIS
ClientSourceARP
ClientSourceRDNS
ClientSourceDHCP
ClientSourceHostsFile
ClientSourcePersistent
)
// type check
var _ fmt.Stringer = clientSource(0)
// String returns a human-readable name of cs.
@ -96,6 +99,7 @@ func (cs clientSource) String() (s string) {
}
}
// type check
var _ encoding.TextMarshaler = clientSource(0)
// MarshalText implements encoding.TextMarshaler for the clientSource.
@ -332,23 +336,24 @@ func (clients *clientsContainer) onDHCPLeaseChanged(flags int) {
}
}
// exists checks if client with this IP address already exists.
func (clients *clientsContainer) exists(ip netip.Addr, source clientSource) (ok bool) {
// clientSource checks if client with this IP address already exists and returns
// the source which updated it last. It returns [ClientSourceNone] if the
// client doesn't exist.
func (clients *clientsContainer) clientSource(ip netip.Addr) (src clientSource) {
clients.lock.Lock()
defer clients.lock.Unlock()
_, ok = clients.findLocked(ip.String())
_, ok := clients.findLocked(ip.String())
if ok {
return true
return ClientSourcePersistent
}
rc, ok := clients.ipToRC[ip]
if !ok {
return false
return ClientSourceNone
}
// Return false if the new source has higher priority.
return source <= rc.Source
return rc.Source
}
func toQueryLogWHOIS(wi *RuntimeClientWHOISInfo) (cw *querylog.ClientWHOIS) {

18
internal/home/clients_test.go
View File

@ -67,9 +67,9 @@ func TestClients(t *testing.T) {
assert.Equal(t, "client2", c.Name)
assert.False(t, clients.exists(cliNoneIP, ClientSourceHostsFile))
assert.True(t, clients.exists(cli1IP, ClientSourceHostsFile))
assert.True(t, clients.exists(cli2IP, ClientSourceHostsFile))
assert.Equal(t, clients.clientSource(cliNoneIP), ClientSourceNone)
assert.Equal(t, clients.clientSource(cli1IP), ClientSourcePersistent)
assert.Equal(t, clients.clientSource(cli2IP), ClientSourcePersistent)
})
t.Run("add_fail_name", func(t *testing.T) {
@ -127,8 +127,8 @@ func TestClients(t *testing.T) {
})
require.NoError(t, err)
assert.False(t, clients.exists(cliOldIP, ClientSourceHostsFile))
assert.True(t, clients.exists(cliNewIP, ClientSourceHostsFile))
assert.Equal(t, clients.clientSource(cliOldIP), ClientSourceNone)
assert.Equal(t, clients.clientSource(cliNewIP), ClientSourcePersistent)
err = clients.Update("client1", &Client{
IDs: []string{cliNew},
@ -157,7 +157,7 @@ func TestClients(t *testing.T) {
ok := clients.Del("client1-renamed")
require.True(t, ok)
assert.False(t, clients.exists(netip.MustParseAddr("1.1.1.2"), ClientSourceHostsFile))
assert.Equal(t, clients.clientSource(netip.MustParseAddr("1.1.1.2")), ClientSourceNone)
})
t.Run("del_fail", func(t *testing.T) {
@ -176,18 +176,18 @@ func TestClients(t *testing.T) {
ok = clients.AddHost(ip, "host3", ClientSourceHostsFile)
assert.True(t, ok)
assert.True(t, clients.exists(ip, ClientSourceHostsFile))
assert.Equal(t, clients.clientSource(ip), ClientSourceHostsFile)
})
t.Run("dhcp_replaces_arp", func(t *testing.T) {
ip := netip.MustParseAddr("1.2.3.4")
ok := clients.AddHost(ip, "from_arp", ClientSourceARP)
assert.True(t, ok)
assert.True(t, clients.exists(ip, ClientSourceARP))
assert.Equal(t, clients.clientSource(ip), ClientSourceARP)
ok = clients.AddHost(ip, "from_dhcp", ClientSourceDHCP)
assert.True(t, ok)
assert.True(t, clients.exists(ip, ClientSourceDHCP))
assert.Equal(t, clients.clientSource(ip), ClientSourceDHCP)
})
t.Run("addhost_fail", func(t *testing.T) {

54
internal/home/rdns.go
View File

@ -8,6 +8,7 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
"github.com/AdguardTeam/golibs/cache"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
)
@ -30,11 +31,15 @@ type RDNS struct {
usePrivate atomic.Bool
}
// Default rDNS values.
// Default AdGuard Home reverse DNS values.
const (
defaultRDNSCacheSize = 10000
defaultRDNSCacheTTL = 1 * 60 * 60
defaultRDNSIPChSize = 256
revDNSCacheSize = 10000
// TODO(e.burkov): Make these values configurable.
revDNSCacheTTL = 24 * 60 * 60
revDNSFailureCacheTTL = 1 * 60 * 60
revDNSQueueSize = 256
)
// NewRDNS creates and returns initialized RDNS.
@ -48,9 +53,9 @@ func NewRDNS(
clients: clients,
ipCache: cache.New(cache.Config{
EnableLRU: true,
MaxCount: defaultRDNSCacheSize,
MaxCount: revDNSCacheSize,
}),
ipCh: make(chan netip.Addr, defaultRDNSIPChSize),
ipCh: make(chan netip.Addr, revDNSQueueSize),
}
rDNS.usePrivate.Store(usePrivate)
@ -79,25 +84,28 @@ func (r *RDNS) isCached(ip netip.Addr) (ok bool) {
ipBytes := ip.AsSlice()
now := uint64(time.Now().Unix())
if expire := r.ipCache.Get(ipBytes); len(expire) != 0 {
if binary.BigEndian.Uint64(expire) > now {
return true
}
return binary.BigEndian.Uint64(expire) > now
}
// The cache entry either expired or doesn't exist.
ttl := make([]byte, 8)
binary.BigEndian.PutUint64(ttl, now+defaultRDNSCacheTTL)
r.ipCache.Set(ipBytes, ttl)
return false
}
// cache caches the ip address for ttl seconds.
func (r *RDNS) cache(ip netip.Addr, ttl uint64) {
ipData := ip.AsSlice()
ttlData := [8]byte{}
binary.BigEndian.PutUint64(ttlData[:], uint64(time.Now().Unix())+ttl)
r.ipCache.Set(ipData, ttlData[:])
}
// Begin adds the ip to the resolving queue if it is not cached or already
// resolved.
func (r *RDNS) Begin(ip netip.Addr) {
r.ensurePrivateCache()
if r.isCached(ip) || r.clients.exists(ip, ClientSourceRDNS) {
if r.isCached(ip) || r.clients.clientSource(ip) > ClientSourceRDNS {
return
}
@ -115,15 +123,21 @@ func (r *RDNS) workerLoop() {
defer log.OnPanic("rdns")
for ip := range r.ipCh {
ttl := uint64(revDNSCacheTTL)
host, err := r.exchanger.Exchange(ip.AsSlice())
if err != nil {
log.Debug("rdns: resolving %q: %s", ip, err)
continue
} else if host == "" {
continue
if errors.Is(err, dnsforward.ErrRDNSFailed) {
// Cache failure for a less time.
ttl = revDNSFailureCacheTTL
}
}
_ = r.clients.AddHost(ip, host, ClientSourceRDNS)
r.cache(ip, ttl)
if host != "" {
_ = r.clients.AddHost(ip, host, ClientSourceRDNS)
}
}
}

48
internal/home/rdns_test.go
View File

@ -76,7 +76,7 @@ func TestRDNS_Begin(t *testing.T) {
ipCache := cache.New(cache.Config{
EnableLRU: true,
MaxCount: defaultRDNSCacheSize,
MaxCount: revDNSCacheSize,
})
ttl := make([]byte, binary.Size(uint64(0)))
binary.BigEndian.PutUint64(ttl, uint64(time.Now().Add(100*time.Hour).Unix()))
@ -153,7 +153,7 @@ func TestRDNS_ensurePrivateCache(t *testing.T) {
ipCache := cache.New(cache.Config{
EnableLRU: true,
MaxCount: defaultRDNSCacheSize,
MaxCount: revDNSCacheSize,
})
ex := &rDNSExchanger{
@ -200,25 +200,29 @@ func TestRDNS_WorkerLoop(t *testing.T) {
errUpstream := aghtest.NewErrorUpstream()
testCases := []struct {
ups upstream.Upstream
cliIP netip.Addr
wantLog string
name string
ups upstream.Upstream
cliIP netip.Addr
wantLog string
name string
wantClientSource clientSource
}{{
ups: locUpstream,
cliIP: localIP,
wantLog: "",
name: "all_good",
ups: locUpstream,
cliIP: localIP,
wantLog: "",
name: "all_good",
wantClientSource: ClientSourceRDNS,
}, {
ups: errUpstream,
cliIP: netip.MustParseAddr("192.168.1.2"),
wantLog: `rdns: resolving "192.168.1.2": test upstream error`,
name: "resolve_error",
ups: errUpstream,
cliIP: netip.MustParseAddr("192.168.1.2"),
wantLog: `rdns: resolving "192.168.1.2": test upstream error`,
name: "resolve_error",
wantClientSource: ClientSourceNone,
}, {
ups: locUpstream,
cliIP: netip.MustParseAddr("2a00:1450:400c:c06::93"),
wantLog: "",
name: "ipv6_good",
ups: locUpstream,
cliIP: netip.MustParseAddr("2a00:1450:400c:c06::93"),
wantLog: "",
name: "ipv6_good",
wantClientSource: ClientSourceRDNS,
}}
for _, tc := range testCases {
@ -237,6 +241,10 @@ func TestRDNS_WorkerLoop(t *testing.T) {
},
clients: cc,
ipCh: ch,
ipCache: cache.New(cache.Config{
EnableLRU: true,
MaxCount: revDNSCacheSize,
}),
}
t.Run(tc.name, func(t *testing.T) {
@ -253,11 +261,9 @@ func TestRDNS_WorkerLoop(t *testing.T) {
if tc.wantLog != "" {
assert.Contains(t, w.String(), tc.wantLog)
return
}
assert.True(t, cc.exists(tc.cliIP, ClientSourceRDNS))
assert.Equal(t, tc.wantClientSource, cc.clientSource(tc.cliIP))
})
}
}