From e18c4f40801c38d2bd3775a44692134f6d1ec9b0 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Sat, 26 Jul 2025 01:33:00 +0400 Subject: [PATCH] fix: ignores special checksum headers when parsing x-amz-checksum-x headers Fixes #1345 The previous implementation incorrectly parsed the `x-amz-sdk-checksum-algorithm` header for the `CompleteMultipartUpload` operation, even though this header is not expected and should be ignored. It also mistakenly treated the `x-amz-checksum-algorithm` header as an invalid value for `x-amz-checksum-x`. The updated implementation only parses the `x-amz-sdk-checksum-algorithm` header for `PutObject` and `UploadPart` operations. Additionally, `x-amz-checksum-algorithm` and `x-amz-checksum-type` headers are now correctly ignored when parsing the precalculated checksum headers (`x-amz-checksum-x`). --- s3api/controllers/object-post.go | 2 +- s3api/controllers/object-post_test.go | 4 +- s3api/controllers/object-put.go | 4 +- s3api/utils/utils.go | 62 +++++++++++++++++++++------ 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index e9b7293..18b9280 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -305,7 +305,7 @@ func (c S3ApiController) CompleteMultipartUpload(ctx *fiber.Ctx) (*Response, err mpuObjectSize = &val } - _, checksums, err := utils.ParseChecksumHeaders(ctx) + checksums, err := utils.ParseChecksumHeaders(ctx) if err != nil { return &Response{ MetaOpts: &MetaOptions{ diff --git a/s3api/controllers/object-post_test.go b/s3api/controllers/object-post_test.go index d3a87a0..cf07ec7 100644 --- a/s3api/controllers/object-post_test.go +++ b/s3api/controllers/object-post_test.go @@ -449,7 +449,7 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) { locals: defaultLocals, body: validMpBody, headers: map[string]string{ - "X-Amz-Sdk-Checksum-Algorithm": "invalid_checksum_algo", + "X-Amz-Checksum-Crc32": "invalid_checksum", }, }, output: testOutput{ @@ -458,7 +458,7 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) { BucketOwner: "root", }, }, - err: s3err.GetAPIError(s3err.ErrInvalidChecksumAlgorithm), + err: s3err.GetInvalidChecksumHeaderErr("x-amz-checksum-crc32"), }, }, { diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index 4ec2405..b2f8f9d 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -251,7 +251,7 @@ func (c S3ApiController) UploadPart(ctx *fiber.Ctx) (*Response, error) { }, s3err.GetAPIError(s3err.ErrInvalidRequest) } - algorithm, checksums, err := utils.ParseChecksumHeaders(ctx) + algorithm, checksums, err := utils.ParseChecksumHeadersAndSdkAlgo(ctx) if err != nil { debuglogger.Logf("err parsing checksum headers: %v", err) return &Response{ @@ -674,7 +674,7 @@ func (c S3ApiController) PutObject(ctx *fiber.Ctx) (*Response, error) { }, err } - algorithm, checksums, err := utils.ParseChecksumHeaders(ctx) + algorithm, checksums, err := utils.ParseChecksumHeadersAndSdkAlgo(ctx) if err != nil { return &Response{ MetaOpts: &MetaOptions{ diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index f8d17f9..9ce6670 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -413,22 +413,22 @@ func (cv ChecksumValues) Headers() string { return result } -func ParseChecksumHeaders(ctx *fiber.Ctx) (types.ChecksumAlgorithm, ChecksumValues, error) { - sdkAlgorithm := types.ChecksumAlgorithm(strings.ToUpper(ctx.Get("X-Amz-Sdk-Checksum-Algorithm"))) - - err := IsChecksumAlgorithmValid(sdkAlgorithm) - if err != nil { - debuglogger.Logf("invalid checksum algorithm: %v\n", sdkAlgorithm) - return "", nil, err - } - +// ParseCalculatedChecksumHeaders parses and validates x-amz-checksum-x header keys +// e.g x-amz-checksum-crc32, x-amz-checksum-sha256 ... +func ParseCalculatedChecksumHeaders(ctx *fiber.Ctx) (ChecksumValues, error) { checksums := ChecksumValues{} var hdrErr error // Parse and validate checksum headers for key, value := range ctx.Request().Header.All() { - // Skip `X-Amz-Checksum-Type` as it's a special header - if !strings.HasPrefix(string(key), "X-Amz-Checksum-") || string(key) == "X-Amz-Checksum-Type" { + // only check the headers with 'X-Amz-Checksum-' prefix + if !strings.HasPrefix(string(key), "X-Amz-Checksum-") { + continue + } + // "X-Amz-Checksum-Type" and "X-Amz-Checksum-Algorithm" aren't considered + // as invalid values, even if the s3 action doesn't expect these headers + switch string(key) { + case "X-Amz-Checksum-Type", "X-Amz-Checksum-Algorithm": continue } @@ -444,12 +444,48 @@ func ParseChecksumHeaders(ctx *fiber.Ctx) (types.ChecksumAlgorithm, ChecksumValu } if hdrErr != nil { - return sdkAlgorithm, nil, hdrErr + return checksums, hdrErr } if len(checksums) > 1 { debuglogger.Logf("multiple checksum headers provided: %v\n", checksums.Headers()) - return sdkAlgorithm, checksums, s3err.GetAPIError(s3err.ErrMultipleChecksumHeaders) + return checksums, s3err.GetAPIError(s3err.ErrMultipleChecksumHeaders) + } + + return checksums, nil +} + +// ParseChecksumHeaders parses/validates x-amz-checksum-x headers key/values +func ParseChecksumHeaders(ctx *fiber.Ctx) (ChecksumValues, error) { + // first parse/validate 'x-amz-checksum-x' headers + checksums, err := ParseCalculatedChecksumHeaders(ctx) + if err != nil { + return checksums, err + } + + // check if the values are valid + for al, val := range checksums { + if !IsValidChecksum(val, al) { + return checksums, s3err.GetInvalidChecksumHeaderErr(fmt.Sprintf("x-amz-checksum-%v", strings.ToLower(string(al)))) + } + } + + return checksums, nil +} + +// ParseChecksumHeadersAndSdkAlgo parses/validates 'x-amz-sdk-checksum-algorithm' and +// 'x-amz-checksum-x' precalculated request headers +func ParseChecksumHeadersAndSdkAlgo(ctx *fiber.Ctx) (types.ChecksumAlgorithm, ChecksumValues, error) { + sdkAlgorithm := types.ChecksumAlgorithm(strings.ToUpper(ctx.Get("X-Amz-Sdk-Checksum-Algorithm"))) + err := IsChecksumAlgorithmValid(sdkAlgorithm) + if err != nil { + debuglogger.Logf("invalid checksum algorithm: %v\n", sdkAlgorithm) + return "", nil, err + } + + checksums, err := ParseCalculatedChecksumHeaders(ctx) + if err != nil { + return sdkAlgorithm, checksums, err } for al, val := range checksums {