diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 9671c2554..c9a6a89bf 100644 --- a/ipn/localapi/localapi.go +++ b/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) diff --git a/ipn/localapi/localapi_test.go b/ipn/localapi/localapi_test.go index dff58a66d..b534c594f 100644 --- a/ipn/localapi/localapi_test.go +++ b/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) + } + }) + } +} diff --git a/ipn/serve.go b/ipn/serve.go index b22a5bdb7..84db09d1d 100644 --- a/ipn/serve.go +++ b/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 { diff --git a/ipn/serve_test.go b/ipn/serve_test.go index 2576b2b66..38a158f3f 100644 --- a/ipn/serve_test.go +++ b/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) + } + }) + } +}