Merge tag 'v1.50.1' into sunos-1.50

Release 1.50.1

Author: nshalman

VERSION.txt

@@ -1 +1 @@
-1.50.0
+1.50.1

cmd/containerboot/main.go

@@ -36,9 +36,15 @@
 //   - TS_SOCKET: the path where the tailscaled LocalAPI socket should
 //     be created.
 //   - TS_AUTH_ONCE: if true, only attempt to log in if not already
-//     logged in. If false (the default, for backwards
-//     compatibility), forcibly log in every time the
-//     container starts.
+//     logged in. If false, forcibly log in every time the container starts.
+//     The default until 1.50.0 was false, but that was misleading: until
+//     1.50, containerboot used `tailscale up` which would ignore an authkey
+//     argument if there was already a node key. Effectively, this behaved
+//     as though TS_AUTH_ONCE were always true.
+//     In 1.50.0 the change was made to use `tailscale login` instead of `up`,
+//     and login will reauthenticate every time it is given an authkey.
+//     In 1.50.1 we set the TS_AUTH_ONCE to true, to match the previously
+//     observed behavior.
 //   - TS_SERVE_CONFIG: if specified, is the file path where the ipn.ServeConfig is located.
 //     It will be applied once tailscaled is up and running. If the file contains
 //     ${TS_CERT_DOMAIN}, it will be replaced with the value of the available FQDN.
@@ -103,7 +109,7 @@ func main() {
 		SOCKSProxyAddr:  defaultEnv("TS_SOCKS5_SERVER", ""),
 		HTTPProxyAddr:   defaultEnv("TS_OUTBOUND_HTTP_PROXY_LISTEN", ""),
 		Socket:          defaultEnv("TS_SOCKET", "/tmp/tailscaled.sock"),
-		AuthOnce:        defaultBool("TS_AUTH_ONCE", false),
+		AuthOnce:        defaultBool("TS_AUTH_ONCE", true),
 		Root:            defaultEnv("TS_TEST_ONLY_ROOT", "/"),
 	}
 
@@ -252,10 +258,13 @@ authLoop:
 	if err := tailscaleSet(ctx, cfg); err != nil {
 		log.Fatalf("failed to auth tailscale: %v", err)
 	}
