fix(crypto): return client errors for malformed KMS contexts

Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-11-12 07:49:47 +08:00
parent c39605d4f8
commit 2ed720a7fb
3 changed files with 40 additions and 3 deletions
+14
View File
@@ -84,6 +84,11 @@ func TestAPISSEKMSHTTPContext(t *testing.T) {
invalid bool invalid bool
}{ }{
{"unicode", `{"区域":"杭州","emoji":"🔑","�":"replacement"}`, false}, {"unicode", `{"区域":"杭州","emoji":"🔑","�":"replacement"}`, false},
{"array", `[]`, true},
{"invalid-json", `not-json`, true},
{"trailing-json", `{"tenant":"ok"} {}`, true},
{"number-value", `{"tenant":1}`, true},
{"invalid-base64", "", true},
{"invalid-key", "{\"\xff\":\"value\"}", true}, {"invalid-key", "{\"\xff\":\"value\"}", true},
{"invalid-value", "{\"tenant\":\"\xff\"}", true}, {"invalid-value", "{\"tenant\":\"\xff\"}", true},
{"colliding-keys", "{\"\xff\":\"a\",\"\xfe\":\"b\"}", true}, {"colliding-keys", "{\"\xff\":\"a\",\"\xfe\":\"b\"}", true},
@@ -98,6 +103,9 @@ func TestAPISSEKMSHTTPContext(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
headers := encryption(tc.raw) headers := encryption(tc.raw)
if tc.name == "invalid-base64" {
headers[xhttp.AmzServerSideEncryptionKmsContext] = "not-base64!"
}
wantData := replacement wantData := replacement
var rec *httptest.ResponseRecorder var rec *httptest.ResponseRecorder
switch operation { switch operation {
@@ -155,6 +163,12 @@ func TestAPISSEKMSHTTPContext(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
if tc.invalid { if tc.invalid {
if operation == "multipart" {
uploads, err := obj.ListMultipartUploads(t.Context(), bucket, key, "", "", "", 100)
if err != nil || len(uploads.Uploads) != 0 {
t.Fatalf("rejected context created multipart upload: %+v, %v", uploads, err)
}
}
if !maps.Equal(before.UserDefined, after.UserDefined) || !before.ModTime.Equal(after.ModTime) { if !maps.Equal(before.UserDefined, after.UserDefined) || !before.ModTime.Equal(after.ModTime) {
t.Fatal("rejected context changed existing object metadata") t.Fatal("rejected context changed existing object metadata")
} }
+10 -2
View File
@@ -76,9 +76,17 @@ func (ssekms) ParseHTTP(h http.Header) (string, kms.Context, error) {
var ctx kms.Context var ctx kms.Context
if context, ok := h[xhttp.AmzServerSideEncryptionKmsContext]; ok { if context, ok := h[xhttp.AmzServerSideEncryptionKmsContext]; ok {
invalidContext := kms.Error{
Code: http.StatusBadRequest,
APICode: "InvalidArgument",
Err: "KMS context must be valid base64-encoded JSON with string values",
}
if len(context) == 0 {
return "", nil, invalidContext
}
b, err := base64.StdEncoding.DecodeString(context[0]) b, err := base64.StdEncoding.DecodeString(context[0])
if err != nil { if err != nil {
return "", nil, err return "", nil, invalidContext
} }
// Persisting invalid UTF-8 replaces bytes and can collapse distinct // Persisting invalid UTF-8 replaces bytes and can collapse distinct
// keys, changing the associated data needed to decrypt the object. // keys, changing the associated data needed to decrypt the object.
@@ -92,7 +100,7 @@ func (ssekms) ParseHTTP(h http.Header) (string, kms.Context, error) {
json := jsoniter.ConfigCompatibleWithStandardLibrary json := jsoniter.ConfigCompatibleWithStandardLibrary
if err := json.Unmarshal(b, &ctx); err != nil { if err := json.Unmarshal(b, &ctx); err != nil {
return "", nil, err return "", nil, invalidContext
} }
} }
+16 -1
View File
@@ -38,6 +38,13 @@ func TestKMSParseHTTPContext(t *testing.T) {
{name: "empty", raw: `{}`, want: kms.Context{}}, {name: "empty", raw: `{}`, want: kms.Context{}},
{name: "null", raw: `null`}, {name: "null", raw: `null`},
{name: "unicode", raw: `{"区域":"杭州","emoji":"🔑","�":"replacement"}`, want: kms.Context{"区域": "杭州", "emoji": "🔑", "�": "replacement"}}, {name: "unicode", raw: `{"区域":"杭州","emoji":"🔑","�":"replacement"}`, want: kms.Context{"区域": "杭州", "emoji": "🔑", "�": "replacement"}},
{name: "array", raw: `[]`, invalid: true},
{name: "invalid-json", raw: `not-json`, invalid: true},
{name: "trailing-json", raw: `{"tenant":"ok"} {}`, invalid: true},
{name: "number-value", raw: `{"tenant":1}`, invalid: true},
{name: "invalid-base64", invalid: true},
{name: "empty-header", invalid: true},
{name: "nil-header-values", invalid: true},
{name: "invalid-key", raw: "{\"\xff\":\"value\"}", invalid: true}, {name: "invalid-key", raw: "{\"\xff\":\"value\"}", invalid: true},
{name: "invalid-value", raw: "{\"tenant\":\"\xff\"}", invalid: true}, {name: "invalid-value", raw: "{\"tenant\":\"\xff\"}", invalid: true},
{name: "colliding-keys", raw: "{\"\xff\":\"a\",\"\xfe\":\"b\"}", invalid: true}, {name: "colliding-keys", raw: "{\"\xff\":\"a\",\"\xfe\":\"b\"}", invalid: true},
@@ -50,11 +57,19 @@ func TestKMSParseHTTPContext(t *testing.T) {
if tc.raw != "" { if tc.raw != "" {
header.Set(xhttp.AmzServerSideEncryptionKmsContext, base64.StdEncoding.EncodeToString([]byte(tc.raw))) header.Set(xhttp.AmzServerSideEncryptionKmsContext, base64.StdEncoding.EncodeToString([]byte(tc.raw)))
} }
switch tc.name {
case "invalid-base64":
header.Set(xhttp.AmzServerSideEncryptionKmsContext, "not-base64!")
case "empty-header":
header.Set(xhttp.AmzServerSideEncryptionKmsContext, "")
case "nil-header-values":
header[xhttp.AmzServerSideEncryptionKmsContext] = nil
}
key, context, err := S3KMS.ParseHTTP(header) key, context, err := S3KMS.ParseHTTP(header)
if tc.invalid { if tc.invalid {
var apiErr kms.Error var apiErr kms.Error
if !errors.As(err, &apiErr) || apiErr.Code != http.StatusBadRequest || apiErr.APICode != "InvalidArgument" { if !errors.As(err, &apiErr) || apiErr.Code != http.StatusBadRequest || apiErr.APICode != "InvalidArgument" {
t.Fatalf("invalid UTF-8: got %v, want 400 InvalidArgument", err) t.Fatalf("invalid context: got %v, want 400 InvalidArgument", err)
} }
return return
} }