Merge pull request #2389 from dualfroz/fix-illegal-location-constraint

Return IllegalLocationConstraintException for a mismatched LocationConstraint

s3api/controllers/bucket-put.go

@@ -658,18 +658,19 @@ func (c S3ApiController) CreateBucket(ctx fiber.Ctx) (*Response, error) {
 				},
 			}, s3err.GetAPIError(s3err.ErrMalformedXML)
 		}
+	}
 
-		if body.LocationConstraint != nil {
-			region := utils.ContextKeyRegion.Get(ctx).(string)
-			if *body.LocationConstraint != region || *body.LocationConstraint == "us-east-1" {
-				debuglogger.Logf("invalid location constraint: %s", *body.LocationConstraint)
-				return &Response{
-					MetaOpts: &MetaOptions{
-						BucketOwner: bucketOwner.Access,
-					},
-				}, s3err.GetInvalidLocationConstraintErr(*body.LocationConstraint)
-			}
-		}
+	region, ok := utils.ContextKeyRegion.Get(ctx).(string)
+	if !ok {
+		region = defaultRegion
+	}
+
+	if err := utils.ValidateLocationConstraint(body.LocationConstraint, region); err != nil {
+		return &Response{
+			MetaOpts: &MetaOptions{
+				BucketOwner: bucketOwner.Access,
+			},
+		}, err
 	}
 
 	defACL := auth.ACL{

s3api/controllers/bucket-put_test.go

@@ -726,6 +726,11 @@ func TestS3ApiController_CreateBucket(t *testing.T) {
 	})
 	assert.NoError(t, err)
 
+	euLocConstBody, err := xml.Marshal(s3response.CreateBucketConfiguration{
+		LocationConstraint: utils.GetStringPtr("eu-central-1"),
+	})
+	assert.NoError(t, err)
+
 	tests := []struct {
 		name   string
 		input  testInput
@@ -812,6 +817,59 @@ func TestS3ApiController_CreateBucket(t *testing.T) {
 				err: s3err.GetInvalidLocationConstraintErr("us-west-1"),
 			},
 		},
