mirror of https://github.com/status-im/consul.git
agent: introduce path allow list for requests going through the metrics proxy (#9059)
Added a new option `ui_config.metrics_proxy.path_allowlist`. This defaults to `["/api/v1/query", "/api/v1/query_range"]` when the metrics provider is set to `prometheus`. Requests that do not use one of the allow-listed paths (via exact match) get a 403 Forbidden response instead.
This commit is contained in:
parent
26bb51e337
commit
d6e7bf40f8
|
@ -9,6 +9,7 @@ import (
|
||||||
"net"
|
"net"
|
||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
|
"path"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"reflect"
|
"reflect"
|
||||||
"regexp"
|
"regexp"
|
||||||
|
@ -1102,6 +1103,16 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
|
||||||
return RuntimeConfig{}, fmt.Errorf("cache.entry_fetch_rate must be strictly positive, was: %v", rt.Cache.EntryFetchRate)
|
return RuntimeConfig{}, fmt.Errorf("cache.entry_fetch_rate must be strictly positive, was: %v", rt.Cache.EntryFetchRate)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if rt.UIConfig.MetricsProvider == "prometheus" {
|
||||||
|
// Handle defaulting for the built-in version of prometheus.
|
||||||
|
if len(rt.UIConfig.MetricsProxy.PathAllowlist) == 0 {
|
||||||
|
rt.UIConfig.MetricsProxy.PathAllowlist = []string{
|
||||||
|
"/api/v1/query",
|
||||||
|
"/api/v1/query_range",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if err := b.BuildEnterpriseRuntimeConfig(&rt, &c); err != nil {
|
if err := b.BuildEnterpriseRuntimeConfig(&rt, &c); err != nil {
|
||||||
return rt, err
|
return rt, err
|
||||||
}
|
}
|
||||||
|
@ -1180,6 +1191,11 @@ func (b *Builder) Validate(rt RuntimeConfig) error {
|
||||||
rt.UIConfig.MetricsProxy.BaseURL)
|
rt.UIConfig.MetricsProxy.BaseURL)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
for _, allowedPath := range rt.UIConfig.MetricsProxy.PathAllowlist {
|
||||||
|
if err := validateAbsoluteURLPath(allowedPath); err != nil {
|
||||||
|
return fmt.Errorf("ui_config.metrics_proxy.path_allowlist: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
for k, v := range rt.UIConfig.DashboardURLTemplates {
|
for k, v := range rt.UIConfig.DashboardURLTemplates {
|
||||||
if err := validateBasicName("ui_config.dashboard_url_templates key names", k, false); err != nil {
|
if err := validateBasicName("ui_config.dashboard_url_templates key names", k, false); err != nil {
|
||||||
return err
|
return err
|
||||||
|
@ -1746,8 +1762,9 @@ func (b *Builder) uiMetricsProxyVal(v RawUIMetricsProxy) UIMetricsProxy {
|
||||||
}
|
}
|
||||||
|
|
||||||
return UIMetricsProxy{
|
return UIMetricsProxy{
|
||||||
BaseURL: b.stringVal(v.BaseURL),
|
BaseURL: b.stringVal(v.BaseURL),
|
||||||
AddHeaders: hdrs,
|
AddHeaders: hdrs,
|
||||||
|
PathAllowlist: v.PathAllowlist,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2325,3 +2342,24 @@ func validateRemoteScriptsChecks(conf RuntimeConfig) error {
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func validateAbsoluteURLPath(p string) error {
|
||||||
|
if !path.IsAbs(p) {
|
||||||
|
return fmt.Errorf("path %q is not an absolute path", p)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A bit more extra validation that these are actually paths.
|
||||||
|
u, err := url.Parse(p)
|
||||||
|
if err != nil ||
|
||||||
|
u.Scheme != "" ||
|
||||||
|
u.Opaque != "" ||
|
||||||
|
u.User != nil ||
|
||||||
|
u.Host != "" ||
|
||||||
|
u.RawQuery != "" ||
|
||||||
|
u.Fragment != "" ||
|
||||||
|
u.Path != p {
|
||||||
|
return fmt.Errorf("path %q is not an absolute path", p)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
|
@ -797,8 +797,9 @@ type RawUIConfig struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
type RawUIMetricsProxy struct {
|
type RawUIMetricsProxy struct {
|
||||||
BaseURL *string `json:"base_url,omitempty" hcl:"base_url" mapstructure:"base_url"`
|
BaseURL *string `json:"base_url,omitempty" hcl:"base_url" mapstructure:"base_url"`
|
||||||
AddHeaders []RawUIMetricsProxyAddHeader `json:"add_headers,omitempty" hcl:"add_headers" mapstructure:"add_headers"`
|
AddHeaders []RawUIMetricsProxyAddHeader `json:"add_headers,omitempty" hcl:"add_headers" mapstructure:"add_headers"`
|
||||||
|
PathAllowlist []string `json:"path_allowlist,omitempty" hcl:"path_allowlist" mapstructure:"path_allowlist"`
|
||||||
}
|
}
|
||||||
|
|
||||||
type RawUIMetricsProxyAddHeader struct {
|
type RawUIMetricsProxyAddHeader struct {
|
||||||
|
|
|
@ -1540,8 +1540,9 @@ type UIConfig struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
type UIMetricsProxy struct {
|
type UIMetricsProxy struct {
|
||||||
BaseURL string
|
BaseURL string
|
||||||
AddHeaders []UIMetricsProxyAddHeader
|
AddHeaders []UIMetricsProxyAddHeader
|
||||||
|
PathAllowlist []string
|
||||||
}
|
}
|
||||||
|
|
||||||
type UIMetricsProxyAddHeader struct {
|
type UIMetricsProxyAddHeader struct {
|
||||||
|
|
|
@ -4545,6 +4545,189 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
|
||||||
`},
|
`},
|
||||||
err: `ui_config.metrics_proxy.base_url must be a valid http or https URL.`,
|
err: `ui_config.metrics_proxy.base_url must be a valid http or https URL.`,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist invalid (empty)",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
err: `ui_config.metrics_proxy.path_allowlist: path "" is not an absolute path`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist invalid (relative)",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
err: `ui_config.metrics_proxy.path_allowlist: path "bar/baz" is not an absolute path`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist invalid (weird)",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["://bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["://bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
err: `ui_config.metrics_proxy.path_allowlist: path "://bar/baz" is not an absolute path`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist invalid (fragment)",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["/bar/baz#stuff", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["/bar/baz#stuff", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
err: `ui_config.metrics_proxy.path_allowlist: path "/bar/baz#stuff" is not an absolute path`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist invalid (querystring)",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["/bar/baz?stu=ff", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["/bar/baz?stu=ff", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
err: `ui_config.metrics_proxy.path_allowlist: path "/bar/baz?stu=ff" is not an absolute path`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist invalid (encoded slash)",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["/bar%2fbaz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["/bar%2fbaz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
err: `ui_config.metrics_proxy.path_allowlist: path "/bar%2fbaz" is not an absolute path`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist ok",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["/bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["/bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
patch: func(rt *RuntimeConfig) {
|
||||||
|
rt.UIConfig.MetricsProxy.PathAllowlist = []string{"/bar/baz", "/foo"}
|
||||||
|
rt.DataDir = dataDir
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist defaulted for prometheus",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_provider": "prometheus"
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_provider = "prometheus"
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
patch: func(rt *RuntimeConfig) {
|
||||||
|
rt.UIConfig.MetricsProvider = "prometheus"
|
||||||
|
rt.UIConfig.MetricsProxy.PathAllowlist = []string{
|
||||||
|
"/api/v1/query",
|
||||||
|
"/api/v1/query_range",
|
||||||
|
}
|
||||||
|
rt.DataDir = dataDir
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "metrics_proxy.path_allowlist not overridden with defaults for prometheus",
|
||||||
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
json: []string{`{
|
||||||
|
"ui_config": {
|
||||||
|
"metrics_provider": "prometheus",
|
||||||
|
"metrics_proxy": {
|
||||||
|
"path_allowlist": ["/bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`},
|
||||||
|
hcl: []string{`
|
||||||
|
ui_config {
|
||||||
|
metrics_provider = "prometheus"
|
||||||
|
metrics_proxy {
|
||||||
|
path_allowlist = ["/bar/baz", "/foo"]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
`},
|
||||||
|
patch: func(rt *RuntimeConfig) {
|
||||||
|
rt.UIConfig.MetricsProvider = "prometheus"
|
||||||
|
rt.UIConfig.MetricsProxy.PathAllowlist = []string{"/bar/baz", "/foo"}
|
||||||
|
rt.DataDir = dataDir
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
desc: "metrics_proxy.base_url http(s)",
|
desc: "metrics_proxy.base_url http(s)",
|
||||||
args: []string{`-data-dir=` + dataDir},
|
args: []string{`-data-dir=` + dataDir},
|
||||||
|
@ -5459,7 +5642,8 @@ func TestFullConfig(t *testing.T) {
|
||||||
"name": "p3nynwc9",
|
"name": "p3nynwc9",
|
||||||
"value": "TYBgnN2F"
|
"value": "TYBgnN2F"
|
||||||
}
|
}
|
||||||
]
|
],
|
||||||
|
"path_allowlist": ["/aSh3cu", "/eiK/2Th"]
|
||||||
},
|
},
|
||||||
"dashboard_url_templates": {
|
"dashboard_url_templates": {
|
||||||
"u2eziu2n_lower_case": "http://lkjasd.otr"
|
"u2eziu2n_lower_case": "http://lkjasd.otr"
|
||||||
|
@ -6149,6 +6333,7 @@ func TestFullConfig(t *testing.T) {
|
||||||
value = "TYBgnN2F"
|
value = "TYBgnN2F"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
path_allowlist = ["/aSh3cu", "/eiK/2Th"]
|
||||||
}
|
}
|
||||||
dashboard_url_templates {
|
dashboard_url_templates {
|
||||||
u2eziu2n_lower_case = "http://lkjasd.otr"
|
u2eziu2n_lower_case = "http://lkjasd.otr"
|
||||||
|
@ -6942,6 +7127,7 @@ func TestFullConfig(t *testing.T) {
|
||||||
Value: "TYBgnN2F",
|
Value: "TYBgnN2F",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
PathAllowlist: []string{"/aSh3cu", "/eiK/2Th"},
|
||||||
},
|
},
|
||||||
DashboardURLTemplates: map[string]string{"u2eziu2n_lower_case": "http://lkjasd.otr"},
|
DashboardURLTemplates: map[string]string{"u2eziu2n_lower_case": "http://lkjasd.otr"},
|
||||||
},
|
},
|
||||||
|
@ -7627,7 +7813,8 @@ func TestSanitize(t *testing.T) {
|
||||||
"MetricsProviderOptionsJSON": "",
|
"MetricsProviderOptionsJSON": "",
|
||||||
"MetricsProxy": {
|
"MetricsProxy": {
|
||||||
"AddHeaders": [],
|
"AddHeaders": [],
|
||||||
"BaseURL": ""
|
"BaseURL": "",
|
||||||
|
"PathAllowlist": []
|
||||||
},
|
},
|
||||||
"DashboardURLTemplates": {}
|
"DashboardURLTemplates": {}
|
||||||
},
|
},
|
||||||
|
|
|
@ -589,6 +589,29 @@ func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Reques
|
||||||
// double slashes etc.
|
// double slashes etc.
|
||||||
u.Path = path.Clean(u.Path)
|
u.Path = path.Clean(u.Path)
|
||||||
|
|
||||||
|
if len(cfg.PathAllowlist) > 0 {
|
||||||
|
// This could be done better with a map, but for the prometheus default
|
||||||
|
// integration this list has two items in it, so the straight iteration
|
||||||
|
// isn't awful.
|
||||||
|
denied := true
|
||||||
|
for _, allowedPath := range cfg.PathAllowlist {
|
||||||
|
if u.Path == allowedPath {
|
||||||
|
denied = false
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if denied {
|
||||||
|
log.Error("target URL path is not allowed",
|
||||||
|
"base_url", cfg.BaseURL,
|
||||||
|
"path", subPath,
|
||||||
|
"target_url", u.String(),
|
||||||
|
"path_allowlist", cfg.PathAllowlist,
|
||||||
|
)
|
||||||
|
resp.WriteHeader(http.StatusForbidden)
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Pass through query params
|
// Pass through query params
|
||||||
u.RawQuery = req.URL.RawQuery
|
u.RawQuery = req.URL.RawQuery
|
||||||
|
|
||||||
|
|
|
@ -1631,6 +1631,25 @@ func TestUIEndpoint_MetricsProxy(t *testing.T) {
|
||||||
path: endpointPath + "/ok",
|
path: endpointPath + "/ok",
|
||||||
wantCode: http.StatusNotFound,
|
wantCode: http.StatusNotFound,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "blocked path",
|
||||||
|
config: config.UIMetricsProxy{
|
||||||
|
BaseURL: backendURL,
|
||||||
|
PathAllowlist: []string{"/some/other-prefix/ok"},
|
||||||
|
},
|
||||||
|
path: endpointPath + "/ok",
|
||||||
|
wantCode: http.StatusForbidden,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "allowed path",
|
||||||
|
config: config.UIMetricsProxy{
|
||||||
|
BaseURL: backendURL,
|
||||||
|
PathAllowlist: []string{"/some/prefix/ok"},
|
||||||
|
},
|
||||||
|
path: endpointPath + "/ok",
|
||||||
|
wantCode: http.StatusOK,
|
||||||
|
wantContains: "OK",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "basic proxying",
|
name: "basic proxying",
|
||||||
config: config.UIMetricsProxy{
|
config: config.UIMetricsProxy{
|
||||||
|
|
|
@ -157,6 +157,7 @@ function assert_envoy_http_rbac_policy_count {
|
||||||
local EXPECT_COUNT=$2
|
local EXPECT_COUNT=$2
|
||||||
|
|
||||||
GOT_COUNT=$(get_envoy_http_rbac_once $HOSTPORT | jq '.rules.policies | length')
|
GOT_COUNT=$(get_envoy_http_rbac_once $HOSTPORT | jq '.rules.policies | length')
|
||||||
|
echo "GOT_COUNT = $GOT_COUNT"
|
||||||
[ "${GOT_COUNT:-0}" -eq $EXPECT_COUNT ]
|
[ "${GOT_COUNT:-0}" -eq $EXPECT_COUNT ]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -172,6 +173,7 @@ function assert_envoy_network_rbac_policy_count {
|
||||||
local EXPECT_COUNT=$2
|
local EXPECT_COUNT=$2
|
||||||
|
|
||||||
GOT_COUNT=$(get_envoy_network_rbac_once $HOSTPORT | jq '.rules.policies | length')
|
GOT_COUNT=$(get_envoy_network_rbac_once $HOSTPORT | jq '.rules.policies | length')
|
||||||
|
echo "GOT_COUNT = $GOT_COUNT"
|
||||||
[ "${GOT_COUNT:-0}" -eq $EXPECT_COUNT ]
|
[ "${GOT_COUNT:-0}" -eq $EXPECT_COUNT ]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue