From 28f34ca399ab27d4780fab6b53a55af8fd7f408f Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Mon, 28 Jun 2021 17:02:45 +0300 Subject: [PATCH] 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 Date: Mon Jun 28 16:53:36 2021 +0300 Merge branch 'master' into 3257-ifaces-source commit 530a1a23a601c5575c8dc5f6f97cd84801cf911b Author: Eugene Burkov Date: Fri Jun 25 19:43:55 2021 +0300 aghnet: imp code, add docs commit 58de84821b93bcbb3df1834ba33fbad817201b1d Author: Eugene Burkov Date: Fri Jun 25 13:34:43 2021 +0300 aghnet: sup "source" directive commit c0901abd5212902295e8ee546fad652092fdb5a8 Author: Eugene Burkov Date: Thu Jun 24 16:46:03 2021 +0300 aghos: mv func to aghnet --- CHANGELOG.md | 1 + HACKING.md | 5 +- internal/aghnet/net.go | 6 + internal/aghnet/net_darwin.go | 4 + internal/aghnet/net_linux.go | 200 +++++++++++++++----- internal/aghnet/net_linux_test.go | 51 ++++- internal/aghnet/net_others.go | 6 + internal/aghnet/testdata/ifaces | 1 + internal/aghnet/testdata/include-subsources | 5 + internal/aghos/os.go | 6 - internal/aghos/os_bsd.go | 4 - internal/aghos/os_freebsd.go | 4 - internal/aghos/os_linux.go | 10 - internal/aghos/os_windows.go | 4 - internal/home/controlupdate.go | 4 +- 15 files changed, 229 insertions(+), 82 deletions(-) create mode 100644 internal/aghnet/testdata/ifaces create mode 100644 internal/aghnet/testdata/include-subsources diff --git a/CHANGELOG.md b/CHANGELOG.md index dd838bc6..f49e0e22 100644 --- a/CHANGELOG.md +++ b/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]). diff --git a/HACKING.md b/HACKING.md index 4c2ee095..101a7757 100644 --- a/HACKING.md +++ b/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 - [can have surprising results][staticcheck-911]. + * Prefer to define methods on pointer receievers, unless the type is small or + 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: diff --git a/internal/aghnet/net.go b/internal/aghnet/net.go index 5a25dc35..ba3d053a 100644 --- a/internal/aghnet/net.go +++ b/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"` diff --git a/internal/aghnet/net_darwin.go b/internal/aghnet/net_darwin.go index 1eb9ce29..4da5d56c 100644 --- a/internal/aghnet/net_darwin.go +++ b/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 { diff --git a/internal/aghnet/net_linux.go b/internal/aghnet/net_linux.go index 63e79148..bf0c65d0 100644 --- a/internal/aghnet/net_linux.go +++ b/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 -// configuration file. -const maxConfigFileSize = 1024 * 1024 +// recurrentChecker is used to check all the files which may include references +// for other ones. +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 { - checker func(io.Reader, string) (bool, error) - filePath string - }{{ + for _, rc := range []*recurrentChecker{{ checker: dhcpcdStaticConfig, - filePath: "/etc/dhcpcd.conf", + initPath: "/etc/dhcpcd.conf", }, { checker: ifacesStaticConfig, - filePath: "/etc/network/interfaces", + initPath: "/etc/network/interfaces", }} { - var f *os.File - 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) + has, err = rc.check(ifaceName) if err != nil { return false, err } - has, err = check.checker(fileReader, ifaceName) - if err != nil { - return false, err + if has { + return true, nil } - - 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 -// /etc/network/interfaces to have a static IP. -func ifacesStaticConfig(r io.Reader, ifaceName string) (has bool, err error) { +// ifacesStaticConfig checks if the interface is configured by any file of +// /etc/network/interfaces format to have a static IP. +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 len(line) == 0 || line[0] == '#' { + if aghstrings.IsCommentOrEmpty(line) { 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" { - return true, nil + if fieldsNum >= 4 && + 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 diff --git a/internal/aghnet/net_linux_test.go b/internal/aghnet/net_linux_test.go index 1bc5e180..9dd9a866 100644 --- a/internal/aghnet/net_linux_test.go +++ b/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) @@ -59,9 +74,10 @@ func TestDHCPCDStaticConfig(t *testing.T) { func TestIfacesStaticConfig(t *testing.T) { testCases := []struct { - name string - data []byte - want bool + name string + data []byte + want bool + wantPatterns []string }{{ name: "has_not", data: []byte(`allow-hotplug enp0s3` + nl + @@ -71,7 +87,8 @@ func TestIfacesStaticConfig(t *testing.T) { `# gateway 192.168.0.1` + nl + `iface enp0s3 inet dhcp` + nl, ), - want: false, + want: false, + wantPatterns: []string{}, }, { name: "has", data: []byte(`allow-hotplug enp0s3` + nl + @@ -81,16 +98,36 @@ func TestIfacesStaticConfig(t *testing.T) { ` gateway 192.168.0.1` + nl + `#iface enp0s3 inet dhcp` + nl, ), - want: true, + 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) }) } } diff --git a/internal/aghnet/net_others.go b/internal/aghnet/net_others.go index 7ee8e964..07add540 100644 --- a/internal/aghnet/net_others.go +++ b/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) } diff --git a/internal/aghnet/testdata/ifaces b/internal/aghnet/testdata/ifaces new file mode 100644 index 00000000..2bd48802 --- /dev/null +++ b/internal/aghnet/testdata/ifaces @@ -0,0 +1 @@ +iface sample_name inet static \ No newline at end of file diff --git a/internal/aghnet/testdata/include-subsources b/internal/aghnet/testdata/include-subsources new file mode 100644 index 00000000..1663e423 --- /dev/null +++ b/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/* \ No newline at end of file diff --git a/internal/aghos/os.go b/internal/aghos/os.go index e3e2fb5e..2ecd87b2 100644 --- a/internal/aghos/os.go +++ b/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. diff --git a/internal/aghos/os_bsd.go b/internal/aghos/os_bsd.go index 8e726737..d8557b43 100644 --- a/internal/aghos/os_bsd.go +++ b/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 diff --git a/internal/aghos/os_freebsd.go b/internal/aghos/os_freebsd.go index d1f4083c..b3ec7e77 100644 --- a/internal/aghos/os_freebsd.go +++ b/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) diff --git a/internal/aghos/os_linux.go b/internal/aghos/os_linux.go index d185daf3..f61a6874 100644 --- a/internal/aghos/os_linux.go +++ b/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 diff --git a/internal/aghos/os_windows.go b/internal/aghos/os_windows.go index aa95d9a7..8972e7b5 100644 --- a/internal/aghos/os_windows.go +++ b/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") } diff --git a/internal/home/controlupdate.go b/internal/home/controlupdate.go index 74c5bfbc..5217190e 100644 --- a/internal/home/controlupdate.go +++ b/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 }