diff --git a/backend/cmd/headlamp.go b/backend/cmd/headlamp.go index 6e7c7f70863..1105bc8ec3d 100644 --- a/backend/cmd/headlamp.go +++ b/backend/cmd/headlamp.go @@ -80,11 +80,16 @@ import ( type HeadlampConfig struct { *headlampconfig.HeadlampConfig proxyURLMu sync.Mutex - compiledProxyURLs []glob.Glob + compiledProxyURLs []proxyURLAllowlistEntry } -func compileProxyURLPatterns(patterns []string) ([]glob.Glob, error) { - compiledProxyURLs := make([]glob.Glob, 0, len(patterns)) +type proxyURLAllowlistEntry struct { + pattern string + matcher glob.Glob +} + +func compileProxyURLPatterns(patterns []string) ([]proxyURLAllowlistEntry, error) { + compiledProxyURLs := make([]proxyURLAllowlistEntry, 0, len(patterns)) for _, pattern := range patterns { g, err := glob.Compile(pattern) @@ -92,7 +97,10 @@ func compileProxyURLPatterns(patterns []string) ([]glob.Glob, error) { return nil, fmt.Errorf("compiling proxy URL pattern %q: %w", pattern, err) } - compiledProxyURLs = append(compiledProxyURLs, g) + compiledProxyURLs = append(compiledProxyURLs, proxyURLAllowlistEntry{ + pattern: pattern, + matcher: g, + }) } return compiledProxyURLs, nil @@ -115,13 +123,19 @@ func (c *HeadlampConfig) proxyURLAllowed(proxyURL string) (bool, error) { compiledProxyURLs := c.compiledProxyURLs c.proxyURLMu.Unlock() - for _, g := range compiledProxyURLs { - if g.Match(proxyURL) { - return true, nil + _, allowed := matchProxyURLAllowlist(proxyURL, compiledProxyURLs) + + return allowed, nil +} + +func matchProxyURLAllowlist(proxyURL string, allowlist []proxyURLAllowlistEntry) (string, bool) { + for _, entry := range allowlist { + if entry.matcher.Match(proxyURL) { + return entry.pattern, true } } - return false, nil + return "", false } const DrainNodeCacheTTL = 20 // seconds @@ -151,6 +165,30 @@ var maxProxyResponseSize int64 = 100 * 1024 * 1024 //nolint:gochecknoglobals // allow test override var externalProxyTimeout = 30 * time.Second +type proxyURLListContextKey struct{} + +//nolint:gochecknoglobals // shared client preserves connection pooling for external proxy requests +var externalProxyHTTPClient = &http.Client{ + Transport: newExternalProxyTransport(), + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects while requesting %q", req.URL.Redacted()) + } + + proxyURLAllowlist, ok := req.Context().Value(proxyURLListContextKey{}).([]proxyURLAllowlistEntry) + if !ok || len(proxyURLAllowlist) == 0 { + // If not an external proxy request or no allowlist provided, do not follow redirects + return http.ErrUseLastResponse + } + + if _, ok := matchProxyURLAllowlist(req.URL.String(), proxyURLAllowlist); !ok { + return fmt.Errorf("redirect target %q not in allowlist", req.URL.Redacted()) + } + + return nil + }, +} + const kubeConfigSource = "kubeconfig" // source for kubeconfig contexts const ( @@ -551,6 +589,17 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han logger.Log(logger.LevelInfo, nil, nil, "Dynamic clusters support: "+fmt.Sprint(config.EnableDynamicClusters)) logger.Log(logger.LevelInfo, nil, nil, "Helm support: "+fmt.Sprint(config.EnableHelm)) logger.Log(logger.LevelInfo, nil, nil, "Proxy URLs: "+fmt.Sprint(config.ProxyURLs)) + proxyURLAllowlist := config.compiledProxyURLs + if proxyURLAllowlist == nil { + var err error + + proxyURLAllowlist, err = compileProxyURLPatterns(config.ProxyURLs) + if err != nil { + panic(err) + } + + config.compiledProxyURLs = proxyURLAllowlist + } logger.Log(logger.LevelInfo, nil, nil, "TLS certificate path: "+config.TLSCertPath) logger.Log(logger.LevelInfo, nil, nil, "TLS key path: "+config.TLSKeyPath) logger.Log(logger.LevelInfo, nil, nil, "me Username Paths: "+config.MeUsernamePaths) @@ -695,7 +744,7 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han return } - url, err := url.Parse(proxyURL) + targetURL, err := url.Parse(proxyURL) if err != nil { logger.Log(logger.LevelError, map[string]string{"proxyURL": proxyURL}, err, "The provided proxy URL is invalid") @@ -704,22 +753,17 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han return } - isURLContainedInProxyURLs, err := config.proxyURLAllowed(url.String()) - if err != nil { - logger.Log(logger.LevelError, map[string]string{"proxyURL": proxyURL}, err, "compiling proxy URL patterns") - http.Error(w, "failed to compile proxy URL patterns", http.StatusInternalServerError) - - return - } - - if !isURLContainedInProxyURLs { - logger.Log(logger.LevelError, nil, err, "no allowed proxy url match, request denied") + if _, ok := matchProxyURLAllowlist(targetURL.String(), proxyURLAllowlist); !ok { + logger.Log(logger.LevelError, map[string]string{"proxyURL": targetURL.Redacted()}, + err, "no allowed proxy url match, request denied") http.Error(w, "no allowed proxy url match, request denied ", http.StatusBadRequest) return } - proxyCtx, cancel := context.WithTimeout(r.Context(), externalProxyTimeout) + proxyCtx := context.WithValue(r.Context(), proxyURLListContextKey{}, proxyURLAllowlist) + + proxyCtx, cancel := context.WithTimeout(proxyCtx, externalProxyTimeout) defer cancel() proxyReq, err := http.NewRequestWithContext(proxyCtx, r.Method, proxyURL, r.Body) //nolint:gosec @@ -732,7 +776,17 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han // We may want to filter some headers, otherwise we could just use a shallow copy proxyReq.Header = make(http.Header) + connectionHeaderTokens := externalProxyConnectionHeaderTokens(r.Header) + for h, val := range r.Header { + if shouldFilterExternalProxyRequestHeader(h) { + continue + } + + if _, ok := connectionHeaderTokens[strings.ToLower(h)]; ok { + continue + } + proxyReq.Header[h] = val } @@ -742,12 +796,10 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han w.Header().Set("Pragma", "no-cache") w.Header().Set("X-Accel-Expires", "0") - client := http.Client{} - - resp, err := client.Do(proxyReq) //nolint:gosec + resp, err := externalProxyHTTPClient.Do(proxyReq) //nolint:gosec if err != nil { logger.Log(logger.LevelError, nil, err, "making request") - http.Error(w, err.Error(), http.StatusBadGateway) + http.Error(w, "external proxy request failed", http.StatusBadGateway) return } @@ -796,6 +848,15 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han return } + // Prevent unexpected or unfollowed redirects from being returned or breaking the proxy. + if resp.StatusCode >= 300 && resp.StatusCode <= 399 && resp.StatusCode != http.StatusNotModified { + logger.Log(logger.LevelError, nil, nil, + fmt.Sprintf("proxy response returned an unexpected or unfollowed redirect (status %d)", resp.StatusCode)) + http.Error(w, "external proxy request failed: unexpected redirect", http.StatusBadGateway) + + return + } + w.WriteHeader(resp.StatusCode) // Stream the body, putting the already-read first chunk back in front of @@ -1365,6 +1426,63 @@ func isLoopbackAddr(addr string) bool { return ip != nil && ip.IsLoopback() } +func shouldFilterExternalProxyRequestHeader(headerName string) bool { + hLower := strings.ToLower(headerName) + + if hLower == "authorization" || + hLower == "cookie" || + hLower == "accept-encoding" || + hLower == "proxy-authorization" || + strings.HasPrefix(hLower, "x-headlamp-") || + strings.HasPrefix(hLower, "x-headlamp_") { + return true + } + + switch hLower { + case "connection", + "forward-to", + "keep-alive", + "proxy-connection", + "proxy-to", + "te", + "trailer", + "transfer-encoding", + "upgrade": + return true + default: + return false + } +} + +func externalProxyConnectionHeaderTokens(headers http.Header) map[string]struct{} { + tokens := map[string]struct{}{} + + for _, headerValue := range headers.Values("Connection") { + for _, token := range strings.Split(headerValue, ",") { + token = strings.ToLower(strings.TrimSpace(token)) + if token != "" { + tokens[token] = struct{}{} + } + } + } + + return tokens +} + +func newExternalProxyTransport() http.RoundTripper { + defaultTransport, ok := http.DefaultTransport.(*http.Transport) + if !ok { + return &http.Transport{ + DisableCompression: true, + } + } + + transport := defaultTransport.Clone() + transport.DisableCompression = true + + return transport +} + // allowedHosts returns the set of normalized host values that are considered // valid for the given listen address and port. All entries are lowercased and // host:port pairs are built with net.JoinHostPort so that IPv6 literals are diff --git a/backend/cmd/headlamp_test.go b/backend/cmd/headlamp_test.go index 6ce3a6c3beb..eabbf46e201 100644 --- a/backend/cmd/headlamp_test.go +++ b/backend/cmd/headlamp_test.go @@ -788,7 +788,7 @@ func TestExternalProxyTimeout(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusBadGateway, rr.Code) - assert.Contains(t, rr.Body.String(), "context deadline exceeded") + assert.Contains(t, rr.Body.String(), "external proxy request failed") } func TestDrainAndCordonNode(t *testing.T) { //nolint:funlen @@ -3541,3 +3541,269 @@ func TestExternalProxyOversizeResponseGzip(t *testing.T) { assert.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, int(maxProxyResponseSize), rr.Body.Len()) } + +func assertHeadersEmpty(t *testing.T, headers http.Header, headerNames ...string) { + t.Helper() + + for _, headerName := range headerNames { + assert.Empty(t, headers.Get(headerName), "%s header should be filtered", headerName) + } +} + +func assertHeaderPrefixAbsent(t *testing.T, headers http.Header, prefix string) { + t.Helper() + + for h := range headers { + if strings.HasPrefix(strings.ToUpper(h), prefix) { + t.Errorf("Header %s should have been filtered", h) + } + } +} + +func newExternalProxyHeaderFilteringHandler( + t *testing.T, +) (http.Handler, *url.URL, *httptest.Server, func() http.Header) { + t.Helper() + + var mu sync.Mutex + + var receivedHeaders http.Header + + proxyTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + receivedHeaders = r.Header.Clone() + mu.Unlock() + + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("OK")) + })) + + proxyURL, err := url.Parse(proxyTarget.URL) + if err != nil { + t.Fatal(err) + } + + cache := cache.New[interface{}]() + kubeConfigStore := kubeconfig.NewContextStore() + + c := &HeadlampConfig{ + HeadlampConfig: &headlampconfig.HeadlampConfig{ + HeadlampCFG: &headlampconfig.HeadlampCFG{ + UseInCluster: false, + ProxyURLs: []string{proxyURL.String()}, + KubeConfigStore: kubeConfigStore, + }, + Cache: cache, + }, + } + + receivedHeadersSnapshot := func() http.Header { + mu.Lock() + defer mu.Unlock() + + return receivedHeaders.Clone() + } + + return createHeadlampHandler(context.Background(), c), proxyURL, proxyTarget, receivedHeadersSnapshot +} + +func TestExternalProxyHeaderFiltering(t *testing.T) { + handler, proxyURL, proxyTarget, receivedHeadersSnapshot := newExternalProxyHeaderFilteringHandler(t) + defer proxyTarget.Close() + + req, err := http.NewRequestWithContext(context.Background(), "GET", "/externalproxy", nil) + if err != nil { + t.Fatal(err) + } + + // Set the proxy-to header (internal routing header that should be filtered) + req.Header.Set("proxy-to", proxyURL.String()) + req.Header.Set("Forward-to", "/some/path") + + // Set sensitive headers that should be filtered + req.Header.Set("Authorization", "Bearer sensitive-token") + req.Header.Set("Cookie", "session=sensitive-cookie") + req.Header.Set("Proxy-Authorization", "Basic sensitive-proxy-token") + // Test hyphenated X-Headlamp-* headers + req.Header.Set("X-Headlamp-Backend-Token", "sensitive-backend-token") + req.Header.Set("X-Headlamp-Custom", "sensitive-custom-header") + // Test an underscore-style X-HEADLAMP_* variant defensively as well. + req.Header.Set("X-HEADLAMP_BACKEND-TOKEN", "sensitive-underscore-token") + + // Set transport/protocol headers that should not be forwarded. + req.Header.Set("Accept-Encoding", "br, zstd") + req.Header.Set("Connection", "Upgrade, X-Connection-Only") + req.Header.Set("Keep-Alive", "timeout=5") + req.Header.Set("Proxy-Connection", "keep-alive") + req.Header.Set("TE", "trailers") + req.Header.Set("Trailer", "Expires") + req.Header.Set("Transfer-Encoding", "chunked") + req.Header.Set("Upgrade", "websocket") + req.Header.Set("X-Connection-Only", "drop-me") + + // Set a non-sensitive header that should be preserved + req.Header.Set("X-Custom-Preserve", "preserve-me") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + + receivedHeaders := receivedHeadersSnapshot() + assert.Equal(t, "preserve-me", receivedHeaders.Get("X-Custom-Preserve"), "Non-sensitive header should be preserved") + assertHeadersEmpty(t, receivedHeaders, "Authorization", "Cookie", "Proxy-Authorization") + assertHeaderPrefixAbsent(t, receivedHeaders, "X-HEADLAMP-") + assertHeadersEmpty(t, receivedHeaders, "X-HEADLAMP_BACKEND-TOKEN") + assertHeaderPrefixAbsent(t, receivedHeaders, "X-HEADLAMP_") + assertHeadersEmpty(t, receivedHeaders, "proxy-to", "Forward-to") + assertHeadersEmpty(t, receivedHeaders, + "Accept-Encoding", + "Connection", + "Keep-Alive", + "Proxy-Connection", + "TE", + "Trailer", + "Transfer-Encoding", + "Upgrade", + "X-Connection-Only", + ) +} + +func TestExternalProxyDoesNotFollowRedirects(t *testing.T) { + var mu sync.Mutex + + disallowedTargetHit := false + + disallowedTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + disallowedTargetHit = true + mu.Unlock() + })) + defer disallowedTarget.Close() + + proxyTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, disallowedTarget.URL, http.StatusFound) + })) + defer proxyTarget.Close() + + proxyURL, err := url.Parse(proxyTarget.URL) + require.NoError(t, err) + + cache := cache.New[interface{}]() + kubeConfigStore := kubeconfig.NewContextStore() + + handler := createHeadlampHandler(context.Background(), &HeadlampConfig{ + HeadlampConfig: &headlampconfig.HeadlampConfig{ + HeadlampCFG: &headlampconfig.HeadlampCFG{ + UseInCluster: false, + ProxyURLs: []string{proxyURL.String()}, + KubeConfigStore: kubeConfigStore, + }, + Cache: cache, + }, + }) + + req, err := http.NewRequestWithContext(context.Background(), "GET", "/externalproxy", nil) + require.NoError(t, err) + + req.Header.Set("proxy-to", proxyURL.String()) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusBadGateway, rr.Code) + assert.Contains(t, rr.Body.String(), "external proxy request failed") + + mu.Lock() + assert.False(t, disallowedTargetHit, "external proxy should not follow redirect to a disallowed target") + mu.Unlock() +} + +func TestExternalProxyInvalidProxyURLGlobPanics(t *testing.T) { + cache := cache.New[interface{}]() + kubeConfigStore := kubeconfig.NewContextStore() + + require.Panics(t, func() { + createHeadlampHandler(context.Background(), &HeadlampConfig{ + HeadlampConfig: &headlampconfig.HeadlampConfig{ + HeadlampCFG: &headlampconfig.HeadlampCFG{ + UseInCluster: false, + ProxyURLs: []string{"["}, + KubeConfigStore: kubeConfigStore, + }, + Cache: cache, + }, + }) + }) +} + +func TestExternalProxyRedirectErrorIncludesTargetURL(t *testing.T) { + redirectTarget := "https://redirect.example.com/disallowed" + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, redirectTarget, nil) + require.NoError(t, err) + + proxyURLAllowlist, err := compileProxyURLPatterns([]string{"https://allowed.example.com/*"}) + require.NoError(t, err) + + req = req.WithContext(context.WithValue(req.Context(), proxyURLListContextKey{}, proxyURLAllowlist)) + + err = externalProxyHTTPClient.CheckRedirect(req, []*http.Request{{}}) + + require.Error(t, err) + assert.Contains(t, err.Error(), redirectTarget) +} + +func TestExternalProxyFollowsAllowedRedirects(t *testing.T) { + var mu sync.Mutex + + allowedTargetHit := false + + allowedTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + allowedTargetHit = true + mu.Unlock() + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("OK")) + })) + defer allowedTarget.Close() + + proxyTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, allowedTarget.URL, http.StatusFound) + })) + defer proxyTarget.Close() + + proxyURL, err := url.Parse(proxyTarget.URL) + require.NoError(t, err) + + allowedURL, err := url.Parse(allowedTarget.URL) + require.NoError(t, err) + + cache := cache.New[interface{}]() + kubeConfigStore := kubeconfig.NewContextStore() + + handler := createHeadlampHandler(context.Background(), &HeadlampConfig{ + HeadlampConfig: &headlampconfig.HeadlampConfig{ + HeadlampCFG: &headlampconfig.HeadlampCFG{ + UseInCluster: false, + ProxyURLs: []string{proxyURL.String(), allowedURL.String()}, + KubeConfigStore: kubeConfigStore, + }, + Cache: cache, + }, + }) + + req, err := http.NewRequestWithContext(context.Background(), "GET", "/externalproxy", nil) + require.NoError(t, err) + + req.Header.Set("proxy-to", proxyURL.String()) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, "OK", rr.Body.String()) + + mu.Lock() + assert.True(t, allowedTargetHit, "external proxy should follow redirect to an allowed target") + mu.Unlock() +}