[ARVADOS] created: 2.1.0-213-gb14def1a3

Git user git at public.arvados.org
Thu Dec 10 21:47:29 UTC 2020


        at  b14def1a3c07506ef1251223224e88d2d0a1805d (commit)


commit b14def1a3c07506ef1251223224e88d2d0a1805d
Author: Tom Clegg <tom at curii.com>
Date:   Thu Dec 10 16:41:41 2020 -0500

    17208: Use normalized path to compute signatures.
    
    Transparently clean paths containing "//".
    
    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 97201d292..1c84976d2 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -75,6 +75,8 @@ func s3querystring(u *url.URL) string {
 	return strings.Join(keys, "&")
 }
 
+var reMultipleSlashChars = regexp.MustCompile(`//+`)
+
 func s3stringToSign(alg, scope, signedHeaders string, r *http.Request) (string, error) {
 	timefmt, timestr := "20060102T150405Z", r.Header.Get("X-Amz-Date")
 	if timestr == "" {
@@ -97,7 +99,10 @@ func s3stringToSign(alg, scope, signedHeaders string, r *http.Request) (string,
 		}
 	}
 
-	canonicalRequest := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s", r.Method, r.URL.EscapedPath(), s3querystring(r.URL), canonicalHeaders, signedHeaders, r.Header.Get("X-Amz-Content-Sha256"))
+	normalizedURL := *r.URL
+	normalizedURL.RawPath = ""
+	normalizedURL.Path = reMultipleSlashChars.ReplaceAllString(normalizedURL.Path, "/")
+	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
 }
@@ -259,7 +264,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
 		bucketName = strings.SplitN(strings.TrimPrefix(r.URL.Path, "/"), "/", 2)[0]
 		objectNameGiven = strings.Count(strings.TrimSuffix(r.URL.Path, "/"), "/") > 1
 	}
