Merge pull request #2431 from versity/sis/region-mismatch-error-parity

fix: align signing region mismatch errors with s3 regional endpoints

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))
 		}
 

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.

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")))
+		})
+	}
+}

s3err/invalid-argument.go

@@ -225,11 +225,13 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
 }
 
 // InvalidArgumentError is returned when a request argument is invalid.
-// Produces <ArgumentName> and <ArgumentValue> fields in the XML response.
+// Produces <ArgumentName> and <ArgumentValue> fields in the XML response, and
+// a <Region> 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

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 <ArgumentName> 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 \"<YOUR-AKID>/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,
 	)
 }
 

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 <Region> 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 {

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

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
 }

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)
@@ -2315,6 +2316,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,

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{

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:

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))