From db97c90a9a2a945f39fc71eab4b595c13c90a1f2 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 1 Sep 2026 14:51:42 +1000 Subject: [PATCH] fix: bind the EKS internal control-plane routes to the calling CP VM (#970) --- spinifex/gateway/eks.go | 31 ++- spinifex/gateway/eks/internal_addons.go | 3 +- spinifex/gateway/eks/internal_authz.go | 129 ++++++++++++ spinifex/gateway/eks/internal_authz_test.go | 189 ++++++++++++++++++ spinifex/gateway/eks/internal_recovery.go | 5 +- spinifex/gateway/eks_authz_test.go | 65 ++++++ spinifex/handlers/eks/cluster_state.go | 15 ++ spinifex/handlers/eks/k3s_server_role.go | 24 ++- spinifex/handlers/eks/k3s_server_role_test.go | 18 +- 9 files changed, 456 insertions(+), 23 deletions(-) create mode 100644 spinifex/gateway/eks/internal_authz.go create mode 100644 spinifex/gateway/eks/internal_authz_test.go diff --git a/spinifex/gateway/eks.go b/spinifex/gateway/eks.go index a53534193..04ba3f522 100644 --- a/spinifex/gateway/eks.go +++ b/spinifex/gateway/eks.go @@ -9,6 +9,7 @@ import ( "net/url" "regexp" + "github.com/mulgadc/bluebottle/pkg/auth" "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_eks "github.com/mulgadc/spinifex/spinifex/gateway/eks" ) @@ -67,7 +68,8 @@ var eksRoutes = []eksRoute{ // of staged add-on manifests for its cluster (system SigV4 creds) to render // the baked bundles into the K3s auto-deploy dir. acct (system account) is // ignored — the cluster account is the {accountId} path segment, since a GET - // carries no body to hold it (cf. PublishInternal). + // carries no body to hold it (cf. PublishInternal). AuthorizeInternal binds + // that segment to the caller's own cluster. {"GET", regexp.MustCompile(`^/clusters/([^/]+)/internal-addons/([^/]+)$`), "ListInternalAddons", func(ctx context.Context, gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { return gateway_eks.ListInternalAddons(ctx, gw.NATSConn, p[0], p[1]) @@ -265,6 +267,15 @@ func (gw *GatewayConfig) EKS_Request(w http.ResponseWriter, r *http.Request) err return errors.New(awserrors.ErrorInternalError) } + // Ahead of the policy check: the internal routes name the target account in + // the path, so an eks:* grant evaluates as permitted and only the principal + // class plus the caller's own instance say whether that account is its own. + if gateway_eks.IsInternalAction(action) { + if err := gateway_eks.AuthorizeInternal(r.Context(), gw.NATSConn, action, eksCaller(r), params); err != nil { + return err + } + } + body, err := readBoundedBody(r) if err != nil { slog.ErrorContext(r.Context(), "EKS_Request: failed to read body", "err", err) @@ -311,6 +322,24 @@ func (gw *GatewayConfig) EKS_Request(w http.ResponseWriter, r *http.Request) err return nil } +// eksCaller reads the principal behind the request. The role name comes from the +// underlying role ARN, never the session name, which the caller picks at +// AssumeRole time and could name its way past the gate. +func eksCaller(r *http.Request) gateway_eks.Caller { + accountID := mustCtxString(r, ctxAccountID) + caller := gateway_eks.Caller{ + AccountID: accountID, + PrincipalType: mustCtxString(r, ctxPrincipalType), + SessionName: mustCtxString(r, ctxIdentity), + } + if roleARN := mustCtxString(r, ctxUnderlyingRoleARN); roleARN != "" { + if roleAcct, roleName, err := auth.ParseRoleARN(roleARN); err == nil && roleAcct == accountID { + caller.RoleName = roleName + } + } + return caller +} + // eksCallerPrincipalARN resolves the caller's IAM principal ARN from the SigV4 // auth context. Returns "" when the ARN can't be composed; CreateCluster then // skips the creator-admin AccessEntry rather than failing. diff --git a/spinifex/gateway/eks/internal_addons.go b/spinifex/gateway/eks/internal_addons.go index ec8c8881e..473b8d2c4 100644 --- a/spinifex/gateway/eks/internal_addons.go +++ b/spinifex/gateway/eks/internal_addons.go @@ -18,7 +18,8 @@ type internalAddonsOutput struct { // ListInternalAddons — GET /clusters/{name}/internal-addons?accountId={acct}. // Internal control-plane VM route (not an AWS-SDK action): the CP VM holds // system SigV4 creds, so accountID names the customer cluster account explicitly -// — same carve-out as PublishInternal. Returns every staged add-on manifest so +// — same carve-out as PublishInternal, gated by AuthorizeInternal, which admits +// only a CP agent serving that account's cluster. Returns every staged manifest so // the VM can render the baked bundles into the K3s auto-deploy dir and GC the // locally-rendered manifests for add-ons no longer staged. func ListInternalAddons(ctx context.Context, natsConn *nats.Conn, clusterName, accountID string) (*internalAddonsOutput, error) { diff --git a/spinifex/gateway/eks/internal_authz.go b/spinifex/gateway/eks/internal_authz.go new file mode 100644 index 000000000..d760443f4 --- /dev/null +++ b/spinifex/gateway/eks/internal_authz.go @@ -0,0 +1,129 @@ +package gateway_eks + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "slices" + + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_eks "github.com/mulgadc/spinifex/spinifex/handlers/eks" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" +) + +// Only an assumed-role session can be a control-plane VM. +const principalTypeAssumedRole = "assumed-role" + +// Caller is the principal behind an EKS request. The internal routes have to +// tell the CP VM's instance role apart from a user, which the account alone +// cannot do, and then tell one VM from another. +type Caller struct { + AccountID string + PrincipalType string + RoleName string + // The RoleSessionName of an assumed-role session. For IMDS instance-role + // credentials it is the internal EC2 instance ID. + SessionName string +} + +// IsInternalAction reports whether action is one of the internal CP-VM routes: +// the ones naming the customer account in the path, which no customer grant may +// reach. Derived from eksScopes so a new such route cannot ship ungated — that +// table is exhaustive by contract against the dispatch table. +func IsInternalAction(action string) bool { + return slices.Contains(eksScopes[action], sourceInternalCluster) +} + +// AuthorizeInternal is the principal gate for the internal CP-VM routes, run +// ahead of the policy check. params are the route captures: cluster name, +// account ID, and for GetRecoveryDirective the member's instance ID. +func AuthorizeInternal(ctx context.Context, natsConn *nats.Conn, action string, caller Caller, params []string) error { + if !IsInternalAction(action) { + return nil + } + if err := requireCPAgent(ctx, action, caller); err != nil { + return err + } + + clusterName, accountID := param(params, 0), param(params, 1) + if clusterName == "" || accountID == "" { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + + // A member reads its own directive and no other's: the path segment is only + // ever used to reject a mismatch, since the caller's own instance ID is the + // one the credentials prove. + if action == "GetRecoveryDirective" && param(params, 2) != caller.SessionName { + slog.WarnContext(ctx, "EKS: CP agent asked for another member's recovery directive", + "callerInstanceID", caller.SessionName, "requested", param(params, 2)) + return errors.New(awserrors.ErrorAccessDenied) + } + + meta, err := lookupClusterMeta(ctx, natsConn, accountID, clusterName) + if err != nil { + slog.ErrorContext(ctx, "EKS: internal route cluster-meta lookup failed", + "action", action, "accountID", accountID, "cluster", clusterName, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + // An absent cluster and a cluster the caller does not serve are the same + // denial: either way this caller is not a member of what it named. + if !meta.IsControlPlaneMember(caller.SessionName) { + slog.WarnContext(ctx, "EKS: internal route named a cluster the caller does not serve", + "action", action, "callerInstanceID", caller.SessionName, "accountID", accountID, "cluster", clusterName) + return errors.New(awserrors.ErrorAccessDenied) + } + return nil +} + +// The class check on its own: a session assumed from the CP VM's instance role +// in the system account. It says the caller is a control-plane VM, not which +// one — the binding to a cluster is AuthorizeInternal's, and needs NATS. +func requireCPAgent(ctx context.Context, action string, caller Caller) error { + if caller.PrincipalType != principalTypeAssumedRole || + caller.AccountID != utils.GlobalAccountID || + caller.RoleName != handlers_eks.CPInstanceRoleName || + caller.SessionName == "" { + slog.WarnContext(ctx, "EKS: internal route rejected for non-CP-agent caller", + "action", action, "principalType", caller.PrincipalType, + "accountID", caller.AccountID, "roleName", caller.RoleName) + return errors.New(awserrors.ErrorAccessDenied) + } + return nil +} + +// Reads JetStream directly, matching the OIDC discovery path, rather than a +// service round trip through the handler. js.KeyValue still costs a STREAM.INFO +// for the bucket handle. A nil meta is returned for an account with no clusters +// and for a cluster that is absent; both are denials rather than failures. +func lookupClusterMeta(ctx context.Context, natsConn *nats.Conn, accountID, clusterName string) (*handlers_eks.ClusterMeta, error) { + if natsConn == nil { + return nil, errors.New("gateway NATS connection not initialised") + } + js, err := jetstream.New(natsConn) + if err != nil { + return nil, err + } + kv, err := js.KeyValue(ctx, handlers_eks.AccountBucketName(accountID)) + if err != nil { + if errors.Is(err, jetstream.ErrBucketNotFound) { + return nil, nil + } + return nil, err + } + entry, err := kv.Get(ctx, handlers_eks.ClusterMetaKey(clusterName)) + if errors.Is(err, jetstream.ErrKeyNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + var meta handlers_eks.ClusterMeta + if err := json.Unmarshal(entry.Value(), &meta); err != nil { + return nil, fmt.Errorf("unmarshal cluster meta %s: %w", clusterName, err) + } + return &meta, nil +} diff --git a/spinifex/gateway/eks/internal_authz_test.go b/spinifex/gateway/eks/internal_authz_test.go new file mode 100644 index 000000000..1ab270870 --- /dev/null +++ b/spinifex/gateway/eks/internal_authz_test.go @@ -0,0 +1,189 @@ +package gateway_eks_test + +import ( + "testing" + + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_eks "github.com/mulgadc/spinifex/spinifex/gateway/eks" + handlers_eks "github.com/mulgadc/spinifex/spinifex/handlers/eks" + "github.com/mulgadc/spinifex/spinifex/testutil" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + // The literal STS puts in the auth context, spelled out rather than shared + // with the gate: a silent change to either side must fail a test, not agree + // with itself. + principalTypeAssumedRole = "assumed-role" + + tenantAccount = "111122223333" + cpInstanceID = "i-cp0000000000001" + otherInstance = "i-cp0000000000002" +) + +// cpAgent is the principal an IMDS-credentialed control-plane VM presents. +func cpAgent(instanceID string) gateway_eks.Caller { + return gateway_eks.Caller{ + AccountID: utils.GlobalAccountID, + PrincipalType: principalTypeAssumedRole, + RoleName: handlers_eks.CPInstanceRoleName, + SessionName: instanceID, + } +} + +// seedCluster writes a cluster meta whose control plane is the given members. +func seedCluster(t *testing.T, nc *nats.Conn, accountID, cluster string, meta *handlers_eks.ClusterMeta) { + t.Helper() + js := testutil.NewJetStream(t, nc) + kv, err := js.CreateKeyValue(t.Context(), jetstream.KeyValueConfig{ + Bucket: handlers_eks.AccountBucketName(accountID), + }) + require.NoError(t, err) + meta.Name = cluster + require.NoError(t, handlers_eks.PutClusterMeta(t.Context(), kv, meta)) +} + +func assertDenied(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) +} + +// A tenant holding eks:* names another account in the path. The class gate +// rejects it before any cluster state is read, so it holds with no NATS +// connection at all. +func TestAuthorizeInternal_RejectsNonCPPrincipals(t *testing.T) { + cases := []struct { + name string + caller gateway_eks.Caller + }{ + {"tenant user with eks:*", gateway_eks.Caller{ + AccountID: tenantAccount, PrincipalType: "user", SessionName: "alice", + }}, + {"tenant role session", gateway_eks.Caller{ + AccountID: tenantAccount, + PrincipalType: principalTypeAssumedRole, + RoleName: handlers_eks.CPInstanceRoleName, + SessionName: cpInstanceID, + }}, + {"system user, not a role session", gateway_eks.Caller{ + AccountID: utils.GlobalAccountID, PrincipalType: "user", SessionName: "root", + }}, + {"another role in the system account", gateway_eks.Caller{ + AccountID: utils.GlobalAccountID, + PrincipalType: principalTypeAssumedRole, + RoleName: "spinifex-rds-instance", + SessionName: cpInstanceID, + }}, + {"no session name", gateway_eks.Caller{ + AccountID: utils.GlobalAccountID, + PrincipalType: principalTypeAssumedRole, + RoleName: handlers_eks.CPInstanceRoleName, + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assertDenied(t, gateway_eks.AuthorizeInternal(t.Context(), nil, "ListInternalAddons", + tc.caller, []string{"alpha", tenantAccount})) + assertDenied(t, gateway_eks.AuthorizeInternal(t.Context(), nil, "GetRecoveryDirective", + tc.caller, []string{"alpha", tenantAccount, cpInstanceID})) + }) + } +} + +// Actions outside the internal set keep their own gating; this one adds none. +func TestAuthorizeInternal_IgnoresOtherActions(t *testing.T) { + assert.False(t, gateway_eks.IsInternalAction("DescribeCluster")) + require.NoError(t, gateway_eks.AuthorizeInternal(t.Context(), nil, "DescribeCluster", + gateway_eks.Caller{AccountID: tenantAccount, PrincipalType: "user"}, []string{"alpha"})) +} + +func TestAuthorizeInternal_AllowsClusterMember(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + seedCluster(t, nc, tenantAccount, "alpha", &handlers_eks.ClusterMeta{ + ControlPlaneNodes: []handlers_eks.ControlPlaneNode{ + {InstanceID: otherInstance}, {InstanceID: cpInstanceID}, + }, + }) + + require.NoError(t, gateway_eks.AuthorizeInternal(t.Context(), nc, "ListInternalAddons", + cpAgent(cpInstanceID), []string{"alpha", tenantAccount})) + require.NoError(t, gateway_eks.AuthorizeInternal(t.Context(), nc, "GetRecoveryDirective", + cpAgent(cpInstanceID), []string{"alpha", tenantAccount, cpInstanceID})) +} + +// Clusters persisted before ControlPlaneNodes existed carry the member in the +// scalar field only, and their VMs must keep reaching the routes. +func TestAuthorizeInternal_AllowsLegacyScalarMember(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + seedCluster(t, nc, tenantAccount, "alpha", &handlers_eks.ClusterMeta{ + ControlPlaneInstanceID: cpInstanceID, + }) + + require.NoError(t, gateway_eks.AuthorizeInternal(t.Context(), nc, "ListInternalAddons", + cpAgent(cpInstanceID), []string{"alpha", tenantAccount})) +} + +// A CP VM that is a genuine agent still cannot name an account or a cluster it +// does not serve, which is what the path segment previously let it do. +func TestAuthorizeInternal_RejectsClusterTheCallerDoesNotServe(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + seedCluster(t, nc, tenantAccount, "alpha", &handlers_eks.ClusterMeta{ + ControlPlaneNodes: []handlers_eks.ControlPlaneNode{{InstanceID: cpInstanceID}}, + }) + seedCluster(t, nc, "444455556666", "beta", &handlers_eks.ClusterMeta{ + ControlPlaneNodes: []handlers_eks.ControlPlaneNode{{InstanceID: otherInstance}}, + }) + + cases := []struct { + name string + account string + cluster string + }{ + {"another account's cluster", "444455556666", "beta"}, + {"an account with no clusters", "999988887777", "alpha"}, + {"a cluster that does not exist", tenantAccount, "gamma"}, + {"its own account, another cluster", tenantAccount, "beta"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assertDenied(t, gateway_eks.AuthorizeInternal(t.Context(), nc, "ListInternalAddons", + cpAgent(cpInstanceID), []string{tc.cluster, tc.account})) + }) + } +} + +// A member reads its own directive and no other member's. +func TestAuthorizeInternal_RecoveryIsSelfOnly(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + seedCluster(t, nc, tenantAccount, "alpha", &handlers_eks.ClusterMeta{ + ControlPlaneNodes: []handlers_eks.ControlPlaneNode{ + {InstanceID: cpInstanceID}, {InstanceID: otherInstance}, + }, + }) + + assertDenied(t, gateway_eks.AuthorizeInternal(t.Context(), nc, "GetRecoveryDirective", + cpAgent(cpInstanceID), []string{"alpha", tenantAccount, otherInstance})) +} + +func TestAuthorizeInternal_RejectsEmptyPathSegments(t *testing.T) { + for _, params := range [][]string{{"", tenantAccount}, {"alpha", ""}, {"alpha"}} { + err := gateway_eks.AuthorizeInternal(t.Context(), nil, "ListInternalAddons", cpAgent(cpInstanceID), params) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) + } +} + +// A CP agent whose lookup cannot run at all is an infrastructure fault, not a +// denial: reporting AccessDenied would send the on-VM agent's retry budget +// chasing a permission it already has. +func TestAuthorizeInternal_NoNATSIsServerInternal(t *testing.T) { + err := gateway_eks.AuthorizeInternal(t.Context(), nil, "ListInternalAddons", + cpAgent(cpInstanceID), []string{"alpha", tenantAccount}) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) +} diff --git a/spinifex/gateway/eks/internal_recovery.go b/spinifex/gateway/eks/internal_recovery.go index de6a2efb5..14ea8c8df 100644 --- a/spinifex/gateway/eks/internal_recovery.go +++ b/spinifex/gateway/eks/internal_recovery.go @@ -18,8 +18,9 @@ type internalRecoveryOutput struct { // GetRecoveryDirective — GET /clusters/{name}/internal-recovery/{acct}/{instanceId}. // Internal control-plane VM route (not an AWS-SDK action): the CP VM holds system // SigV4 creds, so accountID names the customer cluster account explicitly — same -// carve-out as ListInternalAddons. Returns the per-member recovery directive the -// agent applies (none, cluster-reset, or wipe-rejoin) before k3s starts. +// carve-out as ListInternalAddons, gated by AuthorizeInternal, which admits only +// the member itself. Returns the per-member recovery directive the agent applies +// (none, cluster-reset, or wipe-rejoin) before k3s starts. func GetRecoveryDirective(ctx context.Context, natsConn *nats.Conn, clusterName, accountID, instanceID string) (*internalRecoveryOutput, error) { if natsConn == nil { return nil, errors.New(awserrors.ErrorServerInternal) diff --git a/spinifex/gateway/eks_authz_test.go b/spinifex/gateway/eks_authz_test.go index 0a362eb07..92c3dbe64 100644 --- a/spinifex/gateway/eks_authz_test.go +++ b/spinifex/gateway/eks_authz_test.go @@ -15,6 +15,9 @@ import ( "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_eks "github.com/mulgadc/spinifex/spinifex/gateway/eks" handlers_eks "github.com/mulgadc/spinifex/spinifex/handlers/eks" + "github.com/mulgadc/spinifex/spinifex/testutil" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go/jetstream" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -138,6 +141,68 @@ func TestEKSRequest_AccountWideGrantStillPermitsEveryAction(t *testing.T) { } } +// The internal CP-VM routes name the target account in the path, so eks:* on * +// evaluated as permitted and returned another tenant's cluster state. The +// principal gate now rejects the tenant ahead of the policy check. +func TestEKSRequest_InternalRoutesRejectTenantPrincipal(t *testing.T) { + gw := scopedPolicyGateway(statement("Allow", "eks:*", "*")) + + assertDenied(t, dispatchEKS(t, gw, http.MethodGet, "/clusters/prod/internal-addons/999988887777", "")) + assertDenied(t, dispatchEKS(t, gw, http.MethodGet, + "/clusters/prod/internal-recovery/999988887777/i-0abc", "")) + // Its own account is no different: a tenant is not a control-plane VM. + assertDenied(t, dispatchEKS(t, gw, http.MethodGet, "/clusters/prod/internal-addons/"+authzAccountID, "")) +} + +// dispatchEKSAsCPAgent drives the gateway with the auth context a CP VM's IMDS +// credentials actually produce, so the gate is exercised through eksCaller's +// field mapping rather than a hand-built Caller. +func dispatchEKSAsCPAgent(t *testing.T, gw *GatewayConfig, path, instanceID string) error { + t.Helper() + roleName := handlers_eks.CPInstanceRoleName + req := httptest.NewRequest(http.MethodGet, path, nil) + ctx := context.WithValue(req.Context(), ctxService, "eks") + ctx = context.WithValue(ctx, ctxAccountID, utils.GlobalAccountID) + ctx = context.WithValue(ctx, ctxIdentity, instanceID) + ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeAssumedRole) + ctx = context.WithValue(ctx, ctxUnderlyingRoleARN, + "arn:aws:iam::"+utils.GlobalAccountID+":role/"+roleName) + ctx = context.WithValue(ctx, ctxAssumedRoleARN, + "arn:aws:sts::"+utils.GlobalAccountID+":assumed-role/"+roleName+"/"+instanceID) + return gw.EKS_Request(httptest.NewRecorder(), req.WithContext(ctx)) +} + +// The denial tests above all fail on the first clause of the class gate, so on +// their own they would still pass if eksCaller stopped producing a CP agent at +// all — which would deny every control-plane VM in production. This is the +// other half: the real IMDS-shaped context reaches the handler. +func TestEKSRequest_InternalRoutesAdmitTheCPAgentPrincipal(t *testing.T) { + const cpInstanceID = "i-cp0000000000001" + _, nc, js := testutil.StartTestJetStream(t) + kv, err := js.CreateKeyValue(t.Context(), jetstream.KeyValueConfig{ + Bucket: handlers_eks.AccountBucketName(authzAccountID), + }) + require.NoError(t, err) + require.NoError(t, handlers_eks.PutClusterMeta(t.Context(), kv, &handlers_eks.ClusterMeta{ + Name: "prod", + ControlPlaneNodes: []handlers_eks.ControlPlaneNode{{InstanceID: cpInstanceID}}, + })) + + gw := scopedPolicyGateway(statement("Allow", "eks:*", "*")) + gw.NATSConn = nc + + // No eks daemon is subscribed, so the handler fails on the absent responder. + // Anything but AccessDenied means the gate and the policy check both passed. + assertNotDenied(t, dispatchEKSAsCPAgent(t, gw, + "/clusters/prod/internal-addons/"+authzAccountID, cpInstanceID)) + assertNotDenied(t, dispatchEKSAsCPAgent(t, gw, + "/clusters/prod/internal-recovery/"+authzAccountID+"/"+cpInstanceID, cpInstanceID)) + + // Same credentials, a cluster this VM does not serve: the binding still bites. + assertDenied(t, dispatchEKSAsCPAgent(t, gw, + "/clusters/other/internal-addons/"+authzAccountID, cpInstanceID)) +} + // The account-ID read moved above the gate, which used to reject a missing // account itself. InternalError is the code the caller has always seen. func TestEKSRequest_MissingAccountIDReturnsInternalError(t *testing.T) { diff --git a/spinifex/handlers/eks/cluster_state.go b/spinifex/handlers/eks/cluster_state.go index 268c934a4..12653b19b 100644 --- a/spinifex/handlers/eks/cluster_state.go +++ b/spinifex/handlers/eks/cluster_state.go @@ -189,6 +189,21 @@ func PutClusterMeta(ctx context.Context, kv jetstream.KeyValue, meta *ClusterMet return nil } +// IsControlPlaneMember reports whether instanceID is one of this cluster's +// control-plane server VMs. The scalar ControlPlaneInstanceID is consulted too, +// for clusters persisted before ControlPlaneNodes existed. +func (m *ClusterMeta) IsControlPlaneMember(instanceID string) bool { + if m == nil || instanceID == "" { + return false + } + for _, node := range m.ControlPlaneNodes { + if node.InstanceID == instanceID { + return true + } + } + return m.ControlPlaneInstanceID == instanceID +} + // GetClusterMeta reads the meta record. Returns ErrClusterNotFound if the // key is absent. func GetClusterMeta(ctx context.Context, kv jetstream.KeyValue, name string) (*ClusterMeta, error) { diff --git a/spinifex/handlers/eks/k3s_server_role.go b/spinifex/handlers/eks/k3s_server_role.go index 97663933a..f5cbda482 100644 --- a/spinifex/handlers/eks/k3s_server_role.go +++ b/spinifex/handlers/eks/k3s_server_role.go @@ -7,10 +7,11 @@ import ( ) const ( - // eksServerSystemRoleName is the Spinifex-managed instance role attached to - // the k3s control-plane VM. IMDS serves it so the VM's gateway publishes are + // CPInstanceRoleName is the Spinifex-managed instance role attached to the + // k3s control-plane VM. IMDS serves it so the VM's gateway publishes are // signed with scoped, rotating credentials instead of a baked static key. - eksServerSystemRoleName = "spinifex-eks-server" + // The gateway's internal-route gate names it to tell a CP VM from a tenant. + CPInstanceRoleName = "spinifex-eks-server" // eksServerInlinePolicyName is the inline policy granting the CP VM the // internal gateway actions its bootstrap/state-report/addon-fetch need. @@ -27,20 +28,23 @@ const ( ) // ensureCPInstanceProfile returns the CP instance-profile ARN, or "" to signal -// the caller should fall back to static creds. IAM unwired or a transient -// failure both fall back rather than brick a cluster launch — the static-key -// path still authenticates (it is what shipped before IMDS creds). +// the caller should fall back to static creds. That fallback still launches, but +// baked keys authenticate as a system user with no instance identity, so the VM +// is denied the internal routes for its life — see the log lines below. func (s *EKSServiceImpl) ensureCPInstanceProfile(accountID string) string { iam := s.iamEnsurer() if iam == nil { - slog.Warn("EKS: IAM service unwired; CP VM falls back to baked static gateway creds") + slog.Warn("EKS: IAM service unwired; CP VM falls back to baked static gateway creds and "+ + "will be denied eks:ListInternalAddons and eks:GetRecoveryDirective (no instance identity to bind)", + "accountID", accountID) return "" } profileARN, err := handlers_iam.EnsureSystemInstanceProfile(iam, accountID, - eksServerSystemRoleName, eksServerInlinePolicyName, eksServerInlinePolicy) + CPInstanceRoleName, eksServerInlinePolicyName, eksServerInlinePolicy) if err != nil { - slog.Warn("EKS: ensure CP instance profile failed; falling back to baked static gateway creds", - "err", err) + slog.Error("EKS: ensure CP instance profile failed; CP VM falls back to baked static gateway creds and "+ + "will be denied eks:ListInternalAddons and eks:GetRecoveryDirective (no instance identity to bind)", + "accountID", accountID, "role", CPInstanceRoleName, "err", err) return "" } return profileARN diff --git a/spinifex/handlers/eks/k3s_server_role_test.go b/spinifex/handlers/eks/k3s_server_role_test.go index a06f39f34..4bb22ee0d 100644 --- a/spinifex/handlers/eks/k3s_server_role_test.go +++ b/spinifex/handlers/eks/k3s_server_role_test.go @@ -20,13 +20,13 @@ func TestEnsureK3sServerInstanceProfile_CreatesRolePolicyProfile(t *testing.T) { s := &EKSServiceImpl{deps: EKSServiceDeps{IAM: f}} arn := s.ensureCPInstanceProfile(testSysAcct) - assert.Equal(t, "arn:aws:iam::"+testSysAcct+":instance-profile/"+eksServerSystemRoleName, arn) + assert.Equal(t, "arn:aws:iam::"+testSysAcct+":instance-profile/"+CPInstanceRoleName, arn) assert.Equal(t, 1, f.CreateRoleCalls) assert.Contains(t, f.LastTrustDoc, "ec2.amazonaws.com") assert.Contains(t, f.LastTrustDoc, "sts:AssumeRole") - policy := f.RolePolicies[eksServerSystemRoleName] + policy := f.RolePolicies[CPInstanceRoleName] assert.Contains(t, policy, "eks:PublishInternal") assert.Contains(t, policy, "eks:ListInternalAddons") // The eks-token-webhook relays get-token bearer tokens to the token-review @@ -38,8 +38,8 @@ func TestEnsureK3sServerInstanceProfile_CreatesRolePolicyProfile(t *testing.T) { // and a wedged control plane can never be auto-reset. assert.Contains(t, policy, "eks:GetRecoveryDirective") - require.NotNil(t, f.Profiles[eksServerSystemRoleName]) - assert.Len(t, f.Profiles[eksServerSystemRoleName].Roles, 1) + require.NotNil(t, f.Profiles[CPInstanceRoleName]) + assert.Len(t, f.Profiles[CPInstanceRoleName].Roles, 1) } // TestEnsureK3sServerInstanceProfile_ExistingRoleConverges asserts a re-launch @@ -47,16 +47,16 @@ func TestEnsureK3sServerInstanceProfile_CreatesRolePolicyProfile(t *testing.T) { // inline policy so a stale role converges onto the current permissions. func TestEnsureK3sServerInstanceProfile_ExistingRoleConverges(t *testing.T) { f := newFakeEnsurer() - f.Roles[eksServerSystemRoleName] = &iam.Role{ - RoleName: aws.String(eksServerSystemRoleName), - Arn: aws.String("arn:aws:iam::" + testSysAcct + ":role/" + eksServerSystemRoleName), + f.Roles[CPInstanceRoleName] = &iam.Role{ + RoleName: aws.String(CPInstanceRoleName), + Arn: aws.String("arn:aws:iam::" + testSysAcct + ":role/" + CPInstanceRoleName), } s := &EKSServiceImpl{deps: EKSServiceDeps{IAM: f}} arn := s.ensureCPInstanceProfile(testSysAcct) assert.NotEmpty(t, arn) assert.Zero(t, f.CreateRoleCalls) - assert.Contains(t, f.RolePolicies[eksServerSystemRoleName], "eks:PublishInternal") + assert.Contains(t, f.RolePolicies[CPInstanceRoleName], "eks:PublishInternal") } // TestEnsureCPInstanceProfile_NilIAMFallsBack asserts an unwired IAM service @@ -75,7 +75,7 @@ func TestEnsureCPInstanceProfile_UsesLazyProvider(t *testing.T) { IAMProvider: func() handlers_iam.SystemInstanceRoleEnsurer { return f }, }} arn := s.ensureCPInstanceProfile(testSysAcct) - assert.Equal(t, "arn:aws:iam::"+testSysAcct+":instance-profile/"+eksServerSystemRoleName, arn) + assert.Equal(t, "arn:aws:iam::"+testSysAcct+":instance-profile/"+CPInstanceRoleName, arn) assert.Equal(t, 1, f.CreateRoleCalls) }