From 051ad2017b69ca8e438396b461525e485a896321 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 12 Oct 2018 13:52:08 -0400 Subject: [PATCH] 14285: Add token middleware. Require management token for metrics. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- doc/admin/health-checks.html.textile.liquid | 19 ++++---- .../install-keep-balance.html.textile.liquid | 2 + .../install-keepstore.html.textile.liquid | 6 +-- sdk/go/auth/auth.go | 6 ++- sdk/go/auth/handlers.go | 47 +++++++++++++++++++ sdk/go/health/aggregator.go | 2 +- sdk/go/httpserver/metrics.go | 16 +++++-- services/arv-git-httpd/auth_handler.go | 2 +- services/keep-balance/balance_run_test.go | 7 +++ services/keep-balance/server.go | 8 +++- services/keep-balance/usage.go | 1 + services/keep-web/handler.go | 4 +- services/keep-web/server.go | 2 +- services/keep-web/server_test.go | 13 +++++ services/keepstore/config.go | 3 +- services/keepstore/handlers.go | 4 +- services/keepstore/mounts_test.go | 7 ++- 17 files changed, 120 insertions(+), 29 deletions(-) create mode 100644 sdk/go/auth/handlers.go diff --git a/doc/admin/health-checks.html.textile.liquid b/doc/admin/health-checks.html.textile.liquid index 630c6a178f..fa2668c710 100644 --- a/doc/admin/health-checks.html.textile.liquid +++ b/doc/admin/health-checks.html.textile.liquid @@ -39,32 +39,33 @@ The healthcheck aggregator uses the @NodeProfile@ section of the cluster-wide @a Cluster: # The cluster uuid prefix zzzzz: + ManagementToken: xyzzy NodeProfile: # For each node, the profile name corresponds to a # locally-resolvable hostname, and describes which Arvados # services are available on that machine. api: arvados-controller: - Listen: 8000 + Listen: :8000 arvados-api-server: - Listen: 8001 + Listen: :8001 manage: arvados-node-manager: - Listen: 8002 + Listen: :8002 workbench: arvados-workbench: - Listen: 8003 + Listen: :8003 arvados-ws: - Listen: 8004 + Listen: :8004 keep: keep-web: - Listen: 8005 + Listen: :8005 keepproxy: - Listen: 8006 + Listen: :8006 keep0: keepstore: - Listen: 25701 + Listen: :25107 keep1: keepstore: - Listen: 25701 + Listen: :25107 diff --git a/doc/install/install-keep-balance.html.textile.liquid b/doc/install/install-keep-balance.html.textile.liquid index 043f3ebfd2..3b8b3c0533 100644 --- a/doc/install/install-keep-balance.html.textile.liquid +++ b/doc/install/install-keep-balance.html.textile.liquid @@ -81,6 +81,8 @@ Client: AuthToken: zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz KeepServiceTypes: - disk +Listen: :9005 +ManagementToken: xyzzy RunPeriod: 10m CollectionBatchSize: 100000 CollectionBuffers: 1000 diff --git a/doc/install/install-keepstore.html.textile.liquid b/doc/install/install-keepstore.html.textile.liquid index 943c9bae36..fc4914efdb 100644 --- a/doc/install/install-keepstore.html.textile.liquid +++ b/doc/install/install-keepstore.html.textile.liquid @@ -88,9 +88,9 @@ Listen: :25107 # Format of request/response and error logs: "json" or "text". LogFormat: json -# The secret key that must be provided by monitoring services -# wishing to access the health check endpoint (/_health). -ManagementToken: "" +# The secret key that must be provided by monitoring services when +# using the health check and metrics endpoints (/_health, /metrics). +ManagementToken: xyzzy # Maximum RAM to use for data buffers, given in multiples of block # size (64 MiB). When this limit is reached, HTTP requests requiring diff --git a/sdk/go/auth/auth.go b/sdk/go/auth/auth.go index ad1d398c76..3c266e0d3a 100644 --- a/sdk/go/auth/auth.go +++ b/sdk/go/auth/auth.go @@ -19,7 +19,11 @@ func NewCredentials() *Credentials { return &Credentials{Tokens: []string{}} } -func NewCredentialsFromHTTPRequest(r *http.Request) *Credentials { +func CredentialsFromRequest(r *http.Request) *Credentials { + if c, ok := r.Context().Value(contextKeyCredentials).(*Credentials); ok { + // preloaded by middleware + return c + } c := NewCredentials() c.LoadTokensFromHTTPRequest(r) return c diff --git a/sdk/go/auth/handlers.go b/sdk/go/auth/handlers.go new file mode 100644 index 0000000000..7b1760f4b8 --- /dev/null +++ b/sdk/go/auth/handlers.go @@ -0,0 +1,47 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +package auth + +import ( + "context" + "net/http" +) + +type contextKey string + +var contextKeyCredentials contextKey = "credentials" + +// LoadToken wraps the next handler, adding credentials to the request +// context so subsequent handlers can access them efficiently via +// CredentialsFromRequest. +func LoadToken(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), contextKeyCredentials, CredentialsFromRequest(r)))) + }) +} + +// RequireLiteralToken wraps the next handler, rejecting any request +// that doesn't supply the given token. If the given token is empty, +// RequireLiteralToken returns next (i.e., no auth checks are +// performed). +func RequireLiteralToken(token string, next http.Handler) http.Handler { + if token == "" { + return next + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c := CredentialsFromRequest(r) + if len(c.Tokens) == 0 { + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + for _, t := range c.Tokens { + if t == token { + next.ServeHTTP(w, r) + return + } + } + http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) + }) +} diff --git a/sdk/go/health/aggregator.go b/sdk/go/health/aggregator.go index a6cb8798aa..564331327a 100644 --- a/sdk/go/health/aggregator.go +++ b/sdk/go/health/aggregator.go @@ -217,7 +217,7 @@ func (agg *Aggregator) ping(url string, cluster *arvados.Cluster) (result CheckR } func (agg *Aggregator) checkAuth(req *http.Request, cluster *arvados.Cluster) bool { - creds := auth.NewCredentialsFromHTTPRequest(req) + creds := auth.CredentialsFromRequest(req) for _, token := range creds.Tokens { if token != "" && token == cluster.ManagementToken { return true diff --git a/sdk/go/httpserver/metrics.go b/sdk/go/httpserver/metrics.go index b52068e957..a0455f11b1 100644 --- a/sdk/go/httpserver/metrics.go +++ b/sdk/go/httpserver/metrics.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "git.curoverse.com/arvados.git/sdk/go/auth" "git.curoverse.com/arvados.git/sdk/go/stats" "github.com/Sirupsen/logrus" "github.com/gogo/protobuf/jsonpb" @@ -23,7 +24,7 @@ type Handler interface { // Returns an http.Handler that serves the Handler's metrics // data at /metrics and /metrics.json, and passes other // requests through to next. - ServeAPI(next http.Handler) http.Handler + ServeAPI(token string, next http.Handler) http.Handler } type metrics struct { @@ -73,19 +74,24 @@ func (m *metrics) ServeHTTP(w http.ResponseWriter, req *http.Request) { // metrics API endpoints (currently "GET /metrics(.json)?") and passes // other requests through to next. // +// If the given token is not empty, that token must be supplied by a +// client in order to access the metrics endpoints. +// // Typical example: // // m := Instrument(...) -// srv := http.Server{Handler: m.ServeAPI(m)} -func (m *metrics) ServeAPI(next http.Handler) http.Handler { +// srv := http.Server{Handler: m.ServeAPI("secrettoken", m)} +func (m *metrics) ServeAPI(token string, next http.Handler) http.Handler { + jsonMetrics := auth.RequireLiteralToken(token, http.HandlerFunc(m.exportJSON)) + plainMetrics := auth.RequireLiteralToken(token, m.exportProm) return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { switch { case req.Method != "GET" && req.Method != "HEAD": next.ServeHTTP(w, req) case req.URL.Path == "/metrics.json": - m.exportJSON(w, req) + jsonMetrics.ServeHTTP(w, req) case req.URL.Path == "/metrics": - m.exportProm.ServeHTTP(w, req) + plainMetrics.ServeHTTP(w, req) default: next.ServeHTTP(w, req) } diff --git a/services/arv-git-httpd/auth_handler.go b/services/arv-git-httpd/auth_handler.go index b4dc58b24f..3b3032afda 100644 --- a/services/arv-git-httpd/auth_handler.go +++ b/services/arv-git-httpd/auth_handler.go @@ -91,7 +91,7 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { httpserver.Log(r.RemoteAddr, passwordToLog, w.WroteStatus(), statusText, repoName, r.Method, r.URL.Path) }() - creds := auth.NewCredentialsFromHTTPRequest(r) + creds := auth.CredentialsFromRequest(r) if len(creds.Tokens) == 0 { statusCode, statusText = http.StatusUnauthorized, "no credentials provided" w.Header().Add("WWW-Authenticate", "Basic realm=\"git\"") diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go index 26aee213df..f42383297f 100644 --- a/services/keep-balance/balance_run_test.go +++ b/services/keep-balance/balance_run_test.go @@ -433,6 +433,7 @@ func (s *runSuite) TestDryRun(c *check.C) { func (s *runSuite) TestCommit(c *check.C) { s.config.Listen = ":" + s.config.ManagementToken = "xyzzy" opts := RunOptions{ CommitPulls: true, CommitTrash: true, @@ -466,6 +467,7 @@ func (s *runSuite) TestCommit(c *check.C) { func (s *runSuite) TestRunForever(c *check.C) { s.config.Listen = ":" + s.config.ManagementToken = "xyzzy" opts := RunOptions{ CommitPulls: true, CommitTrash: true, @@ -508,6 +510,11 @@ func (s *runSuite) TestRunForever(c *check.C) { func (s *runSuite) getMetrics(c *check.C, srv *Server) string { resp, err := http.Get("http://" + srv.listening + "/metrics") c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized) + + resp, err = http.Get("http://" + srv.listening + "/metrics?api_token=xyzzy") + c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusOK) buf, err := ioutil.ReadAll(resp.Body) c.Check(err, check.IsNil) return string(buf) diff --git a/services/keep-balance/server.go b/services/keep-balance/server.go index c47305aefc..ad13be7511 100644 --- a/services/keep-balance/server.go +++ b/services/keep-balance/server.go @@ -13,6 +13,7 @@ import ( "time" "git.curoverse.com/arvados.git/sdk/go/arvados" + "git.curoverse.com/arvados.git/sdk/go/auth" "git.curoverse.com/arvados.git/sdk/go/httpserver" "github.com/Sirupsen/logrus" ) @@ -40,6 +41,9 @@ type Config struct { // address, address:port, or :port for management interface Listen string + // token for management APIs + ManagementToken string + // How often to check RunPeriod arvados.Duration @@ -121,7 +125,9 @@ func (srv *Server) start() error { } server := &httpserver.Server{ Server: http.Server{ - Handler: httpserver.LogRequests(srv.Logger, srv.metrics.Handler(srv.Logger)), + Handler: httpserver.LogRequests(srv.Logger, + auth.RequireLiteralToken(srv.config.ManagementToken, + srv.metrics.Handler(srv.Logger))), }, Addr: srv.config.Listen, } diff --git a/services/keep-balance/usage.go b/services/keep-balance/usage.go index 8df4a23644..b39e83905d 100644 --- a/services/keep-balance/usage.go +++ b/services/keep-balance/usage.go @@ -18,6 +18,7 @@ Client: KeepServiceTypes: - disk Listen: ":9005" +ManagementToken: xyzzy RunPeriod: 600s CollectionBatchSize: 100000 CollectionBuffers: 1000 diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go index 912398fa64..95948e3250 100644 --- a/services/keep-web/handler.go +++ b/services/keep-web/handler.go @@ -320,7 +320,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { if useSiteFS { if tokens == nil { - tokens = auth.NewCredentialsFromHTTPRequest(r).Tokens + tokens = auth.CredentialsFromRequest(r).Tokens } h.serveSiteFS(w, r, tokens, credentialsOK, attachment) return @@ -342,7 +342,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { if tokens == nil { if credentialsOK { - reqTokens = auth.NewCredentialsFromHTTPRequest(r).Tokens + reqTokens = auth.CredentialsFromRequest(r).Tokens } tokens = append(reqTokens, h.Config.AnonymousTokens...) } diff --git a/services/keep-web/server.go b/services/keep-web/server.go index 68ff8a7b01..f70dd1a71f 100644 --- a/services/keep-web/server.go +++ b/services/keep-web/server.go @@ -21,7 +21,7 @@ func (srv *server) Start() error { reg := prometheus.NewRegistry() h.Config.Cache.registry = reg mh := httpserver.Instrument(reg, nil, httpserver.AddRequestIDs(httpserver.LogRequests(nil, h))) - h.MetricsAPI = mh.ServeAPI(http.NotFoundHandler()) + h.MetricsAPI = mh.ServeAPI(h.Config.ManagementToken, http.NotFoundHandler()) srv.Handler = mh srv.Addr = srv.Config.Listen return srv.Server.Start() diff --git a/services/keep-web/server_test.go b/services/keep-web/server_test.go index 7e738cb9f3..48c9726e3b 100644 --- a/services/keep-web/server_test.go +++ b/services/keep-web/server_test.go @@ -323,6 +323,18 @@ func (s *IntegrationSuite) TestMetrics(c *check.C) { req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) resp, err = http.DefaultClient.Do(req) c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized) + + req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) + req.Header.Set("Authorization", "Bearer badtoken") + resp, err = http.DefaultClient.Do(req) + c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusForbidden) + + req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) + req.Header.Set("Authorization", "Bearer "+arvadostest.ManagementToken) + resp, err = http.DefaultClient.Do(req) + c.Assert(err, check.IsNil) c.Check(resp.StatusCode, check.Equals, http.StatusOK) type summary struct { SampleCount string `json:"sample_count"` @@ -418,6 +430,7 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) { Insecure: true, } cfg.Listen = "127.0.0.1:0" + cfg.ManagementToken = arvadostest.ManagementToken s.testServer = &server{Config: cfg} err := s.testServer.Start() c.Assert(err, check.Equals, nil) diff --git a/services/keepstore/config.go b/services/keepstore/config.go index 1f8c7e31a2..2e3fe0a5b1 100644 --- a/services/keepstore/config.go +++ b/services/keepstore/config.go @@ -46,8 +46,7 @@ type Config struct { systemAuthToken string debugLogf func(string, ...interface{}) - ManagementToken string `doc: The secret key that must be provided by monitoring services -wishing to access the health check endpoint (/_health).` + ManagementToken string } var ( diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go index 2426c9cbda..a325d9820a 100644 --- a/services/keepstore/handlers.go +++ b/services/keepstore/handlers.go @@ -87,9 +87,9 @@ func MakeRESTRouter(cluster *arvados.Cluster) http.Handler { rtr.limiter = httpserver.NewRequestLimiter(theConfig.MaxRequests, rtr) - stack := httpserver.Instrument(nil, nil, + instrumented := httpserver.Instrument(nil, nil, httpserver.AddRequestIDs(httpserver.LogRequests(nil, rtr.limiter))) - return stack.ServeAPI(stack) + return instrumented.ServeAPI(theConfig.ManagementToken, instrumented) } // BadRequestHandler is a HandleFunc to address bad requests. diff --git a/services/keepstore/mounts_test.go b/services/keepstore/mounts_test.go index 9fa0090aa7..31b1a684fe 100644 --- a/services/keepstore/mounts_test.go +++ b/services/keepstore/mounts_test.go @@ -27,6 +27,7 @@ func (s *MountsSuite) SetUpTest(c *check.C) { KeepVM = s.vm theConfig = DefaultConfig() theConfig.systemAuthToken = arvadostest.DataManagerToken + theConfig.ManagementToken = arvadostest.ManagementToken theConfig.Start() s.rtr = MakeRESTRouter(testCluster) } @@ -104,6 +105,10 @@ func (s *MountsSuite) TestMetrics(c *check.C) { s.call("PUT", "/"+TestHash, "", TestBlock) s.call("PUT", "/"+TestHash2, "", TestBlock2) resp := s.call("GET", "/metrics.json", "", nil) + c.Check(resp.Code, check.Equals, http.StatusUnauthorized) + resp = s.call("GET", "/metrics.json", "foobar", nil) + c.Check(resp.Code, check.Equals, http.StatusForbidden) + resp = s.call("GET", "/metrics.json", arvadostest.ManagementToken, nil) c.Check(resp.Code, check.Equals, http.StatusOK) var j []struct { Name string @@ -144,7 +149,7 @@ func (s *MountsSuite) call(method, path, tok string, body []byte) *httptest.Resp resp := httptest.NewRecorder() req, _ := http.NewRequest(method, path, bytes.NewReader(body)) if tok != "" { - req.Header.Set("Authorization", "OAuth2 "+tok) + req.Header.Set("Authorization", "Bearer "+tok) } s.rtr.ServeHTTP(resp, req) return resp -- 2.30.2