From e18debc25a930580e64c7f206df2ae5b8a8c545c Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Mon, 3 Feb 2020 16:47:50 +0100 Subject: [PATCH 1/3] Return error when a standby node receives a metrics request --- http/handler.go | 6 ++++-- http/logical.go | 9 ++++++++- http/sys_metrics.go | 6 ++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/http/handler.go b/http/handler.go index 7f5fbb6ee142..a29a4a4cb111 100644 --- a/http/handler.go +++ b/http/handler.go @@ -19,8 +19,8 @@ import ( "github.com/NYTimes/gziphandler" assetfs "github.com/elazarl/go-bindata-assetfs" "github.com/hashicorp/errwrap" - "github.com/hashicorp/go-cleanhttp" - "github.com/hashicorp/go-sockaddr" + cleanhttp "github.com/hashicorp/go-cleanhttp" + sockaddr "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" @@ -160,6 +160,8 @@ func Handler(props *vault.HandlerProperties) http.Handler { // Register metrics path without authentication if enabled if props.UnauthenticatedMetricsAccess { mux.Handle("/v1/sys/metrics", handleMetricsUnauthenticated(core)) + } else { + mux.Handle("/v1/sys/metrics", handleLogicalNoForward(core)) } additionalRoutes(mux, core) diff --git a/http/logical.go b/http/logical.go index 445c842b65df..b8cbcfd727c4 100644 --- a/http/logical.go +++ b/http/logical.go @@ -12,7 +12,7 @@ import ( "time" "github.com/hashicorp/errwrap" - "github.com/hashicorp/go-uuid" + uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/logical" @@ -234,6 +234,13 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool, noForw return } + // Prevent any metrics requests to be forwarded from a standby node. + // Instead, we return an error since we cannot be sure if we have an + // active token store to validate the provided token. + if isStandby, _ := core.Standby(); isStandby { + respondError(w, http.StatusBadRequest, vault.ErrCannotForwardLocalOnly) + } + // req.Path will be relative by this point. The prefix check is first // to fail faster if we're not in this situation since it's a hot path switch { diff --git a/http/sys_metrics.go b/http/sys_metrics.go index ee55383f9816..b97786b79f6e 100644 --- a/http/sys_metrics.go +++ b/http/sys_metrics.go @@ -13,6 +13,12 @@ func handleMetricsUnauthenticated(core *vault.Core) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { req := &logical.Request{Headers: r.Header} + switch r.Method { + case "GET": + default: + respondError(w, http.StatusMethodNotAllowed, nil) + } + // Parse form if err := r.ParseForm(); err != nil { respondError(w, http.StatusBadRequest, err) From 06c8d65f41aec0648abf336afea312ea1e616718 Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Tue, 4 Feb 2020 12:04:43 +0100 Subject: [PATCH 2/3] fix test --- http/logical.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/http/logical.go b/http/logical.go index b8cbcfd727c4..f198ff403e9a 100644 --- a/http/logical.go +++ b/http/logical.go @@ -234,13 +234,6 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool, noForw return } - // Prevent any metrics requests to be forwarded from a standby node. - // Instead, we return an error since we cannot be sure if we have an - // active token store to validate the provided token. - if isStandby, _ := core.Standby(); isStandby { - respondError(w, http.StatusBadRequest, vault.ErrCannotForwardLocalOnly) - } - // req.Path will be relative by this point. The prefix check is first // to fail faster if we're not in this situation since it's a hot path switch { @@ -339,6 +332,14 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool, noForw } } } + + // Prevent any metrics requests to be forwarded from a standby node. + // Instead, we return an error since we cannot be sure if we have an + // active token store to validate the provided token. + case strings.HasPrefix(req.Path, "sys/metrics"): + if isStandby, _ := core.Standby(); isStandby { + respondError(w, http.StatusBadRequest, vault.ErrCannotForwardLocalOnly) + } } // Make the internal request. We attach the connection info From 6de051b3a0b50c5de09ed08f4779c3c5662236fc Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Thu, 6 Feb 2020 10:24:35 +0100 Subject: [PATCH 3/3] Add documentation note --- website/pages/docs/configuration/telemetry.mdx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/pages/docs/configuration/telemetry.mdx b/website/pages/docs/configuration/telemetry.mdx index 939bc4845229..0abbaddeab8b 100644 --- a/website/pages/docs/configuration/telemetry.mdx +++ b/website/pages/docs/configuration/telemetry.mdx @@ -141,6 +141,10 @@ These `telemetry` parameters apply to ### `prometheus` +~> **Note:** The `/v1/sys/metrics` endpoint is only accessible on active nodes +and automatically disabled on standby nodes. You can enable the `/v1/sys/metrics` +endpoint on standby nodes by [enabling unauthenticated metrics access][telemetry-tcp]. + These `telemetry` parameters apply to [prometheus](https://prometheus.io). @@ -206,3 +210,5 @@ telemetry { enable_hostname_label = true } ``` + +[telemetry-tcp]: /docs/configuration/listener/tcp#telemetry