diff --git a/spinifex/arn/iam.go b/spinifex/arn/iam.go new file mode 100644 index 000000000..44a861b89 --- /dev/null +++ b/spinifex/arn/iam.go @@ -0,0 +1,30 @@ +package arn + +import "fmt" + +// IAMResourceType is the resource-type segment of an arn:aws:iam ARN. +type IAMResourceType string + +const ( + IAMUser IAMResourceType = "user" + IAMRole IAMResourceType = "role" + IAMGroup IAMResourceType = "group" + IAMPolicy IAMResourceType = "policy" + IAMInstanceProfile IAMResourceType = "instance-profile" + IAMOIDCProvider IAMResourceType = "oidc-provider" +) + +// FormatIAMPath builds an IAM ARN for a path-bearing resource. The path is +// preserved verbatim; callers are responsible for validating it. +func FormatIAMPath(kind IAMResourceType, accountID, resourcePath, name string) string { + if resourcePath == "" { + resourcePath = "/" + } + return fmt.Sprintf("arn:aws:iam::%s:%s%s%s", accountID, kind, resourcePath, name) +} + +// FormatIAMResource builds an IAM ARN from an already-formed resource +// component, such as an OIDC provider issuer host/path. +func FormatIAMResource(kind IAMResourceType, accountID, resource string) string { + return fmt.Sprintf("arn:aws:iam::%s:%s/%s", accountID, kind, resource) +} diff --git a/spinifex/arn/iam_test.go b/spinifex/arn/iam_test.go new file mode 100644 index 000000000..29599f553 --- /dev/null +++ b/spinifex/arn/iam_test.go @@ -0,0 +1,38 @@ +package arn_test + +import ( + "testing" + + "github.com/mulgadc/spinifex/spinifex/arn" +) + +func TestFormatIAMPath(t *testing.T) { + tests := []struct { + name string + kind arn.IAMResourceType + resourcePath string + resourceName string + want string + }{ + {"empty path defaults to root", arn.IAMUser, "", "alice", "arn:aws:iam::123456789012:user/alice"}, + {"root path", arn.IAMPolicy, "/", "app", "arn:aws:iam::123456789012:policy/app"}, + {"nested path", arn.IAMRole, "/service-roles/", "app", "arn:aws:iam::123456789012:role/service-roles/app"}, + {"path is not normalized", arn.IAMGroup, "/teams//ops/", "admins", "arn:aws:iam::123456789012:group/teams//ops/admins"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := arn.FormatIAMPath(tt.kind, "123456789012", tt.resourcePath, tt.resourceName) + if got != tt.want { + t.Fatalf("FormatIAMPath() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestFormatIAMResource(t *testing.T) { + got := arn.FormatIAMResource(arn.IAMOIDCProvider, "123456789012", "issuer.example/id/cluster") + want := "arn:aws:iam::123456789012:oidc-provider/issuer.example/id/cluster" + if got != want { + t.Fatalf("FormatIAMResource() = %q, want %q", got, want) + } +} diff --git a/spinifex/gateway/iam.go b/spinifex/gateway/iam.go index 273ae045f..d557c49d5 100644 --- a/spinifex/gateway/iam.go +++ b/spinifex/gateway/iam.go @@ -14,34 +14,46 @@ import ( "github.com/mulgadc/spinifex/spinifex/utils" ) -// IAMHandler processes parsed query args and returns XML response bytes. -type IAMHandler func(action string, q map[string]string, gw *GatewayConfig, accountID string) ([]byte, error) +// iamAction parses once so authorization and dispatch share one typed input. +type iamAction struct { + parse func(map[string]string) (any, error) + dispatch func(string, any, *GatewayConfig, string) ([]byte, error) +} -// iamHandler creates a type-safe IAMHandler: allocates the input struct, -// parses query params, calls the handler, and marshals output to XML. -func iamHandler[In any](handler func(string, *In, *GatewayConfig) (any, error)) IAMHandler { - return func(action string, q map[string]string, gw *GatewayConfig, accountID string) ([]byte, error) { - input := new(In) - if err := awsec2query.QueryParamsToStruct(q, input); err != nil { - if errors.Is(err, awsec2query.ErrSliceTooLarge) { - return nil, errors.New(awserrors.ErrorMalformedQueryString) +// iamHandler creates a type-safe IAM action with separate parse and dispatch +// stages so policy resolution consumes the exact handler input. +func iamHandler[In any](handler func(string, *In, *GatewayConfig) (any, error)) iamAction { + return iamAction{ + parse: func(q map[string]string) (any, error) { + input := new(In) + if err := awsec2query.QueryParamsToStruct(q, input); err != nil { + if errors.Is(err, awsec2query.ErrSliceTooLarge) { + return nil, errors.New(awserrors.ErrorMalformedQueryString) + } + return nil, errors.New(awserrors.ErrorIAMInvalidInput) } - return nil, errors.New(awserrors.ErrorIAMInvalidInput) - } - output, err := handler(accountID, input, gw) - if err != nil { - return nil, err - } - payload := utils.GenerateIAMXMLPayload(action, output) - xmlOutput, err := utils.MarshalToXML(payload) - if err != nil { - return nil, errors.New(awserrors.ErrorInternalError) - } - return xmlOutput, nil + return input, nil + }, + dispatch: func(action string, parsed any, gw *GatewayConfig, accountID string) ([]byte, error) { + input, ok := parsed.(*In) + if !ok { + return nil, errors.New(awserrors.ErrorInternalError) + } + output, err := handler(accountID, input, gw) + if err != nil { + return nil, err + } + payload := utils.GenerateIAMXMLPayload(action, output) + xmlOutput, err := utils.MarshalToXML(payload) + if err != nil { + return nil, errors.New(awserrors.ErrorInternalError) + } + return xmlOutput, nil + }, } } -var iamActions = map[string]IAMHandler{ +var iamActions = map[string]iamAction{ "CreateUser": iamHandler(func(accountID string, input *iam.CreateUserInput, gw *GatewayConfig) (any, error) { return gateway_iam.CreateUser(accountID, input, gw.IAMService) }), @@ -323,17 +335,25 @@ func (gw *GatewayConfig) IAM_Request(w http.ResponseWriter, r *http.Request) err return errors.New(awserrors.ErrorInternalError) } - if err := gw.checkPolicy(r, "iam", action); err != nil { - return err - } - accountID, _ := r.Context().Value(ctxAccountID).(string) if accountID == "" { slog.Error("IAM_Request: no account ID in auth context") return errors.New(awserrors.ErrorInternalError) } - xmlOutput, err := handler(action, queryArgs, gw, accountID) + input, err := handler.parse(queryArgs) + if err != nil { + return err + } + resources, err := gateway_iam.ResourceARNs(action, accountID, input, gw.IAMService) + if err != nil { + return err + } + if err := gw.checkPolicyResources(r, "iam", action, resources); err != nil { + return err + } + + xmlOutput, err := handler.dispatch(action, input, gw, accountID) if err != nil { return err } diff --git a/spinifex/gateway/iam/authz.go b/spinifex/gateway/iam/authz.go new file mode 100644 index 000000000..f6d99bbf0 --- /dev/null +++ b/spinifex/gateway/iam/authz.go @@ -0,0 +1,301 @@ +// Package gateway_iam contains IAM request validation and authorization helpers. +package gateway_iam + +import ( + "errors" + "log/slog" + "reflect" + "sort" + + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" +) + +const anyResource = "*" + +type resourceSource uint8 + +const ( + sourceAccount resourceSource = iota + sourceCreate + sourceExisting + sourcePolicyARN + sourceOIDCCreate + sourceOIDCARN + sourceAccessKeyOwner +) + +type resourceScope struct { + source resourceSource + kind arn.IAMResourceType + nameField string + pathField string +} + +func createScope(kind arn.IAMResourceType, name string) resourceScope { + return resourceScope{source: sourceCreate, kind: kind, nameField: name, pathField: "Path"} +} + +func existingScope(kind arn.IAMResourceType, name string) resourceScope { + return resourceScope{source: sourceExisting, kind: kind, nameField: name} +} + +var iamScopes = map[string]resourceScope{ + // Users. + "CreateUser": createScope(arn.IAMUser, "UserName"), + "GetUser": existingScope(arn.IAMUser, "UserName"), + "DeleteUser": existingScope(arn.IAMUser, "UserName"), + "CreateAccessKey": existingScope(arn.IAMUser, "UserName"), + "ListAccessKeys": existingScope(arn.IAMUser, "UserName"), + "DeleteAccessKey": existingScope(arn.IAMUser, "UserName"), + "UpdateAccessKey": {source: sourceAccessKeyOwner, nameField: "AccessKeyId"}, + "AttachUserPolicy": existingScope(arn.IAMUser, "UserName"), + "DetachUserPolicy": existingScope(arn.IAMUser, "UserName"), + "ListAttachedUserPolicies": existingScope(arn.IAMUser, "UserName"), + "PutUserPolicy": existingScope(arn.IAMUser, "UserName"), + "GetUserPolicy": existingScope(arn.IAMUser, "UserName"), + "DeleteUserPolicy": existingScope(arn.IAMUser, "UserName"), + "ListUserPolicies": existingScope(arn.IAMUser, "UserName"), + "TagUser": existingScope(arn.IAMUser, "UserName"), + "UntagUser": existingScope(arn.IAMUser, "UserName"), + "ListUserTags": existingScope(arn.IAMUser, "UserName"), + "ListGroupsForUser": existingScope(arn.IAMUser, "UserName"), + + // Roles. + "CreateRole": createScope(arn.IAMRole, "RoleName"), + "GetRole": existingScope(arn.IAMRole, "RoleName"), + "DeleteRole": existingScope(arn.IAMRole, "RoleName"), + "UpdateRole": existingScope(arn.IAMRole, "RoleName"), + "UpdateAssumeRolePolicy": existingScope(arn.IAMRole, "RoleName"), + "AttachRolePolicy": existingScope(arn.IAMRole, "RoleName"), + "DetachRolePolicy": existingScope(arn.IAMRole, "RoleName"), + "PutRolePolicy": existingScope(arn.IAMRole, "RoleName"), + "GetRolePolicy": existingScope(arn.IAMRole, "RoleName"), + "DeleteRolePolicy": existingScope(arn.IAMRole, "RoleName"), + "ListAttachedRolePolicies": existingScope(arn.IAMRole, "RoleName"), + "ListRolePolicies": existingScope(arn.IAMRole, "RoleName"), + "TagRole": existingScope(arn.IAMRole, "RoleName"), + "UntagRole": existingScope(arn.IAMRole, "RoleName"), + "ListRoleTags": existingScope(arn.IAMRole, "RoleName"), + "ListInstanceProfilesForRole": existingScope(arn.IAMRole, "RoleName"), + + // Groups. Membership actions evaluate only the group. + "CreateGroup": createScope(arn.IAMGroup, "GroupName"), + "GetGroup": existingScope(arn.IAMGroup, "GroupName"), + "DeleteGroup": existingScope(arn.IAMGroup, "GroupName"), + "AddUserToGroup": existingScope(arn.IAMGroup, "GroupName"), + "RemoveUserFromGroup": existingScope(arn.IAMGroup, "GroupName"), + "AttachGroupPolicy": existingScope(arn.IAMGroup, "GroupName"), + "DetachGroupPolicy": existingScope(arn.IAMGroup, "GroupName"), + "ListAttachedGroupPolicies": existingScope(arn.IAMGroup, "GroupName"), + "PutGroupPolicy": existingScope(arn.IAMGroup, "GroupName"), + "GetGroupPolicy": existingScope(arn.IAMGroup, "GroupName"), + "DeleteGroupPolicy": existingScope(arn.IAMGroup, "GroupName"), + "ListGroupPolicies": existingScope(arn.IAMGroup, "GroupName"), + + // Managed policies use the exact ARN the handler requires. + "CreatePolicy": createScope(arn.IAMPolicy, "PolicyName"), + "GetPolicy": {source: sourcePolicyARN, nameField: "PolicyArn"}, + "GetPolicyVersion": {source: sourcePolicyARN, nameField: "PolicyArn"}, + "ListPolicyVersions": {source: sourcePolicyARN, nameField: "PolicyArn"}, + "DeletePolicy": {source: sourcePolicyARN, nameField: "PolicyArn"}, + "TagPolicy": {source: sourcePolicyARN, nameField: "PolicyArn"}, + "UntagPolicy": {source: sourcePolicyARN, nameField: "PolicyArn"}, + "ListPolicyTags": {source: sourcePolicyARN, nameField: "PolicyArn"}, + + // Instance profiles. Role operands are IAM condition-key values, not extra resources. + "CreateInstanceProfile": createScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "GetInstanceProfile": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "DeleteInstanceProfile": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "AddRoleToInstanceProfile": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "RemoveRoleFromInstanceProfile": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "TagInstanceProfile": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "UntagInstanceProfile": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + "ListInstanceProfileTags": existingScope(arn.IAMInstanceProfile, "InstanceProfileName"), + + // OIDC providers are normalized into the caller's trusted account. + "CreateOpenIDConnectProvider": {source: sourceOIDCCreate, nameField: "Url"}, + "GetOpenIDConnectProvider": {source: sourceOIDCARN, nameField: "OpenIDConnectProviderArn"}, + "DeleteOpenIDConnectProvider": {source: sourceOIDCARN, nameField: "OpenIDConnectProviderArn"}, + "TagOpenIDConnectProvider": {source: sourceOIDCARN, nameField: "OpenIDConnectProviderArn"}, + "UntagOpenIDConnectProvider": {source: sourceOIDCARN, nameField: "OpenIDConnectProviderArn"}, + "ListOpenIDConnectProviderTags": {source: sourceOIDCARN, nameField: "OpenIDConnectProviderArn"}, + + // Account-wide actions are explicit entries, not omissions. + "ListUsers": {source: sourceAccount}, + "ListPolicies": {source: sourceAccount}, + "ListRoles": {source: sourceAccount}, + "ListInstanceProfiles": {source: sourceAccount}, + "ListOpenIDConnectProviders": {source: sourceAccount}, + "ListGroups": {source: sourceAccount}, + "GetAccountSummary": {source: sourceAccount}, +} + +// HasScope reports whether action has an explicit IAM scope-table entry. +func HasScope(action string) bool { + _, ok := iamScopes[action] + return ok +} + +// ScopedActions returns every action represented in the IAM scope table. +func ScopedActions() []string { + actions := make([]string, 0, len(iamScopes)) + for action := range iamScopes { + actions = append(actions, action) + } + sort.Strings(actions) + return actions +} + +// ResourceARNs resolves the resource an IAM request authorizes against from +// the same parsed SDK input that dispatch receives. +func ResourceARNs(action, accountID string, input any, svc handlers_iam.IAMService) ([]string, error) { + scope, ok := iamScopes[action] + if !ok { + slog.Error("IAM authz: action is served but absent from the scope table", "action", action) + return nil, errors.New(awserrors.ErrorInvalidAction) + } + resource, err := scope.resolve(action, accountID, input, svc) + if err != nil { + return nil, err + } + if resource == "" { + resource = anyResource + } + return []string{resource}, nil +} + +func (s resourceScope) resolve(action, accountID string, input any, svc handlers_iam.IAMService) (string, error) { + if s.source == sourceAccount { + return anyResource, nil + } + target := lookupTarget{action: action, kind: string(s.kind), accountID: accountID} + name, err := stringField(input, s.nameField) + if err != nil { + slog.Error("IAM authz: scope field is unreadable on the handler input, failing closed", + "action", action, "field", s.nameField, "err", err) + return "", err + } + target.name = name + if name == "" { + // The handler rejects the request as a validation fault, so this + // widening cannot reach a mutation. Logged at Debug, not Warn. + slog.Debug("IAM authz: identifier absent, authorizing account-wide", + "action", action, "field", s.nameField, "account_id", accountID) + return anyResource, nil + } + + switch s.source { + case sourceCreate: + resourcePath, err := stringField(input, s.pathField) + if err != nil { + slog.Error("IAM authz: path field is unreadable on the handler input, failing closed", + "action", action, "field", s.pathField, "err", err) + return "", err + } + return arn.FormatIAMPath(s.kind, accountID, resourcePath, name), nil + case sourceExisting: + return canonicalARN(target, s.kind, svc) + case sourcePolicyARN: + return name, nil + case sourceOIDCCreate: + hostPath, err := handlers_iam.OIDCProviderHostPathFromURL(name) + if hostPath == "" { + slog.Warn("IAM authz: issuer URL unresolvable, authorizing account-wide", + "action", action, "url", name, "account_id", accountID, "err", err) + return anyResource, nil + } + return arn.FormatIAMResource(arn.IAMOIDCProvider, accountID, hostPath), nil + case sourceOIDCARN: + hostPath, err := handlers_iam.OIDCProviderHostPathFromARN(name) + if hostPath == "" { + slog.Warn("IAM authz: provider ARN unresolvable, authorizing account-wide", + "action", action, "provider_arn", name, "account_id", accountID, "err", err) + return anyResource, nil + } + return arn.FormatIAMResource(arn.IAMOIDCProvider, accountID, hostPath), nil + case sourceAccessKeyOwner: + target.kind = "access-key" + key, err := svc.LookupAccessKey(name) + if err != nil { + return lookupResult(target, "", err) + } + if key == nil || key.AccountID != accountID || key.UserName == "" { + slog.Warn("IAM authz: access key unknown, cross-account, or ownerless, authorizing account-wide", + "action", action, "access_key_id", name, "account_id", accountID) + return anyResource, nil + } + target.kind, target.name = string(arn.IAMUser), key.UserName + return canonicalARN(target, arn.IAMUser, svc) + default: + slog.Error("IAM authz: unhandled resource source, failing closed", + "action", action, "source", s.source) + return "", errors.New(awserrors.ErrorInternalError) + } +} + +// lookupTarget carries the identity of the object being authorized so every +// resolution outcome can be logged with enough context to act on. +type lookupTarget struct { + action string + kind string + name string + accountID string +} + +func canonicalARN(t lookupTarget, kind arn.IAMResourceType, svc handlers_iam.IAMService) (string, error) { + resource, err := svc.CanonicalResourceARN(t.accountID, kind, t.name) + return lookupResult(t, resource, err) +} + +// lookupResult widens authorization to account-wide for a missing object and +// fails closed on a storage or decode fault. Both outcomes are logged because +// neither is visible in the response the caller receives. +func lookupResult(t lookupTarget, resource string, err error) (string, error) { + if err == nil { + if resource == "" { + slog.Error("IAM authz: stored record carries no ARN, failing closed", + "action", t.action, "kind", t.kind, "name", t.name, "account_id", t.accountID) + return "", errors.New(awserrors.ErrorInternalError) + } + return resource, nil + } + if err.Error() == awserrors.ErrorIAMNoSuchEntity { + // Authorization widens to "*", so a Deny scoped to the real target + // will not fire. Without this line that widening leaves no trace. + slog.Warn("IAM authz: target not found, authorizing account-wide", + "action", t.action, "kind", t.kind, "name", t.name, "account_id", t.accountID) + return anyResource, nil + } + slog.Error("IAM authz: canonical ARN lookup failed, failing closed", + "action", t.action, "kind", t.kind, "name", t.name, "account_id", t.accountID, "err", err) + return "", errors.New(awserrors.ErrorInternalError) +} + +func stringField(input any, name string) (string, error) { + value := reflect.ValueOf(input) + for value.IsValid() && value.Kind() == reflect.Pointer { + if value.IsNil() { + return "", errors.New(awserrors.ErrorInternalError) + } + value = value.Elem() + } + if !value.IsValid() || value.Kind() != reflect.Struct { + return "", errors.New(awserrors.ErrorInternalError) + } + field := value.FieldByName(name) + if !field.IsValid() { + return "", errors.New(awserrors.ErrorInternalError) + } + if field.Kind() == reflect.Pointer { + if field.IsNil() { + return "", nil + } + field = field.Elem() + } + if field.Kind() != reflect.String { + return "", errors.New(awserrors.ErrorInternalError) + } + return field.String(), nil +} diff --git a/spinifex/gateway/iam/authz_test.go b/spinifex/gateway/iam/authz_test.go new file mode 100644 index 000000000..17eb6f234 --- /dev/null +++ b/spinifex/gateway/iam/authz_test.go @@ -0,0 +1,152 @@ +package gateway_iam_test + +import ( + "errors" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_iam "github.com/mulgadc/spinifex/spinifex/gateway/iam" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testAccount = "123456789012" + +type resolverService struct { + handlers_iam.IAMService + + arns map[arn.IAMResourceType]map[string]string + lookupErr error + key *handlers_iam.AccessKey + gotKind arn.IAMResourceType + gotName string +} + +func (s *resolverService) CanonicalResourceARN(_ string, kind arn.IAMResourceType, name string) (string, error) { + s.gotKind, s.gotName = kind, name + if byName := s.arns[kind]; byName != nil { + if resource := byName[name]; resource != "" { + return resource, nil + } + } + return "", errors.New(awserrors.ErrorIAMNoSuchEntity) +} + +func (s *resolverService) LookupAccessKey(_ string) (*handlers_iam.AccessKey, error) { + if s.lookupErr != nil { + return nil, s.lookupErr + } + if s.key == nil { + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return s.key, nil +} + +func TestResourceARNs_Fidelity(t *testing.T) { + svc := &resolverService{arns: map[arn.IAMResourceType]map[string]string{ + arn.IAMUser: {"alice": "arn:aws:iam::123456789012:user/teams/alice"}, + arn.IAMRole: {"app": "arn:aws:iam::123456789012:role/service-roles/app"}, + arn.IAMGroup: {"ops": "arn:aws:iam::123456789012:group/teams/ops"}, + arn.IAMInstanceProfile: {"nodes": "arn:aws:iam::123456789012:instance-profile/eks/nodes"}, + }} + + tests := []struct { + name string + action string + input any + want string + }{ + {"create user default path", "CreateUser", &iam.CreateUserInput{UserName: aws.String("alice")}, "arn:aws:iam::123456789012:user/alice"}, + {"create nested role", "CreateRole", &iam.CreateRoleInput{RoleName: aws.String("app"), Path: aws.String("/service-roles/")}, "arn:aws:iam::123456789012:role/service-roles/app"}, + {"existing user canonical path", "DeleteUser", &iam.DeleteUserInput{UserName: aws.String("alice")}, "arn:aws:iam::123456789012:user/teams/alice"}, + {"existing role canonical path", "DeleteRole", &iam.DeleteRoleInput{RoleName: aws.String("app")}, "arn:aws:iam::123456789012:role/service-roles/app"}, + {"existing group canonical path", "DeleteGroup", &iam.DeleteGroupInput{GroupName: aws.String("ops")}, "arn:aws:iam::123456789012:group/teams/ops"}, + {"direct managed policy ARN", "DeletePolicy", &iam.DeletePolicyInput{PolicyArn: aws.String("arn:aws:iam::123456789012:policy/team/app")}, "arn:aws:iam::123456789012:policy/team/app"}, + {"existing instance profile", "DeleteInstanceProfile", &iam.DeleteInstanceProfileInput{InstanceProfileName: aws.String("nodes")}, "arn:aws:iam::123456789012:instance-profile/eks/nodes"}, + {"OIDC create", "CreateOpenIDConnectProvider", &iam.CreateOpenIDConnectProviderInput{Url: aws.String("https://issuer.example/id/cluster")}, "arn:aws:iam::123456789012:oidc-provider/issuer.example/id/cluster"}, + {"OIDC caller account normalization", "DeleteOpenIDConnectProvider", &iam.DeleteOpenIDConnectProviderInput{OpenIDConnectProviderArn: aws.String("arn:aws:iam::999999999999:oidc-provider/issuer.example/id/cluster")}, "arn:aws:iam::123456789012:oidc-provider/issuer.example/id/cluster"}, + {"account wide", "ListUsers", &iam.ListUsersInput{}, "*"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := gateway_iam.ResourceARNs(tt.action, testAccount, tt.input, svc) + require.NoError(t, err) + assert.Equal(t, []string{tt.want}, got) + }) + } +} + +func TestResourceARNs_SecondaryOperandsAreNotResources(t *testing.T) { + svc := &resolverService{arns: map[arn.IAMResourceType]map[string]string{ + arn.IAMUser: {"alice": "arn:aws:iam::123456789012:user/alice"}, + arn.IAMRole: {"app": "arn:aws:iam::123456789012:role/app"}, + arn.IAMGroup: {"ops": "arn:aws:iam::123456789012:group/ops"}, + arn.IAMInstanceProfile: {"nodes": "arn:aws:iam::123456789012:instance-profile/nodes"}, + }} + tests := []struct { + action string + input any + want string + }{ + {"AttachUserPolicy", &iam.AttachUserPolicyInput{UserName: aws.String("alice"), PolicyArn: aws.String("arn:aws:iam::123456789012:policy/p")}, "arn:aws:iam::123456789012:user/alice"}, + {"AttachRolePolicy", &iam.AttachRolePolicyInput{RoleName: aws.String("app"), PolicyArn: aws.String("arn:aws:iam::123456789012:policy/p")}, "arn:aws:iam::123456789012:role/app"}, + {"AttachGroupPolicy", &iam.AttachGroupPolicyInput{GroupName: aws.String("ops"), PolicyArn: aws.String("arn:aws:iam::123456789012:policy/p")}, "arn:aws:iam::123456789012:group/ops"}, + {"AddUserToGroup", &iam.AddUserToGroupInput{GroupName: aws.String("ops"), UserName: aws.String("alice")}, "arn:aws:iam::123456789012:group/ops"}, + {"AddRoleToInstanceProfile", &iam.AddRoleToInstanceProfileInput{InstanceProfileName: aws.String("nodes"), RoleName: aws.String("app")}, "arn:aws:iam::123456789012:instance-profile/nodes"}, + } + for _, tt := range tests { + t.Run(tt.action, func(t *testing.T) { + got, err := gateway_iam.ResourceARNs(tt.action, testAccount, tt.input, svc) + require.NoError(t, err) + assert.Equal(t, []string{tt.want}, got) + }) + } +} + +func TestResourceARNs_UpdateAccessKeyUsesRecordedOwner(t *testing.T) { + svc := &resolverService{ + arns: map[arn.IAMResourceType]map[string]string{ + arn.IAMUser: {"owner": "arn:aws:iam::123456789012:user/team/owner"}, + }, + key: &handlers_iam.AccessKey{AccountID: testAccount, UserName: "owner"}, + } + input := &iam.UpdateAccessKeyInput{ + AccessKeyId: aws.String("AKIAEXAMPLE"), + UserName: aws.String("attacker-selected"), + } + got, err := gateway_iam.ResourceARNs("UpdateAccessKey", testAccount, input, svc) + require.NoError(t, err) + assert.Equal(t, []string{"arn:aws:iam::123456789012:user/team/owner"}, got) + assert.Equal(t, "owner", svc.gotName) + + svc.key.AccountID = "999999999999" + got, err = gateway_iam.ResourceARNs("UpdateAccessKey", testAccount, input, svc) + require.NoError(t, err) + assert.Equal(t, []string{"*"}, got) +} + +func TestResourceARNs_MissingAndLookupErrors(t *testing.T) { + svc := &resolverService{} + + got, err := gateway_iam.ResourceARNs("DeleteRole", testAccount, &iam.DeleteRoleInput{}, svc) + require.NoError(t, err) + assert.Equal(t, []string{"*"}, got) + + got, err = gateway_iam.ResourceARNs("DeleteRole", testAccount, &iam.DeleteRoleInput{RoleName: aws.String("missing")}, svc) + require.NoError(t, err) + assert.Equal(t, []string{"*"}, got) + + svc.arns = map[arn.IAMResourceType]map[string]string{} + svc.lookupErr = errors.New("storage unavailable") + _, err = gateway_iam.ResourceARNs("UpdateAccessKey", testAccount, + &iam.UpdateAccessKeyInput{AccessKeyId: aws.String("AKIAEXAMPLE")}, svc) + require.EqualError(t, err, awserrors.ErrorInternalError) + + _, err = gateway_iam.ResourceARNs("NotAnAction", testAccount, &iam.ListUsersInput{}, svc) + require.EqualError(t, err, awserrors.ErrorInvalidAction) +} diff --git a/spinifex/gateway/iam_authz_completeness_test.go b/spinifex/gateway/iam_authz_completeness_test.go new file mode 100644 index 000000000..5e43cdf49 --- /dev/null +++ b/spinifex/gateway/iam_authz_completeness_test.go @@ -0,0 +1,21 @@ +//test:in-package — iamActions is unexported and must be compared with the scope table. + +package gateway + +import ( + "testing" + + gateway_iam "github.com/mulgadc/spinifex/spinifex/gateway/iam" + "github.com/stretchr/testify/assert" +) + +func TestIAMScopeTableIsExhaustive(t *testing.T) { + for action := range iamActions { + assert.True(t, gateway_iam.HasScope(action), + "IAM action %q has no resource scope entry", action) + } + for _, action := range gateway_iam.ScopedActions() { + _, ok := iamActions[action] + assert.True(t, ok, "IAM scope table contains unserved action %q", action) + } +} diff --git a/spinifex/gateway/iam_authz_test.go b/spinifex/gateway/iam_authz_test.go new file mode 100644 index 000000000..cdc03c541 --- /dev/null +++ b/spinifex/gateway/iam_authz_test.go @@ -0,0 +1,126 @@ +//test:in-package — exercises IAM_Request with policy and auth context helpers. + +package gateway + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type iamAuthzPolicyService struct { + policyMockIAMService + + arns map[string]string + canonicalErr error + deleteRoleErr error + deleted []string +} + +func (s *iamAuthzPolicyService) CanonicalResourceARN(_ string, _ arn.IAMResourceType, name string) (string, error) { + if s.canonicalErr != nil { + return "", s.canonicalErr + } + resource, ok := s.arns[name] + if !ok { + return "", errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return resource, nil +} + +func (s *iamAuthzPolicyService) DeleteRole(_ string, input *iam.DeleteRoleInput) (*iam.DeleteRoleOutput, error) { + s.deleted = append(s.deleted, *input.RoleName) + if s.deleteRoleErr != nil { + return nil, s.deleteRoleErr + } + return &iam.DeleteRoleOutput{}, nil +} + +func iamScopedGateway(resources map[string]string, statements ...handlers_iam.Statement) (*GatewayConfig, *iamAuthzPolicyService) { + docs := []handlers_iam.PolicyDocument{{Version: "2012-10-17", Statement: statements}} + svc := &iamAuthzPolicyService{ + policyMockIAMService: policyMockIAMService{ + getUserPoliciesFn: func(_, _ string) ([]handlers_iam.PolicyDocument, error) { return docs, nil }, + getRolePoliciesFn: func(_, _ string) ([]handlers_iam.PolicyDocument, error) { return docs, nil }, + }, + arns: resources, + } + return &GatewayConfig{DisableLogging: true, IAMService: svc}, svc +} + +func dispatchIAM(t *testing.T, gw *GatewayConfig, body string) error { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + ctx := context.WithValue(req.Context(), ctxService, "iam") + ctx = context.WithValue(ctx, ctxAccountID, authzAccountID) + req = withTestIdentity(req.WithContext(ctx)) + return gw.IAM_Request(httptest.NewRecorder(), req) +} + +func TestIAMRequest_ScopedDenyFires(t *testing.T) { + gw, svc := iamScopedGateway(map[string]string{ + "prod": "arn:aws:iam::123456789012:role/service-roles/prod", + "dev": "arn:aws:iam::123456789012:role/service-roles/dev", + }, + statement("Allow", "iam:*", "*"), + statement("Deny", "iam:DeleteRole", "arn:aws:iam::*:role/service-roles/prod"), + ) + + err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=prod") + require.EqualError(t, err, awserrors.ErrorAccessDenied) + require.NoError(t, dispatchIAM(t, gw, "Action=DeleteRole&RoleName=dev")) + assert.Equal(t, []string{"dev"}, svc.deleted) +} + +func TestIAMRequest_ScopedAllowGrants(t *testing.T) { + gw, svc := iamScopedGateway(map[string]string{ + "prod": "arn:aws:iam::123456789012:role/service-roles/prod", + "dev": "arn:aws:iam::123456789012:role/service-roles/dev", + }, statement("Allow", "iam:DeleteRole", "arn:aws:iam::*:role/service-roles/dev")) + + require.NoError(t, dispatchIAM(t, gw, "Action=DeleteRole&RoleName=dev")) + err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=prod") + require.EqualError(t, err, awserrors.ErrorAccessDenied) + assert.Equal(t, []string{"dev"}, svc.deleted) +} + +func TestIAMRequest_MissingTargetAndLookupFailure(t *testing.T) { + t.Run("account-wide grant reaches handler no-such-entity", func(t *testing.T) { + gw, svc := iamScopedGateway(nil, statement("Allow", "iam:*", "*")) + svc.deleteRoleErr = errors.New(awserrors.ErrorIAMNoSuchEntity) + err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=missing") + require.EqualError(t, err, awserrors.ErrorIAMNoSuchEntity) + }) + + t.Run("scoped grant denies unresolved target", func(t *testing.T) { + gw, _ := iamScopedGateway(nil, + statement("Allow", "iam:DeleteRole", "arn:aws:iam::*:role/missing")) + err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=missing") + require.EqualError(t, err, awserrors.ErrorAccessDenied) + }) + + t.Run("storage failure fails closed", func(t *testing.T) { + gw, svc := iamScopedGateway(nil, statement("Allow", "iam:*", "*")) + svc.canonicalErr = errors.New("storage unavailable") + err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=app") + require.EqualError(t, err, awserrors.ErrorInternalError) + assert.Empty(t, svc.deleted) + }) +} + +func TestIAMRequest_MissingIdentifierStaysValidationFault(t *testing.T) { + gw, _ := iamScopedGateway(nil, statement("Allow", "iam:*", "*")) + err := dispatchIAM(t, gw, "Action=DeleteRole") + require.EqualError(t, err, awserrors.ErrorMissingParameter) +} diff --git a/spinifex/gateway/iam_request_test.go b/spinifex/gateway/iam_request_test.go index cf7462198..171d363b3 100644 --- a/spinifex/gateway/iam_request_test.go +++ b/spinifex/gateway/iam_request_test.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" "github.com/stretchr/testify/assert" @@ -23,14 +24,16 @@ import ( type flexMockIAMService struct { handlers_iam.IAMService - createUserFn func(string, *iam.CreateUserInput) (*iam.CreateUserOutput, error) - getUserFn func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) - listUsersFn func(string, *iam.ListUsersInput) (*iam.ListUsersOutput, error) - deleteUserFn func(string, *iam.DeleteUserInput) (*iam.DeleteUserOutput, error) - createAccessKeyFn func(string, *iam.CreateAccessKeyInput) (*iam.CreateAccessKeyOutput, error) - listAccessKeysFn func(string, *iam.ListAccessKeysInput) (*iam.ListAccessKeysOutput, error) - deleteAccessKeyFn func(string, *iam.DeleteAccessKeyInput) (*iam.DeleteAccessKeyOutput, error) - updateAccessKeyFn func(string, *iam.UpdateAccessKeyInput) (*iam.UpdateAccessKeyOutput, error) + createUserFn func(string, *iam.CreateUserInput) (*iam.CreateUserOutput, error) + getUserFn func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) + listUsersFn func(string, *iam.ListUsersInput) (*iam.ListUsersOutput, error) + deleteUserFn func(string, *iam.DeleteUserInput) (*iam.DeleteUserOutput, error) + createAccessKeyFn func(string, *iam.CreateAccessKeyInput) (*iam.CreateAccessKeyOutput, error) + listAccessKeysFn func(string, *iam.ListAccessKeysInput) (*iam.ListAccessKeysOutput, error) + deleteAccessKeyFn func(string, *iam.DeleteAccessKeyInput) (*iam.DeleteAccessKeyOutput, error) + updateAccessKeyFn func(string, *iam.UpdateAccessKeyInput) (*iam.UpdateAccessKeyOutput, error) + canonicalResourceARNFn func(string, arn.IAMResourceType, string) (string, error) + lookupAccessKeyFn func(string) (*handlers_iam.AccessKey, error) getAccountSummaryFn func(string, *iam.GetAccountSummaryInput) (*iam.GetAccountSummaryOutput, error) } @@ -91,6 +94,20 @@ func (m *flexMockIAMService) UpdateAccessKey(accountID string, input *iam.Update return &iam.UpdateAccessKeyOutput{}, nil } +func (m *flexMockIAMService) CanonicalResourceARN(accountID string, kind arn.IAMResourceType, name string) (string, error) { + if m.canonicalResourceARNFn != nil { + return m.canonicalResourceARNFn(accountID, kind, name) + } + return arn.FormatIAMPath(kind, accountID, "/", name), nil +} + +func (m *flexMockIAMService) LookupAccessKey(accessKeyID string) (*handlers_iam.AccessKey, error) { + if m.lookupAccessKeyFn != nil { + return m.lookupAccessKeyFn(accessKeyID) + } + return &handlers_iam.AccessKey{AccessKeyID: accessKeyID, AccountID: "000000000000", UserName: "user"}, nil +} + func (m *flexMockIAMService) GetAccountSummary(accountID string, input *iam.GetAccountSummaryInput) (*iam.GetAccountSummaryOutput, error) { if m.getAccountSummaryFn != nil { return m.getAccountSummaryFn(accountID, input) diff --git a/spinifex/gateway/policy/evaluator_test.go b/spinifex/gateway/policy/evaluator_test.go index 7fcb1d57b..aa54efb28 100644 --- a/spinifex/gateway/policy/evaluator_test.go +++ b/spinifex/gateway/policy/evaluator_test.go @@ -151,6 +151,22 @@ func TestEvaluate_ResourceScopedStatementsNeedARealARN(t *testing.T) { // Resource "*" for the action, and "*" as a pattern matches any ARN value, so // handing the evaluator a real ARN cannot turn an Allow into a Deny — not even // a malformed one, which bounds a resolver bug to the newly-scoped statements. +func TestEvaluate_IAMResourceARNs(t *testing.T) { + resources := []string{ + "arn:aws:iam::123456789012:user/alice", + "arn:aws:iam::123456789012:role/service-roles/app", + "arn:aws:iam::123456789012:policy/team/app", + "arn:aws:iam::123456789012:instance-profile/eks/nodes", + "arn:aws:iam::123456789012:oidc-provider/issuer.example/id/cluster", + } + for _, resource := range resources { + policies := []handlers_iam.PolicyDocument{doc("Allow", "iam:Delete*", resource)} + if got := iampolicy.EvaluateWithKeys("iam:DeleteRole", resource, policies, nil); got != iampolicy.Allow { + t.Errorf("IAM scoped policy against %q: got %v, want Allow", resource, got) + } + } +} + func TestEvaluate_PassingARealARNCannotWithdrawAccess(t *testing.T) { resources := []string{ "*", diff --git a/spinifex/handlers/ecs/instancerole.go b/spinifex/handlers/ecs/instancerole.go index 4257801cb..6bc07cd9e 100644 --- a/spinifex/handlers/ecs/instancerole.go +++ b/spinifex/handlers/ecs/instancerole.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" ) @@ -80,7 +81,7 @@ func (s *Service) ensureECSRole(accountID string) error { // ensureECSRolePolicy creates the ecs:* policy (resolving an existing one on a // race) and attaches it to the role; an already-attached policy is success. func (s *Service) ensureECSRolePolicy(accountID string) error { - policyARN := fmt.Sprintf("arn:aws:iam::%s:policy/%s", accountID, ecsInstanceRolePolicyName) + policyARN := arn.FormatIAMPath(arn.IAMPolicy, accountID, "/", ecsInstanceRolePolicyName) out, err := s.deps.IAM.CreatePolicy(accountID, &iam.CreatePolicyInput{ PolicyName: aws.String(ecsInstanceRolePolicyName), diff --git a/spinifex/handlers/iam/authorization.go b/spinifex/handlers/iam/authorization.go new file mode 100644 index 000000000..8691af24b --- /dev/null +++ b/spinifex/handlers/iam/authorization.go @@ -0,0 +1,100 @@ +package handlers_iam + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// canonicalARNAttempts bounds retries of a transient JetStream fault. This read +// gates every name-addressed IAM request, so a leader election that the policy +// snapshot resolver rides out must not fail the request here instead. +const canonicalARNAttempts = 3 + +// CanonicalResourceARN returns the ARN stored on a name-addressed IAM record. +// It avoids public IAM operations that perform membership or attachment scans. +func (s *IAMServiceImpl) CanonicalResourceARN(accountID string, kind arn.IAMResourceType, name string) (string, error) { + var resource string + var err error + for attempt := range canonicalARNAttempts { + resource, err = s.canonicalResourceARN(accountID, kind, name) + if err == nil || !isNATSTransient(err) { + break + } + if attempt < canonicalARNAttempts-1 { + slog.Debug("CanonicalResourceARN: transient NATS error, retrying", + "accountID", accountID, "kind", kind, "name", name, "attempt", attempt+1, "err", err) + time.Sleep(time.Duration(attempt+1) * 200 * time.Millisecond) + } + } + return resource, err +} + +func isNATSTransient(err error) bool { + return err != nil && (errors.Is(err, nats.ErrNoResponders) || + errors.Is(err, nats.ErrTimeout) || + errors.Is(err, nats.ErrNoStreamResponse)) +} + +func (s *IAMServiceImpl) canonicalResourceARN(accountID string, kind arn.IAMResourceType, name string) (string, error) { + ctx := context.Background() + switch kind { + case arn.IAMUser: + resource, err := s.getUser(ctx, accountID, name) + if err != nil { + return "", err + } + return resource.ARN, nil + case arn.IAMRole: + resource, err := s.getRole(ctx, accountID, name) + if err != nil { + return "", err + } + return resource.ARN, nil + case arn.IAMGroup: + resource, err := s.getGroup(ctx, accountID, name) + if err != nil { + return "", err + } + return resource.ARN, nil + case arn.IAMPolicy: + resource, err := s.getPolicyByName(ctx, accountID, name) + if err != nil { + return "", err + } + return resource.ARN, nil + case arn.IAMInstanceProfile: + resource, err := s.getInstanceProfile(ctx, accountID, name) + if err != nil { + return "", err + } + return resource.ARN, nil + default: + return "", fmt.Errorf("unsupported IAM resource type %q", kind) + } +} + +func (s *IAMServiceImpl) getPolicyByName(ctx context.Context, accountID, policyName string) (*Policy, error) { + entry, err := s.policiesBucket.Get(ctx, accountID+"."+policyName) + if err != nil { + if errors.Is(err, jetstream.ErrKeyNotFound) { + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return nil, fmt.Errorf("get policy: %w", err) + } + + var policy Policy + if err := json.Unmarshal(entry.Value(), &policy); err != nil { + return nil, fmt.Errorf("unmarshal policy: %w", err) + } + return &policy, nil +} diff --git a/spinifex/handlers/iam/authorization_test.go b/spinifex/handlers/iam/authorization_test.go new file mode 100644 index 000000000..3f3204937 --- /dev/null +++ b/spinifex/handlers/iam/authorization_test.go @@ -0,0 +1,48 @@ +//test:in-package — uses setupTestIAMService and testAccountID, unexported fixtures shared with the other handler tests. + +package handlers_iam + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/stretchr/testify/require" +) + +func TestCanonicalResourceARNPreservesStoredPaths(t *testing.T) { + svc := setupTestIAMService(t) + trust := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"ec2.amazonaws.com"},"Action":"sts:AssumeRole"}]}` + policy := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}` + + _, err := svc.CreateUser(testAccountID, &iam.CreateUserInput{UserName: aws.String("alice"), Path: aws.String("/teams/")}) + require.NoError(t, err) + _, err = svc.CreateRole(testAccountID, &iam.CreateRoleInput{RoleName: aws.String("app"), Path: aws.String("/service-roles/"), AssumeRolePolicyDocument: aws.String(trust)}) + require.NoError(t, err) + _, err = svc.CreateGroup(testAccountID, &iam.CreateGroupInput{GroupName: aws.String("ops"), Path: aws.String("/teams/")}) + require.NoError(t, err) + _, err = svc.CreatePolicy(testAccountID, &iam.CreatePolicyInput{PolicyName: aws.String("app"), Path: aws.String("/managed/"), PolicyDocument: aws.String(policy)}) + require.NoError(t, err) + _, err = svc.CreateInstanceProfile(testAccountID, &iam.CreateInstanceProfileInput{InstanceProfileName: aws.String("nodes"), Path: aws.String("/eks/")}) + require.NoError(t, err) + + tests := []struct { + kind arn.IAMResourceType + name string + want string + }{ + {arn.IAMUser, "alice", "arn:aws:iam::000000000000:user/teams/alice"}, + {arn.IAMRole, "app", "arn:aws:iam::000000000000:role/service-roles/app"}, + {arn.IAMGroup, "ops", "arn:aws:iam::000000000000:group/teams/ops"}, + {arn.IAMPolicy, "app", "arn:aws:iam::000000000000:policy/managed/app"}, + {arn.IAMInstanceProfile, "nodes", "arn:aws:iam::000000000000:instance-profile/eks/nodes"}, + } + for _, tt := range tests { + t.Run(string(tt.kind), func(t *testing.T) { + got, err := svc.CanonicalResourceARN(testAccountID, tt.kind, tt.name) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/spinifex/handlers/iam/groups.go b/spinifex/handlers/iam/groups.go index 0dc1e08fb..739c22773 100644 --- a/spinifex/handlers/iam/groups.go +++ b/spinifex/handlers/iam/groups.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/service/iam" "github.com/nats-io/nats.go/jetstream" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/kvutil" "github.com/mulgadc/spinifex/spinifex/utils" @@ -49,7 +50,7 @@ func (s *IAMServiceImpl) CreateGroup(accountID string, input *iam.CreateGroupInp GroupName: groupName, GroupID: groupID, AccountID: accountID, - ARN: fmt.Sprintf("arn:aws:iam::%s:group%s%s", accountID, path, groupName), + ARN: arn.FormatIAMPath(arn.IAMGroup, accountID, path, groupName), Path: path, CreatedAt: time.Now().UTC().Format(time.RFC3339), AttachedPolicies: []string{}, diff --git a/spinifex/handlers/iam/instance_profiles.go b/spinifex/handlers/iam/instance_profiles.go index 6fded06f9..38c0ae3e3 100644 --- a/spinifex/handlers/iam/instance_profiles.go +++ b/spinifex/handlers/iam/instance_profiles.go @@ -13,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go/service/iam" "github.com/nats-io/nats.go/jetstream" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/kvutil" "github.com/mulgadc/spinifex/spinifex/utils" @@ -47,7 +48,7 @@ func (s *IAMServiceImpl) CreateInstanceProfile(accountID string, input *iam.Crea InstanceProfileName: profileName, InstanceProfileID: profileID, AccountID: accountID, - ARN: fmt.Sprintf("arn:aws:iam::%s:instance-profile%s%s", accountID, path, profileName), + ARN: arn.FormatIAMPath(arn.IAMInstanceProfile, accountID, path, profileName), Path: path, CreatedAt: time.Now().UTC().Format(time.RFC3339), Tags: copyTags(input.Tags), diff --git a/spinifex/handlers/iam/oidc_providers.go b/spinifex/handlers/iam/oidc_providers.go index 6d5055df5..6e6979182 100644 --- a/spinifex/handlers/iam/oidc_providers.go +++ b/spinifex/handlers/iam/oidc_providers.go @@ -16,6 +16,7 @@ import ( "github.com/aws/aws-sdk-go/service/iam" "github.com/nats-io/nats.go/jetstream" + resourcearn "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/kvutil" "github.com/mulgadc/spinifex/spinifex/utils" @@ -45,7 +46,7 @@ func OIDCProviderKey(issuer string) string { // OIDCProviderARN returns the arn:aws:iam::{accountID}:oidc-provider/{issuerHostPath} ARN. // issuerHostPath is the issuer URL with the https:// scheme stripped. func OIDCProviderARN(accountID, issuerHostPath string) string { - return fmt.Sprintf("arn:aws:iam::%s:oidc-provider/%s", accountID, issuerHostPath) + return resourcearn.FormatIAMResource(resourcearn.IAMOIDCProvider, accountID, issuerHostPath) } // GetOrCreateIAMAccountBucket opens the per-account IAM bucket, creating it on first use. @@ -63,34 +64,51 @@ type OIDCProviderRecord struct { Tags []Tag `json:"tags,omitempty"` } -// issuerFromOIDCProviderARN reverses OIDCProviderARN, reconstructing the full https issuer URL. -func issuerFromOIDCProviderARN(arn string) (string, error) { - _, hostPath, ok := strings.Cut(arn, ":oidc-provider/") +// OIDCProviderHostPathFromARN extracts the provider component accepted by IAM +// handlers. The account segment is intentionally not trusted by callers. +func OIDCProviderHostPathFromARN(providerARN string) (string, error) { + _, hostPath, ok := strings.Cut(providerARN, ":oidc-provider/") if !ok { return "", errors.New("not an oidc-provider ARN") } if hostPath == "" { return "", errors.New("oidc-provider ARN missing host/path") } + return hostPath, nil +} + +// OIDCProviderHostPathFromURL validates an issuer and returns its exact +// resource component without normalizing host or path text. +func OIDCProviderHostPathFromURL(raw string) (string, error) { + if raw == "" { + return "", errors.New("url is required") + } + u, err := url.Parse(raw) + if err != nil { + return "", fmt.Errorf("parse url: %w", err) + } + if u.Scheme != "https" { + return "", fmt.Errorf("url scheme must be https, got %q", u.Scheme) + } + if u.Host == "" { + return "", errors.New("url missing host") + } + return strings.TrimPrefix(raw, "https://"), nil +} + +// issuerFromOIDCProviderARN reverses OIDCProviderARN, reconstructing the full https issuer URL. +func issuerFromOIDCProviderARN(providerARN string) (string, error) { + hostPath, err := OIDCProviderHostPathFromARN(providerARN) + if err != nil { + return "", err + } return "https://" + hostPath, nil } // validateOIDCProviderURL enforces the issuer shape: a parseable https URL with a non-empty host. func validateOIDCProviderURL(raw string) error { - if raw == "" { - return errors.New("url is required") - } - u, err := url.Parse(raw) - if err != nil { - return fmt.Errorf("parse url: %w", err) - } - if u.Scheme != "https" { - return fmt.Errorf("url scheme must be https, got %q", u.Scheme) - } - if u.Host == "" { - return errors.New("url missing host") - } - return nil + _, err := OIDCProviderHostPathFromURL(raw) + return err } func awsStrings(in []*string) []string { @@ -135,7 +153,8 @@ func (s *IAMServiceImpl) CreateOpenIDConnectProvider(accountID string, input *ia return nil, fmt.Errorf("store OIDC provider: %w", err) } - arn := OIDCProviderARN(accountID, strings.TrimPrefix(issuer, "https://")) + issuerHostPath, _ := OIDCProviderHostPathFromURL(issuer) + arn := OIDCProviderARN(accountID, issuerHostPath) slog.Info("IAM OIDC provider created", "accountID", accountID, "url", issuer, "arn", arn) return &iam.CreateOpenIDConnectProviderOutput{ OpenIDConnectProviderArn: aws.String(arn), diff --git a/spinifex/handlers/iam/provision.go b/spinifex/handlers/iam/provision.go index d9aef893a..2afbc0fca 100644 --- a/spinifex/handlers/iam/provision.go +++ b/spinifex/handlers/iam/provision.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" ) @@ -70,7 +71,7 @@ func ProvisionAccount(svc IAMService, accountID, name string) (*ProvisionedAccou return nil, fmt.Errorf("create admin policy: %w", err) } - policyARN := fmt.Sprintf("arn:aws:iam::%s:policy/%s", accountID, AdminPolicyName) + policyARN := arn.FormatIAMPath(arn.IAMPolicy, accountID, "/", AdminPolicyName) if _, err := svc.AttachUserPolicy(accountID, &iam.AttachUserPolicyInput{ UserName: aws.String(AdminUserName), PolicyArn: aws.String(policyARN), diff --git a/spinifex/handlers/iam/roles.go b/spinifex/handlers/iam/roles.go index a39e298f4..bf9802ddc 100644 --- a/spinifex/handlers/iam/roles.go +++ b/spinifex/handlers/iam/roles.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/service/iam" "github.com/nats-io/nats.go/jetstream" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/kvutil" "github.com/mulgadc/spinifex/spinifex/utils" @@ -72,7 +73,7 @@ func (s *IAMServiceImpl) CreateRole(accountID string, input *iam.CreateRoleInput RoleName: roleName, RoleID: roleID, AccountID: accountID, - ARN: fmt.Sprintf("arn:aws:iam::%s:role%s%s", accountID, path, roleName), + ARN: arn.FormatIAMPath(arn.IAMRole, accountID, path, roleName), Path: path, Description: aws.StringValue(input.Description), AssumeRolePolicyDocument: *input.AssumeRolePolicyDocument, diff --git a/spinifex/handlers/iam/service.go b/spinifex/handlers/iam/service.go index ef2cdc5e9..47cc2f34e 100644 --- a/spinifex/handlers/iam/service.go +++ b/spinifex/handlers/iam/service.go @@ -2,6 +2,7 @@ package handlers_iam import ( "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" ) // IAMService defines the interface for IAM operations. @@ -118,6 +119,10 @@ type IAMService interface { // check; the gateway also enforces this and returns AccessDenied. ResolveInstanceProfile(accountID, nameOrARN string) (*InstanceProfile, error) + // CanonicalResourceARN resolves a name-addressed IAM object to the ARN + // persisted on its record, preserving the object's full path. + CanonicalResourceARN(accountID string, kind arn.IAMResourceType, name string) (string, error) + // Policy evaluation (internal — used by gateway enforcement) GetUserPolicies(accountID, userName string) ([]PolicyDocument, error) // GetRolePolicies resolves an assumed-role session's permission policies for diff --git a/spinifex/handlers/iam/service_impl.go b/spinifex/handlers/iam/service_impl.go index 6e40baaad..260073bb2 100644 --- a/spinifex/handlers/iam/service_impl.go +++ b/spinifex/handlers/iam/service_impl.go @@ -23,6 +23,7 @@ import ( "github.com/mulgadc/bluebottle/pkg/iampolicy" "github.com/mulgadc/bluebottle/pkg/masterkey" "github.com/mulgadc/spinifex/spinifex/admin" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/kvutil" "github.com/mulgadc/spinifex/spinifex/migrate" @@ -339,7 +340,7 @@ func (s *IAMServiceImpl) CreateUser(accountID string, input *iam.CreateUserInput UserName: userName, UserID: userID, AccountID: accountID, - ARN: fmt.Sprintf("arn:aws:iam::%s:user%s%s", accountID, path, userName), + ARN: arn.FormatIAMPath(arn.IAMUser, accountID, path, userName), Path: path, CreatedAt: time.Now().UTC().Format(time.RFC3339), AccessKeys: []string{}, @@ -836,7 +837,7 @@ func (s *IAMServiceImpl) seedAdminAccount(ctx context.Context, admin *AdminBoots } // Create AdministratorAccess policy - policyARN := fmt.Sprintf("arn:aws:iam::%s:policy/AdministratorAccess", admin.AccountID) + policyARN := arn.FormatIAMPath(arn.IAMPolicy, admin.AccountID, "/", "AdministratorAccess") policyDoc := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}` policyID, err := generateIAMID("ANPA") if err != nil { @@ -872,7 +873,7 @@ func (s *IAMServiceImpl) seedAdminAccount(ctx context.Context, admin *AdminBoots UserName: admin.UserName, UserID: adminUserID, AccountID: admin.AccountID, - ARN: fmt.Sprintf("arn:aws:iam::%s:user/%s", admin.AccountID, admin.UserName), + ARN: arn.FormatIAMPath(arn.IAMUser, admin.AccountID, "/", admin.UserName), Path: "/", CreatedAt: now, AccessKeys: []string{admin.AccessKeyID}, @@ -1191,7 +1192,7 @@ func (s *IAMServiceImpl) CreatePolicy(accountID string, input *iam.CreatePolicyI policy := Policy{ PolicyName: policyName, PolicyID: newPolicyID, - ARN: fmt.Sprintf("arn:aws:iam::%s:policy%s%s", accountID, path, policyName), + ARN: arn.FormatIAMPath(arn.IAMPolicy, accountID, path, policyName), Path: path, Description: aws.StringValue(input.Description), PolicyDocument: *input.PolicyDocument,