[ARVADOS] updated: 2.1.0-449-gfdc4063c9

Git user git at public.arvados.org
Tue Mar 2 20:59:58 UTC 2021


Summary of changes:
 services/keep-web/cache.go   |  5 ++--
 services/keep-web/s3.go      | 28 +++++++++++++++++++++++
 services/keep-web/s3_test.go | 54 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 82 insertions(+), 5 deletions(-)

       via  fdc4063c901cc5f326046dbb15ec6a52ccc0edce (commit)
       via  b6131783bf7f0ca8035b1461688af09c292b8e7f (commit)
       via  7d3775f7dcd87bb5c210e33ff099460074080749 (commit)
      from  8c2bbecb1c09fbc3818dc1a2d73b3fda2ba68e02 (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 fdc4063c901cc5f326046dbb15ec6a52ccc0edce
Author: Tom Clegg <tom at curii.com>
Date:   Tue Mar 2 15:58:56 2021 -0500

    16745: Reject unsupported APIs instead of mishandling.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 9479b5886..4e8634c36 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -102,6 +102,7 @@ func s3stringToSign(alg, scope, signedHeaders string, r *http.Request) (string,
 	normalizedURL := *r.URL
 	normalizedURL.RawPath = ""
 	normalizedURL.Path = reMultipleSlashChars.ReplaceAllString(normalizedURL.Path, "/")
+	ctxlog.FromContext(r.Context()).Infof("escapedPath %s", normalizedURL.EscapedPath())
 	canonicalRequest := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s", r.Method, normalizedURL.EscapedPath(), s3querystring(r.URL), canonicalHeaders, signedHeaders, r.Header.Get("X-Amz-Content-Sha256"))
 	ctxlog.FromContext(r.Context()).Debugf("s3stringToSign: canonicalRequest %s", canonicalRequest)
 	return fmt.Sprintf("%s\n%s\n%s\n%s", alg, r.Header.Get("X-Amz-Date"), scope, hashdigest(sha256.New(), canonicalRequest)), nil
@@ -221,6 +222,8 @@ var UnauthorizedAccess = "UnauthorizedAccess"
 var InvalidRequest = "InvalidRequest"
 var SignatureDoesNotMatch = "SignatureDoesNotMatch"
 
+var reRawQueryIndicatesAPI = regexp.MustCompile(`[a-z]+(&.*)?`)
+
 // serveS3 handles r and returns true if r is a request from an S3
 // client, otherwise it returns false.
 func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
@@ -292,13 +295,23 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
 			// GetBucketLocation
 			w.Header().Set("Content-Type", "application/xml")
 			io.WriteString(w, xml.Header)
-			fmt.Fprintln(w, `<LocationConstraint xmlns="http://s3.amazonaws.com/doc/2006-03-01/">`+h.Config.cluster.ClusterID+`</LocationConstraint>`)
+			fmt.Fprintln(w, `<LocationConstraint><LocationConstraint xmlns="http://s3.amazonaws.com/doc/2006-03-01/">`+
+				h.Config.cluster.ClusterID+
+				`</LocationConstraint></LocationConstraint>`)
+		} else if reRawQueryIndicatesAPI.MatchString(r.URL.RawQuery) {
+			// GetBucketWebsite ("GET /bucketid/?website"), GetBucketTagging, etc.
+			s3ErrorResponse(w, InvalidRequest, "API not supported", r.URL.Path+"?"+r.URL.RawQuery, http.StatusBadRequest)
 		} else {
 			// ListObjects
 			h.s3list(bucketName, w, r, fs)
 		}
 		return true
 	case r.Method == http.MethodGet || r.Method == http.MethodHead:
+		if reRawQueryIndicatesAPI.MatchString(r.URL.RawQuery) {
+			// GetObjectRetention ("GET /bucketid/objectid?retention&versionID=..."), etc.
+			s3ErrorResponse(w, InvalidRequest, "API not supported", r.URL.Path+"?"+r.URL.RawQuery, http.StatusBadRequest)
+			return true
+		}
 		fi, err := fs.Stat(fspath)
 		if r.Method == "HEAD" && !objectNameGiven {
 			// HeadBucket
@@ -328,6 +341,11 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
 		http.FileServer(fs).ServeHTTP(w, &r)
 		return true
 	case r.Method == http.MethodPut:
+		if reRawQueryIndicatesAPI.MatchString(r.URL.RawQuery) {
+			// PutObjectAcl ("PUT /bucketid/objectid?acl&versionID=..."), etc.
+			s3ErrorResponse(w, InvalidRequest, "API not supported", r.URL.Path+"?"+r.URL.RawQuery, http.StatusBadRequest)
+			return true
+		}
 		if !objectNameGiven {
 			s3ErrorResponse(w, InvalidArgument, "Missing object name in PUT request.", r.URL.Path, http.StatusBadRequest)
 			return true
@@ -424,6 +442,11 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
 		w.WriteHeader(http.StatusOK)
 		return true
 	case r.Method == http.MethodDelete:
+		if reRawQueryIndicatesAPI.MatchString(r.URL.RawQuery) {
+			// DeleteObjectTagging ("DELETE /bucketid/objectid?tagging&versionID=..."), etc.
+			s3ErrorResponse(w, InvalidRequest, "API not supported", r.URL.Path+"?"+r.URL.RawQuery, http.StatusBadRequest)
+			return true
+		}
 		if !objectNameGiven || r.URL.Path == "/" {
 			s3ErrorResponse(w, InvalidArgument, "missing object name in DELETE request", r.URL.Path, http.StatusBadRequest)
 			return true
diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index 4b92d4dad..e60b55c93 100644
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -76,7 +76,7 @@ func (s *IntegrationSuite) s3setup(c *check.C) s3stage {
 
 	auth := aws.NewAuth(arvadostest.ActiveTokenUUID, arvadostest.ActiveToken, "", time.Now().Add(time.Hour))
 	region := aws.Region{
-		Name:       s.testServer.Addr,
+		Name:       "zzzzz",
 		S3Endpoint: "http://" + s.testServer.Addr,
 	}
 	client := s3.New(*auth, region)
@@ -455,7 +455,7 @@ func (stage *s3stage) writeBigDirs(c *check.C, dirs int, filesPerDir int) {
 }
 
 func (s *IntegrationSuite) sign(c *check.C, req *http.Request, key, secret string) {
-	scope := "20200202/region/service/aws4_request"
+	scope := "20200202/zzzzz/service/aws4_request"
 	signedHeaders := "date"
 	req.Header.Set("Date", time.Now().UTC().Format(time.RFC1123))
 	stringToSign, err := s3stringToSign(s3SignAlgorithm, scope, signedHeaders, req)
@@ -560,7 +560,7 @@ func (s *IntegrationSuite) TestS3NormalizeURIForSignature(c *check.C) {
 		{"/foo%5bbar", "/foo%5Bbar"}, // %XX must be uppercase
 	} {
 		date := time.Now().UTC().Format("20060102T150405Z")
-		scope := "20200202/fakeregion/S3/aws4_request"
+		scope := "20200202/zzzzz/S3/aws4_request"
 		canonicalRequest := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s", "GET", trial.normalizedPath, "", "host:host.example.com\n", "host", "")
 		c.Logf("canonicalRequest %q", canonicalRequest)
 		expect := fmt.Sprintf("%s\n%s\n%s\n%s", s3SignAlgorithm, date, scope, hashdigest(sha256.New(), canonicalRequest))
@@ -579,6 +579,23 @@ func (s *IntegrationSuite) TestS3NormalizeURIForSignature(c *check.C) {
 	}
 }
 
+func (s *IntegrationSuite) TestS3GetBucketLocation(c *check.C) {
+	stage := s.s3setup(c)
+	defer stage.teardown(c)
+	for _, bucket := range []*s3.Bucket{stage.collbucket, stage.projbucket} {
+		req, err := http.NewRequest("GET", bucket.URL("/"), nil)
+		c.Check(err, check.IsNil)
+		req.Header.Set("Authorization", "AWS "+arvadostest.ActiveTokenV2+":none")
+		req.URL.RawQuery = "location"
+		resp, err := http.DefaultClient.Do(req)
+		c.Assert(err, check.IsNil)
+		c.Check(resp.Header.Get("Content-Type"), check.Equals, "application/xml")
+		buf, err := ioutil.ReadAll(resp.Body)
+		c.Assert(err, check.IsNil)
+		c.Check(string(buf), check.Equals, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<LocationConstraint><LocationConstraint xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">zzzzz</LocationConstraint></LocationConstraint>\n")
+	}
+}
+
 func (s *IntegrationSuite) TestS3GetBucketVersioning(c *check.C) {
 	stage := s.s3setup(c)
 	defer stage.teardown(c)
@@ -596,6 +613,37 @@ func (s *IntegrationSuite) TestS3GetBucketVersioning(c *check.C) {
 	}
 }
 
+func (s *IntegrationSuite) TestS3UnsupportedAPIs(c *check.C) {
+	stage := s.s3setup(c)
+	defer stage.teardown(c)
+	for _, trial := range []struct {
+		method   string
+		path     string
+		rawquery string
+	}{
+		{"GET", "/", "acl&versionId=1234"},    // GetBucketAcl
+		{"GET", "/foo", "acl&versionId=1234"}, // GetObjectAcl
+		{"PUT", "/", "acl"},                   // PutBucketAcl
+		{"PUT", "/foo", "acl"},                // PutObjectAcl
+		{"DELETE", "/", "tagging"},            // DeleteBucketTagging
+		{"DELETE", "/foo", "tagging"},         // DeleteObjectTagging
+	} {
+		for _, bucket := range []*s3.Bucket{stage.collbucket, stage.projbucket} {
+			c.Logf("trial %v bucket %v", trial, bucket)
+			req, err := http.NewRequest(trial.method, bucket.URL(trial.path), nil)
+			c.Check(err, check.IsNil)
+			req.Header.Set("Authorization", "AWS "+arvadostest.ActiveTokenV2+":none")
+			req.URL.RawQuery = trial.rawquery
+			resp, err := http.DefaultClient.Do(req)
+			c.Assert(err, check.IsNil)
+			c.Check(resp.Header.Get("Content-Type"), check.Equals, "application/xml")
+			buf, err := ioutil.ReadAll(resp.Body)
+			c.Assert(err, check.IsNil)
+			c.Check(string(buf), check.Matches, "(?ms).*InvalidRequest.*API not supported.*")
+		}
+	}
+}
+
 // If there are no CommonPrefixes entries, the CommonPrefixes XML tag
 // should not appear at all.
 func (s *IntegrationSuite) TestS3ListNoCommonPrefixes(c *check.C) {

commit b6131783bf7f0ca8035b1461688af09c292b8e7f
Author: Tom Clegg <tom at curii.com>
Date:   Tue Mar 2 11:12:11 2021 -0500

    16745: Handle GetBucketLocation API.
    
    Previously misinterpreted as ListObjects with no delimiter, which is
    very slow.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index d500f1e65..9479b5886 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -288,6 +288,11 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
 			w.Header().Set("Content-Type", "application/xml")
 			io.WriteString(w, xml.Header)
 			fmt.Fprintln(w, `<VersioningConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"/>`)
+		} else if _, ok = r.URL.Query()["location"]; ok {
+			// GetBucketLocation
+			w.Header().Set("Content-Type", "application/xml")
+			io.WriteString(w, xml.Header)
+			fmt.Fprintln(w, `<LocationConstraint xmlns="http://s3.amazonaws.com/doc/2006-03-01/">`+h.Config.cluster.ClusterID+`</LocationConstraint>`)
 		} else {
 			// ListObjects
 			h.s3list(bucketName, w, r, fs)

commit 7d3775f7dcd87bb5c210e33ff099460074080749
Author: Tom Clegg <tom at curii.com>
Date:   Tue Mar 2 09:55:16 2021 -0500

    16745: Don't store nil in an atomic.Value (panic).
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index af143c77e..ec48be609 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -218,6 +218,7 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, error) {
 	now := time.Now()
 	ent, _ := c.sessions.Get(token)
 	sess, _ := ent.(*cachedSession)
+	expired := false
 	if sess == nil {
 		c.metrics.sessionMisses.Inc()
 		sess = &cachedSession{
@@ -226,13 +227,13 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, error) {
 		c.sessions.Add(token, sess)
 	} else if sess.expire.Before(now) {
 		c.metrics.sessionMisses.Inc()
-		sess.fs.Store(nil)
+		expired = true
 	} else {
 		c.metrics.sessionHits.Inc()
 	}
 	go c.pruneSessions()
 	fs, _ := sess.fs.Load().(arvados.CustomFileSystem)
-	if fs != nil {
+	if fs != nil && !expired {
 		return fs, nil
 	}
 	ac, err := arvados.NewClientFromConfig(c.cluster)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list