From 4a9bdb54072dbd7e95dd303a190cda7237ce4fcd Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 11 Nov 2026 14:44:50 +0800 Subject: [PATCH] fix(batch): reject invalid UTF-8 before KMS context rotation Reject contexts whose JSON serialization could change associated data or collapse distinct keys. Validate both submissions and persisted jobs before changing an object key; retain valid Unicode, including replacement characters. Exercise malformed-key collisions, invalid values, manifest reload failures and readback with the real builtin KMS. Refs #346. Signed-off-by: Feng Ruohang --- cmd/batch-rotate-context_test.go | 74 +++++++++++++++++++++++++++++++- cmd/batch-rotate.go | 6 +++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/cmd/batch-rotate-context_test.go b/cmd/batch-rotate-context_test.go index c60b5bca7..5ad91105b 100644 --- a/cmd/batch-rotate-context_test.go +++ b/cmd/batch-rotate-context_test.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "maps" + "net/http" "strings" "testing" @@ -56,7 +57,7 @@ func TestBatchRotationContextInitialRetryAndReload(t *testing.T) { mustIAM(t, err) original, err := f.obj.PutObject(f.ctx, f.bucket, object, mustGetPutObjReader(t, bytes.NewReader(ciphertext), int64(len(ciphertext)), "", ""), ObjectOptions{Versioned: true, UserDefined: metadata}) mustIAM(t, err) - contextJSON := `{"tenant":"acme","purpose":"batch-rotation"}` + contextJSON := `{"tenant":"acme","purpose":"batch-rotation","区域":"杭州","�":"replacement","emoji":"🔑"}` encoded := base64.StdEncoding.EncodeToString([]byte(contextJSON)) rec, job := f.submit(t, globalActiveCred, fmt.Sprintf("keyrotate:\n apiVersion: v1\n bucket: %s\n prefix: rotation/\n encryption:\n type: sse-kms\n key: context-key\n context: %s\n", f.bucket, encoded)) if job == nil { @@ -91,7 +92,7 @@ func TestBatchRotationContextInitialRetryAndReload(t *testing.T) { mustIAM(t, err) var got kms.Context mustIAM(t, json.Unmarshal(raw, &got)) - if !maps.Equal(got, kms.Context{"tenant": "acme", "purpose": "batch-rotation"}) { + if !maps.Equal(got, kms.Context{"tenant": "acme", "purpose": "batch-rotation", "区域": "杭州", "�": "replacement", "emoji": "🔑"}) { t.Fatalf("stored context %v", got) } actual, err := crypto.S3KMS.UnsealObjectKey(configured, after.UserDefined, f.bucket, object) @@ -105,3 +106,72 @@ func TestBatchRotationContextInitialRetryAndReload(t *testing.T) { }) } } + +func TestBatchRotationInvalidUTF8LeavesObjectReadable(t *testing.T) { + for _, tc := range []struct{ name, raw string }{ + {"colliding-keys", "{\"\xff\":\"a\",\"\xfe\":\"b\"}"}, + {"replacement-key", "{\"\xff\":\"a\",\"�\":\"b\"}"}, + {"invalid-value", "{\"tenant\":\"\xff\"}"}, + } { + for _, phase := range []string{"submit", "reload"} { + t.Run(tc.name+"/"+phase, func(t *testing.T) { + f := newBatchAuthzFixture(t) + previous := GlobalKMS + t.Cleanup(func() { GlobalKMS = previous }) + configured, err := kms.NewBuiltin("context-key", bytes.Repeat([]byte{0x42}, 32)) + mustIAM(t, err) + GlobalKMS = configured + const object, payload = "rotation/context", "the original encrypted payload must remain readable" + metadata := map[string]string{} + key, err := newEncryptMetadata(f.ctx, crypto.S3KMS, "context-key", nil, f.bucket, object, metadata, kms.Context{"tenant": "old"}) + mustIAM(t, err) + ciphertext, err := io.ReadAll(crypto.EncryptSinglePart(strings.NewReader(payload), key)) + mustIAM(t, err) + original, err := f.obj.PutObject(f.ctx, f.bucket, object, mustGetPutObjReader(t, bytes.NewReader(ciphertext), int64(len(ciphertext)), "", ""), ObjectOptions{Versioned: true, UserDefined: metadata}) + mustIAM(t, err) + encoded := base64.StdEncoding.EncodeToString([]byte(tc.raw)) + submitContext := encoded + if phase == "reload" { + submitContext = base64.StdEncoding.EncodeToString([]byte(`{"tenant":"safe"}`)) + } + rec, job := f.submit(t, globalActiveCred, fmt.Sprintf("keyrotate:\n apiVersion: v1\n bucket: %s\n prefix: rotation/\n encryption:\n type: sse-kms\n key: context-key\n context: %s\n", f.bucket, submitContext)) + if phase == "submit" { + if job != nil || rec.Code != http.StatusBadRequest { + t.Fatalf("invalid UTF-8 context accepted: %d %s", rec.Code, rec.Body.String()) + } + } else { + if job == nil { + t.Fatalf("valid submit: %d %s", rec.Code, rec.Body.String()) + } + // Simulate a manifest saved by a version which accepted these + // bytes. Execution must validate again before changing the key. + job.KeyRotate.Encryption.Context = encoded + job.KeyRotate.Flags.Retry.Attempts = 1 + manifest, err := job.MarshalMsg([]byte(batchJobEnvelope)) + mustIAM(t, err) + mustIAM(t, saveConfig(f.ctx, f.obj, getJobPath(*job), manifest)) + loaded := &BatchJobRequest{} + mustIAM(t, loaded.load(f.ctx, f.obj, getJobPath(*job))) + mustIAM(t, loaded.KeyRotate.Start(f.ctx, f.obj, *loaded)) + progress := f.jobInfo(t, loaded) + if progress.Complete || !progress.Failed || progress.Objects != 0 || progress.ObjectsFailed != 1 { + t.Fatalf("invalid context reported as success: %+v", progress) + } + } + after, err := f.obj.GetObjectInfo(f.ctx, f.bucket, object, ObjectOptions{VersionID: original.VersionID}) + mustIAM(t, err) + if after.UserDefined[crypto.MetaContext] != metadata[crypto.MetaContext] { + t.Fatal("rejected rotation changed stored context") + } + actual, err := crypto.S3KMS.UnsealObjectKey(configured, after.UserDefined, f.bucket, object) + mustIAM(t, err) + if actual != key { + t.Fatal("rejected rotation changed data key") + } + if got := readCopyChecksumObject(t, f.obj, f.bucket, object, ObjectOptions{VersionID: original.VersionID}); string(got) != payload { + t.Fatal("rejected rotation damaged stored content") + } + }) + } + } +} diff --git a/cmd/batch-rotate.go b/cmd/batch-rotate.go index 4d85990b4..d3d9c416d 100644 --- a/cmd/batch-rotate.go +++ b/cmd/batch-rotate.go @@ -29,6 +29,7 @@ import ( "strconv" "strings" "time" + "unicode/utf8" jsoniter "github.com/json-iterator/go" "github.com/minio/minio-go/v7/pkg/tags" @@ -99,6 +100,11 @@ func batchKeyRotateContext(encoded string) (kms.Context, error) { if err != nil { return nil, err } + // Saving the context replaces invalid UTF-8 and can collapse distinct + // keys, changing the associated data required to decrypt the object. + if !utf8.Valid(b) { + return nil, fmt.Errorf("KMS context must be valid UTF-8: %w", errInvalidArgument) + } if err = jsoniter.ConfigCompatibleWithStandardLibrary.Unmarshal(b, &ctx); err != nil { return nil, err }