From 74243730fa0f3e1eac59fe1deb15176dff538fad Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:13:30 +1000 Subject: [PATCH] fix(iam): resolve policy and instance-profile ARNs against the stored ARN (#1144) --- go.mod | 2 +- go.sum | 4 +- spinifex/handlers/iam/instance_profiles.go | 37 +++++++++------- .../handlers/iam/instance_profiles_test.go | 23 ++++++++++ spinifex/handlers/iam/service_impl.go | 44 ++++++++++--------- 5 files changed, 69 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index d096d2cf1..087e2eafa 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/klauspost/cpuid/v2 v2.4.0 github.com/miekg/dns v1.1.73 github.com/minio/highwayhash v1.0.4 - github.com/mulgadc/bluebottle v1.21.1-0.20261001004744-156519746c8a + github.com/mulgadc/bluebottle v1.21.1-0.20261001011202-4904308df7ef github.com/mulgadc/northstar v1.21.0 github.com/mulgadc/predastore v1.21.0 github.com/mulgadc/viperblock v1.21.0 diff --git a/go.sum b/go.sum index 6f845e1fb..2603c8c85 100644 --- a/go.sum +++ b/go.sum @@ -312,8 +312,8 @@ github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELU github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= -github.com/mulgadc/bluebottle v1.21.1-0.20261001004744-156519746c8a h1:Qoycvq0OWPk+KpN4Gb+dG3plkWx4AGGJOJWz0ZJsuSM= -github.com/mulgadc/bluebottle v1.21.1-0.20261001004744-156519746c8a/go.mod h1:LH0MuQcFxet0dDEISlZsuULIYj592eCN2Fv39A6EH5o= +github.com/mulgadc/bluebottle v1.21.1-0.20261001011202-4904308df7ef h1:7xjmp8lTRhxnZqviKV0tDbNcCnBwp6PGr6eNJm+ZDYs= +github.com/mulgadc/bluebottle v1.21.1-0.20261001011202-4904308df7ef/go.mod h1:LH0MuQcFxet0dDEISlZsuULIYj592eCN2Fv39A6EH5o= github.com/mulgadc/northstar v1.21.0 h1:LE/5kiT3kBelCg02cXa3b7awhaYkGHsOXeJ4nHRLNaQ= github.com/mulgadc/northstar v1.21.0/go.mod h1:O446cApNMB6qebqieBhDZfCIGVWX++v3QdOz+wIGhUk= github.com/mulgadc/predastore v1.21.0 h1:pAm4fFWf0vnlwRgnoL2oJ+KSCw8mtjT2rQoBHFmnjzk= diff --git a/spinifex/handlers/iam/instance_profiles.go b/spinifex/handlers/iam/instance_profiles.go index 0f69a5200..6ba739625 100644 --- a/spinifex/handlers/iam/instance_profiles.go +++ b/spinifex/handlers/iam/instance_profiles.go @@ -266,25 +266,28 @@ func (s *IAMServiceImpl) ResolveInstanceProfile(accountID, nameOrARN string) (*I return s.getInstanceProfile(ctx, accountID, nameOrARN) } - profileAccountID, profileName, err := parseInstanceProfileARN(nameOrARN) - if err != nil { + var profile *InstanceProfile + _, _, err := auth.ResolveInstanceProfileARN(nameOrARN, func(profileAccountID, profileName string) (string, error) { + if profileAccountID != accountID { + return "", errors.New(awserrors.ErrorAccessDenied) + } + p, err := s.getInstanceProfile(ctx, accountID, profileName) + if err != nil { + return "", err + } + profile = p + return p.ARN, nil + }) + switch { + case errors.Is(err, auth.ErrInvalidInstanceProfileARN): + return nil, errors.New(awserrors.ErrorInvalidIamInstanceProfileArnMalformed) + case errors.Is(err, auth.ErrInstanceProfileARNMismatch): + // A non-canonical spelling names no stored profile. + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + case err != nil: return nil, err } - if profileAccountID != accountID { - return nil, errors.New(awserrors.ErrorAccessDenied) - } - return s.getInstanceProfile(ctx, accountID, profileName) -} - -// parseInstanceProfileARN extracts accountID and profile name from an IAM -// instance-profile ARN, reporting every malformed shape as the one AWS error -// code this API returns. -func parseInstanceProfileARN(arnStr string) (accountID, name string, err error) { - accountID, name, err = auth.ParseInstanceProfileARN(arnStr) - if err != nil { - return "", "", errors.New(awserrors.ErrorInvalidIamInstanceProfileArnMalformed) - } - return accountID, name, nil + return profile, nil } // TagInstanceProfile upserts tags on an instance profile under CAS, like the diff --git a/spinifex/handlers/iam/instance_profiles_test.go b/spinifex/handlers/iam/instance_profiles_test.go index 367d81673..f93999bea 100644 --- a/spinifex/handlers/iam/instance_profiles_test.go +++ b/spinifex/handlers/iam/instance_profiles_test.go @@ -686,6 +686,29 @@ func TestResolveInstanceProfile_ARNNotFound(t *testing.T) { assert.Contains(t, err.Error(), awserrors.ErrorIAMNoSuchEntity) } +// The store is keyed by name alone, so an ARN that spells a different path +// must not resolve to the profile its trailing name matches. +func TestResolveInstanceProfile_NonCanonicalARNRejected(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + createTestInstanceProfile(t, svc, "flat-profile") + _, err := svc.CreateInstanceProfile(testAccountID, &iam.CreateInstanceProfileInput{ + InstanceProfileName: aws.String("pathed-profile"), + Path: aws.String("/team/"), + }) + require.NoError(t, err) + + for _, arn := range []string{ + "arn:aws:iam::" + testAccountID + ":instance-profile/decoy/flat-profile", + "arn:aws:iam::" + testAccountID + ":instance-profile/pathed-profile", + } { + _, err := svc.ResolveInstanceProfile(testAccountID, arn) + require.Error(t, err, arn) + // The EC2 gateway maps this to InvalidIamInstanceProfile.NotFound by exact match. + assert.Equal(t, awserrors.ErrorIAMNoSuchEntity, err.Error(), arn) + } +} + func TestResolveInstanceProfile_MalformedARN(t *testing.T) { t.Parallel() svc := setupTestIAMService(t) diff --git a/spinifex/handlers/iam/service_impl.go b/spinifex/handlers/iam/service_impl.go index 5d604e0d4..e98d6a44d 100644 --- a/spinifex/handlers/iam/service_impl.go +++ b/spinifex/handlers/iam/service_impl.go @@ -2016,34 +2016,36 @@ func (s *IAMServiceImpl) GetUserPolicies(accountID, userName string) ([]PolicyDo // --------------------------------------------------------------------------- func (s *IAMServiceImpl) getPolicyByARN(ctx context.Context, accountID, policyARN string) (*Policy, error) { - _, policyName, err := iamarn.ParsePolicyARN(policyARN) - if err != nil { + var policy *Policy + _, _, err := iamarn.ResolvePolicyARN(policyARN, func(_, policyName string) (string, error) { + kvKey := accountID + "." + policyName + entry, err := s.policiesBucket.Get(ctx, kvKey) + if err != nil { + if errors.Is(err, jetstream.ErrKeyNotFound) { + return "", errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return "", fmt.Errorf("get policy: %w", err) + } + var p Policy + if err := json.Unmarshal(entry.Value(), &p); err != nil { + return "", fmt.Errorf("unmarshal policy: %w", err) + } + policy = &p + return p.ARN, nil + }) + switch { + case errors.Is(err, iamarn.ErrInvalidPolicyARN): // NoSuchEntity is the response contract, so log the parse reason — // otherwise a malformed ARN is indistinguishable from a missing policy. slog.Debug("getPolicyByARN: unparseable policy ARN", "accountID", accountID, "policyArn", policyARN, "err", err) return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) - } - - kvKey := accountID + "." + policyName - entry, err := s.policiesBucket.Get(ctx, kvKey) - 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) - } - - if policy.ARN != policyARN { + case errors.Is(err, iamarn.ErrPolicyARNMismatch): return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + case err != nil: + return nil, err } - - return &policy, nil + return policy, nil } // entityRef identifies one principal record found to have a given policy