Pull request: 3257 source directive

Merge in DNS/adguard-home from 3257-ifaces-source to master

Updates #3257.

Squashed commit of the following:

commit 0b9b42bab731bbd048e97893cf209794ea014dfe
Merge: 530a1a23 e25a5329
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Jun 28 16:53:36 2021 +0300

    Merge branch 'master' into 3257-ifaces-source

commit 530a1a23a601c5575c8dc5f6f97cd84801cf911b
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Jun 25 19:43:55 2021 +0300

    aghnet: imp code, add docs

commit 58de84821b93bcbb3df1834ba33fbad817201b1d
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Jun 25 13:34:43 2021 +0300

    aghnet: sup "source" directive

commit c0901abd5212902295e8ee546fad652092fdb5a8
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 24 16:46:03 2021 +0300

    aghos: mv func to aghnet

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to
 
 ### Added
 
+- `source` directives support in `/etc/network/interfaces` on Linux ([#3257]).
 - RFC 9000 support in DNS-over-QUIC.
 - Completely disabling statistics by setting the statistics interval to zero
   ([#2141]).

HACKING.md

@@ -98,8 +98,9 @@ attributes to make it work in Markdown renderers that strip "id". -->
  *  Constructors should validate their arguments and return meaningful errors.
     As a corollary, avoid lazy initialization.
 
- *  Define `MarshalFoo` methods on non-pointer receivers, as pointer receivers
+ *  Prefer to define methods on pointer receievers, unless the type is small or
-    [can have surprising results][staticcheck-911].
+    a non-pointer receiever is required, for example `MarshalFoo` methods (see
+    [staticcheck-911]).
 
  *  Don't mix horizontal and vertical placement of arguments in function and
     method calls.  That is, either this:

internal/aghnet/net.go

@@ -71,6 +71,12 @@ func CanBindPort(port int) (can bool, err error) {
 	return true, nil
 }
 
+// CanBindPrivilegedPorts checks if current process can bind to privileged
+// ports.
+func CanBindPrivilegedPorts() (can bool, err error) {
+	return canBindPrivilegedPorts()
+}
+
 // NetInterface represents an entry of network interfaces map.
 type NetInterface struct {
 	MTU          int              `json:"mtu"`

internal/aghnet/net_darwin.go

@@ -23,6 +23,10 @@ type hardwarePortInfo struct {
 	static    bool
 }
 
+func canBindPrivilegedPorts() (can bool, err error) {
+	return aghos.HaveAdminRights()
+}
+
 func ifaceHasStaticIP(ifaceName string) (bool, error) {
 	portInfo, err := getCurrentHardwarePortInfo(ifaceName)
 	if err != nil {

internal/aghnet/net_linux.go

@@ -9,16 +9,132 @@ import (
 	"io"
 	"net"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghio"
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
+	"github.com/AdguardTeam/AdGuardHome/internal/aghstrings"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/google/renameio/maybe"
+	"golang.org/x/sys/unix"
 )
 
-// maxConfigFileSize is the maximum assumed length of the interface
+// recurrentChecker is used to check all the files which may include references
-// configuration file.
+// for other ones.
-const maxConfigFileSize = 1024 * 1024
+type recurrentChecker struct {
+	// checker is the function to check if r's stream contains the desired
+	// attribute.  It must return all the patterns for files which should
+	// also be checked and each of them should be valid for filepath.Glob
+	// function.
+	checker func(r io.Reader, desired string) (patterns []string, has bool, err error)
+	// initPath is the path of the first member in the sequence of checked
+	// files.
+	initPath string
+}
+
+// maxCheckedFileSize is the maximum length of the file that recurrentChecker
+// may check.
+const maxCheckedFileSize = 1024 * 1024
+
+// checkFile tries to open and to check single file located on the sourcePath.
+func (rc *recurrentChecker) checkFile(sourcePath, desired string) (
+	subsources []string,
+	has bool,
+	err error,
+) {
+	var f *os.File
+	f, err = os.Open(sourcePath)
+	if err != nil {
+		return nil, false, err
+	}
+
+	defer func() { err = errors.WithDeferred(err, f.Close()) }()
+
+	var r io.Reader
+	r, err = aghio.LimitReader(f, maxCheckedFileSize)
+	if err != nil {
+		return nil, false, err
+	}
+
+	subsources, has, err = rc.checker(r, desired)
+	if err != nil {
+		return nil, false, err
+	}
+
+	if has {
+		return nil, true, nil
+	}
+
+	return subsources, has, nil
+}
+
+// handlePatterns parses the patterns and takes care of duplicates.
+func (rc *recurrentChecker) handlePatterns(sourcesSet *aghstrings.Set, patterns []string) (
+	subsources []string,
+	err error,
+) {
+	subsources = make([]string, 0, len(patterns))
+	for _, p := range patterns {
+		var matches []string
+		matches, err = filepath.Glob(p)
+		if err != nil {
+			return nil, fmt.Errorf("invalid pattern %q: %w", p, err)
+		}
+
+		for _, m := range matches {
+			if sourcesSet.Has(m) {
+				continue
+			}
+
+			sourcesSet.Add(m)
+			subsources = append(subsources, m)
+		}
+	}
+
+	return subsources, nil
+}
+
+// check walks through all the files searching for the desired attribute.
+func (rc *recurrentChecker) check(desired string) (has bool, err error) {
+	var i int
+	sources := []string{rc.initPath}
+
+	defer func() {
+		if i >= len(sources) {
+			return
+		}
+
+		err = errors.Annotate(err, "checking %q: %w", sources[i])
+	}()
+
+	var patterns, subsources []string
+	// The slice of sources is separate from the set of sources to keep the
+	// order in which the files are walked.
+	for sourcesSet := aghstrings.NewSet(rc.initPath); i < len(sources); i++ {
+		patterns, has, err = rc.checkFile(sources[i], desired)
+		if err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				continue
+			}
+
+			return false, err
+		}
+
+		if has {
+			return true, nil
+		}
+
+		subsources, err = rc.handlePatterns(sourcesSet, patterns)
+		if err != nil {
+			return false, err
+		}
+
+		sources = append(sources, subsources...)
+	}
+
+	return false, nil
+}
 
 func ifaceHasStaticIP(ifaceName string) (has bool, err error) {
 	// TODO(a.garipov): Currently, this function returns the first
@@ -26,48 +142,34 @@ func ifaceHasStaticIP(ifaceName string) (has bool, err error) {
 	// /etc/network/interfaces doesn't, it will return true.  Perhaps this
 	// is not the most desirable behavior.
 
-	for _, check := range []struct {
+	for _, rc := range []*recurrentChecker{{
-		checker  func(io.Reader, string) (bool, error)
-		filePath string
-	}{{
 		checker:  dhcpcdStaticConfig,
-		filePath: "/etc/dhcpcd.conf",
+		initPath: "/etc/dhcpcd.conf",
 	}, {
 		checker:  ifacesStaticConfig,
-		filePath: "/etc/network/interfaces",
+		initPath: "/etc/network/interfaces",
 	}} {
-		var f *os.File
+		has, err = rc.check(ifaceName)
-		f, err = os.Open(check.filePath)
-		if err != nil {
-			// ErrNotExist can happen here if there is no such file.
-			// This is normal, as not every system uses those files.
-			if errors.Is(err, os.ErrNotExist) {
-				err = nil
-
-				continue
-			}
-
-			return false, err
-		}
-		defer func() { err = errors.WithDeferred(err, f.Close()) }()
-
-		var fileReader io.Reader
-		fileReader, err = aghio.LimitReader(f, maxConfigFileSize)
 		if err != nil {
 			return false, err
 		}
 
-		has, err = check.checker(fileReader, ifaceName)
+		if has {
-		if err != nil {
+			return true, nil
-			return false, err
 		}
-
-		return has, nil
 	}
 
 	return false, ErrNoStaticIPInfo
 }
 
+func canBindPrivilegedPorts() (can bool, err error) {
+	cnbs, err := unix.PrctlRetInt(unix.PR_CAP_AMBIENT, unix.PR_CAP_AMBIENT_IS_SET, unix.CAP_NET_BIND_SERVICE, 0, 0)
+	// Don't check the error because it's always nil on Linux.
+	adm, _ := aghos.HaveAdminRights()
+
+	return cnbs == 1 || adm, err
+}
+
 // findIfaceLine scans s until it finds the line that declares an interface with
 // the given name.  If findIfaceLine can't find the line, it returns false.
 func findIfaceLine(s *bufio.Scanner, name string) (ok bool) {
@@ -84,11 +186,11 @@ func findIfaceLine(s *bufio.Scanner, name string) (ok bool) {
 
 // dhcpcdStaticConfig checks if interface is configured by /etc/dhcpcd.conf to
 // have a static IP.
-func dhcpcdStaticConfig(r io.Reader, ifaceName string) (has bool, err error) {
+func dhcpcdStaticConfig(r io.Reader, ifaceName string) (subsources []string, has bool, err error) {
 	s := bufio.NewScanner(r)
 	ifaceFound := findIfaceLine(s, ifaceName)
 	if !ifaceFound {
-		return false, s.Err()
+		return nil, false, s.Err()
 	}
 
 	for s.Scan() {
@@ -97,7 +199,7 @@ func dhcpcdStaticConfig(r io.Reader, ifaceName string) (has bool, err error) {
 		if len(fields) >= 2 &&
 			fields[0] == "static" &&
 			strings.HasPrefix(fields[1], "ip_address=") {
-			return true, s.Err()
+			return nil, true, s.Err()
 		}
 
 		if len(fields) > 0 && fields[0] == "interface" {
@@ -106,29 +208,41 @@ func dhcpcdStaticConfig(r io.Reader, ifaceName string) (has bool, err error) {
 		}
 	}
 
-	return false, s.Err()
+	return nil, false, s.Err()
 }
 
-// ifacesStaticConfig checks if interface is configured by
+// ifacesStaticConfig checks if the interface is configured by any file of
-// /etc/network/interfaces to have a static IP.
+// /etc/network/interfaces format to have a static IP.
-func ifacesStaticConfig(r io.Reader, ifaceName string) (has bool, err error) {
+func ifacesStaticConfig(r io.Reader, ifaceName string) (subsources []string, has bool, err error) {
 	s := bufio.NewScanner(r)
 	for s.Scan() {
 		line := strings.TrimSpace(s.Text())
-
+		if aghstrings.IsCommentOrEmpty(line) {
-		if len(line) == 0 || line[0] == '#' {
 			continue
 		}
 
+		// TODO(e.burkov): As man page interfaces(5) says, a line may be
+		// extended across multiple lines by making the last character a
+		// backslash.  Provide extended lines and "source-directory"
+		// stanzas support.
+
 		fields := strings.Fields(line)
+		fieldsNum := len(fields)
+
 		// Man page interfaces(5) declares that interface definition
 		// should consist of the key word "iface" followed by interface
 		// name, and method at fourth field.
-		if len(fields) >= 4 && fields[0] == "iface" && fields[1] == ifaceName && fields[3] == "static" {
+		if fieldsNum >= 4 &&
-			return true, nil
+			fields[0] == "iface" && fields[1] == ifaceName && fields[3] == "static" {
+			return nil, true, nil
+		}
+
+		if fieldsNum >= 2 && fields[0] == "source" {
+			subsources = append(subsources, fields[1])
 		}
 	}
-	return false, s.Err()
+
+	return subsources, false, s.Err()
 }
 
 // ifaceSetStaticIP configures the system to retain its current IP on the

internal/aghnet/net_linux_test.go

@@ -12,6 +12,21 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestRecurrentChecker(t *testing.T) {
+	c := &recurrentChecker{
+		checker:  ifacesStaticConfig,
+		initPath: "./testdata/include-subsources",
+	}
+
+	has, err := c.check("sample_name")
+	require.NoError(t, err)
+	assert.True(t, has)
+
+	has, err = c.check("another_name")
+	require.NoError(t, err)
+	assert.False(t, has)
+}
+
 const nl = "\n"
 
 func TestDHCPCDStaticConfig(t *testing.T) {
@@ -49,7 +64,7 @@ func TestDHCPCDStaticConfig(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			r := bytes.NewReader(tc.data)
-			has, err := dhcpcdStaticConfig(r, "wlan0")
+			_, has, err := dhcpcdStaticConfig(r, "wlan0")
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.want, has)
@@ -62,6 +77,7 @@ func TestIfacesStaticConfig(t *testing.T) {
 		name         string
 		data         []byte
 		want         bool
+		wantPatterns []string
 	}{{
 		name: "has_not",
 		data: []byte(`allow-hotplug enp0s3` + nl +
@@ -72,6 +88,7 @@ func TestIfacesStaticConfig(t *testing.T) {
 			`iface enp0s3 inet dhcp` + nl,
 		),
 		want:         false,
+		wantPatterns: []string{},
 	}, {
 		name: "has",
 		data: []byte(`allow-hotplug enp0s3` + nl +
@@ -82,15 +99,35 @@ func TestIfacesStaticConfig(t *testing.T) {
 			`#iface enp0s3 inet dhcp` + nl,
 		),
 		want:         true,
+		wantPatterns: []string{},
+	}, {
+		name: "return_patterns",
+		data: []byte(`source hello` + nl +
+			`source world` + nl +
+			`#iface enp0s3 inet static` + nl,
+		),
+		want:         false,
+		wantPatterns: []string{"hello", "world"},
+	}, {
+		// This one tests if the first found valid interface prevents
+		// checking files under the `source` directive.
+		name: "ignore_patterns",
+		data: []byte(`source hello` + nl +
+			`source world` + nl +
+			`iface enp0s3 inet static` + nl,
+		),
+		want:         true,
+		wantPatterns: []string{},
 	}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			r := bytes.NewReader(tc.data)
-			has, err := ifacesStaticConfig(r, "enp0s3")
+			patterns, has, err := ifacesStaticConfig(r, "enp0s3")
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.want, has)
+			assert.ElementsMatch(t, tc.wantPatterns, patterns)
 		})
 	}
 }

internal/aghnet/net_others.go

@@ -6,8 +6,14 @@ package aghnet
 import (
 	"fmt"
 	"runtime"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 )
 
+func canBindPrivilegedPorts() (can bool, err error) {
+	return aghos.HaveAdminRights()
+}
+
 func ifaceHasStaticIP(string) (bool, error) {
 	return false, fmt.Errorf("cannot check if IP is static: not supported on %s", runtime.GOOS)
 }

internal/aghnet/testdata/ifaces

@@ -0,0 +1 @@
+iface sample_name inet static

internal/aghnet/testdata/include-subsources

@@ -0,0 +1,5 @@
+# The "testdata" part is added here because the test is actually run from the
+# parent directory.  Real interface files usually contain only absolute paths.
+
+source ./testdata/ifaces
+source ./testdata/*

internal/aghos/os.go

@@ -31,12 +31,6 @@ func Unsupported(op string) (err error) {
 	}
 }
 
-// CanBindPrivilegedPorts checks if current process can bind to privileged
-// ports.
-func CanBindPrivilegedPorts() (can bool, err error) {
-	return canBindPrivilegedPorts()
-}
-
 // SetRlimit sets user-specified limit of how many fd's we can use.
 //
 // See https://github.com/AdguardTeam/AdGuardHome/internal/issues/659.

internal/aghos/os_bsd.go

@@ -8,10 +8,6 @@ import (
 	"syscall"
 )
 
-func canBindPrivilegedPorts() (can bool, err error) {
-	return HaveAdminRights()
-}
-
 func setRlimit(val uint64) (err error) {
 	var rlim syscall.Rlimit
 	rlim.Max = val

internal/aghos/os_freebsd.go

@@ -8,10 +8,6 @@ import (
 	"syscall"
 )
 
-func canBindPrivilegedPorts() (can bool, err error) {
-	return HaveAdminRights()
-}
-
 func setRlimit(val uint64) (err error) {
 	var rlim syscall.Rlimit
 	rlim.Max = int64(val)

internal/aghos/os_linux.go

@@ -9,18 +9,8 @@ import (
 	"path/filepath"
 	"strings"
 	"syscall"
-
-	"golang.org/x/sys/unix"
 )
 
-func canBindPrivilegedPorts() (can bool, err error) {
-	cnbs, err := unix.PrctlRetInt(unix.PR_CAP_AMBIENT, unix.PR_CAP_AMBIENT_IS_SET, unix.CAP_NET_BIND_SERVICE, 0, 0)
-	// Don't check the error because it's always nil on Linux.
-	adm, _ := haveAdminRights()
-
-	return cnbs == 1 || adm, err
-}
-
 func setRlimit(val uint64) (err error) {
 	var rlim syscall.Rlimit
 	rlim.Max = val

internal/aghos/os_windows.go

@@ -9,10 +9,6 @@ import (
 	"golang.org/x/sys/windows"
 )
 
-func canBindPrivilegedPorts() (can bool, err error) {
-	return HaveAdminRights()
-}
-
 func setRlimit(val uint64) (err error) {
 	return Unsupported("setrlimit")
 }

internal/home/controlupdate.go

@@ -11,7 +11,7 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
+	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/AdGuardHome/internal/updater"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
@@ -141,7 +141,7 @@ func (vr *versionResponse) confirmAutoUpdate() {
 				tlsConf.PortDNSOverQUIC < 1024)) ||
 				config.BindPort < 1024 ||
 				config.DNS.Port < 1024) {
-			canUpdate, _ = aghos.CanBindPrivilegedPorts()
+			canUpdate, _ = aghnet.CanBindPrivilegedPorts()
 		}
 		vr.CanAutoUpdate = &canUpdate
 	}