+		{
+			name: "illegal location constraint",
+			input: testInput{
+				locals: map[utils.ContextKey]any{
+					utils.ContextKeyAccount: adminAcc,
+					utils.ContextKeyRegion:  "us-west-1",
+				},
+				body: euLocConstBody,
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{BucketOwner: adminAcc.Access},
+				},
+				err: s3err.GetIllegalLocationConstraintErr("eu-central-1"),
+			},
+		},
+		{
+			name: "missing location constraint",
+			input: testInput{
+				locals: map[utils.ContextKey]any{
+					utils.ContextKeyAccount: adminAcc,
+					utils.ContextKeyRegion:  "us-west-1",
+				},
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{BucketOwner: adminAcc.Access},
+				},
+				err: s3err.GetIllegalLocationConstraintErr(""),
+			},
+		},
+		{
+			name: "matching location constraint",
+			input: testInput{
+				locals: map[utils.ContextKey]any{
+					utils.ContextKeyAccount: adminAcc,
+					utils.ContextKeyRegion:  "us-west-1",
+				},
+				bucket: "my-bucket",
+				body:   invLocConstBody,
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{
+						BucketOwner: adminAcc.Access,
+					},
+					Headers: map[string]*string{
+						"Location":         utils.GetStringPtr("/my-bucket"),
+						"x-amz-bucket-arn": utils.GetStringPtr("arn:aws:s3:::my-bucket"),
+					},
+				},
+			},
+		},
 		{
 			name: "invalid ownership",
 			input: testInput{

s3api/utils/utils.go

@@ -1086,3 +1086,29 @@ func DetectResourceType(ctx fiber.Ctx) s3err.ResourceType {
 
 	return s3err.ResourceTypeObject
 }
+
+// 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.
+func ValidateLocationConstraint(constraint *string, region string) error {
+	if region == "us-east-1" {
+		if constraint != nil {
+			debuglogger.Logf("invalid location constraint: %s", *constraint)
+			return s3err.GetInvalidLocationConstraintErr(*constraint)
+		}
+
+		return nil
+	}
+
+	if constraint == nil {
+		debuglogger.Logf("missing location constraint for region %s", region)
+		return s3err.GetIllegalLocationConstraintErr("")
+	}
+
+	if *constraint != region {
+		debuglogger.Logf("illegal location constraint %s for region %s", *constraint, region)
+		return s3err.GetIllegalLocationConstraintErr(*constraint)
+	}
+
+	return nil
+}

s3err/s3err.go

@@ -984,6 +984,20 @@ func GetCopySourceObjectTooLargeErr(limit int64) APIError {
 	}
 }
 
+// Returns illegal location constraint APIError. An empty constraint stands for
+// a CreateBucket request that carried no LocationConstraint at all.
+func GetIllegalLocationConstraintErr(constraint string) APIError {
+	if constraint == "" {
+		constraint = "unspecified"
+	}
+
+	return APIError{
+		Code:           "IllegalLocationConstraintException",
+		Description:    fmt.Sprintf("The %s location constraint is incompatible for the region specific endpoint this request was sent to.", constraint),
+		HTTPStatusCode: http.StatusBadRequest,
+	}
+}
+
 func GetInvalidRedirectCodeErr(input int) APIError {
 	return APIError{
 		Code:           "InvalidRequest",

tests/commands/create_bucket.sh

@@ -26,13 +26,17 @@ create_bucket() {
   fi
 
   local exit_code=0 error
+  local location_args=()
+  if [ -n "$AWS_REGION" ] && [ "$AWS_REGION" != "us-east-1" ]; then
+    location_args=(--create-bucket-configuration LocationConstraint="$AWS_REGION")
+  fi
   if [[ $1 == 's3' ]]; then
     error=$(send_command aws --no-verify-ssl s3 mb s3://"$2" 2>&1) || exit_code=$?
   elif [[ $1 == 's3api' ]]; then
-    error=$(send_command aws --no-verify-ssl s3api create-bucket --bucket "$2" 2>&1) || exit_code=$?
+    error=$(send_command aws --no-verify-ssl s3api create-bucket --bucket "$2" "${location_args[@]}" 2>&1) || exit_code=$?
   elif [[ $1 == "s3cmd" ]]; then
     log 5 "s3cmd ${S3CMD_OPTS[*]} --no-check-certificate mb s3://$2"
-    error=$(send_command s3cmd "${S3CMD_OPTS[@]}" --no-check-certificate mb s3://"$2" 2>&1) || exit_code=$?
+    error=$(send_command s3cmd "${S3CMD_OPTS[@]}" --no-check-certificate mb --region="$AWS_REGION" s3://"$2" 2>&1) || exit_code=$?
   elif [[ $1 == "mc" ]]; then
     error=$(send_command mc --insecure mb "$MC_ALIAS"/"$2" --region "$AWS_REGION" 2>&1) || exit_code=$?
   else
@@ -101,7 +105,11 @@ create_bucket_object_lock_enabled() {
   fi
 
   local exit_code=0
-  error=$(send_command aws --no-verify-ssl s3api create-bucket --bucket "$1" 2>&1 --object-lock-enabled-for-bucket 2>&1) || local exit_code=$?
+  local location_args=()
+  if [ -n "$AWS_REGION" ] && [ "$AWS_REGION" != "us-east-1" ]; then
+    location_args=(--create-bucket-configuration LocationConstraint="$AWS_REGION")
+  fi
+  error=$(send_command aws --no-verify-ssl s3api create-bucket --bucket "$1" "${location_args[@]}" 2>&1 --object-lock-enabled-for-bucket 2>&1) || local exit_code=$?
   if [ $exit_code -ne 0 ]; then
     log 2 "error creating bucket: $error"
     return 1

tests/rest_scripts/create_bucket.sh

@@ -67,6 +67,9 @@ create_canonical_hash_sts_and_signature
 curl_command+=(curl -ks -w "%{http_code}" -X PUT "$AWS_ENDPOINT_URL/$bucket_name")
 curl_command+=(-H "\"Authorization: AWS4-HMAC-SHA256 Credential=$aws_access_key_id/$year_month_day/$aws_region/s3/aws4_request,SignedHeaders=$param_list,Signature=$signature\"")
 curl_command+=("${header_fields[@]}")
+if [ "$aws_region" != "us-east-1" ]; then
+  curl_command+=(-d "\"<CreateBucketConfiguration><LocationConstraint>$aws_region</LocationConstraint></CreateBucketConfiguration>\"")
+fi
 curl_command+=(-o "$OUTPUT_FILE")
 # shellcheck disable=SC2154
 eval "${curl_command[*]}" 2>&1