diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index 407e8e16..61216d30 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -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 := ctx.Get("Content-Encoding") + contentEncoding := utils.ParseContentEncoding(ctx) tagging := ctx.Get("X-Amz-Tagging") expires := ctx.Get("Expires") websiteRedirectLocation := ctx.Get("X-Amz-Website-Redirect-Location") diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index c07dd072..12487ef3 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -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 := ctx.Get("Content-Encoding") + contentEncoding := utils.ParseContentEncoding(ctx) 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 := ctx.Get("Content-Encoding") + contentEncoding := utils.ParseContentEncoding(ctx) contentDisposition := ctx.Get("Content-Disposition") contentLanguage := ctx.Get("Content-Language") cacheControl := ctx.Get("Cache-Control") diff --git a/s3api/controllers/object-put_test.go b/s3api/controllers/object-put_test.go index 14611a2d..71497b82 100644 --- a/s3api/controllers/object-put_test.go +++ b/s3api/controllers/object-put_test.go @@ -1331,6 +1331,118 @@ func TestS3ApiController_PutObject(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 { + t.Fatal("expected content encoding to be set") + } + if *input.ContentEncoding != "gzip" { + t.Fatalf("expected content encoding %q, got %q", "gzip", *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,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", + }, + }) + }) + tests := []struct { name string input testInput diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go index 7d8ab356..c3a304e4 100644 --- a/s3api/middlewares/authentication.go +++ b/s3api/middlewares/authentication.go @@ -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, utils.AwsChunkedEncoding) + } canonicalString, err := utils.CheckValidSignature(ctx, authData, derivedKey, hashPayload, tdate, contentLength) if err != nil { diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index f0b42bdc..841d6e94 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1112,3 +1112,56 @@ func ValidateLocationConstraint(constraint *string, region string) error { return nil } + +// AwsChunkedEncoding is 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" + +// 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 unless the request declares a non streaming +// payload type in x-amz-content-sha256. +// +// S3 keys this on the declaration alone. A streaming payload type frames the +// body in aws-chunked, and a request that declares no payload type - a +// presigned URL, signed as UNSIGNED-PAYLOAD and carrying no header - may frame +// it too, so both drop the token. A declared hex digest or UNSIGNED-PAYLOAD +// says the body is sent as-is, so there the token is a coding the client chose +// and is stored as sent. +// +// S3 applies this to the Content-Encoding header itself, before any API sees +// it, so every controller that reads the header inherits it - including +// CopyObject and CreateMultipartUpload, which carry no body to frame. POSTObject +// takes its value from a form field and ignores the header, so it stores the +// token as sent and must not call this. +func ParseContentEncoding(ctx fiber.Ctx) string { + contentEncoding := ctx.Get("Content-Encoding") + payloadType := ctx.Get("X-Amz-Content-Sha256") + if payloadType != "" && !IsStreamingPayload(payloadType) { + return contentEncoding + } + + codings := strings.Split(contentEncoding, ",") + kept := make([]string, 0, len(codings)) + for _, coding := range codings { + trimmed := strings.TrimSpace(coding) + if trimmed == "" || strings.EqualFold(trimmed, AwsChunkedEncoding) { + continue + } + + kept = append(kept, trimmed) + } + + return strings.Join(kept, ",") +} diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 10390de7..b7b5bf47 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -20,6 +20,7 @@ import ( "encoding/xml" "errors" "math/rand" + "net/http" "net/url" "reflect" "strings" @@ -1515,3 +1516,74 @@ func TestValidateCopySource(t *testing.T) { }) } } + +func TestHasAwsChunkedEncoding(t *testing.T) { + tests := []struct { + name string + contentEncoding string + want bool + }{ + {"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, 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", "", ""}, + // no payload type: a presigned request is signed as UNSIGNED-PAYLOAD and + // may still frame its body, so S3 drops the token + {"no payload type, only aws-chunked", "", "aws-chunked", ""}, + {"no payload type, aws-chunked first", "", "aws-chunked,gzip", "gzip"}, + {"no payload type, aws-chunked last", "", "gzip,aws-chunked", "gzip"}, + {"no payload type, uppercase", "", "AWS-CHUNKED", ""}, + {"no payload type, token is a prefix of another coding", "", "aws-chunked-custom", "aws-chunked-custom"}, + {"no payload type, no aws-chunked", "", "gzip", "gzip"}, + // a declared non streaming payload type: 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"}, + {"unsigned payload keeps aws-chunked", "UNSIGNED-PAYLOAD", "aws-chunked", "aws-chunked"}, + {"ecdsa streaming is not chunk decoded, so the value is kept", "STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD", "aws-chunked", "aws-chunked"}, + } + 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)) + }) + } +} diff --git a/s3err/invalid-argument.go b/s3err/invalid-argument.go index 17a0e587..aad893f7 100644 --- a/s3err/invalid-argument.go +++ b/s3err/invalid-argument.go @@ -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: "Content-Encoding", + }, InvalidArgCopySource: { Description: "You can only specify a copy source header for copy requests.", ArgumentName: "x-amz-copy-source", diff --git a/tests/drivers/head_object/head_object_rest.sh b/tests/drivers/head_object/head_object_rest.sh index 26ff1229..69391bbf 100644 --- a/tests/drivers/head_object/head_object_rest.sh +++ b/tests/drivers/head_object/head_object_rest.sh @@ -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 diff --git a/tests/drivers/put_object/put_object_rest.sh b/tests/drivers/put_object/put_object_rest.sh index d28ac772..62ca6ce4 100644 --- a/tests/drivers/put_object/put_object_rest.sh +++ b/tests/drivers/put_object/put_object_rest.sh @@ -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 diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 50f837e0..4ff6be5b 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -1468,3 +1468,92 @@ func PutObject_plain_body_with_decoded_length(s *S3Conf) error { return nil }) } + +// PutObject_plain_stores_aws_chunked_content_encoding checks that aws-chunked +// is stored as sent when the request wasn't framed in it. +// +// The token is transport only where x-amz-content-sha256 names a streaming +// payload type. On a plain PUT it is a coding the client chose, and S3 keeps it +// like any other. +func PutObject_plain_stores_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PutObject_plain_stores_aws_chunked_content_encoding" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + for i, contentEncoding := range []string{"aws-chunked", "gzip,aws-chunked"} { + object := fmt.Sprintf("plain-obj-%v", i) + + req, err := createSignedReq(http.MethodPut, s.endpoint, fmt.Sprintf("%s/%s", bucket, object), + s.awsID, s.awsSecret, "s3", s.awsRegion, "", []byte("hello world"), time.Now(), + map[string]string{"Content-Encoding": contentEncoding}) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != contentEncoding { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, contentEncoding, stored) + } + } + + return nil + }) +} + +func PutObject_unsigned_payload_with_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PutObject_unsigned_payload_with_aws_chunked_content_encoding" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + object := "my-obj" + for i, contentEncoding := range []string{ + "aws-chunked", + "aws-chunked,gzip", + "gzip, aws-chunked", + "AWS-Chunked", + } { + req, err := createSignedReq(http.MethodPut, s.endpoint, fmt.Sprintf("%s/%s", bucket, object), + s.awsID, s.awsSecret, "s3", s.awsRegion, "UNSIGNED-PAYLOAD", []byte("hello world"), time.Now(), + map[string]string{"Content-Encoding": contentEncoding}) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + + // the error names the offending header and reports the token + // alone, not the whole header value + err = checkHTTPResponseApiErr(resp, + s3err.GetInvalidArgumentErr(s3err.InvalidArgAwsChunkedUnsignedPayload, "aws-chunked")) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err == nil { + return fmt.Errorf("expected the rejected uploads to leave no object, but %v exists", object) + } + + return nil + }) +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 01542058..cf3493c6 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -68,6 +68,9 @@ func TestPresignedAuthentication(ts *TestState) { ts.Run(PresignedAuth_incorrect_secret_key) ts.Run(PresignedAuth_sigv2_not_supported) ts.Run(PresignedAuth_PutObject_success) + ts.Run(PresignedAuth_PutObject_strips_aws_chunked_content_encoding) + ts.Run(PresignedAuth_CopyObject_strips_aws_chunked_content_encoding) + ts.Run(PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding) ts.Run(PresignedAuth_Put_GetObject_with_data) if !ts.conf.azureTests { ts.Run(PresignedAuth_Put_GetObject_with_UTF8_chars) @@ -170,6 +173,8 @@ func TestPutObject(ts *TestState) { ts.Run(PutObject_special_chars) ts.Run(PutObject_aborted_plain_body) ts.Run(PutObject_plain_body_with_decoded_length) + ts.Run(PutObject_plain_stores_aws_chunked_content_encoding) + ts.Run(PutObject_unsigned_payload_with_aws_chunked_content_encoding) ts.Run(PutObject_tagging) ts.Run(PutObject_missing_object_lock_retention_config) ts.Run(PutObject_with_object_lock) @@ -2161,6 +2166,7 @@ func TestUnsignedStreaminPayloadTrailer(ts *TestState) { ts.Run(UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme) ts.Run(UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers) ts.Run(UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer) + ts.Run(UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding) ts.Run(UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum) ts.Run(UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_full_object) ts.Run(UnsignedStreamingPayloadTrailer_UploadPart_trailer_and_mp_algo_mismatch) @@ -2815,6 +2821,9 @@ func GetIntTests() IntTests { "PresignedAuth_incorrect_secret_key": PresignedAuth_incorrect_secret_key, "PresignedAuth_sigv2_not_supported": PresignedAuth_sigv2_not_supported, "PresignedAuth_PutObject_success": PresignedAuth_PutObject_success, + "PresignedAuth_PutObject_strips_aws_chunked_content_encoding": PresignedAuth_PutObject_strips_aws_chunked_content_encoding, + "PresignedAuth_CopyObject_strips_aws_chunked_content_encoding": PresignedAuth_CopyObject_strips_aws_chunked_content_encoding, + "PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding": PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding, "PutObject_missing_object_lock_retention_config": PutObject_missing_object_lock_retention_config, "PutObject_name_too_long": PutObject_name_too_long, "PutObject_with_object_lock": PutObject_with_object_lock, @@ -2905,6 +2914,8 @@ func GetIntTests() IntTests { "PutObject_aborted_plain_body": PutObject_aborted_plain_body, "PutObject_plain_body_with_decoded_length": PutObject_plain_body_with_decoded_length, "UploadPart_plain_body_with_decoded_length": UploadPart_plain_body_with_decoded_length, + "PutObject_plain_stores_aws_chunked_content_encoding": PutObject_plain_stores_aws_chunked_content_encoding, + "PutObject_unsigned_payload_with_aws_chunked_content_encoding": PutObject_unsigned_payload_with_aws_chunked_content_encoding, "PutObject_tagging": PutObject_tagging, "PutObject_success": PutObject_success, "PutObject_default_content_type": PutObject_default_content_type, @@ -3633,6 +3644,7 @@ func GetIntTests() IntTests { "UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme": UnsignedStreamingPayloadTrailer_no_trailer_should_calculate_crc64nvme, "UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers": UnsignedStreamingPayloadTrailer_no_payload_trailer_only_headers, "UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer": UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer, + "UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding": UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding, "UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum": UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum, "UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_full_object": UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_full_object, "UnsignedStreamingPayloadTrailer_UploadPart_trailer_and_mp_algo_mismatch": UnsignedStreamingPayloadTrailer_UploadPart_trailer_and_mp_algo_mismatch, diff --git a/tests/integration/presigned_urls.go b/tests/integration/presigned_urls.go index ae59a005..c4c29003 100644 --- a/tests/integration/presigned_urls.go +++ b/tests/integration/presigned_urls.go @@ -16,6 +16,7 @@ package integration import ( "context" + "encoding/xml" "fmt" "io" "net/http" @@ -25,7 +26,9 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/versity/versitygw/s3err" + "github.com/versity/versitygw/s3response" ) func PresignedAuth_security_token_with_permanent_credentials(s *S3Conf) error { @@ -966,3 +969,224 @@ func PresignedAuth_UploadPart(s *S3Conf) error { return nil }) } + +// PresignedAuth_PutObject_strips_aws_chunked_content_encoding checks that +// aws-chunked is dropped from the stored Content-Encoding on a presigned PUT. +func PresignedAuth_PutObject_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PresignedAuth_PutObject_strips_aws_chunked_content_encoding" + return presignedAuthHandler(s, testName, func(client *s3.PresignClient, bucket string) error { + s3client := s.GetClient() + for i, test := range []struct { + contentEncoding string + stored string + }{ + // nothing is left, so no Content-Encoding is stored at all + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + // the remaining codings keep their order + {"gzip,aws-chunked,br", "gzip,br"}, + {"AWS-Chunked", ""}, + // the token has to match in full + {"aws-chunked-custom", "aws-chunked-custom"}, + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("presigned-obj-%v", i) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + v4req, err := client.PresignPutObject(ctx, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + req, err := http.NewRequest(v4req.Method, v4req.URL, strings.NewReader("hello world")) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + req.Header.Set("Content-Encoding", test.contentEncoding) + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} + +// PresignedAuth_CopyObject_strips_aws_chunked_content_encoding checks that +// aws-chunked is dropped from the stored Content-Encoding on a presigned copy. +func PresignedAuth_CopyObject_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PresignedAuth_CopyObject_strips_aws_chunked_content_encoding" + return presignedAuthHandler(s, testName, func(_ *s3.PresignClient, bucket string) error { + s3client := s.GetClient() + srcObj := "copy-source" + + // the source carries a coding of its own, so a stored value can only + // have come from the copy request + _, err := putObjectWithData(int64(len("hello world")), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &srcObj, + ContentEncoding: getPtr("br"), + }, s3client) + if err != nil { + return err + } + + for i, test := range []struct { + contentEncoding string + stored string + }{ + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + {"gzip,aws-chunked,br", "gzip,br"}, + {"AWS-Chunked", ""}, + // the token has to match in full + {"aws-chunked-custom", "aws-chunked-custom"}, + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("copy-dst-%v", i) + + req, err := createPresignedReq(http.MethodPut, s.endpoint, + fmt.Sprintf("%s/%s", bucket, object), s.awsID, s.awsSecret, s.awsRegion, time.Now(), + map[string]string{ + "X-Amz-Copy-Source": fmt.Sprintf("/%s/%s", bucket, srcObj), + "X-Amz-Metadata-Directive": string(types.MetadataDirectiveReplace), + "Content-Encoding": test.contentEncoding, + }) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} + +// PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding checks +// that aws-chunked is dropped from the Content-Encoding a presigned +// CreateMultipartUpload stores for the completed object. +func PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "PresignedAuth_CreateMultipartUpload_strips_aws_chunked_content_encoding" + return presignedAuthHandler(s, testName, func(_ *s3.PresignClient, bucket string) error { + s3client := s.GetClient() + for i, test := range []struct { + contentEncoding string + stored string + }{ + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + {"gzip,aws-chunked,br", "gzip,br"}, + {"AWS-Chunked", ""}, + // the token has to match in full + {"aws-chunked-custom", "aws-chunked-custom"}, + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("mp-obj-%v", i) + + req, err := createPresignedReq(http.MethodPost, s.endpoint, + fmt.Sprintf("%s/%s?uploads=", bucket, object), + s.awsID, s.awsSecret, s.awsRegion, time.Now(), + map[string]string{"Content-Encoding": test.contentEncoding}) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("test %v failed to send the request: %w", i+1, err) + } + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return fmt.Errorf("test %v failed to read the response body: %w", i+1, err) + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("test %v: expected the response status code to be %v, instead got %v", + i+1, http.StatusOK, resp.StatusCode) + } + + var out s3response.InitiateMultipartUploadResult + if err := xml.Unmarshal(body, &out); err != nil { + return fmt.Errorf("test %v failed to parse the response body: %w", i+1, err) + } + + // the parts carry no Content-Encoding: the completed object can + // only inherit what the create named + parts, _, err := uploadParts(s3client, 5*1024*1024, 1, bucket, object, out.UploadId) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + compParts := []types.CompletedPart{} + for _, el := range parts { + compParts = append(compParts, types.CompletedPart{ + ETag: el.ETag, + PartNumber: el.PartNumber, + }) + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{ + Bucket: &bucket, + Key: &object, + UploadId: &out.UploadId, + MultipartUpload: &types.CompletedMultipartUpload{Parts: compParts}, + }) + cancel() + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} diff --git a/tests/integration/unsigned_streaming_payload_trailer.go b/tests/integration/unsigned_streaming_payload_trailer.go index ca93953e..fe1feac0 100644 --- a/tests/integration/unsigned_streaming_payload_trailer.go +++ b/tests/integration/unsigned_streaming_payload_trailer.go @@ -437,6 +437,53 @@ func UnsignedStreamingPayloadTrailer_success_both_sdk_algo_and_trailer(s *S3Conf }) } +// UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding checks that the +// aws-chunked token is dropped from the stored Content-Encoding when the body +// really was framed in it, and that every other coding survives in order. +func UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding(s *S3Conf) error { + testName := "UnsignedStreamingPayloadTrailer_strips_aws_chunked_content_encoding" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + for i, test := range []struct { + contentEncoding string + stored string + }{ + // nothing is left, so no Content-Encoding is stored at all + {"aws-chunked", ""}, + {"aws-chunked,gzip", "gzip"}, + // the remaining codings keep their order + {"gzip,aws-chunked,br", "gzip,br"}, + // a framed body doesn't make every coding transport + {"gzip", "gzip"}, + } { + object := fmt.Sprintf("streaming-obj-%v", i) + reqHeaders := map[string]string{ + "x-amz-decoded-content-length": "11", + "Content-Encoding": test.contentEncoding, + } + body := []byte("B\r\nhello world\r\n0\r\n\r\n") + + _, apiErr, err := testUnsignedStreamingPayloadTrailerObjectPut(s, bucket, object, body, reqHeaders) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if apiErr != nil { + return fmt.Errorf("test %v failed: (%s) %s", i+1, apiErr.Code, apiErr.Message) + } + + stored, err := getStoredContentEncoding(s3client, bucket, object) + if err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + if stored != test.stored { + return fmt.Errorf("test %v: expected the stored content encoding to be %q, instead got %q", + i+1, test.stored, stored) + } + } + + return nil + }) +} + func UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum(s *S3Conf) error { testName := "UnsignedStreamingPayloadTrailer_UploadPart_no_trailer_composite_checksum" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index f849a966..e2a9650c 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -412,6 +412,40 @@ func createSignedReq(method, endpoint, path, access, secret, service, region, ov return req, nil } +// createPresignedReq presigns an S3 request for an operation the SDK's +// PresignClient doesn't cover. Every header passed is signed, and the returned +// request carries back exactly what the signer decided to sign. +func createPresignedReq(method, endpoint, path, access, secret, region string, date time.Time, headers map[string]string) (*http.Request, error) { + req, err := http.NewRequest(method, fmt.Sprintf("%v/%v", endpoint, path), nil) + if err != nil { + return nil, fmt.Errorf("failed to create the request: %w", err) + } + // the signer doesn't add the expiration - the SDK's PresignClient puts it in + // the query before signing - and S3 rejects a presigned URL without it + query := req.URL.Query() + query.Set("X-Amz-Expires", "900") + req.URL.RawQuery = query.Encode() + + for key, val := range headers { + req.Header.Set(key, val) + } + + uri, signedHeaders, err := v4.NewSigner().PresignHTTP(req.Context(), + aws.Credentials{AccessKeyID: access, SecretAccessKey: secret}, + req, "UNSIGNED-PAYLOAD", "s3", region, date) + if err != nil { + return nil, fmt.Errorf("failed to presign the request: %w", err) + } + + presigned, err := http.NewRequest(method, uri, nil) + if err != nil { + return nil, fmt.Errorf("failed to create the presigned request: %w", err) + } + presigned.Header = signedHeaders + + return presigned, nil +} + type APIErrorResponse struct { XMLName xml.Name `xml:"Error"` Code string @@ -4178,3 +4212,22 @@ func checkAndAbortUpload(client *s3.Client, bucket, key, uploadId string) error cancel() return err } + +// getStoredContentEncoding returns the Content-Encoding stored for an object, +// reporting an absent header as the empty string. +func getStoredContentEncoding(s3client *s3.Client, bucket, object string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &object, + }) + cancel() + if err != nil { + return "", err + } + if out.ContentEncoding == nil { + return "", nil + } + + return *out.ContentEncoding, nil +} diff --git a/tests/rest_scripts/put_object.sh b/tests/rest_scripts/put_object.sh index a4c1038d..6b01f75f 100755 --- a/tests/rest_scripts/put_object.sh +++ b/tests/rest_scripts/put_object.sh @@ -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 diff --git a/tests/test_rest_chunked.sh b/tests/test_rest_chunked.sh index eccf947c..e149aae2 100755 --- a/tests/test_rest_chunked.sh +++ b/tests/test_rest_chunked.sh @@ -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" diff --git a/tests/test_rest_put_object.sh b/tests/test_rest_put_object.sh index 37cf8a86..4a6b6d77 100755 --- a/tests/test_rest_put_object.sh +++ b/tests/test_rest_put_object.sh @@ -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"