From 4de1eb2bca1047426e02ba680c212f46782e5616 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Wed, 10 Apr 2024 16:09:58 +0300 Subject: [PATCH] WIP --- internal/dnsforward/config.go | 24 ++++- internal/dnsforward/dnsforward.go | 167 +++++++++++++++++------------ internal/dnsforward/http.go | 68 ++++++------ internal/dnsforward/upstreams.go | 170 +++++++++++++++--------------- internal/home/dns.go | 6 +- 5 files changed, 241 insertions(+), 194 deletions(-) diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 10f238d2682..4d2924abfa3 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -472,6 +472,26 @@ func (s *Server) prepareIpsetListSettings() (err error) { return s.ipset.init(ipsets) } +// loadUpstreams parses upstream DNS servers from the configured file or from +// the configuration itself. +func (conf *ServerConfig) loadUpstreams() (upstreams []string, err error) { + if conf.UpstreamDNSFileName == "" { + return stringutil.FilterOut(conf.UpstreamDNS, IsCommentOrEmpty), nil + } + + var data []byte + data, err = os.ReadFile(conf.UpstreamDNSFileName) + if err != nil { + return nil, fmt.Errorf("reading upstream from file: %w", err) + } + + upstreams = stringutil.SplitTrimmed(string(data), "\n") + + log.Debug("dnsforward: got %d upstreams in %q", len(upstreams), conf.UpstreamDNSFileName) + + return stringutil.FilterOut(upstreams, IsCommentOrEmpty), nil +} + // collectListenAddr adds addrPort to addrs. It also adds its port to // unspecPorts if its address is unspecified. func collectListenAddr( @@ -543,8 +563,8 @@ func (m *combinedAddrPortSet) Has(addrPort netip.AddrPort) (ok bool) { return m.ports.Has(addrPort.Port()) && m.addrs.Has(addrPort.Addr()) } -// filterOut filters out all the upstreams that match um. It returns all the -// closing errors joined. +// filterOutAddrs filters out all the upstreams that match um. It returns all +// the closing errors joined. func filterOutAddrs(upsConf *proxy.UpstreamConfig, set addrPortSet) (err error) { var errs []error delFunc := func(u upstream.Upstream) (ok bool) { diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index 6776b6e8f02..8b4c6afb2e3 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -451,69 +451,6 @@ func (s *Server) startLocked() error { return err } -// ErrBadPrivateRDNSUpstreams is returned when the private rDNS upstreams are -// invalid but enabled. -// -// TODO(e.burkov): Consider allowing to use incomplete private rDNS upstreams -// configuration in proxy when the private rDNS function is enabled. In theory, -// proxy supports the case when no upstreams provided to resolve the private -// request, since it already supports this for DNS64-prefixed PTR requests. -const ErrBadPrivateRDNSUpstreams errors.Error = "bad private rDNS upstreams" - -// prepareLocalResolvers initializes the local upstreams configuration using -// boot as bootstrap. It assumes that s.serverLock is locked or s not running. -func (s *Server) prepareLocalResolvers( - boot upstream.Resolver, -) (uc *proxy.UpstreamConfig, err error) { - set, err := s.conf.ourAddrsSet() - if err != nil { - // Don't wrap the error because it's informative enough as is. - return nil, err - } - - resolvers := s.conf.LocalPTRResolvers - confNeedsFiltering := len(resolvers) > 0 - if confNeedsFiltering { - resolvers = stringutil.FilterOut(resolvers, IsCommentOrEmpty) - } else { - sysResolvers := slices.DeleteFunc(slices.Clone(s.sysResolvers.Addrs()), set.Has) - resolvers = make([]string, 0, len(sysResolvers)) - for _, r := range sysResolvers { - resolvers = append(resolvers, r.String()) - } - } - - log.Debug("dnsforward: upstreams to resolve ptr for local addresses: %v", resolvers) - - uc, err = s.prepareUpstreamConfig(resolvers, nil, &upstream.Options{ - Bootstrap: boot, - Timeout: defaultLocalTimeout, - // TODO(e.burkov): Should we verify server's certificates? - PreferIPv6: s.conf.BootstrapPreferIPv6, - }) - if err != nil { - return nil, fmt.Errorf("preparing private upstreams: %w", err) - } - - if confNeedsFiltering { - err = filterOutAddrs(uc, set) - if err != nil { - return nil, fmt.Errorf("filtering private upstreams: %w", err) - } - } - - // Prevalidate the config to catch the exact error before creating proxy. - // See TODO on [ErrBadPrivateRDNSUpstreams]. - err = proxy.ValidatePrivateConfig(uc, s.privateNets) - if err != nil { - log.Debug("dnsforward: validating private rdns upstreams: %s", err) - - return nil, ErrBadPrivateRDNSUpstreams - } - - return uc, nil -} - // Prepare initializes parameters of s using data from conf. conf must not be // nil. func (s *Server) Prepare(conf *ServerConfig) (err error) { @@ -571,6 +508,95 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) { return nil } +// prepareUpstreamSettings sets upstream DNS server settings. +func (s *Server) prepareUpstreamSettings(boot upstream.Resolver) (err error) { + // Load upstreams either from the file, or from the settings + var upstreams []string + upstreams, err = s.conf.loadUpstreams() + if err != nil { + return fmt.Errorf("loading upstreams: %w", err) + } + + s.conf.UpstreamConfig, err = newUpstreamConfig(upstreams, defaultDNS, &upstream.Options{ + Bootstrap: boot, + Timeout: s.conf.UpstreamTimeout, + HTTPVersions: UpstreamHTTPVersions(s.conf.UseHTTP3Upstreams), + PreferIPv6: s.conf.BootstrapPreferIPv6, + // Use a customized set of RootCAs, because Go's default mechanism of + // loading TLS roots does not always work properly on some routers so we're + // loading roots manually and pass it here. + // + // See [aghtls.SystemRootCAs]. + // + // TODO(a.garipov): Investigate if that's true. + RootCAs: s.conf.TLSv12Roots, + CipherSuites: s.conf.TLSCiphers, + }) + if err != nil { + return fmt.Errorf("preparing upstream config: %w", err) + } + + return nil +} + +// PrivateRDNSError is returned when the private rDNS upstreams are +// invalid but enabled. +// +// TODO(e.burkov): Consider allowing to use incomplete private rDNS upstreams +// configuration in proxy when the private rDNS function is enabled. In theory, +// proxy supports the case when no upstreams provided to resolve the private +// request, since it already supports this for DNS64-prefixed PTR requests. +type PrivateRDNSError struct { + err error +} + +// Error implements the [errors.Error] interface. +func (e *PrivateRDNSError) Error() (s string) { + return e.err.Error() +} + +func (e *PrivateRDNSError) Unwrap() (err error) { + return e.err +} + +// prepareLocalResolvers initializes the private RDNS upstream configuration +// according to the server's settings. It assumes s.serverLock is locked or the +// Server not running. +func (s *Server) prepareLocalResolvers() (uc *proxy.UpstreamConfig, err error) { + if !s.conf.UsePrivateRDNS { + return nil, nil + } + + var ownAddrs addrPortSet + ownAddrs, err = s.conf.ourAddrsSet() + if err != nil { + // Don't wrap the error, because it's informative enough as is. + return nil, err + } + + opts := &upstream.Options{ + Bootstrap: s.bootstrap, + Timeout: defaultLocalTimeout, + // TODO(e.burkov): Should we verify server's certificates? + PreferIPv6: s.conf.BootstrapPreferIPv6, + } + + addrs := s.conf.LocalPTRResolvers + uc, err = newLocalResolvers(addrs, ownAddrs, s.sysResolvers, s.privateNets, opts) + if err != nil { + return nil, fmt.Errorf("preparing resolvers: %w", err) + } + + // Prevalidate the config to catch the exact error before creating proxy. + // See TODO on [ErrBadPrivateRDNSUpstreams]. + err = proxy.ValidatePrivateConfig(uc, s.privateNets) + if err != nil { + return nil, err + } + + return uc, nil +} + // prepareInternalDNS initializes the internal state of s before initializing // the primary DNS proxy instance. It assumes s.serverLock is locked or the // Server not running. @@ -580,10 +606,12 @@ func (s *Server) prepareInternalDNS() (err error) { return fmt.Errorf("preparing ipset settings: %w", err) } - s.bootstrap, s.bootResolvers, err = s.createBootstrap(s.conf.BootstrapDNS, &upstream.Options{ + bootOpts := &upstream.Options{ Timeout: DefaultTimeout, HTTPVersions: UpstreamHTTPVersions(s.conf.UseHTTP3Upstreams), - }) + } + + s.bootstrap, s.bootResolvers, err = newBootstrap(s.conf.BootstrapDNS, s.etcHosts, bootOpts) if err != nil { // Don't wrap the error, because it's informative enough as is. return err @@ -595,11 +623,9 @@ func (s *Server) prepareInternalDNS() (err error) { return err } - if s.conf.UsePrivateRDNS { - s.conf.PrivateRDNSUpstreamConfig, err = s.prepareLocalResolvers(s.bootstrap) - if err != nil { - return fmt.Errorf("setting up resolvers: %w", err) - } + s.conf.PrivateRDNSUpstreamConfig, err = s.prepareLocalResolvers() + if err != nil { + return err } err = s.prepareInternalProxy() @@ -694,6 +720,7 @@ func (s *Server) prepareInternalProxy() (err error) { UseDNS64: srvConf.UseDNS64, DNS64Prefs: srvConf.DNS64Prefixes, UsePrivateRDNS: srvConf.UsePrivateRDNS, + PrivateSubnets: s.privateNets, MessageConstructor: s, } diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go index 2514f6640ed..ea7592937d6 100644 --- a/internal/dnsforward/http.go +++ b/internal/dnsforward/http.go @@ -264,10 +264,14 @@ func (req *jsonDNSConfig) checkUpstreamMode() (err error) { // validate returns an error if any field of req is invalid. // // TODO(s.chzhen): Parse, don't validate. -func (req *jsonDNSConfig) validate(privateNets netutil.SubnetSet) (err error) { +func (req *jsonDNSConfig) validate( + ownAddrs addrPortSet, + sysResolvers SystemResolvers, + privateNets netutil.SubnetSet, +) (err error) { defer func() { err = errors.Annotate(err, "validating dns config: %w") }() - err = req.validateUpstreamDNSServers(privateNets) + err = req.validateUpstreamDNSServers(ownAddrs, sysResolvers, privateNets) if err != nil { // Don't wrap the error since it's informative enough as is. return err @@ -300,25 +304,6 @@ func (req *jsonDNSConfig) validate(privateNets netutil.SubnetSet) (err error) { return nil } -// checkUpstreams returns an error if lines can't be parsed as an upstream -// configuration. If privateNets is not nil, it also checks that the domain -// specifications are strictly ARPA domains containing the prefixes within the -// set. -func checkUpstreams(lines []string, section string, privateNets netutil.SubnetSet) (err error) { - defer func() { err = errors.Annotate(err, "%s servers: %w", section) }() - - uc, err := proxy.ParseUpstreamsConfig(lines, &upstream.Options{}) - if err == nil { - defer func() { err = errors.WithDeferred(err, uc.Close()) }() - - if privateNets != nil { - err = proxy.ValidatePrivateConfig(uc, privateNets) - } - } - - return err -} - // checkBootstrap returns an error if any bootstrap address is invalid. func (req *jsonDNSConfig) checkBootstrap() (err error) { if req.Bootstraps == nil { @@ -348,20 +333,27 @@ func (req *jsonDNSConfig) checkBootstrap() (err error) { } // validateUpstreamDNSServers returns an error if any field of req is invalid. -func (req *jsonDNSConfig) validateUpstreamDNSServers(privateNets netutil.SubnetSet) (err error) { +func (req *jsonDNSConfig) validateUpstreamDNSServers( + ownAddrs addrPortSet, + sysResolvers SystemResolvers, + privateNets netutil.SubnetSet, +) (err error) { + var uc *proxy.UpstreamConfig + opts := &upstream.Options{} + if req.Upstreams != nil { - err = checkUpstreams(*req.Upstreams, "upstream", nil) + uc, err = newUpstreamConfig(*req.Upstreams, nil, opts) + err = errors.WithDeferred(err, uc.Close()) if err != nil { - // Don't wrap the error since it's informative enough as is. - return err + return fmt.Errorf("upstream servers: %w", err) } } - if req.LocalPTRUpstreams != nil { - err = checkUpstreams(*req.LocalPTRUpstreams, "private upstream", privateNets) + if addrs := req.LocalPTRUpstreams; addrs != nil { + uc, err = newLocalResolvers(*addrs, ownAddrs, sysResolvers, privateNets, opts) + err = errors.WithDeferred(err, uc.Close()) if err != nil { - // Don't wrap the error since it's informative enough as is. - return err + return fmt.Errorf("private upstream servers: %w", err) } } @@ -372,10 +364,10 @@ func (req *jsonDNSConfig) validateUpstreamDNSServers(privateNets netutil.SubnetS } if req.Fallbacks != nil { - err = checkUpstreams(*req.Fallbacks, "fallback", nil) + uc, err = newUpstreamConfig(*req.Fallbacks, nil, opts) + err = errors.WithDeferred(err, uc.Close()) if err != nil { - // Don't wrap the error since it's informative enough as is. - return err + return fmt.Errorf("fallback servers: %w", err) } } @@ -445,7 +437,15 @@ func (s *Server) handleSetConfig(w http.ResponseWriter, r *http.Request) { return } - err = req.validate(s.privateNets) + // TODO(e.burkov): Consider prebuilding this set on startup. + ourAddrs, err := s.conf.ourAddrsSet() + if err != nil { + aghhttp.Error(r, w, http.StatusInternalServerError, "getting our addresses: %s", err) + + return + } + + err = req.validate(ourAddrs, s.sysResolvers, s.privateNets) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) @@ -596,7 +596,7 @@ func (s *Server) handleTestUpstreamDNS(w http.ResponseWriter, r *http.Request) { } var boots []*upstream.UpstreamResolver - opts.Bootstrap, boots, err = s.createBootstrap(req.BootstrapDNS, opts) + opts.Bootstrap, boots, err = newBootstrap(req.BootstrapDNS, s.etcHosts, opts) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "Failed to parse bootstrap servers: %s", err) diff --git a/internal/dnsforward/upstreams.go b/internal/dnsforward/upstreams.go index 200b71fe519..9fd574f071e 100644 --- a/internal/dnsforward/upstreams.go +++ b/internal/dnsforward/upstreams.go @@ -2,85 +2,77 @@ package dnsforward import ( "fmt" - "os" + "slices" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/dnsproxy/proxy" "github.com/AdguardTeam/dnsproxy/upstream" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/stringutil" ) -// loadUpstreams parses upstream DNS servers from the configured file or from -// the configuration itself. -func (s *Server) loadUpstreams() (upstreams []string, err error) { - if s.conf.UpstreamDNSFileName == "" { - return stringutil.FilterOut(s.conf.UpstreamDNS, IsCommentOrEmpty), nil +// newBootstrap returns a bootstrap resolver based on the configuration of s. +// boots are the upstream resolvers that should be closed after use. r is the +// actual bootstrap resolver, which may include the system hosts. +// +// TODO(e.burkov): This function currently returns a resolver and a slice of +// the upstream resolvers, which are essentially the same. boots are returned +// for being able to close them afterwards, but it introduces an implicit +// contract that r could only be used before that. Anyway, this code should +// improve when the [proxy.UpstreamConfig] will become an [upstream.Resolver] +// and be used here. +func newBootstrap( + addrs []string, + etcHosts upstream.Resolver, + opts *upstream.Options, +) (r upstream.Resolver, boots []*upstream.UpstreamResolver, err error) { + if len(addrs) == 0 { + addrs = defaultBootstrap } - var data []byte - data, err = os.ReadFile(s.conf.UpstreamDNSFileName) + boots, err = aghnet.ParseBootstraps(addrs, opts) if err != nil { - return nil, fmt.Errorf("reading upstream from file: %w", err) + // Don't wrap the error, since it's informative enough as is. + return nil, nil, err } - upstreams = stringutil.SplitTrimmed(string(data), "\n") - - log.Debug("dnsforward: got %d upstreams in %q", len(upstreams), s.conf.UpstreamDNSFileName) - - return stringutil.FilterOut(upstreams, IsCommentOrEmpty), nil -} - -// prepareUpstreamSettings sets upstream DNS server settings. -func (s *Server) prepareUpstreamSettings(boot upstream.Resolver) (err error) { - // Load upstreams either from the file, or from the settings - var upstreams []string - upstreams, err = s.loadUpstreams() - if err != nil { - return fmt.Errorf("loading upstreams: %w", err) + var parallel upstream.ParallelResolver + for _, b := range boots { + parallel = append(parallel, upstream.NewCachingResolver(b)) } - s.conf.UpstreamConfig, err = s.prepareUpstreamConfig(upstreams, defaultDNS, &upstream.Options{ - Bootstrap: boot, - Timeout: s.conf.UpstreamTimeout, - HTTPVersions: UpstreamHTTPVersions(s.conf.UseHTTP3Upstreams), - PreferIPv6: s.conf.BootstrapPreferIPv6, - // Use a customized set of RootCAs, because Go's default mechanism of - // loading TLS roots does not always work properly on some routers so we're - // loading roots manually and pass it here. - // - // See [aghtls.SystemRootCAs]. - // - // TODO(a.garipov): Investigate if that's true. - RootCAs: s.conf.TLSv12Roots, - CipherSuites: s.conf.TLSCiphers, - }) - if err != nil { - return fmt.Errorf("preparing upstream config: %w", err) + if etcHosts != nil { + r = upstream.ConsequentResolver{etcHosts, parallel} + } else { + r = parallel } - return nil + return r, boots, nil } -// prepareUpstreamConfig returns the upstream configuration based on upstreams -// and configuration of s. -func (s *Server) prepareUpstreamConfig( +// newUpstreamConfig returns the upstream configuration based on upstreams. If +// upstreams slice specifies no default upstreams, defaultUpstreams are used to +// create upstreams with no domain specifications. opts are used when creating +// upstream configuration. +func newUpstreamConfig( upstreams []string, defaultUpstreams []string, opts *upstream.Options, ) (uc *proxy.UpstreamConfig, err error) { uc, err = proxy.ParseUpstreamsConfig(upstreams, opts) if err != nil { - return nil, fmt.Errorf("parsing upstream config: %w", err) + return uc, fmt.Errorf("parsing upstreams: %w", err) } - if len(uc.Upstreams) == 0 && defaultUpstreams != nil { + if len(uc.Upstreams) == 0 && len(defaultUpstreams) > 0 { log.Info("dnsforward: warning: no default upstreams specified, using %v", defaultUpstreams) + var defaultUpstreamConfig *proxy.UpstreamConfig defaultUpstreamConfig, err = proxy.ParseUpstreamsConfig(defaultUpstreams, opts) if err != nil { - return nil, fmt.Errorf("parsing default upstreams: %w", err) + return uc, fmt.Errorf("parsing default upstreams: %w", err) } uc.Upstreams = defaultUpstreamConfig.Upstreams @@ -89,6 +81,52 @@ func (s *Server) prepareUpstreamConfig( return uc, nil } +// newLocalResolvers creates an upstream configuration for resolving PTR records +// for local addresses. The configuration is built either from the provided +// addresses or from the system resolvers. unwanted filters the resulting +// upstream configuration. +func newLocalResolvers( + addrs []string, + unwanted addrPortSet, + sysResolvers SystemResolvers, + privateNets netutil.SubnetSet, + opts *upstream.Options, +) (uc *proxy.UpstreamConfig, err error) { + confNeedsFiltering := len(addrs) > 0 + if confNeedsFiltering { + addrs = stringutil.FilterOut(addrs, IsCommentOrEmpty) + } else { + sysResolvers := slices.DeleteFunc(slices.Clone(sysResolvers.Addrs()), unwanted.Has) + addrs = make([]string, 0, len(sysResolvers)) + for _, r := range sysResolvers { + addrs = append(addrs, r.String()) + } + } + + log.Debug("dnsforward: upstreams to resolve ptr for local addresses: %v", addrs) + + uc, err = newUpstreamConfig(addrs, nil, opts) + if err != nil { + return nil, fmt.Errorf("preparing private upstreams: %w", err) + } + + if confNeedsFiltering { + err = filterOutAddrs(uc, unwanted) + if err != nil { + return nil, fmt.Errorf("filtering private upstreams: %w", err) + } + + err = proxy.ValidatePrivateConfig(uc, privateNets) + if err != nil { + log.Debug("dnsforward: validating private rdns upstreams: %s", err) + + return nil, &PrivateRDNSError{err: err} + } + } + + return uc, nil +} + // UpstreamHTTPVersions returns the HTTP versions for upstream configuration // depending on configuration. func UpstreamHTTPVersions(http3 bool) (v []upstream.HTTPVersion) { @@ -125,44 +163,6 @@ func setProxyUpstreamMode( return nil } -// createBootstrap returns a bootstrap resolver based on the configuration of s. -// boots are the upstream resolvers that should be closed after use. r is the -// actual bootstrap resolver, which may include the system hosts. -// -// TODO(e.burkov): This function currently returns a resolver and a slice of -// the upstream resolvers, which are essentially the same. boots are returned -// for being able to close them afterwards, but it introduces an implicit -// contract that r could only be used before that. Anyway, this code should -// improve when the [proxy.UpstreamConfig] will become an [upstream.Resolver] -// and be used here. -func (s *Server) createBootstrap( - addrs []string, - opts *upstream.Options, -) (r upstream.Resolver, boots []*upstream.UpstreamResolver, err error) { - if len(addrs) == 0 { - addrs = defaultBootstrap - } - - boots, err = aghnet.ParseBootstraps(addrs, opts) - if err != nil { - // Don't wrap the error, since it's informative enough as is. - return nil, nil, err - } - - var parallel upstream.ParallelResolver - for _, b := range boots { - parallel = append(parallel, upstream.NewCachingResolver(b)) - } - - if s.etcHosts != nil { - r = upstream.ConsequentResolver{s.etcHosts, parallel} - } else { - r = parallel - } - - return r, boots, nil -} - // IsCommentOrEmpty returns true if s starts with a "#" character or is empty. // This function is useful for filtering out non-upstream lines from upstream // configs. diff --git a/internal/home/dns.go b/internal/home/dns.go index 5a23e3575fc..0ca9717d50f 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -159,9 +159,9 @@ func initDNSServer( // Try to prepare the server with disabled private RDNS resolution if it // failed to prepare as is. See TODO on [ErrBadPrivateRDNSUpstreams]. err = Context.dnsServer.Prepare(dnsConf) - if errors.Is(err, dnsforward.ErrBadPrivateRDNSUpstreams) { - log.Info("WARNING: no local resolvers configured while private RDNS " + - "resolution enabled, trying to disable") + if privRDNSErr := (&dnsforward.PrivateRDNSError{}); errors.As(err, &privRDNSErr) { + log.Info("WARNING: %s; trying to disable private RDNS resolution", err) + dnsConf.UsePrivateRDNS = false err = Context.dnsServer.Prepare(dnsConf) }