From 3659ea41692068b1da69ff813284b091cebb8671 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curii.com>
Date: Wed, 2 Oct 2024 12:35:34 -0400
Subject: [PATCH] 21566: Fix V4 signature check for paths with double slash.

Unfortunately goamz squashes repeated slashes with path.Join() so we
can't use it to test such cases.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---
 services/keep-web/s3.go      |  4 ++--
 services/keep-web/s3_test.go | 26 +++++++-------------------
 2 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 75dc8f98e5..6f4d49e2f4 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -186,7 +186,7 @@ func s3stringToSign(alg, scope, signedHeaders string, r *http.Request) (string,
 	}
 
 	normalizedPath := normalizePath(r.URL.Path)
-	ctxlog.FromContext(r.Context()).Debugf("normalizedPath %q", normalizedPath)
+	ctxlog.FromContext(r.Context()).Debugf("normalizedPath %s", normalizedPath)
 	canonicalRequest := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s", r.Method, normalizedPath, 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
@@ -201,7 +201,7 @@ func normalizePath(s string) string {
 	// even chars like ";" and "," that are not normally
 	// percent-encoded in paths.
 	out := ""
-	for _, c := range []byte(reMultipleSlashChars.ReplaceAllString(s, "/")) {
+	for _, c := range []byte(s) {
 		if (c >= 'a' && c <= 'z') ||
 			(c >= 'A' && c <= 'Z') ||
 			(c >= '0' && c <= '9') ||
diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index f8dc8add94..1ba299bf51 100644
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -424,7 +424,7 @@ func (s *IntegrationSuite) testS3GetObject(c *check.C, bucket *s3.Bucket, prefix
 	// HeadObject with superfluous leading slashes
 	exists, err = bucket.Exists(prefix + "//sailboat.txt")
 	c.Check(err, check.IsNil)
-	c.Check(exists, check.Equals, true)
+	c.Check(exists, check.Equals, false)
 }
 
 func (s *IntegrationSuite) checkMetaEquals(c *check.C, hdr http.Header, expect map[string]string) {
@@ -526,15 +526,7 @@ func (s *IntegrationSuite) testS3PutObjectSuccess(c *check.C, bucket *s3.Bucket,
 			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//",
+			path:        "ccc/",
 			size:        0,
 			contentType: "application/x-directory",
 		}, {
@@ -698,16 +690,8 @@ func (s *IntegrationSuite) testS3PutObjectFailure(c *check.C, bucket *s3.Bucket,
 			path: "emptydir", // dir already exists, see s3setup()
 		}, {
 			path: "emptydir/",
-		}, {
-			path: "emptydir//",
 		}, {
 			path: "newdir/",
-		}, {
-			path: "newdir//",
-		}, {
-			path: "/",
-		}, {
-			path: "//",
 		}, {
 			path: "",
 		},
@@ -865,7 +849,11 @@ func (s *IntegrationSuite) TestS3NormalizeURIForSignature(c *check.C) {
 		{"/foo%2fbar", "/foo/bar"},                 // / must not be escaped
 		{"/(foo)/[];,", "/%28foo%29/%5B%5D%3B%2C"}, // ()[];, must be escaped
 		{"/foo%5bbar", "/foo%5Bbar"},               // %XX must be uppercase
-		{"//foo///.bar", "/foo/.bar"},              // "//" and "///" must be squashed to "/"
+		// unicode chars must be UTF-8 encoded and escaped
+		{"/\u26f5", "/%E2%9B%B5"},
+		// "//" and "///" must not be squashed -- see example,
+		// https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
+		{"//foo///.bar", "//foo///.bar"},
 	} {
 		c.Logf("trial %q", trial)
 
-- 
2.39.5