From d8861941ef704dd729252ee1592e48a082798b65 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 6 Feb 2020 14:16:55 -0500 Subject: [PATCH] 16133: Don't take multiple federation hops. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/controller/federation/conn.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go index 2aebc0e970..42083cb83d 100644 --- a/lib/controller/federation/conn.go +++ b/lib/controller/federation/conn.go @@ -120,8 +120,13 @@ func (conn *Conn) chooseBackend(id string) backend { // or "" for the local backend. // // A non-nil error means all backends failed. -func (conn *Conn) tryLocalThenRemotes(ctx context.Context, fn func(context.Context, string, backend) error) error { - if err := fn(ctx, "", conn.local); err == nil || errStatus(err) != http.StatusNotFound { +func (conn *Conn) tryLocalThenRemotes(ctx context.Context, forwardedFor string, fn func(context.Context, string, backend) error) error { + if err := fn(ctx, "", conn.local); err == nil || errStatus(err) != http.StatusNotFound || forwardedFor != "" { + // Note: forwardedFor != "" means this request came + // from a remote cluster, so we don't take a second + // hop. This avoids cycles, redundant calls to a + // mutually reachable remote, and use of double-salted + // tokens. return err } @@ -218,8 +223,6 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva } func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions) (arvados.Collection, error) { - downstream := options.ForwardedFor - options.ForwardedFor = conn.cluster.ClusterID + "-" + downstream if len(options.UUID) == 27 { // UUID is really a UUID c, err := conn.chooseBackend(options.UUID).CollectionGet(ctx, options) @@ -230,17 +233,10 @@ func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions) } else { // UUID is a PDH first := make(chan arvados.Collection, 1) - err := conn.tryLocalThenRemotes(ctx, func(ctx context.Context, remoteID string, be backend) error { - if remoteID != "" && downstream != "" { - // If remoteID isn't in downstream, we - // might find the collection by taking - // another hop, but we don't bother: - // token salting and blob signature - // rewriting don't work over multiple - // hops. - return notFoundError{} - } - c, err := be.CollectionGet(ctx, options) + err := conn.tryLocalThenRemotes(ctx, options.ForwardedFor, func(ctx context.Context, remoteID string, be backend) error { + remoteOpts := options + remoteOpts.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor + c, err := be.CollectionGet(ctx, remoteOpts) if err != nil { return err } -- 2.30.2