From 9f4dade8d4afe463356bc43ff0287b1ae2b8b49c Mon Sep 17 00:00:00 2001 From: Mark Youngson Date: Thu, 26 Mar 2026 01:36:22 -0600 Subject: [PATCH] fix: require auth on /api/* and /metrics endpoints - Add authWrappedMux to enforce auth on all registered handlers - Move auth setup before api.RegisterHandlers so auth applies to /api/* - Add httpAuthFile params to RegisterPrometheusHandler for /metrics auth - Add tests verifying auth is required when configured Fixes Google VRP submission for cAdvisor OT2 tier finding. --- cmd/cadvisor.go | 2 +- cmd/internal/http/handlers.go | 123 ++++++++++++++----- cmd/internal/http/handlers_test.go | 182 +++++++++++++++++++++++++++++ 3 files changed, 280 insertions(+), 27 deletions(-) create mode 100644 cmd/internal/http/handlers_test.go diff --git a/cmd/cadvisor.go b/cmd/cadvisor.go index fac72809bd..7e92cf508d 100644 --- a/cmd/cadvisor.go +++ b/cmd/cadvisor.go @@ -166,7 +166,7 @@ func main() { } // Register Prometheus collector to gather information about containers, Go runtime, processes, and machine - cadvisorhttp.RegisterPrometheusHandler(mux, resourceManager, *prometheusEndpoint, containerLabelFunc, includedMetrics) + cadvisorhttp.RegisterPrometheusHandler(mux, resourceManager, *prometheusEndpoint, containerLabelFunc, includedMetrics, *httpAuthFile, *httpAuthRealm, *httpDigestFile, *httpDigestRealm) // Start the manager. if err := resourceManager.Start(); err != nil { diff --git a/cmd/internal/http/handlers.go b/cmd/internal/http/handlers.go index 8e0713d71b..bc5cffd921 100644 --- a/cmd/internal/http/handlers.go +++ b/cmd/internal/http/handlers.go @@ -37,6 +37,50 @@ import ( "k8s.io/utils/clock" ) +// authWrappedMux wraps every registered handler with auth enforcement. +type authWrappedMux struct { + mux httpmux.Mux + wrapFunc func(http.Handler) http.Handler +} + +func (a *authWrappedMux) Handle(pattern string, h http.Handler) { + a.mux.Handle(pattern, a.wrapFunc(h)) +} + +func (a *authWrappedMux) HandleFunc(pattern string, h func(http.ResponseWriter, *http.Request)) { + a.Handle(pattern, http.HandlerFunc(h)) +} + +func (a *authWrappedMux) Handler(r *http.Request) (http.Handler, string) { + return a.mux.Handler(r) +} + +// basicAuthMiddleware returns a middleware that requires basic authentication. +func basicAuthMiddleware(authenticator *auth.BasicAuth) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if authenticator.CheckAuth(r) == "" { + authenticator.RequireAuth(w, r) + return + } + next.ServeHTTP(w, r) + }) + } +} + +// digestAuthMiddleware returns a middleware that requires digest authentication. +func digestAuthMiddleware(authenticator *auth.DigestAuth) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if authenticator.CheckAuth(r) == "" { + authenticator.RequireAuth(w, r) + return + } + next.ServeHTTP(w, r) + }) + } +} + func RegisterHandlers(mux httpmux.Mux, containerManager manager.Manager, httpAuthFile, httpAuthRealm, httpDigestFile, httpDigestRealm string, urlBasePrefix string) error { // Basic health handler. if err := healthz.RegisterHandler(mux); err != nil { @@ -51,40 +95,50 @@ func RegisterHandlers(mux httpmux.Mux, containerManager manager.Manager, httpAut } }) - // Register API handler. - if err := api.RegisterHandlers(mux, containerManager); err != nil { - return fmt.Errorf("failed to register API handlers: %s", err) - } - - // Redirect / to containers page. - mux.Handle("/", http.RedirectHandler(urlBasePrefix+pages.ContainersPage, http.StatusTemporaryRedirect)) - + // Setup authentication BEFORE registering protected handlers. + var basicAuthenticator *auth.BasicAuth + var digestAuthenticator *auth.DigestAuth + var authWrap func(http.Handler) http.Handler var authenticated bool - // Setup the authenticator object if httpAuthFile != "" { klog.V(1).Infof("Using auth file %s", httpAuthFile) secrets := auth.HtpasswdFileProvider(httpAuthFile) - authenticator := auth.NewBasicAuthenticator(httpAuthRealm, secrets) - mux.HandleFunc(static.StaticResource, authenticator.Wrap(staticHandler)) - if err := pages.RegisterHandlersBasic(mux, containerManager, authenticator, urlBasePrefix); err != nil { - return fmt.Errorf("failed to register pages auth handlers: %s", err) - } + basicAuthenticator = auth.NewBasicAuthenticator(httpAuthRealm, secrets) + authWrap = basicAuthMiddleware(basicAuthenticator) authenticated = true - } - if httpAuthFile == "" && httpDigestFile != "" { + } else if httpDigestFile != "" { klog.V(1).Infof("Using digest file %s", httpDigestFile) secrets := auth.HtdigestFileProvider(httpDigestFile) - authenticator := auth.NewDigestAuthenticator(httpDigestRealm, secrets) - mux.HandleFunc(static.StaticResource, authenticator.Wrap(staticHandler)) - if err := pages.RegisterHandlersDigest(mux, containerManager, authenticator, urlBasePrefix); err != nil { - return fmt.Errorf("failed to register pages digest handlers: %s", err) - } + digestAuthenticator = auth.NewDigestAuthenticator(httpDigestRealm, secrets) + authWrap = digestAuthMiddleware(digestAuthenticator) authenticated = true } - // Change handler based on authenticator initialization - if !authenticated { + // Register API handler with auth if configured. + apiMux := httpmux.Mux(mux) + if authWrap != nil { + apiMux = &authWrappedMux{mux: mux, wrapFunc: authWrap} + } + if err := api.RegisterHandlers(apiMux, containerManager); err != nil { + return fmt.Errorf("failed to register API handlers: %s", err) + } + + // Redirect / to containers page. + mux.Handle("/", http.RedirectHandler(urlBasePrefix+pages.ContainersPage, http.StatusTemporaryRedirect)) + + // Register pages and static resources with auth if configured. + if basicAuthenticator != nil { + mux.HandleFunc(static.StaticResource, basicAuthenticator.Wrap(staticHandler)) + if err := pages.RegisterHandlersBasic(mux, containerManager, basicAuthenticator, urlBasePrefix); err != nil { + return fmt.Errorf("failed to register pages auth handlers: %s", err) + } + } else if digestAuthenticator != nil { + mux.HandleFunc(static.StaticResource, digestAuthenticator.Wrap(staticHandler)) + if err := pages.RegisterHandlersDigest(mux, containerManager, digestAuthenticator, urlBasePrefix); err != nil { + return fmt.Errorf("failed to register pages digest handlers: %s", err) + } + } else { mux.HandleFunc(static.StaticResource, staticHandlerNoAuth) if err := pages.RegisterHandlersBasic(mux, containerManager, nil, urlBasePrefix); err != nil { return fmt.Errorf("failed to register pages handlers: %s", err) @@ -96,13 +150,16 @@ func RegisterHandlers(mux httpmux.Mux, containerManager manager.Manager, httpAut // RegisterPrometheusHandler creates a new PrometheusCollector and configures // the provided HTTP mux to handle the given Prometheus endpoint. +// If auth is configured, the Prometheus endpoint requires authentication. func RegisterPrometheusHandler(mux httpmux.Mux, resourceManager manager.Manager, prometheusEndpoint string, - f metrics.ContainerLabelsFunc, includedMetrics container.MetricSet) { + f metrics.ContainerLabelsFunc, includedMetrics container.MetricSet, + httpAuthFile, httpAuthRealm, httpDigestFile, httpDigestRealm string) { + goCollector := collectors.NewGoCollector() processCollector := collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}) machineCollector := metrics.NewPrometheusMachineCollector(resourceManager, includedMetrics) - mux.Handle(prometheusEndpoint, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + prometheusHandler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { opts, err := api.GetRequestOptions(req) if err != nil { http.Error(w, "No metrics gathered, last error:\n\n"+err.Error(), http.StatusInternalServerError) @@ -119,7 +176,21 @@ func RegisterPrometheusHandler(mux httpmux.Mux, resourceManager manager.Manager, processCollector, ) promhttp.HandlerFor(r, promhttp.HandlerOpts{ErrorHandling: promhttp.ContinueOnError}).ServeHTTP(w, req) - })) + }) + + // Wrap with authentication if configured. + finalHandler := http.Handler(prometheusHandler) + if httpAuthFile != "" { + secrets := auth.HtpasswdFileProvider(httpAuthFile) + authenticator := auth.NewBasicAuthenticator(httpAuthRealm, secrets) + finalHandler = basicAuthMiddleware(authenticator)(finalHandler) + } else if httpDigestFile != "" { + secrets := auth.HtdigestFileProvider(httpDigestFile) + authenticator := auth.NewDigestAuthenticator(httpDigestRealm, secrets) + finalHandler = digestAuthMiddleware(authenticator)(finalHandler) + } + + mux.Handle(prometheusEndpoint, finalHandler) } func staticHandlerNoAuth(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/internal/http/handlers_test.go b/cmd/internal/http/handlers_test.go new file mode 100644 index 0000000000..efc2262910 --- /dev/null +++ b/cmd/internal/http/handlers_test.go @@ -0,0 +1,182 @@ +// Copyright 2015 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package http + +import ( + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/google/cadvisor/container" + "github.com/google/cadvisor/manager" +) + +// mockManager implements a minimal manager.Manager for testing. +type mockManager struct{} + +func (m *mockManager) Start() error { return nil } +func (m *mockManager) Stop() error { return nil } + +// TestAPIRequiresAuthWhenConfigured verifies that /api/ routes require authentication +// when an auth file is configured. +func TestAPIRequiresAuthWhenConfigured(t *testing.T) { + // Create a temporary htpasswd file with test credentials (user:pass = test:test) + // Format: username:hashed_password (using htpasswd -c) + // "test" hashed with default crypt: $apr1$6RXm1v7P$G3XC9xGP1MaQrV/Y.OVIB1 (example hash) + tmpFile, err := ioutil.TempFile("", "htpasswd") + if err != nil { + t.Fatalf("Failed to create temp file: %v", err) + } + defer os.Remove(tmpFile.Name()) + + // Write a simple htpasswd entry (user:password with plaintext for testing) + // Actually, go-http-auth reads htpasswd format. We'll use a basic entry. + // This is user "testuser" with password "testpass" (apache format: user:$apr1$...$...) + // For simplicity, we create a file that the library can parse. + content := "testuser:$apr1$6RXm1v7P$G3XC9xGP1MaQrV/Y.OVIB1\n" + if _, err := tmpFile.WriteString(content); err != nil { + t.Fatalf("Failed to write to temp file: %v", err) + } + tmpFile.Close() + + // Create a test mux + mux := http.NewServeMux() + mockMgr := &mockManager{} + + // Register handlers with auth file + err = RegisterHandlers(mux, mockMgr, tmpFile.Name(), "TestRealm", "", "", "") + if err != nil { + t.Fatalf("Failed to register handlers: %v", err) + } + + // Create a test server + server := httptest.NewServer(mux) + defer server.Close() + + // Test 1: Request to /api/ without credentials should return 401 + resp, err := http.Get(server.URL + "/api/") + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("GET /api/ without auth: expected %d, got %d", http.StatusUnauthorized, resp.StatusCode) + } + + // Test 2: Health check should NOT require auth + resp, err = http.Get(server.URL + "/healthz") + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusUnauthorized { + t.Errorf("GET /healthz should not require auth, got %d", resp.StatusCode) + } +} + +// TestPrometheusRequiresAuthWhenConfigured verifies that Prometheus endpoint +// requires authentication when an auth file is configured. +func TestPrometheusRequiresAuthWhenConfigured(t *testing.T) { + // Create a temporary htpasswd file + tmpFile, err := ioutil.TempFile("", "htpasswd") + if err != nil { + t.Fatalf("Failed to create temp file: %v", err) + } + defer os.Remove(tmpFile.Name()) + + content := "testuser:$apr1$6RXm1v7P$G3XC9xGP1MaQrV/Y.OVIB1\n" + if _, err := tmpFile.WriteString(content); err != nil { + t.Fatalf("Failed to write to temp file: %v", err) + } + tmpFile.Close() + + // Create a test mux + mux := http.NewServeMux() + mockMgr := &mockManager{} + + prometheusEndpoint := "/metrics" + labelFunc := func(containerName string) map[string]string { return map[string]string{} } + includedMetrics := container.MetricSet{} + + // Register Prometheus handler with auth + RegisterPrometheusHandler(mux, mockMgr, prometheusEndpoint, labelFunc, includedMetrics, + tmpFile.Name(), "TestRealm", "", "") + + // Create a test server + server := httptest.NewServer(mux) + defer server.Close() + + // Request to /metrics without credentials should return 401 + resp, err := http.Get(server.URL + prometheusEndpoint) + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("GET %s without auth: expected %d, got %d", prometheusEndpoint, http.StatusUnauthorized, resp.StatusCode) + } +} + +// TestHandlersWithoutAuth verifies that API and Prometheus handlers work without auth. +func TestHandlersWithoutAuth(t *testing.T) { + // Create a test mux without auth + mux := http.NewServeMux() + mockMgr := &mockManager{} + + // Register handlers WITHOUT auth file + err := RegisterHandlers(mux, mockMgr, "", "", "", "", "") + if err != nil { + t.Fatalf("Failed to register handlers: %v", err) + } + + prometheusEndpoint := "/metrics" + labelFunc := func(containerName string) map[string]string { return map[string]string{} } + includedMetrics := container.MetricSet{} + + // Register Prometheus handler WITHOUT auth + RegisterPrometheusHandler(mux, mockMgr, prometheusEndpoint, labelFunc, includedMetrics, "", "", "", "") + + // Create a test server + server := httptest.NewServer(mux) + defer server.Close() + + // Request to /api/ without auth configured should NOT return 401 + resp, err := http.Get(server.URL + "/api/") + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusUnauthorized { + t.Errorf("GET /api/ without auth configured should not return 401, got %d", resp.StatusCode) + } + + // Request to /metrics without auth configured should NOT return 401 + resp, err = http.Get(server.URL + prometheusEndpoint) + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusUnauthorized { + t.Errorf("GET %s without auth configured should not return 401, got %d", prometheusEndpoint, resp.StatusCode) + } +}