[ARVADOS] updated: 2.3.0-9-g7de380d5e
Git user
git at public.arvados.org
Wed Nov 10 20:12:49 UTC 2021
Summary of changes:
lib/controller/federation/conn.go | 14 ++++++-
lib/controller/federation/federation_test.go | 2 +-
lib/controller/integration_test.go | 60 ++++++++++++++++++++++++++++
sdk/go/arvados/container.go | 1 +
4 files changed, 74 insertions(+), 3 deletions(-)
via 7de380d5e7dbc3361c15d48d92619b222b77f6f8 (commit)
from 4c53d93b1c9356aea2c509fcfc79cc48aa0e2fa1 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 7de380d5e7dbc3361c15d48d92619b222b77f6f8
Author: Tom Clegg <tom at curii.com>
Date: Wed Nov 10 11:13:09 2021 -0500
18346: Do not forward locally-issued token to own login cluster.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index aa05cb1e6..39e4f2676 100644
--- 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 fdc4d96cf..984d32dc3 100644
--- 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 8a23bccfb..f2f88eb25 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -961,3 +961,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 at 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 384bebb59..7c68bdb20 100644
--- 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.
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list