From 6828a912f66a98aea35133dbfeda1319a34c8a34 Mon Sep 17 00:00:00 2001 From: Satyam Trivedi Date: Thu, 22 Oct 2026 16:18:03 +0530 Subject: [PATCH 1/2] fix(aws-kms): return InvalidMarkerException for an unknown Marker on list operations A Marker the server never issued now gives InvalidMarkerException on ListKeys, ListAliases, ListGrants, ListRetirableGrants, ListKeyRotations, ListResourceTags and DescribeCustomKeyStores, which all model it. ListKeyPolicies does not model it, so it keeps ValidationException. A Marker longer than 1024 characters or with a character outside U+0020-U+00FF fails the modeled MarkerType constraint and gives ValidationException on every operation. --- server/aws/kms/customkeystore_ops.go | 2 +- server/aws/kms/management_ops.go | 8 +-- server/aws/kms/operations.go | 6 +- server/aws/kms/paginate.go | 66 ++++++++++++++++++-- server/aws/kms/pagination_test.go | 93 ++++++++++++++++++++++++++++ services/kms/driver/errors.go | 1 + 6 files changed, 163 insertions(+), 13 deletions(-) diff --git a/server/aws/kms/customkeystore_ops.go b/server/aws/kms/customkeystore_ops.go index 2adca2f89..a101a655b 100644 --- a/server/aws/kms/customkeystore_ops.go +++ b/server/aws/kms/customkeystore_ops.go @@ -151,7 +151,7 @@ func (h *Handler) describeCustomKeyStores(w http.ResponseWriter, r *http.Request return nil, err } - start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(stores)) + start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(stores), markerInvalid) if err != nil { return nil, err } diff --git a/server/aws/kms/management_ops.go b/server/aws/kms/management_ops.go index 9de0dc325..7f8f311af 100644 --- a/server/aws/kms/management_ops.go +++ b/server/aws/kms/management_ops.go @@ -36,7 +36,7 @@ func (h *Handler) listGrants(w http.ResponseWriter, r *http.Request) { sort.Slice(grants, func(i, j int) bool { return grants[i].GrantID < grants[j].GrantID }) - start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(grants)) + start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(grants), markerInvalid) if perr != nil { return nil, perr } @@ -80,7 +80,7 @@ func (h *Handler) listRetirableGrants(w http.ResponseWriter, r *http.Request) { sort.Slice(grants, func(i, j int) bool { return grants[i].GrantID < grants[j].GrantID }) - start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(grants)) + start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(grants), markerInvalid) if perr != nil { return nil, perr } @@ -138,7 +138,7 @@ func (h *Handler) listKeyRotations(w http.ResponseWriter, r *http.Request) { return nil, err } - start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(events)) + start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(events), markerInvalid) if perr != nil { return nil, perr } @@ -201,7 +201,7 @@ func (h *Handler) listKeyPolicies(w http.ResponseWriter, r *http.Request) { sort.Strings(names) - start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(names)) + start, end, next, truncated, perr := pageWindow(req.Marker, req.Limit, len(names), markerValidation) if perr != nil { return nil, perr } diff --git a/server/aws/kms/operations.go b/server/aws/kms/operations.go index c7ad38656..54cb04d78 100644 --- a/server/aws/kms/operations.go +++ b/server/aws/kms/operations.go @@ -70,7 +70,7 @@ func (h *Handler) listKeys(w http.ResponseWriter, r *http.Request) { // unsorted offset Marker could repeat or skip keys across pages. sort.Slice(keys, func(i, j int) bool { return keys[i].KeyID < keys[j].KeyID }) - start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(keys)) + start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(keys), markerInvalid) if err != nil { writeErr(w, err) return @@ -217,7 +217,7 @@ func (h *Handler) listAliases(w http.ResponseWriter, r *http.Request) { // Sort before paging: the provider returns aliases in map-iteration order. sort.Slice(aliases, func(i, j int) bool { return aliases[i].Name < aliases[j].Name }) - start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(aliases)) + start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(aliases), markerInvalid) if err != nil { writeErr(w, err) return @@ -280,7 +280,7 @@ func (h *Handler) listResourceTags(w http.ResponseWriter, r *http.Request) { all := mapToTags(tags) sort.Slice(all, func(i, j int) bool { return all[i].TagKey < all[j].TagKey }) - start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(all)) + start, end, next, truncated, err := pageWindow(req.Marker, req.Limit, len(all), markerInvalid) if err != nil { writeErr(w, err) return diff --git a/server/aws/kms/paginate.go b/server/aws/kms/paginate.go index 506a3f24f..736b5a85f 100644 --- a/server/aws/kms/paginate.go +++ b/server/aws/kms/paginate.go @@ -1,20 +1,42 @@ package kms import ( + "unicode/utf8" + cerrors "github.com/stackshy/cloudemu/v2/errors" "github.com/stackshy/cloudemu/v2/server/wire" + kmsdriver "github.com/stackshy/cloudemu/v2/services/kms/driver" ) // defaultPageLimit is the page size KMS list operations use when Limit is unset. const defaultPageLimit = 100 +// maxMarkerLen is the modeled MarkerType length limit (1-1024). +const maxMarkerLen = 1024 + +// markerFault says which exception a well-formed Marker that this server did +// not issue produces. Most KMS list operations model InvalidMarkerException; +// ListKeyPolicies does not, so it answers ValidationException. +type markerFault int + +const ( + // markerInvalid answers InvalidMarkerException. + markerInvalid markerFault = iota + // markerValidation answers ValidationException, for operations whose error + // list has no InvalidMarkerException. + markerValidation +) + // pageWindow resolves an opaque Marker + Limit into a [start,end) slice window // over a stable result set of length total. It returns the NextMarker to hand // back ("" on the last page) and whether the result was truncated. KMS keys its // pagination on Marker/Limit and echoes Truncated/NextMarker; an offset-based // marker is stable because each list op returns a deterministically ordered set. -func pageWindow(marker string, limit int32, total int) (start, end int, nextMarker string, truncated bool, err error) { - start, err = decodeMarker(marker) +// fault picks the exception for a Marker that decodes to no position. +func pageWindow( + marker string, limit int32, total int, fault markerFault, +) (start, end int, nextMarker string, truncated bool, err error) { + start, err = decodeMarker(marker, fault) if err != nil { return 0, 0, "", false, err } @@ -36,11 +58,45 @@ func pageWindow(marker string, limit int32, total int) (start, end int, nextMark return start, end, wire.EncodeOffset(end), true, nil } -func decodeMarker(marker string) (int, error) { +// decodeMarker turns a Marker back into an offset. A Marker that breaks the +// modeled MarkerType constraints (length up to 1024, characters U+0020-U+00FF) +// fails request validation with ValidationException on every operation. A +// Marker that passes them but was not issued by this server is +// InvalidMarkerException where the operation models it. +func decodeMarker(marker string, fault markerFault) (int, error) { + if err := validateMarkerConstraint(marker); err != nil { + return 0, err + } + n, err := wire.DecodeOffset(marker) - if err != nil { + if err == nil { + return n, nil + } + + if fault == markerValidation { return 0, cerrors.New(cerrors.InvalidArgument, "invalid Marker") } - return n, nil + return 0, kmsdriver.NewAPIError(kmsdriver.ExInvalidMarker, cerrors.InvalidArgument, + "The request was rejected because the marker that specifies where pagination should next begin is not valid.") +} + +// validateMarkerConstraint enforces the MarkerType length and pattern, with +// the message AWS returns for a failed constraint. +func validateMarkerConstraint(marker string) error { + const prefix = "1 validation error detected: Value '%s' at 'marker' failed to satisfy constraint: " + + if utf8.RuneCountInString(marker) > maxMarkerLen { + return cerrors.Newf(cerrors.InvalidArgument, + prefix+"Member must have length less than or equal to %d", marker, maxMarkerLen) + } + + for _, r := range marker { + if r < 0x20 || r > 0xFF { + return cerrors.Newf(cerrors.InvalidArgument, + prefix+"Member must satisfy regular expression pattern: [\\u0020-\\u00FF]*", marker) + } + } + + return nil } diff --git a/server/aws/kms/pagination_test.go b/server/aws/kms/pagination_test.go index 5980d4a1c..8cee8b434 100644 --- a/server/aws/kms/pagination_test.go +++ b/server/aws/kms/pagination_test.go @@ -2,10 +2,14 @@ package kms_test import ( "context" + "errors" + "strings" "testing" "github.com/aws/aws-sdk-go-v2/aws" awskms "github.com/aws/aws-sdk-go-v2/service/kms" + kmstypes "github.com/aws/aws-sdk-go-v2/service/kms/types" + "github.com/aws/smithy-go" ) // TestSDKListKeysPaginationDeterministic walks ListKeys one key per page and @@ -53,3 +57,92 @@ func TestSDKListKeysPaginationDeterministic(t *testing.T) { } } } + +// TestSDKListBadMarker sends a Marker the server never issued to every KMS list +// operation. Operations that model InvalidMarkerException return it; only +// ListKeyPolicies, whose error list has no InvalidMarkerException, answers +// ValidationException. A Marker over the modeled 1024-character limit, or with +// a character outside U+0020-U+00FF, fails request validation with +// ValidationException everywhere. +func TestSDKListBadMarker(t *testing.T) { + ctx := context.Background() + c := newKMSClient(t) + + key, err := c.CreateKey(ctx, &awskms.CreateKeyInput{}) + if err != nil { + t.Fatalf("CreateKey: %v", err) + } + + keyID := key.KeyMetadata.KeyId + + if _, err := c.CreateGrant(ctx, &awskms.CreateGrantInput{ + KeyId: keyID, + GranteePrincipal: aws.String("arn:aws:iam::111122223333:role/R"), + RetiringPrincipal: aws.String("arn:aws:iam::111122223333:role/R"), + Operations: []kmstypes.GrantOperation{kmstypes.GrantOperationEncrypt}, + }); err != nil { + t.Fatalf("CreateGrant: %v", err) + } + + ops := []struct { + name string + call func(marker *string) error + want string + }{ + {"ListKeys", func(m *string) error { + _, err := c.ListKeys(ctx, &awskms.ListKeysInput{Marker: m}) + return err + }, "InvalidMarkerException"}, + {"ListAliases", func(m *string) error { + _, err := c.ListAliases(ctx, &awskms.ListAliasesInput{Marker: m}) + return err + }, "InvalidMarkerException"}, + {"ListGrants", func(m *string) error { + _, err := c.ListGrants(ctx, &awskms.ListGrantsInput{KeyId: keyID, Marker: m}) + return err + }, "InvalidMarkerException"}, + {"ListRetirableGrants", func(m *string) error { + _, err := c.ListRetirableGrants(ctx, &awskms.ListRetirableGrantsInput{ + RetiringPrincipal: aws.String("arn:aws:iam::111122223333:role/R"), Marker: m, + }) + return err + }, "InvalidMarkerException"}, + {"ListKeyRotations", func(m *string) error { + _, err := c.ListKeyRotations(ctx, &awskms.ListKeyRotationsInput{KeyId: keyID, Marker: m}) + return err + }, "InvalidMarkerException"}, + {"ListResourceTags", func(m *string) error { + _, err := c.ListResourceTags(ctx, &awskms.ListResourceTagsInput{KeyId: keyID, Marker: m}) + return err + }, "InvalidMarkerException"}, + {"DescribeCustomKeyStores", func(m *string) error { + _, err := c.DescribeCustomKeyStores(ctx, &awskms.DescribeCustomKeyStoresInput{Marker: m}) + return err + }, "InvalidMarkerException"}, + {"ListKeyPolicies", func(m *string) error { + _, err := c.ListKeyPolicies(ctx, &awskms.ListKeyPoliciesInput{KeyId: keyID, Marker: m}) + return err + }, "ValidationException"}, + } + + for _, op := range ops { + t.Run(op.name, func(t *testing.T) { + assertErrorCode(t, op.call(aws.String("not-a-marker")), op.want) + assertErrorCode(t, op.call(aws.String(strings.Repeat("a", 1025))), "ValidationException") + assertErrorCode(t, op.call(aws.String("bad\u0100marker")), "ValidationException") + }) + } +} + +func assertErrorCode(t *testing.T, err error, want string) { + t.Helper() + + var apiErr smithy.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("want %s, got %v", want, err) + } + + if apiErr.ErrorCode() != want { + t.Fatalf("want %s, got %s: %s", want, apiErr.ErrorCode(), apiErr.ErrorMessage()) + } +} diff --git a/services/kms/driver/errors.go b/services/kms/driver/errors.go index 867d8c5fa..bfd6f5510 100644 --- a/services/kms/driver/errors.go +++ b/services/kms/driver/errors.go @@ -12,6 +12,7 @@ import ( const ( ExTag = "TagException" ExInvalidArn = "InvalidArnException" + ExInvalidMarker = "InvalidMarkerException" ExCustomKeyStoreNotFound = "CustomKeyStoreNotFoundException" ExCustomKeyStoreNameInUse = "CustomKeyStoreNameInUseException" ExCustomKeyStoreInvalidState = "CustomKeyStoreInvalidStateException" From f52de43fd0eaf43f1cdf3a4c7b51d8e5a0574615 Mon Sep 17 00:00:00 2001 From: Satyam Trivedi Date: Thu, 22 Oct 2026 16:20:02 +0530 Subject: [PATCH 2/2] fix(aws-kms): name the key in the DisabledException message Real KMS answers an operation on a Disabled key with " is disabled." instead of a generic "key is disabled". The error is built with driver.WithMessage, so errors.Is still matches driver.ErrKeyDisabled for Secrets Manager and other callers. Every operation that returns DisabledException goes through requireUsable, so they all get the same message. --- providers/aws/kms/crypto.go | 3 +- providers/aws/kms/key_state_test.go | 57 ++++++++++++++++++++++++++++ server/aws/kms/key_state_sdk_test.go | 39 +++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 providers/aws/kms/key_state_test.go create mode 100644 server/aws/kms/key_state_sdk_test.go diff --git a/providers/aws/kms/crypto.go b/providers/aws/kms/crypto.go index 62a2548d3..36a190076 100644 --- a/providers/aws/kms/crypto.go +++ b/providers/aws/kms/crypto.go @@ -160,7 +160,8 @@ func (m *Mock) requireUsable(kd *keyData) error { case driver.StateEnabled: return nil case driver.StateDisabled: - return driver.ErrKeyDisabled + // Real KMS names the key: " is disabled." + return driver.WithMessage(driver.ErrKeyDisabled, "%s is disabled.", kd.meta.ARN) default: return driver.ErrKeyInvalidState } diff --git a/providers/aws/kms/key_state_test.go b/providers/aws/kms/key_state_test.go new file mode 100644 index 000000000..3094a65cf --- /dev/null +++ b/providers/aws/kms/key_state_test.go @@ -0,0 +1,57 @@ +package kms_test + +import ( + "context" + "errors" + "testing" + + cerrors "github.com/stackshy/cloudemu/v2/errors" + "github.com/stackshy/cloudemu/v2/services/kms/driver" +) + +// TestDisabledKeyMessageNamesKey checks that every operation refused for a +// Disabled key returns DisabledException with the key ARN in the message, as +// real KMS does (" is disabled."), while errors.Is still matches +// driver.ErrKeyDisabled for callers such as Secrets Manager. +func TestDisabledKeyMessageNamesKey(t *testing.T) { + ctx := context.Background() + m := newMock(t) + + k, err := m.CreateKey(ctx, driver.CreateKeyInput{}) + if err != nil { + t.Fatalf("CreateKey: %v", err) + } + + if err := m.DisableKey(ctx, k.KeyID); err != nil { + t.Fatalf("DisableKey: %v", err) + } + + want := k.ARN + " is disabled." + + ops := map[string]func() error{ + "Encrypt": func() error { + _, err := m.Encrypt(ctx, driver.EncryptInput{KeyID: k.KeyID, Plaintext: []byte("x")}) + return err + }, + "GenerateDataKey": func() error { + _, err := m.GenerateDataKey(ctx, driver.GenerateDataKeyInput{KeyID: k.KeyID, KeySpec: "AES_256"}) + return err + }, + "EnableKeyRotation": func() error { + return m.EnableKeyRotation(ctx, k.KeyID, 0) + }, + } + + for name, call := range ops { + t.Run(name, func(t *testing.T) { + err := call() + if !errors.Is(err, driver.ErrKeyDisabled) { + t.Fatalf("err = %v, want ErrKeyDisabled", err) + } + + if got := cerrors.Message(err); got != want { + t.Fatalf("message = %q, want %q", got, want) + } + }) + } +} diff --git a/server/aws/kms/key_state_sdk_test.go b/server/aws/kms/key_state_sdk_test.go new file mode 100644 index 000000000..dd93e2999 --- /dev/null +++ b/server/aws/kms/key_state_sdk_test.go @@ -0,0 +1,39 @@ +package kms_test + +import ( + "context" + "errors" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + awskms "github.com/aws/aws-sdk-go-v2/service/kms" + kmstypes "github.com/aws/aws-sdk-go-v2/service/kms/types" +) + +// TestSDKDisabledExceptionMessage checks that DisabledException reaches the +// client with the message real KMS sends, " is disabled.". +func TestSDKDisabledExceptionMessage(t *testing.T) { + ctx := context.Background() + c := newKMSClient(t) + + key, err := c.CreateKey(ctx, &awskms.CreateKeyInput{}) + if err != nil { + t.Fatalf("CreateKey: %v", err) + } + + if _, err := c.DisableKey(ctx, &awskms.DisableKeyInput{KeyId: key.KeyMetadata.KeyId}); err != nil { + t.Fatalf("DisableKey: %v", err) + } + + _, err = c.Encrypt(ctx, &awskms.EncryptInput{KeyId: key.KeyMetadata.KeyId, Plaintext: []byte("x")}) + + var disabled *kmstypes.DisabledException + if !errors.As(err, &disabled) { + t.Fatalf("want DisabledException, got %v", err) + } + + want := aws.ToString(key.KeyMetadata.Arn) + " is disabled." + if got := disabled.ErrorMessage(); got != want { + t.Fatalf("message = %q, want %q", got, want) + } +}