net/dns/resolvconffile: unify three /etc/resolv.conf parsers into new package

Change-Id: I2120893ca802d12f1bd0407d49077d3672627d33
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/depaware.txt

@@ -193,6 +193,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/logtail/filch                                  from tailscale.com/logpolicy
      💣 tailscale.com/metrics                                        from tailscale.com/derp+
         tailscale.com/net/dns                                        from tailscale.com/cmd/tailscaled+
+        tailscale.com/net/dns/resolvconffile                         from tailscale.com/net/dns+
         tailscale.com/net/dns/resolver                               from tailscale.com/net/dns+
         tailscale.com/net/dnscache                                   from tailscale.com/control/controlclient+
         tailscale.com/net/dnsfallback                                from tailscale.com/control/controlclient+

net/dns/direct.go

@@ -5,7 +5,6 @@
 package dns
 
 import (
-	"bufio"
 	"bytes"
 	"context"
 	"crypto/rand"
@@ -20,6 +19,7 @@ import (
 	"time"
 
 	"inet.af/netaddr"
+	"tailscale.com/net/dns/resolvconffile"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/dnsname"
 )
@@ -30,64 +30,23 @@ const (
 )
 
 // writeResolvConf writes DNS configuration in resolv.conf format to the given writer.
-func writeResolvConf(w io.Writer, servers []netaddr.IP, domains []dnsname.FQDN) {
+func writeResolvConf(w io.Writer, servers []netaddr.IP, domains []dnsname.FQDN) error {
-	io.WriteString(w, "# resolv.conf(5) file generated by tailscale\n")
+	c := &resolvconffile.Config{
-	io.WriteString(w, "# DO NOT EDIT THIS FILE BY HAND -- CHANGES WILL BE OVERWRITTEN\n\n")
+		Nameservers:   servers,
-	for _, ns := range servers {
+		SearchDomains: domains,
-		io.WriteString(w, "nameserver ")
-		io.WriteString(w, ns.String())
-		io.WriteString(w, "\n")
-	}
-	if len(domains) > 0 {
-		io.WriteString(w, "search")
-		for _, domain := range domains {
-			io.WriteString(w, " ")
-			io.WriteString(w, domain.WithoutTrailingDot())
-		}
-		io.WriteString(w, "\n")
 	}
+	return c.Write(w)
 }
 
-func readResolv(r io.Reader) (config OSConfig, err error) {
+func readResolv(r io.Reader) (OSConfig, error) {
-	scanner := bufio.NewScanner(r)
+	c, err := resolvconffile.Parse(r)
-	for scanner.Scan() {
+	if err != nil {
-		line := strings.TrimSpace(scanner.Text())
+		return OSConfig{}, err
-		i := strings.IndexByte(line, '#')
-		if i >= 0 {
-			line = line[:i]
-		}
-
-		if strings.HasPrefix(line, "nameserver") {
-			s := strings.TrimPrefix(line, "nameserver")
-			nameserver := strings.TrimSpace(s)
-			if len(nameserver) == len(s) {
-				return OSConfig{}, fmt.Errorf("missing space after \"nameserver\" in %q", line)
-			}
-			ip, err := netaddr.ParseIP(nameserver)
-			if err != nil {
-				return OSConfig{}, err
-			}
-			config.Nameservers = append(config.Nameservers, ip)
-			continue
-		}
-
-		if strings.HasPrefix(line, "search") {
-			s := strings.TrimPrefix(line, "search")
-			domain := strings.TrimSpace(s)
-			if len(domain) == len(s) {
-				// No leading space?!
-				return OSConfig{}, fmt.Errorf("missing space after \"domain\" in %q", line)
-			}
-			fqdn, err := dnsname.ToFQDN(domain)
-			if err != nil {
-				return OSConfig{}, fmt.Errorf("parsing search domains %q: %w", line, err)
-			}
-			config.SearchDomains = append(config.SearchDomains, fqdn)
-			continue
-		}
 	}
-
+	return OSConfig{
-	return config, nil
+		Nameservers:   c.Nameservers,
+		SearchDomains: c.SearchDomains,
+	}, nil
 }
 
 // resolvOwner returns the apparent owner of the resolv.conf

net/dns/resolvconffile/resolvconffile.go

@@ -0,0 +1,120 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package resolvconffile parses & serializes /etc/resolv.conf-style files.
+//
+// It's a leaf package so both net/dns and net/dns/resolver can depend
+// on it and we can unify a handful of implementations.
+//
+// The package is verbosely named to disambiguate it from resolvconf
+// the daemon, which Tailscale also supports.
+package resolvconffile
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"inet.af/netaddr"
+	"tailscale.com/util/dnsname"
+)
+
+// Path is the canonical location of resolv.conf.
+const Path = "/etc/resolv.conf"
+
+// Config represents a resolv.conf(5) file.
+type Config struct {
+	// Nameservers are the IP addresses of the nameservers to use.
+	Nameservers []netaddr.IP
+
+	// SearchDomains are the domain suffixes to use when expanding
+	// single-label name queries. SearchDomains is additive to
+	// whatever non-Tailscale search domains the OS has.
+	SearchDomains []dnsname.FQDN
+}
+
+// Write writes c to w. It does so in one Write call.
+func (c *Config) Write(w io.Writer) error {
+	buf := new(bytes.Buffer)
+	io.WriteString(buf, "# resolv.conf(5) file generated by tailscale\n")
+	io.WriteString(buf, "# DO NOT EDIT THIS FILE BY HAND -- CHANGES WILL BE OVERWRITTEN\n\n")
+	for _, ns := range c.Nameservers {
+		io.WriteString(buf, "nameserver ")
+		io.WriteString(buf, ns.String())
+		io.WriteString(buf, "\n")
+	}
+	if len(c.SearchDomains) > 0 {
+		io.WriteString(buf, "search")
+		for _, domain := range c.SearchDomains {
+			io.WriteString(buf, " ")
+			io.WriteString(buf, domain.WithoutTrailingDot())
+		}
+		io.WriteString(buf, "\n")
+	}
+	_, err := w.Write(buf.Bytes())
+	return err
+}
+
+// Parse parses a resolv.conf file from r.
+func Parse(r io.Reader) (*Config, error) {
+	config := new(Config)
+	scanner := bufio.NewScanner(r)
+	for scanner.Scan() {
+		line := scanner.Text()
+		i := strings.IndexByte(line, '#')
+		if i >= 0 {
+			line = line[:i]
+		}
+		line = strings.TrimSpace(line)
+
+		if strings.HasPrefix(line, "nameserver") {
+			s := strings.TrimPrefix(line, "nameserver")
+			nameserver := strings.TrimSpace(s)
+			if len(nameserver) == len(s) {
+				return nil, fmt.Errorf("missing space after \"nameserver\" in %q", line)
+			}
+			ip, err := netaddr.ParseIP(nameserver)
+			if err != nil {
+				return nil, err
+			}
+			config.Nameservers = append(config.Nameservers, ip)
+			continue
+		}
+
+		if strings.HasPrefix(line, "search") {
+			s := strings.TrimPrefix(line, "search")
+			domain := strings.TrimSpace(s)
+			if len(domain) == len(s) {
+				// No leading space?!
+				return nil, fmt.Errorf("missing space after \"domain\" in %q", line)
+			}
+			fqdn, err := dnsname.ToFQDN(domain)
+			if err != nil {
+				return nil, fmt.Errorf("parsing search domains %q: %w", line, err)
+			}
+			config.SearchDomains = append(config.SearchDomains, fqdn)
+			continue
+		}
+	}
+	return config, nil
+}
+
+// ParseFile parses the named resolv.conf file.
+func ParseFile(name string) (*Config, error) {
+	fi, err := os.Stat(name)
+	if err != nil {
+		return nil, err
+	}
+	if n := fi.Size(); n > 10<<10 {
+		return nil, fmt.Errorf("unexpectedly large %q file: %d bytes", name, n)
+	}
+	all, err := os.ReadFile(name)
+	if err != nil {
+		return nil, err
+	}
+	return Parse(bytes.NewReader(all))
+}

net/dns/resolvconffile/resolvconffile_test.go

@@ -0,0 +1,79 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package resolvconffile
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"inet.af/netaddr"
+	"tailscale.com/util/dnsname"
+)
+
+func TestParse(t *testing.T) {
+	tests := []struct {
+		in      string
+		want    *Config
+		wantErr bool
+	}{
+		{in: `nameserver 192.168.0.100`,
+			want: &Config{
+				Nameservers: []netaddr.IP{
+					netaddr.MustParseIP("192.168.0.100"),
+				},
+			},
+		},
+		{in: `nameserver 192.168.0.100 # comment`,
+			want: &Config{
+				Nameservers: []netaddr.IP{
+					netaddr.MustParseIP("192.168.0.100"),
+				},
+			},
+		},
+		{in: `nameserver 192.168.0.100#`,
+			want: &Config{
+				Nameservers: []netaddr.IP{
+					netaddr.MustParseIP("192.168.0.100"),
+				},
+			},
+		},
+		{in: `nameserver #192.168.0.100`, wantErr: true},
+		{in: `nameserver`, wantErr: true},
+		{in: `# nameserver 192.168.0.100`, want: &Config{}},
+		{in: `nameserver192.168.0.100`, wantErr: true},
+
+		{in: `search tailsacle.com`,
+			want: &Config{
+				SearchDomains: []dnsname.FQDN{"tailsacle.com."},
+			},
+		},
+		{in: `search tailsacle.com # typo`,
+			want: &Config{
+				SearchDomains: []dnsname.FQDN{"tailsacle.com."},
+			},
+		},
+		{in: `searchtailsacle.com`, wantErr: true},
+		{in: `search`, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		cfg, err := Parse(strings.NewReader(tt.in))
+		if tt.wantErr {
+			if err != nil {
+				continue
+			}
+			t.Errorf("missing error for %q", tt.in)
+			continue
+		}
+		if err != nil {
+			t.Errorf("unexpected error for %q: %v", tt.in, err)
+			continue
+		}
+		if !reflect.DeepEqual(cfg, tt.want) {
+			t.Errorf("got: %v\nwant: %v\n", cfg, tt.want)
+		}
+	}
+}

net/dns/resolvd.go

@@ -8,17 +8,14 @@
 package dns
 
 import (
-	"bufio"
 	"bytes"
-	"fmt"
 	"os"
 	"os/exec"
 	"regexp"
 	"strings"
 
-	"inet.af/netaddr"
+	"tailscale.com/net/dns/resolvconffile"
 	"tailscale.com/types/logger"
-	"tailscale.com/util/dnsname"
 )
 
 func newResolvdManager(logf logger.Logf, interfaceName string) (*resolvdManager, error) {
@@ -117,44 +114,14 @@ func (m resolvdManager) readResolvConf() (config OSConfig, err error) {
 		return OSConfig{}, err
 	}
 
-	scanner := bufio.NewScanner(bytes.NewReader(b))
+	rconf, err := resolvconffile.Parse(bytes.NewReader(b))
-	for scanner.Scan() {
+	if err != nil {
-		line := strings.TrimSpace(scanner.Text())
+		return config, err
-
-		// resolvd manages "nameserver" lines, we only need to handle
-		// "search".
-		if strings.HasPrefix(line, "search") {
-			domain := strings.TrimPrefix(line, "search")
-			domain = strings.TrimSpace(domain)
-			fqdn, err := dnsname.ToFQDN(domain)
-			if err != nil {
-				return OSConfig{}, fmt.Errorf("parsing search domains %q: %w", line, err)
-			}
-			config.SearchDomains = append(config.SearchDomains, fqdn)
-			continue
-		}
-
-		if strings.HasPrefix(line, "nameserver") {
-			s := strings.TrimPrefix(line, "nameserver")
-			parts := strings.Split(s, " # ")
-			if len(parts) == 0 {
-				return OSConfig{}, err
-			}
-			nameserver := strings.TrimSpace(parts[0])
-			ip, err := netaddr.ParseIP(nameserver)
-			if err != nil {
-				return OSConfig{}, err
-			}
-			config.Nameservers = append(config.Nameservers, ip)
-			continue
-		}
 	}
-
+	return OSConfig{
-	if err = scanner.Err(); err != nil {
+		Nameservers:   rconf.Nameservers,
-		return OSConfig{}, err
+		SearchDomains: rconf.SearchDomains,
-	}
+	}, nil
-
-	return config, nil
 }
 
 func removeSearchLines(orig []byte) []byte {

net/dns/resolver/tsdns.go

@@ -8,7 +8,6 @@ package resolver
 
 import (
 	"bufio"
-	"bytes"
 	"context"
 	"encoding/hex"
 	"errors"
@@ -23,16 +22,15 @@ import (
 	"sync/atomic"
 	"time"
 
-	"go4.org/mem"
 	dns "golang.org/x/net/dns/dnsmessage"
 	"inet.af/netaddr"
+	"tailscale.com/net/dns/resolvconffile"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/net/tsdial"
 	"tailscale.com/types/dnstype"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/dnsname"
-	"tailscale.com/util/lineread"
 	"tailscale.com/wgengine/monitor"
 )
 
@@ -518,7 +516,7 @@ var errEmptyResolvConf = errors.New("resolv.conf has no nameservers")
 // stubResolverForOS returns the IP address of the first nameserver in
 // /etc/resolv.conf.
 func stubResolverForOS() (ip netaddr.IP, err error) {
-	fi, err := os.Stat("/etc/resolv.conf")
+	fi, err := os.Stat(resolvconffile.Path)
 	if err != nil {
 		return netaddr.IP{}, err
 	}
@@ -529,37 +527,14 @@ func stubResolverForOS() (ip netaddr.IP, err error) {
 	if c, ok := resolvConfCacheValue.Load().(resolvConfCache); ok && c.mod == cur.mod && c.size == cur.size {
 		return c.ip, nil
 	}
-	// TODO(bradfitz): unify this /etc/resolv.conf parsing code with readResolv
+	conf, err := resolvconffile.ParseFile(resolvconffile.Path)
-	// in net/dns, which we can't use due to circular dependency reasons.
-	// Move it to a leaf, including the OSConfig type (perhaps in its own dnstype
-	// package?)
-	err = lineread.File("/etc/resolv.conf", func(line []byte) error {
-		if !ip.IsZero() {
-			return nil
-		}
-		line = bytes.TrimSpace(line)
-		if len(line) == 0 || line[0] == '#' {
-			return nil
-		}
-		// Normalize tabs to spaces to simplify parsing code later.
-		for i, b := range line {
-			if b == '\t' {
-				line[i] = ' '
-			}
-		}
-		if mem.HasPrefix(mem.B(line), mem.S("nameserver ")) {
-			s := strings.TrimSpace(strings.TrimPrefix(string(line), "nameserver "))
-			ip, err = netaddr.ParseIP(s)
-			return err
-		}
-		return nil
-	})
 	if err != nil {
 		return netaddr.IP{}, err
 	}
-	if !ip.IsValid() {
+	if len(conf.Nameservers) == 0 {
 		return netaddr.IP{}, errEmptyResolvConf
 	}
+	ip = conf.Nameservers[0]
 	cur.ip = ip
 	resolvConfCacheValue.Store(cur)
 	return ip, nil