amadan
Merge pull request #352 from carlosframework/idms-gate
carlos-app@ units can no longer reach IMDS; host_role drop-ins re-open it
7 files changed,
+39
−5
internal/caps/converge_test.go+1 −1internal/caps/render.go+8 −0internal/caps/render_test.go+2 −2internal/caps/testdata/console.service.conf+1 −0internal/caps/testdata/keymail.service.conf+1 −0internal/host/host_test.go+18 −0internal/host/units/carlos-app@.service+8 −2
diff --git a/internal/caps/converge_test.go b/internal/caps/converge_test.go| index 8e64d1710..045b4964b 100644 |
| --- a/internal/caps/converge_test.go |
| +++ b/internal/caps/converge_test.go |
| @@ -374,7 +374,7 @@ func TestGrantRevokedRewritesTheDropIn(t *testing.T) { |
| svcPath, sockPath := DropInPaths(o.EtcDir, p.Host) |
| mustNotExist(t, sockPath) |
| svc := string(readFile(t, svcPath)) |
| - for _, gone := range []string{"DynamicUser=no", "User=carlos-keymail", "Group=carlos-keymail"} { |
| + for _, gone := range []string{"DynamicUser=no", "User=carlos-keymail", "Group=carlos-keymail", "IPAddressAllow"} { |
| if strings.Contains(svc, gone) { |
| t.Fatalf("service drop-in still holds %q after the revoke:\n%s", gone, svc) |
| } |
diff --git a/internal/caps/render.go b/internal/caps/render.go| index c4c2f01ae..65b9bd043 100644 |
| --- a/internal/caps/render.go |
| +++ b/internal/caps/render.go |
| @@ -110,6 +110,14 @@ func RenderService(app string, s *Set, g Grants) ([]byte, error) { |
| "RestrictSUIDSGID=yes", |
| "User="+user, |
| "Group="+user, |
| + // The template denies IMDS unit-wide (issue #351); a matching |
| + // IPAddressAllow re-opens it for exactly the tenant that holds |
| + // the grant — in systemd's BPF firewall an allow-list match |
| + // wins over the deny list. The nftables skuid gate stays the |
| + // operator-owned authority on which uids reach the role |
| + // (spec §4b); this only lifts the unit-level fence the |
| + // template put in everyone's way. |
| + "IPAddressAllow=169.254.169.254/32 fd00:ec2::254/128", |
| ) |
| } |
diff --git a/internal/caps/render_test.go b/internal/caps/render_test.go| index 870a5defd..e8f67a3e5 100644 |
| --- a/internal/caps/render_test.go |
| +++ b/internal/caps/render_test.go |
| @@ -171,7 +171,7 @@ func TestUngrantedCapabilitiesMaterializeNothing(t *testing.T) { |
| t.Errorf("service body missing self-service line %q:\n%s", want, got) |
| } |
| } |
| - for _, unwanted := range []string{"DynamicUser", "RestrictSUIDSGID", "User=", "Group="} { |
| + for _, unwanted := range []string{"DynamicUser", "RestrictSUIDSGID", "User=", "Group=", "IPAddressAllow"} { |
| if bytes.Contains(got, []byte(unwanted)) { |
| t.Errorf("service body has ungranted host_role directive %q:\n%s", unwanted, got) |
| } |
| @@ -202,7 +202,7 @@ func TestHostRoleGrantLingersButManifestNoLongerDeclaresFallsBackToDynamicUser(t |
| t.Errorf("service body missing %q:\n%s", want, got) |
| } |
| } |
| - for _, unwanted := range []string{"DynamicUser", "RestrictSUIDSGID", "User=", "Group="} { |
| + for _, unwanted := range []string{"DynamicUser", "RestrictSUIDSGID", "User=", "Group=", "IPAddressAllow"} { |
| if bytes.Contains(got, []byte(unwanted)) { |
| t.Errorf("service body has host_role directive %q though the manifest no longer declares host_role:\n%s", unwanted, got) |
| } |
diff --git a/internal/caps/testdata/console.service.conf b/internal/caps/testdata/console.service.conf| index bf1d4c8f6..8eeb03948 100644 |
| --- a/internal/caps/testdata/console.service.conf |
| +++ b/internal/caps/testdata/console.service.conf |
| @@ -4,3 +4,4 @@ DynamicUser=no |
| RestrictSUIDSGID=yes |
| User=carlos-console |
| Group=carlos-console |
| +IPAddressAllow=169.254.169.254/32 fd00:ec2::254/128 |
diff --git a/internal/caps/testdata/keymail.service.conf b/internal/caps/testdata/keymail.service.conf| index facb583d8..468f92bd2 100644 |
| --- a/internal/caps/testdata/keymail.service.conf |
| +++ b/internal/caps/testdata/keymail.service.conf |
| @@ -13,3 +13,4 @@ DynamicUser=no |
| RestrictSUIDSGID=yes |
| User=carlos-keymail |
| Group=carlos-keymail |
| +IPAddressAllow=169.254.169.254/32 fd00:ec2::254/128 |
diff --git a/internal/host/host_test.go b/internal/host/host_test.go| index a4d664c12..2dad9eb30 100644 |
| --- a/internal/host/host_test.go |
| +++ b/internal/host/host_test.go |
| @@ -1382,6 +1382,24 @@ func shipVersion(t *testing.T, s release.Store, acct, app, version string) { |
| // |
| // This is a one-line property that would regress silently (the drill still |
| // passes, right up until a tenant grows), so it is pinned here. |
| +// The tenant template must deny IMDS at the unit level (issue #351): the |
| +// nftables skuid gate is operator territory and a box can simply not have |
| +// one (tito didn't), at which point any app process could read the box |
| +// instance-role credentials off 169.254.169.254. Apps get per-app scoped |
| +// IAM keys via env; the one legitimate IMDS consumer under this template |
| +// is a host_role tenant, whose drop-in re-opens it with IPAddressAllow |
| +// (allow beats deny in systemd's BPF firewall) — see caps.RenderService. |
| +// One-line property, silent regression, so it is pinned here. |
| +func TestAppTemplateDeniesIMDS(t *testing.T) { |
| + body, err := unitsFS.ReadFile("units/carlos-app@.service") |
| + if err != nil { |
| + t.Fatalf("carlos-app@.service: %v", err) |
| + } |
| + if !strings.Contains(string(body), "IPAddressDeny=169.254.169.254/32 fd00:ec2::254/128") { |
| + t.Errorf("carlos-app@.service does not deny IMDS — app code on an ungated box can take the instance role's credentials (issue #351)") |
| + } |
| +} |
| + |
| func TestVerifyDrillsRestoreToDiskNotTmpfs(t *testing.T) { |
| for _, unit := range []string{"carlos-restore-verify.service", "carlos-hibernate-verify.service"} { |
| body, err := unitsFS.ReadFile("units/" + unit) |
diff --git a/internal/host/units/carlos-app@.service b/internal/host/units/carlos-app@.service| index 613eea27b..d8509d698 100644 |
| --- a/internal/host/units/carlos-app@.service |
| +++ b/internal/host/units/carlos-app@.service |
| @@ -6,8 +6,13 @@ |
| # share no identity with each other or the platform, and nobody manages |
| # user accounts. State lives in /var/lib/carlos-app/<host> (chowned to |
| # the fresh uid each start); the app finds it via $STATE_DIRECTORY. |
| -# The IMDS firewall gate (harden-host) blocks dynamic uids by |
| -# construction — only root and carlos may mint host credentials. |
| +# IMDS is fenced twice: IPAddressDeny below blocks the metadata address |
| +# for every tenant at the unit level (a box with no nftables gate — tito, |
| +# issue #351 — is otherwise wide open), and on hardened boxes the |
| +# harden-host skuid gate blocks dynamic uids by construction — only |
| +# allowlisted uids may mint host credentials. A host_role tenant's |
| +# drop-in re-opens the unit-level fence with IPAddressAllow (allow wins |
| +# over deny); the skuid gate still has the last word. |
| # |
| # Caveat: DynamicUser is per-unit, so an app whose processes must share |
| # files across units (a router + instance family like titogo's) needs a |
| @@ -55,6 +60,7 @@ RestrictRealtime=yes |
| RestrictNamespaces=yes |
| CapabilityBoundingSet= |
| RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX |
| +IPAddressDeny=169.254.169.254/32 fd00:ec2::254/128 |
| MemoryDenyWriteExecute=yes |
| MemoryMax=512M |
| # Throttle-and-reclaim before the kill — the same band the exec path's |