diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 11be7824..ce47540a 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -208,6 +208,9 @@ func WrapMiddleware(handler fiber.Handler, logger s3log.AuditLogger, mm metrics. // for MethodNotAllowed errors, set the 'Allow' header ctx.Response().Header.Set("Allow", mnaErr.AllowedMethodsString()) } + // for signing region mismatches, set the + // 'x-amz-bucket-region' header + utils.SetRegionMismatchHeader(ctx, serr) return ctx.Status(serr.StatusCode()).Send(serr.XMLBody(requestID, hostID)) } @@ -262,6 +265,7 @@ func ProcessController(ctx fiber.Ctx, controller Controller, s3action string, sv if mnaErr, ok := serr.(s3err.MethodNotAllowedError); ok && len(mnaErr.AllowedMethods) != 0 { ctx.Response().Header.Set("Allow", mnaErr.AllowedMethodsString()) } + utils.SetRegionMismatchHeader(ctx, serr) return ctx.Status(serr.StatusCode()).Send(serr.XMLBody(requestID, hostID)) } diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index 841d6e94..6783ea2a 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1087,6 +1087,25 @@ func DetectResourceType(ctx fiber.Ctx) s3err.ResourceType { return s3err.ResourceTypeObject } +// SetRegionMismatchHeader reports the region the request should have been +// signed for in the x-amz-bucket-region response header, when err is a signing +// region mismatch. HEAD requests carry no response body, so the header is the +// only channel through which a client can discover the gateway region and +// retry against it, which is how the aws sdks recover from the mismatch. +func SetRegionMismatchHeader(ctx fiber.Ctx, err error) { + rerr, ok := err.(s3err.RegionMismatchError) + if !ok { + return + } + + region := rerr.ExpectedRegion() + if region == "" { + return + } + + ctx.Response().Header.Set("x-amz-bucket-region", region) +} + // ValidateLocationConstraint checks a CreateBucket location constraint. The // global endpoint serves us-east-1 and takes no constraint; any other // region is a region specific endpoint and requires the constraint to name it. diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index b7b5bf47..217d4bc6 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -28,6 +28,7 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/gofiber/fiber/v3" "github.com/stretchr/testify/assert" "github.com/valyala/fasthttp" "github.com/versity/versitygw/backend" @@ -1587,3 +1588,64 @@ func TestParseContentEncoding(t *testing.T) { }) } } + +func TestSetRegionMismatchHeader(t *testing.T) { + tests := []struct { + name string + err error + want string + }{ + { + "signature v4 header region mismatch", + s3err.MalformedAuth.IncorrectRegion("eu-west-1", "us-east-1"), + "eu-west-1", + }, + { + "presigned url region mismatch", + s3err.QueryAuthErrors.IncorrectRegion("eu-west-1", "us-east-1"), + "eu-west-1", + }, + { + "post object region mismatch", + s3err.PostAuth.IncorrectRegion("creds", "eu-west-1", "us-east-1"), + "eu-west-1", + }, + // only the region mismatch carries a region, the rest of the errors + // must not report one + { + "malformed auth error without a region", + s3err.MalformedAuth.MissingSignature(), + "", + }, + { + "presigned url error without a region", + s3err.QueryAuthErrors.IncorrectService("", "iam"), + "", + }, + { + "invalid argument error without a region", + s3err.GetInvalidArgumentErr(s3err.InvalidArgCopySourceBucket, "bucket"), + "", + }, + { + "unrelated s3 error", + s3err.GetAPIError(s3err.ErrNoSuchBucket), + "", + }, + { + "non s3 error", + errors.New("internal"), + "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := fiber.New().AcquireCtx(&fasthttp.RequestCtx{}) + + SetRegionMismatchHeader(ctx, tt.err) + + assert.Equal(t, tt.want, + string(ctx.Response().Header.Peek("x-amz-bucket-region"))) + }) + } +} diff --git a/s3err/invalid-argument.go b/s3err/invalid-argument.go index aad893f7..65ba8d8a 100644 --- a/s3err/invalid-argument.go +++ b/s3err/invalid-argument.go @@ -225,11 +225,13 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ } // InvalidArgumentError is returned when a request argument is invalid. -// Produces and fields in the XML response. +// Produces and fields in the XML response, and +// a field when the expected gateway region is known. type InvalidArgumentError struct { Description string ArgumentName string ArgumentValue string + Region string `xml:",omitempty"` } func (e InvalidArgumentError) BaseError() APIError { @@ -258,6 +260,7 @@ func (e InvalidArgumentError) XMLBody(requestID, hostID string) []byte { Message string ArgumentName string `xml:"ArgumentName,omitempty"` ArgumentValue string `xml:"ArgumentValue,omitempty"` + Region string `xml:"Region,omitempty"` RequestId string `xml:"RequestId,omitempty"` HostId string `xml:"HostId,omitempty"` }{ @@ -265,18 +268,30 @@ func (e InvalidArgumentError) XMLBody(requestID, hostID string) []byte { Message: e.Description, ArgumentName: e.ArgumentName, ArgumentValue: e.ArgumentValue, + Region: e.Region, RequestId: requestID, HostId: hostID, }) } func (e InvalidArgumentError) HTMLBody(requestID, hostID string) []byte { - return e.BaseError().encodeHTMLResponse(requestID, hostID, - ErrorField{Name: "ArgumentName", Value: e.ArgumentName}, - ErrorField{Name: "ArgumentValue", Value: e.ArgumentValue}, - ) + fields := []ErrorField{ + {Name: "ArgumentName", Value: e.ArgumentName}, + {Name: "ArgumentValue", Value: e.ArgumentValue}, + } + // Region is only set for the credential region mismatch; every other + // invalid argument error leaves it out. + if e.Region != "" { + fields = append(fields, ErrorField{Name: "Region", Value: e.Region}) + } + + return e.BaseError().encodeHTMLResponse(requestID, hostID, fields...) } +// ExpectedRegion implements RegionMismatchError. Only the credential region +// mismatch sets Region; every other invalid argument error returns "". +func (e InvalidArgumentError) ExpectedRegion() string { return e.Region } + func GetInvalidArgumentErr(code InvalidArgErrorCode, value string) InvalidArgumentError { err := invalidArgErrResponses[code] err.ArgumentValue = value diff --git a/s3err/post-object.go b/s3err/post-object.go index e99bce2a..7a5b7a26 100644 --- a/s3err/post-object.go +++ b/s3err/post-object.go @@ -17,10 +17,37 @@ package s3err import ( "fmt" "net/http" + "strings" ) +// argCredential is the canonical spelling of the POST form credential field. +// S3 echoes the form field names back in canonicalized, not as +// they are spelled in the submitted form. +const argCredential = "X-Amz-Credential" + +// canonicalPOSTFormFields maps the POST form authentication field names to the +// spelling S3 reports them with. Fields that are not part of the signing +// protocol, such as "key", are reported as submitted. +var canonicalPOSTFormFields = map[string]string{ + "x-amz-algorithm": "X-Amz-Algorithm", + "x-amz-credential": argCredential, + "x-amz-date": "X-Amz-Date", + "x-amz-signature": "X-Amz-Signature", + "x-amz-security-token": "X-Amz-Security-Token", +} + +// canonicalPOSTFormField returns the spelling S3 reports field with, leaving +// fields outside the signing protocol untouched. +func canonicalPOSTFormField(field string) string { + if canonical, ok := canonicalPOSTFormFields[strings.ToLower(field)]; ok { + return canonical + } + + return field +} + // Factory for building s3 object POST authentication errors. -func invalidPOSTObjectAuthErr(argName, argValue, format string, args ...any) S3Error { +func invalidPOSTObjectAuthErr(argName, argValue, format string, args ...any) InvalidArgumentError { return InvalidArgumentError{ ArgumentName: argName, ArgumentValue: argValue, @@ -32,7 +59,7 @@ type invalidPostAuthErr struct{} func (invalidPostAuthErr) InvalidDateFormat(creds, date string) S3Error { return invalidPOSTObjectAuthErr( - "x-amz-credential", + argCredential, creds, "incorrect date format %q. This date in the credential must be in the format \"yyyyMMdd\".", date, @@ -41,7 +68,7 @@ func (invalidPostAuthErr) InvalidDateFormat(creds, date string) S3Error { func (invalidPostAuthErr) MalformedCredential(creds string) S3Error { return invalidPOSTObjectAuthErr( - "x-amz-credential", + argCredential, creds, "the Credential is mal-formed; expecting \"/YYYYMMDD/REGION/SERVICE/aws4_request\".", ) @@ -49,7 +76,7 @@ func (invalidPostAuthErr) MalformedCredential(creds string) S3Error { func (invalidPostAuthErr) IncorrectTerminal(creds, terminal string) S3Error { return invalidPOSTObjectAuthErr( - "x-amz-credential", + argCredential, creds, "incorrect terminal %q. This endpoint uses \"aws4_request\".", terminal, @@ -57,18 +84,21 @@ func (invalidPostAuthErr) IncorrectTerminal(creds, terminal string) S3Error { } func (invalidPostAuthErr) IncorrectRegion(creds, expected, actual string) S3Error { - return invalidPOSTObjectAuthErr( - "x-amz-credential", + err := invalidPOSTObjectAuthErr( + argCredential, creds, - "the region %q is wrong; expecting %q", + "the region '%s' is wrong; expecting '%s'", actual, expected, ) + err.Region = expected + + return err } func (invalidPostAuthErr) IncorrectService(creds, service string) S3Error { return invalidPOSTObjectAuthErr( - "x-amz-credential", + argCredential, creds, "incorrect service %q. This endpoint belongs to \"s3\".", service, @@ -76,11 +106,13 @@ func (invalidPostAuthErr) IncorrectService(creds, service string) S3Error { } func (invalidPostAuthErr) MissingField(field string) S3Error { + name := canonicalPOSTFormField(field) + return invalidPOSTObjectAuthErr( - field, + name, "", "Bucket POST must contain a field named '%s'. If it is specified, please check the order of the fields.", - field, + name, ) } diff --git a/s3err/presigned-urls.go b/s3err/presigned-urls.go index 2204e7a6..c1e3775f 100644 --- a/s3err/presigned-urls.go +++ b/s3err/presigned-urls.go @@ -15,16 +15,64 @@ package s3err import ( + "encoding/xml" "fmt" "net/http" ) +// AuthQueryParamError is returned when the Signature V4 query parameters of a +// presigned URL are malformed. Produces a field in the XML response +// when the expected gateway region is known. +type AuthQueryParamError struct { + APIError + Region string +} + +func (e AuthQueryParamError) XMLBody(requestID, hostID string) []byte { + return encodeResponse(struct { + XMLName xml.Name `xml:"Error"` + Code string + Message string + Region string `xml:",omitempty"` + RequestID string `xml:"RequestId,omitempty"` + HostID string `xml:"HostId,omitempty"` + }{ + Code: e.Code, + Message: e.Description, + Region: e.Region, + RequestID: requestID, + HostID: hostID, + }) +} + +func (e AuthQueryParamError) HTMLBody(requestID, hostID string) []byte { + // Region is only set for the credential region mismatch; every other + // query parameter error leaves it out. + if e.Region == "" { + return e.APIError.encodeHTMLResponse(requestID, hostID) + } + + return e.APIError.encodeHTMLResponse(requestID, hostID, + ErrorField{Name: "Region", Value: e.Region}, + ) +} + +// ExpectedRegion implements RegionMismatchError. +func (e AuthQueryParamError) ExpectedRegion() string { return e.Region } + +func (e AuthQueryParamError) Is(target error) bool { + t, ok := target.(APIError) + return ok && e.APIError == t +} + // Factory for building AuthorizationQueryParametersError errors. -func authQueryParamError(format string, args ...any) S3Error { - return APIError{ - Code: "AuthorizationQueryParametersError", - Description: fmt.Sprintf(format, args...), - HTTPStatusCode: http.StatusBadRequest, +func authQueryParamError(format string, args ...any) AuthQueryParamError { + return AuthQueryParamError{ + APIError: APIError{ + Code: "AuthorizationQueryParametersError", + Description: fmt.Sprintf(format, args...), + HTTPStatusCode: http.StatusBadRequest, + }, } } @@ -43,7 +91,9 @@ func (queryAuthErrors) IncorrectService(_, s string) S3Error { } func (queryAuthErrors) IncorrectRegion(expected, actual string) S3Error { - return authQueryParamError(`Error parsing the X-Amz-Credential parameter; the region %q is wrong; expecting %q`, actual, expected) + err := authQueryParamError(`Error parsing the X-Amz-Credential parameter; the region '%s' is wrong; expecting '%s'`, actual, expected) + err.Region = expected + return err } func (queryAuthErrors) IncorrectTerminal(_, s string) S3Error { diff --git a/s3err/s3err.go b/s3err/s3err.go index 6262d77e..3548c757 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -35,6 +35,16 @@ type S3Error interface { HTMLBody(requestID, hostID string) []byte } +// RegionMismatchError is implemented by the error types that report a signing +// region mismatch. S3 reports the region the request should have been signed +// for in the x-amz-bucket-region response header as well as in the response +// body, since HEAD requests carry no body for the client to parse. +type RegionMismatchError interface { + // ExpectedRegion returns the region the gateway serves, or an empty + // string when the error is not a region mismatch. + ExpectedRegion() string +} + // APIError structure type APIError struct { Code string diff --git a/s3err/sigv4.go b/s3err/sigv4.go index ebb25be3..7667fd30 100644 --- a/s3err/sigv4.go +++ b/s3err/sigv4.go @@ -50,6 +50,9 @@ func (e MalformedAuthError) HTMLBody(requestID, hostID string) []byte { ) } +// ExpectedRegion implements RegionMismatchError. +func (e MalformedAuthError) ExpectedRegion() string { return e.Region } + func (e MalformedAuthError) Is(target error) bool { t, ok := target.(APIError) return ok && e.APIError == t @@ -98,7 +101,7 @@ func (malformedAuthErrors) IncorrectTerminal(_, s string) S3Error { } func (malformedAuthErrors) IncorrectRegion(expected, actual string) S3Error { - err := malformedAuthError("the region %q is wrong; expecting %q", actual, expected) + err := malformedAuthError("the region '%s' is wrong; expecting '%s'", actual, expected) err.Region = expected return err } diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index cf3493c6..7ea11a62 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -26,6 +26,7 @@ func TestAuthentication(ts *TestState) { ts.Run(Authentication_credentials_invalid_terminal) ts.Run(Authentication_credentials_incorrect_service) ts.Run(Authentication_credentials_incorrect_region) + ts.Run(Authentication_credentials_incorrect_region_head) ts.Run(Authentication_credentials_invalid_date) ts.Run(Authentication_credentials_future_date) ts.Run(Authentication_credentials_past_date) @@ -2303,6 +2304,7 @@ func GetIntTests() IntTests { "Authentication_credentials_invalid_terminal": Authentication_credentials_invalid_terminal, "Authentication_credentials_incorrect_service": Authentication_credentials_incorrect_service, "Authentication_credentials_incorrect_region": Authentication_credentials_incorrect_region, + "Authentication_credentials_incorrect_region_head": Authentication_credentials_incorrect_region_head, "Authentication_credentials_invalid_date": Authentication_credentials_invalid_date, "Authentication_credentials_future_date": Authentication_credentials_future_date, "Authentication_credentials_past_date": Authentication_credentials_past_date, diff --git a/tests/integration/sigv4_auth.go b/tests/integration/sigv4_auth.go index afdedfb4..551b43f0 100644 --- a/tests/integration/sigv4_auth.go +++ b/tests/integration/sigv4_auth.go @@ -267,6 +267,43 @@ func Authentication_credentials_incorrect_region(s *S3Conf) error { }) } +// Authentication_credentials_incorrect_region_head verifies that a HEAD request +// signed for the wrong region reports the gateway region in the +// x-amz-bucket-region response header. HEAD responses carry no body, so the +// header is the only channel through which a client can discover the region and +// retry against it, which is how the aws sdks recover from the mismatch. +func Authentication_credentials_incorrect_region_head(s *S3Conf) error { + testName := "Authentication_credentials_incorrect_region_head" + cfg := *s + if cfg.awsRegion == "us-east-1" { + cfg.awsRegion = "us-west-1" + } else { + cfg.awsRegion = "us-east-1" + } + return authHandler(&cfg, &authConfig{ + testName: testName, + path: getBucketName(), + method: http.MethodHead, + body: nil, + service: "s3", + date: time.Now(), + }, func(req *http.Request) error { + resp, err := s.httpClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + expected := s3err.MalformedAuth.IncorrectRegion(s.awsRegion, cfg.awsRegion) + if resp.StatusCode != expected.BaseError().HTTPStatusCode { + return fmt.Errorf("expected response status code to be %v, instead got %v", + expected.BaseError().HTTPStatusCode, resp.StatusCode) + } + + return checkRegionMismatchHeader(resp, expected) + }) +} + func Authentication_credentials_invalid_date(s *S3Conf) error { testName := "Authentication_credentials_invalid_date" return authHandler(s, &authConfig{ diff --git a/tests/integration/utils.go b/tests/integration/utils.go index e2a9650c..d7130a06 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -527,9 +527,36 @@ func checkHTTPResponseApiErr(resp *http.Response, expected s3err.S3Error) error if resp.StatusCode != apiErr.HTTPStatusCode { return fmt.Errorf("expected response status code to be %v, instead got %v", apiErr.HTTPStatusCode, resp.StatusCode) } + + if err := checkRegionMismatchHeader(resp, expected); err != nil { + return err + } + return compareS3ApiError(expected, &errResp) } +// checkRegionMismatchHeader verifies that a signing region mismatch reports the +// gateway region in the x-amz-bucket-region response header. HEAD requests +// carry no response body, so the header is the only channel through which a +// client can discover the region and retry against it. +func checkRegionMismatchHeader(resp *http.Response, expected s3err.S3Error) error { + rerr, ok := expected.(s3err.RegionMismatchError) + if !ok { + return nil + } + + region := rerr.ExpectedRegion() + if region == "" { + return nil + } + + if got := resp.Header.Get("x-amz-bucket-region"); got != region { + return fmt.Errorf("expected x-amz-bucket-region response header to be %v, instead got %v", region, got) + } + + return nil +} + // testEmptyVersionId verifies an action rejects an empty versionId query // parameter. The SDK drops empty query parameters, so the request has to be // signed and sent by hand. subresource is the action's query flag, e.g. @@ -841,6 +868,7 @@ func compareS3ApiErr(expected s3err.S3Error, received *APIErrorResponse) error { return compareS3ApiErrFields( compareErrField("ArgumentName", err.ArgumentName, received.ArgumentName), compareErrField("ArgumentValue", err.ArgumentValue, received.ArgumentValue), + compareErrField("Region", err.Region, received.Region), ) case s3err.InvalidChunkSizeError: return compareS3ApiErrFields( @@ -891,6 +919,8 @@ func compareS3ApiErr(expected s3err.S3Error, received *APIErrorResponse) error { ) case s3err.MalformedAuthError: return compareErrField("Region", err.Region, received.Region) + case s3err.AuthQueryParamError: + return compareErrField("Region", err.Region, received.Region) case s3err.HeadersNotSignedError: return compareErrField("HeadersNotSigned", err.HeadersNotSigned, received.HeadersNotSigned) case s3err.NoSuchUploadError: diff --git a/website/handler.go b/website/handler.go index 1ac38e74..36999d3b 100644 --- a/website/handler.go +++ b/website/handler.go @@ -588,6 +588,7 @@ func sendError(ctx fiber.Ctx, err error) error { if methodErr, ok := serr.(s3err.MethodNotAllowedError); ok && len(methodErr.AllowedMethods) != 0 { ctx.Response().Header.Set("Allow", methodErr.AllowedMethodsString()) } + utils.SetRegionMismatchHeader(ctx, serr) ctx.Response().Header.SetContentType(fiber.MIMETextHTMLCharsetUTF8) return ctx.Status(serr.StatusCode()).Send(serr.HTMLBody(requestId, hostId))