Skip to content

Commit 332477f

Browse files
committed
Add /v1/internal/ui/metrics-proxy API endpoint that proxies to a configured metrics provider backend.
1 parent 80910b4 commit 332477f

File tree

5 files changed

+264
-2
lines changed

5 files changed

+264
-2
lines changed

agent/http.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"regexp"
1313
"strconv"
1414
"strings"
15+
"sync/atomic"
1516
"time"
1617

1718
"github.com/NYTimes/gziphandler"
@@ -83,6 +84,7 @@ type HTTPHandlers struct {
8384
denylist *Denylist
8485
configReloaders []ConfigReloader
8586
h http.Handler
87+
metricsProxyCfg atomic.Value
8688
}
8789

8890
// 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 {
273275
if s.IsUIEnabled() {
274276
// Note that we _don't_ support reloading ui_config.{enabled, content_dir,
275277
// content_path} since this only runs at initial startup.
276-
277278
uiHandler := uiserver.NewHandler(s.agent.config, s.agent.logger.Named(logging.HTTP))
278279
s.configReloaders = append(s.configReloaders, uiHandler.ReloadConfig)
279280

@@ -295,6 +296,12 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler {
295296
),
296297
)
297298
}
299+
// Initialize (reloadable) metrics proxy config
300+
s.metricsProxyCfg.Store(s.agent.config.UIConfig.MetricsProxy)
301+
s.configReloaders = append(s.configReloaders, func(cfg *config.RuntimeConfig) error {
302+
s.metricsProxyCfg.Store(cfg.UIConfig.MetricsProxy)
303+
return nil
304+
})
298305

299306
// Wrap the whole mux with a handler that bans URLs with non-printable
300307
// characters, unless disabled explicitly to deal with old keys that fail this

agent/http_register.go

+1
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ func init() {
9494
registerEndpoint("/v1/health/service/", []string{"GET"}, (*HTTPHandlers).HealthServiceNodes)
9595
registerEndpoint("/v1/health/connect/", []string{"GET"}, (*HTTPHandlers).HealthConnectServiceNodes)
9696
registerEndpoint("/v1/health/ingress/", []string{"GET"}, (*HTTPHandlers).HealthIngressServiceNodes)
97+
registerEndpoint("/v1/internal/ui/metrics-proxy/", []string{"GET"}, (*HTTPHandlers).UIMetricsProxy)
9798
registerEndpoint("/v1/internal/ui/nodes", []string{"GET"}, (*HTTPHandlers).UINodes)
9899
registerEndpoint("/v1/internal/ui/node/", []string{"GET"}, (*HTTPHandlers).UINodeInfo)
99100
registerEndpoint("/v1/internal/ui/services", []string{"GET"}, (*HTTPHandlers).UIServices)

agent/ui_endpoint.go

+82
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@ package agent
33
import (
44
"fmt"
55
"net/http"
6+
"net/http/httputil"
7+
"net/url"
8+
"path"
69
"sort"
710
"strings"
811

912
"github.com/hashicorp/consul/agent/config"
1013
"github.com/hashicorp/consul/agent/structs"
1114
"github.com/hashicorp/consul/api"
15+
"github.com/hashicorp/consul/logging"
16+
"github.com/hashicorp/go-hclog"
1217
)
1318

1419
// ServiceSummary is used to summarize a service
@@ -524,3 +529,80 @@ func (s *HTTPHandlers) UIGatewayIntentions(resp http.ResponseWriter, req *http.R
524529

525530
return reply.Intentions, nil
526531
}
532+
533+
// UIMetricsProxy handles the /v1/internal/ui/metrics-proxy/ endpoint which, if
534+
// configured, provides a simple read-only HTTP proxy to a single metrics
535+
// backend to expose it to the UI.
536+
func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
537+
// Check the UI was enabled at agent startup (note this is not reloadable
538+
// currently).
539+
if !s.IsUIEnabled() {
540+
return nil, NotFoundError{Reason: "UI is not enabled"}
541+
}
542+
543+
// Load reloadable proxy config
544+
cfg, ok := s.metricsProxyCfg.Load().(config.UIMetricsProxy)
545+
if !ok || cfg.BaseURL == "" {
546+
// Proxy not configured
547+
return nil, NotFoundError{Reason: "Metrics proxy is not enabled"}
548+
}
549+
550+
log := s.agent.logger.Named(logging.UIMetricsProxy)
551+
552+
// Construct the new URL from the path and the base path. Note we do this here
553+
// not in the Director function below because we can handle any errors cleanly
554+
// here.
555+
556+
// Replace prefix in the path
557+
subPath := strings.TrimPrefix(req.URL.Path, "/v1/internal/ui/metrics-proxy")
558+
559+
// Append that to the BaseURL (which might contain a path prefix component)
560+
newURL := cfg.BaseURL + subPath
561+
562+
// Parse it into a new URL
563+
u, err := url.Parse(newURL)
564+
if err != nil {
565+
log.Error("couldn't parse target URL", "base_url", cfg.BaseURL, "path", subPath)
566+
return nil, BadRequestError{Reason: "Invalid path."}
567+
}
568+
569+
// Clean the new URL path to prevent path traversal attacks and remove any
570+
// double slashes etc.
571+
u.Path = path.Clean(u.Path)
572+
573+
// Validate that the full BaseURL is still a prefix - if there was a path
574+
// prefix on the BaseURL but an attacker tried to circumvent it with path
575+
// traversal then the Clean above would have resolve the /../ components back
576+
// to the actual path which means part of the prefix will now be missing.
577+
//
578+
// Note that in practice this is not currently possible since any /../ in the
579+
// path would have already been resolved by the API server mux and so not even
580+
// hit this handler. Any /../ that are far enough into the path to hit this
581+
// handler, can't backtrack far enough to eat into the BaseURL either. But we
582+
// leave this in anyway in case something changes in the future.
583+
if !strings.HasPrefix(u.String(), cfg.BaseURL) {
584+
log.Error("target URL escaped from base path",
585+
"base_url", cfg.BaseURL,
586+
"path", subPath,
587+
"target_url", u.String(),
588+
)
589+
return nil, BadRequestError{Reason: "Invalid path."}
590+
}
591+
592+
// Add any configured headers
593+
for _, h := range cfg.AddHeaders {
594+
req.Header.Set(h.Name, h.Value)
595+
}
596+
597+
proxy := httputil.ReverseProxy{
598+
Director: func(r *http.Request) {
599+
r.URL = u
600+
},
601+
ErrorLog: log.StandardLogger(&hclog.StandardLoggerOptions{
602+
InferLevels: true,
603+
}),
604+
}
605+
606+
proxy.ServeHTTP(resp, req)
607+
return nil, nil
608+
}