-	// Remove any serve config that may have been set by a previous
-	// run of containerboot.
-	if err := client.SetServeConfig(ctx, new(ipn.ServeConfig)); err != nil {
-		log.Fatalf("failed to unset serve config: %v", err)
+
+	if cfg.ServeConfigPath != "" {
+		// Remove any serve config that may have been set by a previous run of
+		// containerboot, but only if we're providing a new one.
+		if err := client.SetServeConfig(ctx, new(ipn.ServeConfig)); err != nil {
+			log.Fatalf("failed to unset serve config: %v", err)
+		}
 	}
 
 	if cfg.InKubernetes && cfg.KubeSecret != "" && cfg.KubernetesCanPatch && cfg.AuthOnce {

cmd/containerboot/main_test.go

@@ -129,7 +129,10 @@ func TestContainerBoot(t *testing.T) {
 		{
 			// Out of the box default: runs in userspace mode, ephemeral storage, interactive login.
 			Name: "no_args",
-			Env:  nil,
+			Env: map[string]string{
+				"TS_AUTH_ONCE": "false",
+			},
+
 			Phases: []phase{
 				{
 					WantCmds: []string{
@@ -149,7 +152,8 @@ func TestContainerBoot(t *testing.T) {
 			// Userspace mode, ephemeral storage, authkey provided on every run.
 			Name: "authkey",
 			Env: map[string]string{
-				"TS_AUTHKEY": "tskey-key",
+				"TS_AUTHKEY":   "tskey-key",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -170,7 +174,8 @@ func TestContainerBoot(t *testing.T) {
 			// Userspace mode, ephemeral storage, authkey provided on every run.
 			Name: "authkey-old-flag",
 			Env: map[string]string{
-				"TS_AUTH_KEY": "tskey-key",
+				"TS_AUTH_KEY":  "tskey-key",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -192,6 +197,7 @@ func TestContainerBoot(t *testing.T) {
 			Env: map[string]string{
 				"TS_AUTHKEY":   "tskey-key",
 				"TS_STATE_DIR": filepath.Join(d, "tmp"),
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -211,8 +217,9 @@ func TestContainerBoot(t *testing.T) {
 		{
 			Name: "routes",
 			Env: map[string]string{
-				"TS_AUTHKEY": "tskey-key",
-				"TS_ROUTES":  "1.2.3.0/24,10.20.30.0/24",
+				"TS_AUTHKEY":   "tskey-key",
+				"TS_ROUTES":    "1.2.3.0/24,10.20.30.0/24",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -239,6 +246,7 @@ func TestContainerBoot(t *testing.T) {
 				"TS_AUTHKEY":   "tskey-key",
 				"TS_ROUTES":    "1.2.3.0/24,10.20.30.0/24",
 				"TS_USERSPACE": "false",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -265,6 +273,7 @@ func TestContainerBoot(t *testing.T) {
 				"TS_AUTHKEY":   "tskey-key",
 				"TS_ROUTES":    "::/64,1::/64",
 				"TS_USERSPACE": "false",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -291,6 +300,7 @@ func TestContainerBoot(t *testing.T) {
 				"TS_AUTHKEY":   "tskey-key",
 				"TS_ROUTES":    "::/64,1.2.3.0/24",
 				"TS_USERSPACE": "false",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -317,6 +327,7 @@ func TestContainerBoot(t *testing.T) {
 				"TS_AUTHKEY":   "tskey-key",
 				"TS_DEST_IP":   "1.2.3.4",
 				"TS_USERSPACE": "false",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{
@@ -341,6 +352,7 @@ func TestContainerBoot(t *testing.T) {
 				"TS_AUTHKEY":           "tskey-key",
 				"TS_TAILNET_TARGET_IP": "100.99.99.99",
 				"TS_USERSPACE":         "false",
+				"TS_AUTH_ONCE":         "false",
 			},
 			Phases: []phase{
 				{
@@ -393,6 +405,7 @@ func TestContainerBoot(t *testing.T) {
 			Env: map[string]string{
 				"KUBERNETES_SERVICE_HOST":       kube.Host,
 				"KUBERNETES_SERVICE_PORT_HTTPS": kube.Port,
+				"TS_AUTH_ONCE":                  "false",
 			},
 			KubeSecret: map[string]string{
 				"authkey": "tskey-key",
@@ -430,6 +443,7 @@ func TestContainerBoot(t *testing.T) {
 				"TS_KUBE_SECRET": "",
 				"TS_STATE_DIR":   filepath.Join(d, "tmp"),
 				"TS_AUTHKEY":     "tskey-key",
+				"TS_AUTH_ONCE":   "false",
 			},
 			KubeSecret: map[string]string{},
 			Phases: []phase{
@@ -455,6 +469,7 @@ func TestContainerBoot(t *testing.T) {
 				"KUBERNETES_SERVICE_HOST":       kube.Host,
 				"KUBERNETES_SERVICE_PORT_HTTPS": kube.Port,
 				"TS_AUTHKEY":                    "tskey-key",
+				"TS_AUTH_ONCE":                  "false",
 			},
 			KubeSecret:    map[string]string{},
 			KubeDenyPatch: true,
@@ -524,6 +539,7 @@ func TestContainerBoot(t *testing.T) {
 			Env: map[string]string{
 				"KUBERNETES_SERVICE_HOST":       kube.Host,
 				"KUBERNETES_SERVICE_PORT_HTTPS": kube.Port,
+				"TS_AUTH_ONCE":                  "false",
 			},
 			KubeSecret: map[string]string{
 				"authkey": "tskey-key",
@@ -575,6 +591,7 @@ func TestContainerBoot(t *testing.T) {
 			Env: map[string]string{
 				"TS_SOCKS5_SERVER":              "localhost:1080",
 				"TS_OUTBOUND_HTTP_PROXY_LISTEN": "localhost:8080",
+				"TS_AUTH_ONCE":                  "false",
 			},
 			Phases: []phase{
 				{
@@ -595,6 +612,7 @@ func TestContainerBoot(t *testing.T) {
 			Name: "dns",
 			Env: map[string]string{
 				"TS_ACCEPT_DNS": "true",
+				"TS_AUTH_ONCE":  "false",
 			},
 			Phases: []phase{
 				{
@@ -616,6 +634,7 @@ func TestContainerBoot(t *testing.T) {
 			Env: map[string]string{
 				"TS_EXTRA_ARGS":            "--widget=rotated",
 				"TS_TAILSCALED_EXTRA_ARGS": "--experiments=widgets",
+				"TS_AUTH_ONCE":             "false",
 			},
 			Phases: []phase{
 				{
@@ -635,7 +654,8 @@ func TestContainerBoot(t *testing.T) {
 		{
 			Name: "hostname",
 			Env: map[string]string{
-				"TS_HOSTNAME": "my-server",
+				"TS_HOSTNAME":  "my-server",
+				"TS_AUTH_ONCE": "false",
 			},
 			Phases: []phase{
 				{

cmd/derper/depaware.txt

@@ -270,7 +270,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         runtime/metrics                                              from github.com/prometheus/client_golang/prometheus+
         runtime/pprof                                                from net/http/pprof
         runtime/trace                                                from net/http/pprof
-        slices                                                       from tailscale.com/ipn+
+        slices                                                       from tailscale.com/ipn/ipnstate+
         sort                                                         from compress/flate+
         strconv                                                      from compress/flate+
         strings                                                      from bufio+

cmd/tailscale/cli/funnel.go

@@ -164,12 +164,12 @@ func (e *serveEnv) verifyFunnelEnabled(ctx context.Context, st *ipnstate.Status,
 		// the feature flag on.
 		// TODO(sonia,tailscale/corp#10577): Remove this fallback once the
 		// control flag is turned on for all domains.
-		if err := ipn.CheckFunnelAccess(port, st.Self.Capabilities); err != nil {
+		if err := ipn.CheckFunnelAccess(port, st.Self); err != nil {
 			return err
 		}
 	default:
 		// Done with enablement, make sure the requested port is allowed.
-		if err := ipn.CheckFunnelPort(port, st.Self.Capabilities); err != nil {
+		if err := ipn.CheckFunnelPort(port, st.Self); err != nil {
 			return err
 		}
 	}

ipn/serve.go

@@ -9,10 +9,10 @@ import (
 	"net"
 	"net/netip"
 	"net/url"
-	"slices"
 	"strconv"
 	"strings"
 
+	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/tailcfg"
 )
 
@@ -237,48 +237,72 @@ func (sc *ServeConfig) IsFunnelOn() bool {
 //  2. the node has the "funnel" nodeAttr
 //  3. the port is allowed for Funnel
 //
-// The nodeAttrs arg should be the node's Self.Capabilities which should contain
-// the attribute we're checking for and possibly warning-capabilities for
-// Funnel.
-func CheckFunnelAccess(port uint16, nodeAttrs []tailcfg.NodeCapability) error {
-	if !slices.Contains(nodeAttrs, tailcfg.CapabilityHTTPS) {
+// The node arg should be the ipnstate.Status.Self node.
+func CheckFunnelAccess(port uint16, node *ipnstate.PeerStatus) error {
+	if !node.HasCap(tailcfg.CapabilityHTTPS) {
 		return errors.New("Funnel not available; HTTPS must be enabled. See https://tailscale.com/s/https.")
 	}
-	if !slices.Contains(nodeAttrs, tailcfg.NodeAttrFunnel) {
+	if !node.HasCap(tailcfg.NodeAttrFunnel) {
 		return errors.New("Funnel not available; \"funnel\" node attribute not set. See https://tailscale.com/s/no-funnel.")
 	}
-	return CheckFunnelPort(port, nodeAttrs)
+	return CheckFunnelPort(port, node)
 }
 
 // CheckFunnelPort checks whether the given port is allowed for Funnel.
 // It uses the tailcfg.CapabilityFunnelPorts nodeAttr to determine the allowed
 // ports.
-func CheckFunnelPort(wantedPort uint16, nodeAttrs []tailcfg.NodeCapability) error {
+func CheckFunnelPort(wantedPort uint16, node *ipnstate.PeerStatus) error {
 	deny := func(allowedPorts string) error {
 		if allowedPorts == "" {
 			return fmt.Errorf("port %d is not allowed for funnel", wantedPort)
 		}
 		return fmt.Errorf("port %d is not allowed for funnel; allowed ports are: %v", wantedPort, allowedPorts)
 	}
 	var portsStr string
-	for _, attr := range nodeAttrs {
-		attr := string(attr)
-		if !strings.HasPrefix(attr, string(tailcfg.CapabilityFunnelPorts)) {
-			continue
-		}
+	parseAttr := func(attr string) (string, error) {
 		u, err := url.Parse(attr)
 		if err != nil {
-			return deny("")
+			return "", deny("")
 		}
-		portsStr = u.Query().Get("ports")
+		portsStr := u.Query().Get("ports")
 		if portsStr == "" {
-			return deny("")
+			return "", deny("")
 		}
 		u.RawQuery = ""
 		if u.String() != string(tailcfg.CapabilityFunnelPorts) {
-			return deny("")
+			return "", deny("")
+		}
+		return portsStr, nil
+	}
+	for attr := range node.CapMap {
+		attr := string(attr)
+		if !strings.HasPrefix(attr, string(tailcfg.CapabilityFunnelPorts)) {
+			continue
+		}
+		var err error
+		portsStr, err = parseAttr(attr)
+		if err != nil {
+			return err
+		}
+		break
+	}
+	if portsStr == "" {
+		for _, attr := range node.Capabilities {
+			attr := string(attr)
+			if !strings.HasPrefix(attr, string(tailcfg.CapabilityFunnelPorts)) {
+				continue
+			}
+			var err error
+			portsStr, err = parseAttr(attr)
+			if err != nil {
+				return err
+			}
+			break
 		}
 	}
+	if portsStr == "" {
+		return deny("")
+	}
 	wantedPortString := strconv.Itoa(int(wantedPort))
 	for _, ps := range strings.Split(portsStr, ",") {
 		if ps == "" {

ipn/serve_test.go

@@ -5,6 +5,7 @@ package ipn
 import (
 	"testing"
 
+	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/tailcfg"
 )
 
@@ -26,7 +27,11 @@ func TestCheckFunnelAccess(t *testing.T) {
 		{3000, caps(portAttr, tailcfg.CapabilityHTTPS, tailcfg.NodeAttrFunnel), true},
 	}
 	for _, tt := range tests {
-		err := CheckFunnelAccess(tt.port, tt.caps)
+		cm := tailcfg.NodeCapMap{}
+		for _, c := range tt.caps {
+			cm[c] = nil
+		}
+		err := CheckFunnelAccess(tt.port, &ipnstate.PeerStatus{CapMap: cm})
 		switch {
 		case err != nil && tt.wantErr,
 			err == nil && !tt.wantErr:

net/portmapper/portmapper.go

@@ -1040,7 +1040,16 @@ func getUPnPErrorsMetric(code int) *clientmetric.Metric {
 		return mm
 	}
 
-	mm = clientmetric.NewCounter(fmt.Sprintf("portmap_upnp_errors_with_code_%d", code))
+	// Metric names cannot contain a hyphen, so we handle negative numbers
+	// by prefixing the name with a "minus_".
+	var codeStr string
+	if code < 0 {
+		codeStr = fmt.Sprintf("portmap_upnp_errors_with_code_minus_%d", -code)
+	} else {
+		codeStr = fmt.Sprintf("portmap_upnp_errors_with_code_%d", code)
+	}
+
+	mm = clientmetric.NewCounter(codeStr)
 	mak.Set(&metricUPnPErrorsByCode, code, mm)
 	return mm
 }

net/portmapper/portmapper_test.go

@@ -124,3 +124,14 @@ func TestPCPIntegration(t *testing.T) {
 		t.Errorf("got nil mapping after successful createOrGetMapping")
 	}
 }
+
+// Test to ensure that metric names generated by this function do not contain
+// invalid characters.
+//
+// See https://github.com/tailscale/tailscale/issues/9551
+func TestGetUPnPErrorsMetric(t *testing.T) {
+	// This will panic if the metric name is invalid.
+	getUPnPErrorsMetric(100)
+	getUPnPErrorsMetric(0)
+	getUPnPErrorsMetric(-100)
+}

tsnet/tsnet.go

@@ -926,7 +926,7 @@ func (s *Server) ListenFunnel(network, addr string, opts ...FunnelOption) (net.L
 	// flow here instead of CheckFunnelAccess to allow the user to turn on Funnel
 	// if not already on. Specifically when running from a terminal.
 	// See cli.serveEnv.verifyFunnelEnabled.
-	if err := ipn.CheckFunnelAccess(uint16(port), st.Self.Capabilities); err != nil {
+	if err := ipn.CheckFunnelAccess(uint16(port), st.Self); err != nil {
 		return nil, err
 	}