18346: Do not forward locally-issued token to own login cluster.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index aa05cb1e6d58bb954e4573e5c54b4416d6f671d3..39e4f2676695bed5fd478a2b1470de1e1d63b6f8 100644 (file)
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -37,7 +37,7 @@ func New(cluster *arvados.Cluster) *Conn {
                if !remote.Proxy || id == cluster.ClusterID {
                        continue
                }
-               conn := rpc.NewConn(id, &url.URL{Scheme: remote.Scheme, Host: remote.Host}, remote.Insecure, saltedTokenProvider(local, id))
+               conn := rpc.NewConn(id, &url.URL{Scheme: remote.Scheme, Host: remote.Host}, remote.Insecure, saltedTokenProvider(cluster, local, id))
                // Older versions of controller rely on the Via header
                // to detect loops.
                conn.SendHeader = http.Header{"Via": {"HTTP/1.1 arvados-controller"}}
@@ -55,7 +55,7 @@ func New(cluster *arvados.Cluster) *Conn {
 // tokens from an incoming request context, determines whether they
 // should (and can) be salted for the given remoteID, and returns the
 // resulting tokens.
-func saltedTokenProvider(local backend, remoteID string) rpc.TokenProvider {
+func saltedTokenProvider(cluster *arvados.Cluster, local backend, remoteID string) rpc.TokenProvider {
        return func(ctx context.Context) ([]string, error) {
                var tokens []string
                incoming, ok := auth.FromContext(ctx)
@@ -63,6 +63,16 @@ func saltedTokenProvider(local backend, remoteID string) rpc.TokenProvider {
                        return nil, errors.New("no token provided")
                }
                for _, token := range incoming.Tokens {
+                       if strings.HasPrefix(token, "v2/"+cluster.ClusterID+"-") && remoteID == cluster.Login.LoginCluster {
+                               // If we did this, the login cluster
+                               // would call back to us and then
+                               // reject our response because the
+                               // user UUID prefix (i.e., the
+                               // LoginCluster prefix) won't match
+                               // the token UUID prefix (i.e., our
+                               // prefix).
+                               return nil, httpErrorf(http.StatusUnauthorized, "cannot use a locally issued token to forward a request to our login cluster (%s)", remoteID)
+                       }
                        salted, err := auth.SaltToken(token, remoteID)
                        switch err {
                        case nil:

diff --git a/lib/controller/federation/federation_test.go b/lib/controller/federation/federation_test.go
index fdc4d96cfaa90b3e28dd2048c2c5bd0f73bf9dc5..984d32dc3d9a294fc20bd60b38295aaf890abb37 100644 (file)
--- a/lib/controller/federation/federation_test.go
+++ b/lib/controller/federation/federation_test.go
@@ -93,5 +93,5 @@ func (s *FederationSuite) addHTTPRemote(c *check.C, id string, backend backend)
                Host:   srv.Addr,
                Proxy:  true,
        }
-       s.fed.remotes[id] = rpc.NewConn(id, &url.URL{Scheme: "http", Host: srv.Addr}, true, saltedTokenProvider(s.fed.local, id))
+       s.fed.remotes[id] = rpc.NewConn(id, &url.URL{Scheme: "http", Host: srv.Addr}, true, saltedTokenProvider(s.cluster, s.fed.local, id))
 }

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 02061547bf5b826b1618ac37298a876cd205b30b..4cf6a683287ae211f58f13cdb664d087f2ceff00 100644 (file)
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -959,3 +959,63 @@ func (s *IntegrationSuite) TestOIDCAccessTokenAuth(c *check.C) {
                }
        }
 }
+
+// z3333 should not forward a locally-issued container runtime token,
+// associated with a z1111 user, to its login cluster z1111. z1111
+// would only call back to z3333 and then reject the response because
+// the user ID does not match the token prefix. See
+// dev.arvados.org/issues/18346
+func (s *IntegrationSuite) TestForwardRuntimeTokenToLoginCluster(c *check.C) {
+       db3, db3conn := s.dbConn(c, "z3333")
+       defer db3.Close()
+       defer db3conn.Close()
+       rootctx1, _, _ := s.testClusters["z1111"].RootClients()
+       rootctx3, _, _ := s.testClusters["z3333"].RootClients()
+       conn1 := s.testClusters["z1111"].Conn()
+       conn3 := s.testClusters["z3333"].Conn()
+       userctx1, _, _, _ := s.testClusters["z1111"].UserClients(rootctx1, c, conn1, "user@example.com", true)
+
+       user1, err := conn1.UserGetCurrent(userctx1, arvados.GetOptions{})
+       c.Assert(err, check.IsNil)
+       c.Logf("user1 %+v", user1)
+
+       imageColl, err := conn3.CollectionCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
+               "manifest_text": ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.tar\n",
+       }})
+       c.Assert(err, check.IsNil)
+       c.Logf("imageColl %+v", imageColl)
+
+       cr, err := conn3.ContainerRequestCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
+               "state":           "Committed",
+               "command":         []string{"echo"},
+               "container_image": imageColl.PortableDataHash,
+               "cwd":             "/",
+               "output_path":     "/",
+               "priority":        1,
+               "runtime_constraints": arvados.RuntimeConstraints{
+                       VCPUs: 1,
+                       RAM:   1000000000,
+               },
+       }})
+       c.Assert(err, check.IsNil)
+       c.Logf("container request %+v", cr)
+       ctr, err := conn3.ContainerLock(rootctx3, arvados.GetOptions{UUID: cr.ContainerUUID})
+       c.Assert(err, check.IsNil)
+       c.Logf("container %+v", ctr)
+
+       // We could use conn3.ContainerAuth() here, but that API
+       // hasn't been added to sdk/go/arvados/api.go yet.
+       row := db3conn.QueryRowContext(context.Background(), `SELECT api_token from api_client_authorizations where uuid=$1`, ctr.AuthUUID)
+       c.Check(row, check.NotNil)
+       var val sql.NullString
+       row.Scan(&val)
+       c.Assert(val.Valid, check.Equals, true)
+       runtimeToken := "v2/" + ctr.AuthUUID + "/" + val.String
+       ctrctx, _, _ := s.testClusters["z3333"].ClientsWithToken(runtimeToken)
+       c.Logf("container runtime token %+v", runtimeToken)
+
+       _, err = conn3.UserGet(ctrctx, arvados.GetOptions{UUID: user1.UUID})
+       c.Assert(err, check.NotNil)
+       c.Check(err, check.ErrorMatches, `request failed: .* 401 Unauthorized: cannot use a locally issued token to forward a request to our login cluster \(z1111\)`)
+       c.Check(err, check.Not(check.ErrorMatches), `(?ms).*127\.0\.0\.11.*`)
+}

diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index 384bebb5997ee86b1b1be2396498f1554ee32ecc..7c68bdb20222f59067b5c5f1d89bad8ea6fef5fe 100644 (file)
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -36,6 +36,7 @@ type Container struct {
        RuntimeUserUUID           string                 `json:"runtime_user_uuid"`
        RuntimeAuthScopes         []string               `json:"runtime_auth_scopes"`
        RuntimeToken              string                 `json:"runtime_token"`
+       AuthUUID                  string                 `json:"auth_uuid"`
 }
 
 // ContainerRequest is an arvados#container_request resource.