[ARVADOS] created: 1.2.0-145-g1e27ecf13

Git user git at public.curoverse.com
Thu Oct 4 01:29:17 EDT 2018


        at  1e27ecf1368de5932c4af00c6cff9595c501f5f6 (commit)


commit 1e27ecf1368de5932c4af00c6cff9595c501f5f6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Oct 4 01:27:19 2018 -0400

    14199: Store data if X-Keep-Signature given in proxied GET/HEAD req.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 2426c9cbd..2210c8c6d 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -101,9 +101,14 @@ func (rtr *router) handleGET(resp http.ResponseWriter, req *http.Request) {
 	ctx, cancel := contextForResponse(context.TODO(), resp)
 	defer cancel()
 
+	// Intervening proxies must not return a cached GET response
+	// to a prior request if a X-Keep-Signature request header has
+	// been added or changed.
+	resp.Header().Add("Vary", "X-Keep-Signature")
+
 	locator := req.URL.Path[1:]
 	if strings.Contains(locator, "+R") && !strings.Contains(locator, "+A") {
-		rtr.remoteProxy.Get(resp, req, rtr.cluster)
+		rtr.remoteProxy.Get(ctx, resp, req, rtr.cluster)
 		return
 	}
 
diff --git a/services/keepstore/proxy_remote.go b/services/keepstore/proxy_remote.go
index 2e3d66351..aaa1a0188 100644
--- a/services/keepstore/proxy_remote.go
+++ b/services/keepstore/proxy_remote.go
@@ -5,10 +5,14 @@
 package main
 
 import (
+	"context"
+	"errors"
 	"io"
 	"net/http"
+	"regexp"
 	"strings"
 	"sync"
+	"time"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
@@ -21,7 +25,28 @@ type remoteProxy struct {
 	mtx     sync.Mutex
 }
 
-func (rp *remoteProxy) Get(w http.ResponseWriter, r *http.Request, cluster *arvados.Cluster) {
+func (rp *remoteProxy) Get(ctx context.Context, w http.ResponseWriter, r *http.Request, cluster *arvados.Cluster) {
+	token := GetAPIToken(r)
+	if token == "" {
+		http.Error(w, "no token provided in Authorization header", http.StatusUnauthorized)
+		return
+	}
+	if sign := r.Header.Get("X-Keep-Signature"); sign != "" {
+		buf, err := getBufferWithContext(ctx, bufs, BlockSize)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusServiceUnavailable)
+			return
+		}
+		defer bufs.Put(buf)
+		rrc := &remoteResponseCacher{
+			Locator:        r.URL.Path[1:],
+			Token:          token,
+			Buffer:         buf[:0],
+			ResponseWriter: w,
+		}
+		defer rrc.Flush(ctx)
+		w = rrc
+	}
 	var remoteClient *keepclient.KeepClient
 	var parts []string
 	for i, part := range strings.Split(r.URL.Path[1:], "+") {
@@ -38,11 +63,6 @@ func (rp *remoteProxy) Get(w http.ResponseWriter, r *http.Request, cluster *arva
 				http.Error(w, "remote cluster not configured", http.StatusBadGateway)
 				return
 			}
-			token := GetAPIToken(r)
-			if token == "" {
-				http.Error(w, "no token provided in Authorization header", http.StatusUnauthorized)
-				return
-			}
 			kc, err := rp.remoteClient(remoteID, remote, token)
 			if err == auth.ErrObsoleteToken {
 				http.Error(w, err.Error(), http.StatusBadRequest)
@@ -111,3 +131,62 @@ func (rp *remoteProxy) remoteClient(remoteID string, remoteCluster arvados.Remot
 	kccopy.Arvados.ApiToken = token
 	return &kccopy, nil
 }
+
+var localOrRemoteSignature = regexp.MustCompile(`\+[AR][^\+]*`)
+
+// remoteResponseCacher wraps http.ResponseWriter. It buffers the
+// response data in the provided buffer, writes/touches a copy on a
+// local volume, adds a response header with a locally-signed locator,
+// and finally writes the data through.
+type remoteResponseCacher struct {
+	Locator string
+	Token   string
+	Buffer  []byte
+	http.ResponseWriter
+	statusCode int
+}
+
+func (rrc *remoteResponseCacher) Write(p []byte) (int, error) {
+	if len(rrc.Buffer)+len(p) > cap(rrc.Buffer) {
+		return 0, errors.New("buffer full")
+	}
+	rrc.Buffer = append(rrc.Buffer, p...)
+	return len(p), nil
+}
+
+func (rrc *remoteResponseCacher) WriteHeader(statusCode int) {
+	rrc.statusCode = statusCode
+}
+
+func (rrc *remoteResponseCacher) Flush(ctx context.Context) {
+	if rrc.statusCode == 0 {
+		rrc.statusCode = http.StatusOK
+	} else if rrc.statusCode != http.StatusOK {
+		rrc.ResponseWriter.WriteHeader(rrc.statusCode)
+		rrc.ResponseWriter.Write(rrc.Buffer)
+		return
+	}
+	_, err := PutBlock(ctx, rrc.Buffer, rrc.Locator[:32])
+	if err == RequestHashError {
+		http.Error(rrc.ResponseWriter, "checksum mismatch in remote response", http.StatusBadGateway)
+		return
+	}
+	if err, ok := err.(*KeepError); ok {
+		http.Error(rrc.ResponseWriter, err.Error(), err.HTTPCode)
+		return
+	}
+	if err != nil {
+		http.Error(rrc.ResponseWriter, err.Error(), http.StatusBadGateway)
+		return
+	}
+
+	unsigned := localOrRemoteSignature.ReplaceAllLiteralString(rrc.Locator, "")
+	signed := SignLocator(unsigned, rrc.Token, time.Now().Add(theConfig.BlobSignatureTTL.Duration()))
+	if signed == unsigned {
+		http.Error(rrc.ResponseWriter, "could not sign locator", http.StatusInternalServerError)
+		return
+	}
+	rrc.Header().Set("X-Keep-Locator", signed)
+	rrc.ResponseWriter.WriteHeader(rrc.statusCode)
+	rrc.ResponseWriter.Write(rrc.Buffer)
+}
diff --git a/services/keepstore/proxy_remote_test.go b/services/keepstore/proxy_remote_test.go
index b15e0b068..6e720b849 100644
--- a/services/keepstore/proxy_remote_test.go
+++ b/services/keepstore/proxy_remote_test.go
@@ -99,6 +99,7 @@ func (s *ProxyRemoteSuite) SetUpTest(c *check.C) {
 	KeepVM = s.vm
 	theConfig = DefaultConfig()
 	theConfig.systemAuthToken = arvadostest.DataManagerToken
+	theConfig.blobSigningKey = []byte(knownKey)
 	theConfig.Start()
 	s.rtr = MakeRESTRouter(s.cluster)
 }
@@ -122,28 +123,59 @@ func (s *ProxyRemoteSuite) TestProxyRemote(c *check.C) {
 
 	for _, trial := range []struct {
 		label            string
+		method           string
 		token            string
+		xKeepSignature   string
 		expectRemoteReqs int64
 		expectCode       int
+		expectSignature  bool
 	}{
 		{
-			label:            "happy path",
+			label:            "GET only",
+			method:           "GET",
 			token:            arvadostest.ActiveTokenV2,
 			expectRemoteReqs: 1,
 			expectCode:       http.StatusOK,
 		},
 		{
 			label:            "obsolete token",
+			method:           "GET",
 			token:            arvadostest.ActiveToken,
 			expectRemoteReqs: 0,
 			expectCode:       http.StatusBadRequest,
 		},
 		{
 			label:            "bad token",
+			method:           "GET",
 			token:            arvadostest.ActiveTokenV2[:len(arvadostest.ActiveTokenV2)-3] + "xxx",
 			expectRemoteReqs: 1,
 			expectCode:       http.StatusNotFound,
 		},
+		{
+			label:            "HEAD only",
+			method:           "HEAD",
+			token:            arvadostest.ActiveTokenV2,
+			expectRemoteReqs: 1,
+			expectCode:       http.StatusOK,
+		},
+		{
+			label:            "HEAD with local signature",
+			method:           "HEAD",
+			xKeepSignature:   "local, time=" + time.Now().Format(time.RFC3339),
+			token:            arvadostest.ActiveTokenV2,
+			expectRemoteReqs: 1,
+			expectCode:       http.StatusOK,
+			expectSignature:  true,
+		},
+		{
+			label:            "GET with local signature",
+			method:           "GET",
+			xKeepSignature:   "local, time=" + time.Now().Format(time.RFC3339),
+			token:            arvadostest.ActiveTokenV2,
+			expectRemoteReqs: 1,
+			expectCode:       http.StatusOK,
+			expectSignature:  true,
+		},
 	} {
 		c.Logf("trial: %s", trial.label)
 
@@ -151,8 +183,11 @@ func (s *ProxyRemoteSuite) TestProxyRemote(c *check.C) {
 
 		var req *http.Request
 		var resp *httptest.ResponseRecorder
-		req = httptest.NewRequest("GET", path, nil)
+		req = httptest.NewRequest(trial.method, path, nil)
 		req.Header.Set("Authorization", "Bearer "+trial.token)
+		if trial.xKeepSignature != "" {
+			req.Header.Set("X-Keep-Signature", trial.xKeepSignature)
+		}
 		resp = httptest.NewRecorder()
 		s.rtr.ServeHTTP(resp, req)
 		c.Check(s.remoteKeepRequests, check.Equals, trial.expectRemoteReqs)
@@ -162,5 +197,25 @@ func (s *ProxyRemoteSuite) TestProxyRemote(c *check.C) {
 		} else {
 			c.Check(resp.Body.String(), check.Not(check.Equals), string(data))
 		}
+
+		c.Check(resp.Header().Get("Vary"), check.Matches, `(.*, )?X-Keep-Signature(, .*)?`)
+
+		locHdr := resp.Header().Get("X-Keep-Locator")
+		if !trial.expectSignature {
+			c.Check(locHdr, check.Equals, "")
+			continue
+		}
+
+		c.Check(locHdr, check.Not(check.Equals), "")
+		c.Check(locHdr, check.Not(check.Matches), `.*\+R.*`)
+		c.Check(VerifySignature(locHdr, trial.token), check.IsNil)
+
+		// Ensure block can be requested using new signature
+		req = httptest.NewRequest("GET", "/"+locHdr, nil)
+		req.Header.Set("Authorization", "Bearer "+trial.token)
+		resp = httptest.NewRecorder()
+		s.rtr.ServeHTTP(resp, req)
+		c.Check(resp.Code, check.Equals, http.StatusOK)
+		c.Check(s.remoteKeepRequests, check.Equals, trial.expectRemoteReqs)
 	}
 }

commit a1745a13f68363598cc121b617436f3ac5b1654d
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Oct 3 15:54:45 2018 -0400

    14199: Refactor test case.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keepstore/proxy_remote_test.go b/services/keepstore/proxy_remote_test.go
index 4c5051380..b15e0b068 100644
--- a/services/keepstore/proxy_remote_test.go
+++ b/services/keepstore/proxy_remote_test.go
@@ -120,30 +120,47 @@ func (s *ProxyRemoteSuite) TestProxyRemote(c *check.C) {
 
 	path := "/" + strings.Replace(s.remoteKeepLocator, "+A", "+R"+s.remoteClusterID+"-", 1)
 
-	var req *http.Request
-	var resp *httptest.ResponseRecorder
-	tryWithToken := func(token string) {
+	for _, trial := range []struct {
+		label            string
+		token            string
+		expectRemoteReqs int64
+		expectCode       int
+	}{
+		{
+			label:            "happy path",
+			token:            arvadostest.ActiveTokenV2,
+			expectRemoteReqs: 1,
+			expectCode:       http.StatusOK,
+		},
+		{
+			label:            "obsolete token",
+			token:            arvadostest.ActiveToken,
+			expectRemoteReqs: 0,
+			expectCode:       http.StatusBadRequest,
+		},
+		{
+			label:            "bad token",
+			token:            arvadostest.ActiveTokenV2[:len(arvadostest.ActiveTokenV2)-3] + "xxx",
+			expectRemoteReqs: 1,
+			expectCode:       http.StatusNotFound,
+		},
+	} {
+		c.Logf("trial: %s", trial.label)
+
+		s.remoteKeepRequests = 0
+
+		var req *http.Request
+		var resp *httptest.ResponseRecorder
 		req = httptest.NewRequest("GET", path, nil)
-		req.Header.Set("Authorization", "Bearer "+token)
+		req.Header.Set("Authorization", "Bearer "+trial.token)
 		resp = httptest.NewRecorder()
 		s.rtr.ServeHTTP(resp, req)
+		c.Check(s.remoteKeepRequests, check.Equals, trial.expectRemoteReqs)
+		c.Check(resp.Code, check.Equals, trial.expectCode)
+		if resp.Code == http.StatusOK {
+			c.Check(resp.Body.String(), check.Equals, string(data))
+		} else {
+			c.Check(resp.Body.String(), check.Not(check.Equals), string(data))
+		}
 	}
-
-	// Happy path
-	tryWithToken(arvadostest.ActiveTokenV2)
-	c.Check(s.remoteKeepRequests, check.Equals, int64(1))
-	c.Check(resp.Code, check.Equals, http.StatusOK)
-	c.Check(resp.Body.String(), check.Equals, string(data))
-
-	// Obsolete token
-	tryWithToken(arvadostest.ActiveToken)
-	c.Check(s.remoteKeepRequests, check.Equals, int64(1))
-	c.Check(resp.Code, check.Equals, http.StatusBadRequest)
-	c.Check(resp.Body.String(), check.Not(check.Equals), string(data))
-
-	// Bad token
-	tryWithToken(arvadostest.ActiveTokenV2[:len(arvadostest.ActiveTokenV2)-3] + "xxx")
-	c.Check(s.remoteKeepRequests, check.Equals, int64(2))
-	c.Check(resp.Code, check.Equals, http.StatusNotFound)
-	c.Check(resp.Body.String(), check.Not(check.Equals), string(data))
 }

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list