agent/ui_endpoint_test.go

+172-1
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@ package agent
33
import (
44
"bytes"
55
"fmt"
6-
"github.com/hashicorp/consul/sdk/testutil/retry"
76
"io"
87
"io/ioutil"
98
"net/http"
109
"net/http/httptest"
1110
"net/url"
1211
"path/filepath"
12+
"sync/atomic"
1313
"testing"
1414

15+
"github.com/hashicorp/consul/sdk/testutil/retry"
16+
1517
"github.com/hashicorp/consul/agent/config"
1618
"github.com/hashicorp/consul/agent/structs"
1719
"github.com/hashicorp/consul/api"
@@ -1522,3 +1524,172 @@ func TestUIServiceTopology(t *testing.T) {
15221524
})
15231525
})
15241526
}
1527+
1528+
func TestUIEndpoint_MetricsProxy(t *testing.T) {
1529+
t.Parallel()
1530+
1531+
var lastHeadersSent atomic.Value
1532+
1533+
backendH := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1534+
lastHeadersSent.Store(r.Header)
1535+
if r.URL.Path == "/some/prefix/ok" {
1536+
w.Header().Set("X-Random-Header", "Foo")
1537+
w.Write([]byte("OK"))
1538+
return
1539+
}
1540+
if r.URL.Path == "/.passwd" {
1541+
w.Write([]byte("SECRETS!"))
1542+
return
1543+
}
1544+
http.Error(w, "not found on backend", http.StatusNotFound)
1545+
})
1546+
1547+
backend := httptest.NewServer(backendH)
1548+
defer backend.Close()
1549+
1550+
backendURL := backend.URL + "/some/prefix"
1551+
1552+
// Share one agent for all these test cases. This has a few nice side-effects:
1553+
// 1. it's cheaper
1554+
// 2. it implicitly tests that config reloading works between cases
1555+
//
1556+
// Note we can't test the case where UI is disabled though as that's not
1557+
// reloadable so we'll do that in a separate test below rather than have many
1558+
// new tests all with a new agent. response headers also aren't reloadable
1559+
// currently due to the way we wrap API endpoints at startup.
1560+
a := NewTestAgent(t, `
1561+
ui_config {
1562+
enabled = true
1563+
}
1564+
http_config {
1565+
response_headers {
1566+
"Access-Control-Allow-Origin" = "*"
1567+
}
1568+
}
1569+
`)
1570+
defer a.Shutdown()
1571+
1572+
endpointPath := "/v1/internal/ui/metrics-proxy"
1573+
1574+
cases := []struct {
1575+
name string
1576+
config config.UIMetricsProxy
1577+
path string
1578+
wantCode int
1579+
wantContains string
1580+
wantHeaders map[string]string
1581+
wantHeadersSent map[string]string
1582+
}{
1583+
{
1584+
name: "disabled",
1585+
config: config.UIMetricsProxy{},
1586+
path: endpointPath + "/ok",
1587+
wantCode: http.StatusNotFound,
1588+
},
1589+
{
1590+
name: "basic proxying",
1591+
config: config.UIMetricsProxy{
1592+
BaseURL: backendURL,
1593+
},
1594+
path: endpointPath + "/ok",
1595+
wantCode: http.StatusOK,
1596+
wantContains: "OK",
1597+
wantHeaders: map[string]string{
1598+
"X-Random-Header": "Foo",
1599+
},
1600+
},
1601+
{
1602+
name: "404 on backend",
1603+
config: config.UIMetricsProxy{
1604+
BaseURL: backendURL,
1605+
},
1606+
path: endpointPath + "/random-path",
1607+
wantCode: http.StatusNotFound,
1608+
wantContains: "not found on backend",
1609+
},
1610+
{
1611+
// Note that this case actually doesn't exercise our validation logic at
1612+
// all since the top level API mux resolves this to /v1/internal/.passwd
1613+
// and it never hits our handler at all. I left it in though as this
1614+
// wasn't obvious and it's worth knowing if we change something in our mux
1615+
// that might affect path traversal opportunity here. In fact this makes
1616+
// our path traversal handling somewhat redundant because any traversal
1617+
// that goes "back" far enough to traverse up from the BaseURL of the
1618+
// proxy target will in fact miss our handler entirely. It's still better
1619+
// to be safe than sorry though.
1620+
name: "path traversal should fail - api mux",
1621+
config: config.UIMetricsProxy{
1622+
BaseURL: backendURL,
1623+
},
1624+
path: endpointPath + "/../../.passwd",
1625+
wantCode: http.StatusMovedPermanently,
1626+
wantContains: "Moved Permanently",
1627+
},
1628+
{
1629+
name: "adding auth header",
1630+
config: config.UIMetricsProxy{
1631+
BaseURL: backendURL,
1632+
AddHeaders: []config.UIMetricsProxyAddHeader{
1633+
{
1634+
Name: "Authorization",
1635+
Value: "SECRET_KEY",
1636+
},
1637+
{
1638+
Name: "X-Some-Other-Header",
1639+
Value: "foo",
1640+
},
1641+
},
1642+
},
1643+
path: endpointPath + "/ok",
1644+
wantCode: http.StatusOK,
1645+
wantContains: "OK",
1646+
wantHeaders: map[string]string{
1647+
"X-Random-Header": "Foo",
1648+
},
1649+
wantHeadersSent: map[string]string{
1650+
"X-Some-Other-Header": "foo",
1651+
"Authorization": "SECRET_KEY",
1652+
},
1653+
},
1654+
}
1655+
1656+
for _, tc := range cases {
1657+
tc := tc
1658+
t.Run(tc.name, func(t *testing.T) {
1659+
// Reload the agent config with the desired UI config by making a copy and
1660+
// using internal reload.
1661+
cfg := *a.Agent.config
1662+
1663+
// Modify the UIConfig part (this is a copy remember and that struct is
1664+
// not a pointer)
1665+
cfg.UIConfig.MetricsProxy = tc.config
1666+
1667+
require.NoError(t, a.Agent.reloadConfigInternal(&cfg))
1668+
1669+
// Now fetch the API handler to run requests against
1670+
h := a.srv.handler(true)
1671+
1672+
req := httptest.NewRequest("GET", tc.path, nil)
1673+
rec := httptest.NewRecorder()
1674+
1675+
h.ServeHTTP(rec, req)
1676+
1677+
require.Equal(t, tc.wantCode, rec.Code,
1678+
"Wrong status code. Body = %s", rec.Body.String())
1679+
require.Contains(t, rec.Body.String(), tc.wantContains)
1680+
for k, v := range tc.wantHeaders {
1681+
// Headers are a slice of values, just assert that one of the values is
1682+
// the one we want.
1683+
require.Contains(t, rec.Result().Header[k], v)
1684+
}
1685+
if len(tc.wantHeadersSent) > 0 {
1686+
headersSent, ok := lastHeadersSent.Load().(http.Header)
1687+
require.True(t, ok, "backend not called")
1688+
for k, v := range tc.wantHeadersSent {
1689+
require.Contains(t, headersSent[k], v,
1690+
"header %s doesn't have the right value set", k)
1691+
}
1692+
}
1693+
})
1694+
}
1695+
}

logging/names.go

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const (
5353
Transaction string = "txn"
5454
UsageMetrics string = "usage_metrics"
5555
UIServer string = "ui_server"
56+
UIMetricsProxy string = "ui_metrics_proxy"
5657
WAN string = "wan"
5758
Watch string = "watch"
5859
Vault string = "vault"

0 commit comments

Comments
 (0)