-	fspath += r.URL.Path
+	fspath += reMultipleSlashChars.ReplaceAllString(r.URL.Path, "/")
 
 	switch {
 	case r.Method == http.MethodGet && !objectNameGiven:
diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index a6aab357e..3a06270b0 100644
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -202,6 +202,11 @@ func (s *IntegrationSuite) testS3GetObject(c *check.C, bucket *s3.Bucket, prefix
 	c.Check(err, check.IsNil)
 	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
 	c.Check(resp.ContentLength, check.Equals, int64(4))
+
+	// HeadObject with superfluous leading slashes
+	exists, err = bucket.Exists(prefix + "//sailboat.txt")
+	c.Check(err, check.IsNil)
+	c.Check(exists, check.Equals, true)
 }
 
 func (s *IntegrationSuite) TestS3CollectionPutObjectSuccess(c *check.C) {
@@ -228,6 +233,18 @@ func (s *IntegrationSuite) testS3PutObjectSuccess(c *check.C, bucket *s3.Bucket,
 			path:        "newdir/newfile",
 			size:        1 << 26,
 			contentType: "application/octet-stream",
+		}, {
+			path:        "/aaa",
+			size:        2,
+			contentType: "application/octet-stream",
+		}, {
+			path:        "//bbb",
+			size:        2,
+			contentType: "application/octet-stream",
+		}, {
+			path:        "ccc//",
+			size:        0,
+			contentType: "application/x-directory",
 		}, {
 			path:        "newdir1/newdir2/newfile",
 			size:        0,
@@ -243,9 +260,14 @@ func (s *IntegrationSuite) testS3PutObjectSuccess(c *check.C, bucket *s3.Bucket,
 		objname := prefix + trial.path
 
 		_, err := bucket.GetReader(objname)
+		if !c.Check(err, check.NotNil) {
+			continue
+		}
 		c.Check(err.(*s3.Error).StatusCode, check.Equals, 404)
 		c.Check(err.(*s3.Error).Code, check.Equals, `NoSuchKey`)
-		c.Assert(err, check.ErrorMatches, `The specified key does not exist.`)
+		if !c.Check(err, check.ErrorMatches, `The specified key does not exist.`) {
+			continue
+		}
 
 		buf := make([]byte, trial.size)
 		rand.Read(buf)
@@ -363,14 +385,6 @@ func (s *IntegrationSuite) TestS3ProjectPutObjectFailure(c *check.C) {
 func (s *IntegrationSuite) testS3PutObjectFailure(c *check.C, bucket *s3.Bucket, prefix string) {
 	s.testServer.Config.cluster.Collections.S3FolderObjects = false
 
-	// Can't use V4 signature for these tests, because
-	// double-slash is incorrectly cleaned by the aws.V4Signature,
-	// resulting in a "bad signature" error. (Cleaning the path is
-	// appropriate for other services, but not in S3 where object
-	// names "foo//bar" and "foo/bar" are semantically different.)
-	bucket.S3.Auth = *(aws.NewAuth(arvadostest.ActiveToken, "none", "", time.Now().Add(time.Hour)))
-	bucket.S3.Signature = aws.V2Signature
-
 	var wg sync.WaitGroup
 	for _, trial := range []struct {
 		path string
@@ -393,8 +407,6 @@ func (s *IntegrationSuite) testS3PutObjectFailure(c *check.C, bucket *s3.Bucket,
 			path: "/",
 		}, {
 			path: "//",
-		}, {
-			path: "foo//bar",
 		}, {
 			path: "",
 		},
@@ -441,6 +453,17 @@ func (stage *s3stage) writeBigDirs(c *check.C, dirs int, filesPerDir int) {
 	c.Assert(fs.Sync(), check.IsNil)
 }
 
+func (s *IntegrationSuite) sign(c *check.C, req *http.Request, key, secret string) {
+	scope := "20200202/region/service/aws4_request"
+	signedHeaders := "date"
+	req.Header.Set("Date", time.Now().UTC().Format(time.RFC1123))
+	stringToSign, err := s3stringToSign(s3SignAlgorithm, scope, signedHeaders, req)
+	c.Assert(err, check.IsNil)
+	sig, err := s3signature(secret, scope, signedHeaders, stringToSign)
+	c.Assert(err, check.IsNil)
+	req.Header.Set("Authorization", s3SignAlgorithm+" Credential="+key+"/"+scope+", SignedHeaders="+signedHeaders+", Signature="+sig)
+}
+
 func (s *IntegrationSuite) TestS3VirtualHostStyleRequests(c *check.C) {
 	stage := s.s3setup(c)
 	defer stage.teardown(c)
@@ -487,12 +510,29 @@ func (s *IntegrationSuite) TestS3VirtualHostStyleRequests(c *check.C) {
 			responseCode:   http.StatusOK,
 			responseRegexp: []string{`boop`},
 		},
+		{
+			url:          "https://" + stage.projbucket.Name + ".example.com/" + stage.coll.Name + "//boop",
+			method:       "GET",
+			responseCode: http.StatusNotFound,
+		},
+		{
+			url:          "https://" + stage.projbucket.Name + ".example.com/" + stage.coll.Name + "//boop",
+			method:       "PUT",
+			body:         "boop",
+			responseCode: http.StatusOK,
+		},
+		{
+			url:            "https://" + stage.projbucket.Name + ".example.com/" + stage.coll.Name + "//boop",
+			method:         "GET",
+			responseCode:   http.StatusOK,
+			responseRegexp: []string{`boop`},
+		},
 	} {
 		url, err := url.Parse(trial.url)
 		c.Assert(err, check.IsNil)
 		req, err := http.NewRequest(trial.method, url.String(), bytes.NewReader([]byte(trial.body)))
 		c.Assert(err, check.IsNil)
-		req.Header.Set("Authorization", "AWS "+arvadostest.ActiveTokenV2+":none")
+		s.sign(c, req, arvadostest.ActiveTokenUUID, arvadostest.ActiveToken)
 		rr := httptest.NewRecorder()
 		s.testServer.Server.Handler.ServeHTTP(rr, req)
 		resp := rr.Result()

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list