docs: record EKS internal-route and role-ARN security corrections

This commit is contained in:
joshsiv-mulga
2026-10-09 10:27:04 +10:00
parent 642cb48898
commit 25b3efdec4
2 changed files with 17 additions and 5 deletions
+12
View File
@@ -410,6 +410,18 @@ These residuals were recorded by individual slices and must be resolved once the
- `docs/service-interfaces.yaml` (coverage gap): no `ecs.*` subjects are listed and the manifest lint does not cover them, so ECS publisher/subscriber edges play no part in suite selection. Add them only after tracing the actual publishers and subscribers.
- Merge-time follow-up in the umbrella repository: the `Makefile` `invariants-check` target, the `docs-pr.yml` clause cross-check, `.claude/hooks/architecture-traps.sh` and a `check-invariants.sh` comment hard-code `spinifex/spinifex/network/invariants`. They must change with the pointer bump that brings in this branch, or the docs-pr cross-check silently skips. Live specs and proposals citing `spinifex/network/` will report unresolved citations until updated.
### Security corrections and branch preflight exception
These are security corrections found during discovery, not lifecycle work.
- `ee13be950`: `PublishInternal` required only the IAM policy, built from the body account and URL cluster, and the control-plane role grants it on `*`. Any control-plane VM session, any tenant holding `eks:*`, or static system credentials could publish cluster state, bootstrap and add-on reports for another cluster or account. It now passes the internal-route membership gate, with one body decoder shared by the gate and the handler. Evidence: `gateway/eks_authz_test.go` `TestEKSRequest_PublishInternal*`.
- `a23e1ccf5`: `WebhookTokenReview` had the same flaw and could reveal another cluster's Kubernetes group mappings. It now passes the same gate, and every unauthorised target returns the same `AccessDenied` without reaching STS. Evidence: `TestEKSRequest_WebhookTokenReview*`, `gateway/eks/internal_authz_test.go` `TestInternalBodyAccounts_CoverEveryBodyScopedRoute`.
- `642cb4889`: `serviceAccountRoleArn` on CreateAddon and UpdateAddon must be syntactically an IAM role ARN (`foundation/aws/arn.ValidateRoleARN`), returning `InvalidParameterException`; `iam:PassRole` remains the permission and existence check. Evidence: `handlers/eks/addons_test.go` `TestCreateAddon_RejectsMalformedServiceAccountRoleArn`, `TestUpdateAddon_*ServiceAccountRoleArn*`.
- Remaining render gap: IAM's path grammar permits `"`, `&`, `\` and `|`, which the guest's `sed` replacement and double-quoted YAML value in `mulga-eks-addon-sync.sh` do not escape. A role whose path contains them must also exist and pass `iam:PassRole`. Removal condition: the guest renders the role ARN with escaping for both `sed` and YAML (or a structured renderer), with a test rendering such a path verbatim.
- Static-credential control-plane VMs (IAM wiring failed) are now denied every internal route and cannot bootstrap; that fallback is unsupported.
Preflight exception (branch only): from `ee13be950`, full `make preflight` on this branch is red solely because the vulnerability gate reports GO-2026-6599 to GO-2026-6617 against the pinned Go 1.27.0 toolchain. Every other gate passes. This branch is not production. No advisory is accepted or suppressed, and the accepted-advisory list, `go.mod` and the toolchain pins are unchanged; the toolchain is updated through Dependabot. Normal full-preflight enforcement resumes when the toolchain baseline is updated.
## ADR-0004 source inventory and first-resource ranking
The per-file inventories for ECS, EKS and RDS are in `docs/package-boundary/adr-0004-inventory-{ecs,eks,rds}.md`.
@@ -51,9 +51,9 @@ Paths are relative to `spinifex/` unless they start with `scripts/`, `images/`,
| Role policy | `eks:PublishInternal`, `eks:ListInternalAddons`, `eks:WebhookTokenReview`, `eks:GetRecoveryDirective` on `Resource: "*"`; one role shared by every cluster's CP VMs. | `k3s_server_role.go:27`, `:42-43` |
| Caller mapping | `eksCaller`: `SessionName` from `ctxIdentity`, role name from the underlying role ARN, not the session name. | `gateway/eks.go:307-320` |
| `AuthorizeInternal` | Requires the CP principal class, then `meta.IsControlPlaneMember(caller.SessionName)` for the named cluster; `GetRecoveryDirective` additionally requires the path instance ID to equal the caller's. | `gateway/eks/internal_authz.go:44-96`; tests `gateway/eks_authz_test.go` `TestEKSRequest_InternalRoutesAdmitTheCPAgentPrincipal`, `gateway/eks/internal_authz_test.go` `TestAuthorizeInternal_RejectsClusterTheCallerDoesNotServe` |
| Which routes it gates | Only actions scoped `sourceInternalCluster`: `ListInternalAddons` and `GetRecoveryDirective`. `PublishInternal` and `WebhookTokenReview` are `sourceBodyAccountCluster`, so `IsInternalAction` is false and only the IAM policy check applies. | `gateway/eks/authz.go:52-60`, `gateway/eks/internal_authz.go:37-39`, `gateway/eks.go:216-220`, `:248` |
| Which routes it gates | All four internal routes. `ListInternalAddons` and `GetRecoveryDirective` are scoped `sourceInternalCluster`; `PublishInternal` and `WebhookTokenReview` are scoped `sourceInternalBodyCluster`, binding the URL cluster and the body account (read by the same decoder the handler uses) to the caller's membership before policy evaluation. Before that correction both were policy-only. | `gateway/eks/authz.go`, `gateway/eks/internal_authz.go`; tests `gateway/eks_authz_test.go` `TestEKSRequest_PublishInternal*`, `TestEKSRequest_WebhookTokenReview*` |
| Member vs primary | The gateway can tell members apart (instance ID) and bind a fetch to a cluster, but cannot identify the delivery executor: meta has no role field, and `[0]` stops being the `server`-role VM after `[0]` is replaced (Section 6). | `internal_authz.go:74`, `cluster_state.go:463-490` |
| Static-cred fallback | When the instance profile cannot be ensured, the VM gets the shared system access key; it has no instance identity, so `ListInternalAddons` is denied for the VM's life while `PublishInternal` remains policy-gated only. | `k3s_server_role.go:30-50`, `k3s_ha_control_plane.go:161-166`, `k3s_server_vm.go:549-557` |
| Static-cred fallback | When the instance profile cannot be ensured, the VM gets the shared system access key; it has no instance identity, so every internal route, including `PublishInternal` and `WebhookTokenReview`, is denied for the VM's life; this fallback is unsupported. | `k3s_server_role.go:30-50`, `k3s_ha_control_plane.go:161-166`, `k3s_server_vm.go:549-557` |
| Handler-side identity | `ListStagedAddonManifests` receives only the cluster name; no caller identity reaches the daemon. | `gateway/eks/internal_addons.go:20-33`, `handlers/eks/addons.go:117-128`, `daemon/daemon.go:1116` |
## 5. Status relay path
@@ -103,7 +103,7 @@ Paths are relative to `spinifex/` unless they start with `scripts/`, `images/`,
| EC2 state `terminated` or instance not found | `cpControlAdapter.InstanceState` (system account `DescribeInstances`); `TerminateK3sServerVM` treats not-found as success. | Yes for render and apply once the VM is gone, if the instance record is authoritative; host-partition behaviour of the state record is unverified. | Reports already queued or retried by `eks-gateway-publish` are still accepted. Needs: state proven authoritative under host loss, and report acceptance bound to the current executor. `service_impl.go:153-170`, `k3s_server_vm.go:292-330` |
| EC2 state `stopped` / `error` | Same query. | Only momentarily: the reconciler's restart ladder starts stopped/error members automatically, and the VM resumes delivery with the same identity. | Needs a stop that nothing restarts, or a guest-side disable that survives restart. `cluster_reconciler.go:708-771`, `:927-937` |
| Describe error | Treated as lost. | No: the VM may be running. | Must not count as fencing evidence. `cluster_reconciler.go:982-1007` |
| Removal from `ControlPlaneNodes` | `SwapControlPlaneMember`, `replaceControlPlaneForRestore`. | Partly: `ListInternalAddons` is denied, but the agent keeps its current render on fetch failure, K3s keeps applying those files, and `PublishInternal` still accepts its reports. | Needs: publish gated by membership/executor and identity attached on relay; a guest rule that a denied fetch stops delivery. `internal_authz.go:74`, `mulga-eks-addon-sync.sh:344-348` |
| Removal from `ControlPlaneNodes` | `SwapControlPlaneMember`, `replaceControlPlaneForRestore`. | Partly: `ListInternalAddons` is denied, but the agent keeps its current render on fetch failure, K3s keeps applying those files; its reports are now denied by the membership check, but reports already in flight are still applied. | Needs: publish gated by executor and identity attached on relay; a guest rule that a denied fetch stops delivery. `internal_authz.go:74`, `mulga-eks-addon-sync.sh:344-348` |
| Instance ID / incarnation | Session name = instance ID; stable across stop/start. | No: there is no per-boot incarnation, so a restarted VM is indistinguishable from before. | Needs an executor epoch recorded host-side and checked per request; instance ID alone identifies, it does not fence. `assume_role.go:136-161` |
| KV revisions | Meta revision, add-on record revision, lease revision. | No: meta revision churns with health writes, add-on CAS ignores sender, lease revision is host-only and private. | Needs a dedicated, monotonic executor epoch written only on reassignment. `cluster_state.go:416-436`, `kvlease/lease.go:44-52` |
| IMDS credential expiry | Instance-role creds last 3600 s, re-minted 5 min early. | No: up to an hour of validity after the last mint, and a running VM keeps re-minting. | Not a fence on its own. `domains/ec2/guestmetadata/credentials.go:14-20` |
@@ -119,7 +119,7 @@ No option is chosen here; each row lists only what it would rely on.
| Option | Evidence it needs that does not exist today |
|---|---|
| Host-assigned executor `{instanceId, epoch}` on the cluster record | An executor field and epoch advanced by CAS; `AuthorizeInternal`-style gating on `PublishInternal` (today policy-only); the gateway attaching the caller instance ID (and epoch) to the relayed message; report apply checking it; a reassignment trigger that does not fire on describe errors; fence evidence from Section 8 rows that say "Yes". |
| Host-assigned executor `{instanceId, epoch}` on the cluster record | An executor field and epoch advanced by CAS; the gateway attaching the caller instance ID (and epoch) to the relayed message; report apply checking it; a reassignment trigger that does not fire on describe errors; fence evidence from Section 8 rows that say "Yes". |
| Executor follows meta `[0]` | A guest that can become executor after first boot (today role is fixed and replacements are `server-join`); the same gateway and relay identity checks; `[0]` reassignment tied to fencing of the old `[0]`. |
| Run the agent on every server with guest-side leader election (kube Lease) | Host-visible proof of the guest leader (the host cannot read the kube Lease today); verification of K3s auto-deploy behaviour across servers; reports still need gateway-attached identity, since a partitioned old leader can still publish. |
| Disable-on-handover directive | An online directive channel and agent polling; a rule that a fetch denial or missing directive stops rendering; an acknowledgement or instance-state proof before the successor acts. |
@@ -128,7 +128,7 @@ No option is chosen here; each row lists only what it would rely on.
## 10. Gaps
1. Delivery runs only on the first-boot `server` VM; replacing HA `[0]` leaves no member running `mulga-eks-addon-sync`, with no detection or recovery.
2. `PublishInternal` is not gated by `AuthorizeInternal`; with the CP role's `Resource: "*"`, any CP-role session can publish add-on (and state) reports for any cluster and account.
2. Corrected: `PublishInternal` and `WebhookTokenReview` were policy-only, so any CP-role session, any tenant holding `eks:*`, or static system credentials could publish reports or resolve tokens for any cluster and account. Both now require cluster membership (see Section 3); the binding is to a member, not to the delivery executor.
3. The relay attaches no caller identity, and the payload carries none, so the owner cannot tell which member, executor or epoch produced a report; this conflicts with the direction in draft PROP-SPINIFEXARCH-007 S3, which is not accepted.
4. `ClusterMeta` has no role, executor or epoch field; `[0]` as "primary" is a convention that diverges from the guest role after a swap.
5. A member removed from meta is denied fetches but keeps applying its last render and keeps having its reports accepted.