From 6a9b5d6763008354c9a99a5d1ba2413df17d80d1 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sun, 6 Sep 2026 22:28:18 +0800 Subject: [PATCH] fix: honor header checksum when x-amz-trailer is advertised on non-trailer chunked PUT (#107) The AWS Java SDK v2, with chunked encoding enabled (its default), sends a PutObject as a non-trailer signed aws-chunked stream (x-amz-content-sha256: STREAMING-AWS4-HMAC-SHA256-PAYLOAD). When a checksum algorithm is set it puts the precomputed value in the x-amz-checksum-crc32 header, yet still advertises the checksum in x-amz-trailer even though no trailer chunk is ever sent. GetContentChecksum treated any x-amz-trailer-advertised checksum as trailing with an empty value, deferring it to a trailer. For the non-trailer auth type the handler sets req.Trailer = nil, so at EOF the hash.Reader looked the value up in a nil trailer, got "", and returned XAmzContentChecksumMismatch (HTTP 400) even though the correct value sat in the request header. Real S3 accepts the request, and disabling chunked encoding removed the trailer advertisement, matching the reported symptom. Honor the header value directly when a trailer-advertised checksum is already present in the request headers; fall back to trailing delivery only when the header is absent. When the header carries the checksum but it does not parse, reject the request with ErrInvalidChecksum instead of falling through to a no-validation path, so a malformed client-supplied checksum is never silently dropped. This also restores the checksum echo on the response and the stored value, while keeping genuine trailer uploads and wrong-checksum rejection intact. Fixes #107. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe Signed-off-by: Feng Ruohang --- cmd/object-handlers-chunked-checksum_test.go | 138 +++++++++++++++++++ internal/hash/checksum.go | 20 ++- internal/hash/checksum_test.go | 69 ++++++++++ 3 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 cmd/object-handlers-chunked-checksum_test.go diff --git a/cmd/object-handlers-chunked-checksum_test.go b/cmd/object-handlers-chunked-checksum_test.go new file mode 100644 index 000000000..abd9f0d16 --- /dev/null +++ b/cmd/object-handlers-chunked-checksum_test.go @@ -0,0 +1,138 @@ +// Copyright (c) 2015-2026 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "bytes" + "encoding/base64" + "encoding/binary" + "encoding/xml" + "hash/crc32" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/minio/minio/internal/auth" +) + +func crc32Checksum(data []byte) string { + var c [4]byte + binary.BigEndian.PutUint32(c[:], crc32.ChecksumIEEE(data)) + return base64.StdEncoding.EncodeToString(c[:]) +} + +// TestAPIPutObjectChunkedChecksum exercises PutObject with aws-chunked streaming +// transfer encoding combined with a CRC32 checksum. It reproduces issue #107: +// the AWS Java SDK v2, with chunked encoding enabled, sends a non-trailer signed +// chunked body (x-amz-content-sha256: STREAMING-AWS4-HMAC-SHA256-PAYLOAD), places +// the precomputed checksum in the x-amz-checksum-crc32 header, yet still advertises +// it in x-amz-trailer even though no trailer is ever sent. Before the fix the +// server treated the checksum as trailing (empty value) and returned HTTP 400 +// XAmzContentChecksumMismatch. +func TestAPIPutObjectChunkedChecksum(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPIPutObjectChunkedChecksum, + endpoints: []string{"PutObject", "GetObject"}, + }) +} + +func testAPIPutObjectChunkedChecksum(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials auth.Credentials, t *testing.T, +) { + data := bytes.Repeat([]byte("a"), 4096) + goodCRC := crc32Checksum(data) + // A well-formed 4-byte value that does not match the content. + wrongCRC := base64.StdEncoding.EncodeToString([]byte{0x01, 0x02, 0x03, 0x04}) + + apiCode := func(rec *httptest.ResponseRecorder) string { + var apiErr APIErrorResponse + b, _ := io.ReadAll(rec.Body) + _ = xml.Unmarshal(b, &apiErr) + return apiErr.Code + } + + // newChunkedJavaForm builds a non-trailer signed chunked PutObject request that + // mirrors the AWS Java SDK v2 wire form: the checksum value sits in the header + // while x-amz-trailer still advertises it (no trailer is actually sent). The + // checksum headers are set BEFORE signing so they are part of the signed + // headers, exactly as the captured Java SDK request sends them. + newChunkedJavaForm := func(object, crc string) *http.Request { + body := bytes.NewReader(data) + req, err := newTestStreamingRequest(http.MethodPut, + getPutObjectURL("", bucketName, object), + int64(len(data)), int64(len(data)), body) + if err != nil { + t.Fatalf("Failed to create streaming request: %v", err) + } + req.Header.Set("x-amz-checksum-crc32", crc) + req.Header.Set("x-amz-trailer", "x-amz-checksum-crc32") + req.Header.Set("x-amz-sdk-checksum-algorithm", "CRC32") + currTime := UTCNow() + signature, err := signStreamingRequest(req, credentials.AccessKey, credentials.SecretKey, currTime) + if err != nil { + t.Fatalf("Failed to sign streaming request: %v", err) + } + req, err = assembleStreamingChunks(req, body, int64(len(data)), credentials.SecretKey, signature, currTime) + if err != nil { + t.Fatalf("Failed to assemble streaming chunks: %v", err) + } + return req + } + + // 1. Correct checksum: must succeed and echo the checksum on the response. + { + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, newChunkedJavaForm("chunked-ok", goodCRC)) + if rec.Code != http.StatusOK { + t.Fatalf("%s: chunked+CRC32 PutObject: expected 200, got %d (%s)", instanceType, rec.Code, apiCode(rec)) + } + if got := rec.Header().Get("x-amz-checksum-crc32"); got != goodCRC { + t.Fatalf("%s: response checksum echo = %q, want %q", instanceType, got, goodCRC) + } + + // Read the stored checksum back via GetObject with checksum mode enabled. + greq, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucketName, "chunked-ok"), + 0, nil, credentials.AccessKey, credentials.SecretKey, map[string]string{"x-amz-checksum-mode": "ENABLED"}) + if err != nil { + t.Fatalf("Failed to create GET request: %v", err) + } + grec := httptest.NewRecorder() + apiRouter.ServeHTTP(grec, greq) + if grec.Code != http.StatusOK { + t.Fatalf("%s: GetObject: expected 200, got %d", instanceType, grec.Code) + } + if got := grec.Header().Get("x-amz-checksum-crc32"); got != goodCRC { + t.Fatalf("%s: stored checksum read back = %q, want %q", instanceType, got, goodCRC) + } + } + + // 2. Wrong checksum over the same chunked form: must still be rejected. + { + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, newChunkedJavaForm("chunked-wrong", wrongCRC)) + if rec.Code != http.StatusBadRequest { + t.Fatalf("%s: chunked+wrong CRC32: expected 400, got %d", instanceType, rec.Code) + } + if code := apiCode(rec); code != "XAmzContentChecksumMismatch" { + t.Fatalf("%s: chunked+wrong CRC32: want XAmzContentChecksumMismatch, got %q", instanceType, code) + } + } +} diff --git a/internal/hash/checksum.go b/internal/hash/checksum.go index e4ced2fba..f8238c506 100644 --- a/internal/hash/checksum.go +++ b/internal/hash/checksum.go @@ -698,7 +698,25 @@ func GetContentChecksum(h http.Header) (*Checksum, error) { for _, t := range BaseChecksumTypes { if strings.EqualFold(t.Key(), header) { duplicates = res != nil - res = NewChecksumWithType(t|ChecksumTrailing, "") + // A checksum can be advertised via x-amz-trailer while its + // value is still delivered in the request headers. The AWS + // Java SDK v2 does this on chunked (aws-chunked) uploads: + // it sends STREAMING-AWS4-HMAC-SHA256-PAYLOAD (no trailer), + // puts the precomputed value in x-amz-checksum-*, yet still + // lists it in x-amz-trailer, so no trailer ever arrives. + // When the value is present as a header, honor it directly + // instead of waiting for a trailer that will never be read. + if v := h.Get(t.Key()); v != "" { + res = NewChecksumWithType(t, v) + if res == nil { + // The value is supplied in the header but does + // not parse. A malformed client-supplied checksum + // is an error, not a reason to skip validation. + return nil, ErrInvalidChecksum + } + } else { + res = NewChecksumWithType(t|ChecksumTrailing, "") + } } } if strings.HasPrefix(strings.ToLower(header), "x-amz-checksum-") && !isSupportedChecksumHeader(header) { diff --git a/internal/hash/checksum_test.go b/internal/hash/checksum_test.go index 595302818..f033db20e 100644 --- a/internal/hash/checksum_test.go +++ b/internal/hash/checksum_test.go @@ -257,3 +257,72 @@ func TestChecksumSerializeDeserializeMultiPart(t *testing.T) { } } } + +// TestGetContentChecksumTrailerWithHeaderValue covers the case where a checksum +// is advertised via x-amz-trailer while its value is delivered as a request +// header (no trailer is actually sent). The AWS Java SDK v2 does this on chunked +// (aws-chunked) uploads that use STREAMING-AWS4-HMAC-SHA256-PAYLOAD (non-trailer) +// but still list the checksum in x-amz-trailer. See issue #107. The header value +// must be honored as a non-trailing checksum instead of being treated as an empty +// trailing checksum. +func TestGetContentChecksumTrailerWithHeaderValue(t *testing.T) { + const crc = "Hkksgg==" // CRC32 of "Hello CRC32!" + + // Trailer advertised AND value present in header -> non-trailing, value honored. + h := http.Header{} + h.Set(xhttp.AmzTrailer, xhttp.AmzChecksumCRC32) + h.Set(xhttp.AmzChecksumCRC32, crc) + cs, err := GetContentChecksum(h) + if err != nil { + t.Fatalf("GetContentChecksum error = %v, want nil", err) + } + if cs == nil { + t.Fatal("GetContentChecksum returned nil checksum") + } + if cs.Type.Trailing() { + t.Errorf("checksum reported as trailing; want non-trailing since value is in the header") + } + if !cs.Type.Is(ChecksumCRC32) { + t.Errorf("checksum type = %s, want CRC32", cs.Type.StringFull()) + } + if cs.Encoded != crc { + t.Errorf("checksum value = %q, want %q", cs.Encoded, crc) + } + + // Trailer advertised WITHOUT a header value -> stays trailing (unchanged). + h2 := http.Header{} + h2.Set(xhttp.AmzTrailer, xhttp.AmzChecksumCRC32) + cs2, err := GetContentChecksum(h2) + if err != nil { + t.Fatalf("GetContentChecksum (no header value) error = %v, want nil", err) + } + if cs2 == nil || !cs2.Type.Trailing() { + t.Errorf("checksum = %v, want a trailing CRC32 checksum", cs2) + } +} + +// TestGetContentChecksumTrailerMalformedHeaderValue guards against turning a +// malformed client-supplied checksum into a no-op. When a checksum is advertised +// via x-amz-trailer and its header value is present but does not parse, the +// request must be rejected (ErrInvalidChecksum) rather than silently dropped. +// The mismatched x-amz-checksum-algorithm selector makes the regression visible: +// without the guard, execution falls through to getContentChecksum which would +// return (nil, nil) and install no validator at all. +func TestGetContentChecksumTrailerMalformedHeaderValue(t *testing.T) { + h := http.Header{} + h.Set(xhttp.AmzTrailer, xhttp.AmzChecksumCRC32) + h.Set(xhttp.AmzChecksumCRC32, "AQID") // decodes to 3 bytes -> invalid CRC32 + h.Set(xhttp.AmzChecksumAlgo, "SHA256") + cs, err := GetContentChecksum(h) + if !errors.Is(err, ErrInvalidChecksum) { + t.Fatalf("GetContentChecksum error = %v (checksum %v), want ErrInvalidChecksum", err, cs) + } + + // Same, without the misleading algorithm selector: still an error. + h2 := http.Header{} + h2.Set(xhttp.AmzTrailer, xhttp.AmzChecksumCRC32) + h2.Set(xhttp.AmzChecksumCRC32, "AQID") + if _, err := GetContentChecksum(h2); !errors.Is(err, ErrInvalidChecksum) { + t.Fatalf("GetContentChecksum (no algo selector) error = %v, want ErrInvalidChecksum", err) + } +}