From 2fe25e2b32042098106acead136fd3064bab30e3 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 2 Mar 2021 16:27:18 -0500 Subject: [PATCH] 16745: Reject unsupported APIs instead of mishandling. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- services/keep-web/s3.go | 25 ++++++++++++++++- services/keep-web/s3_test.go | 54 ++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go index 9479b58860..620a21b883 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, ``+h.Config.cluster.ClusterID+``) + fmt.Fprintln(w, ``+ + h.Config.cluster.ClusterID+ + ``) + } 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 4b92d4dad3..e60b55c935 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, "\nzzzzz\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) { -- 2.30.2