From 336b3b7df0ab8f9877dcf0b62a09851e4c5178a3 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Thu, 15 May 2025 14:26:19 -0700 Subject: [PATCH] cmd/proxy-to-grafana: strip X-Webauth* headers from all requests (#15985) Update proxy-to-grafana to strip any X-Webauth prefixed headers passed by the client in *every* request, not just those to /login. /api/ routes will also accept these headers to authenticate users, necessitating their removal to prevent forgery. Updates tailscale/corp#28687 Signed-off-by: Patrick O'Doherty --- cmd/proxy-to-grafana/proxy-to-grafana.go | 23 +++--- cmd/proxy-to-grafana/proxy-to-grafana_test.go | 77 +++++++++++++++++++ 2 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 cmd/proxy-to-grafana/proxy-to-grafana_test.go diff --git a/cmd/proxy-to-grafana/proxy-to-grafana.go b/cmd/proxy-to-grafana/proxy-to-grafana.go index bdabd650f..27f5e338c 100644 --- a/cmd/proxy-to-grafana/proxy-to-grafana.go +++ b/cmd/proxy-to-grafana/proxy-to-grafana.go @@ -53,7 +53,7 @@ import ( "strings" "time" - "tailscale.com/client/local" + "tailscale.com/client/tailscale/apitype" "tailscale.com/tailcfg" "tailscale.com/tsnet" ) @@ -195,13 +195,7 @@ func main() { log.Fatal(http.Serve(ln, proxy)) } -func modifyRequest(req *http.Request, localClient *local.Client) { - // with enable_login_token set to true, we get a cookie that handles - // auth for paths that are not /login - if req.URL.Path != "/login" { - return - } - +func modifyRequest(req *http.Request, localClient whoisIdentitySource) { // Delete any existing X-Webauth-* headers to prevent possible spoofing // if getting Tailnet identity fails. for h := range req.Header { @@ -210,6 +204,13 @@ func modifyRequest(req *http.Request, localClient *local.Client) { } } + // Set the X-Webauth-* headers only for the /login path + // With enable_login_token set to true, we get a cookie that handles + // auth for paths that are not /login + if req.URL.Path != "/login" { + return + } + user, role, err := getTailscaleIdentity(req.Context(), localClient, req.RemoteAddr) if err != nil { log.Printf("error getting Tailscale user: %v", err) @@ -221,7 +222,7 @@ func modifyRequest(req *http.Request, localClient *local.Client) { req.Header.Set("X-Webauth-Role", role.String()) } -func getTailscaleIdentity(ctx context.Context, localClient *local.Client, ipPort string) (*tailcfg.UserProfile, grafanaRole, error) { +func getTailscaleIdentity(ctx context.Context, localClient whoisIdentitySource, ipPort string) (*tailcfg.UserProfile, grafanaRole, error) { whois, err := localClient.WhoIs(ctx, ipPort) if err != nil { return nil, ViewerRole, fmt.Errorf("failed to identify remote host: %w", err) @@ -248,3 +249,7 @@ func getTailscaleIdentity(ctx context.Context, localClient *local.Client, ipPort return whois.UserProfile, role, nil } + +type whoisIdentitySource interface { + WhoIs(ctx context.Context, ipPort string) (*apitype.WhoIsResponse, error) +} diff --git a/cmd/proxy-to-grafana/proxy-to-grafana_test.go b/cmd/proxy-to-grafana/proxy-to-grafana_test.go new file mode 100644 index 000000000..083c4bc49 --- /dev/null +++ b/cmd/proxy-to-grafana/proxy-to-grafana_test.go @@ -0,0 +1,77 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause +package main + +import ( + "context" + "fmt" + "net/http/httptest" + "testing" + + "tailscale.com/client/tailscale/apitype" + "tailscale.com/tailcfg" +) + +type mockWhoisSource struct { + id *apitype.WhoIsResponse +} + +func (m *mockWhoisSource) WhoIs(ctx context.Context, remoteAddr string) (*apitype.WhoIsResponse, error) { + if m.id == nil { + return nil, fmt.Errorf("missing mock identity") + } + return m.id, nil +} + +var whois = &apitype.WhoIsResponse{ + UserProfile: &tailcfg.UserProfile{ + LoginName: "foobar@example.com", + DisplayName: "Foobar", + }, + Node: &tailcfg.Node{ + ID: 1, + }, +} + +func TestModifyRequest_Login(t *testing.T) { + req := httptest.NewRequest("GET", "/login", nil) + modifyRequest(req, &mockWhoisSource{id: whois}) + + if got := req.Header.Get("X-Webauth-User"); got != "foobar@example.com" { + t.Errorf("X-Webauth-User = %q; want %q", got, "foobar@example.com") + } + + if got := req.Header.Get("X-Webauth-Role"); got != "Viewer" { + t.Errorf("X-Webauth-Role = %q; want %q", got, "Viewer") + } +} + +func TestModifyRequest_RemoveHeaders_Login(t *testing.T) { + req := httptest.NewRequest("GET", "/login", nil) + req.Header.Set("X-Webauth-User", "malicious@example.com") + req.Header.Set("X-Webauth-Role", "Admin") + + modifyRequest(req, &mockWhoisSource{id: whois}) + + if got := req.Header.Get("X-Webauth-User"); got != "foobar@example.com" { + t.Errorf("X-Webauth-User = %q; want %q", got, "foobar@example.com") + } + if got := req.Header.Get("X-Webauth-Role"); got != "Viewer" { + t.Errorf("X-Webauth-Role = %q; want %q", got, "Viewer") + } +} + +func TestModifyRequest_RemoveHeaders_API(t *testing.T) { + req := httptest.NewRequest("DELETE", "/api/org/users/1", nil) + req.Header.Set("X-Webauth-User", "malicious@example.com") + req.Header.Set("X-Webauth-Role", "Admin") + + modifyRequest(req, &mockWhoisSource{id: whois}) + + if got := req.Header.Get("X-Webauth-User"); got != "" { + t.Errorf("X-Webauth-User = %q; want %q", got, "") + } + if got := req.Header.Get("X-Webauth-Role"); got != "" { + t.Errorf("X-Webauth-Role = %q; want %q", got, "") + } +}