ipn/localapi: require local Windows admin to set serve path (#9969)

For a serve config with a path handler, ensure the caller is a local administrator on Windows.

updates #8489

Signed-off-by: Tyler Smalley <[email protected]>

ipn/localapi/localapi.go

@@ -915,6 +915,15 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) {
 			writeErrorJSON(w, fmt.Errorf("decoding config: %w", err))
 			return
 		}
+
+		// require a local admin when setting a path handler
+		// TODO: roll-up this Windows-specific check into either PermitWrite
+		// or a global admin escalation check.
+		if shouldDenyServeConfigForGOOSAndUserContext(runtime.GOOS, configIn, h) {
+			http.Error(w, "must be a Windows local admin to serve a path", http.StatusUnauthorized)
+			return
+		}
+
 		etag := r.Header.Get("If-Match")
 		if err := h.b.SetServeConfig(configIn, etag); err != nil {
 			if errors.Is(err, ipnlocal.ErrETagMismatch) {
@@ -930,6 +939,16 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func shouldDenyServeConfigForGOOSAndUserContext(goos string, configIn *ipn.ServeConfig, h *Handler) bool {
+	if goos != "windows" {
+		return false
+	}
+	if !configIn.HasPathHandler() {
+		return false
+	}
+	return !h.CallerIsLocalAdmin
+}
+
 func (h *Handler) serveCheckIPForwarding(w http.ResponseWriter, r *http.Request) {
 	if !h.PermitRead {
 		http.Error(w, "IP forwarding check access denied", http.StatusForbidden)

ipn/localapi/localapi_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
@@ -145,3 +146,69 @@ func TestWhoIsJustIP(t *testing.T) {
 		})
 	}
 }
+
+func TestShouldDenyServeConfigForGOOSAndUserContext(t *testing.T) {
+	tests := []struct {
+		name     string
+		goos     string
+		configIn *ipn.ServeConfig
+		h        *Handler
+		want     bool
+	}{
+		{
+			name:     "linux",
+			goos:     "linux",
+			configIn: &ipn.ServeConfig{},
+			h:        &Handler{CallerIsLocalAdmin: false},
+			want:     false,
+		},
+		{
+			name: "windows-not-path-handler",
+			goos: "windows",
+			configIn: &ipn.ServeConfig{
+				Web: map[ipn.HostPort]*ipn.WebServerConfig{
+					"foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+						"/": {Proxy: "http://127.0.0.1:3000"},
+					}},
+				},
+			},
+			h:    &Handler{CallerIsLocalAdmin: false},
+			want: false,
+		},
+		{
+			name: "windows-path-handler-admin",
+			goos: "windows",
+			configIn: &ipn.ServeConfig{
+				Web: map[ipn.HostPort]*ipn.WebServerConfig{
+					"foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+						"/": {Path: "/tmp"},
+					}},
+				},
+			},
+			h:    &Handler{CallerIsLocalAdmin: true},
+			want: false,
+		},
+		{
+			name: "windows-path-handler-not-admin",
+			goos: "windows",
+			configIn: &ipn.ServeConfig{
+				Web: map[ipn.HostPort]*ipn.WebServerConfig{
+					"foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+						"/": {Path: "/tmp"},
+					}},
+				},
+			},
+			h:    &Handler{CallerIsLocalAdmin: false},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := shouldDenyServeConfigForGOOSAndUserContext(tt.goos, tt.configIn, tt.h)
+			if got != tt.want {
+				t.Errorf("shouldDenyServeConfigForGOOSAndUserContext() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

ipn/serve.go

@@ -163,6 +163,30 @@ func (sc *ServeConfig) GetTCPPortHandler(port uint16) *TCPPortHandler {
 	return sc.TCP[port]
 }
 
+// HasPathHandler reports whether if ServeConfig has at least
+// one path handler, including foreground configs.
+func (sc *ServeConfig) HasPathHandler() bool {
+	if sc.Web != nil {
+		for _, webServerConfig := range sc.Web {
+			for _, httpHandler := range webServerConfig.Handlers {
+				if httpHandler.Path != "" {
+					return true
+				}
+			}
+		}
+	}
+
+	if sc.Foreground != nil {
+		for _, fgConfig := range sc.Foreground {
+			if fgConfig.HasPathHandler() {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // IsTCPForwardingAny reports whether ServeConfig is currently forwarding in
 // TCPForward mode on any port. This is exclusive of Web/HTTPS serving.
 func (sc *ServeConfig) IsTCPForwardingAny() bool {

ipn/serve_test.go

@@ -43,3 +43,86 @@ func TestCheckFunnelAccess(t *testing.T) {
 		}
 	}
 }
+
+func TestHasPathHandler(t *testing.T) {
+	tests := []struct {
+		name string
+		cfg  ServeConfig
+		want bool
+	}{
+		{
+			name: "empty-config",
+			cfg:  ServeConfig{},
+			want: false,
+		},
+		{
+			name: "with-bg-path-handler",
+			cfg: ServeConfig{
+				TCP: map[uint16]*TCPPortHandler{80: {HTTP: true}},
+				Web: map[HostPort]*WebServerConfig{
+					"foo.test.ts.net:80": {Handlers: map[string]*HTTPHandler{
+						"/": {Path: "/tmp"},
+					}},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-fg-path-handler",
+			cfg: ServeConfig{
+				TCP: map[uint16]*TCPPortHandler{
+					443: {HTTPS: true},
+				},
+				Foreground: map[string]*ServeConfig{
+					"abc123": {
+						TCP: map[uint16]*TCPPortHandler{80: {HTTP: true}},
+						Web: map[HostPort]*WebServerConfig{
+							"foo.test.ts.net:80": {Handlers: map[string]*HTTPHandler{
+								"/": {Path: "/tmp"},
+							}},
+						},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-no-bg-path-handler",
+			cfg: ServeConfig{
+				TCP: map[uint16]*TCPPortHandler{443: {HTTPS: true}},
+				Web: map[HostPort]*WebServerConfig{
+					"foo.test.ts.net:443": {Handlers: map[string]*HTTPHandler{
+						"/": {Proxy: "http://127.0.0.1:3000"},
+					}},
+				},
+				AllowFunnel: map[HostPort]bool{"foo.test.ts.net:443": true},
+			},
+			want: false,
+		},
+		{
+			name: "with-no-fg-path-handler",
+			cfg: ServeConfig{
+				Foreground: map[string]*ServeConfig{
+					"abc123": {
+						TCP: map[uint16]*TCPPortHandler{443: {HTTPS: true}},
+						Web: map[HostPort]*WebServerConfig{
+							"foo.test.ts.net:443": {Handlers: map[string]*HTTPHandler{
+								"/": {Proxy: "http://127.0.0.1:3000"},
+							}},
+						},
+						AllowFunnel: map[HostPort]bool{"foo.test.ts.net:443": true},
+					},
+				},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.cfg.HasPathHandler()
+			if tt.want != got {
+				t.Errorf("HasPathHandler() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}