VERSION.txt: this is v1.24.0

Signed-off-by: Denton Gentry <dgentry@tailscale.com>

VERSION.txt

@@ -1 +1 @@
-1.23.0
+1.24.0

cmd/tailscale/cli/ssh.go

@@ -18,12 +18,12 @@ import (
 	"strings"
 	"syscall"
 
-	"github.com/alessio/shellescape"
 	"github.com/peterbourgon/ff/v3/ffcli"
 	"inet.af/netaddr"
 	"tailscale.com/client/tailscale"
 	"tailscale.com/envknob"
 	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/version"
 )
 
 var sshCmd = &ffcli.Command{
@@ -80,15 +80,13 @@ func runSSH(ctx context.Context, args []string) error {
 		ssh,
 
 		// Only trust SSH hosts that we know about.
-		"-o", fmt.Sprintf("UserKnownHostsFile %s",
-			shellescape.Quote(knownHostsFile),
-		),
+		"-o", fmt.Sprintf("UserKnownHostsFile %q", knownHostsFile),
 		"-o", "UpdateHostKeys no",
 		"-o", "StrictHostKeyChecking yes",
 
-		"-o", fmt.Sprintf("ProxyCommand %s --socket=%s nc %%h %%p",
-			shellescape.Quote(tailscaleBin),
-			shellescape.Quote(rootArgs.socket),
+		"-o", fmt.Sprintf("ProxyCommand %q --socket=%q nc %%h %%p",
+			tailscaleBin,
+			rootArgs.socket,
 		),
 
 		// Explicitly rebuild the user@host argument rather than
@@ -104,8 +102,8 @@ func runSSH(ctx context.Context, args []string) error {
 		username + "@" + hostForSSH,
 	}, argRest...)
 
-	if runtime.GOOS == "windows" {
-		// Don't use syscall.Exec on Windows.
+	if runtime.GOOS == "windows" || version.IsSandboxedMacOS() {
+		// Don't use syscall.Exec on Windows or in the macOS sandbox.
 		cmd := exec.Command(ssh, argv[1:]...)
 		cmd.Stderr = os.Stderr
 		cmd.Stdout = os.Stdout

cmd/tailscale/depaware.txt

@@ -1,6 +1,5 @@
 tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/depaware)
 
-        github.com/alessio/shellescape                               from tailscale.com/cmd/tailscale/cli
    W 💣 github.com/alexbrainman/sspi                                 from github.com/alexbrainman/sspi/negotiate+
    W    github.com/alexbrainman/sspi/internal/common                 from github.com/alexbrainman/sspi/negotiate
    W 💣 github.com/alexbrainman/sspi/negotiate                       from tailscale.com/net/tshttpproxy

cmd/tailscaled/depaware.txt

@@ -82,6 +82,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         github.com/klauspost/compress/internal/snapref               from github.com/klauspost/compress/zstd
         github.com/klauspost/compress/zstd                           from tailscale.com/smallzstd
         github.com/klauspost/compress/zstd/internal/xxhash           from github.com/klauspost/compress/zstd
+  LD    github.com/kr/fs                                             from github.com/pkg/sftp
    L    github.com/mdlayher/genetlink                                from tailscale.com/net/tstun
    L 💣 github.com/mdlayher/netlink                                  from github.com/jsimonetti/rtnetlink+
    L 💣 github.com/mdlayher/netlink/nlenc                            from github.com/jsimonetti/rtnetlink+
@@ -89,6 +90,8 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
    L 💣 github.com/mdlayher/socket                                   from github.com/mdlayher/netlink
      💣 github.com/mitchellh/go-ps                                   from tailscale.com/safesocket
    W    github.com/pkg/errors                                        from github.com/tailscale/certstore
+  LD    github.com/pkg/sftp                                          from tailscale.com/ssh/tailssh
+  LD    github.com/pkg/sftp/internal/encoding/ssh/filexfer           from github.com/pkg/sftp
    W 💣 github.com/tailscale/certstore                               from tailscale.com/control/controlclient
   LD    github.com/tailscale/golang-x-crypto/chacha20                from github.com/tailscale/golang-x-crypto/ssh
   LD 💣 github.com/tailscale/golang-x-crypto/internal/subtle         from github.com/tailscale/golang-x-crypto/chacha20
@@ -261,6 +264,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
   LW    tailscale.com/util/endian                                    from tailscale.com/net/dns+
         tailscale.com/util/groupmember                               from tailscale.com/ipn/ipnserver
         tailscale.com/util/lineread                                  from tailscale.com/hostinfo+
+        tailscale.com/util/mak                                       from tailscale.com/control/controlclient+
         tailscale.com/util/multierr                                  from tailscale.com/cmd/tailscaled+
         tailscale.com/util/netconv                                   from tailscale.com/wgengine/magicsock
         tailscale.com/util/osshare                                   from tailscale.com/cmd/tailscaled+
@@ -298,7 +302,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         golang.org/x/crypto/nacl/secretbox                           from golang.org/x/crypto/nacl/box
         golang.org/x/crypto/poly1305                                 from golang.zx2c4.com/wireguard/device+
         golang.org/x/crypto/salsa20/salsa                            from golang.org/x/crypto/nacl/box+
-  LD    golang.org/x/crypto/ssh                                      from tailscale.com/ssh/tailssh
+  LD    golang.org/x/crypto/ssh                                      from tailscale.com/ssh/tailssh+
         golang.org/x/net/bpf                                         from github.com/mdlayher/genetlink+
         golang.org/x/net/dns/dnsmessage                              from net+
         golang.org/x/net/http/httpguts                               from golang.org/x/net/http2+

control/controlclient/noise.go

@@ -20,6 +20,7 @@ import (
 	"tailscale.com/control/controlhttp"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
+	"tailscale.com/util/mak"
 	"tailscale.com/util/multierr"
 )
 
@@ -137,9 +138,6 @@ func (nc *noiseClient) Close() error {
 func (nc *noiseClient) dial(_, _ string, _ *tls.Config) (net.Conn, error) {
 	nc.mu.Lock()
 	connID := nc.nextID
-	if nc.connPool == nil {
-		nc.connPool = make(map[int]*noiseConn)
-	}
 	nc.nextID++
 	nc.mu.Unlock()
 
@@ -161,6 +159,6 @@ func (nc *noiseClient) dial(_, _ string, _ *tls.Config) (net.Conn, error) {
 	nc.mu.Lock()
 	defer nc.mu.Unlock()
 	ncc := &noiseConn{Conn: conn, id: connID, pool: nc}
-	nc.connPool[ncc.id] = ncc
+	mak.Set(&nc.connPool, ncc.id, ncc)
 	return ncc, nil
 }

go.mod

@@ -5,7 +5,6 @@ go 1.18
 require (
 	filippo.io/mkcert v1.4.3
 	github.com/akutz/memconn v0.1.0
-	github.com/alessio/shellescape v1.4.1
 	github.com/alexbrainman/sspi v0.0.0-20210105120005-909beea2cc74
 	github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be
 	github.com/aws/aws-sdk-go-v2 v1.11.2

go.sum

@@ -104,8 +104,6 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
-github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0=
-github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
 github.com/alexbrainman/sspi v0.0.0-20210105120005-909beea2cc74 h1:Kk6a4nehpJ3UuJRqlA3JxYxBZEqCeOmATOvrbT4p9RA=
 github.com/alexbrainman/sspi v0.0.0-20210105120005-909beea2cc74/go.mod h1:cEWa1LVoE5KvSD9ONXsZrj0z6KqySlCCNKHlLzbqAt4=
 github.com/alexkohler/prealloc v1.0.0 h1:Hbq0/3fJPQhNkN0dR95AVrr6R7tou91y0uHG5pOcUuw=

ipn/store/stores.go

@@ -21,6 +21,7 @@ import (
 	"tailscale.com/ipn/store/mem"
 	"tailscale.com/paths"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/mak"
 )
 
 // Provider returns a StateStore for the provided path.
@@ -82,10 +83,7 @@ func Register(prefix string, fn Provider) {
 	if _, ok := knownStores[prefix]; ok {
 		panic(fmt.Sprintf("%q already registered", prefix))
 	}
-	if knownStores == nil {
-		knownStores = make(map[string]Provider)
-	}
-	knownStores[prefix] = fn
+	mak.Set(&knownStores, prefix, fn)
 }
 
 // TryWindowsAppDataMigration attempts to copy the Windows state file

net/dns/resolver/tsdns.go

@@ -17,6 +17,7 @@ import (
 	"os"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -633,6 +634,10 @@ func (r *Resolver) resolveLocal(domain dnsname.FQDN, typ dns.Type) (netaddr.IP,
 			return tsaddr.TailscaleServiceIPv6(), dns.RCodeSuccess
 		}
 	}
+	// Special-case: 'via-<siteid>.<ipv4>' queries.
+	if ip, ok := r.parseViaDomain(domain, typ); ok {
+		return ip, dns.RCodeSuccess
+	}
 
 	r.mu.Lock()
 	hosts := r.hostToIP
@@ -708,6 +713,46 @@ func (r *Resolver) resolveLocal(domain dnsname.FQDN, typ dns.Type) (netaddr.IP,
 	}
 }
 
+// parseViaDomain synthesizes an IP address for quad-A DNS requests of
+// the form 'via-<X>.<IPv4-address>', where X is a decimal, or hex-encoded
+// number with a '0x' prefix.
+//
+// This exists as a convenient mapping into Tailscales 'Via Range'.
+func (r *Resolver) parseViaDomain(domain dnsname.FQDN, typ dns.Type) (netaddr.IP, bool) {
+	fqdn := string(domain.WithoutTrailingDot())
+	if typ != dns.TypeAAAA {
+		return netaddr.IP{}, false
+	}
+	if len(fqdn) < len("via-X.0.0.0.0") {
+		return netaddr.IP{}, false // too short to be valid
+	}
+	if !strings.HasPrefix(fqdn, "via-") {
+		return netaddr.IP{}, false
+	}
+
+	firstDot := strings.Index(fqdn, ".")
+	if firstDot < 0 {
+		return netaddr.IP{}, false // missing dot delimiters
+	}
+
+	siteID := fqdn[len("via-"):firstDot]
+	ip4Str := fqdn[firstDot+1:]
+
+	ip4, err := netaddr.ParseIP(ip4Str)
+	if err != nil {
+		return netaddr.IP{}, false // badly formed, dont respond
+	}
+
+	prefix, err := strconv.ParseUint(siteID, 0, 32)
+	if err != nil {
+		return netaddr.IP{}, false // badly formed, dont respond
+	}
+
+	// MapVia will never error when given an ipv4 netaddr.IPPrefix.
+	out, _ := tsaddr.MapVia(uint32(prefix), netaddr.IPPrefixFrom(ip4, ip4.BitLen()))
+	return out.IP(), true
+}
+
 // resolveReverse returns the unique domain name that maps to the given address.
 func (r *Resolver) resolveLocalReverse(name dnsname.FQDN) (dnsname.FQDN, dns.RCode) {
 	var ip netaddr.IP

net/dns/resolver/tsdns_test.go

@@ -348,6 +348,9 @@ func TestResolveLocal(t *testing.T) {
 		{"ns-nxdomain", "test3.ipn.dev.", dns.TypeNS, netaddr.IP{}, dns.RCodeNameError},
 		{"onion-domain", "footest.onion.", dns.TypeA, netaddr.IP{}, dns.RCodeNameError},
 		{"magicdns", dnsSymbolicFQDN, dns.TypeA, netaddr.MustParseIP("100.100.100.100"), dns.RCodeSuccess},
+		{"via_hex", dnsname.FQDN("via-0xff.1.2.3.4."), dns.TypeAAAA, netaddr.MustParseIP("fd7a:115c:a1e0:b1a:0:ff:102:304"), dns.RCodeSuccess},
+		{"via_dec", dnsname.FQDN("via-1.10.0.0.1."), dns.TypeAAAA, netaddr.MustParseIP("fd7a:115c:a1e0:b1a:0:1:a00:1"), dns.RCodeSuccess},
+		{"via_invalid", dnsname.FQDN("via-."), dns.TypeA, netaddr.IP{}, dns.RCodeRefused},
 	}
 
 	for _, tt := range tests {

net/tshttpproxy/tshttpproxy_linux.go

@@ -0,0 +1,26 @@
+// 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.
+
+//go:build linux
+// +build linux
+
+package tshttpproxy
+
+import (
+	"net/http"
+	"net/url"
+
+	"tailscale.com/version/distro"
+)
+
+func init() {
+	sysProxyFromEnv = linuxSysProxyFromEnv
+}
+
+func linuxSysProxyFromEnv(req *http.Request) (*url.URL, error) {
+	if distro.Get() == distro.Synology {
+		return synologyProxyFromConfigCached(req)
+	}
+	return nil, nil
+}

net/tshttpproxy/tshttpproxy_synology.go

@@ -0,0 +1,132 @@
+// 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.
+
+//go:build linux
+// +build linux
+
+package tshttpproxy
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"net/url"
+	"os"
+	"strings"
+	"sync"
+	"time"
+
+	"tailscale.com/util/lineread"
+)
+
+// These vars are overridden for tests.
+var (
+	synologyProxyConfigPath = "/etc/proxy.conf"
+
+	openSynologyProxyConf = func() (io.ReadCloser, error) {
+		return os.Open(synologyProxyConfigPath)
+	}
+)
+
+var cache struct {
+	sync.Mutex
+	proxy   *url.URL
+	updated time.Time
+}
+
+func synologyProxyFromConfigCached(req *http.Request) (*url.URL, error) {
+	if req.URL == nil {
+		return nil, nil
+	}
+
+	cache.Lock()
+	defer cache.Unlock()
+
+	modtime := mtime(synologyProxyConfigPath)
+
+	if cache.updated == modtime {
+		return cache.proxy, nil
+	}
+
+	val, err := synologyProxyFromConfig(req)
+	cache.proxy = val
+
+	cache.updated = modtime
+
+	return val, err
+}
+
+func synologyProxyFromConfig(req *http.Request) (*url.URL, error) {
+	r, err := openSynologyProxyConf()
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, nil
+		}
+		return nil, err
+	}
+	defer r.Close()
+
+	return parseSynologyConfig(r)
+}
+
+func parseSynologyConfig(r io.Reader) (*url.URL, error) {
+	cfg := map[string]string{}
+
+	if err := lineread.Reader(r, func(line []byte) error {
+		// accept and skip over empty lines
+		line = bytes.TrimSpace(line)
+		if len(line) == 0 {
+			return nil
+		}
+
+		key, value, ok := strings.Cut(string(line), "=")
+		if !ok {
+			return fmt.Errorf("missing \"=\" in proxy.conf line: %q", line)
+		}
+		cfg[string(key)] = string(value)
+		return nil
+	}); err != nil {
+		return nil, err
+	}
+
+	if cfg["proxy_enabled"] != "yes" {
+		return nil, nil
+	}
+
+	proxyURL := new(url.URL)
+	if cfg["auth_enabled"] == "yes" {
+		proxyURL.User = url.UserPassword(cfg["proxy_user"], cfg["proxy_pwd"])
+	}
+
+	proxyURL.Scheme = "https"
+	host, port := cfg["https_host"], cfg["https_port"]
+	if host == "" {
+		proxyURL.Scheme = "http"
+		host, port = cfg["http_host"], cfg["http_port"]
+	}
+
+	if host == "" {
+		return nil, nil
+	}
+
+	if port != "" {
+		proxyURL.Host = net.JoinHostPort(host, port)
+	} else {
+		proxyURL.Host = host
+	}
+
+	return proxyURL, nil
+}
+
+// mtime stat's path and returns it's modification time. If path does not exist,
+// it returns the unix epoch.
+func mtime(path string) time.Time {
+	fi, err := os.Stat(path)
+	if err != nil {
+		return time.Unix(0, 0)
+	}
+	return fi.ModTime()
+}

net/tshttpproxy/tshttpproxy_synology_test.go

@@ -0,0 +1,288 @@
+// 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.
+
+//go:build linux
+// +build linux
+
+package tshttpproxy
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestSynologyProxyFromConfigCached(t *testing.T) {
+	req, err := http.NewRequest("GET", "https://example.org/", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var orig string
+	orig, synologyProxyConfigPath = synologyProxyConfigPath, filepath.Join(t.TempDir(), "proxy.conf")
+	defer func() { synologyProxyConfigPath = orig }()
+
+	t.Run("no config file", func(t *testing.T) {
+		if _, err := os.Stat(synologyProxyConfigPath); err == nil {
+			t.Fatalf("%s must not exist for this test", synologyProxyConfigPath)
+		}
+
+		cache.updated = time.Time{}
+		cache.proxy = nil
+
+		if val, err := synologyProxyFromConfigCached(req); val != nil || err != nil {
+			t.Fatalf("got %s, %v; want nil, nil", val, err)
+		}
+
+		if got, want := cache.updated, time.Unix(0, 0); got != want {
+			t.Fatalf("got %s, want %s", got, want)
+		}
+		if cache.proxy != nil {
+			t.Fatalf("got %s, want nil", cache.proxy)
+		}
+	})
+
+	t.Run("config file updated", func(t *testing.T) {
+		cache.updated = time.Now()
+		cache.proxy = nil
+
+		if err := ioutil.WriteFile(synologyProxyConfigPath, []byte(`
+proxy_enabled=yes
+http_host=10.0.0.55
+http_port=80
+		`), 0600); err != nil {
+			t.Fatal(err)
+		}
+
+		val, err := synologyProxyFromConfigCached(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if want := urlMustParse("http://10.0.0.55:80"); val.String() != want.String() {
+			t.Fatalf("got %s; want %s", val, want)
+		}
+	})
+
+	t.Run("config file removed", func(t *testing.T) {
+		cache.updated = time.Now()
+		cache.proxy = urlMustParse("http://127.0.0.1/")
+
+		if err := os.Remove(synologyProxyConfigPath); err != nil && !os.IsNotExist(err) {
+			t.Fatal(err)
+		}
+
+		val, err := synologyProxyFromConfigCached(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if val != nil {
+			t.Fatalf("got %s; want nil", val)
+		}
+		if cache.proxy != nil {
+			t.Fatalf("got %s, want nil", cache.proxy)
+		}
+	})
+}
+
+func TestSynologyProxyFromConfig(t *testing.T) {
+	var (
+		openReader io.ReadCloser
+		openErr    error
+	)
+	var origOpen func() (io.ReadCloser, error)
+	origOpen, openSynologyProxyConf = openSynologyProxyConf, func() (io.ReadCloser, error) {
+		return openReader, openErr
+	}
+	defer func() { openSynologyProxyConf = origOpen }()
+
+	req, err := http.NewRequest("GET", "https://example.com/", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Run("with config", func(t *testing.T) {
+		mc := &mustCloser{Reader: strings.NewReader(`
+proxy_user=foo
+proxy_pwd=bar
+proxy_enabled=yes
+adv_enabled=yes
+bypass_enabled=yes
+auth_enabled=yes
+https_host=10.0.0.66
+https_port=8443
+http_host=10.0.0.55
+http_port=80
+	`)}
+		defer mc.check(t)
+		openReader = mc
+
+		proxyURL, err := synologyProxyFromConfig(req)
+
+		if got, want := err, openErr; got != want {
+			t.Fatalf("got %s, want %s", got, want)
+		}
+
+		if got, want := proxyURL, urlMustParse("https://foo:bar@10.0.0.66:8443"); got.String() != want.String() {
+			t.Fatalf("got %s, want %s", got, want)
+		}
+
+	})
+
+	t.Run("non-existent config", func(t *testing.T) {
+		openReader = nil
+		openErr = os.ErrNotExist
+
+		proxyURL, err := synologyProxyFromConfig(req)
+		if err != nil {
+			t.Fatalf("expected no error, got %s", err)
+		}
+		if proxyURL != nil {
+			t.Fatalf("expected no url, got %s", proxyURL)
+		}
+	})
+
+	t.Run("error opening config", func(t *testing.T) {
+		openReader = nil
+		openErr = errors.New("example error")
+
+		proxyURL, err := synologyProxyFromConfig(req)
+		if err != openErr {
+			t.Fatalf("expected %s, got %s", openErr, err)
+		}
+		if proxyURL != nil {
+			t.Fatalf("expected no url, got %s", proxyURL)
+		}
+	})
+
+}
+
+func TestParseSynologyConfig(t *testing.T) {
+	cases := map[string]struct {
+		input string
+		url   *url.URL
+		err   error
+	}{
+		"populated": {
+			input: `
+proxy_user=foo
+proxy_pwd=bar
+proxy_enabled=yes
+adv_enabled=yes
+bypass_enabled=yes
+auth_enabled=yes
+https_host=10.0.0.66
+https_port=8443
+http_host=10.0.0.55
+http_port=80
+`,
+			url: urlMustParse("https://foo:bar@10.0.0.66:8443"),
+			err: nil,
+		},
+		"no-auth": {
+			input: `
+proxy_user=foo
+proxy_pwd=bar
+proxy_enabled=yes
+adv_enabled=yes
+bypass_enabled=yes
+auth_enabled=no
+https_host=10.0.0.66
+https_port=8443
+http_host=10.0.0.55
+http_port=80
+`,
+			url: urlMustParse("https://10.0.0.66:8443"),
+			err: nil,
+		},
+		"http": {
+			input: `
+proxy_user=foo
+proxy_pwd=bar
+proxy_enabled=yes
+adv_enabled=yes
+bypass_enabled=yes
+auth_enabled=yes
+https_host=
+https_port=8443
+http_host=10.0.0.55
+http_port=80
+`,
+			url: urlMustParse("http://foo:bar@10.0.0.55:80"),
+			err: nil,
+		},
+		"empty": {
+			input: `
+proxy_user=
+proxy_pwd=
+proxy_enabled=
+adv_enabled=
+bypass_enabled=
+auth_enabled=
+https_host=
+https_port=
+http_host=
+http_port=
+`,
+			url: nil,
+			err: nil,
+		},
+	}
+
+	for name, example := range cases {
+		t.Run(name, func(t *testing.T) {
+			url, err := parseSynologyConfig(strings.NewReader(example.input))
+			if err != example.err {
+				t.Fatal(err)
+			}
+			if example.err != nil {
+				return
+			}
+
+			if url == nil && example.url == nil {
+				return
+			}
+
+			if example.url == nil {
+				if url != nil {
+					t.Fatalf("got %s, want nil", url)
+				}
+			}
+
+			if got, want := example.url.String(), url.String(); got != want {
+				t.Fatalf("got %s, want %s", got, want)
+			}
+		})
+	}
+}
+func urlMustParse(u string) *url.URL {
+	r, err := url.Parse(u)
+	if err != nil {
+		panic(fmt.Sprintf("urlMustParse: %s", err))
+	}
+	return r
+}
+
+type mustCloser struct {
+	io.Reader
+	closed bool
+}
+
+func (m *mustCloser) Close() error {
+	m.closed = true
+	return nil
+}
+
+func (m *mustCloser) check(t *testing.T) {
+	if !m.closed {
+		t.Errorf("mustCloser wrapping %#v was not closed at time of check", m.Reader)
+	}
+}

shell.nix

@@ -14,8 +14,8 @@
 {
 	pkgs ? import <nixpkgs> {},
 	nixosUnstable ? import (fetchTarball https://github.com/NixOS/nixpkgs/archive/refs/heads/nixpkgs-unstable.tar.gz) { },
-	tailscale-go-rev ? "5ce3ec4d89c72f2a2b6f6f5089c950d7a6a33530",
-	tailscale-go-sha ? "sha256-KMOfzmikh30vEkViEkWUsOHczUifSTiRL6rhKQpHCRI=",
+	tailscale-go-rev ? "710a0d861098c07540ad073bb73a42ce81bf54a8",
+	tailscale-go-sha ? "sha256-hnyddxiyqMFHGwV3I4wkBcYNd56schYFi0SL5/0PnMI=",
 }:
 let
 	tailscale-go = pkgs.lib.overrideDerivation nixosUnstable.go_1_18 (attrs: rec {

ssh/tailssh/incubator.go

@@ -2,10 +2,11 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// This file contains the code for the incubator process.
-// Taiscaled launches the incubator as the same user as it was launched as.
-// The incbuator then registers a new session with the OS, sets its own UID to
-// the specified `--uid`` and then lauches the requested `--cmd`.
+// This file contains the code for the incubator process.  Taiscaled
+// launches the incubator as the same user as it was launched as.  The
+// incubator then registers a new session with the OS, sets its UID
+// and groups to the specified `--uid`, `--gid` and `--groups`, and
+// then lauches the requested `--cmd`.
 
 //go:build linux || (darwin && !ios)
 // +build linux darwin,!ios
@@ -13,7 +14,6 @@
 package tailssh
 
 import (
-	"context"
 	"errors"
 	"flag"
 	"fmt"
@@ -25,10 +25,12 @@ import (
 	"os/user"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 	"syscall"
 
 	"github.com/creack/pty"
+	"github.com/pkg/sftp"
 	"github.com/u-root/u-root/pkg/termios"
 	gossh "golang.org/x/crypto/ssh"
 	"golang.org/x/sys/unix"
@@ -58,9 +60,29 @@ var maybeStartLoginSession = func(logf logger.Logf, uid uint32, localUser, remot
 //
 // If ss.srv.tailscaledPath is empty, this method is equivalent to
 // exec.CommandContext.
-func (ss *sshSession) newIncubatorCommand(ctx context.Context, name string, args []string) *exec.Cmd {
+func (ss *sshSession) newIncubatorCommand() *exec.Cmd {
+	var (
+		name   string
+		args   []string
+		isSFTP bool
+	)
+	switch ss.Subsystem() {
+	case "sftp":
+		isSFTP = true
+	case "":
+		name = loginShell(ss.conn.localUser.Uid)
+		if rawCmd := ss.RawCommand(); rawCmd != "" {
+			args = append(args, "-c", rawCmd)
+		} else {
+			args = append(args, "-l") // login shell
+		}
+	default:
+		panic(fmt.Sprintf("unexpected subsystem: %v", ss.Subsystem()))
+	}
+
 	if ss.conn.srv.tailscaledPath == "" {
-		return exec.CommandContext(ctx, name, args...)
+		// TODO(maisem): this doesn't work with sftp
+		return exec.CommandContext(ss.ctx, name, args...)
 	}
 	lu := ss.conn.localUser
 	ci := ss.conn.info
@@ -73,41 +95,66 @@ func (ss *sshSession) newIncubatorCommand(ctx context.Context, name string, args
 		"be-child",
 		"ssh",
 		"--uid=" + lu.Uid,
+		"--gid=" + lu.Gid,
+		"--groups=" + strings.Join(ss.conn.userGroupIDs, ","),
 		"--local-user=" + lu.Username,
 		"--remote-user=" + remoteUser,
 		"--remote-ip=" + ci.src.IP().String(),
-		"--cmd=" + name,
 		"--has-tty=false", // updated in-place by startWithPTY
 		"--tty-name=",     // updated in-place by startWithPTY
 	}
-	if len(args) > 0 {
-		incubatorArgs = append(incubatorArgs, "--")
-		incubatorArgs = append(incubatorArgs, args...)
-	}
 
-	return exec.CommandContext(ctx, ss.conn.srv.tailscaledPath, incubatorArgs...)
+	if isSFTP {
+		incubatorArgs = append(incubatorArgs, "--sftp")
+	} else {
+		incubatorArgs = append(incubatorArgs, "--cmd="+name)
+		if len(args) > 0 {
+			incubatorArgs = append(incubatorArgs, "--")
+			incubatorArgs = append(incubatorArgs, args...)
+		}
+	}
+	return exec.CommandContext(ss.ctx, ss.conn.srv.tailscaledPath, incubatorArgs...)
 }
 
 const debugIncubator = false
 
+type stdRWC struct{}
+
+func (stdRWC) Read(p []byte) (n int, err error) {
+	return os.Stdin.Read(p)
+}
+
+func (stdRWC) Write(b []byte) (n int, err error) {
+	return os.Stdout.Write(b)
+}
+
+func (stdRWC) Close() error {
+	os.Exit(0)
+	return nil
+}
+
 // beIncubator is the entrypoint to the `tailscaled be-child ssh` subcommand.
 // It is responsible for informing the system of a new login session for the user.
 // This is sometimes necessary for mounting home directories and decrypting file
 // systems.
 //
-// Taiscaled launches the incubator as the same user as it was launched as.
-// The incbuator then registers a new session with the OS, sets its own UID to
-// the specified `--uid`` and then lauches the requested `--cmd`.
+// Tailscaled launches the incubator as the same user as it was
+// launched as.  The incubator then registers a new session with the
+// OS, sets its UID and groups to the specified `--uid`, `--gid` and
+// `--groups` and then launches the requested `--cmd`.
 func beIncubator(args []string) error {
 	var (
 		flags      = flag.NewFlagSet("", flag.ExitOnError)
 		uid        = flags.Uint64("uid", 0, "the uid of local-user")
+		gid        = flags.Int("gid", 0, "the gid of local-user")
+		groups     = flags.String("groups", "", "comma-separated list of gids of local-user")
 		localUser  = flags.String("local-user", "", "the user to run as")
 		remoteUser = flags.String("remote-user", "", "the remote user/tags")
 		remoteIP   = flags.String("remote-ip", "", "the remote Tailscale IP")
 		ttyName    = flags.String("tty-name", "", "the tty name (pts/3)")
 		hasTTY     = flags.Bool("has-tty", false, "is the output attached to a tty")
-		cmdName    = flags.String("cmd", "", "the cmd to launch")
+		cmdName    = flags.String("cmd", "", "the cmd to launch (ignored in sftp mode)")
+		sftpMode   = flags.Bool("sftp", false, "run sftp server (cmd is ignored)")
 	)
 	if err := flags.Parse(args); err != nil {
 		return err
@@ -126,11 +173,28 @@ func beIncubator(args []string) error {
 	// Inform the system that we are about to log someone in.
 	// We can only do this if we are running as root.
 	// This is best effort to still allow running on machines where
-	// we don't support starting session, e.g. darwin.
+	// we don't support starting sessions, e.g. darwin.
 	sessionCloser, err := maybeStartLoginSession(logf, uint32(*uid), *localUser, *remoteUser, *remoteIP, *ttyName)
 	if err == nil && sessionCloser != nil {
 		defer sessionCloser()
 	}
+	var groupIDs []int
+	for _, g := range strings.Split(*groups, ",") {
+		gid, err := strconv.ParseInt(g, 10, 32)
+		if err != nil {
+			return err
+		}
+		groupIDs = append(groupIDs, int(gid))
+	}
+	if err := syscall.Setgroups(groupIDs); err != nil {
+		return err
+	}
+	if egid := os.Getegid(); egid != *gid {
+		if err := syscall.Setgid(int(*gid)); err != nil {
+			logf(err.Error())
+			os.Exit(1)
+		}
+	}
 	if euid != *uid {
 		// Switch users if required before starting the desired process.
 		if err := syscall.Setuid(int(*uid)); err != nil {
@@ -138,6 +202,15 @@ func beIncubator(args []string) error {
 			os.Exit(1)
 		}
 	}
+	if *sftpMode {
+		logf("handling sftp")
+
+		server, err := sftp.NewServer(stdRWC{})
+		if err != nil {
+			return err
+		}
+		return server.Serve()
+	}
 
 	cmd := exec.Command(*cmdName, cmdArgs...)
 	cmd.Stdin = os.Stdin
@@ -165,27 +238,24 @@ func beIncubator(args []string) error {
 // The caller can wait for the process to exit by calling cmd.Wait().
 //
 // It sets ss.cmd, stdin, stdout, and stderr.
-func (ss *sshSession) launchProcess(ctx context.Context) error {
-	shell := loginShell(ss.conn.localUser.Uid)
-	var args []string
-	if rawCmd := ss.RawCommand(); rawCmd != "" {
-		args = append(args, "-c", rawCmd)
-	} else {
-		args = append(args, "-l") // login shell
+func (ss *sshSession) launchProcess() error {
+	ss.cmd = ss.newIncubatorCommand()
+
+	cmd := ss.cmd
+	cmd.Dir = ss.conn.localUser.HomeDir
+	cmd.Env = append(cmd.Env, envForUser(ss.conn.localUser)...)
+	for _, kv := range ss.Environ() {
+		if acceptEnvPair(kv) {
+			cmd.Env = append(cmd.Env, kv)
+		}
 	}
 
 	ci := ss.conn.info
-	cmd := ss.newIncubatorCommand(ctx, shell, args)
-	cmd.Dir = ss.conn.localUser.HomeDir
-	cmd.Env = append(cmd.Env, envForUser(ss.conn.localUser)...)
-	cmd.Env = append(cmd.Env, ss.Environ()...)
 	cmd.Env = append(cmd.Env,
 		fmt.Sprintf("SSH_CLIENT=%s %d %d", ci.src.IP(), ci.src.Port(), ci.dst.Port()),
 		fmt.Sprintf("SSH_CONNECTION=%s %d %s %d", ci.src.IP(), ci.src.Port(), ci.dst.IP(), ci.dst.Port()),
 	)
 
-	ss.cmd = cmd
-
 	if ss.agentListener != nil {
 		cmd.Env = append(cmd.Env, fmt.Sprintf("SSH_AUTH_SOCK=%s", ss.agentListener.Addr()))
 	}
@@ -217,7 +287,7 @@ func resizeWindow(f *os.File, winCh <-chan ssh.Window) {
 }
 
 // opcodeShortName is a mapping of SSH opcode
-// to mnemonic names expected by the termios packaage.
+// to mnemonic names expected by the termios package.
 // These are meant to be platform independent.
 var opcodeShortName = map[uint8]string{
 	gossh.VINTR:         "intr",
@@ -430,7 +500,7 @@ func loginShell(uid string) string {
 	if e := os.Getenv("SHELL"); e != "" {
 		return e
 	}
-	return "/bin/bash"
+	return "/bin/sh"
 }
 
 func envForUser(u *user.User) []string {
@@ -451,3 +521,14 @@ func updateStringInSlice(ss []string, a, b string) {
 		}
 	}
 }
+
+// acceptEnvPair reports whether the environment variable key=value pair
+// should be accepted from the client. It uses the same default as OpenSSH
+// AcceptEnv.
+func acceptEnvPair(kv string) bool {
+	k, _, ok := strings.Cut(kv, "=")
+	if !ok {
+		return false
+	}
+	return k == "TERM" || k == "LANG" || strings.HasPrefix(k, "LC_")
+}

ssh/tailssh/tailssh.go

@@ -40,6 +40,7 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/tempfork/gliderlabs/ssh"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/mak"
 )
 
 var (
@@ -118,10 +119,11 @@ type conn struct {
 	// purposes of rule evaluation.
 	now time.Time
 
-	action0   *tailcfg.SSHAction // first matching action
-	srv       *server
-	info      *sshConnInfo // set by setInfo
-	localUser *user.User   // set by checkAuth
+	action0      *tailcfg.SSHAction // first matching action
+	srv          *server
+	info         *sshConnInfo // set by setInfo
+	localUser    *user.User   // set by checkAuth
+	userGroupIDs []string     // set by checkAuth
 
 	insecureSkipTailscaleAuth bool // used by tests.
 }
@@ -191,6 +193,11 @@ func (c *conn) checkAuth(pubKey ssh.PublicKey) error {
 				Message: fmt.Sprintf("failed to lookup %v\r\n", localUser),
 			}
 		}
+		gids, err := lu.GroupIds()
+		if err != nil {
+			return err
+		}
+		c.userGroupIDs = gids
 		c.localUser = lu
 		return nil
 	}
@@ -221,10 +228,12 @@ func (c *conn) ServerConfig(ctx ssh.Context) *gossh.ServerConfig {
 func (srv *server) newConn() (*conn, error) {
 	c := &conn{srv: srv, now: srv.now()}
 	c.Server = &ssh.Server{
-		Version:           "Tailscale",
-		Handler:           c.handleConnPostSSHAuth,
-		RequestHandlers:   map[string]ssh.RequestHandler{},
-		SubsystemHandlers: map[string]ssh.SubsystemHandler{},
+		Version:         "Tailscale",
+		Handler:         c.handleConnPostSSHAuth,
+		RequestHandlers: map[string]ssh.RequestHandler{},
+		SubsystemHandlers: map[string]ssh.SubsystemHandler{
+			"sftp": c.handleConnPostSSHAuth,
+		},
 
 		// Note: the direct-tcpip channel handler and LocalPortForwardingCallback
 		// only adds support for forwarding ports from the local machine.
@@ -359,10 +368,8 @@ func (c *conn) setInfo(cm gossh.ConnMetadata) error {
 	return nil
 }
 
-// evaluatePolicy returns the SSHAction, sshConnInfo and localUser after
-// evaluating the sshUser and remoteAddr against the SSHPolicy. The remoteAddr
-// and localAddr params must be Tailscale IPs. The pubKey may be nil for "none"
-// auth.
+// evaluatePolicy returns the SSHAction and localUser after evaluating
+// the SSHPolicy for this conn. The pubKey may be nil for "none" auth.
 func (c *conn) evaluatePolicy(pubKey gossh.PublicKey) (_ *tailcfg.SSHAction, localUser string, _ error) {
 	pol, ok := c.sshPolicy()
 	if !ok {
@@ -465,7 +472,7 @@ func (srv *server) fetchPublicKeysURL(url string) ([]string, error) {
 
 	srv.mu.Lock()
 	defer srv.mu.Unlock()
-	mapSet(&srv.fetchPublicKeysCache, url, pubKeyCacheEntry{
+	mak.Set(&srv.fetchPublicKeysCache, url, pubKeyCacheEntry{
 		at:    srv.now(),
 		lines: lines,
 		etag:  etag,
@@ -475,7 +482,7 @@ func (srv *server) fetchPublicKeysURL(url string) ([]string, error) {
 
 // handleConnPostSSHAuth runs an SSH session after the SSH-level authentication,
 // but not necessarily before all the Tailscale-level extra verification has
-// completed.
+// completed. It also handles SFTP requests.
 func (c *conn) handleConnPostSSHAuth(s ssh.Session) {
 	sshUser := s.User()
 	action, err := c.resolveTerminalAction(s)
@@ -491,6 +498,15 @@ func (c *conn) handleConnPostSSHAuth(s ssh.Session) {
 		return
 	}
 
+	// Do this check after auth, but before starting the session.
+	switch s.Subsystem() {
+	case "sftp", "":
+	default:
+		fmt.Fprintf(s.Stderr(), "Unsupported subsystem %q \r\n", s.Subsystem())
+		s.Exit(1)
+		return
+	}
+
 	ss := c.newSSHSession(s, action)
 	ss.logf("handling new SSH connection from %v (%v) to ssh-user %q", c.info.uprof.LoginName, c.info.src.IP(), sshUser)
 	ss.logf("access granted to %v as ssh-user %q", c.info.uprof.LoginName, sshUser)
@@ -675,7 +691,7 @@ func (c *conn) fetchSSHAction(ctx context.Context, url string) (*tailcfg.SSHActi
 // unless the process has already exited.
 func (ss *sshSession) killProcessOnContextDone() {
 	<-ss.ctx.Done()
-	// Either the process has already existed, in which case this does nothing.
+	// Either the process has already exited, in which case this does nothing.
 	// Or, the process is still running in which case this will kill it.
 	ss.exitOnce.Do(func() {
 		err := ss.ctx.Err()
@@ -686,6 +702,8 @@ func (ss *sshSession) killProcessOnContextDone() {
 			}
 		}
 		ss.logf("terminating SSH session from %v: %v", ss.conn.info.src.IP(), err)
+		// We don't need to Process.Wait here, sshSession.run() does
+		// the waiting regardless of termination reason.
 		ss.cmd.Process.Kill()
 	})
 }
@@ -714,8 +732,8 @@ func (srv *server) startSession(ss *sshSession) {
 	if _, dup := srv.activeSessionBySharedID[ss.sharedID]; dup {
 		panic("dup sharedID")
 	}
-	mapSet(&srv.activeSessionByH, ss.idH, ss)
-	mapSet(&srv.activeSessionBySharedID, ss.sharedID, ss)
+	mak.Set(&srv.activeSessionByH, ss.idH, ss)
+	mak.Set(&srv.activeSessionBySharedID, ss.sharedID, ss)
 }
 
 // endSession unregisters s from the list of active sessions.
@@ -729,7 +747,7 @@ func (srv *server) endSession(ss *sshSession) {
 var errSessionDone = errors.New("session is done")
 
 // handleSSHAgentForwarding starts a Unix socket listener and in the background
-// forwards agent connections between the listenr and the ssh.Session.
+// forwards agent connections between the listener and the ssh.Session.
 // On success, it assigns ss.agentListener.
 func (ss *sshSession) handleSSHAgentForwarding(s ssh.Session, lu *user.User) error {
 	if !ssh.AgentRequested(ss) || !ss.action.AllowAgentForwarding {
@@ -756,10 +774,14 @@ func (ss *sshSession) handleSSHAgentForwarding(s ssh.Session, lu *user.User) err
 	}
 	socket := ln.Addr().String()
 	dir := filepath.Dir(socket)
-	// Make sure the socket is accessible by the user.
+	// Make sure the socket is accessible only by the user.
+	if err := os.Chmod(socket, 0600); err != nil {
+		return err
+	}
 	if err := os.Chown(socket, int(uid), int(gid)); err != nil {
 		return err
 	}
+	// Make sure the dir is also accessible.
 	if err := os.Chmod(dir, 0755); err != nil {
 		return err
 	}
@@ -786,10 +808,10 @@ func (ss *sshSession) run() {
 
 	defer ss.ctx.CloseWithError(errSessionDone)
 
-	if ss.action.SesssionDuration != 0 {
-		t := time.AfterFunc(ss.action.SesssionDuration, func() {
+	if ss.action.SessionDuration != 0 {
+		t := time.AfterFunc(ss.action.SessionDuration, func() {
 			ss.ctx.CloseWithError(userVisibleError{
-				fmt.Sprintf("Session timeout of %v elapsed.", ss.action.SesssionDuration),
+				fmt.Sprintf("Session timeout of %v elapsed.", ss.action.SessionDuration),
 				context.DeadlineExceeded,
 			})
 		})
@@ -813,27 +835,29 @@ func (ss *sshSession) run() {
 	// See https://github.com/tailscale/tailscale/issues/4146
 	ss.DisablePTYEmulation()
 
-	if err := ss.handleSSHAgentForwarding(ss, lu); err != nil {
-		ss.logf("agent forwarding failed: %v", err)
-	} else if ss.agentListener != nil {
-		// TODO(maisem/bradfitz): add a way to close all session resources
-		defer ss.agentListener.Close()
-	}
-
 	var rec *recording // or nil if disabled
-	if ss.shouldRecord() {
-		var err error
-		rec, err = ss.startNewRecording()
-		if err != nil {
-			fmt.Fprintf(ss, "can't start new recording\r\n")
-			ss.logf("startNewRecording: %v", err)
-			ss.Exit(1)
-			return
+	if ss.Subsystem() != "sftp" {
+		if err := ss.handleSSHAgentForwarding(ss, lu); err != nil {
+			ss.logf("agent forwarding failed: %v", err)
+		} else if ss.agentListener != nil {
+			// TODO(maisem/bradfitz): add a way to close all session resources
+			defer ss.agentListener.Close()
+		}
+
+		if ss.shouldRecord() {
+			var err error
+			rec, err = ss.startNewRecording()
+			if err != nil {
+				fmt.Fprintf(ss, "can't start new recording\r\n")
+				ss.logf("startNewRecording: %v", err)
+				ss.Exit(1)
+				return
+			}
+			defer rec.Close()
 		}
-		defer rec.Close()
 	}
 
-	err := ss.launchProcess(ss.ctx)
+	err := ss.launchProcess()
 	if err != nil {
 		logf("start failed: %v", err.Error())
 		ss.Exit(1)
@@ -873,7 +897,6 @@ func (ss *sshSession) run() {
 	ss.exitOnce.Do(func() {})
 
 	if err == nil {
-		ss.logf("Wait: ok")
 		ss.Exit(0)
 		return
 	}
@@ -953,7 +976,10 @@ func (c *conn) matchRule(r *tailcfg.SSHRule, pubKey gossh.PublicKey) (a *tailcfg
 	if c.ruleExpired(r) {
 		return nil, "", errRuleExpired
 	}
-	if !r.Action.Reject || r.SSHUsers != nil {
+	if !r.Action.Reject {
+		// For all but Reject rules, SSHUsers is required.
+		// If SSHUsers is nil or empty, mapLocalUser will return an
+		// empty string anyway.
 		localUser = mapLocalUser(r.SSHUsers, c.info.sshUser)
 		if localUser == "" {
 			return nil, "", errUserMatch
@@ -1188,7 +1214,7 @@ func (w loggingWriter) Write(p []byte) (n int, err error) {
 	}
 	j = append(j, '\n')
 	if err := w.writeCastLine(j); err != nil {
-		return 0, nil
+		return 0, err
 	}
 	return w.w.Write(p)
 }
@@ -1223,11 +1249,3 @@ func envEq(a, b string) bool {
 	}
 	return a == b
 }
-
-// mapSet assigns m[k] = v, making m if necessary.
-func mapSet[K comparable, V any](m *map[K]V, k K, v V) {
-	if *m == nil {
-		*m = make(map[K]V)
-	}
-	(*m)[k] = v
-}

ssh/tailssh/tailssh_test.go

@@ -265,6 +265,8 @@ func TestSSH(t *testing.T) {
 
 	execSSH := func(args ...string) *exec.Cmd {
 		cmd := exec.Command("ssh",
+			"-F",
+			"none",
 			"-v",
 			"-p", fmt.Sprint(port),
 			"-o", "StrictHostKeyChecking=no",
@@ -431,3 +433,22 @@ func TestExpandPublicKeyURL(t *testing.T) {
 		t.Errorf("on empty: got %q; want %q", got, want)
 	}
 }
+
+func TestAcceptEnvPair(t *testing.T) {
+	tests := []struct {
+		in   string
+		want bool
+	}{
+		{"TERM=x", true},
+		{"term=x", false},
+		{"TERM", false},
+		{"LC_FOO=x", true},
+		{"LD_PRELOAD=naah", false},
+		{"TERM=screen-256color", true},
+	}
+	for _, tt := range tests {
+		if got := acceptEnvPair(tt.in); got != tt.want {
+			t.Errorf("for %q, got %v; want %v", tt.in, got, tt.want)
+		}
+	}
+}

tailcfg/tailcfg.go

@@ -1730,9 +1730,9 @@ type SSHAction struct {
 	// without further prompts.
 	Accept bool `json:"accept,omitempty"`
 
-	// SesssionDuration, if non-zero, is how long the session can stay open
+	// SessionDuration, if non-zero, is how long the session can stay open
 	// before being forcefully terminated.
-	SesssionDuration time.Duration `json:"sessionDuration,omitempty"`
+	SessionDuration time.Duration `json:"sessionDuration,omitempty"`
 
 	// AllowAgentForwarding, if true, allows accepted connections to forward
 	// the ssh agent if requested.

util/mak/mak.go

@@ -0,0 +1,53 @@
+// 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 mak helps make maps. It contains generic helpers to make/assign
+// things, notably to maps, but also slices.
+package mak
+
+import (
+	"fmt"
+	"reflect"
+)
+
+// Set populates an entry in a map, making the map if necessary.
+//
+// That is, it assigns (*m)[k] = v, making *m if it was nil.
+func Set[K comparable, V any, T ~map[K]V](m *T, k K, v V) {
+	if *m == nil {
+		*m = make(map[K]V)
+	}
+	(*m)[k] = v
+}
+
+// NonNil takes a pointer to a Go data structure
+// (currently only a slice or a map) and makes sure it's non-nil for
+// JSON serialization. (In particular, JavaScript clients usually want
+// the field to be defined after they decode the JSON.)
+// MakeNonNil takes a pointer to a Go data structure
+// (currently only a slice or a map) and makes sure it's non-nil for
+// JSON serialization. (In particular, JavaScript clients usually want
+// the field to be defined after they decode the JSON.)
+func NonNil(ptr interface{}) {
+	if ptr == nil {
+		panic("nil interface")
+	}
+	rv := reflect.ValueOf(ptr)
+	if rv.Kind() != reflect.Ptr {
+		panic(fmt.Sprintf("kind %v, not Ptr", rv.Kind()))
+	}
+	if rv.Pointer() == 0 {
+		panic("nil pointer")
+	}
+	rv = rv.Elem()
+	if rv.Pointer() != 0 {
+		return
+	}
+	switch rv.Type().Kind() {
+	case reflect.Slice:
+		rv.Set(reflect.MakeSlice(rv.Type(), 0, 0))
+	case reflect.Map:
+		rv.Set(reflect.MakeMap(rv.Type()))
+	}
+}

util/mak/mak_test.go

@@ -0,0 +1,71 @@
+// 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 mak contains code to help make things.
+package mak
+
+import (
+	"reflect"
+	"testing"
+)
+
+type M map[string]int
+
+func TestSet(t *testing.T) {
+	t.Run("unnamed", func(t *testing.T) {
+		var m map[string]int
+		Set(&m, "foo", 42)
+		Set(&m, "bar", 1)
+		Set(&m, "bar", 2)
+		want := map[string]int{
+			"foo": 42,
+			"bar": 2,
+		}
+		if got := m; !reflect.DeepEqual(got, want) {
+			t.Errorf("got %v; want %v", got, want)
+		}
+	})
+	t.Run("named", func(t *testing.T) {
+		var m M
+		Set(&m, "foo", 1)
+		Set(&m, "bar", 1)
+		Set(&m, "bar", 2)
+		want := M{
+			"foo": 1,
+			"bar": 2,
+		}
+		if got := m; !reflect.DeepEqual(got, want) {
+			t.Errorf("got %v; want %v", got, want)
+		}
+	})
+}
+
+func TestNonNil(t *testing.T) {
+	var s []string
+	NonNil(&s)
+	if len(s) != 0 {
+		t.Errorf("slice len = %d; want 0", len(s))
+	}
+	if s == nil {
+		t.Error("slice still nil")
+	}
+
+	s = append(s, "foo")
+	NonNil(&s)
+	if len(s) != 1 {
+		t.Errorf("len = %d; want 1", len(s))
+	}
+	if s[0] != "foo" {
+		t.Errorf("value = %q; want foo", s)
+	}
+
+	var m map[string]string
+	NonNil(&m)
+	if len(m) != 0 {
+		t.Errorf("map len = %d; want 0", len(s))
+	}
+	if m == nil {
+		t.Error("map still nil")
+	}
+}

wgengine/magicsock/magicsock.go

@@ -54,6 +54,7 @@ import (
 	"tailscale.com/types/netmap"
 	"tailscale.com/types/nettype"
 	"tailscale.com/util/clientmetric"
+	"tailscale.com/util/mak"
 	"tailscale.com/util/netconv"
 	"tailscale.com/util/uniq"
 	"tailscale.com/version"
@@ -438,11 +439,7 @@ func (c *Conn) removeDerpPeerRoute(peer key.NodePublic, derpID int, dc *derphttp
 func (c *Conn) addDerpPeerRoute(peer key.NodePublic, derpID int, dc *derphttp.Client) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if c.derpRoute == nil {
-		c.derpRoute = make(map[key.NodePublic]derpRoute)
-	}
-	r := derpRoute{derpID, dc}
-	c.derpRoute[peer] = r
+	mak.Set(&c.derpRoute, peer, derpRoute{derpID, dc})
 }
 
 // DerpMagicIP is a fake WireGuard endpoint IP address that means
@@ -606,6 +603,7 @@ func (c *Conn) stopPeriodicReSTUNTimerLocked() {
 
 // c.mu must NOT be held.
 func (c *Conn) updateEndpoints(why string) {
+	metricUpdateEndpoints.Add(1)
 	defer func() {
 		c.mu.Lock()
 		defer c.mu.Unlock()
@@ -1050,8 +1048,8 @@ func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, erro
 		}, nil
 	}
 
-	already := make(map[netaddr.IPPort]tailcfg.EndpointType) // endpoint -> how it was found
-	var eps []tailcfg.Endpoint                               // unique endpoints
+	var already map[netaddr.IPPort]tailcfg.EndpointType // endpoint -> how it was found
+	var eps []tailcfg.Endpoint                          // unique endpoints
 
 	ipp := func(s string) (ipp netaddr.IPPort) {
 		ipp, _ = netaddr.ParseIPPort(s)
@@ -1062,7 +1060,7 @@ func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, erro
 			return
 		}
 		if _, ok := already[ipp]; !ok {
-			already[ipp] = et
+			mak.Set(&already, ipp, et)
 			eps = append(eps, tailcfg.Endpoint{Addr: ipp, Type: et})
 		}
 	}
@@ -2771,6 +2769,7 @@ func (c *Conn) ReSTUN(why string) {
 		// raced with a shutdown.
 		return
 	}
+	metricReSTUNCalls.Add(1)
 
 	// If the user stopped the app, stop doing work. (When the
 	// user stops Tailscale via the GUI apps, ipn/local.go
@@ -2920,6 +2919,7 @@ func (c *Conn) rebind(curPortFate currentPortFate) error {
 // Rebind closes and re-binds the UDP sockets and resets the DERP connection.
 // It should be followed by a call to ReSTUN.
 func (c *Conn) Rebind() {
+	metricRebindCalls.Add(1)
 	if err := c.rebind(keepCurrentPort); err != nil {
 		c.logf("%w", err)
 		return
@@ -3957,9 +3957,6 @@ func (de *endpoint) handleCallMeMaybe(m *disco.CallMeMaybe) {
 	for ep := range de.isCallMeMaybeEP {
 		de.isCallMeMaybeEP[ep] = false // mark for deletion
 	}
-	if de.isCallMeMaybeEP == nil {
-		de.isCallMeMaybeEP = map[netaddr.IPPort]bool{}
-	}
 	var newEPs []netaddr.IPPort
 	for _, ep := range m.MyNumber {
 		if ep.IP().Is6() && ep.IP().IsLinkLocalUnicast() {
@@ -3968,7 +3965,7 @@ func (de *endpoint) handleCallMeMaybe(m *disco.CallMeMaybe) {
 			// for these.
 			continue
 		}
-		de.isCallMeMaybeEP[ep] = true
+		mak.Set(&de.isCallMeMaybeEP, ep, true)
 		if es, ok := de.endpointState[ep]; ok {
 			es.callMeMaybeTime = now
 		} else {
@@ -4144,6 +4141,10 @@ var (
 	metricNumPeers     = clientmetric.NewGauge("magicsock_netmap_num_peers")
 	metricNumDERPConns = clientmetric.NewGauge("magicsock_num_derp_conns")
 
+	metricRebindCalls     = clientmetric.NewCounter("magicsock_rebind_calls")
+	metricReSTUNCalls     = clientmetric.NewCounter("magicsock_restun_calls")
+	metricUpdateEndpoints = clientmetric.NewCounter("magicsock_update_endpoints")
+
 	// Sends (data or disco)
 	metricSendDERPQueued      = clientmetric.NewCounter("magicsock_send_derp_queued")
 	metricSendDERPErrorChan   = clientmetric.NewCounter("magicsock_send_derp_error_chan")

wgengine/pendopen.go

@@ -15,6 +15,7 @@ import (
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/net/tstun"
 	"tailscale.com/types/ipproto"
+	"tailscale.com/util/mak"
 	"tailscale.com/wgengine/filter"
 )
 
@@ -115,14 +116,11 @@ func (e *userspaceEngine) trackOpenPostFilterOut(pp *packet.Parsed, t *tstun.Wra
 
 	e.mu.Lock()
 	defer e.mu.Unlock()
-	if e.pendOpen == nil {
-		e.pendOpen = make(map[flowtrack.Tuple]*pendingOpenFlow)
-	}
 	if _, dup := e.pendOpen[flow]; dup {
 		// Duplicates are expected when the OS retransmits. Ignore.
 		return
 	}
-	e.pendOpen[flow] = &pendingOpenFlow{timer: timer}
+	mak.Set(&e.pendOpen, flow, &pendingOpenFlow{timer: timer})
 
 	return filter.Accept
 }

wgengine/userspace.go

@@ -1203,7 +1203,10 @@ func (e *userspaceEngine) linkChange(changed bool, cur *interfaces.State) {
 	why := "link-change-minor"
 	if changed {
 		why = "link-change-major"
+		metricNumMajorChanges.Add(1)
 		e.magicConn.Rebind()
+	} else {
+		metricNumMinorChanges.Add(1)
 	}
 	e.magicConn.ReSTUN(why)
 }
@@ -1551,4 +1554,7 @@ func (ls fwdDNSLinkSelector) PickLink(ip netaddr.IP) (linkName string) {
 var (
 	metricMagicDNSPacketIn = clientmetric.NewGauge("magicdns_packet_in") // for 100.100.100.100
 	metricReflectToOS      = clientmetric.NewGauge("packet_reflect_to_os")
+
+	metricNumMajorChanges = clientmetric.NewCounter("wgengine_major_changes")
+	metricNumMinorChanges = clientmetric.NewCounter("wgengine_minor_changes")
 )