From 47cd7807d37e9bf0006ff3da5d6679cd0f208a25 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Thu, 27 Aug 2026 10:23:33 +0800 Subject: [PATCH 01/18] test: align federated client version Signed-off-by: Feng Ruohang --- cmd/object-multipart-federation-checksum_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/object-multipart-federation-checksum_test.go b/cmd/object-multipart-federation-checksum_test.go index f7b8f5017..85269c7ee 100644 --- a/cmd/object-multipart-federation-checksum_test.go +++ b/cmd/object-multipart-federation-checksum_test.go @@ -40,7 +40,7 @@ import ( xhttp "github.com/minio/minio/internal/http" ) -const federatedTestUserAgent = "MinIO (linux; amd64) minio-go/v7.0.99 minio-federated/RELEASE.TEST" +const federatedTestUserAgent = "MinIO (linux; amd64) minio-go/v7.3.1 minio-federated/RELEASE.TEST" func TestAPIFederatedUploadPartChecksumResponse(t *testing.T) { defer DetectTestLeak(t)() From 38ed9d1e1fb23b0db05b9b507b893c26809a9cee Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Thu, 27 Aug 2026 10:23:51 +0800 Subject: [PATCH 02/18] docs(security): record inherited upstream advisory Signed-off-by: Feng Ruohang --- SECURITY.md | 19 +++++++++++++++++++ docs/security/advisories.md | 8 ++++++++ 2 files changed, 27 insertions(+) diff --git a/SECURITY.md b/SECURITY.md index fcd954b01..dcfd6e4ea 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -10,6 +10,25 @@ Security fixes are tracked on the active development branch and summarized in [docs/security/advisories.md](docs/security/advisories.md). Only the current Silo release line is supported unless an advisory says otherwise. +## Inherited Fix Evidence + +The canonical ledger also records security fixes inherited from upstream when +they are part of the Silo release baseline. Source and fork commits are linked +separately even when the fork preserves the original commit object and SHA. + +- [CVE-2025-62506](https://github.com/advisories/GHSA-jjjj-jwhf-8rgr): + upstream [PR #21642](https://github.com/minio/minio/pull/21642) merged as + [`minio/minio@c1a49490`](https://github.com/minio/minio/commit/c1a49490c78e9c3ebcad86ba0662319138ace190), + inherited unchanged as + [`pgsty/silo@c1a49490`](https://github.com/pgsty/silo/commit/c1a49490c78e9c3ebcad86ba0662319138ace190), + and is present in every Silo community release beginning with + [`RELEASE.2025-12-03T12-00-00Z`](https://github.com/pgsty/silo/releases/tag/RELEASE.2025-12-03T12-00-00Z). + The inherited [service-account](https://github.com/pgsty/silo/blob/c1a49490c78e9c3ebcad86ba0662319138ace190/cmd/admin-handlers-users_test.go#L211-L212) + and [STS](https://github.com/pgsty/silo/blob/c1a49490c78e9c3ebcad86ba0662319138ace190/cmd/sts-handlers_test.go#L45-L46) + regression groups remain part of `go test ./cmd`; see the + [canonical ledger](docs/security/advisories.md#inherited-upstream-advisory-baseline) + for the operator-facing record. + ## Reporting a Vulnerability For vulnerabilities in this fork: diff --git a/docs/security/advisories.md b/docs/security/advisories.md index 6aa2df3f9..7c7523ca8 100644 --- a/docs/security/advisories.md +++ b/docs/security/advisories.md @@ -4,6 +4,14 @@ This document summarizes fork-specific security fixes and closely related upgrad Entries carry a CVE identifier where one exists. Where none does, they carry a fork-local `SN--` identifier so that a finding without a CVE can still be referenced stably from release notes, commits and issues. An `SN-` identifier is **not** a CVE and is not registered in any vulnerability database; it is deliberately not written in CVE form so that scanners do not mistake it for one. Upstream `minio/minio` is archived, so for findings in inherited code there is no upstream maintainer to coordinate a CVE assignment with. `SN-2026-001` is the streaming-flush regression in `trackingResponseWriter`, which is a reliability defect rather than a security one and is tracked in the release notes rather than here. +## Inherited upstream advisory baseline + +The first Silo community release was cut from upstream history that already contained the following security fix. Upstream and Silo links are both recorded even when the fork preserves the same commit object and SHA; that identity is the inheritance evidence, not a claim that Silo independently reimplemented the patch. + +| ID | Upstream remediation | Silo inheritance | Regression evidence | Release / operator note | +| :-- | :-- | :-- | :-- | :-- | +| [CVE-2025-62506](https://github.com/advisories/GHSA-jjjj-jwhf-8rgr) | [minio/minio#21642](https://github.com/minio/minio/pull/21642), merged as [`c1a49490`](https://github.com/minio/minio/commit/c1a49490c78e9c3ebcad86ba0662319138ace190) | The same commit object is present as [`pgsty/silo@c1a49490`](https://github.com/pgsty/silo/commit/c1a49490c78e9c3ebcad86ba0662319138ace190) | The inherited [service-account](https://github.com/pgsty/silo/blob/c1a49490c78e9c3ebcad86ba0662319138ace190/cmd/admin-handlers-users_test.go#L211-L212) and [STS](https://github.com/pgsty/silo/blob/c1a49490c78e9c3ebcad86ba0662319138ace190/cmd/sts-handlers_test.go#L45-L46) regression groups run for root and non-root parents through `go test ./cmd` | Resets `DenyOnly` while evaluating a restricted session policy so service or STS accounts cannot mint an unrestricted child service account. Upstream first fixed this in [`RELEASE.2025-10-15T17-29-55Z`](https://github.com/minio/minio/releases/tag/RELEASE.2025-10-15T17-29-55Z); every Silo community release, beginning with [`RELEASE.2025-12-03T12-00-00Z`](https://github.com/pgsty/silo/releases/tag/RELEASE.2025-12-03T12-00-00Z), contains it. Operators migrating from an older upstream build should upgrade and audit service accounts created by restricted service or STS identities. | + ## Advisories since `RELEASE.2026-03-21T00-00-00Z` | ID | Fixed by | Affected area | Remote exploitability | Summary | Upgrade / workaround notes | From 229fe2b3c3d38345927992c6ab2d81bc5201f0a8 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Fri, 28 Aug 2026 08:56:58 +0800 Subject: [PATCH 03/18] fix: authorize group status changes by target status Signed-off-by: Feng Ruohang --- cmd/admin-handlers-users.go | 17 +++-- cmd/admin-handlers-users_test.go | 121 +++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 5 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index c9ac0f4d8..e5dc080c6 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -355,18 +355,25 @@ func (a adminAPIHandlers) ListGroups(w http.ResponseWriter, r *http.Request) { } // SetGroupStatus - PUT /minio/admin/v3/set-group-status?group=mygroup1&status=enabled +func setGroupStatusAdminAction(status string) policy.AdminAction { + if madmin.GroupStatus(status) == madmin.GroupDisabled { + return policy.DisableGroupAdminAction + } + return policy.EnableGroupAdminAction +} + func (a adminAPIHandlers) SetGroupStatus(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - objectAPI, _ := validateAdminReq(ctx, w, r, policy.EnableGroupAdminAction) - if objectAPI == nil { - return - } - vars := mux.Vars(r) group := vars["group"] status := vars["status"] + objectAPI, _ := validateAdminReq(ctx, w, r, setGroupStatusAdminAction(status)) + if objectAPI == nil { + return + } + var ( err error updatedAt time.Time diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 74ad6b8a3..9eea58877 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -68,6 +68,26 @@ func TestSetUserStatusAdminAction(t *testing.T) { } } +func TestSetGroupStatusAdminAction(t *testing.T) { + tests := []struct { + name string + status string + want policy.AdminAction + }{ + {name: "enable", status: string(madmin.GroupEnabled), want: policy.EnableGroupAdminAction}, + {name: "disable", status: string(madmin.GroupDisabled), want: policy.DisableGroupAdminAction}, + {name: "invalid preserves authenticated default", status: "invalid", want: policy.EnableGroupAdminAction}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := setGroupStatusAdminAction(tt.status); got != tt.want { + t.Fatalf("setGroupStatusAdminAction(%q) = %q, want %q", tt.status, got, tt.want) + } + }) + } +} + // API suite container for IAM type TestSuiteIAM struct { TestSuiteCommon @@ -224,6 +244,7 @@ func TestIAMInternalIDPServerSuite(t *testing.T) { suite.SetUpSuite(c) suite.TestUserCreate(c) suite.TestUserStatusActionAuthorization(c) + suite.TestGroupStatusActionAuthorization(c) suite.TestUserPolicyEscalationBug(c) suite.TestPolicyCreate(c) suite.TestServiceAccountBareARNPolicyRejected(c) @@ -413,6 +434,106 @@ func (s *TestSuiteIAM) TestUserStatusActionAuthorization(c *check) { } } +func (s *TestSuiteIAM) TestGroupStatusActionAuthorization(c *check) { + ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) + defer cancel() + + var createdUsers []string + var createdPolicies []string + group := getRandomBucketName() + var groupCreated bool + defer func() { + if groupCreated { + if err := s.adm.UpdateGroupMembers(ctx, madmin.GroupAddRemove{ + Group: group, + Members: createdUsers[:1], + IsRemove: true, + }); err != nil { + c.Errorf("unable to remove group member: %v", err) + } + if err := s.adm.UpdateGroupMembers(ctx, madmin.GroupAddRemove{Group: group, IsRemove: true}); err != nil { + c.Errorf("unable to remove test group: %v", err) + } + } + for _, user := range createdUsers { + if err := s.adm.RemoveUser(ctx, user); err != nil { + c.Errorf("unable to remove test user %s: %v", user, err) + } + } + for _, policyName := range createdPolicies { + if err := s.adm.RemoveCannedPolicy(ctx, policyName); err != nil { + c.Errorf("unable to remove test policy %s: %v", policyName, err) + } + } + }() + + createUser := func() (string, string) { + accessKey, secretKey := mustGenerateCredentials(c) + if err := s.adm.SetUser(ctx, accessKey, secretKey, madmin.AccountEnabled); err != nil { + c.Fatalf("unable to create test user: %v", err) + } + createdUsers = append(createdUsers, accessKey) + return accessKey, secretKey + } + + targetAccessKey, _ := createUser() + if err := s.adm.UpdateGroupMembers(ctx, madmin.GroupAddRemove{ + Group: group, + Members: []string{targetAccessKey}, + }); err != nil { + c.Fatalf("unable to create test group: %v", err) + } + groupCreated = true + + createStatusClient := func(action policy.AdminAction) *madmin.AdminClient { + accessKey, secretKey := createUser() + policyName := getRandomBucketName() + policyBytes := fmt.Appendf(nil, `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["%s"] + }] +}`, action) + if err := s.adm.AddCannedPolicy(ctx, policyName, policyBytes); err != nil { + c.Fatalf("unable to add group status policy: %v", err) + } + createdPolicies = append(createdPolicies, policyName) + if _, err := s.adm.AttachPolicy(ctx, madmin.PolicyAssociationReq{ + Policies: []string{policyName}, + User: accessKey, + }); err != nil { + c.Fatalf("unable to attach group status policy: %v", err) + } + + client, err := madmin.NewWithOptions(s.endpoint, &madmin.Options{ + Creds: credentials.NewStaticV4(accessKey, secretKey, ""), + Secure: s.secure, + }) + if err != nil { + c.Fatalf("unable to create group status admin client: %v", err) + } + client.SetCustomTransport(s.TestSuiteCommon.client.Transport) + return client + } + + disableClient := createStatusClient(policy.DisableGroupAdminAction) + if err := disableClient.SetGroupStatus(ctx, group, madmin.GroupDisabled); err != nil { + c.Fatalf("DisableGroup-only client could not disable a group: %v", err) + } + if err := disableClient.SetGroupStatus(ctx, group, madmin.GroupEnabled); err == nil || madmin.ToErrorResponse(err).Code != "AccessDenied" { + c.Fatalf("DisableGroup-only client unexpectedly enabled a group: %v", err) + } + + enableClient := createStatusClient(policy.EnableGroupAdminAction) + if err := enableClient.SetGroupStatus(ctx, group, madmin.GroupEnabled); err != nil { + c.Fatalf("EnableGroup-only client could not enable a group: %v", err) + } + if err := enableClient.SetGroupStatus(ctx, group, madmin.GroupDisabled); err == nil || madmin.ToErrorResponse(err).Code != "AccessDenied" { + c.Fatalf("EnableGroup-only client unexpectedly disabled a group: %v", err) + } +} + func (s *TestSuiteIAM) TestUserPolicyEscalationBug(c *check) { ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) defer cancel() From 7e079ff05c842afabb73c4572885cfb74604df17 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Fri, 28 Aug 2026 08:57:06 +0800 Subject: [PATCH 04/18] fix: validate explicit multipart checksum type Signed-off-by: Feng Ruohang --- cmd/erasure-multipart-fullobject_test.go | 66 ++++++++++++++++++++++++ cmd/erasure-multipart.go | 14 +++-- cmd/object-api-interface.go | 4 +- cmd/object-api-options.go | 8 ++- 4 files changed, 86 insertions(+), 6 deletions(-) diff --git a/cmd/erasure-multipart-fullobject_test.go b/cmd/erasure-multipart-fullobject_test.go index 0bcb14c4f..b9d71aa21 100644 --- a/cmd/erasure-multipart-fullobject_test.go +++ b/cmd/erasure-multipart-fullobject_test.go @@ -420,8 +420,74 @@ func testAPICompleteMultipartChecksumTypeMismatch(obj ObjectLayer, instanceType, t.Fatalf("%s: object was created despite a rejected checksum type", instanceType) } }) + + t.Run(test.name+"-type-only", func(t *testing.T) { + objectName := "type-mismatch/type-only-" + test.name + uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, + typ.String(), test.createdType) + etags := uploadPartsHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, typ, partData) + partCS := []string{mustChecksum(t, typ, partData[0]), mustChecksum(t, typ, partData[1])} + rec := completeMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, etags, partCS, + map[string]string{xhttp.AmzChecksumType: test.providedType}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("%s: checksum type-only mismatch returned %d, want 400", instanceType, rec.Code) + } + apiErr := apiError(t, rec) + if apiErr.Code != "BadDigest" { + t.Fatalf("%s: expected BadDigest, got %q", instanceType, apiErr.Code) + } + wantMessage := fmt.Sprintf("The checksum type %s does not match the multipart upload checksum type %s.", test.providedType, test.createdType) + if apiErr.Message != wantMessage { + t.Fatalf("%s: expected message %q, got %q", instanceType, wantMessage, apiErr.Message) + } + if _, err := obj.GetObjectInfo(t.Context(), bucketName, objectName, ObjectOptions{}); err == nil { + t.Fatalf("%s: object was created despite a rejected checksum type-only assertion", instanceType) + } + }) } + for _, test := range []struct { + name string + providedType string + withChecksum bool + }{ + {name: "unknown-type-only", providedType: "NOT_A_TYPE"}, + {name: "lowercase-type-only", providedType: "full_object"}, + {name: "unknown-with-checksum", providedType: "NOT_A_TYPE", withChecksum: true}, + } { + t.Run(test.name, func(t *testing.T) { + objectName := "type-mismatch/invalid-" + test.name + uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, + typ.String(), xhttp.AmzChecksumTypeComposite) + etags := uploadPartsHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, typ, partData) + partCS := []string{mustChecksum(t, typ, partData[0]), mustChecksum(t, typ, partData[1])} + headers := map[string]string{xhttp.AmzChecksumType: test.providedType} + if test.withChecksum { + headers[typ.Key()] = mustChecksum(t, typ, full) + } + rec := completeMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, etags, partCS, headers) + if rec.Code != http.StatusBadRequest || apiErrorCode(t, rec) != "InvalidArgument" { + t.Fatalf("%s: invalid checksum type returned %d %s", instanceType, rec.Code, rec.Body.String()) + } + if _, err := obj.GetObjectInfo(t.Context(), bucketName, objectName, ObjectOptions{}); err == nil { + t.Fatalf("%s: object was created despite an invalid checksum type", instanceType) + } + }) + } + + t.Run("matching-type-only", func(t *testing.T) { + objectName := "type-mismatch/matching-type-only" + uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, + typ.String(), xhttp.AmzChecksumTypeComposite) + etags := uploadPartsHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, typ, partData) + partCS := []string{mustChecksum(t, typ, partData[0]), mustChecksum(t, typ, partData[1])} + rec := completeMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, etags, partCS, + map[string]string{xhttp.AmzChecksumType: xhttp.AmzChecksumTypeComposite}) + if rec.Code != http.StatusOK { + t.Fatalf("%s: matching checksum type-only assertion returned %d %s", instanceType, rec.Code, rec.Body.String()) + } + }) + t.Run("omitted-type-is-not-composite", func(t *testing.T) { objectName := "type-mismatch/omitted-type" uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go index 222c458b5..d99492368 100644 --- a/cmd/erasure-multipart.go +++ b/cmd/erasure-multipart.go @@ -1173,9 +1173,9 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str var checksumType hash.ChecksumType if cs := fi.Metadata[hash.MinIOMultipartChecksum]; cs != "" { checksumType = hash.NewChecksumType(cs, fi.Metadata[hash.MinIOMultipartChecksumType]) + expectedType := checksumType | hash.ChecksumMultipart | hash.ChecksumIncludesMultipart if opts.WantChecksum != nil { providedType := opts.WantChecksum.Type | hash.ChecksumMultipart | hash.ChecksumIncludesMultipart - expectedType := checksumType | hash.ChecksumMultipart | hash.ChecksumIncludesMultipart if providedType.Base() != expectedType.Base() { return oi, InvalidArgument{ Bucket: bucket, @@ -1183,8 +1183,16 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str Err: fmt.Errorf("checksum algorithm mismatch. got %q expected %q", providedType.String(), expectedType.String()), } } - if opts.wantChecksumTypeSet && providedType.ObjType() != expectedType.ObjType() { - return oi, completeMultipartChecksumTypeMismatch(providedType.ObjType(), expectedType.ObjType()) + } + if opts.wantChecksumType != "" { + providedObjectType := opts.wantChecksumType + // CRC64NVME is always canonicalized to FULL_OBJECT. Preserve this + // behavior until its exact AWS wire semantics have been probed. + if checksumType.Base().Is(hash.ChecksumCRC64NVME) { + providedObjectType = xhttp.AmzChecksumTypeFullObject + } + if providedObjectType != expectedType.ObjType() { + return oi, completeMultipartChecksumTypeMismatch(opts.wantChecksumType, expectedType.ObjType()) } } checksumType |= hash.ChecksumMultipart | hash.ChecksumIncludesMultipart diff --git a/cmd/object-api-interface.go b/cmd/object-api-interface.go index 35d353ca6..f8664310d 100644 --- a/cmd/object-api-interface.go +++ b/cmd/object-api-interface.go @@ -84,8 +84,8 @@ type ObjectOptions struct { Expiration ExpirationOptions LifecycleAuditEvent lcAuditEvent - WantChecksum *hash.Checksum // x-amz-checksum-XXX checksum sent to PutObject/ CompleteMultipartUpload. - wantChecksumTypeSet bool // x-amz-checksum-type was explicitly set on CompleteMultipartUpload. + WantChecksum *hash.Checksum // x-amz-checksum-XXX checksum sent to PutObject/ CompleteMultipartUpload. + wantChecksumType string // explicit x-amz-checksum-type value on CompleteMultipartUpload. WantServerSideChecksumType hash.ChecksumType // if set, we compute a server-side checksum of this type diff --git a/cmd/object-api-options.go b/cmd/object-api-options.go index 828a8ff00..0d496e811 100644 --- a/cmd/object-api-options.go +++ b/cmd/object-api-options.go @@ -469,11 +469,17 @@ func completeMultipartOpts(ctx context.Context, r *http.Request, bucket, object } } + opts.wantChecksumType = r.Header.Get(xhttp.AmzChecksumType) + switch opts.wantChecksumType { + case "", xhttp.AmzChecksumTypeComposite, xhttp.AmzChecksumTypeFullObject: + default: + return opts, hash.ErrInvalidChecksum + } + opts.WantChecksum, err = hash.GetContentChecksum(r.Header) if err != nil { return opts, err } - opts.wantChecksumTypeSet = r.Header.Get(xhttp.AmzChecksumType) != "" opts.MTime = mtime opts.UserDefined = make(map[string]string) // Transfer SSEC key in opts.EncryptFn From 0b0ae2423af5a98e28d248c0e1de5f88760ff621 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Tue, 25 Aug 2026 08:20:23 +0800 Subject: [PATCH 05/18] fix: keep copy metadata consistent with a rewritten null version CopyObjectHandler recorded the source compression metadata whenever the copy was metadata-only, on the assumption that the object layer would then leave the stored bytes alone. That assumption does not hold. Both erasureServerPools.CopyObject and erasureSets.CopyObject only skip a data rewrite in three cases, and otherwise fall back to a full PutObject. The reachable gap is a copy whose source is a null version on a bucket that gained versioning after the object was written. Neither version ID is set, so the self-referential version branch is skipped, the data is rewritten as plaintext, and the preserved compression metadata then described bytes that no longer exist. A subsequent GET failed with "s2: corrupt input". Mirror the object layer's decision in copyRewritesObjectData and record the compression metadata from it, so the metadata always describes whichever bytes are finally stored. The source version selection that lets a versioned metadata-only copy add a self-referential version moves next to the same decision, since both depend on the effective metadata-only value. Signed-off-by: Feng Ruohang Co-Authored-By: Claude Opus 5 (1M context) --- cmd/object-copy-metadata_test.go | 119 +++++++++++++++++++++++++++++++ cmd/object-handlers.go | 46 ++++++++++-- 2 files changed, 158 insertions(+), 7 deletions(-) diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index 399f192ee..d3f81b774 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -202,3 +202,122 @@ func testAPICopyObjectSSECKeyRotationKeepsCompressionState(obj ObjectLayer, inst instanceType, response.Code, response.Body.Len(), len(data), response.Body.String()) } } + +// TestAPICopyObjectMetadataOnlyNullVersion covers the copy whose source is a +// null version on a bucket that gained versioning after the object was written. +// The object layer cannot reference such a version, so it rewrites the data and +// the recorded compression metadata has to describe the rewritten bytes. +func TestAPICopyObjectMetadataOnlyNullVersion(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPICopyObjectMetadataOnlyNullVersion, + endpoints: []string{"CopyObject", "PutObject", "GetObject"}, + }) +} + +func testAPICopyObjectMetadataOnlyNullVersion(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + restoreCompression := setCopyChecksumCompression(true) + compressionRestored := false + defer func() { + if !compressionRestored { + restoreCompression() + } + }() + + data := bytes.Repeat([]byte("null-version-metadata-copy-"), 64*1024) + want := mustChecksum(t, hash.ChecksumCRC32, data) + object := "copy-metadata/null-version.txt" + putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, + map[string]string{xhttp.AmzChecksumCRC32: want}) + + before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if !before.IsCompressed() || before.VersionID != "" { + t.Fatalf("%s: invalid null-version precondition: compressed=%v versionID=%q", + instanceType, before.IsCompressed(), before.VersionID) + } + + // Versioning is enabled after the write, so the object keeps a null version. + if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, + bucketVersioningConfig, enabledBucketVersioningConfig); err != nil { + t.Fatalf("%s: unable to enable versioning: %v", instanceType, err) + } + if !globalBucketVersioningSys.PrefixEnabled(bucketName, object) { + t.Fatalf("%s: versioning did not become enabled", instanceType) + } + + // Without compression the rewritten destination stores plaintext. + restoreCompression() + compressionRestored = true + + rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, + map[string]string{xhttp.AmzMetadataDirective: "REPLACE"}) + if rec.Code != http.StatusOK { + t.Fatalf("%s: metadata-only CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String()) + } + + after := assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, false, nil) + if after.VersionID == "" { + t.Fatalf("%s: versioned copy did not create a new version", instanceType) + } + if got := readCopyChecksumObject(t, obj, bucketName, object, ObjectOptions{}); !bytes.Equal(got, data) { + t.Fatalf("%s: copied object body differs: got %d bytes, want %d", instanceType, len(got), len(data)) + } +} + +func TestCopyRewritesObjectData(t *testing.T) { + tests := []struct { + name string + metadataOnly bool + srcOpts ObjectOptions + dstOpts ObjectOptions + want bool + }{ + { + name: "data copy always rewrites", + want: true, + }, + { + name: "unversioned in-place metadata update", + metadataOnly: true, + }, + { + name: "addressed version updated in place", + metadataOnly: true, + srcOpts: ObjectOptions{VersionID: "v1"}, + dstOpts: ObjectOptions{VersionID: "v1"}, + }, + { + name: "versioned self referential version", + metadataOnly: true, + srcOpts: ObjectOptions{VersionID: "v1"}, + dstOpts: ObjectOptions{Versioned: true}, + }, + { + name: "versioned null source version cannot be referenced", + metadataOnly: true, + dstOpts: ObjectOptions{Versioned: true}, + want: true, + }, + { + name: "suspended destination with an addressed source version", + metadataOnly: true, + srcOpts: ObjectOptions{VersionID: "v1"}, + dstOpts: ObjectOptions{VersionSuspended: true, VersionID: nullVersionID}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := copyRewritesObjectData(tt.metadataOnly, tt.srcOpts, tt.dstOpts); got != tt.want { + t.Fatalf("copyRewritesObjectData() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 649136227..d25e99b5c 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1179,6 +1179,31 @@ func isRemoteCallRequired(ctx context.Context, bucket string, objAPI ObjectLayer return false } +// copyRewritesObjectData reports whether the object layer stores new object data +// for this copy instead of updating metadata in place or adding a +// self-referential version. It mirrors the metadata-only decision taken by +// erasureServerPools.CopyObject and erasureSets.CopyObject. CopyObjectHandler +// has to predict that decision because the compression metadata it records must +// describe whichever bytes are finally stored. metadataOnly already excludes +// legacy sources, which the object layer always rewrites. +func copyRewritesObjectData(metadataOnly bool, srcOpts, dstOpts ObjectOptions) bool { + if !metadataOnly { + return true + } + switch { + case dstOpts.VersionID != "" && srcOpts.VersionID == dstOpts.VersionID: + // In-place update of the addressed version. + return false + case !dstOpts.Versioned && srcOpts.VersionID == "": + // In-place update of an unversioned object. + return false + case dstOpts.Versioned && srcOpts.VersionID != dstOpts.VersionID: + // A new version referencing the existing data. + return false + } + return true +} + // CopyObjectHandler - Copy Object // ---------- // This implementation of the PUT operation adds an object to a bucket @@ -1708,8 +1733,20 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re srcInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationStatus] = dsc.PendingStatus() srcInfo.UserDefined[ReservedMetadataPrefixLower+ReplicationTimestamp] = UTCNow().Format(time.RFC3339Nano) } - // Compression metadata must describe data that is actually rewritten. - if !srcInfo.metadataOnly || srcInfo.Legacy || dstOpts.WantServerSideChecksumType.IsSet() { + // srcInfo.metadataOnly is still cleared below for legacy sources and for + // server-side checksum recomputation; both of those rewrite the object data. + metadataOnly := srcInfo.metadataOnly && !srcInfo.Legacy && !dstOpts.WantServerSideChecksumType.IsSet() + + // Name the source version explicitly so a metadata-only copy into a + // versioned bucket adds a self-referential version instead of rewriting the + // object data. A null source version cannot be referenced this way. + copySrcOpts := srcOpts + if metadataOnly && dstOpts.Versioned && copySrcOpts.VersionID == "" { + copySrcOpts.VersionID = srcInfo.VersionID + } + + // Compression metadata must describe the bytes that are actually stored. + if copyRewritesObjectData(metadataOnly, copySrcOpts, dstOpts) { if isDstCompressed { maps.Copy(srcInfo.UserDefined, compressMetadata) } else { @@ -1808,11 +1845,6 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re copyObjectFn := objectAPI.CopyObject - copySrcOpts := srcOpts - if srcInfo.metadataOnly && dstOpts.Versioned && copySrcOpts.VersionID == "" { - copySrcOpts.VersionID = srcInfo.VersionID - } - // Copy source object to destination, if source and destination // object is same then only metadata is updated. objInfo, err = copyObjectFn(ctx, srcBucket, srcObject, dstBucket, dstObject, srcInfo, copySrcOpts, dstOpts) From e73436c99d65e9b40cde2149111f62e0b0907aae Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Fri, 28 Aug 2026 08:57:15 +0800 Subject: [PATCH 06/18] fix: decrypt CopyObject checksums with destination key Signed-off-by: Feng Ruohang --- cmd/api-response.go | 3 +-- cmd/object-copy-checksum_test.go | 38 ++++++++++++++++++++++++++++++++ cmd/object-copy-metadata_test.go | 2 ++ cmd/object-handlers-common.go | 6 ++++- cmd/object-handlers.go | 14 ++++++++++-- 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/cmd/api-response.go b/cmd/api-response.go index c6ad7b07e..43020a5a3 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -777,8 +777,7 @@ func generateListObjectsV2Response(ctx context.Context, bucket, prefix, token, n type metaCheckFn = func(name string, action policy.Action) (s3Err APIErrorCode) // generates CopyObjectResponse from the committed object information. -func generateCopyObjectResponse(oi ObjectInfo, h http.Header) CopyObjectResponse { - cs, _ := oi.decryptChecksums(0, h) +func generateCopyObjectResponse(oi ObjectInfo, cs map[string]string) CopyObjectResponse { return CopyObjectResponse{ ETag: "\"" + oi.ETag + "\"", LastModified: amztime.ISO8601Format(oi.ModTime.UTC()), diff --git a/cmd/object-copy-checksum_test.go b/cmd/object-copy-checksum_test.go index ad373f171..2f2c56328 100644 --- a/cmd/object-copy-checksum_test.go +++ b/cmd/object-copy-checksum_test.go @@ -387,6 +387,44 @@ func testAPICopyObjectServerSideChecksumEncryption(obj ObjectLayer, instanceType } }) } + + oldKey := bytes.Repeat([]byte{0x31}, 32) + oldKeyMD5 := md5.Sum(oldKey) + newKey := bytes.Repeat([]byte{0x42}, 32) + newKeyMD5 := md5.Sum(newKey) + encryptedSource := "copy-checksum/sse-c-different-key-source.bin" + putCopyChecksumSource(t, apiRouter, credentials, bucketName, encryptedSource, data, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldKeyMD5[:]), + }) + + destination := "copy-checksum/sse-c-different-key-destination.bin" + rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, encryptedSource, destination, map[string]string{ + xhttp.AmzChecksumAlgo: hash.ChecksumCRC32.String(), + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newKeyMD5[:]), + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldKeyMD5[:]), + }) + if rec.Code != http.StatusOK { + t.Fatalf("%s: different-key SSE-C CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String()) + } + assertCopyChecksumResponse(t, rec, hash.ChecksumCRC32, data) + if got, want := rec.Header().Get(hash.ChecksumCRC32.Key()), mustChecksum(t, hash.ChecksumCRC32, data); got != want { + t.Fatalf("%s: different-key SSE-C response header checksum %q, want %q", instanceType, got, want) + } + if got := rec.Header().Get(xhttp.AmzChecksumType); got != xhttp.AmzChecksumTypeFullObject { + t.Fatalf("%s: different-key SSE-C response checksum type %q, want %q", instanceType, got, xhttp.AmzChecksumTypeFullObject) + } + newKeyHeaders := http.Header{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: []string{xhttp.AmzEncryptionAES}, + xhttp.AmzServerSideEncryptionCustomerKey: []string{base64.StdEncoding.EncodeToString(newKey)}, + xhttp.AmzServerSideEncryptionCustomerKeyMD5: []string{base64.StdEncoding.EncodeToString(newKeyMD5[:])}, + } + assertCopyChecksum(t, obj, bucketName, destination, hash.ChecksumCRC32, data, false, newKeyHeaders) }) } diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index 43470436d..399f192ee 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -150,6 +150,7 @@ func testAPICopyObjectSSECKeyRotationKeepsCompressionState(obj ObjectLayer, inst newMD5 := md5.Sum(newKey) putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, map[string]string{ + xhttp.AmzChecksumCRC32: mustChecksum(t, hash.ChecksumCRC32, data), xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), @@ -172,6 +173,7 @@ func testAPICopyObjectSSECKeyRotationKeepsCompressionState(obj ObjectLayer, inst if rec.Code != http.StatusOK { t.Fatalf("%s: key rotation failed: %d %s", instanceType, rec.Code, rec.Body.String()) } + assertCopyChecksumResponse(t, rec, hash.ChecksumCRC32, data) after, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) if err != nil { t.Fatal(err) diff --git a/cmd/object-handlers-common.go b/cmd/object-handlers-common.go index a6febc122..abb4c49f8 100644 --- a/cmd/object-handlers-common.go +++ b/cmd/object-handlers-common.go @@ -353,6 +353,11 @@ func isETagEqual(left, right string) bool { // upon a success Put/Copy/CompleteMultipart/Delete requests // to activate delete only headers set delete as true func setPutObjHeaders(w http.ResponseWriter, objInfo ObjectInfo, del bool, h http.Header) { + cs, _ := objInfo.decryptChecksums(0, h) + setPutObjHeadersWithChecksum(w, objInfo, del, cs) +} + +func setPutObjHeadersWithChecksum(w http.ResponseWriter, objInfo ObjectInfo, del bool, cs map[string]string) { // We must not use the http.Header().Set method here because some (broken) // clients expect the ETag header key to be literally "ETag" - not "Etag" (case-sensitive). // Therefore, we have to set the ETag directly as map entry. @@ -374,7 +379,6 @@ func setPutObjHeaders(w http.ResponseWriter, objInfo ObjectInfo, del bool, h htt lc.SetPredictionHeaders(w, objInfo.ToLifecycleOpts()) } } - cs, _ := objInfo.decryptChecksums(0, h) hash.AddChecksumHeader(w, cs) } diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index d5e868925..649136227 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1107,6 +1107,14 @@ func cloneRequestWithoutCopyReplicationHeaders(r *http.Request) *http.Request { return clone } +func copyDestinationSSEHeaders(h http.Header) http.Header { + dst := h.Clone() + dst.Del(xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm) + dst.Del(xhttp.AmzServerSideEncryptionCopyCustomerKey) + dst.Del(xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5) + return dst +} + // getRemoteInstanceTransport contains a roundtripper for external (not peers) servers var remoteInstanceTransport atomic.Value @@ -1816,14 +1824,16 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re origETag := objInfo.ETag objInfo.ETag = getDecryptedETag(r.Header, objInfo, false) - response := generateCopyObjectResponse(objInfo, r.Header) + dstHeaders := copyDestinationSSEHeaders(r.Header) + checksums, _ := objInfo.decryptChecksums(0, dstHeaders) + response := generateCopyObjectResponse(objInfo, checksums) encodedSuccessResponse := encodeResponse(response) if dsc := mustReplicate(ctx, dstBucket, dstObject, objInfo.getMustReplicateOptions(replication.ObjectReplicationType, dstOpts)); dsc.ReplicateAny() { scheduleReplication(ctx, objInfo, objectAPI, dsc, replication.ObjectReplicationType) } - setPutObjHeaders(w, objInfo, false, r.Header) + setPutObjHeadersWithChecksum(w, objInfo, false, checksums) // We must not use the http.Header().Set method here because some (broken) // clients expect the x-amz-copy-source-version-id header key to be literally // "x-amz-copy-source-version-id"- not in canonicalized form, preserve it. From ffb70eb373640b1a67f7426d879a8dd94310dfe8 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 13:28:47 +0800 Subject: [PATCH 07/18] fix: re-encrypt a key rotation the object layer has to rewrite A key rotation rewraps the object key held in metadata; it never re-encrypts the stored bytes. CopyObjectHandler took that shortcut whenever the request looked like a same-object SSE-C rotation, on the assumption that the object layer would then leave the stored bytes alone. That is the same assumption copyRewritesObjectData() was added to stop making. When the source is a null version on a bucket that gained versioning after the object was written, the object layer cannot reference that version and falls back to PutObject. The reader at that point holds plaintext decrypted with the old key and no EncryptFn is set, so the destination ends up storing plaintext under metadata that claims the object is SSE-C encrypted. A subsequent GET failed with "sio: unsupported version". Gate the rotation shortcut on the same prediction the compression metadata already uses. When the object layer stores new object data the rotation falls through to the regular re-encrypting copy, which decrypts with the old key and re-encrypts with the new one. The source version selection moves next to the gate because both decisions need it. That fallback authenticates the source key through the source decryptor, which GetObjectNInfo does not build for a zero byte object. Check the key explicitly before the destination is written, so the gate cannot turn a rotation that the shortcut rejected with AccessDenied into one that succeeds. The re-encrypting copy regenerates the encrypted ETag, unlike an in-place rotation; the test records that difference. The other three object layer CopyObject callers that set metadataOnly - PostRestoreObjectHandler, updateRestoreMetadata and batchKeyRotate - address the same version on both sides and never set Versioned, so they only reach the two in-place cases already covered by the copyRewritesObjectData table. Signed-off-by: Feng Ruohang Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Fk3PAD7DHCYzcyegYWqAmt --- cmd/encryption-v1.go | 23 ++++ cmd/erasure-server-pool.go | 2 + cmd/erasure-sets.go | 2 + cmd/object-copy-metadata_test.go | 187 +++++++++++++++++++++++++++++++ cmd/object-handlers.go | 37 ++++-- 5 files changed, 242 insertions(+), 9 deletions(-) diff --git a/cmd/encryption-v1.go b/cmd/encryption-v1.go index c3da051a8..848f8cee1 100644 --- a/cmd/encryption-v1.go +++ b/cmd/encryption-v1.go @@ -355,6 +355,29 @@ func rotateKey(ctx context.Context, oldKey []byte, newKeyID string, newKey []byt } } +// checkSSECCopySourceKey authenticates the SSE-C copy source key against the +// sealed object key held in metadata. GetObjectNInfo builds no decryptor for a +// zero byte object, so a copy whose data path never decrypts anything has to +// verify the source key explicitly. Mirrors the errors rotateKey reports. +func checkSSECCopySourceKey(h http.Header, metadata map[string]string, bucket, object string, newKey []byte) error { + oldKey, err := ParseSSECopyCustomerRequest(h, metadata) + if err != nil { + return err + } + sealedKey, err := crypto.SSEC.ParseMetadata(metadata) + if err != nil { + return err + } + var objectKey crypto.ObjectKey + if err := objectKey.Unseal(oldKey, sealedKey, crypto.SSEC.String(), bucket, object); err != nil { + if subtle.ConstantTimeCompare(oldKey, newKey) == 1 { + return errInvalidSSEParameters + } + return crypto.ErrInvalidCustomerKey + } + return nil +} + func newEncryptMetadata(ctx context.Context, kind crypto.Type, keyID string, key []byte, bucket, object string, metadata map[string]string, cryptoCtx kms.Context) (crypto.ObjectKey, error) { var sealedKey crypto.SealedKey switch kind { diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index dc65d06ee..af804fb02 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -1328,6 +1328,8 @@ func (z *erasureServerPools) CopyObject(ctx context.Context, srcBucket, srcObjec return objInfo, err } + // CopyObjectHandler predicts the outcome of this decision in + // copyRewritesObjectData(); keep the two in sync. if cpSrcDstSame && srcInfo.metadataOnly { // Version ID is set for the destination and source == destination version ID. if dstOpts.VersionID != "" && srcOpts.VersionID == dstOpts.VersionID { diff --git a/cmd/erasure-sets.go b/cmd/erasure-sets.go index 95a7ed339..44401e0db 100644 --- a/cmd/erasure-sets.go +++ b/cmd/erasure-sets.go @@ -839,6 +839,8 @@ func (s *erasureSets) CopyObject(ctx context.Context, srcBucket, srcObject, dstB cpSrcDstSame := srcSet == dstSet // Check if this request is only metadata update. + // CopyObjectHandler predicts the outcome of this decision in + // copyRewritesObjectData(); keep the two in sync. if cpSrcDstSame && srcInfo.metadataOnly { // Version ID is set for the destination and source == destination version ID. // perform an in-place update. diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index d3f81b774..e224ffecd 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -282,6 +282,9 @@ func TestCopyRewritesObjectData(t *testing.T) { name: "data copy always rewrites", want: true, }, + // PostRestoreObjectHandler, updateRestoreMetadata and batchKeyRotate all + // address the same version on both sides and never set Versioned, so they + // only ever reach these two cases. { name: "unversioned in-place metadata update", metadataOnly: true, @@ -321,3 +324,187 @@ func TestCopyRewritesObjectData(t *testing.T) { }) } } + +// TestAPICopyObjectSSECKeyRotationNullVersion covers an SSE-C key rotation whose +// source is a null version on a bucket that gained versioning after the object +// was written. A rotation only rewraps the object key held in metadata, so it +// may not take the metadata-only path when the object layer stores new object +// data; the rotation has to re-encrypt instead. +func TestAPICopyObjectSSECKeyRotationNullVersion(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPICopyObjectSSECKeyRotationNullVersion, + endpoints: []string{"CopyObject", "PutObject", "GetObject"}, + }) +} + +func testAPICopyObjectSSECKeyRotationNullVersion(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + previousTLS := globalIsTLS + globalIsTLS = true + defer func() { globalIsTLS = previousTLS }() + + data := bytes.Repeat([]byte("key-rotation-null-version-"), 64*1024) + object := "copy-metadata/key-rotation-null.txt" + oldKey := bytes.Repeat([]byte{0x11}, 32) + oldMD5 := md5.Sum(oldKey) + newKey := bytes.Repeat([]byte{0x22}, 32) + newMD5 := md5.Sum(newKey) + + putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, map[string]string{ + xhttp.AmzChecksumCRC32: mustChecksum(t, hash.ChecksumCRC32, data), + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), + }) + before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if before.VersionID != "" { + t.Fatalf("%s: invalid null-version precondition: versionID=%q", instanceType, before.VersionID) + } + + // Versioning is enabled after the write, so the object keeps a null version. + if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, + bucketVersioningConfig, enabledBucketVersioningConfig); err != nil { + t.Fatalf("%s: unable to enable versioning: %v", instanceType, err) + } + if !globalBucketVersioningSys.PrefixEnabled(bucketName, object) { + t.Fatalf("%s: versioning did not become enabled", instanceType) + } + + rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]), + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), + }) + if rec.Code != http.StatusOK { + t.Fatalf("%s: key rotation failed: %d %s", instanceType, rec.Code, rec.Body.String()) + } + + assertCopyChecksumResponse(t, rec, hash.ChecksumCRC32, data) + + getHeaders := map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]), + } + req, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucketName, object), + 0, nil, credentials.AccessKey, credentials.SecretKey, getHeaders) + if err != nil { + t.Fatalf("failed to build GetObject request: %v", err) + } + response := httptest.NewRecorder() + apiRouter.ServeHTTP(response, req) + if response.Code != http.StatusOK || !bytes.Equal(response.Body.Bytes(), data) { + t.Fatalf("%s: post-rotation GetObject returned %d with %d bytes, want 200 with %d bytes: %s", + instanceType, response.Code, response.Body.Len(), len(data), response.Body.String()) + } + + decryptHeaders := http.Header{} + for key, value := range getHeaders { + decryptHeaders.Set(key, value) + } + after := assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, false, decryptHeaders) + if after.VersionID == "" { + t.Fatalf("%s: rotation into a versioned bucket did not create a new version", instanceType) + } + // The rotation could not be applied in place, so the object was re-encrypted + // under a fresh object key. That regenerates the encrypted ETag, unlike an + // in-place rotation which leaves the stored bytes and the ETag alone. + if after.ETag == before.ETag { + t.Fatalf("%s: re-encrypting rotation kept the source ETag %q", instanceType, after.ETag) + } +} + +// TestAPICopyObjectSSECKeyRotationNullVersionWrongKey pins the source key +// authentication of the re-encrypting fallback. A zero byte source has no data +// to decrypt, so the copy would otherwise reach the destination write without +// ever proving the caller holds the current key. +func TestAPICopyObjectSSECKeyRotationNullVersionWrongKey(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPICopyObjectSSECKeyRotationNullVersionWrongKey, + endpoints: []string{"CopyObject", "PutObject", "GetObject"}, + }) +} + +func testAPICopyObjectSSECKeyRotationNullVersionWrongKey(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + previousTLS := globalIsTLS + globalIsTLS = true + defer func() { globalIsTLS = previousTLS }() + + object := "copy-metadata/key-rotation-null-empty.txt" + oldKey := bytes.Repeat([]byte{0x11}, 32) + oldMD5 := md5.Sum(oldKey) + wrongKey := bytes.Repeat([]byte{0x33}, 32) + wrongMD5 := md5.Sum(wrongKey) + newKey := bytes.Repeat([]byte{0x22}, 32) + newMD5 := md5.Sum(newKey) + + putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, nil, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), + }) + before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if before.Size != 0 || before.VersionID != "" || len(before.Checksum) != 0 { + t.Fatalf("%s: invalid empty null-version precondition: size=%d versionID=%q checksum=%d", + instanceType, before.Size, before.VersionID, len(before.Checksum)) + } + + if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, + bucketVersioningConfig, enabledBucketVersioningConfig); err != nil { + t.Fatalf("%s: unable to enable versioning: %v", instanceType, err) + } + + rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]), + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(wrongKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(wrongMD5[:]), + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("%s: rotation with an incorrect source key returned %d, want %d: %s", + instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) + } + + after, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if after.VersionID != "" { + t.Fatalf("%s: rejected rotation still created version %q", instanceType, after.VersionID) + } + + // The object stays readable with the key it was written under. + req, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucketName, object), + 0, nil, credentials.AccessKey, credentials.SecretKey, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), + }) + if err != nil { + t.Fatalf("failed to build GetObject request: %v", err) + } + response := httptest.NewRecorder() + apiRouter.ServeHTTP(response, req) + if response.Code != http.StatusOK || response.Body.Len() != 0 { + t.Fatalf("%s: original object no longer readable: %d with %d bytes: %s", + instanceType, response.Code, response.Body.Len(), response.Body.String()) + } +} diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index d25e99b5c..2a74c31b2 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1488,12 +1488,39 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re } } + // Name the source version explicitly so a metadata-only copy into a + // versioned bucket adds a self-referential version instead of rewriting the + // object data. A null source version cannot be referenced this way. + copySrcOpts := srcOpts + if dstOpts.Versioned && copySrcOpts.VersionID == "" { + copySrcOpts.VersionID = srcInfo.VersionID + } + + // A key rotation rewraps the object key held in metadata; it never + // re-encrypts the stored bytes. When the object layer stores new object + // data instead, the rotation has to go through the regular re-encrypting + // copy, or the destination ends up holding plaintext under metadata that + // claims the object is encrypted. + canRotateKeyInPlace := !srcInfo.Legacy && + !copyRewritesObjectData(srcInfo.metadataOnly, copySrcOpts, dstOpts) + + // The rotation shortcut authenticates the source key by unsealing it. The + // re-encrypting fallback authenticates it only through the source decryptor, + // which GetObjectNInfo skips for a zero byte object, so check it here before + // the destination is written under the new key. + if cpSrcDstSame && sseCopyC && sseC && !chStorageClass && !canRotateKeyInPlace { + if err := checkSSECCopySourceKey(r.Header, srcInfo.UserDefined, srcBucket, srcObject, newKey); err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) + return + } + } + // If src == dst and either // - the object is encrypted using SSE-C and two different SSE-C keys are present // - the object is encrypted using SSE-S3 and the SSE-S3 header is present // - the object storage class is not changing // then execute a key rotation. - if cpSrcDstSame && (sseCopyC && sseC) && !chStorageClass { + if cpSrcDstSame && (sseCopyC && sseC) && !chStorageClass && canRotateKeyInPlace { oldKey, err = ParseSSECopyCustomerRequest(r.Header, srcInfo.UserDefined) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) @@ -1737,14 +1764,6 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re // server-side checksum recomputation; both of those rewrite the object data. metadataOnly := srcInfo.metadataOnly && !srcInfo.Legacy && !dstOpts.WantServerSideChecksumType.IsSet() - // Name the source version explicitly so a metadata-only copy into a - // versioned bucket adds a self-referential version instead of rewriting the - // object data. A null source version cannot be referenced this way. - copySrcOpts := srcOpts - if metadataOnly && dstOpts.Versioned && copySrcOpts.VersionID == "" { - copySrcOpts.VersionID = srcInfo.VersionID - } - // Compression metadata must describe the bytes that are actually stored. if copyRewritesObjectData(metadataOnly, copySrcOpts, dstOpts) { if isDstCompressed { From 2aea7fe9c48008e4ba438b9e6e6bb29de9b3de40 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Fri, 28 Aug 2026 08:57:22 +0800 Subject: [PATCH 08/18] fix: preserve named targets in config environment files Signed-off-by: Feng Ruohang --- cmd/common-main.go | 11 +++------ cmd/common-main_test.go | 54 ++++++++++++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/cmd/common-main.go b/cmd/common-main.go index 7f6ead797..e859f881a 100644 --- a/cmd/common-main.go +++ b/cmd/common-main.go @@ -37,6 +37,7 @@ import ( "syscall" "time" "unicode" + "unicode/utf8" "github.com/dustin/go-humanize" fcolor "github.com/fatih/color" @@ -542,21 +543,17 @@ func (e envKV) String() string { } func isValidEnvName(name string) bool { - if name == "" || !isEnvNameStart(name[0]) { + if name == "" || !utf8.ValidString(name) { return false } - for i := 1; i < len(name); i++ { - if !isEnvNameStart(name[i]) && (name[i] < '0' || name[i] > '9') { + for _, ch := range name { + if ch == '=' || unicode.IsSpace(ch) || !unicode.IsGraphic(ch) { return false } } return true } -func isEnvNameStart(ch byte) bool { - return ch == '_' || ch >= 'A' && ch <= 'Z' || ch >= 'a' && ch <= 'z' -} - func trimExportPrefix(envEntry string) string { rest, ok := strings.CutPrefix(envEntry, "export") if !ok || rest == "" { diff --git a/cmd/common-main_test.go b/cmd/common-main_test.go index 341ddba49..8afcaaf2b 100644 --- a/cmd/common-main_test.go +++ b/cmd/common-main_test.go @@ -22,8 +22,11 @@ import ( "fmt" "os" "reflect" + "slices" "strings" "testing" + + "github.com/minio/minio/internal/config" ) func Test_readFromSecret(t *testing.T) { @@ -240,6 +243,19 @@ func Test_minioEnvironFromFileWhitespaceAndValidation(t *testing.T) { {Key: "_VALID_2", Value: "value"}, }, }, + { + name: "named target punctuation and unicode", + content: "MINIO_NOTIFY_WEBHOOK_ENABLE_my-hook=off\n" + + "MINIO_NOTIFY_WEBHOOK_ENABLE_site.eu=off\n" + + "MINIO_NOTIFY_WEBHOOK_ENABLE_team:blue=off\n" + + "MINIO_NOTIFY_WEBHOOK_ENABLE_目标=off", + want: []envKV{ + {Key: "MINIO_NOTIFY_WEBHOOK_ENABLE_my-hook", Value: "off"}, + {Key: "MINIO_NOTIFY_WEBHOOK_ENABLE_site.eu", Value: "off"}, + {Key: "MINIO_NOTIFY_WEBHOOK_ENABLE_team:blue", Value: "off"}, + {Key: "MINIO_NOTIFY_WEBHOOK_ENABLE_目标", Value: "off"}, + }, + }, { name: "missing separator redacts the line", content: "MINIO_ROOT_PASSWORD=valid\nsuper-secret-without-equals", @@ -255,18 +271,12 @@ func Test_minioEnvironFromFileWhitespaceAndValidation(t *testing.T) { errExcludes: "empty-name-secret", }, { - name: "digit leading name", - content: "1MINIO_ROOT_USER=digit-leading-secret", - errLine: 1, - errContains: `invalid environment variable name "1MINIO_ROOT_USER"`, - errExcludes: "digit-leading-secret", - }, - { - name: "hyphenated name", - content: "MINIO-ROOT-USER=hyphen-secret", - errLine: 1, - errContains: `invalid environment variable name "MINIO-ROOT-USER"`, - errExcludes: "hyphen-secret", + name: "os compatible leading digit and punctuation", + content: "1MINIO_ROOT_USER=digit-leading-secret\n-MINIO-ROOT-USER=hyphen-secret", + want: []envKV{ + {Key: "1MINIO_ROOT_USER", Value: "digit-leading-secret"}, + {Key: "-MINIO-ROOT-USER", Value: "hyphen-secret"}, + }, }, { name: "whitespace in name", @@ -282,6 +292,13 @@ func Test_minioEnvironFromFileWhitespaceAndValidation(t *testing.T) { errContains: "invalid environment variable name", errExcludes: "nul-name-secret", }, + { + name: "format character in name", + content: "MINIO\u200bROOT=format-secret", + errLine: 1, + errContains: "invalid environment variable name", + errExcludes: "format-secret", + }, { name: "NUL in value", content: "MINIO_ROOT_USER=before\x00nul-value-secret", @@ -342,3 +359,16 @@ func Test_minioEnvironFromFileWhitespaceAndValidation(t *testing.T) { }) } } + +func TestConfigEnvFileNamedTargetDiscovery(t *testing.T) { + key := "MINIO_NOTIFY_WEBHOOK_ENABLE_my-hook" + t.Setenv(key, "off") + + targets, err := (config.Config{}).GetAvailableTargets(config.NotifyWebhookSubSys) + if err != nil { + t.Fatal(err) + } + if !slices.Contains(targets, "my-hook") { + t.Fatalf("named target %q not discovered from %s: %v", "my-hook", key, targets) + } +} From 57329301023067ea3b4c86d2dcb5e7aa30a8ab35 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 16:28:42 +0800 Subject: [PATCH 09/18] test: cover null-version copy rewrite directions Exercise the silent compression-on-copy path, compressed SSE-C re-encryption, and the equal-invalid-key error contract on both object-layer backends. Signed-off-by: Feng Ruohang --- cmd/object-copy-metadata_test.go | 100 ++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index e224ffecd..95d4f7d3d 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -270,6 +270,64 @@ func testAPICopyObjectMetadataOnlyNullVersion(obj ObjectLayer, instanceType, buc } } +func TestAPICopyObjectMetadataOnlyNullVersionCompressesRewrite(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPICopyObjectMetadataOnlyNullVersionCompressesRewrite, + endpoints: []string{"CopyObject", "PutObject", "GetObject"}, + }) +} + +func testAPICopyObjectMetadataOnlyNullVersionCompressesRewrite(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + globalCompressConfigMu.Lock() + previousCompression := globalCompressConfig + globalCompressConfig.Enabled = false + globalCompressConfigMu.Unlock() + defer func() { + globalCompressConfigMu.Lock() + globalCompressConfig = previousCompression + globalCompressConfigMu.Unlock() + }() + + data := bytes.Repeat([]byte("null-version-compress-rewrite-"), 64*1024) + want := mustChecksum(t, hash.ChecksumCRC32, data) + object := "copy-metadata/null-version-compress.txt" + putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, data, + map[string]string{xhttp.AmzChecksumCRC32: want}) + + before, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if before.IsCompressed() || before.VersionID != "" { + t.Fatalf("%s: invalid null-version precondition: compressed=%v versionID=%q", + instanceType, before.IsCompressed(), before.VersionID) + } + if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, + bucketVersioningConfig, enabledBucketVersioningConfig); err != nil { + t.Fatalf("%s: unable to enable versioning: %v", instanceType, err) + } + + restoreCopyCompression := setCopyChecksumCompression(false) + defer restoreCopyCompression() + rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, + map[string]string{xhttp.AmzMetadataDirective: "REPLACE"}) + if rec.Code != http.StatusOK { + t.Fatalf("%s: metadata-only CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String()) + } + + after := assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, true, nil) + if after.VersionID == "" { + t.Fatalf("%s: versioned copy did not create a new version", instanceType) + } + if got := readCopyChecksumObject(t, obj, bucketName, object, ObjectOptions{}); !bytes.Equal(got, data) { + t.Fatalf("%s: copied object body differs: got %d bytes, want %d", instanceType, len(got), len(data)) + } +} + func TestCopyRewritesObjectData(t *testing.T) { tests := []struct { name string @@ -341,6 +399,29 @@ func TestAPICopyObjectSSECKeyRotationNullVersion(t *testing.T) { func testAPICopyObjectSSECKeyRotationNullVersion(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + testAPICopyObjectSSECKeyRotationNullVersionWithCompression(obj, instanceType, bucketName, + apiRouter, credentials, false, t) +} + +func TestAPICopyObjectSSECKeyRotationNullVersionCompressesRewrite(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPICopyObjectSSECKeyRotationNullVersionCompressesRewrite, + endpoints: []string{"CopyObject", "PutObject", "GetObject"}, + }) +} + +func testAPICopyObjectSSECKeyRotationNullVersionCompressesRewrite(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + testAPICopyObjectSSECKeyRotationNullVersionWithCompression(obj, instanceType, bucketName, + apiRouter, credentials, true, t) +} + +func testAPICopyObjectSSECKeyRotationNullVersionWithCompression(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, compressAtCopy bool, t *testing.T, ) { previousTLS := globalIsTLS globalIsTLS = true @@ -375,6 +456,10 @@ func testAPICopyObjectSSECKeyRotationNullVersion(obj ObjectLayer, instanceType, if !globalBucketVersioningSys.PrefixEnabled(bucketName, object) { t.Fatalf("%s: versioning did not become enabled", instanceType) } + if compressAtCopy { + restoreCompression := setCopyChecksumCompression(true) + defer restoreCompression() + } rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{ xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, @@ -411,7 +496,7 @@ func testAPICopyObjectSSECKeyRotationNullVersion(obj ObjectLayer, instanceType, for key, value := range getHeaders { decryptHeaders.Set(key, value) } - after := assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, false, decryptHeaders) + after := assertCopyChecksum(t, obj, bucketName, object, hash.ChecksumCRC32, data, compressAtCopy, decryptHeaders) if after.VersionID == "" { t.Fatalf("%s: rotation into a versioned bucket did not create a new version", instanceType) } @@ -483,6 +568,19 @@ func testAPICopyObjectSSECKeyRotationNullVersionWrongKey(obj ObjectLayer, instan instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) } + rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(newKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]), + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(newKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]), + }) + if rec.Code != http.StatusBadRequest { + t.Fatalf("%s: rotation with equal invalid keys returned %d, want %d: %s", + instanceType, rec.Code, http.StatusBadRequest, rec.Body.String()) + } + after, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) if err != nil { t.Fatal(err) From 7c103389f5507b239eadd11ec19fe577e7d85def Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 17:23:26 +0800 Subject: [PATCH 10/18] fix: reject unsupported checksum assertions Reject unimplemented x-amz-checksum value and trailer names instead of accepting uploads without verification. Apply the same contract to PutObject, multipart initiation and parts, CopyObject, and UploadPartCopy while preserving the five supported algorithms. Signed-off-by: Feng Ruohang --- cmd/object-api-options.go | 3 + cmd/object-checksum-unsupported_test.go | 137 ++++++++++++++++++++++++ cmd/object-multipart-handlers.go | 4 + internal/hash/checksum.go | 51 +++++++-- internal/hash/checksum_test.go | 41 +++++++ 5 files changed, 227 insertions(+), 9 deletions(-) create mode 100644 cmd/object-checksum-unsupported_test.go diff --git a/cmd/object-api-options.go b/cmd/object-api-options.go index 828a8ff00..098b4ca1c 100644 --- a/cmd/object-api-options.go +++ b/cmd/object-api-options.go @@ -439,6 +439,9 @@ func putOptsFromHeaders(ctx context.Context, hdr http.Header, metadata map[strin // get ObjectOptions for Copy calls with encryption headers provided on the target side and source side metadata func copyDstOpts(ctx context.Context, r *http.Request, bucket, object string, metadata map[string]string) (opts ObjectOptions, err error) { + if _, err := hash.GetContentChecksum(r.Header); err != nil { + return opts, err + } return putOptsFromReq(ctx, r, bucket, object, metadata) } diff --git a/cmd/object-checksum-unsupported_test.go b/cmd/object-checksum-unsupported_test.go new file mode 100644 index 000000000..be11f8e9c --- /dev/null +++ b/cmd/object-checksum-unsupported_test.go @@ -0,0 +1,137 @@ +// 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/xml" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/minio/minio/internal/auth" + xhttp "github.com/minio/minio/internal/http" +) + +func TestAPIRejectsUnsupportedChecksumHeaders(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPIRejectsUnsupportedChecksumHeaders, + endpoints: []string{"CopyObject", "NewMultipart", "PutObject", "PutObjectPart"}, + }) +} + +func testAPIRejectsUnsupportedChecksumHeaders(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + data := []byte("unsupported-checksum") + unsupportedValue := base64.StdEncoding.EncodeToString(make([]byte, 64)) + + put := func(object string, headers map[string]string) *httptest.ResponseRecorder { + t.Helper() + req, err := newTestSignedRequestV4(http.MethodPut, getPutObjectURL("", bucketName, object), + int64(len(data)), bytes.NewReader(data), credentials.AccessKey, credentials.SecretKey, headers) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + return rec + } + assertRejected := func(name string, rec *httptest.ResponseRecorder) { + t.Helper() + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "InvalidArgument") { + t.Fatalf("%s: %s returned %d, want InvalidArgument: %s", instanceType, name, rec.Code, rec.Body.String()) + } + } + + for _, algorithm := range []string{"md5", "sha512", "xxhash64", "xxhash3", "xxhash128", "future"} { + object := "checksums/unsupported-" + algorithm + assertRejected(algorithm, put(object, map[string]string{ + "x-amz-sdk-checksum-algorithm": "SHA512", + "x-amz-checksum-" + algorithm: unsupportedValue, + })) + if _, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}); !isErrObjectNotFound(err) { + t.Fatalf("%s: rejected %s checksum stored an object: %v", instanceType, algorithm, err) + } + } + + assertRejected("unsupported trailer", put("checksums/unsupported-trailer", map[string]string{ + xhttp.AmzTrailer: "x-amz-checksum-sha512", + })) + + newMultipart := func(name string, headers map[string]string) *httptest.ResponseRecorder { + t.Helper() + req, err := newTestSignedRequestV4(http.MethodPost, getNewMultipartURL("", bucketName, name), + 0, nil, credentials.AccessKey, credentials.SecretKey, headers) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + return rec + } + assertRejected("NewMultipartUpload value header", newMultipart("checksums/mp-value", map[string]string{ + "x-amz-checksum-sha512": unsupportedValue, + })) + assertRejected("NewMultipartUpload trailer", newMultipart("checksums/mp-trailer", map[string]string{ + xhttp.AmzTrailer: "x-amz-checksum-sha512", + })) + + rec := newMultipart("checksums/mp-part", nil) + if rec.Code != http.StatusOK { + t.Fatalf("%s: NewMultipartUpload setup returned %d: %s", instanceType, rec.Code, rec.Body.String()) + } + var initiated InitiateMultipartUploadResponse + if err := xml.Unmarshal(rec.Body.Bytes(), &initiated); err != nil { + t.Fatal(err) + } + req, err := newTestSignedRequestV4(http.MethodPut, + getPutObjectPartURL("", bucketName, "checksums/mp-part", initiated.UploadID, "1"), + int64(len(data)), bytes.NewReader(data), credentials.AccessKey, credentials.SecretKey, + map[string]string{"x-amz-checksum-sha512": unsupportedValue}) + if err != nil { + t.Fatal(err) + } + rec = httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + assertRejected("UploadPart", rec) + parts, err := obj.ListObjectParts(t.Context(), bucketName, "checksums/mp-part", initiated.UploadID, 0, 1000, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if len(parts.Parts) != 0 { + t.Fatalf("%s: rejected UploadPart stored %d parts", instanceType, len(parts.Parts)) + } + if err := obj.AbortMultipartUpload(t.Context(), bucketName, "checksums/mp-part", initiated.UploadID, ObjectOptions{}); err != nil { + t.Fatal(err) + } + + source := "checksums/source" + putCopyChecksumSource(t, apiRouter, credentials, bucketName, source, data, nil) + rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, source, "checksums/copy", map[string]string{ + "x-amz-checksum-sha512": unsupportedValue, + }) + assertRejected("CopyObject", rec) + if _, err := obj.GetObjectInfo(t.Context(), bucketName, "checksums/copy", ObjectOptions{}); !isErrObjectNotFound(err) { + t.Fatalf("%s: rejected CopyObject stored a destination: %v", instanceType, err) + } +} diff --git a/cmd/object-multipart-handlers.go b/cmd/object-multipart-handlers.go index 2f6a68a94..37ff9f880 100644 --- a/cmd/object-multipart-handlers.go +++ b/cmd/object-multipart-handlers.go @@ -309,6 +309,10 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r } } + if _, err := hash.GetContentChecksum(r.Header); err != nil { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidChecksum), r.URL) + return + } checksumType := hash.NewChecksumHeader(r.Header) if checksumType.Is(hash.ChecksumInvalid) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidChecksum), r.URL) diff --git a/internal/hash/checksum.go b/internal/hash/checksum.go index 5131087b8..32d106d90 100644 --- a/internal/hash/checksum.go +++ b/internal/hash/checksum.go @@ -657,22 +657,55 @@ func AddChecksumHeader(w http.ResponseWriter, c map[string]string) { } } +func isSupportedChecksumHeader(name string) bool { + switch { + case strings.EqualFold(name, xhttp.AmzChecksumAlgo), + strings.EqualFold(name, xhttp.AmzChecksumType), + strings.EqualFold(name, xhttp.AmzChecksumMode): + return true + } + for _, checksumType := range BaseChecksumTypes { + if strings.EqualFold(name, checksumType.Key()) { + return true + } + } + return false +} + +func hasUnsupportedChecksumHeader(h http.Header) bool { + for name := range h { + if strings.HasPrefix(strings.ToLower(name), "x-amz-checksum-") && !isSupportedChecksumHeader(name) { + return true + } + } + return false +} + // GetContentChecksum returns content checksum. // Returns ErrInvalidChecksum if so. // Returns nil, nil if no checksum. func GetContentChecksum(h http.Header) (*Checksum, error) { + if hasUnsupportedChecksumHeader(h) { + return nil, ErrInvalidChecksum + } if trailing := h.Values(xhttp.AmzTrailer); len(trailing) > 0 { var res *Checksum - for _, header := range trailing { - var duplicates bool - for _, t := range BaseChecksumTypes { - if strings.EqualFold(t.Key(), header) { - duplicates = res != nil - res = NewChecksumWithType(t|ChecksumTrailing, "") + for _, headers := range trailing { + for header := range strings.SplitSeq(headers, ",") { + header = strings.TrimSpace(header) + var duplicates bool + for _, t := range BaseChecksumTypes { + if strings.EqualFold(t.Key(), header) { + duplicates = res != nil + res = NewChecksumWithType(t|ChecksumTrailing, "") + } + } + if strings.HasPrefix(strings.ToLower(header), "x-amz-checksum-") && !isSupportedChecksumHeader(header) { + return nil, ErrInvalidChecksum + } + if duplicates { + return nil, ErrInvalidChecksum } - } - if duplicates { - return nil, ErrInvalidChecksum } } if res != nil { diff --git a/internal/hash/checksum_test.go b/internal/hash/checksum_test.go index 504803795..9ea81c967 100644 --- a/internal/hash/checksum_test.go +++ b/internal/hash/checksum_test.go @@ -18,12 +18,53 @@ package hash import ( + "errors" + "net/http" "net/http/httptest" "testing" xhttp "github.com/minio/minio/internal/http" ) +func TestGetContentChecksumRejectsUnsupportedHeaders(t *testing.T) { + unsupported := []string{ + "x-amz-checksum-md5", + "x-amz-checksum-sha512", + "x-amz-checksum-xxhash64", + "x-amz-checksum-xxhash3", + "x-amz-checksum-xxhash128", + "x-amz-checksum-future", + } + for _, header := range unsupported { + t.Run("header/"+header, func(t *testing.T) { + h := http.Header{header: {"AA=="}} + if _, err := GetContentChecksum(h); !errors.Is(err, ErrInvalidChecksum) { + t.Fatalf("GetContentChecksum(%s) error = %v, want ErrInvalidChecksum", header, err) + } + }) + t.Run("trailer/"+header, func(t *testing.T) { + h := http.Header{xhttp.AmzTrailer: {header}} + if _, err := GetContentChecksum(h); !errors.Is(err, ErrInvalidChecksum) { + t.Fatalf("GetContentChecksum(trailer %s) error = %v, want ErrInvalidChecksum", header, err) + } + }) + } + + for header, value := range map[string]string{ + xhttp.AmzChecksumAlgo: "CRC32", + xhttp.AmzChecksumType: xhttp.AmzChecksumTypeComposite, + xhttp.AmzChecksumMode: "ENABLED", + "x-amz-sdk-checksum-algorithm": "SHA512", + } { + t.Run("control/"+header, func(t *testing.T) { + h := http.Header{header: {value}} + if _, err := GetContentChecksum(h); errors.Is(err, ErrInvalidChecksum) { + t.Fatalf("control header %s was rejected", header) + } + }) + } +} + // TestChecksumAddToHeader tests that adding and retrieving a checksum on a header works func TestChecksumAddToHeader(t *testing.T) { tests := []struct { From 474cd5801e4e242d27a68cd0ada8a477a86d42e1 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 17:26:37 +0800 Subject: [PATCH 11/18] fix: authenticate SSE-C for object attributes Unseal the supplied customer key after request preconditions before returning object size, ETag, checksum, or part attributes. Cover zero-byte and non-empty objects with correct, wrong, and missing keys.\n\nRefs #84. Signed-off-by: Feng Ruohang --- cmd/object-attributes-ssec_test.go | 99 ++++++++++++++++++++++++++++++ cmd/object-handlers.go | 6 ++ 2 files changed, 105 insertions(+) create mode 100644 cmd/object-attributes-ssec_test.go diff --git a/cmd/object-attributes-ssec_test.go b/cmd/object-attributes-ssec_test.go new file mode 100644 index 000000000..0e617a39f --- /dev/null +++ b/cmd/object-attributes-ssec_test.go @@ -0,0 +1,99 @@ +// 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" + "crypto/md5" + "encoding/base64" + "net/http" + "net/http/httptest" + "testing" + + "github.com/minio/minio/internal/auth" + xhttp "github.com/minio/minio/internal/http" +) + +func TestAPIGetObjectAttributesAuthenticatesSSECKey(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPIGetObjectAttributesAuthenticatesSSECKey, + }) +} + +func testAPIGetObjectAttributesAuthenticatesSSECKey(_ ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + previousTLS := globalIsTLS + globalIsTLS = true + defer func() { globalIsTLS = previousTLS }() + + key := bytes.Repeat([]byte{0x11}, 32) + keyMD5 := md5.Sum(key) + wrongKey := bytes.Repeat([]byte{0x22}, 32) + wrongMD5 := md5.Sum(wrongKey) + correctHeaders := map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(key), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(keyMD5[:]), + } + wrongHeaders := map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(wrongKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(wrongMD5[:]), + } + + for _, test := range []struct { + name string + data []byte + }{ + {name: "zero", data: nil}, + {name: "nonzero", data: []byte("secret")}, + } { + object := "attributes/ssec-" + test.name + putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, test.data, correctHeaders) + if rec := objectAttributesSSECRequest(t, apiRouter, credentials, bucketName, object, correctHeaders); rec.Code != http.StatusOK { + t.Fatalf("%s/%s: correct key returned %d: %s", instanceType, test.name, rec.Code, rec.Body.String()) + } + if rec := objectAttributesSSECRequest(t, apiRouter, credentials, bucketName, object, wrongHeaders); rec.Code != http.StatusForbidden { + t.Fatalf("%s/%s: wrong key returned %d, want %d: %s", instanceType, test.name, rec.Code, http.StatusForbidden, rec.Body.String()) + } + if rec := objectAttributesSSECRequest(t, apiRouter, credentials, bucketName, object, nil); rec.Code != http.StatusBadRequest { + t.Fatalf("%s/%s: missing key returned %d, want %d: %s", instanceType, test.name, rec.Code, http.StatusBadRequest, rec.Body.String()) + } + } +} + +func objectAttributesSSECRequest(t *testing.T, apiRouter http.Handler, credentials auth.Credentials, + bucket, object string, encryptionHeaders map[string]string, +) *httptest.ResponseRecorder { + t.Helper() + headers := map[string]string{xhttp.AmzObjectAttributes: "ObjectSize,ETag,ObjectParts,Checksum"} + for key, value := range encryptionHeaders { + headers[key] = value + } + req, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucket, object)+"?attributes", + 0, nil, credentials.AccessKey, credentials.SecretKey, headers) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + return rec +} diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index d5e868925..b7cd2d58d 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -619,6 +619,12 @@ func (api objectAPIHandlers) getObjectAttributesHandler(ctx context.Context, obj if checkPreconditions(ctx, w, r, objInfo, opts) { return } + if crypto.SSEC.IsEncrypted(objInfo.UserDefined) { + if _, err = crypto.SSEC.UnsealObjectKey(r.Header, objInfo.UserDefined, bucket, object); err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) + return + } + } OA := new(getObjectAttributesResponse) From b73581b05d4f739284b41ed577a3460ce63b0938 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 17:26:09 +0800 Subject: [PATCH 12/18] fix: authenticate SSE-C keys on zero-byte reads Unseal supplied SSE-C and copy-source keys after range and request preconditions when a zero-byte read cannot build a decryptor. Preserve internal no-decryption, replication, restore, and absent-header reads. Signed-off-by: Feng Ruohang --- cmd/encryption-v1.go | 18 +++ cmd/erasure-object.go | 14 +- cmd/object-ssec-zero-byte_test.go | 226 ++++++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 2 deletions(-) create mode 100644 cmd/object-ssec-zero-byte_test.go diff --git a/cmd/encryption-v1.go b/cmd/encryption-v1.go index 848f8cee1..49271803f 100644 --- a/cmd/encryption-v1.go +++ b/cmd/encryption-v1.go @@ -574,6 +574,24 @@ func DecryptCopyRequestR(client io.Reader, h http.Header, bucket, object string, return newDecryptReader(client, key, bucket, object, seqNumber, metadata) } +// checkSSECReadKey authenticates a supplied SSE-C read key against the sealed +// object key when a read has no data from which to build a decryptor. +func checkSSECReadKey(h http.Header, oi ObjectInfo, opts ObjectOptions) error { + if opts.NoDecryption || opts.Transition.RestoreRequest != nil || !crypto.SSEC.IsEncrypted(oi.UserDefined) { + return nil + } + switch { + case crypto.SSECopy.IsRequested(h): + _, err := crypto.SSECopy.UnsealObjectKey(h, oi.UserDefined, oi.Bucket, oi.Name) + return err + case crypto.SSEC.IsRequested(h): + _, err := crypto.SSEC.UnsealObjectKey(h, oi.UserDefined, oi.Bucket, oi.Name) + return err + default: + return nil + } +} + func newDecryptReader(client io.Reader, key []byte, bucket, object string, seqNumber uint32, metadata map[string]string) (io.Reader, error) { objectEncryptionKey, err := decryptObjectMeta(key, bucket, object, metadata) if err != nil { diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index 2364c45b9..79d22cc6f 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -266,9 +266,19 @@ func (er erasureObjects) GetObjectNInfo(ctx context.Context, bucket, object stri ObjInfo: objInfo, }, err } - // Zero byte objects don't even need to further initialize pipes etc. - return NewGetObjectReaderFromReader(bytes.NewReader(nil), objInfo, opts) + gr, err = NewGetObjectReaderFromReader(bytes.NewReader(nil), objInfo, opts) + if err != nil { + return gr, err + } + // With no data, the reader above cannot authenticate an SSE-C key the + // way NewGetObjectReader does. Check it after the preconditions so zero + // and non-zero reads preserve the same error ordering. + if err := checkSSECReadKey(h, objInfo, opts); err != nil { + gr.Close() + return nil, err + } + return gr, nil } if objInfo.IsRemote() { diff --git a/cmd/object-ssec-zero-byte_test.go b/cmd/object-ssec-zero-byte_test.go new file mode 100644 index 000000000..787c11893 --- /dev/null +++ b/cmd/object-ssec-zero-byte_test.go @@ -0,0 +1,226 @@ +// 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" + "crypto/md5" + "encoding/base64" + "encoding/xml" + "net/http" + "net/http/httptest" + "testing" + + "github.com/minio/minio/internal/auth" + xhttp "github.com/minio/minio/internal/http" +) + +func TestAPIZeroByteSSECAuthenticatesKey(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPIZeroByteSSECAuthenticatesKey, + endpoints: []string{"CopyObject", "CopyObjectPart", "PutObject", "GetObject", "HeadObject", "NewMultipart"}, + }) +} + +func testAPIZeroByteSSECAuthenticatesKey(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + previousTLS := globalIsTLS + globalIsTLS = true + defer func() { globalIsTLS = previousTLS }() + + object := "ssec/zero-byte" + oldKey := bytes.Repeat([]byte{0x11}, 32) + oldMD5 := md5.Sum(oldKey) + wrongKey := bytes.Repeat([]byte{0x22}, 32) + wrongMD5 := md5.Sum(wrongKey) + putCopyChecksumSource(t, apiRouter, credentials, bucketName, object, nil, map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), + }) + + correctHeaders := map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(oldKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(oldMD5[:]), + } + wrongHeaders := map[string]string{ + xhttp.AmzServerSideEncryptionCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCustomerKey: base64.StdEncoding.EncodeToString(wrongKey), + xhttp.AmzServerSideEncryptionCustomerKeyMD5: base64.StdEncoding.EncodeToString(wrongMD5[:]), + } + + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, object, correctHeaders); rec.Code != http.StatusOK || rec.Body.Len() != 0 { + t.Fatalf("%s: correct-key GET returned %d with %d bytes: %s", instanceType, rec.Code, rec.Body.Len(), rec.Body.String()) + } + headRec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodHead, bucketName, object, correctHeaders) + if headRec.Code != http.StatusOK { + t.Fatalf("%s: correct-key HEAD returned %d", instanceType, headRec.Code) + } + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, object, wrongHeaders); rec.Code != http.StatusForbidden { + t.Fatalf("%s: wrong-key GET returned %d, want %d: %s", instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) + } + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodHead, bucketName, object, wrongHeaders); rec.Code != http.StatusForbidden { + t.Fatalf("%s: wrong-key HEAD returned %d, want %d", instanceType, rec.Code, http.StatusForbidden) + } + conditionalHeaders := make(map[string]string, len(wrongHeaders)+1) + for key, value := range wrongHeaders { + conditionalHeaders[key] = value + } + conditionalInfo, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + conditionalRequest := httptest.NewRequest(http.MethodGet, getGetObjectURL("", bucketName, object), nil) + for key, value := range wrongHeaders { + conditionalRequest.Header.Set(key, value) + } + if _, err := DecryptObjectInfo(&conditionalInfo, conditionalRequest); err != nil { + t.Fatal(err) + } + conditionalHeaders[xhttp.IfNoneMatch] = conditionalInfo.ETag + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, object, conditionalHeaders); rec.Code != http.StatusNotModified { + t.Fatalf("%s: conditional wrong-key GET returned %d, want %d: %s", instanceType, rec.Code, http.StatusNotModified, rec.Body.String()) + } + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, object, nil); rec.Code != http.StatusBadRequest { + t.Fatalf("%s: missing-key GET returned %d, want %d: %s", instanceType, rec.Code, http.StatusBadRequest, rec.Body.String()) + } + + nonEmptyObject := "ssec/one-byte" + putCopyChecksumSource(t, apiRouter, credentials, bucketName, nonEmptyObject, []byte{1}, correctHeaders) + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, nonEmptyObject, wrongHeaders); rec.Code != http.StatusForbidden { + t.Fatalf("%s: one-byte wrong-key GET returned %d, want %d: %s", instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) + } + + plainObject := "ssec/plain-zero-byte" + putCopyChecksumSource(t, apiRouter, credentials, bucketName, plainObject, nil, nil) + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, plainObject, wrongHeaders); rec.Code != http.StatusBadRequest { + t.Fatalf("%s: unencrypted wrong-key GET returned %d, want %d: %s", instanceType, rec.Code, http.StatusBadRequest, rec.Body.String()) + } + + destination := "ssec/zero-byte-copy" + rec := copyChecksumRequest(t, apiRouter, credentials, bucketName, object, destination, map[string]string{ + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(wrongKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(wrongMD5[:]), + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("%s: wrong-key CopyObject returned %d, want %d: %s", instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) + } + if _, err := obj.GetObjectInfo(t.Context(), bucketName, destination, ObjectOptions{}); !isErrObjectNotFound(err) { + t.Fatalf("%s: rejected CopyObject created the destination: %v", instanceType, err) + } + + rec = copyChecksumRequest(t, apiRouter, credentials, bucketName, object, object, map[string]string{ + xhttp.AmzStorageClass: "REDUCED_REDUNDANCY", + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(wrongKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(wrongMD5[:]), + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("%s: wrong-key storage-class CopyObject returned %d, want %d: %s", instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) + } + + multipartObject := "ssec/zero-byte-multipart-copy" + req, err := newTestSignedRequestV4(http.MethodPost, getNewMultipartURL("", bucketName, multipartObject), + 0, nil, credentials.AccessKey, credentials.SecretKey, nil) + if err != nil { + t.Fatal(err) + } + rec = httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("%s: NewMultipartUpload returned %d: %s", instanceType, rec.Code, rec.Body.String()) + } + var initiated InitiateMultipartUploadResponse + if err := xml.Unmarshal(rec.Body.Bytes(), &initiated); err != nil { + t.Fatal(err) + } + req, err = newTestSignedRequestV4(http.MethodPut, + getCopyObjectPartURL("", bucketName, multipartObject, initiated.UploadID, "1"), + 0, nil, credentials.AccessKey, credentials.SecretKey, map[string]string{ + xhttp.AmzServerSideEncryptionCopyCustomerAlgorithm: xhttp.AmzEncryptionAES, + xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(wrongKey), + xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(wrongMD5[:]), + }) + if err != nil { + t.Fatal(err) + } + req.Header.Set(xhttp.AmzCopySource, SlashSeparator+pathJoin(bucketName, object)) + rec = httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Fatalf("%s: wrong-key UploadPartCopy returned %d, want %d: %s", instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) + } + parts, err := obj.ListObjectParts(t.Context(), bucketName, multipartObject, initiated.UploadID, 0, 1000, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if len(parts.Parts) != 0 { + t.Fatalf("%s: rejected UploadPartCopy stored %d parts", instanceType, len(parts.Parts)) + } + if err := obj.AbortMultipartUpload(t.Context(), bucketName, multipartObject, initiated.UploadID, ObjectOptions{}); err != nil { + t.Fatal(err) + } + + wrongHeader := http.Header{} + for key, value := range wrongHeaders { + wrongHeader.Set(key, value) + } + for _, test := range []struct { + header http.Header + opts ObjectOptions + }{ + {header: nil, opts: ObjectOptions{}}, + {header: wrongHeader, opts: ObjectOptions{NoDecryption: true}}, + {header: wrongHeader, opts: ObjectOptions{ReplicationRequest: true}}, + {header: wrongHeader, opts: ObjectOptions{Transition: TransitionOptions{RestoreRequest: &RestoreObjectRequest{}}}}, + } { + gr, err := obj.GetObjectNInfo(t.Context(), bucketName, object, nil, test.header, test.opts) + if err != nil { + t.Fatalf("%s: internal zero-byte read with opts %+v failed: %v", instanceType, test.opts, err) + } + gr.Close() + } + + rangeHeaders := make(map[string]string, len(wrongHeaders)+1) + for key, value := range wrongHeaders { + rangeHeaders[key] = value + } + rangeHeaders[xhttp.Range] = "bytes=0-0" + if rec := ssecZeroByteRequest(t, apiRouter, credentials, http.MethodGet, bucketName, object, rangeHeaders); rec.Code != http.StatusRequestedRangeNotSatisfiable { + t.Fatalf("%s: ranged wrong-key GET returned %d, want %d: %s", instanceType, rec.Code, http.StatusRequestedRangeNotSatisfiable, rec.Body.String()) + } +} + +func ssecZeroByteRequest(t *testing.T, apiRouter http.Handler, credentials auth.Credentials, + method, bucket, object string, headers map[string]string, +) *httptest.ResponseRecorder { + t.Helper() + req, err := newTestSignedRequestV4(method, getGetObjectURL("", bucket, object), + 0, nil, credentials.AccessKey, credentials.SecretKey, headers) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + return rec +} From c4fd97d0bfface966b04a24e449bba1eb85ad6d0 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 17:28:36 +0800 Subject: [PATCH 13/18] test: align zero-byte rotation key errors After zero-byte reads authenticate SSE-C at the object layer, equal invalid rotation keys match non-empty reads and return AccessDenied before the rotation-specific distinction. Signed-off-by: Feng Ruohang --- cmd/object-copy-metadata_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index 95d4f7d3d..53cb3ff3f 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -576,9 +576,11 @@ func testAPICopyObjectSSECKeyRotationNullVersionWrongKey(obj ObjectLayer, instan xhttp.AmzServerSideEncryptionCopyCustomerKey: base64.StdEncoding.EncodeToString(newKey), xhttp.AmzServerSideEncryptionCopyCustomerKeyMD5: base64.StdEncoding.EncodeToString(newMD5[:]), }) - if rec.Code != http.StatusBadRequest { + // The zero-byte read path authenticates the source key before the + // rotation-specific equal-key distinction, matching non-empty reads. + if rec.Code != http.StatusForbidden { t.Fatalf("%s: rotation with equal invalid keys returned %d, want %d: %s", - instanceType, rec.Code, http.StatusBadRequest, rec.Body.String()) + instanceType, rec.Code, http.StatusForbidden, rec.Body.String()) } after, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}) From d28885d0e51b71d7fbb76af7977917db067158dc Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 17:23:41 +0800 Subject: [PATCH 14/18] fix: reject composite CRC64NVME checksums Return InvalidArgument for CRC64NVME with COMPOSITE at multipart initiation and PutObject instead of silently canonicalizing the request to FULL_OBJECT. Signed-off-by: Feng Ruohang --- cmd/erasure-multipart-fullobject_test.go | 20 ++++---- cmd/object-crc64-composite_test.go | 61 ++++++++++++++++++++++++ internal/hash/checksum.go | 8 +++- internal/hash/checksum_test.go | 3 ++ 4 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 cmd/object-crc64-composite_test.go diff --git a/cmd/erasure-multipart-fullobject_test.go b/cmd/erasure-multipart-fullobject_test.go index 0bcb14c4f..4b7021522 100644 --- a/cmd/erasure-multipart-fullobject_test.go +++ b/cmd/erasure-multipart-fullobject_test.go @@ -450,19 +450,21 @@ func testAPICompleteMultipartChecksumTypeMismatch(obj ObjectLayer, instanceType, } }) - t.Run("crc64nvme-composite-remains-canonicalized", func(t *testing.T) { + t.Run("crc64nvme-composite-is-rejected", func(t *testing.T) { crc64Type := hash.ChecksumCRC64NVME objectName := "type-mismatch/crc64nvme-composite" - uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, - crc64Type.String(), xhttp.AmzChecksumTypeComposite) - etags := uploadPartsHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, crc64Type, partData) - rec := completeMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, uploadID, etags, nil, - map[string]string{ - crc64Type.Key(): mustChecksum(t, crc64Type, full), + req, err := newTestSignedRequestV4(http.MethodPost, getNewMultipartURL("", bucketName, objectName), + 0, nil, credentials.AccessKey, credentials.SecretKey, map[string]string{ + xhttp.AmzChecksumAlgo: crc64Type.String(), xhttp.AmzChecksumType: xhttp.AmzChecksumTypeComposite, }) - if rec.Code != http.StatusOK { - t.Fatalf("%s: CRC64NVME canonicalization changed: %d %s", instanceType, rec.Code, rec.Body.String()) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest || apiErrorCode(t, rec) != "InvalidArgument" { + t.Fatalf("%s: CRC64NVME/COMPOSITE returned %d %s", instanceType, rec.Code, rec.Body.String()) } }) } diff --git a/cmd/object-crc64-composite_test.go b/cmd/object-crc64-composite_test.go new file mode 100644 index 000000000..6db038062 --- /dev/null +++ b/cmd/object-crc64-composite_test.go @@ -0,0 +1,61 @@ +// 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" + "net/http" + "net/http/httptest" + "testing" + + "github.com/minio/minio/internal/auth" + "github.com/minio/minio/internal/hash" + xhttp "github.com/minio/minio/internal/http" +) + +func TestAPIPutObjectRejectsCRC64Composite(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: testAPIPutObjectRejectsCRC64Composite, + endpoints: []string{"PutObject"}, + }) +} + +func testAPIPutObjectRejectsCRC64Composite(obj ObjectLayer, instanceType, bucketName string, + apiRouter http.Handler, credentials auth.Credentials, t *testing.T, +) { + data := []byte("crc64-composite") + object := "checksums/crc64-composite" + req, err := newTestSignedRequestV4(http.MethodPut, getPutObjectURL("", bucketName, object), + int64(len(data)), bytes.NewReader(data), credentials.AccessKey, credentials.SecretKey, map[string]string{ + xhttp.AmzChecksumCRC64NVME: mustChecksum(t, hash.ChecksumCRC64NVME, data), + xhttp.AmzChecksumType: xhttp.AmzChecksumTypeComposite, + }) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest || apiErrorCode(t, rec) != "InvalidArgument" { + t.Fatalf("%s: CRC64NVME/COMPOSITE PutObject returned %d %s", instanceType, rec.Code, rec.Body.String()) + } + if _, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}); !isErrObjectNotFound(err) { + t.Fatalf("%s: rejected PutObject stored an object: %v", instanceType, err) + } +} diff --git a/internal/hash/checksum.go b/internal/hash/checksum.go index 32d106d90..5da17e1b6 100644 --- a/internal/hash/checksum.go +++ b/internal/hash/checksum.go @@ -157,7 +157,6 @@ func ChecksumStringToType(alg string) ChecksumType { case "SHA256": return ChecksumSHA256 case "CRC64NVME": - // AWS seems to ignore full value, and just assume it. return ChecksumCRC64NVME case "": return ChecksumNone @@ -192,7 +191,9 @@ func NewChecksumType(alg, objType string) ChecksumType { } return ChecksumSHA256 case "CRC64NVME": - // AWS seems to ignore full value, and just assume it. + if objType == xhttp.AmzChecksumTypeComposite { + return ChecksumInvalid + } return ChecksumCRC64NVME case "": if full != 0 { @@ -781,5 +782,8 @@ func getContentChecksum(h http.Header) (t ChecksumType, s string) { for _, t := range BaseChecksumTypes { checkType(t) } + if t.Base().Is(ChecksumCRC64NVME) && h.Get(xhttp.AmzChecksumType) == xhttp.AmzChecksumTypeComposite { + return ChecksumInvalid, "" + } return t, s } diff --git a/internal/hash/checksum_test.go b/internal/hash/checksum_test.go index 9ea81c967..74631a329 100644 --- a/internal/hash/checksum_test.go +++ b/internal/hash/checksum_test.go @@ -67,6 +67,9 @@ func TestGetContentChecksumRejectsUnsupportedHeaders(t *testing.T) { // TestChecksumAddToHeader tests that adding and retrieving a checksum on a header works func TestChecksumAddToHeader(t *testing.T) { + if got := NewChecksumType("CRC64NVME", xhttp.AmzChecksumTypeComposite); !got.Is(ChecksumInvalid) { + t.Fatalf("CRC64NVME/COMPOSITE = %s, want invalid", got.StringFull()) + } tests := []struct { name string checksum ChecksumType From d4c8da162bce007ad30135b47c886923bb82f320 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 18:15:51 +0800 Subject: [PATCH 15/18] fix: reject composite CRC64NVME trailers Apply the full-object-only rule to declared streaming checksum trailers and cover the HTTP mutation path. Signed-off-by: Feng Ruohang --- cmd/object-crc64-composite_test.go | 18 ++++++++++++++++++ internal/hash/checksum.go | 6 +++++- internal/hash/checksum_test.go | 10 ++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cmd/object-crc64-composite_test.go b/cmd/object-crc64-composite_test.go index 6db038062..70a80370d 100644 --- a/cmd/object-crc64-composite_test.go +++ b/cmd/object-crc64-composite_test.go @@ -58,4 +58,22 @@ func testAPIPutObjectRejectsCRC64Composite(obj ObjectLayer, instanceType, bucket if _, err := obj.GetObjectInfo(t.Context(), bucketName, object, ObjectOptions{}); !isErrObjectNotFound(err) { t.Fatalf("%s: rejected PutObject stored an object: %v", instanceType, err) } + + trailerObject := "checksums/crc64-composite-trailer" + req, err = newTestSignedRequestV4(http.MethodPut, getPutObjectURL("", bucketName, trailerObject), + int64(len(data)), bytes.NewReader(data), credentials.AccessKey, credentials.SecretKey, map[string]string{ + xhttp.AmzTrailer: xhttp.AmzChecksumCRC64NVME, + xhttp.AmzChecksumType: xhttp.AmzChecksumTypeComposite, + }) + if err != nil { + t.Fatal(err) + } + rec = httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest || apiErrorCode(t, rec) != "InvalidArgument" { + t.Fatalf("%s: trailing CRC64NVME/COMPOSITE PutObject returned %d %s", instanceType, rec.Code, rec.Body.String()) + } + if _, err := obj.GetObjectInfo(t.Context(), bucketName, trailerObject, ObjectOptions{}); !isErrObjectNotFound(err) { + t.Fatalf("%s: rejected trailing PutObject stored an object: %v", instanceType, err) + } } diff --git a/internal/hash/checksum.go b/internal/hash/checksum.go index 5da17e1b6..e4ced2fba 100644 --- a/internal/hash/checksum.go +++ b/internal/hash/checksum.go @@ -716,7 +716,11 @@ func GetContentChecksum(h http.Header) (*Checksum, error) { return nil, ErrInvalidChecksum } res.Type |= ChecksumFullObject - case xhttp.AmzChecksumTypeComposite, "": + case xhttp.AmzChecksumTypeComposite: + if res.Type.Base().Is(ChecksumCRC64NVME) { + return nil, ErrInvalidChecksum + } + case "": default: return nil, ErrInvalidChecksum } diff --git a/internal/hash/checksum_test.go b/internal/hash/checksum_test.go index 74631a329..595302818 100644 --- a/internal/hash/checksum_test.go +++ b/internal/hash/checksum_test.go @@ -150,6 +150,16 @@ func TestChecksumAddToHeader(t *testing.T) { } } +func TestCRC64NVMECompositeTrailerIsInvalid(t *testing.T) { + h := http.Header{} + h.Set(xhttp.AmzTrailer, ChecksumCRC64NVME.Key()) + h.Set(xhttp.AmzChecksumType, xhttp.AmzChecksumTypeComposite) + _, err := GetContentChecksum(h) + if !errors.Is(err, ErrInvalidChecksum) { + t.Fatalf("CRC64NVME/COMPOSITE trailer error = %v, want ErrInvalidChecksum", err) + } +} + // TestChecksumSerializeDeserialize checks AppendTo can be reversed by ChecksumFromBytes func TestChecksumSerializeDeserialize(t *testing.T) { myData := []byte("this-is-a-checksum-data-test") From 21870fa2e7f345ae92ed05ac834b1bc55bee0ff8 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 18:18:01 +0800 Subject: [PATCH 16/18] fix: preserve replicated object attributes reads Keep the existing trusted replication carve-out while authenticating ordinary SSE-C GetObjectAttributes requests. Signed-off-by: Feng Ruohang --- cmd/object-attributes-ssec_test.go | 5 +++++ cmd/object-handlers.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/object-attributes-ssec_test.go b/cmd/object-attributes-ssec_test.go index 0e617a39f..77deca059 100644 --- a/cmd/object-attributes-ssec_test.go +++ b/cmd/object-attributes-ssec_test.go @@ -77,6 +77,11 @@ func testAPIGetObjectAttributesAuthenticatesSSECKey(_ ObjectLayer, instanceType, if rec := objectAttributesSSECRequest(t, apiRouter, credentials, bucketName, object, nil); rec.Code != http.StatusBadRequest { t.Fatalf("%s/%s: missing key returned %d, want %d: %s", instanceType, test.name, rec.Code, http.StatusBadRequest, rec.Body.String()) } + if rec := objectAttributesSSECRequest(t, apiRouter, credentials, bucketName, object, map[string]string{ + xhttp.MinIOSourceReplicationRequest: "true", + }); rec.Code != http.StatusOK { + t.Fatalf("%s/%s: replication request returned %d: %s", instanceType, test.name, rec.Code, rec.Body.String()) + } } } diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index b7cd2d58d..bf073f0c5 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -619,7 +619,7 @@ func (api objectAPIHandlers) getObjectAttributesHandler(ctx context.Context, obj if checkPreconditions(ctx, w, r, objInfo, opts) { return } - if crypto.SSEC.IsEncrypted(objInfo.UserDefined) { + if crypto.SSEC.IsEncrypted(objInfo.UserDefined) && r.Header.Get(xhttp.MinIOSourceReplicationRequest) != "true" { if _, err = crypto.SSEC.UnsealObjectKey(r.Header, objInfo.UserDefined, bucket, object); err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return From 8448512a1f551d3dc64337e4260953e93d93a170 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 18:21:13 +0800 Subject: [PATCH 17/18] docs: clarify layered SSE-C key authentication Signed-off-by: Feng Ruohang --- cmd/encryption-v1.go | 6 +++--- cmd/object-copy-metadata_test.go | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cmd/encryption-v1.go b/cmd/encryption-v1.go index 49271803f..36857851c 100644 --- a/cmd/encryption-v1.go +++ b/cmd/encryption-v1.go @@ -356,9 +356,9 @@ func rotateKey(ctx context.Context, oldKey []byte, newKeyID string, newKey []byt } // checkSSECCopySourceKey authenticates the SSE-C copy source key against the -// sealed object key held in metadata. GetObjectNInfo builds no decryptor for a -// zero byte object, so a copy whose data path never decrypts anything has to -// verify the source key explicitly. Mirrors the errors rotateKey reports. +// sealed object key held in metadata. This keeps the diverted rotation safe on +// its own and remains defense in depth when the read path also authenticates +// zero-byte objects. Mirrors the errors rotateKey reports. func checkSSECCopySourceKey(h http.Header, metadata map[string]string, bucket, object string, newKey []byte) error { oldKey, err := ParseSSECopyCustomerRequest(h, metadata) if err != nil { diff --git a/cmd/object-copy-metadata_test.go b/cmd/object-copy-metadata_test.go index 53cb3ff3f..4c123676e 100644 --- a/cmd/object-copy-metadata_test.go +++ b/cmd/object-copy-metadata_test.go @@ -508,10 +508,9 @@ func testAPICopyObjectSSECKeyRotationNullVersionWithCompression(obj ObjectLayer, } } -// TestAPICopyObjectSSECKeyRotationNullVersionWrongKey pins the source key -// authentication of the re-encrypting fallback. A zero byte source has no data -// to decrypt, so the copy would otherwise reach the destination write without -// ever proving the caller holds the current key. +// TestAPICopyObjectSSECKeyRotationNullVersionWrongKey pins source-key +// authentication in both the standalone rotation fix and the later zero-byte +// read hardening. func TestAPICopyObjectSSECKeyRotationNullVersionWrongKey(t *testing.T) { defer DetectTestLeak(t)() ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ From 32b2aa49f160b4a5e706b7d7d7ad12c5b4f49091 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 29 Aug 2026 18:36:02 +0800 Subject: [PATCH 18/18] fix: reject composite CRC64NVME completion Remove the remaining type-only canonicalization at CompleteMultipartUpload while preserving legacy uploads stored as FULL_OBJECT. Signed-off-by: Feng Ruohang --- cmd/erasure-multipart-fullobject_test.go | 9 ++++++--- cmd/erasure-multipart.go | 8 +------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/cmd/erasure-multipart-fullobject_test.go b/cmd/erasure-multipart-fullobject_test.go index b9d71aa21..c8f3dde11 100644 --- a/cmd/erasure-multipart-fullobject_test.go +++ b/cmd/erasure-multipart-fullobject_test.go @@ -516,7 +516,7 @@ func testAPICompleteMultipartChecksumTypeMismatch(obj ObjectLayer, instanceType, } }) - t.Run("crc64nvme-composite-remains-canonicalized", func(t *testing.T) { + t.Run("crc64nvme-composite-completion-is-rejected", func(t *testing.T) { crc64Type := hash.ChecksumCRC64NVME objectName := "type-mismatch/crc64nvme-composite" uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, objectName, @@ -527,8 +527,11 @@ func testAPICompleteMultipartChecksumTypeMismatch(obj ObjectLayer, instanceType, crc64Type.Key(): mustChecksum(t, crc64Type, full), xhttp.AmzChecksumType: xhttp.AmzChecksumTypeComposite, }) - if rec.Code != http.StatusOK { - t.Fatalf("%s: CRC64NVME canonicalization changed: %d %s", instanceType, rec.Code, rec.Body.String()) + if rec.Code != http.StatusBadRequest || apiErrorCode(t, rec) != "BadDigest" { + t.Fatalf("%s: CRC64NVME composite completion returned %d %s", instanceType, rec.Code, rec.Body.String()) + } + if _, err := obj.GetObjectInfo(t.Context(), bucketName, objectName, ObjectOptions{}); err == nil { + t.Fatalf("%s: object was created despite a rejected CRC64NVME checksum type", instanceType) } }) } diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go index d99492368..be5afb7c6 100644 --- a/cmd/erasure-multipart.go +++ b/cmd/erasure-multipart.go @@ -1185,13 +1185,7 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str } } if opts.wantChecksumType != "" { - providedObjectType := opts.wantChecksumType - // CRC64NVME is always canonicalized to FULL_OBJECT. Preserve this - // behavior until its exact AWS wire semantics have been probed. - if checksumType.Base().Is(hash.ChecksumCRC64NVME) { - providedObjectType = xhttp.AmzChecksumTypeFullObject - } - if providedObjectType != expectedType.ObjType() { + if opts.wantChecksumType != expectedType.ObjType() { return oi, completeMultipartChecksumTypeMismatch(opts.wantChecksumType, expectedType.ObjType()) } }