From e19eb275ea21790aac94be1a5eddcd20ebb40a35 Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Mon, 2 Nov 2026 16:42:36 -0500 Subject: [PATCH] ipn/ipnlocal,cmd/tsnet-proxy: strip underscore variants of identity headers (#22143) Serve deletes client-supplied `Tailscale-User-Login` (and similar) headers before setting its own. However, `http.Header.Del` only matches the exact canonical name. A client could send Tailscale_User_Login instead, which was passed through to the backend. For Go (and some other) backends, that's not an issue, but backends that map headers to CGI-style variables (HTTP_TAILSCALE_USER_LOGIN) treat both names the same. Thus, for Funnel requests (or tagged nodes), where serve sets no identity headers, the backend could trust the spoofed value. The same applies to Tailscale-App-Capabilities. This fixes the issue by also deleting any header whose name matches case-insensitively after replacing '_' with '-'. This is similar to how other projects do this: - Django drops them: https://www.djangoproject.com/weblog/2015/jan/13/security/ - nginx does not allow them by default: https://nginx.org/en/docs/http/ngx_http_core_module.html#underscores_in_headers - Puma had a CVE and now drops them: https://nvd.nist.gov/vuln/detail/cve-2024-45614 Fixes tailscale/corp#50821 Signed-off-by: Andrew Dunham --- cmd/tsnet-proxy/tsnet-proxy.go | 28 +++++++++++++++++---- ipn/ipnlocal/serve.go | 33 ++++++++++++++++++++----- ipn/ipnlocal/serve_test.go | 45 +++++++++++++++++++++++++++++++--- 3 files changed, 91 insertions(+), 15 deletions(-) diff --git a/cmd/tsnet-proxy/tsnet-proxy.go b/cmd/tsnet-proxy/tsnet-proxy.go index 0a83fd1a8..f3966fe64 100644 --- a/cmd/tsnet-proxy/tsnet-proxy.go +++ b/cmd/tsnet-proxy/tsnet-proxy.go @@ -34,6 +34,7 @@ import ( "net/url" "os" "strconv" + "strings" "unicode/utf8" "tailscale.com/client/local" @@ -148,11 +149,14 @@ func proxyTCP(c net.Conn, target string) { } func addTailscaleIdentityHeaders(lc *local.Client, r *httputil.ProxyRequest) { - r.Out.Header.Del("Tailscale-User-Login") - r.Out.Header.Del("Tailscale-User-Name") - r.Out.Header.Del("Tailscale-User-Profile-Pic") - r.Out.Header.Del("Tailscale-Funnel-Request") - r.Out.Header.Del("Tailscale-Headers-Info") + delHeaderVariants(r.Out.Header, + "Tailscale-User-Login", + "Tailscale-User-Name", + "Tailscale-User-Profile-Pic", + "Tailscale-Funnel-Request", + "Tailscale-Headers-Info", + "Tailscale-App-Capabilities", + ) who, err := lc.WhoIs(r.In.Context(), r.In.RemoteAddr) if err != nil || who == nil || who.Node.IsTagged() { @@ -163,6 +167,20 @@ func addTailscaleIdentityHeaders(lc *local.Client, r *httputil.ProxyRequest) { r.Out.Header.Set("Tailscale-User-Profile-Pic", who.UserProfile.ProfilePicURL) } +// delHeaderVariants is the same logic that tailscaled's serve path uses to +// delete headers before adding outbound headers. +func delHeaderVariants(h http.Header, names ...string) { + for k := range h { + norm := strings.ReplaceAll(k, "_", "-") + for _, name := range names { + if strings.EqualFold(norm, name) { + delete(h, k) + break + } + } + } +} + // encHeader mirrors the encoding tailscaled's serve path applies to // user-provided strings destined for HTTP headers. func encHeader(v string) string { diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 8f300aa9f..da0e14928 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -1160,11 +1160,13 @@ func addProxyForwardedHeaders(r *httputil.ProxyRequest) { func (b *LocalBackend) addTailscaleIdentityHeaders(r *httputil.ProxyRequest) { // Clear any incoming values squatting in the headers. - r.Out.Header.Del("Tailscale-User-Login") - r.Out.Header.Del("Tailscale-User-Name") - r.Out.Header.Del("Tailscale-User-Profile-Pic") - r.Out.Header.Del("Tailscale-Funnel-Request") - r.Out.Header.Del("Tailscale-Headers-Info") + delHeaderVariants(r.Out.Header, + "Tailscale-User-Login", + "Tailscale-User-Name", + "Tailscale-User-Profile-Pic", + "Tailscale-Funnel-Request", + "Tailscale-Headers-Info", + ) c, ok := serveHTTPContextKey.ValueOk(r.Out.Context()) if !ok { @@ -1189,6 +1191,25 @@ func (b *LocalBackend) addTailscaleIdentityHeaders(r *httputil.ProxyRequest) { r.Out.Header.Set("Tailscale-Headers-Info", "https://tailscale.com/s/serve-headers") } +// delHeaderVariants deletes each header in 'names' from 'h', along with any +// header whose name matches case-insensitively after replacing '_' with '-'. +// +// Any backends that map headers to CGI-style variables (e.g. PHP; +// HTTP_TAILSCALE_USER_LOGIN) would treat "Tailscale_User_Login" the same as +// "Tailscale-User-Login". If we just used a plain [http.Header.Del], clients +// could spoof the headers we set. +func delHeaderVariants(h http.Header, names ...string) { + for k := range h { + norm := strings.ReplaceAll(k, "_", "-") + for _, name := range names { + if strings.EqualFold(norm, name) { + delete(h, k) + break + } + } + } +} + // encTailscaleHeaderValue cleans or encodes as necessary v, to be suitable in // an HTTP header value. See // https://github.com/tailscale/tailscale/issues/11603. @@ -1206,7 +1227,7 @@ func encTailscaleHeaderValue(v string) string { func (b *LocalBackend) addAppCapabilitiesHeader(r *httputil.ProxyRequest) error { const appCapabilitiesHeaderName = "Tailscale-App-Capabilities" - r.Out.Header.Del(appCapabilitiesHeaderName) + delHeaderVariants(r.Out.Header, appCapabilitiesHeaderName) c, ok := serveHTTPContextKey.ValueOk(r.Out.Context()) if !ok || c.Funnel != nil { diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index 4081835be..b65ebb56d 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -820,8 +820,9 @@ func TestServeHTTPProxyHeaders(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := &http.Request{ - URL: &url.URL{Path: "/"}, - TLS: &tls.ConnectionState{ServerName: "example.ts.net"}, + URL: &url.URL{Path: "/"}, + TLS: &tls.ConnectionState{ServerName: "example.ts.net"}, + Header: spoofedTailscaleHeaders(), } req = req.WithContext(serveHTTPContextKey.WithValue(req.Context(), &serveHTTPContext{ DestPort: 443, @@ -838,6 +839,7 @@ func TestServeHTTPProxyHeaders(t *testing.T) { t.Errorf("invalid %q header; want=%q, got=%q", c.header, c.want, got) } } + checkNoSpoofedTailscaleHeaders(t, h) }) } } @@ -963,8 +965,9 @@ func TestServeHTTPProxyGrantHeader(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := &http.Request{ - URL: &url.URL{Path: "/"}, - TLS: &tls.ConnectionState{ServerName: "example.ts.net"}, + URL: &url.URL{Path: "/"}, + TLS: &tls.ConnectionState{ServerName: "example.ts.net"}, + Header: spoofedTailscaleHeaders(), } req = req.WithContext(serveHTTPContextKey.WithValue(req.Context(), &serveHTTPContext{ DestPort: 443, @@ -988,10 +991,44 @@ func TestServeHTTPProxyGrantHeader(t *testing.T) { t.Errorf("invalid %q header; want=%q, got=%q", c.header, c.want, got) } } + checkNoSpoofedTailscaleHeaders(t, h) }) } } +// spoofedTailscaleHeaders returns client-supplied request headers that try to +// impersonate the headers set by `tailscale serve`. This includes variants +// using '_' in place of '-' that CGI-style backends treat as equivalent. +func spoofedTailscaleHeaders() http.Header { + h := http.Header{} + for _, name := range []string{ + "Tailscale-User-Login", + "Tailscale-User-Name", + "Tailscale-User-Profile-Pic", + "Tailscale-Funnel-Request", + "Tailscale-Headers-Info", + "Tailscale-App-Capabilities", + } { + h[name] = []string{"spoofed"} + h[strings.ReplaceAll(name, "-", "_")] = []string{"spoofed"} + h[http.CanonicalHeaderKey(strings.ReplaceAll(strings.ToLower(name), "-", "_"))] = []string{"spoofed"} + } + return h +} + +// checkNoSpoofedTailscaleHeaders reports an error if any value from +// spoofedTailscaleHeaders made it to the backend. +func checkNoSpoofedTailscaleHeaders(t *testing.T, h http.Header) { + t.Helper() + for k, vv := range h { + for _, v := range vv { + if strings.Contains(v, "spoofed") { + t.Errorf("spoofed header %q reached backend: %q", k, v) + } + } + } +} + // newAppCapsTestBackend returns a test backend whose peer at 100.150.151.152 // holds the example.com/cap/interesting capability for the node 100.150.151.151. func newAppCapsTestBackend(t *testing.T) *LocalBackend {