From 98d65d7762054c64b6bb0a89223b5eb996cfce8c Mon Sep 17 00:00:00 2001 From: winstoncrooker Date: Thu, 14 May 2026 16:14:45 -0700 Subject: [PATCH] token: validate response host in GetAppInfo before trusting AppInfo headers GetAppInfo stops the redirect chain when the last request URL contains AccessLoginWorkerPath but does not check the host serving the response. Once that predicate fires, the response's CF-Access-Domain header and the request's ?kid= query parameter become authoritative for AppInfo.AppDomain and AppInfo.AppAUD, which then flow into GetAppTokenIfExists (on-disk token lookup) and BuildAccessRequest (sends the per-app Cloudflare Access JWT in Cf-Access-Token to the host the user invoked cloudflared against). Add an isCloudflareAccessHost helper that matches *.cloudflareaccess.com (case-insensitive, port-tolerant) and gate the AccessLoginWorkerPath branch on the response host satisfying it. Add a unit test for the helper and a httptest-based test that exercises GetAppInfo against a server mimicking the attacker shape. --- token/token.go | 24 ++++++++++++++++++ token/token_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/token/token.go b/token/token.go index 5d2dd521152..0803574d5fc 100644 --- a/token/token.go +++ b/token/token.go @@ -23,8 +23,23 @@ const ( appAUDHeader = "CF-Access-Aud" AccessLoginWorkerPath = "/cdn-cgi/access/login" AccessAuthorizedWorkerPath = "/cdn-cgi/access/authorized" + cloudflareAccessSuffix = ".cloudflareaccess.com" ) +// isCloudflareAccessHost reports whether host is a Cloudflare Access team +// hostname (*.cloudflareaccess.com). Used by GetAppInfo to verify that a +// response carrying CF-Access-Domain / CF-Access-Aud values actually came +// from the Cloudflare Access edge rather than from whatever host the user +// originally invoked cloudflared against. +func isCloudflareAccessHost(host string) bool { + // Drop a trailing port if present. + if i := strings.IndexByte(host, ':'); i >= 0 { + host = host[:i] + } + host = strings.ToLower(host) + return strings.HasSuffix(host, cloudflareAccessSuffix) && len(host) > len(cloudflareAccessSuffix) +} + var ( userAgent = "DEV" signatureAlgs = []jose.SignatureAlgorithm{jose.RS256} @@ -385,6 +400,15 @@ func GetAppInfo(reqURL *url.URL) (*AppInfo, error) { var aud string location := resp.Request.URL if strings.Contains(location.Path, AccessLoginWorkerPath) { + // The redirect chain stopped at a URL whose path matches the + // AccessLoginWorkerPath substring. That predicate alone does not + // guarantee the response actually came from Cloudflare — any host can + // expose a /cdn-cgi/access/login path. Verify the host belongs to the + // Cloudflare Access edge (*.cloudflareaccess.com) before treating + // CF-Access-Domain and the ?kid= query parameter as authoritative. + if !isCloudflareAccessHost(location.Hostname()) { + return nil, fmt.Errorf("AppInfo redirect endpoint served by non-Cloudflare host %q; refusing to trust response headers", location.Hostname()) + } aud = resp.Request.URL.Query().Get("kid") if aud == "" { return nil, errors.New("Empty app aud") diff --git a/token/token_test.go b/token/token_test.go index da92ed73a02..facd538094c 100644 --- a/token/token_test.go +++ b/token/token_test.go @@ -3,7 +3,9 @@ package token import ( "encoding/json" "net/http" + "net/http/httptest" "net/url" + "strings" "testing" ) @@ -133,3 +135,62 @@ func TestJwtPayloadUnmarshal_FailsWhenAudIsOmitted(t *testing.T) { t.Errorf("Expected %v, got %v", wantErr, err) } } + +func TestIsCloudflareAccessHost(t *testing.T) { + cases := []struct { + host string + want bool + }{ + {"team.cloudflareaccess.com", true}, + {"team.cloudflareaccess.com:8443", true}, + {"TEAM.cloudflareaccess.com", true}, + {"sub.team.cloudflareaccess.com", true}, + {"cloudflareaccess.com", false}, + {".cloudflareaccess.com", false}, + {"attacker.com", false}, + {"127.0.0.1", false}, + {"127.0.0.1:8080", false}, + {"cloudflareaccess.com.attacker.com", false}, + {"team-cloudflareaccess.com", false}, + {"", false}, + } + for _, c := range cases { + if got := isCloudflareAccessHost(c.host); got != c.want { + t.Errorf("isCloudflareAccessHost(%q) = %v, want %v", c.host, got, c.want) + } + } +} + +func TestGetAppInfo_RejectsRedirectFromNonCloudflareHost(t *testing.T) { + // Stand up a server that mimics the attacker shape: a HEAD to / + // redirects to /cdn-cgi/access/login?kid=AUD on the same host, which + // returns 200 with CF-Access-Domain set. Without host validation, the + // (AppDomain, AppAUD) fields would come from this attacker-controlled + // response. With host validation, GetAppInfo must refuse it. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/" { + http.Redirect(w, r, "/cdn-cgi/access/login?kid=AUD123", http.StatusFound) + return + } + if strings.Contains(r.URL.Path, AccessLoginWorkerPath) { + w.Header().Set(appDomainHeader, "victim-app.example.com") + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + reqURL, err := url.Parse(srv.URL + "/") + if err != nil { + t.Fatalf("parse server URL: %v", err) + } + + appInfo, err := GetAppInfo(reqURL) + if err == nil { + t.Fatalf("GetAppInfo accepted response from non-Cloudflare host; got appInfo=%+v", appInfo) + } + if !strings.Contains(err.Error(), "non-Cloudflare host") { + t.Fatalf("expected non-Cloudflare host error, got: %v", err) + } +}