From 4ea6f3b06b5dc18bf967d1c13b97f33cc8bcdd87 Mon Sep 17 00:00:00 2001 From: Mark Theunissen Date: Fri, 22 Aug 2025 16:15:21 +0200 Subject: [PATCH] fix: invalid checksum on site replication with conforming checksum types (#21535) --- cmd/bucket-replication.go | 11 +++-- cmd/erasure-multipart.go | 1 + cmd/object-multipart-handlers.go | 4 ++ internal/hash/checksum.go | 34 ++++++++++++--- internal/hash/checksum_test.go | 75 +++++++++++++++++++++++++------- 5 files changed, 97 insertions(+), 28 deletions(-) diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index e9b4f5f8d..71d753312 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -3797,14 +3797,13 @@ func getCRCMeta(oi ObjectInfo, partNum int, h http.Header) (cs map[string]string meta := make(map[string]string) cs, isMP = oi.decryptChecksums(partNum, h) for k, v := range cs { - cksum := hash.NewChecksumString(k, v) - if cksum == nil { + if k == xhttp.AmzChecksumType { continue } - if cksum.Valid() { - meta[cksum.Type.Key()] = v - meta[xhttp.AmzChecksumType] = cs[xhttp.AmzChecksumType] - meta[xhttp.AmzChecksumAlgo] = cksum.Type.String() + cktype := hash.ChecksumStringToType(k) + if cktype.IsSet() { + meta[cktype.Key()] = v + meta[xhttp.AmzChecksumAlgo] = cktype.String() } } return meta, isMP diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go index 38f1391cf..5a866631c 100644 --- a/cmd/erasure-multipart.go +++ b/cmd/erasure-multipart.go @@ -1161,6 +1161,7 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str Err: fmt.Errorf("checksum type mismatch. got %q (%s) expected %q (%s)", checksumType.String(), checksumType.ObjType(), opts.WantChecksum.Type.String(), opts.WantChecksum.Type.ObjType()), } } + checksumType |= hash.ChecksumMultipart | hash.ChecksumIncludesMultipart } var checksumCombined []byte diff --git a/cmd/object-multipart-handlers.go b/cmd/object-multipart-handlers.go index e6db2a9db..2226e407c 100644 --- a/cmd/object-multipart-handlers.go +++ b/cmd/object-multipart-handlers.go @@ -221,6 +221,10 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r opts.WantChecksum = &hash.Checksum{Type: checksumType} } + if opts.WantChecksum != nil { + opts.WantChecksum.Type |= hash.ChecksumMultipart | hash.ChecksumIncludesMultipart + } + newMultipartUpload := objectAPI.NewMultipartUpload res, err := newMultipartUpload(ctx, bucket, object, opts) diff --git a/internal/hash/checksum.go b/internal/hash/checksum.go index f2cf18c9f..fbf62fd84 100644 --- a/internal/hash/checksum.go +++ b/internal/hash/checksum.go @@ -145,6 +145,26 @@ func (c ChecksumType) IsSet() bool { return !c.Is(ChecksumInvalid) && !c.Base().Is(ChecksumNone) } +// ChecksumStringToType is like NewChecksumType but without the `mode` +func ChecksumStringToType(alg string) ChecksumType { + switch strings.ToUpper(alg) { + case "CRC32": + return ChecksumCRC32 + case "CRC32C": + return ChecksumCRC32C + case "SHA1": + return ChecksumSHA1 + case "SHA256": + return ChecksumSHA256 + case "CRC64NVME": + // AWS seems to ignore full value, and just assume it. + return ChecksumCRC64NVME + case "": + return ChecksumNone + } + return ChecksumInvalid +} + // NewChecksumType returns a checksum type based on the algorithm string and obj type. func NewChecksumType(alg, objType string) ChecksumType { full := ChecksumFullObject @@ -240,6 +260,12 @@ func (c ChecksumType) ObjType() string { if c.FullObjectRequested() { return xhttp.AmzChecksumTypeFullObject } + if c.IsMultipartComposite() { + return xhttp.AmzChecksumTypeComposite + } + if !c.Is(ChecksumMultipart) { + return xhttp.AmzChecksumTypeFullObject + } if c.IsSet() { return xhttp.AmzChecksumTypeComposite } @@ -340,12 +366,8 @@ func ReadCheckSums(b []byte, part int) (cs map[string]string, isMP bool) { } if cs != "" { res[typ.String()] = cs - res[xhttp.AmzChecksumType] = typ.ObjType() - if !typ.Is(ChecksumMultipart) { - // Single part PUTs are always FULL_OBJECT checksum - // Refer https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html - // **For PutObject uploads, the checksum type is always FULL_OBJECT.** - res[xhttp.AmzChecksumType] = ChecksumFullObject.ObjType() + if ckType := typ.ObjType(); ckType != "" { + res[xhttp.AmzChecksumType] = ckType } } } diff --git a/internal/hash/checksum_test.go b/internal/hash/checksum_test.go index 8d74bde3b..504803795 100644 --- a/internal/hash/checksum_test.go +++ b/internal/hash/checksum_test.go @@ -20,6 +20,8 @@ package hash import ( "net/http/httptest" "testing" + + xhttp "github.com/minio/minio/internal/http" ) // TestChecksumAddToHeader tests that adding and retrieving a checksum on a header works @@ -28,36 +30,77 @@ func TestChecksumAddToHeader(t *testing.T) { name string checksum ChecksumType fullobj bool + wantErr bool }{ - {"CRC32-composite", ChecksumCRC32, false}, - {"CRC32-full-object", ChecksumCRC32, true}, - {"CRC32C-composite", ChecksumCRC32C, false}, - {"CRC32C-full-object", ChecksumCRC32C, true}, - {"CRC64NVME-full-object", ChecksumCRC64NVME, false}, // testing with false, because it always is full object. - {"ChecksumSHA1-composite", ChecksumSHA1, false}, - {"ChecksumSHA256-composite", ChecksumSHA256, false}, + {"CRC32-composite", ChecksumCRC32, false, false}, + {"CRC32-full-object", ChecksumCRC32, true, false}, + {"CRC32C-composite", ChecksumCRC32C, false, false}, + {"CRC32C-full-object", ChecksumCRC32C, true, false}, + {"CRC64NVME-full-object", ChecksumCRC64NVME, false, false}, // CRC64NVME is always full object + {"ChecksumSHA1-composite", ChecksumSHA1, false, false}, + {"ChecksumSHA256-composite", ChecksumSHA256, false, false}, + {"ChecksumSHA1-full-object", ChecksumSHA1, true, true}, // SHA1 does not support full object + {"ChecksumSHA256-full-object", ChecksumSHA256, true, true}, // SHA256 does not support full object } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Skip invalid cases where SHA1 or SHA256 is used with full object + if (tt.checksum.Is(ChecksumSHA1) || tt.checksum.Is(ChecksumSHA256)) && tt.fullobj { + // Validate that NewChecksumType correctly marks these as invalid + alg := tt.checksum.String() + typ := NewChecksumType(alg, xhttp.AmzChecksumTypeFullObject) + if !typ.Is(ChecksumInvalid) { + t.Fatalf("Expected ChecksumInvalid for %s with full object, got %s", tt.name, typ.StringFull()) + } + return + } myData := []byte("this-is-a-checksum-data-test") chksm := NewChecksumFromData(tt.checksum, myData) + if chksm == nil { + t.Fatalf("NewChecksumFromData failed for %s", tt.name) + } if tt.fullobj { chksm.Type |= ChecksumFullObject } - w := httptest.NewRecorder() - AddChecksumHeader(w, chksm.AsMap()) - gotChksm, err := GetContentChecksum(w.Result().Header) - if err != nil { - t.Fatalf("GetContentChecksum failed: %v", err) - } - - // In the CRC64NVM case, it is always full object, so add the flag for easier equality comparison + // CRC64NVME is always full object if chksm.Type.Base().Is(ChecksumCRC64NVME) { chksm.Type |= ChecksumFullObject } + + // Prepare the checksum map with appropriate headers + m := chksm.AsMap() + m[xhttp.AmzChecksumAlgo] = chksm.Type.String() // Set the algorithm explicitly + if chksm.Type.FullObjectRequested() { + m[xhttp.AmzChecksumType] = xhttp.AmzChecksumTypeFullObject + } else { + m[xhttp.AmzChecksumType] = xhttp.AmzChecksumTypeComposite + } + + w := httptest.NewRecorder() + AddChecksumHeader(w, m) + gotChksm, err := GetContentChecksum(w.Result().Header) + if tt.wantErr { + if err == nil { + t.Fatalf("Expected error for %s, got none", tt.name) + } + return + } + if err != nil { + t.Fatalf("GetContentChecksum failed for %s: %v", tt.name, err) + } + + if gotChksm == nil { + t.Fatalf("Got nil checksum for %s", tt.name) + } + // Compare the full checksum structs if !chksm.Equal(gotChksm) { - t.Fatalf("Checksum mismatch: expected %+v, got %+v", chksm, gotChksm) + t.Errorf("Checksum mismatch for %s: expected %+v, got %+v", tt.name, chksm, gotChksm) + } + // Verify the checksum type + expectedType := chksm.Type + if gotChksm.Type != expectedType { + t.Errorf("Type mismatch for %s: expected %s, got %s", tt.name, expectedType.StringFull(), gotChksm.Type.StringFull()) } }) }