diff --git a/spinifex/arn/eks.go b/spinifex/arn/eks.go index 1bab60735..6f21b52d3 100644 --- a/spinifex/arn/eks.go +++ b/spinifex/arn/eks.go @@ -1,6 +1,11 @@ package arn -import "fmt" +import ( + "crypto/sha256" + "fmt" + "strings" + "uuid" +) // EKSResourceType is the resource-type segment of an arn:aws:eks ARN. type EKSResourceType string @@ -24,6 +29,24 @@ func FormatEKSNodegroup(region, accountID, cluster, name, discriminator string) return fmt.Sprintf("arn:aws:eks:%s:%s:%s/%s/%s/%s", region, accountID, EKSNodegroup, cluster, name, discriminator) } +// Scopes the digest to this derivation so the value cannot collide with one +// derived elsewhere from the same identifiers. +const eksNodegroupNamespace = "mulga:eks:nodegroup" + +// EKSNodegroupDiscriminator derives a nodegroup ARN's trailing segment from the +// tuple that already identifies the nodegroup. AWS puts a random UUID there, +// which only the stored record knows; deriving it lets the policy gate name the +// exact stored ARN without reading the record. +func EKSNodegroupDiscriminator(accountID, cluster, name string) string { + // NUL-separated so ("a", "bc") and ("ab", "c") cannot digest alike. + sum := sha256.Sum256([]byte(strings.Join([]string{eksNodegroupNamespace, accountID, cluster, name}, "\x00"))) + var u uuid.UUID + copy(u[:], sum[:]) + u[6] = u[6]&0x0f | 0x80 // RFC 9562 version 8: custom, digest-derived. + u[8] = u[8]&0x3f | 0x80 // RFC 9562 variant 10. + return u.String() +} + // FormatEKSAddon builds arn:aws:eks:::addon//. func FormatEKSAddon(region, accountID, cluster, name string) string { return fmt.Sprintf("arn:aws:eks:%s:%s:%s/%s/%s", region, accountID, EKSAddon, cluster, name) diff --git a/spinifex/arn/eks_test.go b/spinifex/arn/eks_test.go index 80fb84bbb..914a8147a 100644 --- a/spinifex/arn/eks_test.go +++ b/spinifex/arn/eks_test.go @@ -2,9 +2,11 @@ package arn_test import ( "testing" + "uuid" "github.com/mulgadc/spinifex/spinifex/arn" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestFormatEKS(t *testing.T) { @@ -25,6 +27,32 @@ func TestFormatEKS(t *testing.T) { arn.FormatEKSResource(region, account, "cluster/prod")) } +// The policy gate rebuilds a nodegroup ARN from identifiers alone and expects +// byte equality with the one the create path persisted, so the derivation has +// to be stable, sensitive to every input, and shaped like the UUID AWS emits. +func TestEKSNodegroupDiscriminator(t *testing.T) { + const ( + account = "123456789012" + cluster = "prod" + name = "workers" + ) + got := arn.EKSNodegroupDiscriminator(account, cluster, name) + + assert.Equal(t, got, arn.EKSNodegroupDiscriminator(account, cluster, name)) + assert.NotEqual(t, got, arn.EKSNodegroupDiscriminator("999988887777", cluster, name)) + assert.NotEqual(t, got, arn.EKSNodegroupDiscriminator(account, "dev", name)) + assert.NotEqual(t, got, arn.EKSNodegroupDiscriminator(account, cluster, "batch")) + + // Concatenating the inputs without a separator would collide these two. + assert.NotEqual(t, + arn.EKSNodegroupDiscriminator(account, "prod", "workers"), + arn.EKSNodegroupDiscriminator(account, "prodwork", "ers")) + + parsed, err := uuid.Parse(got) + require.NoError(t, err) + assert.Equal(t, got, parsed.String()) +} + // A name that needs percent-decoding in a URL path is placed in the ARN // decoded: the gate must name the object the handler acts on. func TestFormatEKSPreservesDecodedNames(t *testing.T) { diff --git a/spinifex/gateway/eks/authz.go b/spinifex/gateway/eks/authz.go index c010de189..05f915cf6 100644 --- a/spinifex/gateway/eks/authz.go +++ b/spinifex/gateway/eks/authz.go @@ -20,11 +20,6 @@ import ( // in particular, or when the identifier cannot be resolved at gate time. const anyResource = "*" -// A nodegroup ARN ends in a UUID the gate cannot see without a describe round -// trip, and (cluster, name) already identifies the nodegroup here. The literal -// "*" is a value, so it matches the AWS-documented spelling without widening. -const anyDiscriminator = "*" - // Where an action's resource is named. Path parameters come from the route // regex; body sources are read from the same JSON the handler unmarshals. type resourceSource uint8 @@ -227,7 +222,10 @@ func nodegroupARN(region, accountID, cluster, nodegroup string) string { if region == "" || accountID == "" || cluster == "" || nodegroup == "" { return anyResource } - return arn.FormatEKSNodegroup(region, accountID, cluster, nodegroup, anyDiscriminator) + // The nodegroup ARN's trailing segment is derived from these same + // identifiers at create time, so this is the ARN on the record, exactly. + return arn.FormatEKSNodegroup(region, accountID, cluster, nodegroup, + arn.EKSNodegroupDiscriminator(accountID, cluster, nodegroup)) } func addonARN(region, accountID, cluster, addon string) string { @@ -246,8 +244,8 @@ func accessEntryARN(region, accountID, cluster, principalARN string) string { // tagARN re-anchors the caller-supplied resource ARN on gw.Region and the // caller's account, because the tag handler ignores those segments and operates -// in the caller's own account bucket. A nodegroup's UUID is dropped for the -// same reason: the handler keys off (cluster, nodegroup) alone. +// in the caller's own account bucket. A nodegroup's discriminator is re-derived +// for the same reason: the handler keys off (cluster, nodegroup) alone. func tagARN(region, accountID, resourceARN string) string { kind, resource, ok := splitEKSARN(resourceARN) if !ok || region == "" || accountID == "" { diff --git a/spinifex/gateway/eks/authz_test.go b/spinifex/gateway/eks/authz_test.go index 68c676b58..ad12beea0 100644 --- a/spinifex/gateway/eks/authz_test.go +++ b/spinifex/gateway/eks/authz_test.go @@ -3,6 +3,7 @@ package gateway_eks_test import ( "testing" + "github.com/mulgadc/spinifex/spinifex/arn" gateway_eks "github.com/mulgadc/spinifex/spinifex/gateway/eks" handlers_eks "github.com/mulgadc/spinifex/spinifex/handlers/eks" "github.com/stretchr/testify/assert" @@ -21,6 +22,13 @@ func resolveARNs(t *testing.T, action string, params []string, body string) []st return resources } +// The nodegroup ARN the create path persists for the same identifiers. Spelled +// through the shared builders so the test cannot pass by agreeing with itself. +func nodegroupARN(cluster, name string) string { + return arn.FormatEKSNodegroup(authzRegion, authzAccountID, cluster, name, + arn.EKSNodegroupDiscriminator(authzAccountID, cluster, name)) +} + // One assertion per resource class: the ARN handed to the evaluator must name // the object the handler acts on, not a plausible-looking neighbour. func TestResourceARNsFidelity(t *testing.T) { @@ -43,7 +51,7 @@ func TestResourceARNsFidelity(t *testing.T) { { action: "DeleteNodegroup", params: []string{"prod", "workers"}, - want: []string{"arn:aws:eks:ap-southeast-2:123456789012:nodegroup/prod/workers/*"}, + want: []string{nodegroupARN("prod", "workers")}, }, { action: "CreateNodegroup", @@ -51,7 +59,7 @@ func TestResourceARNsFidelity(t *testing.T) { body: `{"nodegroupName":"workers"}`, want: []string{ "arn:aws:eks:ap-southeast-2:123456789012:cluster/prod", - "arn:aws:eks:ap-southeast-2:123456789012:nodegroup/prod/workers/*", + nodegroupARN("prod", "workers"), }, }, { @@ -133,9 +141,9 @@ func TestResourceARNsTagARNIsReanchored(t *testing.T) { want: []string{"arn:aws:eks:ap-southeast-2:123456789012:cluster/prod"}, }, { - name: "nodegroup uuid is dropped, as the handler drops it", + name: "nodegroup discriminator is re-derived, as the handler ignores it", resourceARN: "arn:aws:eks:ap-southeast-2:123456789012:nodegroup/prod/workers/8ac0f0b1", - want: []string{"arn:aws:eks:ap-southeast-2:123456789012:nodegroup/prod/workers/*"}, + want: []string{nodegroupARN("prod", "workers")}, }, { name: "addon", diff --git a/spinifex/gateway/eks_authz_test.go b/spinifex/gateway/eks_authz_test.go index 92c3dbe64..3f30c5178 100644 --- a/spinifex/gateway/eks_authz_test.go +++ b/spinifex/gateway/eks_authz_test.go @@ -62,8 +62,26 @@ func TestEKSRequest_ScopedAllowGrants(t *testing.T) { assertDenied(t, dispatchEKS(t, gw, http.MethodDelete, "/clusters/prod", "")) } -// A nodegroup ARN carries a UUID the gate cannot see, so the resolver leaves it -// wildcarded. The AWS-documented policy spelling must still match. +// The bypass this work closes. A tenant pastes the ARN out of DescribeNodegroup +// and fences it; before the discriminator was derived the gate evaluated a +// wildcarded spelling that this exact-ARN pattern could not match, so the fence +// was inert and the delete went through. +func TestEKSRequest_NodegroupScopeMatchesExactStoredARN(t *testing.T) { + stored := arn.FormatEKSNodegroup(authzRegion, authzAccountID, "prod", "workers", + arn.EKSNodegroupDiscriminator(authzAccountID, "prod", "workers")) + + gw := scopedPolicyGateway( + statement("Allow", "eks:*", "*"), + statement("Deny", "eks:DeleteNodegroup", stored), + ) + + assertDenied(t, dispatchEKS(t, gw, http.MethodDelete, "/clusters/prod/node-groups/workers", "")) + assertPermitted(t, dispatchEKS(t, gw, http.MethodDelete, "/clusters/prod/node-groups/batch", "")) + assertPermitted(t, dispatchEKS(t, gw, http.MethodDelete, "/clusters/dev/node-groups/workers", "")) +} + +// The AWS-documented policy spelling wildcards the discriminator, and was the +// only spelling that worked before. It must keep working. func TestEKSRequest_NodegroupScopeMatchesWildcardUUID(t *testing.T) { gw := scopedPolicyGateway( statement("Allow", "eks:*", "*"), diff --git a/spinifex/handlers/eks/nodegroup.go b/spinifex/handlers/eks/nodegroup.go index 93d767e5d..e5baf8f95 100644 --- a/spinifex/handlers/eks/nodegroup.go +++ b/spinifex/handlers/eks/nodegroup.go @@ -273,9 +273,11 @@ func (s *EKSServiceImpl) createNodegroup(ctx context.Context, acctKV jetstream.K now := time.Now().UTC() rec := &NodegroupRecord{ - ClusterName: cluster, - Name: ng, - Arn: arn.FormatEKSNodegroup(s.deps.Region, accountID, cluster, ng, uuid.NewV4().String()), + ClusterName: cluster, + Name: ng, + // Derived, not random: the policy gate builds the same ARN from the same + // identifiers, so a Deny naming this exact ARN fires without a lookup. + Arn: arn.FormatEKSNodegroup(s.deps.Region, accountID, cluster, ng, arn.EKSNodegroupDiscriminator(accountID, cluster, ng)), Status: eks.NodegroupStatusCreating, Subnets: subnets, InstanceTypes: instanceTypes, diff --git a/spinifex/handlers/eks/nodegroup_test.go b/spinifex/handlers/eks/nodegroup_test.go index 4f28ec4e6..4a4b961d1 100644 --- a/spinifex/handlers/eks/nodegroup_test.go +++ b/spinifex/handlers/eks/nodegroup_test.go @@ -13,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/eks" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" "github.com/mulgadc/spinifex/spinifex/tags" @@ -339,7 +340,12 @@ func TestCreateNodegroup_HappyPath(t *testing.T) { // The create accepts the request as CREATING; worker launch runs async. assert.Equal(t, eks.NodegroupStatusCreating, aws.StringValue(out.Nodegroup.Status)) assert.Equal(t, int64(2), aws.Int64Value(out.Nodegroup.ScalingConfig.DesiredSize)) - assert.Contains(t, aws.StringValue(out.Nodegroup.NodegroupArn), ":nodegroup/c1/ng1/") + // Full equality, not a prefix: the policy gate rebuilds this ARN from the + // identifiers alone, and a Deny naming it only fires if it matches exactly. + assert.Equal(t, + arn.FormatEKSNodegroup(f.svc.deps.Region, testAccountID, "c1", "ng1", + arn.EKSNodegroupDiscriminator(testAccountID, "c1", "ng1")), + aws.StringValue(out.Nodegroup.NodegroupArn)) // Both workers register Ready → the Ready-gate lets the nodegroup go ACTIVE. markWorkersReady(t, f, "c1", "ng1", 2)