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 <andrew@tailscale.com>

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 {

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 {

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 {