16745: Reject unsupported APIs instead of mishandling.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 9479b58860bd99fb5ac57066c691158ad85a3fa5..620a21b883aa428748a8d4116abca74f8fe12c10 100644 (file)
--- 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 4b92d4dad35814b87ce9c7939694e79b5d60eaec..e60b55c935779aefeb30a2e57e2670def7c2ff83 100644 (file)
--- 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) {