diff --git a/agent/http.go b/agent/http.go index 0946d8ca5..a3c623170 100644 --- a/agent/http.go +++ b/agent/http.go @@ -12,6 +12,7 @@ import ( "regexp" "strconv" "strings" + "sync/atomic" "time" "github.com/NYTimes/gziphandler" @@ -83,6 +84,7 @@ type HTTPHandlers struct { denylist *Denylist configReloaders []ConfigReloader h http.Handler + metricsProxyCfg atomic.Value } // endpoint is a Consul-specific HTTP handler that takes the usual arguments in @@ -273,7 +275,6 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler { if s.IsUIEnabled() { // Note that we _don't_ support reloading ui_config.{enabled, content_dir, // content_path} since this only runs at initial startup. - uiHandler := uiserver.NewHandler(s.agent.config, s.agent.logger.Named(logging.HTTP)) s.configReloaders = append(s.configReloaders, uiHandler.ReloadConfig) @@ -295,6 +296,12 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler { ), ) } + // Initialize (reloadable) metrics proxy config + s.metricsProxyCfg.Store(s.agent.config.UIConfig.MetricsProxy) + s.configReloaders = append(s.configReloaders, func(cfg *config.RuntimeConfig) error { + s.metricsProxyCfg.Store(cfg.UIConfig.MetricsProxy) + return nil + }) // Wrap the whole mux with a handler that bans URLs with non-printable // characters, unless disabled explicitly to deal with old keys that fail this diff --git a/agent/http_register.go b/agent/http_register.go index 48ef8e4e4..f4c526c28 100644 --- a/agent/http_register.go +++ b/agent/http_register.go @@ -94,6 +94,7 @@ func init() { registerEndpoint("/v1/health/service/", []string{"GET"}, (*HTTPHandlers).HealthServiceNodes) registerEndpoint("/v1/health/connect/", []string{"GET"}, (*HTTPHandlers).HealthConnectServiceNodes) registerEndpoint("/v1/health/ingress/", []string{"GET"}, (*HTTPHandlers).HealthIngressServiceNodes) + registerEndpoint("/v1/internal/ui/metrics-proxy/", []string{"GET"}, (*HTTPHandlers).UIMetricsProxy) registerEndpoint("/v1/internal/ui/nodes", []string{"GET"}, (*HTTPHandlers).UINodes) registerEndpoint("/v1/internal/ui/node/", []string{"GET"}, (*HTTPHandlers).UINodeInfo) registerEndpoint("/v1/internal/ui/services", []string{"GET"}, (*HTTPHandlers).UIServices) diff --git a/agent/ui_endpoint.go b/agent/ui_endpoint.go index 5802c7488..2625d1f45 100644 --- a/agent/ui_endpoint.go +++ b/agent/ui_endpoint.go @@ -3,12 +3,17 @@ package agent import ( "fmt" "net/http" + "net/http/httputil" + "net/url" + "path" "sort" "strings" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/logging" + "github.com/hashicorp/go-hclog" ) // ServiceSummary is used to summarize a service @@ -524,3 +529,80 @@ func (s *HTTPHandlers) UIGatewayIntentions(resp http.ResponseWriter, req *http.R return reply.Intentions, nil } + +// UIMetricsProxy handles the /v1/internal/ui/metrics-proxy/ endpoint which, if +// configured, provides a simple read-only HTTP proxy to a single metrics +// backend to expose it to the UI. +func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // Check the UI was enabled at agent startup (note this is not reloadable + // currently). + if !s.IsUIEnabled() { + return nil, NotFoundError{Reason: "UI is not enabled"} + } + + // Load reloadable proxy config + cfg, ok := s.metricsProxyCfg.Load().(config.UIMetricsProxy) + if !ok || cfg.BaseURL == "" { + // Proxy not configured + return nil, NotFoundError{Reason: "Metrics proxy is not enabled"} + } + + log := s.agent.logger.Named(logging.UIMetricsProxy) + + // Construct the new URL from the path and the base path. Note we do this here + // not in the Director function below because we can handle any errors cleanly + // here. + + // Replace prefix in the path + subPath := strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/metrics-proxy") + + // Append that to the BaseURL (which might contain a path prefix component) + newURL := cfg.BaseURL + subPath + + // Parse it into a new URL + u, err := url.Parse(newURL) + if err != nil { + log.Error("couldn't parse target URL", "base_url", cfg.BaseURL, "path", subPath) + return nil, BadRequestError{Reason: "Invalid path."} + } + + // Clean the new URL path to prevent path traversal attacks and remove any + // double slashes etc. + u.Path = path.Clean(u.Path) + + // Validate that the full BaseURL is still a prefix - if there was a path + // prefix on the BaseURL but an attacker tried to circumvent it with path + // traversal then the Clean above would have resolve the /../ components back + // to the actual path which means part of the prefix will now be missing. + // + // Note that in practice this is not currently possible since any /../ in the + // path would have already been resolved by the API server mux and so not even + // hit this handler. Any /../ that are far enough into the path to hit this + // handler, can't backtrack far enough to eat into the BaseURL either. But we + // leave this in anyway in case something changes in the future. + if !strings.HasPrefix(u.String(), cfg.BaseURL) { + log.Error("target URL escaped from base path", + "base_url", cfg.BaseURL, + "path", subPath, + "target_url", u.String(), + ) + return nil, BadRequestError{Reason: "Invalid path."} + } + + // Add any configured headers + for _, h := range cfg.AddHeaders { + req.Header.Set(h.Name, h.Value) + } + + proxy := httputil.ReverseProxy{ + Director: func(r *http.Request) { + r.URL = u + }, + ErrorLog: log.StandardLogger(&hclog.StandardLoggerOptions{ + InferLevels: true, + }), + } + + proxy.ServeHTTP(resp, req) + return nil, nil +} diff --git a/agent/ui_endpoint_test.go b/agent/ui_endpoint_test.go index 07b13a579..01705a23b 100644 --- a/agent/ui_endpoint_test.go +++ b/agent/ui_endpoint_test.go @@ -3,15 +3,17 @@ package agent import ( "bytes" "fmt" - "github.com/hashicorp/consul/sdk/testutil/retry" "io" "io/ioutil" "net/http" "net/http/httptest" "net/url" "path/filepath" + "sync/atomic" "testing" + "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -1522,3 +1524,172 @@ func TestUIServiceTopology(t *testing.T) { }) }) } + +func TestUIEndpoint_MetricsProxy(t *testing.T) { + t.Parallel() + + var lastHeadersSent atomic.Value + + backendH := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastHeadersSent.Store(r.Header) + if r.URL.Path == "/some/prefix/ok" { + w.Header().Set("X-Random-Header", "Foo") + w.Write([]byte("OK")) + return + } + if r.URL.Path == "/.passwd" { + w.Write([]byte("SECRETS!")) + return + } + http.Error(w, "not found on backend", http.StatusNotFound) + }) + + backend := httptest.NewServer(backendH) + defer backend.Close() + + backendURL := backend.URL + "/some/prefix" + + // Share one agent for all these test cases. This has a few nice side-effects: + // 1. it's cheaper + // 2. it implicitly tests that config reloading works between cases + // + // Note we can't test the case where UI is disabled though as that's not + // reloadable so we'll do that in a separate test below rather than have many + // new tests all with a new agent. response headers also aren't reloadable + // currently due to the way we wrap API endpoints at startup. + a := NewTestAgent(t, ` + ui_config { + enabled = true + } + http_config { + response_headers { + "Access-Control-Allow-Origin" = "*" + } + } + `) + defer a.Shutdown() + + endpointPath := "/v1/internal/ui/metrics-proxy" + + cases := []struct { + name string + config config.UIMetricsProxy + path string + wantCode int + wantContains string + wantHeaders map[string]string + wantHeadersSent map[string]string + }{ + { + name: "disabled", + config: config.UIMetricsProxy{}, + path: endpointPath + "/ok", + wantCode: http.StatusNotFound, + }, + { + name: "basic proxying", + config: config.UIMetricsProxy{ + BaseURL: backendURL, + }, + path: endpointPath + "/ok", + wantCode: http.StatusOK, + wantContains: "OK", + wantHeaders: map[string]string{ + "X-Random-Header": "Foo", + }, + }, + { + name: "404 on backend", + config: config.UIMetricsProxy{ + BaseURL: backendURL, + }, + path: endpointPath + "/random-path", + wantCode: http.StatusNotFound, + wantContains: "not found on backend", + }, + { + // Note that this case actually doesn't exercise our validation logic at + // all since the top level API mux resolves this to /v1/internal/.passwd + // and it never hits our handler at all. I left it in though as this + // wasn't obvious and it's worth knowing if we change something in our mux + // that might affect path traversal opportunity here. In fact this makes + // our path traversal handling somewhat redundant because any traversal + // that goes "back" far enough to traverse up from the BaseURL of the + // proxy target will in fact miss our handler entirely. It's still better + // to be safe than sorry though. + name: "path traversal should fail - api mux", + config: config.UIMetricsProxy{ + BaseURL: backendURL, + }, + path: endpointPath + "/../../.passwd", + wantCode: http.StatusMovedPermanently, + wantContains: "Moved Permanently", + }, + { + name: "adding auth header", + config: config.UIMetricsProxy{ + BaseURL: backendURL, + AddHeaders: []config.UIMetricsProxyAddHeader{ + { + Name: "Authorization", + Value: "SECRET_KEY", + }, + { + Name: "X-Some-Other-Header", + Value: "foo", + }, + }, + }, + path: endpointPath + "/ok", + wantCode: http.StatusOK, + wantContains: "OK", + wantHeaders: map[string]string{ + "X-Random-Header": "Foo", + }, + wantHeadersSent: map[string]string{ + "X-Some-Other-Header": "foo", + "Authorization": "SECRET_KEY", + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + // Reload the agent config with the desired UI config by making a copy and + // using internal reload. + cfg := *a.Agent.config + + // Modify the UIConfig part (this is a copy remember and that struct is + // not a pointer) + cfg.UIConfig.MetricsProxy = tc.config + + require.NoError(t, a.Agent.reloadConfigInternal(&cfg)) + + // Now fetch the API handler to run requests against + h := a.srv.handler(true) + + req := httptest.NewRequest("GET", tc.path, nil) + rec := httptest.NewRecorder() + + h.ServeHTTP(rec, req) + + require.Equal(t, tc.wantCode, rec.Code, + "Wrong status code. Body = %s", rec.Body.String()) + require.Contains(t, rec.Body.String(), tc.wantContains) + for k, v := range tc.wantHeaders { + // Headers are a slice of values, just assert that one of the values is + // the one we want. + require.Contains(t, rec.Result().Header[k], v) + } + if len(tc.wantHeadersSent) > 0 { + headersSent, ok := lastHeadersSent.Load().(http.Header) + require.True(t, ok, "backend not called") + for k, v := range tc.wantHeadersSent { + require.Contains(t, headersSent[k], v, + "header %s doesn't have the right value set", k) + } + } + }) + } +} diff --git a/logging/names.go b/logging/names.go index f8652e7e1..544990061 100644 --- a/logging/names.go +++ b/logging/names.go @@ -53,6 +53,7 @@ const ( Transaction string = "txn" UsageMetrics string = "usage_metrics" UIServer string = "ui_server" + UIMetricsProxy string = "ui_metrics_proxy" WAN string = "wan" Watch string = "watch" Vault string = "vault"