From bdd74e4dff9d9c5c1a329cfbed2e2e08336c8c51 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 24 Aug 2021 16:07:34 -0400 Subject: [PATCH] 17217: Sign manifests in controller instead of RailsAPI. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/controller/localdb/collection.go | 68 +++++++++++++++++++ lib/controller/localdb/collection_test.go | 80 +++++++++++++++++++++++ services/api/app/models/collection.rb | 12 +--- 3 files changed, 149 insertions(+), 11 deletions(-) create mode 100644 lib/controller/localdb/collection.go create mode 100644 lib/controller/localdb/collection_test.go diff --git a/lib/controller/localdb/collection.go b/lib/controller/localdb/collection.go new file mode 100644 index 0000000000..529310519a --- /dev/null +++ b/lib/controller/localdb/collection.go @@ -0,0 +1,68 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: AGPL-3.0 + +package localdb + +import ( + "context" + "time" + + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/auth" +) + +// CollectionGet defers to railsProxy for everything except blob +// signatures. +func (conn *Conn) CollectionGet(ctx context.Context, opts arvados.GetOptions) (arvados.Collection, error) { + if len(opts.Select) > 0 { + // We need to know IsTrashed and TrashAt to implement + // signing properly, even if the caller doesn't want + // them. + opts.Select = append([]string{"is_trashed", "trash_at"}, opts.Select...) + } + resp, err := conn.railsProxy.CollectionGet(ctx, opts) + if err != nil { + return resp, err + } + conn.signCollection(ctx, &resp) + return resp, nil +} + +// CollectionList defers to railsProxy for everything except blob +// signatures. +func (conn *Conn) CollectionList(ctx context.Context, opts arvados.ListOptions) (arvados.CollectionList, error) { + if len(opts.Select) > 0 { + // We need to know IsTrashed and TrashAt to implement + // signing properly, even if the caller doesn't want + // them. + opts.Select = append([]string{"is_trashed", "trash_at"}, opts.Select...) + } + resp, err := conn.railsProxy.CollectionList(ctx, opts) + if err != nil { + return resp, err + } + for i := range resp.Items { + conn.signCollection(ctx, &resp.Items[i]) + } + return resp, nil +} + +func (conn *Conn) signCollection(ctx context.Context, coll *arvados.Collection) { + if coll.IsTrashed || coll.ManifestText == "" { + return + } + var token string + if creds, ok := auth.FromContext(ctx); ok && len(creds.Tokens) > 0 { + token = creds.Tokens[0] + } + if token == "" { + return + } + ttl := conn.cluster.Collections.BlobSigningTTL.Duration() + exp := time.Now().Add(ttl) + if coll.TrashAt != nil && !coll.TrashAt.IsZero() && coll.TrashAt.Before(exp) { + exp = *coll.TrashAt + } + coll.ManifestText = arvados.SignManifest(coll.ManifestText, token, exp, ttl, []byte(conn.cluster.Collections.BlobSigningKey)) +} diff --git a/lib/controller/localdb/collection_test.go b/lib/controller/localdb/collection_test.go new file mode 100644 index 0000000000..d75ca3c406 --- /dev/null +++ b/lib/controller/localdb/collection_test.go @@ -0,0 +1,80 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: AGPL-3.0 + +package localdb + +import ( + "context" + + "git.arvados.org/arvados.git/lib/config" + "git.arvados.org/arvados.git/lib/controller/rpc" + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/arvadostest" + "git.arvados.org/arvados.git/sdk/go/auth" + "git.arvados.org/arvados.git/sdk/go/ctxlog" + check "gopkg.in/check.v1" +) + +var _ = check.Suite(&CollectionSuite{}) + +type CollectionSuite struct { + cluster *arvados.Cluster + localdb *Conn + railsSpy *arvadostest.Proxy +} + +func (s *CollectionSuite) TearDownSuite(c *check.C) { + // Undo any changes/additions to the user database so they + // don't affect subsequent tests. + arvadostest.ResetEnv() + c.Check(arvados.NewClientFromEnv().RequestAndDecode(nil, "POST", "database/reset", nil, nil), check.IsNil) +} + +func (s *CollectionSuite) SetUpTest(c *check.C) { + cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load() + c.Assert(err, check.IsNil) + s.cluster, err = cfg.GetCluster("") + c.Assert(err, check.IsNil) + s.localdb = NewConn(s.cluster) + s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI) + *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider) +} + +func (s *CollectionSuite) TearDownTest(c *check.C) { + s.railsSpy.Close() +} + +func (s *CollectionSuite) TestSignatures(c *check.C) { + ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.ActiveTokenV2}}) + + resp, err := s.localdb.CollectionGet(ctx, arvados.GetOptions{UUID: arvadostest.FooCollection}) + c.Check(err, check.IsNil) + c.Check(resp.ManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3\+A[0-9a-f]+@[0-9a-f]+ 0:.*`) + + resp, err = s.localdb.CollectionGet(ctx, arvados.GetOptions{UUID: arvadostest.FooCollection, Select: []string{"manifest_text"}}) + c.Check(err, check.IsNil) + c.Check(resp.ManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3\+A[0-9a-f]+@[0-9a-f]+ 0:.*`) + + lresp, err := s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}}}) + c.Check(err, check.IsNil) + if c.Check(lresp.Items, check.HasLen, 1) { + c.Check(lresp.Items[0].UUID, check.Equals, arvadostest.FooCollection) + c.Check(lresp.Items[0].ManifestText, check.Equals, "") + c.Check(lresp.Items[0].UnsignedManifestText, check.Equals, "") + } + + lresp, err = s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}}, Select: []string{"manifest_text"}}) + c.Check(err, check.IsNil) + if c.Check(lresp.Items, check.HasLen, 1) { + c.Check(lresp.Items[0].ManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3\+A[0-9a-f]+@[0-9a-f]+ 0:.*`) + c.Check(lresp.Items[0].UnsignedManifestText, check.Equals, "") + } + + lresp, err = s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}}, Select: []string{"unsigned_manifest_text"}}) + c.Check(err, check.IsNil) + if c.Check(lresp.Items, check.HasLen, 1) { + c.Check(lresp.Items[0].ManifestText, check.Equals, "") + c.Check(lresp.Items[0].UnsignedManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3 0:.*`) + } +} diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb index 1bbe8cc661..2560ac2c98 100644 --- a/services/api/app/models/collection.rb +++ b/services/api/app/models/collection.rb @@ -420,17 +420,7 @@ class Collection < ArvadosModel end def self.sign_manifest manifest, token, exp=nil - if exp.nil? - exp = db_current_time.to_i + Rails.configuration.Collections.BlobSigningTTL.to_i - end - signing_opts = { - api_token: token, - expire: exp, - } - m = munge_manifest_locators(manifest) do |match| - Blob.sign_locator(match[0], signing_opts) - end - return m + return manifest end def self.munge_manifest_locators manifest -- 2.30.2