Pull request 2270: AGDNS-2374-slog-ipset

Squashed commit of the following:

commit 51ff7d8c49d174d057b4f508f3e113e1ca86bd1a
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Aug 22 13:50:10 2024 +0300

    dnsforward: imp code

commit a1c0011273fc83ec1b509a9d930bca5e278e1e2c
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Aug 21 21:53:01 2024 +0300

    dnsforward: imp code

commit a64fd6b3f037712927a583d04296fcaf821f6442
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Aug 21 21:28:48 2024 +0300

    dnsforward: imp code

commit 37ccae4e923a7e688e79a135b0e49a746e9b2a06
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Aug 21 20:23:58 2024 +0300

    all: imp code

commit 03c69ab2729eb424d768def986cba83731ad3e3b
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Aug 21 19:08:30 2024 +0300

    all: imp code

commit 72adfb101fcdb42635702c1f1c4e13ddcc95bfdc
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Aug 21 16:42:44 2024 +0300

    all: slog ipset
This commit is contained in:
Stanislav Chzhen
2024-08-26 13:30:00 +03:00
parent 30c0bbe5cc
commit 738958d90a
8 changed files with 157 additions and 87 deletions

View File

@@ -159,7 +159,7 @@ type Config struct {
// IpsetList is the ipset configuration that allows AdGuard Home to add IP
// addresses of the specified domain names to an ipset list. Syntax:
//
// DOMAIN[,DOMAIN].../IPSET_NAME
// DOMAIN[,DOMAIN].../IPSET_NAME[,IPSET_NAME]...
//
// This field is ignored if [IpsetListFileName] is set.
IpsetList []string `yaml:"ipset"`
@@ -457,24 +457,24 @@ func (s *Server) initDefaultSettings() {
// prepareIpsetListSettings reads and prepares the ipset configuration either
// from a file or from the data in the configuration file.
func (s *Server) prepareIpsetListSettings() (err error) {
func (s *Server) prepareIpsetListSettings() (ipsets []string, err error) {
fn := s.conf.IpsetListFileName
if fn == "" {
return s.ipset.init(s.conf.IpsetList)
return s.conf.IpsetList, nil
}
// #nosec G304 -- Trust the path explicitly given by the user.
data, err := os.ReadFile(fn)
if err != nil {
return err
return nil, err
}
ipsets := stringutil.SplitTrimmed(string(data), "\n")
ipsets = stringutil.FilterOut(ipsets, IsCommentOrEmpty)
ipsets = stringutil.SplitTrimmed(string(data), "\n")
ipsets = slices.DeleteFunc(ipsets, IsCommentOrEmpty)
log.Debug("dns: using %d ipset rules from file %q", len(ipsets), fn)
return s.ipset.init(ipsets)
return ipsets, nil
}
// loadUpstreams parses upstream DNS servers from the configured file or from

View File

@@ -133,8 +133,9 @@ type Server struct {
// must be a valid domain name plus dots on each side.
localDomainSuffix string
// ipset processes DNS requests using ipset data.
ipset ipsetCtx
// ipset processes DNS requests using ipset data. It must not be nil after
// initialization. See [newIpsetHandler].
ipset *ipsetHandler
// privateNets is the configured set of IP networks considered private.
privateNets netutil.SubnetSet
@@ -609,11 +610,18 @@ func (s *Server) prepareLocalResolvers() (uc *proxy.UpstreamConfig, err error) {
// the primary DNS proxy instance. It assumes s.serverLock is locked or the
// Server not running.
func (s *Server) prepareInternalDNS() (err error) {
err = s.prepareIpsetListSettings()
ipsetList, err := s.prepareIpsetListSettings()
if err != nil {
return fmt.Errorf("preparing ipset settings: %w", err)
}
ipsetLogger := s.logger.With(slogutil.KeyPrefix, "ipset")
s.ipset, err = newIpsetHandler(context.TODO(), ipsetLogger, ipsetList)
if err != nil {
// Don't wrap the error, because it's informative enough as is.
return err
}
bootOpts := &upstream.Options{
Timeout: DefaultTimeout,
HTTPVersions: UpstreamHTTPVersions(s.conf.UseHTTP3Upstreams),

View File

@@ -1,28 +1,43 @@
package dnsforward
import (
"context"
"fmt"
"log/slog"
"net"
"os"
"strings"
"github.com/AdguardTeam/AdGuardHome/internal/ipset"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
"github.com/AdguardTeam/golibs/logutil/slogutil"
"github.com/miekg/dns"
)
// ipsetCtx is the ipset context. ipsetMgr can be nil.
type ipsetCtx struct {
// ipsetHandler is the ipset context. ipsetMgr can be nil.
type ipsetHandler struct {
ipsetMgr ipset.Manager
logger *slog.Logger
}
// init initializes the ipset context. It is not safe for concurrent use.
//
// TODO(a.garipov): Rewrite into a simple constructor?
func (c *ipsetCtx) init(ipsetConf []string) (err error) {
c.ipsetMgr, err = ipset.NewManager(ipsetConf)
if errors.Is(err, os.ErrInvalid) || errors.Is(err, os.ErrPermission) {
// newIpsetHandler returns a new initialized [ipsetHandler]. It is not safe for
// concurrent use.
func newIpsetHandler(
ctx context.Context,
logger *slog.Logger,
ipsetList []string,
) (h *ipsetHandler, err error) {
h = &ipsetHandler{
logger: logger,
}
conf := &ipset.Config{
Logger: logger,
Lines: ipsetList,
}
h.ipsetMgr, err = ipset.NewManager(ctx, conf)
if errors.Is(err, os.ErrInvalid) ||
errors.Is(err, os.ErrPermission) ||
errors.Is(err, errors.ErrUnsupported) {
// ipset cannot currently be initialized if the server was installed
// from Snap or when the user or the binary doesn't have the required
// permissions, or when the kernel doesn't support netfilter.
@@ -31,30 +46,28 @@ func (c *ipsetCtx) init(ipsetConf []string) (err error) {
//
// TODO(a.garipov): The Snap problem can probably be solved if we add
// the netlink-connector interface plug.
log.Info("ipset: warning: cannot initialize: %s", err)
logger.WarnContext(ctx, "cannot initialize", slogutil.KeyError, err)
return nil
} else if errors.Is(err, errors.ErrUnsupported) {
log.Info("ipset: warning: %s", err)
return nil
return h, nil
} else if err != nil {
return fmt.Errorf("initializing ipset: %w", err)
return nil, fmt.Errorf("initializing ipset: %w", err)
}
return h, nil
}
// close closes the Linux Netfilter connections. close can be called on a nil
// handler.
func (h *ipsetHandler) close() (err error) {
if h != nil && h.ipsetMgr != nil {
return h.ipsetMgr.Close()
}
return nil
}
// close closes the Linux Netfilter connections.
func (c *ipsetCtx) close() (err error) {
if c.ipsetMgr != nil {
return c.ipsetMgr.Close()
}
return nil
}
func (c *ipsetCtx) dctxIsfilled(dctx *dnsContext) (ok bool) {
// dctxIsFilled returns true if dctx has enough information to process.
func dctxIsFilled(dctx *dnsContext) (ok bool) {
return dctx != nil &&
dctx.responseFromUpstream &&
dctx.proxyCtx != nil &&
@@ -65,8 +78,8 @@ func (c *ipsetCtx) dctxIsfilled(dctx *dnsContext) (ok bool) {
// skipIpsetProcessing returns true when the ipset processing can be skipped for
// this request.
func (c *ipsetCtx) skipIpsetProcessing(dctx *dnsContext) (ok bool) {
if c == nil || c.ipsetMgr == nil || !c.dctxIsfilled(dctx) {
func (h *ipsetHandler) skipIpsetProcessing(dctx *dnsContext) (ok bool) {
if h == nil || h.ipsetMgr == nil || !dctxIsFilled(dctx) {
return true
}
@@ -108,31 +121,31 @@ func ipsFromAnswer(ans []dns.RR) (ip4s, ip6s []net.IP) {
}
// process adds the resolved IP addresses to the domain's ipsets, if any.
func (c *ipsetCtx) process(dctx *dnsContext) (rc resultCode) {
log.Debug("dnsforward: ipset: started processing")
defer log.Debug("dnsforward: ipset: finished processing")
func (h *ipsetHandler) process(dctx *dnsContext) (rc resultCode) {
// TODO(s.chzhen): Use passed context.
ctx := context.TODO()
h.logger.DebugContext(ctx, "started processing")
defer h.logger.DebugContext(ctx, "finished processing")
if c.skipIpsetProcessing(dctx) {
if h.skipIpsetProcessing(dctx) {
return resultCodeSuccess
}
log.Debug("ipset: starting processing")
req := dctx.proxyCtx.Req
host := req.Question[0].Name
host = strings.TrimSuffix(host, ".")
host = strings.ToLower(host)
ip4s, ip6s := ipsFromAnswer(dctx.proxyCtx.Res.Answer)
n, err := c.ipsetMgr.Add(host, ip4s, ip6s)
n, err := h.ipsetMgr.Add(ctx, host, ip4s, ip6s)
if err != nil {
// Consider ipset errors non-critical to the request.
log.Error("dnsforward: ipset: adding host ips: %s", err)
h.logger.ErrorContext(ctx, "adding host ips", slogutil.KeyError, err)
return resultCodeSuccess
}
log.Debug("dnsforward: ipset: added %d new ipset entries", n)
h.logger.DebugContext(ctx, "added new ipset entries", "num", n)
return resultCodeSuccess
}

View File

@@ -1,10 +1,12 @@
package dnsforward
import (
"context"
"net"
"testing"
"github.com/AdguardTeam/dnsproxy/proxy"
"github.com/AdguardTeam/golibs/logutil/slogutil"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert"
)
@@ -16,7 +18,7 @@ type fakeIpsetMgr struct {
}
// Add implements the aghnet.IpsetManager interface for *fakeIpsetMgr.
func (m *fakeIpsetMgr) Add(host string, ip4s, ip6s []net.IP) (n int, err error) {
func (m *fakeIpsetMgr) Add(_ context.Context, host string, ip4s, ip6s []net.IP) (n int, err error) {
m.ip4s = append(m.ip4s, ip4s...)
m.ip6s = append(m.ip6s, ip6s...)
@@ -58,7 +60,9 @@ func TestIpsetCtx_process(t *testing.T) {
responseFromUpstream: true,
}
ictx := &ipsetCtx{}
ictx := &ipsetHandler{
logger: slogutil.NewDiscardLogger(),
}
rc := ictx.process(dctx)
assert.Equal(t, resultCodeSuccess, rc)
@@ -77,8 +81,9 @@ func TestIpsetCtx_process(t *testing.T) {
}
m := &fakeIpsetMgr{}
ictx := &ipsetCtx{
ictx := &ipsetHandler{
ipsetMgr: m,
logger: slogutil.NewDiscardLogger(),
}
rc := ictx.process(dctx)
@@ -101,8 +106,9 @@ func TestIpsetCtx_process(t *testing.T) {
}
m := &fakeIpsetMgr{}
ictx := &ipsetCtx{
ictx := &ipsetHandler{
ipsetMgr: m,
logger: slogutil.NewDiscardLogger(),
}
rc := ictx.process(dctx)
@@ -124,8 +130,9 @@ func TestIpsetCtx_SkipIpsetProcessing(t *testing.T) {
}
m := &fakeIpsetMgr{}
ictx := &ipsetCtx{
ictx := &ipsetHandler{
ipsetMgr: m,
logger: slogutil.NewDiscardLogger(),
}
testCases := []struct {