13493: Salt tokens when forwarding requests to remote clusters.
authorTom Clegg <tclegg@veritasgenetics.com>
Wed, 27 Jun 2018 20:43:34 +0000 (16:43 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Wed, 27 Jun 2018 20:43:34 +0000 (16:43 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/controller/federation.go [new file with mode: 0644]
lib/controller/federation_test.go
lib/controller/handler.go
sdk/go/auth/auth.go
sdk/go/auth/salt.go [new file with mode: 0644]

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
new file mode 100644 (file)
index 0000000..5ea305d
--- /dev/null
@@ -0,0 +1,50 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package controller
+
+import (
+       "bytes"
+       "io/ioutil"
+       "net/http"
+
+       "git.curoverse.com/arvados.git/sdk/go/auth"
+)
+
+// Extract the auth token supplied in req, and replace it with a
+// salted token for the remote cluster.
+func (h *Handler) saltAuthToken(req *http.Request, remote string) error {
+       creds := auth.NewCredentials()
+       creds.LoadTokensFromHTTPRequest(req)
+       if len(creds.Tokens) == 0 && req.Header.Get("Content-Type") == "application/x-www-form-encoded" {
+               // Override ParseForm's 10MiB limit by ensuring
+               // req.Body is a *http.maxBytesReader.
+               req.Body = http.MaxBytesReader(nil, req.Body, 1<<28) // 256MiB. TODO: use MaxRequestSize from discovery doc or config.
+               if err := creds.LoadTokensFromHTTPRequestBody(req); err != nil {
+                       return err
+               }
+               // Replace req.Body with a buffer that re-encodes the
+               // form without api_token, in case we end up
+               // forwarding the request to RailsAPI.
+               if req.PostForm != nil {
+                       req.PostForm.Del("api_token")
+               }
+               req.Body = ioutil.NopCloser(bytes.NewBufferString(req.PostForm.Encode()))
+       }
+       if len(creds.Tokens) == 0 {
+               return nil
+       }
+       token, err := auth.SaltToken(creds.Tokens[0], remote)
+       if err == auth.ErrObsoleteToken {
+               // FIXME: If the token exists in our own database,
+               // salt it for the remote. Otherwise, assume it was
+               // issued by the remote, and pass it through
+               // unmodified.
+               token = creds.Tokens[0]
+       } else if err != nil {
+               return err
+       }
+       req.Header.Set("Authorization", "Bearer "+token)
+       return nil
+}
index 6c54fc8d34e9266c5f4283a62387bc6895e40d06..156b8d5f26009f3e35a0968a3aa3e068693b37f5 100644 (file)
@@ -94,7 +94,7 @@ func (s *FederationSuite) TestNoAuth(c *check.C) {
 
 func (s *FederationSuite) TestBadAuth(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
-       req.Header.Set("Authorization", "Bearer aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
+       req.Header.Set("Authorization", "Bearer aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
        resp := httptest.NewRecorder()
        s.handler.ServeHTTP(resp, req)
        c.Check(resp.Code, check.Equals, http.StatusUnauthorized)
@@ -145,14 +145,23 @@ func (s *FederationSuite) TestGetRemoteWorkflow(c *check.C) {
 }
 
 func (s *FederationSuite) TestUpdateRemoteWorkflow(c *check.C) {
-       req := httptest.NewRequest("PATCH", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, strings.NewReader(url.Values{
-               "workflow": {`{"description":"updated by TestUpdateRemoteWorkflow"}`},
-       }.Encode()))
-       req.Header.Set("Content-type", "application/x-www-form-urlencoded")
-       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := httptest.NewRecorder()
-       s.handler.ServeHTTP(resp, req)
-       s.checkResponseOK(c, resp)
+       updateDescription := func(descr string) *httptest.ResponseRecorder {
+               req := httptest.NewRequest("PATCH", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, strings.NewReader(url.Values{
+                       "workflow": {`{"description":"` + descr + `"}`},
+               }.Encode()))
+               req.Header.Set("Content-type", "application/x-www-form-urlencoded")
+               req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+               resp := httptest.NewRecorder()
+               s.handler.ServeHTTP(resp, req)
+               s.checkResponseOK(c, resp)
+               return resp
+       }
+
+       // Update description twice so running this test twice in a
+       // row still causes ModifiedAt to change
+       updateDescription("updated once by TestUpdateRemoteWorkflow")
+       resp := updateDescription("updated twice by TestUpdateRemoteWorkflow")
+
        var wf arvados.Workflow
        c.Check(json.Unmarshal(resp.Body.Bytes(), &wf), check.IsNil)
        c.Check(wf.UUID, check.Equals, arvadostest.WorkflowWithDefinitionYAMLUUID)
index 7f4376e6fc88d69982514c077cd9d232ffaffe89..ab6c72735e0a7b606ca65d5110b9e3b6f9d047bf 100644 (file)
@@ -82,9 +82,10 @@ func (h *Handler) proxyRemoteCluster(w http.ResponseWriter, req *http.Request, n
                next.ServeHTTP(w, req)
                return
        }
-       remote, ok := h.Cluster.RemoteClusters[m[1]]
+       remoteID := m[1]
+       remote, ok := h.Cluster.RemoteClusters[remoteID]
        if !ok {
-               httpserver.Error(w, "no proxy available for cluster "+m[1], http.StatusNotFound)
+               httpserver.Error(w, "no proxy available for cluster "+remoteID, http.StatusNotFound)
                return
        }
        scheme := remote.Scheme
@@ -98,6 +99,11 @@ func (h *Handler) proxyRemoteCluster(w http.ResponseWriter, req *http.Request, n
                RawPath:  req.URL.RawPath,
                RawQuery: req.URL.RawQuery,
        }
+       err := h.saltAuthToken(req, remoteID)
+       if err != nil {
+               httpserver.Error(w, err.Error(), http.StatusBadRequest)
+               return
+       }
        h.proxy(w, req, urlOut)
 }
 
index ea492430e41297ddb8465c73b62c477e20af2357..ad1d398c763d7eaacefefcde8993e39044582f2a 100644 (file)
@@ -34,7 +34,7 @@ var EncodeTokenCookie func([]byte) string = base64.URLEncoding.EncodeToString
 // token.
 var DecodeTokenCookie func(string) ([]byte, error) = base64.URLEncoding.DecodeString
 
-// LoadTokensFromHttpRequest loads all tokens it can find in the
+// LoadTokensFromHTTPRequest loads all tokens it can find in the
 // headers and query string of an http query.
 func (a *Credentials) LoadTokensFromHTTPRequest(r *http.Request) {
        // Load plain token from "Authorization: OAuth2 ..." header
@@ -83,7 +83,21 @@ func (a *Credentials) loadTokenFromCookie(r *http.Request) {
        a.Tokens = append(a.Tokens, string(token))
 }
 
-// TODO: LoadTokensFromHttpRequestBody(). We can't assume in
-// LoadTokensFromHttpRequest() that [or how] we should read and parse
-// the request body. This has to be requested explicitly by the
-// application.
+// LoadTokensFromHTTPRequestBody() loads credentials from the request
+// body.
+//
+// This is separate from LoadTokensFromHTTPRequest() because it's not
+// always desirable to read the request body. This has to be requested
+// explicitly by the application.
+func (a *Credentials) LoadTokensFromHTTPRequestBody(r *http.Request) error {
+       if r.Header.Get("Content-Type") != "application/x-www-form-urlencoded" {
+               return nil
+       }
+       if err := r.ParseForm(); err != nil {
+               return err
+       }
+       if t := r.PostFormValue("api_token"); t != "" {
+               a.Tokens = append(a.Tokens, t)
+       }
+       return nil
+}
diff --git a/sdk/go/auth/salt.go b/sdk/go/auth/salt.go
new file mode 100644 (file)
index 0000000..f669eb2
--- /dev/null
@@ -0,0 +1,45 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package auth
+
+import (
+       "crypto/hmac"
+       "crypto/sha1"
+       "errors"
+       "fmt"
+       "regexp"
+       "strings"
+)
+
+var (
+       reObsoleteToken  = regexp.MustCompile(`^[0-9a-z]{41,}$`)
+       ErrObsoleteToken = errors.New("obsolete token format")
+       ErrTokenFormat   = errors.New("badly formatted token")
+       ErrSalted        = errors.New("token already salted")
+)
+
+func SaltToken(token, remote string) (string, error) {
+       parts := strings.Split(token, "/")
+       if len(parts) < 3 || parts[0] != "v2" {
+               if reObsoleteToken.MatchString(token) {
+                       return "", ErrObsoleteToken
+               } else {
+                       return "", ErrTokenFormat
+               }
+       }
+       uuid := parts[1]
+       secret := parts[2]
+       if len(secret) != 40 {
+               // not already salted
+               secret = fmt.Sprintf("%x", hmac.New(sha1.New, []byte(secret)).Sum([]byte(remote)))
+               return "v2/" + uuid + "/" + secret, nil
+       } else if strings.HasPrefix(uuid, remote) {
+               // already salted for the desired remote
+               return token, nil
+       } else {
+               // salted for a different remote, can't be used
+               return "", ErrSalted
+       }
+}