From 241571abade72544a0987ec718d84a2b1eb58ed6 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Tue, 22 Sep 2026 21:56:32 +0400 Subject: [PATCH] fix: align signing region mismatch errors with s3 regional endpoints Fixes #2239 Running the gateway with a `--region` other than `us-east-1` turned every request from a client signed for a different region into a dead end. The signing region check correctly rejected the request with `AuthorizationHeaderMalformed`, but the gateway gave the client no usable way to learn which region it should have signed for, so the aws sdks could not re-sign and retry the way they do against a real s3 regional endpoint. The gap was `x-amz-bucket-region`. S3 reports the expected region in that response header as well as in the error body, because a `HEAD` request carries no body for the client to parse. The gateway set the header only on successful responses, so `HeadBucket` and `HeadObject` signed for the wrong region came back as a bare `400` with an empty body and some sdks region redirect had nothing to work with. Region mismatches now report the gateway region through a `RegionMismatchError` interface that `MalformedAuthError`, `AuthQueryParamError` and `InvalidArgumentError` implement, and the error dispatch points set the header from it alongside the existing `Allow` header handling for `MethodNotAllowed`. The error bodies were incomplete in the same way. Only the `Authorization` header path carried a `Region` element, while the presigned url and POST form paths reported the mismatch in prose alone. `AuthorizationQueryParametersError` is now backed by a dedicated `AuthQueryParamError` type that adds the element, and `InvalidArgumentError` grew an optional `Region` field for the POST form credential error, both omitted for every other error of those kinds. The messages themselves now read the way s3 writes them. The region and service names are quoted with apostrophes instead of `%q`, whose double quotes the xml encoder rendered as `"` in the raw response, and the POST form authentication fields are echoed back in `ArgumentName` under their canonical spelling, for example `X-Amz-Credential` rather than the lowercase form the browser submits, while fields outside the signing protocol such as `key` are still reported as submitted. --- s3api/controllers/base.go | 4 +++ s3api/utils/utils.go | 19 ++++++++++ s3api/utils/utils_test.go | 62 ++++++++++++++++++++++++++++++++ s3err/invalid-argument.go | 25 ++++++++++--- s3err/post-object.go | 52 +++++++++++++++++++++------ s3err/presigned-urls.go | 62 ++++++++++++++++++++++++++++---- s3err/s3err.go | 10 ++++++ s3err/sigv4.go | 5 ++- tests/integration/group-tests.go | 2 ++ tests/integration/sigv4_auth.go | 37 +++++++++++++++++++ tests/integration/utils.go | 30 ++++++++++++++++ website/handler.go | 1 + 12 files changed, 287 insertions(+), 22 deletions(-) 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))