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/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/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) + } +} 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"