Skip to content

Commit

Permalink
Pull request 2174: 6820 Warn local ptrs
Browse files Browse the repository at this point in the history
Squashed commit of the following:

commit c231965
Merge: c6162a2 bcd1430
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Mar 14 18:10:20 2024 +0300

    Merge branch 'master' into 6820-warn-local-ptrs

commit c6162a2
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 13 19:35:20 2024 +0300

    dnsforward: fix doc

commit c6cce96
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 13 19:19:49 2024 +0300

    all: fix private conf fail on start

commit c11fc3e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 13 18:43:31 2024 +0300

    WIP
  • Loading branch information
EugeneOne1 committed Mar 14, 2024
1 parent bcd1430 commit 4e3b53f
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 3 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ NOTE: Add new changes BELOW THIS COMMENT.
- Ability to define custom directories for storage of query log files and
statistics ([#5992]).

### Changed

- Private RDNS resolution (`dns.use_private_ptr_resolvers` in YAML
configuration) now requires a valid "Private reverse DNS servers", when
enabled ([#6820]).

**NOTE:** Disabling private RDNS resolution behaves effectively the same as if
no private reverse DNS servers provided by user and by the OS.

### Fixed

- Statistics for 7 days displayed by day on the dashboard graph ([#6712]).
Expand All @@ -41,6 +50,7 @@ NOTE: Add new changes BELOW THIS COMMENT.
[#6711]: https://github.com/AdguardTeam/AdGuardHome/issues/6711
[#6712]: https://github.com/AdguardTeam/AdGuardHome/issues/6712
[#6740]: https://github.com/AdguardTeam/AdGuardHome/issues/6740
[#6820]: https://github.com/AdguardTeam/AdGuardHome/issues/6820

<!--
NOTE: Add new changes ABOVE THIS COMMENT.
Expand Down
35 changes: 32 additions & 3 deletions internal/dnsforward/dnsforward.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,29 @@ func (s *Server) prepareLocalResolvers(
return uc, nil
}

// LocalResolversError is an error type for errors during local resolvers setup.
// This is only needed to distinguish these errors from errors returned by
// creating the proxy.
type LocalResolversError struct {
Err error
}

// type check
var _ error = (*LocalResolversError)(nil)

// Error implements the error interface for *LocalResolversError.
func (err *LocalResolversError) Error() (s string) {
return fmt.Sprintf("creating local resolvers: %s", err.Err)
}

// type check
var _ errors.Wrapper = (*LocalResolversError)(nil)

// Unwrap implements the [errors.Wrapper] interface for *LocalResolversError.
func (err *LocalResolversError) Unwrap() error {
return err.Err
}

// setupLocalResolvers initializes and sets the resolvers for local addresses.
// It assumes s.serverLock is locked or s not running. It returns the upstream
// configuration used for private PTR resolving, or nil if it's disabled. Note,
Expand All @@ -534,13 +557,15 @@ func (s *Server) setupLocalResolvers(boot upstream.Resolver) (uc *proxy.Upstream
return nil, err
}

s.localResolvers, err = proxy.New(&proxy.Config{
localResolvers, err := proxy.New(&proxy.Config{
UpstreamConfig: uc,
})
if err != nil {
return nil, fmt.Errorf("creating local resolvers: %w", err)
return nil, &LocalResolversError{Err: err}
}

s.localResolvers = localResolvers

// TODO(e.burkov): Should we also consider the DNS64 usage?
return uc, nil
}
Expand Down Expand Up @@ -594,11 +619,13 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) {
return fmt.Errorf("setting up fallback dns servers: %w", err)
}

s.dnsProxy, err = proxy.New(proxyConfig)
dnsProxy, err := proxy.New(proxyConfig)
if err != nil {
return fmt.Errorf("creating proxy: %w", err)
}

s.dnsProxy = dnsProxy

s.recDetector.clear()

s.setupAddrProc()
Expand Down Expand Up @@ -831,6 +858,8 @@ func (s *Server) Reconfigure(conf *ServerConfig) error {
}
}

// TODO(e.burkov): It seems an error here brings the server down, which is
// not reliable enough.
err = s.Prepare(conf)
if err != nil {
return fmt.Errorf("could not reconfigure the server: %w", err)
Expand Down
12 changes: 12 additions & 0 deletions internal/home/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/filtering"
"github.com/AdguardTeam/AdGuardHome/internal/querylog"
"github.com/AdguardTeam/AdGuardHome/internal/stats"
"github.com/AdguardTeam/dnsproxy/upstream"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
"github.com/AdguardTeam/golibs/netutil"
Expand Down Expand Up @@ -157,6 +158,17 @@ func initDNSServer(
}

err = Context.dnsServer.Prepare(dnsConf)

// TODO(e.burkov): Recreate the server with private RDNS disabled. This
// should go away once the private RDNS resolution is moved to the proxy.
var locResErr *dnsforward.LocalResolversError
if errors.As(err, &locResErr) && errors.Is(locResErr.Err, upstream.ErrNoUpstreams) {
log.Info("WARNING: no local resolvers configured while private RDNS " +
"resolution enabled, trying to disable")
dnsConf.UsePrivateRDNS = false
err = Context.dnsServer.Prepare(dnsConf)
}

if err != nil {
return fmt.Errorf("dnsServer.Prepare: %w", err)
}
Expand Down

0 comments on commit 4e3b53f

Please sign in to comment.