diff --git a/cmd/object-copy-federation_test.go b/cmd/object-copy-federation_test.go index c0fbd4dc9..e4f701ee2 100644 --- a/cmd/object-copy-federation_test.go +++ b/cmd/object-copy-federation_test.go @@ -44,6 +44,16 @@ type federationRemoteCapture struct { headers []http.Header } +type federationResponseFilter struct { + http.ResponseWriter + filter func(http.Header) +} + +func (w federationResponseFilter) WriteHeader(status int) { + w.filter(w.Header()) + w.ResponseWriter.WriteHeader(status) +} + func (c *federationRemoteCapture) record(h http.Header) { c.mu.Lock() c.headers = append(c.headers, h.Clone()) @@ -80,7 +90,7 @@ func (c *federationRemoteCapture) reservedKeys() []string { // minio-go client probes for. Leaving it unregistered makes the probe fall back // to the default region, exactly as TestAPIFederatedCopyObjectPartChecksum does. func setupCopyObjectFederation(t *testing.T, objectAPI ObjectLayer, apiRouter http.Handler, - instanceType, srcBucket string, + instanceType, srcBucket string, responseFilters ...func(http.Header), ) (remoteBucket string, capture *federationRemoteCapture, cleanup func()) { t.Helper() remoteBucket = getRandomBucketName() @@ -91,6 +101,9 @@ func setupCopyObjectFederation(t *testing.T, objectAPI ObjectLayer, apiRouter ht capture = &federationRemoteCapture{} remote := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { capture.record(r.Header) + if len(responseFilters) != 0 && r.Method == http.MethodPut { + w = federationResponseFilter{ResponseWriter: w, filter: responseFilters[0]} + } apiRouter.ServeHTTP(w, r) })) host, port, _ := strings.Cut(remote.Listener.Addr().String(), ":") @@ -237,6 +250,7 @@ func testAPIFederatedCopyObjectRequestedChecksum(objectAPI ObjectLayer, instance }{ {name: "CRC32", typ: hash.ChecksumCRC32, explicit: true}, {name: "CRC32C", typ: hash.ChecksumCRC32C, explicit: true}, + {name: "SHA1", typ: hash.ChecksumSHA1, explicit: true}, {name: "SHA256", typ: hash.ChecksumSHA256, explicit: true}, {name: "CRC64NVME", typ: hash.ChecksumCRC64NVME, explicit: true}, // Default: no requested algorithm still yields the S3 CRC-64NVME. @@ -262,6 +276,31 @@ func testAPIFederatedCopyObjectRequestedChecksum(objectAPI ObjectLayer, instance } } +func TestAPIFederatedCopyObjectRejectsInvalidRemoteChecksum(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: func(obj ObjectLayer, instanceType, bucket string, router http.Handler, credentials auth.Credentials, t *testing.T) { + data := []byte("remote checksum response fixture") + putCopyChecksumSource(t, router, credentials, bucket, "source", data, nil) + for _, value := range []string{"", "invalid-base64", "YQ=="} { + t.Run("checksum="+value, func(t *testing.T) { + remoteBucket, _, cleanup := setupCopyObjectFederation(t, obj, router, instanceType, bucket, func(header http.Header) { + header.Set(xhttp.AmzChecksumCRC32, value) + }) + defer cleanup() + rec := federatedCopyRequest(t, router, credentials, bucket, "source", remoteBucket, "destination", + map[string]string{xhttp.AmzChecksumAlgo: "CRC32"}) + if rec.Code < 500 { + t.Fatalf("invalid remote checksum must fail the copy, got %d: %s", rec.Code, rec.Body.String()) + } + }) + } + }, + endpoints: []string{"CopyObject", "PutObject", "HeadObject", "GetObject"}, + }) +} + // TestAPIFederatedCopyObjectChecksumIsBoundToWrite guards the checksum // representation: a federated copy that requests one algorithm must return only // that algorithm, and a copy of a checksum-less source without a requested diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 2fc0657f4..cbf4b3d14 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1958,15 +1958,14 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re objInfo.UserDefined = cloneMSS(opts.UserMetadata) objInfo.ETag = remoteObjInfo.ETag objInfo.ModTime = remoteObjInfo.LastModified - // Bind the checksum the remote computed for this exact write into the - // response, matching the local CopyObject path and the federated - // UploadPartCopy repair in #72. A malformed or absent remote value - // leaves objInfo.Checksum unset, so an ordinary copy returns none. + // Do not acknowledge a requested checksum the remote did not return. if wantChecksumType.IsSet() { - if cs := hash.NewChecksumWithType(wantChecksumType, - federatedChecksumValue(wantChecksumType, remoteObjInfo)); cs != nil { - objInfo.Checksum = cs.AppendTo(nil, nil) + cs := hash.NewChecksumWithType(wantChecksumType, federatedChecksumValue(wantChecksumType, remoteObjInfo)) + if cs == nil { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInternalError), r.URL) + return } + objInfo.Checksum = cs.AppendTo(nil, nil) } } else { os = newObjSweeper(dstBucket, dstObject).WithVersioning(dstOpts.Versioned, dstOpts.VersionSuspended)