fix(s3api): strip aws-chunked only for streaming uploads

- Replace `StripAwsChunkedEncoding` with `ParseContentEncoding`, which drops
  the token only when `x-amz-content-sha256` names a streaming payload type,
  so a client that sends `aws-chunked` on a hex-payload request keeps it
- Revert the `CopyObject` and `CreateMultipartUpload` call sites: both routes
  are registered with `streamBody` false, so a streaming payload type is
  rejected before the controller and the token can only be a stored value
- Reject `aws-chunked` combined with `UNSIGNED-PAYLOAD` in the authentication
  middleware, beside the existing payload-type validation, with a new
  `InvalidArgAwsChunkedUnsignedPayload`
- Add REST tests for the three cases: stripped from a chunked upload, kept on
  a hex-payload request, and rejected with `UNSIGNED-PAYLOAD`
- Add `CONTENT_ENCODING` to the PutObject REST script and a
  `check_content_encoding` driver for HeadObject
This commit is contained in:
Barry Loong
2026-09-22 02:03:09 +04:00
committed by niksis02
parent 370d909372
commit 25f9790c0a
13 changed files with 253 additions and 58 deletions
+1 -1
View File
@@ -147,7 +147,7 @@ func (c S3ApiController) CreateMultipartUpload(ctx fiber.Ctx) (*Response, error)
contentDisposition := ctx.Get("Content-Disposition")
contentLanguage := ctx.Get("Content-Language")
cacheControl := ctx.Get("Cache-Control")
contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding"))
contentEncoding := ctx.Get("Content-Encoding")
tagging := ctx.Get("X-Amz-Tagging")
expires := ctx.Get("Expires")
websiteRedirectLocation := ctx.Get("X-Amz-Website-Redirect-Location")
-30
View File
@@ -328,28 +328,6 @@ func TestS3ApiController_CreateMultipartUpload(t *testing.T) {
},
},
},
{
name: "strips aws-chunked content encoding",
input: testInput{
locals: defaultLocals,
beRes: s3response.InitiateMultipartUploadResult{},
headers: map[string]string{
"Content-Encoding": "aws-chunked,gzip",
},
},
output: testOutput{
response: &Response{
Data: s3response.InitiateMultipartUploadResult{},
Headers: map[string]*string{
"x-amz-checksum-algorithm": nil,
"x-amz-checksum-type": nil,
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -358,14 +336,6 @@ func TestS3ApiController_CreateMultipartUpload(t *testing.T) {
if tt.name == "successful response" && createMultipartUploadInput.StorageClass != types.StorageClassGlacier {
t.Fatalf("expected storage class %q, got %q", types.StorageClassGlacier, createMultipartUploadInput.StorageClass)
}
if tt.name == "strips aws-chunked content encoding" {
if createMultipartUploadInput.ContentEncoding == nil {
t.Fatal("expected content encoding to be set")
}
if *createMultipartUploadInput.ContentEncoding != "gzip" {
t.Fatalf("expected content encoding %q, got %q", "gzip", *createMultipartUploadInput.ContentEncoding)
}
}
return tt.input.beRes.(s3response.InitiateMultipartUploadResult), tt.input.beErr
},
GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
+2 -2
View File
@@ -516,7 +516,7 @@ func (c S3ApiController) CopyObject(ctx fiber.Ctx) (*Response, error) {
metaDirective := types.MetadataDirective(ctx.Get("X-Amz-Metadata-Directive", string(types.MetadataDirectiveCopy)))
taggingDirective := types.TaggingDirective(ctx.Get("X-Amz-Tagging-Directive", string(types.TaggingDirectiveCopy)))
contentType := ctx.Get("Content-Type", defaultContentType)
contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding"))
contentEncoding := ctx.Get("Content-Encoding")
contentDisposition := ctx.Get("Content-Disposition")
contentLanguage := ctx.Get("Content-Language")
cacheControl := ctx.Get("Cache-Control")
@@ -698,7 +698,7 @@ func (c S3ApiController) PutObject(ctx fiber.Ctx) (*Response, error) {
bucket := ctx.Params("bucket")
key := strings.TrimPrefix(ctx.Path(), fmt.Sprintf("/%s/", bucket))
contentType := ctx.Get("Content-Type", defaultContentType)
contentEncoding := utils.StripAwsChunkedEncoding(ctx.Get("Content-Encoding"))
contentEncoding := utils.ParseContentEncoding(ctx)
contentDisposition := ctx.Get("Content-Disposition")
contentLanguage := ctx.Get("Content-Language")
cacheControl := ctx.Get("Cache-Control")
+59 -2
View File
@@ -1331,7 +1331,7 @@ func TestS3ApiController_PutObject(t *testing.T) {
})
})
t.Run("strips aws-chunked content encoding", func(t *testing.T) {
t.Run("strips aws-chunked from a streaming upload", func(t *testing.T) {
be := &BackendMock{
PutObjectFunc: func(_ context.Context, input s3response.PutObjectInput) (s3response.PutObjectOutput, error) {
if input.ContentEncoding == nil {
@@ -1381,7 +1381,64 @@ func TestS3ApiController_PutObject(t *testing.T) {
}, nil, ctxInputs{
locals: defaultLocals,
headers: map[string]string{
"Content-Encoding": "aws-chunked,gzip",
"Content-Encoding": "aws-chunked,gzip",
"X-Amz-Content-Sha256": "STREAMING-UNSIGNED-PAYLOAD-TRAILER",
},
})
})
t.Run("keeps aws-chunked when the payload is not streamed", func(t *testing.T) {
be := &BackendMock{
PutObjectFunc: func(_ context.Context, input s3response.PutObjectInput) (s3response.PutObjectOutput, error) {
if input.ContentEncoding == nil {
t.Fatal("expected content encoding to be set")
}
if *input.ContentEncoding != "aws-chunked" {
t.Fatalf("expected content encoding %q, got %q", "aws-chunked", *input.ContentEncoding)
}
return s3response.PutObjectOutput{ETag: "etag", VersionID: "version-id"}, nil
},
GetBucketPolicyFunc: func(_ context.Context, _ string) ([]byte, error) {
return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
},
GetObjectLockConfigurationFunc: func(_ context.Context, _ string) ([]byte, error) {
return nil, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound)
},
GetBucketVersioningFunc: func(_ context.Context, _ string) (s3response.GetBucketVersioningOutput, error) {
return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNotImplemented)
},
}
ctrl := S3ApiController{be: be}
testController(t, ctrl.PutObject, &Response{
Headers: map[string]*string{
"ETag": utils.GetStringPtr("etag"),
"x-amz-checksum-crc32": nil,
"x-amz-checksum-crc32c": nil,
"x-amz-checksum-crc64nvme": nil,
"x-amz-checksum-sha1": nil,
"x-amz-checksum-sha256": nil,
"x-amz-checksum-sha512": nil,
"x-amz-checksum-md5": nil,
"x-amz-checksum-xxhash64": nil,
"x-amz-checksum-xxhash3": nil,
"x-amz-checksum-xxhash128": nil,
"x-amz-checksum-type": nil,
"x-amz-version-id": utils.GetStringPtr("version-id"),
"x-amz-object-size": nil,
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
ObjectETag: utils.GetStringPtr("etag"),
ContentLength: 0,
ObjectSize: 0,
EventName: s3event.EventObjectCreatedPut,
},
}, nil, ctxInputs{
locals: defaultLocals,
headers: map[string]string{
"Content-Encoding": "aws-chunked",
"X-Amz-Content-Sha256": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
},
})
})
+5
View File
@@ -132,6 +132,11 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string,
if !streamBody && utils.IsStreamingPayload(hashPayload) {
return s3err.GetAPIError(s3err.ErrInvalidSHA256PayloadUsage)
}
// aws-chunked frames the body, so it contradicts an unsigned payload,
// which declares the body is sent as-is
if utils.IsUnsignedPaylod(hashPayload) && utils.HasAwsChunkedEncoding(ctx.Get("Content-Encoding")) {
return s3err.GetInvalidArgumentErr(s3err.InvalidArgAwsChunkedUnsignedPayload, hashPayload)
}
canonicalString, err := utils.CheckValidSignature(ctx, authData, derivedKey, hashPayload, tdate, contentLength)
if err != nil {
+22 -7
View File
@@ -1113,15 +1113,30 @@ func ValidateLocationConstraint(constraint *string, region string) error {
return nil
}
// The coding announced when a body is framed in aws-chunked, as the SDKs do to
// carry a trailing checksum.
// The coding a client announces when it frames a body in aws-chunked, as the
// SDKs do to carry a trailing checksum.
const awsChunkedEncoding = "aws-chunked"
// StripAwsChunkedEncoding drops the aws-chunked token, which frames the request
// rather than the object, from a Content-Encoding value.
func StripAwsChunkedEncoding(contentEncoding string) string {
if contentEncoding == "" {
return ""
// HasAwsChunkedEncoding reports whether a Content-Encoding value carries the
// aws-chunked token.
func HasAwsChunkedEncoding(contentEncoding string) bool {
for _, coding := range strings.Split(contentEncoding, ",") {
if strings.EqualFold(strings.TrimSpace(coding), awsChunkedEncoding) {
return true
}
}
return false
}
// ParseContentEncoding returns the Content-Encoding to store for a request,
// dropping the aws-chunked token when the payload type says the body was framed
// in it. S3 strips it only for streaming uploads: on any other request the
// token is a value the client chose and is stored as sent.
func ParseContentEncoding(ctx fiber.Ctx) string {
contentEncoding := ctx.Get("Content-Encoding")
if !IsStreamingPayload(ctx.Get("X-Amz-Content-Sha256")) {
return contentEncoding
}
codings := strings.Split(contentEncoding, ",")
+52 -16
View File
@@ -20,6 +20,7 @@ import (
"encoding/xml"
"errors"
"math/rand"
"net/http"
"net/url"
"reflect"
"strings"
@@ -1516,29 +1517,64 @@ func TestValidateCopySource(t *testing.T) {
}
}
func TestStripAwsChunkedEncoding(t *testing.T) {
func TestHasAwsChunkedEncoding(t *testing.T) {
tests := []struct {
name string
contentEncoding string
want string
want bool
}{
{"empty", "", ""},
{"only aws-chunked", "aws-chunked", ""},
{"only aws-chunked, uppercase", "AWS-CHUNKED", ""},
{"only aws-chunked, padded", " aws-chunked ", ""},
{"no aws-chunked", "gzip", "gzip"},
{"other codings kept in order", "deflate,gzip", "deflate,gzip"},
{"aws-chunked first", "aws-chunked,gzip", "gzip"},
{"aws-chunked last", "gzip,aws-chunked", "gzip"},
{"aws-chunked in the middle", "deflate,aws-chunked,gzip", "deflate,gzip"},
{"spaces around codings", "aws-chunked, gzip", "gzip"},
{"repeated aws-chunked", "aws-chunked,aws-chunked", ""},
{"empty coding dropped", "gzip,,aws-chunked", "gzip"},
{"coding containing the token is kept", "aws-chunked-custom", "aws-chunked-custom"},
{"empty", "", false},
{"only aws-chunked", "aws-chunked", true},
{"uppercase", "AWS-CHUNKED", true},
{"padded", " aws-chunked ", true},
{"alongside another coding", "aws-chunked, gzip", true},
{"other coding only", "gzip", false},
{"token is a prefix of another coding", "aws-chunked-custom", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, StripAwsChunkedEncoding(tt.contentEncoding))
assert.Equal(t, tt.want, HasAwsChunkedEncoding(tt.contentEncoding))
})
}
}
func TestParseContentEncoding(t *testing.T) {
tests := []struct {
name string
contentSha256 string
contentEncoding string
want string
}{
// streaming: the token frames the request, so S3 drops it
{"streaming, only aws-chunked", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked", ""},
{"streaming, aws-chunked first", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked,gzip", "gzip"},
{"streaming, aws-chunked last", "STREAMING-AWS4-HMAC-SHA256-PAYLOAD", "gzip,aws-chunked", "gzip"},
{"streaming, aws-chunked in the middle", "STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER", "deflate,aws-chunked,gzip", "deflate,gzip"},
{"streaming, spaces around codings", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked, gzip", "gzip"},
{"streaming, uppercase", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "AWS-CHUNKED,gzip", "gzip"},
{"streaming, repeated", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked,aws-chunked", ""},
{"streaming, empty coding dropped", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "gzip,,aws-chunked", "gzip"},
{"streaming, token is a prefix of another coding", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "aws-chunked-custom", "aws-chunked-custom"},
{"streaming, no aws-chunked", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "gzip", "gzip"},
{"streaming, no content encoding", "STREAMING-UNSIGNED-PAYLOAD-TRAILER", "", ""},
// not streaming: the token is a value the client chose, and S3 keeps it
{"hex payload keeps aws-chunked", "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "aws-chunked", "aws-chunked"},
{"hex payload keeps other codings", "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "aws-chunked,gzip", "aws-chunked,gzip"},
{"ecdsa streaming is not chunk decoded, so the value is kept", "STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD", "aws-chunked", "aws-chunked"},
{"no payload header", "", "gzip", "gzip"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
headers := http.Header{}
if tt.contentSha256 != "" {
headers.Set("X-Amz-Content-Sha256", tt.contentSha256)
}
if tt.contentEncoding != "" {
headers.Set("Content-Encoding", tt.contentEncoding)
}
ctx := fiberCtxFromURL(t, http.MethodPut, "http://localhost/bucket/object", headers)
assert.Equal(t, tt.want, ParseContentEncoding(ctx))
})
}
}
+5
View File
@@ -62,6 +62,7 @@ const (
InvalidArgIndexDocumentSuffix
InvalidArgMissingIndexDocumentSuffix
InvalidArgErrorDocumentKey
InvalidArgAwsChunkedUnsignedPayload
)
var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
@@ -121,6 +122,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
Description: "x-amz-content-sha256 must be UNSIGNED-PAYLOAD, STREAMING-UNSIGNED-PAYLOAD-TRAILER, STREAMING-AWS4-HMAC-SHA256-PAYLOAD, STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER, STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD, STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD-TRAILER or a valid sha256 value.",
ArgumentName: "x-amz-content-sha256",
},
InvalidArgAwsChunkedUnsignedPayload: {
Description: "aws-chunked encoding is not supported when x-amz-content-sha256 UNSIGNED-PAYLOAD is supplied",
ArgumentName: "x-amz-content-sha256",
},
InvalidArgCopySource: {
Description: "You can only specify a copy source header for copy requests.",
ArgumentName: "x-amz-copy-source",
@@ -95,6 +95,29 @@ verify_checksum_doesnt_exist() {
fi
}
parse_content_encoding() {
if ! check_param_count_v2 "file" 1 $#; then
return 1
fi
content_encoding=$(grep -i "^content-encoding:" "$1" | cut -d' ' -f2- | sed 's/\r$//')
echo "$content_encoding"
}
check_content_encoding() {
if ! check_param_count_v2 "bucket, key, expected content encoding" 3 $#; then
return 1
fi
if ! content_encoding=$(head_object_rest_expect_success_callback "$1" "$2" "" "parse_content_encoding" 2>&1); then
log 2 "error calling HeadObject command: $content_encoding"
return 1
fi
if [ "$content_encoding" != "$3" ]; then
log 2 "content encoding mismatch (expected: '$3', actual: '$content_encoding')"
return 1
fi
return 0
}
parse_content_length() {
if ! check_param_count_v2 "file" 1 $#; then
return 1
@@ -224,6 +224,32 @@ attempt_chunked_upload_with_bad_first_signature() {
return 0
}
put_object_rest_with_content_encoding() {
if ! check_param_count_v2 "data file, bucket name, key, content encoding" 4 $#; then
return 1
fi
if ! result=$(COMMAND_LOG="$COMMAND_LOG" DATA_FILE="$1" BUCKET_NAME="$2" OBJECT_KEY="$3" CONTENT_ENCODING="$4" OUTPUT_FILE="$TEST_FILE_FOLDER/result.txt" ./tests/rest_scripts/put_object.sh 2>&1); then
log 2 "error: $result"
return 1
fi
if [ "$result" != "200" ]; then
log 2 "expected response code of '200', was '$result' ($(cat "$TEST_FILE_FOLDER/result.txt"))"
return 1
fi
return 0
}
put_object_rest_unsigned_payload_with_aws_chunked() {
if ! check_param_count_v2 "data file, bucket name, key" 3 $#; then
return 1
fi
if ! put_object_rest_expect_error "$1" "$2" "$3" "PAYLOAD=UNSIGNED-PAYLOAD CONTENT_ENCODING=aws-chunked" "400" "InvalidArgument" "aws-chunked encoding is not supported"; then
log 2 "expected aws-chunked with UNSIGNED-PAYLOAD to be rejected"
return 1
fi
return 0
}
chunked_upload_success() {
if ! check_param_count_v2 "data file, bucket name, key" 3 $#; then
return 1
+5
View File
@@ -30,6 +30,8 @@ checksum_type="$CHECKSUM_TYPE"
payload="$PAYLOAD"
# shellcheck disable=SC2153
expires="$EXPIRES"
# shellcheck disable=SC2153
content_encoding="$CONTENT_ENCODING"
# use this parameter to check incorrect checksums
# shellcheck disable=SC2153,SC2154
checksum_hash="$CHECKSUM"
@@ -47,6 +49,9 @@ else
fi
cr_data=("PUT" "/$bucket_name/$key" "")
if [ -n "$content_encoding" ]; then
cr_data+=("content-encoding:$content_encoding")
fi
if [ -n "$expires" ]; then
cr_data+=("expires:$expires")
fi
+24
View File
@@ -21,6 +21,7 @@ source ./tests/logger.sh
source ./tests/setup.sh
source ./tests/drivers/file.sh
source ./tests/drivers/create_bucket/create_bucket_rest.sh
source ./tests/drivers/head_object/head_object_rest.sh
source ./tests/drivers/get_object_lock_config/get_object_lock_config_rest.sh
source ./tests/drivers/put_bucket_ownership_controls/put_bucket_ownership_controls_rest.sh
@@ -104,6 +105,29 @@ source ./tests/drivers/put_bucket_ownership_controls/put_bucket_ownership_contro
assert_success
}
# tags: openssl,chunked,PutObject,content-encoding
@test "REST - chunked upload, aws-chunked not stored as Content-Encoding" {
run get_bucket_name "$BUCKET_ONE_NAME"
assert_success
bucket_name="$output"
run setup_bucket_v2 "$bucket_name"
assert_success
run get_file_name
assert_success
test_file="$output"
run create_file_single_char "$test_file" 8192 'a'
assert_success
run chunked_upload_success "$TEST_FILE_FOLDER/$test_file" "$bucket_name" "$test_file"
assert_success
run check_content_encoding "$bucket_name" "$test_file" ""
assert_success
}
# tags: openssl,chunked,PutObject
@test "REST - chunked upload, success (null bytes)" {
run get_bucket_name "$BUCKET_ONE_NAME"
+29
View File
@@ -43,6 +43,35 @@ export RUN_USERS=true
assert_success
}
# tags: curl, PutObject, content-encoding
@test "REST - PutObject - aws-chunked kept when the payload is not chunked" {
run get_bucket_name "$BUCKET_ONE_NAME"
assert_success
bucket_name="$output"
run setup_bucket_and_file_v2 "$bucket_name" "$test_file"
assert_success
run put_object_rest_with_content_encoding "$TEST_FILE_FOLDER/$test_file" "$bucket_name" "$test_file" "aws-chunked"
assert_success
run check_content_encoding "$bucket_name" "$test_file" "aws-chunked"
assert_success
}
# tags: curl, PutObject, content-encoding, x-amz-content-sha256, invalid-header
@test "REST - PutObject - aws-chunked with UNSIGNED-PAYLOAD rejected" {
run get_bucket_name "$BUCKET_ONE_NAME"
assert_success
bucket_name="$output"
run setup_bucket_and_file_v2 "$bucket_name" "$test_file"
assert_success
run put_object_rest_unsigned_payload_with_aws_chunked "$TEST_FILE_FOLDER/$test_file" "$bucket_name" "$test_file"
assert_success
}
# tags: curl, PutObject, Expires, invalid-header
@test "REST - PutObject - invalid 'Expires' parameter" {
run get_bucket_name "$BUCKET_ONE_NAME"