From 163038b37d90e9fe17286bc450a573c8e2320107 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Wed, 24 Aug 2022 16:12:05 +0300 Subject: [PATCH] [#672] Fix handling X-Amz-Copy-Source header Signed-off-by: Denis Kirillov --- api/auth/center.go | 14 +++---- api/auth/center_test.go | 2 +- api/auth/regexp_utils.go | 10 ++++- api/handler/copy.go | 23 +++++++---- api/handler/copy_test.go | 72 +++++++++++++++++++++++++++++++++ api/handler/multipart_upload.go | 6 ++- api/router.go | 4 +- 7 files changed, 110 insertions(+), 21 deletions(-) diff --git a/api/auth/center.go b/api/auth/center.go index acbaad94..bc9107d0 100644 --- a/api/auth/center.go +++ b/api/auth/center.go @@ -37,8 +37,8 @@ type ( } center struct { - reg *regexpSubmatcher - postReg *regexpSubmatcher + reg *RegexpSubmatcher + postReg *RegexpSubmatcher cli tokens.Credentials } @@ -88,13 +88,13 @@ var _ io.ReadSeeker = prs(0) func New(neoFS tokens.NeoFS, key *keys.PrivateKey, config *cache.Config) Center { return ¢er{ cli: tokens.New(neoFS, key, config), - reg: ®expSubmatcher{re: authorizationFieldRegexp}, - postReg: ®expSubmatcher{re: postPolicyCredentialRegexp}, + reg: NewRegexpMatcher(authorizationFieldRegexp), + postReg: NewRegexpMatcher(postPolicyCredentialRegexp), } } func (c *center) parseAuthHeader(header string) (*authHeader, error) { - submatches := c.reg.getSubmatches(header) + submatches := c.reg.GetSubmatches(header) if len(submatches) != authHeaderPartsNum { return nil, apiErrors.GetAPIError(apiErrors.ErrAuthorizationHeaderMalformed) } @@ -203,7 +203,7 @@ func (c *center) checkFormData(r *http.Request) (*accessbox.Box, error) { return nil, ErrNoAuthorizationHeader } - submatches := c.postReg.getSubmatches(MultipartFormValue(r, "x-amz-credential")) + submatches := c.postReg.GetSubmatches(MultipartFormValue(r, "x-amz-credential")) if len(submatches) != 4 { return nil, apiErrors.GetAPIError(apiErrors.ErrAuthorizationHeaderMalformed) } @@ -277,7 +277,7 @@ func (c *center) checkSign(authHeader *authHeader, box *accessbox.Box, request * if _, err := signer.Sign(request, nil, authHeader.Service, authHeader.Region, signatureDateTime); err != nil { return fmt.Errorf("failed to sign temporary HTTP request: %w", err) } - signature = c.reg.getSubmatches(request.Header.Get(AuthorizationHdr))["v4_signature"] + signature = c.reg.GetSubmatches(request.Header.Get(AuthorizationHdr))["v4_signature"] } if authHeader.SignatureV4 != signature { diff --git a/api/auth/center_test.go b/api/auth/center_test.go index 8318b5be..8b7a8d8a 100644 --- a/api/auth/center_test.go +++ b/api/auth/center_test.go @@ -13,7 +13,7 @@ func TestAuthHeaderParse(t *testing.T) { defaultHeader := "AWS4-HMAC-SHA256 Credential=oid0cid/20210809/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=2811ccb9e242f41426738fb1f" center := ¢er{ - reg: ®expSubmatcher{re: authorizationFieldRegexp}, + reg: NewRegexpMatcher(authorizationFieldRegexp), } for _, tc := range []struct { diff --git a/api/auth/regexp_utils.go b/api/auth/regexp_utils.go index 1e9b2dde..4b05e802 100644 --- a/api/auth/regexp_utils.go +++ b/api/auth/regexp_utils.go @@ -2,11 +2,17 @@ package auth import "regexp" -type regexpSubmatcher struct { +type RegexpSubmatcher struct { re *regexp.Regexp } -func (r *regexpSubmatcher) getSubmatches(target string) map[string]string { +// NewRegexpMatcher creates a new regexp sub matcher. +func NewRegexpMatcher(re *regexp.Regexp) *RegexpSubmatcher { + return &RegexpSubmatcher{re: re} +} + +// GetSubmatches returns matches from provided string. Zero length indicates no match. +func (r *RegexpSubmatcher) GetSubmatches(target string) map[string]string { matches := r.re.FindStringSubmatch(target) l := len(matches) diff --git a/api/handler/copy.go b/api/handler/copy.go index 69ee9578..50b11b8e 100644 --- a/api/handler/copy.go +++ b/api/handler/copy.go @@ -3,10 +3,11 @@ package handler import ( "net/http" "net/url" - "strings" + "regexp" "time" "github.com/nspcc-dev/neofs-s3-gw/api" + "github.com/nspcc-dev/neofs-s3-gw/api/auth" "github.com/nspcc-dev/neofs-s3-gw/api/data" "github.com/nspcc-dev/neofs-s3-gw/api/errors" "github.com/nspcc-dev/neofs-s3-gw/api/layer" @@ -25,14 +26,16 @@ const ( copyDirective = "COPY" ) +var copySourceMatcher = auth.NewRegexpMatcher(regexp.MustCompile(`^/?(?P[a-z0-9.\-]{3,63})/(?P.+)$`)) + // path2BucketObject returns a bucket and an object. -func path2BucketObject(path string) (bucket, prefix string) { - path = strings.TrimPrefix(path, api.SlashSeparator) - m := strings.Index(path, api.SlashSeparator) - if m < 0 { - return path, "" +func path2BucketObject(path string) (string, string, error) { + matches := copySourceMatcher.GetSubmatches(path) + if len(matches) != 2 { + return "", "", errors.GetAPIError(errors.ErrInvalidRequest) } - return path[:m], path[m+len(api.SlashSeparator):] + + return matches["bucket_name"], matches["object_name"], nil } func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { @@ -59,7 +62,11 @@ func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { src = u.Path } - srcBucket, srcObject := path2BucketObject(src) + srcBucket, srcObject, err := path2BucketObject(src) + if err != nil { + h.logAndSendError(w, "invalid source copy", reqInfo, err) + return + } p := &layer.HeadObjectParams{ Object: srcObject, diff --git a/api/handler/copy_test.go b/api/handler/copy_test.go index 2a13ab03..01b412a5 100644 --- a/api/handler/copy_test.go +++ b/api/handler/copy_test.go @@ -93,3 +93,75 @@ func getObjectTagging(t *testing.T, tc *handlerContext, bktName, objName, versio require.NoError(t, err) return tagging } + +func TestSourceCopyRegexp(t *testing.T) { + for _, tc := range []struct { + path string + err bool + bktName string + objName string + }{ + { + path: "/bucket/object", + err: false, + bktName: "bucket", + objName: "object", + }, + { + path: "bucket/object", + err: false, + bktName: "bucket", + objName: "object", + }, + { + path: "sub-bucket/object", + err: false, + bktName: "sub-bucket", + objName: "object", + }, + { + path: "bucket.domain/object", + err: false, + bktName: "bucket.domain", + objName: "object", + }, + { + path: "bucket/object/deep", + err: false, + bktName: "bucket", + objName: "object/deep", + }, + { + path: "bucket", + err: true, + }, + { + path: "/bucket", + err: true, + }, + { + path: "invalid+bucket/object", + err: true, + }, + { + path: "invaliDBucket/object", + err: true, + }, + { + path: "i/object", + err: true, + }, + } { + t.Run("", func(t *testing.T) { + bktName, objName, err := path2BucketObject(tc.path) + if tc.err { + require.Error(t, err) + return + } + + require.NoError(t, err) + require.Equal(t, tc.bktName, bktName) + require.Equal(t, tc.objName, objName) + }) + } +} diff --git a/api/handler/multipart_upload.go b/api/handler/multipart_upload.go index 1dca5e45..8e9d62f8 100644 --- a/api/handler/multipart_upload.go +++ b/api/handler/multipart_upload.go @@ -266,7 +266,11 @@ func (h *handler) UploadPartCopy(w http.ResponseWriter, r *http.Request) { versionID = u.Query().Get(api.QueryVersionID) src = u.Path } - srcBucket, srcObject := path2BucketObject(src) + srcBucket, srcObject, err := path2BucketObject(src) + if err != nil { + h.logAndSendError(w, "invalid source copy", reqInfo, err) + return + } srcRange, err := parseRange(r.Header.Get(api.AmzCopySourceRange)) if err != nil { diff --git a/api/router.go b/api/router.go index f6c5c10e..d5b37520 100644 --- a/api/router.go +++ b/api/router.go @@ -219,7 +219,7 @@ func Attach(r *mux.Router, domains []string, m MaxClients, h Handler, center aut bucket.Methods(http.MethodHead).Path("/{object:.+}").HandlerFunc( m.Handle(metrics.APIStats("headobject", h.HeadObjectHandler))).Name("HeadObject") // CopyObjectPart - bucket.Methods(http.MethodPut).Path("/{object:.+}").HeadersRegexp(hdrAmzCopySource, ".*?(\\/|%2F).*?").HandlerFunc(m.Handle(metrics.APIStats("uploadpartcopy", h.UploadPartCopy))).Queries("partNumber", "{partNumber:[0-9]+}", "uploadId", "{uploadId:.*}"). + bucket.Methods(http.MethodPut).Path("/{object:.+}").Headers(hdrAmzCopySource, "").HandlerFunc(m.Handle(metrics.APIStats("uploadpartcopy", h.UploadPartCopy))).Queries("partNumber", "{partNumber:[0-9]+}", "uploadId", "{uploadId:.*}"). Name("UploadPartCopy") // PutObjectPart bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc( @@ -286,7 +286,7 @@ func Attach(r *mux.Router, domains []string, m MaxClients, h Handler, center aut m.Handle(metrics.APIStats("getobject", h.GetObjectHandler))). Name("GetObject") // CopyObject - bucket.Methods(http.MethodPut).Path("/{object:.+}").HeadersRegexp(hdrAmzCopySource, ".*?(\\/|%2F).*?").HandlerFunc(m.Handle(metrics.APIStats("copyobject", h.CopyObjectHandler))). + bucket.Methods(http.MethodPut).Path("/{object:.+}").Headers(hdrAmzCopySource, "").HandlerFunc(m.Handle(metrics.APIStats("copyobject", h.CopyObjectHandler))). Name("CopyObject") // PutObjectRetention bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(