fix(apps): refuse reuse of containers exceeding narrowed capability policy
- commit
- 694968b681f45580f8cde08595febeb69251f6a7
- author
- brice · · 2026-10-18 19:44 UTC
- committer
- brice · · 2026-10-18 20:33 UTC
- parent
- 2950329
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
10 files · +252 -6
M docs/config/security-hardening.md +6 -1
| @@ -85,6 +85,8 @@ The manifest references only the logical name: `devices = ["transcode-gpu"]`. | |||
| 85 | 85 | ||
| 86 | 86 | ### Plain device nodes under rootless Podman | |
| 87 | 87 | ||
| 88 | + | > **Verified by source reading only. Confirm on the host:** create a throwaway container with the same device IDs (`podman run --rm --device example.com/vm=kvm --device example.com/vm=net-tun alpine ls -l /dev/kvm /dev/net/tun`) as the rootless user before the first Gordon deploy. | |
| 89 | + | ||
| 88 | 90 | CDI is not GPU-specific. Any device node can be exposed through a user-level CDI spec, so `/dev/kvm` and `/dev/net/tun` reach a rootless Podman service without raw `/dev` paths in manifests and without any privilege: | |
| 89 | 91 | ||
| 90 | 92 | ```yaml | |
| @@ -117,7 +119,7 @@ allowed_apps = ["myapp"] | |||
| 117 | 119 | allowed_services = ["worker"] | |
| 118 | 120 | ``` | |
| 119 | 121 | ||
| 120 | - | - No Gordon change is needed (verified against the Podman 5.7 sources; confirm with a live create on your host). Gordon sends the IDs as a native `cdi` `DeviceRequest`; the Podman compat API forwards them to the same CDI injection as `podman run --device <id>`, which reads `cdi_spec_dirs` from the containers.conf of the user running the API service. Restart that service (`systemctl --user restart podman.socket podman.service`) after editing `containers.conf`; edits to the spec files themselves are picked up on the next container create. | |
| 122 | + | - **Status: verified by source reading (Podman 5.7.1), not by a live run. Confirm on your host before relying on it.** No Gordon change appeared necessary. Gordon sends the IDs as a native `cdi` `DeviceRequest`; the Podman compat API forwards them to the same CDI injection as `podman run --device <id>`, which reads `cdi_spec_dirs` from the containers.conf of the user running the API service. Restart that service (`systemctl --user restart podman.socket podman.service`) after editing `containers.conf`; edits to the spec files themselves are picked up on the next container create. | |
| 121 | 123 | - Under rootless Podman the OCI runtime (crun or runc) cannot `mknod` in a user namespace and falls back to bind-mounting the host device node into the container. The node must therefore be accessible to the rootless user on the host: `/dev/kvm` and `/dev/net/tun` are usually `0666` or group-owned (`kvm`). Supplementary groups do not carry into the user namespace, so prefer a `0666`/ACL-based permission over group membership. | |
| 122 | 124 | - CDI device names Gordon accepts match `[A-Za-z0-9][A-Za-z0-9_.-]*` (so `kvm` and `net-tun` are valid; names containing `:` are not). | |
| 123 | 125 | - A missing spec dir entry, an unreadable spec, or an unknown device name surfaces as a redacted create failure (`runtime error redacted`); check the Podman service journal for the CDI error. | |
| @@ -136,10 +138,13 @@ allowed_services = ["worker"] # exact, non-empty | |||
| 136 | 138 | The manifest references only the logical name: `capabilities = ["vm-net"]`. | |
| 137 | 139 | ||
| 138 | 140 | - `add` holds explicit Linux capability names, written without the `CAP_` prefix. Only this allowed set is accepted: `AUDIT_WRITE`, `CHOWN`, `DAC_OVERRIDE`, `FOWNER`, `FSETID`, `IPC_LOCK`, `KILL`, `MKNOD`, `NET_ADMIN`, `NET_BIND_SERVICE`, `NET_RAW`, `SETFCAP`, `SETGID`, `SETPCAP`, `SETUID`, `SYS_CHROOT`, `SYS_NICE`, `SYS_RESOURCE`. Everything else is rejected when the config loads, including `ALL`, `SYS_ADMIN`, `SYS_PTRACE`, `SYS_MODULE`, `SYS_RAWIO`, `SYS_BOOT`, `SYS_TIME`, `DAC_READ_SEARCH`, `BPF`, `PERFMON`, `MAC_ADMIN`, `MAC_OVERRIDE`, `AUDIT_CONTROL`, and `SYSLOG`. These can reach the host or defeat container confinement; a workload that needs one is not an unprivileged service. | |
| 141 | + | - **The allowed capabilities are not harmless.** The list only removes capabilities that clearly break confinement; each remaining one still needs a workload-specific assessment before you grant it. In particular: `SETFCAP` lets a process write file capabilities, which is dangerous together with writable binds or volumes shared with another service or the host; `MKNOD` creates device nodes (inert in a user namespace, but a risk if the runtime ever runs without one); `NET_RAW` allows packet spoofing and raw sockets inside the container's network namespace and on any shared network; `SYS_CHROOT`, `SETPCAP`, `KILL`, `DAC_OVERRIDE`, `FOWNER` and `FSETID` widen what a compromised process can do to files and processes it can reach; `NET_ADMIN` controls every interface, route, and firewall rule in the container's network namespace, including a shared network attachment. Grant the smallest set to the narrowest app+service pair. | |
| 139 | 142 | - `allowed_apps` and `allowed_services` are exact, non-empty allowlists. There is no wildcard, prefix, or empty-means-all form. Unknown keys inside `[app_capabilities.<name>]` fail closed. | |
| 140 | 143 | - Granted capabilities are added on top of Gordon's default set (`CHOWN`, `DAC_OVERRIDE`, `FOWNER`, `SETUID`, `SETGID`, `NET_BIND_SERVICE`). Gordon's `cap_drop = ALL` and `no-new-privileges` stay in force, and containers are never privileged. The runtime guard re-checks the resolved list before every create. | |
| 141 | 144 | - Capabilities apply inside the container's own user and network namespaces. Under rootless Podman, `NET_ADMIN` lets the service create TAP devices and nftables rules in its own network namespace, not on the host. Because `no-new-privileges` stays on, a process only holds the added capabilities when it runs as container root (uid 0 in the user namespace, which is an unprivileged host user under rootless Podman); a non-root container user cannot gain them through file capabilities. | |
| 142 | 145 | - Gordon resolves logical names at activation time. Revisions persist the logical names, never the resolution. The grant is re-resolved before every preflight, container create, restart, and recovery. Revoking a grant blocks future deploys and restarts while the running service is untouched. | |
| 146 | + | - **Narrowing a policy is enforced on reuse.** A container keeps the capabilities it was created with, so before Gordon restarts, starts, or health-restarts an existing capability-bearing container it inspects the container's configured `CapAdd` and refuses the operation (`capability-policy-violation`, naming the excess capabilities) when it exceeds Gordon's defaults plus the currently authorized grant. If the container cannot be inspected the operation fails closed. Run `gordon apps deploy` to recreate the service with the narrowed set; the superseded container is retired by the normal replacement. A container missing from the runtime is rebuilt from the pinned revision under the current policy. | |
| 147 | + | - **Device grants have the same gap, documented rather than fixed.** Narrowing an `[app_devices]` mapping (removing a CDI ID) does not remove the device from an already created container, and a restart or recovery of that container keeps it. Resolution still fails closed for revoked or unknown policies, but Gordon cannot reliably read back the injected CDI devices (Podman's Docker-compatible inspect does not report them), so it cannot verify a narrowed mapping on reuse. After narrowing or revoking a device mapping, run `gordon apps deploy` for the affected service (or stop it) so the container is recreated with the current mapping. | |
| 143 | 148 | - Adding or removing a policy name shows as `service/<name>/capabilities` in diffs and recreates the container on the next deploy; reorder-only input is a no-op. Changing what a policy grants does not recreate a running container: the next deploy serves the new resolution. | |
| 144 | 149 | ||
| 145 | 150 | ## External image registries | |
| @@ -85,6 +85,8 @@ The manifest references only the logical name: `devices = ["transcode-gpu"]`. | |||||
| 85 | 85 | ||||
| 86 | ### Plain device nodes under rootless Podman | 86 | ### Plain device nodes under rootless Podman | ||
| 87 | 87 | ||||
| 88 | + | > **Verified by source reading only. Confirm on the host:** create a throwaway container with the same device IDs (`podman run --rm --device example.com/vm=kvm --device example.com/vm=net-tun alpine ls -l /dev/kvm /dev/net/tun`) as the rootless user before the first Gordon deploy. | |||
| 89 | + | ||||
| 88 | CDI is not GPU-specific. Any device node can be exposed through a user-level CDI spec, so `/dev/kvm` and `/dev/net/tun` reach a rootless Podman service without raw `/dev` paths in manifests and without any privilege: | 90 | CDI is not GPU-specific. Any device node can be exposed through a user-level CDI spec, so `/dev/kvm` and `/dev/net/tun` reach a rootless Podman service without raw `/dev` paths in manifests and without any privilege: | ||
| 89 | 91 | ||||
| 90 | ```yaml | 92 | ```yaml | ||
| @@ -117,7 +119,7 @@ allowed_apps = ["myapp"] | |||||
| 117 | allowed_services = ["worker"] | 119 | allowed_services = ["worker"] | ||
| 118 | ``` | 120 | ``` | ||
| 119 | 121 | ||||
| 120 | - | - No Gordon change is needed (verified against the Podman 5.7 sources; confirm with a live create on your host). Gordon sends the IDs as a native `cdi` `DeviceRequest`; the Podman compat API forwards them to the same CDI injection as `podman run --device <id>`, which reads `cdi_spec_dirs` from the containers.conf of the user running the API service. Restart that service (`systemctl --user restart podman.socket podman.service`) after editing `containers.conf`; edits to the spec files themselves are picked up on the next container create. | 122 | + | - **Status: verified by source reading (Podman 5.7.1), not by a live run. Confirm on your host before relying on it.** No Gordon change appeared necessary. Gordon sends the IDs as a native `cdi` `DeviceRequest`; the Podman compat API forwards them to the same CDI injection as `podman run --device <id>`, which reads `cdi_spec_dirs` from the containers.conf of the user running the API service. Restart that service (`systemctl --user restart podman.socket podman.service`) after editing `containers.conf`; edits to the spec files themselves are picked up on the next container create. |
| 121 | - Under rootless Podman the OCI runtime (crun or runc) cannot `mknod` in a user namespace and falls back to bind-mounting the host device node into the container. The node must therefore be accessible to the rootless user on the host: `/dev/kvm` and `/dev/net/tun` are usually `0666` or group-owned (`kvm`). Supplementary groups do not carry into the user namespace, so prefer a `0666`/ACL-based permission over group membership. | 123 | - Under rootless Podman the OCI runtime (crun or runc) cannot `mknod` in a user namespace and falls back to bind-mounting the host device node into the container. The node must therefore be accessible to the rootless user on the host: `/dev/kvm` and `/dev/net/tun` are usually `0666` or group-owned (`kvm`). Supplementary groups do not carry into the user namespace, so prefer a `0666`/ACL-based permission over group membership. | ||
| 122 | - CDI device names Gordon accepts match `[A-Za-z0-9][A-Za-z0-9_.-]*` (so `kvm` and `net-tun` are valid; names containing `:` are not). | 124 | - CDI device names Gordon accepts match `[A-Za-z0-9][A-Za-z0-9_.-]*` (so `kvm` and `net-tun` are valid; names containing `:` are not). | ||
| 123 | - A missing spec dir entry, an unreadable spec, or an unknown device name surfaces as a redacted create failure (`runtime error redacted`); check the Podman service journal for the CDI error. | 125 | - A missing spec dir entry, an unreadable spec, or an unknown device name surfaces as a redacted create failure (`runtime error redacted`); check the Podman service journal for the CDI error. | ||
| @@ -136,10 +138,13 @@ allowed_services = ["worker"] # exact, non-empty | |||||
| 136 | The manifest references only the logical name: `capabilities = ["vm-net"]`. | 138 | The manifest references only the logical name: `capabilities = ["vm-net"]`. | ||
| 137 | 139 | ||||
| 138 | - `add` holds explicit Linux capability names, written without the `CAP_` prefix. Only this allowed set is accepted: `AUDIT_WRITE`, `CHOWN`, `DAC_OVERRIDE`, `FOWNER`, `FSETID`, `IPC_LOCK`, `KILL`, `MKNOD`, `NET_ADMIN`, `NET_BIND_SERVICE`, `NET_RAW`, `SETFCAP`, `SETGID`, `SETPCAP`, `SETUID`, `SYS_CHROOT`, `SYS_NICE`, `SYS_RESOURCE`. Everything else is rejected when the config loads, including `ALL`, `SYS_ADMIN`, `SYS_PTRACE`, `SYS_MODULE`, `SYS_RAWIO`, `SYS_BOOT`, `SYS_TIME`, `DAC_READ_SEARCH`, `BPF`, `PERFMON`, `MAC_ADMIN`, `MAC_OVERRIDE`, `AUDIT_CONTROL`, and `SYSLOG`. These can reach the host or defeat container confinement; a workload that needs one is not an unprivileged service. | 140 | - `add` holds explicit Linux capability names, written without the `CAP_` prefix. Only this allowed set is accepted: `AUDIT_WRITE`, `CHOWN`, `DAC_OVERRIDE`, `FOWNER`, `FSETID`, `IPC_LOCK`, `KILL`, `MKNOD`, `NET_ADMIN`, `NET_BIND_SERVICE`, `NET_RAW`, `SETFCAP`, `SETGID`, `SETPCAP`, `SETUID`, `SYS_CHROOT`, `SYS_NICE`, `SYS_RESOURCE`. Everything else is rejected when the config loads, including `ALL`, `SYS_ADMIN`, `SYS_PTRACE`, `SYS_MODULE`, `SYS_RAWIO`, `SYS_BOOT`, `SYS_TIME`, `DAC_READ_SEARCH`, `BPF`, `PERFMON`, `MAC_ADMIN`, `MAC_OVERRIDE`, `AUDIT_CONTROL`, and `SYSLOG`. These can reach the host or defeat container confinement; a workload that needs one is not an unprivileged service. | ||
| 141 | + | - **The allowed capabilities are not harmless.** The list only removes capabilities that clearly break confinement; each remaining one still needs a workload-specific assessment before you grant it. In particular: `SETFCAP` lets a process write file capabilities, which is dangerous together with writable binds or volumes shared with another service or the host; `MKNOD` creates device nodes (inert in a user namespace, but a risk if the runtime ever runs without one); `NET_RAW` allows packet spoofing and raw sockets inside the container's network namespace and on any shared network; `SYS_CHROOT`, `SETPCAP`, `KILL`, `DAC_OVERRIDE`, `FOWNER` and `FSETID` widen what a compromised process can do to files and processes it can reach; `NET_ADMIN` controls every interface, route, and firewall rule in the container's network namespace, including a shared network attachment. Grant the smallest set to the narrowest app+service pair. | |||
| 139 | - `allowed_apps` and `allowed_services` are exact, non-empty allowlists. There is no wildcard, prefix, or empty-means-all form. Unknown keys inside `[app_capabilities.<name>]` fail closed. | 142 | - `allowed_apps` and `allowed_services` are exact, non-empty allowlists. There is no wildcard, prefix, or empty-means-all form. Unknown keys inside `[app_capabilities.<name>]` fail closed. | ||
| 140 | - Granted capabilities are added on top of Gordon's default set (`CHOWN`, `DAC_OVERRIDE`, `FOWNER`, `SETUID`, `SETGID`, `NET_BIND_SERVICE`). Gordon's `cap_drop = ALL` and `no-new-privileges` stay in force, and containers are never privileged. The runtime guard re-checks the resolved list before every create. | 143 | - Granted capabilities are added on top of Gordon's default set (`CHOWN`, `DAC_OVERRIDE`, `FOWNER`, `SETUID`, `SETGID`, `NET_BIND_SERVICE`). Gordon's `cap_drop = ALL` and `no-new-privileges` stay in force, and containers are never privileged. The runtime guard re-checks the resolved list before every create. | ||
| 141 | - Capabilities apply inside the container's own user and network namespaces. Under rootless Podman, `NET_ADMIN` lets the service create TAP devices and nftables rules in its own network namespace, not on the host. Because `no-new-privileges` stays on, a process only holds the added capabilities when it runs as container root (uid 0 in the user namespace, which is an unprivileged host user under rootless Podman); a non-root container user cannot gain them through file capabilities. | 144 | - Capabilities apply inside the container's own user and network namespaces. Under rootless Podman, `NET_ADMIN` lets the service create TAP devices and nftables rules in its own network namespace, not on the host. Because `no-new-privileges` stays on, a process only holds the added capabilities when it runs as container root (uid 0 in the user namespace, which is an unprivileged host user under rootless Podman); a non-root container user cannot gain them through file capabilities. | ||
| 142 | - Gordon resolves logical names at activation time. Revisions persist the logical names, never the resolution. The grant is re-resolved before every preflight, container create, restart, and recovery. Revoking a grant blocks future deploys and restarts while the running service is untouched. | 145 | - Gordon resolves logical names at activation time. Revisions persist the logical names, never the resolution. The grant is re-resolved before every preflight, container create, restart, and recovery. Revoking a grant blocks future deploys and restarts while the running service is untouched. | ||
| 146 | + | - **Narrowing a policy is enforced on reuse.** A container keeps the capabilities it was created with, so before Gordon restarts, starts, or health-restarts an existing capability-bearing container it inspects the container's configured `CapAdd` and refuses the operation (`capability-policy-violation`, naming the excess capabilities) when it exceeds Gordon's defaults plus the currently authorized grant. If the container cannot be inspected the operation fails closed. Run `gordon apps deploy` to recreate the service with the narrowed set; the superseded container is retired by the normal replacement. A container missing from the runtime is rebuilt from the pinned revision under the current policy. | |||
| 147 | + | - **Device grants have the same gap, documented rather than fixed.** Narrowing an `[app_devices]` mapping (removing a CDI ID) does not remove the device from an already created container, and a restart or recovery of that container keeps it. Resolution still fails closed for revoked or unknown policies, but Gordon cannot reliably read back the injected CDI devices (Podman's Docker-compatible inspect does not report them), so it cannot verify a narrowed mapping on reuse. After narrowing or revoking a device mapping, run `gordon apps deploy` for the affected service (or stop it) so the container is recreated with the current mapping. | |||
| 143 | - Adding or removing a policy name shows as `service/<name>/capabilities` in diffs and recreates the container on the next deploy; reorder-only input is a no-op. Changing what a policy grants does not recreate a running container: the next deploy serves the new resolution. | 148 | - Adding or removing a policy name shows as `service/<name>/capabilities` in diffs and recreates the container on the next deploy; reorder-only input is a no-op. Changing what a policy grants does not recreate a running container: the next deploy serves the new resolution. | ||
| 144 | 149 | ||||
| 145 | ## External image registries | 150 | ## External image registries | ||
M internal/adapters/out/docker/runtime.go +7 -1
| @@ -198,7 +198,7 @@ func (r *Runtime) CreateContainer(ctx context.Context, config *domain.ContainerC | |||
| 198 | 198 | resources.PidsLimit = &config.PidsLimit | |
| 199 | 199 | } | |
| 200 | 200 | capDrop := []string{"ALL"} | |
| 201 | - | capAdd := []string{"CHOWN", "DAC_OVERRIDE", "FOWNER", "SETUID", "SETGID", "NET_BIND_SERVICE"} | |
| 201 | + | capAdd := domain.DefaultAppCapAdd() | |
| 202 | 202 | if config.CapDrop != nil { | |
| 203 | 203 | capDrop = config.CapDrop | |
| 204 | 204 | } | |
| @@ -798,6 +798,11 @@ func (r *Runtime) InspectContainer(ctx context.Context, containerID string) (*do | |||
| 798 | 798 | }) | |
| 799 | 799 | } | |
| 800 | 800 | ||
| 801 | + | var capAdd []string | |
| 802 | + | if resp.HostConfig != nil { | |
| 803 | + | capAdd = append([]string(nil), resp.HostConfig.CapAdd...) | |
| 804 | + | } | |
| 805 | + | ||
| 801 | 806 | return &domain.Container{ | |
| 802 | 807 | ID: resp.ID, | |
| 803 | 808 | Image: resp.Config.Image, | |
| @@ -811,6 +816,7 @@ func (r *Runtime) InspectContainer(ctx context.Context, containerID string) (*do | |||
| 811 | 816 | Created: created, | |
| 812 | 817 | StartedAt: startedAt, | |
| 813 | 818 | Env: resp.Config.Env, | |
| 819 | + | CapAdd: capAdd, | |
| 814 | 820 | }, nil | |
| 815 | 821 | } | |
| 816 | 822 | ||
| @@ -198,7 +198,7 @@ func (r *Runtime) CreateContainer(ctx context.Context, config *domain.ContainerC | |||||
| 198 | resources.PidsLimit = &config.PidsLimit | 198 | resources.PidsLimit = &config.PidsLimit | ||
| 199 | } | 199 | } | ||
| 200 | capDrop := []string{"ALL"} | 200 | capDrop := []string{"ALL"} | ||
| 201 | - | capAdd := []string{"CHOWN", "DAC_OVERRIDE", "FOWNER", "SETUID", "SETGID", "NET_BIND_SERVICE"} | 201 | + | capAdd := domain.DefaultAppCapAdd() |
| 202 | if config.CapDrop != nil { | 202 | if config.CapDrop != nil { | ||
| 203 | capDrop = config.CapDrop | 203 | capDrop = config.CapDrop | ||
| 204 | } | 204 | } | ||
| @@ -798,6 +798,11 @@ func (r *Runtime) InspectContainer(ctx context.Context, containerID string) (*do | |||||
| 798 | }) | 798 | }) | ||
| 799 | } | 799 | } | ||
| 800 | 800 | ||||
| 801 | + | var capAdd []string | |||
| 802 | + | if resp.HostConfig != nil { | |||
| 803 | + | capAdd = append([]string(nil), resp.HostConfig.CapAdd...) | |||
| 804 | + | } | |||
| 805 | + | ||||
| 801 | return &domain.Container{ | 806 | return &domain.Container{ | ||
| 802 | ID: resp.ID, | 807 | ID: resp.ID, | ||
| 803 | Image: resp.Config.Image, | 808 | Image: resp.Config.Image, | ||
| @@ -811,6 +816,7 @@ func (r *Runtime) InspectContainer(ctx context.Context, containerID string) (*do | |||||
| 811 | Created: created, | 816 | Created: created, | ||
| 812 | StartedAt: startedAt, | 817 | StartedAt: startedAt, | ||
| 813 | Env: resp.Config.Env, | 818 | Env: resp.Config.Env, | ||
| 819 | + | CapAdd: capAdd, | |||
| 814 | }, nil | 820 | }, nil | ||
| 815 | } | 821 | } | ||
| 816 | 822 | ||||
M internal/adapters/out/docker/runtime_health_status_test.go +20 -0
| @@ -75,3 +75,23 @@ func TestRuntime_GetContainerHealthStatus_DisabledHealthcheck(t *testing.T) { | |||
| 75 | 75 | assert.False(t, hasHealthcheck) | |
| 76 | 76 | assert.Equal(t, "", status) | |
| 77 | 77 | } | |
| 78 | + | ||
| 79 | + | // TestRuntime_InspectContainer_ReportsConfiguredCapAdd proves the reuse | |
| 80 | + | // gate can see the capability set a container was created with. | |
| 81 | + | func TestRuntime_InspectContainer_ReportsConfiguredCapAdd(t *testing.T) { | |
| 82 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 83 | + | w.Header().Set("Content-Type", "application/json") | |
| 84 | + | _, _ = w.Write([]byte(`{ | |
| 85 | + | "Id":"abc123","Name":"/c","State":{"Status":"running"}, | |
| 86 | + | "Config":{"Image":"img"}, | |
| 87 | + | "HostConfig":{"CapAdd":["CAP_CHOWN","NET_ADMIN"]} | |
| 88 | + | }`)) | |
| 89 | + | })) | |
| 90 | + | defer server.Close() | |
| 91 | + | ||
| 92 | + | runtime := newRuntimeForHTTPServer(t, server) | |
| 93 | + | ctr, err := runtime.InspectContainer(context.Background(), "abc123") | |
| 94 | + | ||
| 95 | + | require.NoError(t, err) | |
| 96 | + | assert.Equal(t, []string{"CAP_CHOWN", "NET_ADMIN"}, ctr.CapAdd) | |
| 97 | + | } | |
| @@ -75,3 +75,23 @@ func TestRuntime_GetContainerHealthStatus_DisabledHealthcheck(t *testing.T) { | |||||
| 75 | assert.False(t, hasHealthcheck) | 75 | assert.False(t, hasHealthcheck) | ||
| 76 | assert.Equal(t, "", status) | 76 | assert.Equal(t, "", status) | ||
| 77 | } | 77 | } | ||
| 78 | + | ||||
| 79 | + | // TestRuntime_InspectContainer_ReportsConfiguredCapAdd proves the reuse | |||
| 80 | + | // gate can see the capability set a container was created with. | |||
| 81 | + | func TestRuntime_InspectContainer_ReportsConfiguredCapAdd(t *testing.T) { | |||
| 82 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 83 | + | w.Header().Set("Content-Type", "application/json") | |||
| 84 | + | _, _ = w.Write([]byte(`{ | |||
| 85 | + | "Id":"abc123","Name":"/c","State":{"Status":"running"}, | |||
| 86 | + | "Config":{"Image":"img"}, | |||
| 87 | + | "HostConfig":{"CapAdd":["CAP_CHOWN","NET_ADMIN"]} | |||
| 88 | + | }`)) | |||
| 89 | + | })) | |||
| 90 | + | defer server.Close() | |||
| 91 | + | ||||
| 92 | + | runtime := newRuntimeForHTTPServer(t, server) | |||
| 93 | + | ctr, err := runtime.InspectContainer(context.Background(), "abc123") | |||
| 94 | + | ||||
| 95 | + | require.NoError(t, err) | |||
| 96 | + | assert.Equal(t, []string{"CAP_CHOWN", "NET_ADMIN"}, ctr.CapAdd) | |||
| 97 | + | } | |||
M internal/domain/app_capability_policy.go +36 -0
| @@ -42,6 +42,42 @@ var allowedAppCapabilities = []string{ | |||
| 42 | 42 | "SYS_RESOURCE", | |
| 43 | 43 | } | |
| 44 | 44 | ||
| 45 | + | // defaultAppCapAdd is Gordon's base capability add set for app containers | |
| 46 | + | // (on top of cap_drop = ALL). Granted capabilities are added to it. | |
| 47 | + | var defaultAppCapAdd = []string{"CHOWN", "DAC_OVERRIDE", "FOWNER", "SETUID", "SETGID", "NET_BIND_SERVICE"} | |
| 48 | + | ||
| 49 | + | // DefaultAppCapAdd returns Gordon's base capability add set. The slice is a | |
| 50 | + | // copy. | |
| 51 | + | func DefaultAppCapAdd() []string { | |
| 52 | + | return slices.Clone(defaultAppCapAdd) | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | // NormalizeCapability returns the canonical form of a capability name as | |
| 56 | + | // reported by an engine: upper case without the CAP_ prefix. | |
| 57 | + | func NormalizeCapability(name string) string { | |
| 58 | + | return strings.TrimPrefix(strings.ToUpper(strings.TrimSpace(name)), "CAP_") | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | // ExcessCapabilities returns the sorted, normalized capabilities in actual | |
| 62 | + | // that are not in authorized. Both sides are normalized first, so engines | |
| 63 | + | // that report CAP_ prefixed names compare equal. A container holding fewer | |
| 64 | + | // capabilities than authorized is not excess. | |
| 65 | + | func ExcessCapabilities(actual, authorized []string) []string { | |
| 66 | + | allowed := make(map[string]struct{}, len(authorized)) | |
| 67 | + | for _, name := range authorized { | |
| 68 | + | allowed[NormalizeCapability(name)] = struct{}{} | |
| 69 | + | } | |
| 70 | + | var excess []string | |
| 71 | + | for _, name := range actual { | |
| 72 | + | n := NormalizeCapability(name) | |
| 73 | + | if _, ok := allowed[n]; !ok { | |
| 74 | + | excess = append(excess, n) | |
| 75 | + | } | |
| 76 | + | } | |
| 77 | + | slices.Sort(excess) | |
| 78 | + | return slices.Compact(excess) | |
| 79 | + | } | |
| 80 | + | ||
| 45 | 81 | // AllowedAppCapabilities returns the sorted capability names a policy may | |
| 46 | 82 | // add. The slice is a copy. | |
| 47 | 83 | func AllowedAppCapabilities() []string { | |
| @@ -42,6 +42,42 @@ var allowedAppCapabilities = []string{ | |||||
| 42 | "SYS_RESOURCE", | 42 | "SYS_RESOURCE", | ||
| 43 | } | 43 | } | ||
| 44 | 44 | ||||
| 45 | + | // defaultAppCapAdd is Gordon's base capability add set for app containers | |||
| 46 | + | // (on top of cap_drop = ALL). Granted capabilities are added to it. | |||
| 47 | + | var defaultAppCapAdd = []string{"CHOWN", "DAC_OVERRIDE", "FOWNER", "SETUID", "SETGID", "NET_BIND_SERVICE"} | |||
| 48 | + | ||||
| 49 | + | // DefaultAppCapAdd returns Gordon's base capability add set. The slice is a | |||
| 50 | + | // copy. | |||
| 51 | + | func DefaultAppCapAdd() []string { | |||
| 52 | + | return slices.Clone(defaultAppCapAdd) | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | // NormalizeCapability returns the canonical form of a capability name as | |||
| 56 | + | // reported by an engine: upper case without the CAP_ prefix. | |||
| 57 | + | func NormalizeCapability(name string) string { | |||
| 58 | + | return strings.TrimPrefix(strings.ToUpper(strings.TrimSpace(name)), "CAP_") | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | // ExcessCapabilities returns the sorted, normalized capabilities in actual | |||
| 62 | + | // that are not in authorized. Both sides are normalized first, so engines | |||
| 63 | + | // that report CAP_ prefixed names compare equal. A container holding fewer | |||
| 64 | + | // capabilities than authorized is not excess. | |||
| 65 | + | func ExcessCapabilities(actual, authorized []string) []string { | |||
| 66 | + | allowed := make(map[string]struct{}, len(authorized)) | |||
| 67 | + | for _, name := range authorized { | |||
| 68 | + | allowed[NormalizeCapability(name)] = struct{}{} | |||
| 69 | + | } | |||
| 70 | + | var excess []string | |||
| 71 | + | for _, name := range actual { | |||
| 72 | + | n := NormalizeCapability(name) | |||
| 73 | + | if _, ok := allowed[n]; !ok { | |||
| 74 | + | excess = append(excess, n) | |||
| 75 | + | } | |||
| 76 | + | } | |||
| 77 | + | slices.Sort(excess) | |||
| 78 | + | return slices.Compact(excess) | |||
| 79 | + | } | |||
| 80 | + | ||||
| 45 | // AllowedAppCapabilities returns the sorted capability names a policy may | 81 | // AllowedAppCapabilities returns the sorted capability names a policy may | ||
| 46 | // add. The slice is a copy. | 82 | // add. The slice is a copy. | ||
| 47 | func AllowedAppCapabilities() []string { | 83 | func AllowedAppCapabilities() []string { | ||
M internal/domain/app_capability_policy_test.go +14 -0
| @@ -177,3 +177,17 @@ func TestDiffAppSpecCapabilities(t *testing.T) { | |||
| 177 | 177 | assert.NotContains(t, diff.Changed, "service/worker/capabilities", "nil vs empty is a no-op") | |
| 178 | 178 | assert.NotContains(t, diff.Changed, "service/worker/devices") | |
| 179 | 179 | } | |
| 180 | + | ||
| 181 | + | func TestExcessCapabilities(t *testing.T) { | |
| 182 | + | authorized := append(DefaultAppCapAdd(), "IPC_LOCK") | |
| 183 | + | assert.Empty(t, ExcessCapabilities([]string{"CAP_CHOWN", "ipc_lock"}, authorized)) | |
| 184 | + | assert.Empty(t, ExcessCapabilities(nil, authorized)) | |
| 185 | + | assert.Equal(t, []string{"NET_ADMIN"}, ExcessCapabilities([]string{"CHOWN", "CAP_NET_ADMIN", "NET_ADMIN"}, authorized)) | |
| 186 | + | assert.Equal(t, "NET_ADMIN", NormalizeCapability(" cap_net_admin ")) | |
| 187 | + | } | |
| 188 | + | ||
| 189 | + | func TestDefaultAppCapAddIsCopy(t *testing.T) { | |
| 190 | + | d := DefaultAppCapAdd() | |
| 191 | + | d[0] = "mutated" | |
| 192 | + | assert.NotEqual(t, "mutated", DefaultAppCapAdd()[0]) | |
| 193 | + | } | |
| @@ -177,3 +177,17 @@ func TestDiffAppSpecCapabilities(t *testing.T) { | |||||
| 177 | assert.NotContains(t, diff.Changed, "service/worker/capabilities", "nil vs empty is a no-op") | 177 | assert.NotContains(t, diff.Changed, "service/worker/capabilities", "nil vs empty is a no-op") | ||
| 178 | assert.NotContains(t, diff.Changed, "service/worker/devices") | 178 | assert.NotContains(t, diff.Changed, "service/worker/devices") | ||
| 179 | } | 179 | } | ||
| 180 | + | ||||
| 181 | + | func TestExcessCapabilities(t *testing.T) { | |||
| 182 | + | authorized := append(DefaultAppCapAdd(), "IPC_LOCK") | |||
| 183 | + | assert.Empty(t, ExcessCapabilities([]string{"CAP_CHOWN", "ipc_lock"}, authorized)) | |||
| 184 | + | assert.Empty(t, ExcessCapabilities(nil, authorized)) | |||
| 185 | + | assert.Equal(t, []string{"NET_ADMIN"}, ExcessCapabilities([]string{"CHOWN", "CAP_NET_ADMIN", "NET_ADMIN"}, authorized)) | |||
| 186 | + | assert.Equal(t, "NET_ADMIN", NormalizeCapability(" cap_net_admin ")) | |||
| 187 | + | } | |||
| 188 | + | ||||
| 189 | + | func TestDefaultAppCapAddIsCopy(t *testing.T) { | |||
| 190 | + | d := DefaultAppCapAdd() | |||
| 191 | + | d[0] = "mutated" | |||
| 192 | + | assert.NotEqual(t, "mutated", DefaultAppCapAdd()[0]) | |||
| 193 | + | } | |||
M internal/domain/container.go +4 -0
| @@ -24,6 +24,10 @@ type Container struct { | |||
| 24 | 24 | // Env is the container's KEY=value environment as created. It may hold | |
| 25 | 25 | // secret values: compare it in memory only, never persist or log it. | |
| 26 | 26 | Env []string | |
| 27 | + | // CapAdd is the container's configured capability add set as reported | |
| 28 | + | // by the engine (names may carry a CAP_ prefix). Nil when the engine | |
| 29 | + | // reports none. Reuse paths compare it with the current authorization. | |
| 30 | + | CapAdd []string | |
| 27 | 31 | } | |
| 28 | 32 | ||
| 29 | 33 | // ContainerVolumeMount describes a mounted volume-like resource on a container. | |
| @@ -24,6 +24,10 @@ type Container struct { | |||||
| 24 | // Env is the container's KEY=value environment as created. It may hold | 24 | // Env is the container's KEY=value environment as created. It may hold | ||
| 25 | // secret values: compare it in memory only, never persist or log it. | 25 | // secret values: compare it in memory only, never persist or log it. | ||
| 26 | Env []string | 26 | Env []string | ||
| 27 | + | // CapAdd is the container's configured capability add set as reported | |||
| 28 | + | // by the engine (names may carry a CAP_ prefix). Nil when the engine | |||
| 29 | + | // reports none. Reuse paths compare it with the current authorization. | |||
| 30 | + | CapAdd []string | |||
| 27 | } | 31 | } | ||
| 28 | 32 | ||||
| 29 | // ContainerVolumeMount describes a mounted volume-like resource on a container. | 33 | // ContainerVolumeMount describes a mounted volume-like resource on a container. | ||
M internal/usecase/deployment/capability_policy_internal_test.go +128 -0
| @@ -2,6 +2,7 @@ package deployment | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "fmt" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/zerowrap" | |
| @@ -174,3 +175,130 @@ func TestUnchangedRefusal_CapabilitiesNeverKeepRunningContainer(t *testing.T) { | |||
| 174 | 175 | reason := svc.unchangedRefusal(context.Background(), "blog", p, domain.AppEffectiveService{Spec: p.spec}) | |
| 175 | 176 | assert.Equal(t, "administrative capabilities", reason) | |
| 176 | 177 | } | |
| 178 | + | ||
| 179 | + | // narrowedFixture models a policy narrowed from {NET_ADMIN, IPC_LOCK} to | |
| 180 | + | // {IPC_LOCK} after a container was created with both: policy resolution | |
| 181 | + | // still succeeds, but the existing container holds the revoked NET_ADMIN. | |
| 182 | + | func narrowedFixture(t *testing.T, containerCaps []string) (*Service, *outmocks.MockContainerRuntime, domain.AppEffectiveService) { | |
| 183 | + | t.Helper() | |
| 184 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 185 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()). | |
| 186 | + | WithCapabilityPolicies(map[string]domain.AppCapabilityPolicy{ | |
| 187 | + | "net-admin": {Name: "net-admin", Add: []string{"IPC_LOCK"}, AllowedApps: []string{"blog"}, AllowedServices: []string{"web"}}, | |
| 188 | + | }) | |
| 189 | + | eff := domain.AppEffectiveService{ | |
| 190 | + | Container: "c1", | |
| 191 | + | Spec: domain.AppService{Name: "web", Capabilities: []string{"net-admin"}}, | |
| 192 | + | } | |
| 193 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c1"). | |
| 194 | + | Return(&domain.Container{ID: "c1", CapAdd: containerCaps}, nil).Maybe() | |
| 195 | + | return svc, runtime, eff | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | var staleCaps = []string{"CHOWN", "DAC_OVERRIDE", "FOWNER", "SETUID", "SETGID", "NET_BIND_SERVICE", "NET_ADMIN", "IPC_LOCK"} | |
| 199 | + | ||
| 200 | + | func TestVerifyReusableCapabilities(t *testing.T) { | |
| 201 | + | t.Run("narrowed policy refuses a container holding a revoked capability", func(t *testing.T) { | |
| 202 | + | svc, _, eff := narrowedFixture(t, staleCaps) | |
| 203 | + | err := svc.verifyReusableCapabilities(context.Background(), "blog", eff) | |
| 204 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |
| 205 | + | assert.Contains(t, err.Error(), "NET_ADMIN") | |
| 206 | + | assert.NotContains(t, err.Error(), "IPC_LOCK") | |
| 207 | + | }) | |
| 208 | + | t.Run("CAP_ prefixed engine names compare equal", func(t *testing.T) { | |
| 209 | + | svc, _, eff := narrowedFixture(t, []string{"CAP_CHOWN", "CAP_NET_BIND_SERVICE", "CAP_IPC_LOCK"}) | |
| 210 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |
| 211 | + | }) | |
| 212 | + | t.Run("fewer capabilities than authorized is allowed", func(t *testing.T) { | |
| 213 | + | svc, _, eff := narrowedFixture(t, []string{"CHOWN"}) | |
| 214 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |
| 215 | + | }) | |
| 216 | + | t.Run("prefixed revoked capability is still caught", func(t *testing.T) { | |
| 217 | + | svc, _, eff := narrowedFixture(t, []string{"CAP_NET_ADMIN"}) | |
| 218 | + | require.ErrorIs(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff), domain.ErrCapabilityPolicy) | |
| 219 | + | }) | |
| 220 | + | t.Run("uninspectable container fails closed", func(t *testing.T) { | |
| 221 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 222 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c1").Return(nil, fmt.Errorf("daemon down at /run/podman.sock")).Once() | |
| 223 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()). | |
| 224 | + | WithCapabilityPolicies(map[string]domain.AppCapabilityPolicy{ | |
| 225 | + | "net-admin": capabilityPolicyFor([]string{"blog"}, []string{"web"}), | |
| 226 | + | }) | |
| 227 | + | eff := domain.AppEffectiveService{Container: "c1", Spec: domain.AppService{Name: "web", Capabilities: []string{"net-admin"}}} | |
| 228 | + | err := svc.verifyReusableCapabilities(context.Background(), "blog", eff) | |
| 229 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |
| 230 | + | assert.NotContains(t, err.Error(), "podman.sock") | |
| 231 | + | }) | |
| 232 | + | t.Run("missing container is left to the rebuild path", func(t *testing.T) { | |
| 233 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 234 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c1").Return(nil, domain.ErrContainerNotFound).Once() | |
| 235 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()). | |
| 236 | + | WithCapabilityPolicies(map[string]domain.AppCapabilityPolicy{ | |
| 237 | + | "net-admin": capabilityPolicyFor([]string{"blog"}, []string{"web"}), | |
| 238 | + | }) | |
| 239 | + | eff := domain.AppEffectiveService{Container: "c1", Spec: domain.AppService{Name: "web", Capabilities: []string{"net-admin"}}} | |
| 240 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |
| 241 | + | }) | |
| 242 | + | t.Run("service without capabilities never inspects", func(t *testing.T) { | |
| 243 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 244 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()) | |
| 245 | + | eff := domain.AppEffectiveService{Container: "c1", Spec: domain.AppService{Name: "web"}} | |
| 246 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |
| 247 | + | }) | |
| 248 | + | } | |
| 249 | + | ||
| 250 | + | func TestRestartOneService_NarrowedCapabilityPolicyRefusesInPlaceRestart(t *testing.T) { | |
| 251 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |
| 252 | + | op := &domain.AppOperation{Op: "op-1", Steps: []domain.AppOperationStep{{ | |
| 253 | + | ID: "service.web.restart", State: domain.AppStepPending, Before: eff.Container, | |
| 254 | + | }}} | |
| 255 | + | result := svc.restartOneService(context.Background(), "blog", "op-1", "web", eff, op, 0) | |
| 256 | + | ||
| 257 | + | assert.Equal(t, "failed", result.Result) | |
| 258 | + | assert.Equal(t, domain.AppStepFailed, op.Steps[0].State) | |
| 259 | + | assert.Contains(t, op.Steps[0].Error, "NET_ADMIN") | |
| 260 | + | runtime.AssertNotCalled(t, "RestartContainer") | |
| 261 | + | runtime.AssertNotCalled(t, "StartContainer") | |
| 262 | + | } | |
| 263 | + | ||
| 264 | + | func TestEnsureServiceRunning_NarrowedCapabilityPolicyRefusesStart(t *testing.T) { | |
| 265 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |
| 266 | + | op := &domain.AppOperation{Op: "op-1", Steps: []domain.AppOperationStep{{ | |
| 267 | + | ID: "service.web.start", State: domain.AppStepPending, Before: eff.Container, | |
| 268 | + | }}} | |
| 269 | + | result := &LifecycleResult{Services: map[string]ServiceResult{}} | |
| 270 | + | ||
| 271 | + | svc.ensureServiceRunning(context.Background(), "blog", "op-1", "web", eff, op, 0, result) | |
| 272 | + | ||
| 273 | + | assert.Equal(t, domain.AppStepFailed, op.Steps[0].State) | |
| 274 | + | assert.Contains(t, op.Steps[0].Error, "NET_ADMIN") | |
| 275 | + | runtime.AssertNotCalled(t, "StartContainer") | |
| 276 | + | runtime.AssertNotCalled(t, "IsContainerRunning") | |
| 277 | + | } | |
| 278 | + | ||
| 279 | + | func TestStartRecovered_NarrowedCapabilityPolicyRefusesStart(t *testing.T) { | |
| 280 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |
| 281 | + | started, err := svc.startRecovered(context.Background(), "blog", "web", eff, recoveryKey{app: "blog", service: "web", container: "c1"}) | |
| 282 | + | ||
| 283 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |
| 284 | + | assert.False(t, started) | |
| 285 | + | runtime.AssertNotCalled(t, "StartContainer") | |
| 286 | + | } | |
| 287 | + | ||
| 288 | + | func TestRecoverHealth_NarrowedCapabilityPolicyRefusesRestart(t *testing.T) { | |
| 289 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |
| 290 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, "c1").Return("unhealthy", true, nil).Once() | |
| 291 | + | key := recoveryKey{app: "blog", service: "web", container: "c1"} | |
| 292 | + | for i := 0; i < unhealthyThreshold; i++ { | |
| 293 | + | _, _, err := svc.recoverHealth(context.Background(), "blog", "web", eff, key) | |
| 294 | + | if err != nil { | |
| 295 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |
| 296 | + | runtime.AssertNotCalled(t, "RestartContainer") | |
| 297 | + | return | |
| 298 | + | } | |
| 299 | + | if i < unhealthyThreshold-1 { | |
| 300 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, "c1").Return("unhealthy", true, nil).Once() | |
| 301 | + | } | |
| 302 | + | } | |
| 303 | + | t.Fatal("expected recoverHealth to refuse the restart") | |
| 304 | + | } | |
| @@ -2,6 +2,7 @@ package deployment | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "fmt" | |||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/zerowrap" | 8 | "github.com/bnema/zerowrap" | ||
| @@ -174,3 +175,130 @@ func TestUnchangedRefusal_CapabilitiesNeverKeepRunningContainer(t *testing.T) { | |||||
| 174 | reason := svc.unchangedRefusal(context.Background(), "blog", p, domain.AppEffectiveService{Spec: p.spec}) | 175 | reason := svc.unchangedRefusal(context.Background(), "blog", p, domain.AppEffectiveService{Spec: p.spec}) | ||
| 175 | assert.Equal(t, "administrative capabilities", reason) | 176 | assert.Equal(t, "administrative capabilities", reason) | ||
| 176 | } | 177 | } | ||
| 178 | + | ||||
| 179 | + | // narrowedFixture models a policy narrowed from {NET_ADMIN, IPC_LOCK} to | |||
| 180 | + | // {IPC_LOCK} after a container was created with both: policy resolution | |||
| 181 | + | // still succeeds, but the existing container holds the revoked NET_ADMIN. | |||
| 182 | + | func narrowedFixture(t *testing.T, containerCaps []string) (*Service, *outmocks.MockContainerRuntime, domain.AppEffectiveService) { | |||
| 183 | + | t.Helper() | |||
| 184 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 185 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()). | |||
| 186 | + | WithCapabilityPolicies(map[string]domain.AppCapabilityPolicy{ | |||
| 187 | + | "net-admin": {Name: "net-admin", Add: []string{"IPC_LOCK"}, AllowedApps: []string{"blog"}, AllowedServices: []string{"web"}}, | |||
| 188 | + | }) | |||
| 189 | + | eff := domain.AppEffectiveService{ | |||
| 190 | + | Container: "c1", | |||
| 191 | + | Spec: domain.AppService{Name: "web", Capabilities: []string{"net-admin"}}, | |||
| 192 | + | } | |||
| 193 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c1"). | |||
| 194 | + | Return(&domain.Container{ID: "c1", CapAdd: containerCaps}, nil).Maybe() | |||
| 195 | + | return svc, runtime, eff | |||
| 196 | + | } | |||
| 197 | + | ||||
| 198 | + | var staleCaps = []string{"CHOWN", "DAC_OVERRIDE", "FOWNER", "SETUID", "SETGID", "NET_BIND_SERVICE", "NET_ADMIN", "IPC_LOCK"} | |||
| 199 | + | ||||
| 200 | + | func TestVerifyReusableCapabilities(t *testing.T) { | |||
| 201 | + | t.Run("narrowed policy refuses a container holding a revoked capability", func(t *testing.T) { | |||
| 202 | + | svc, _, eff := narrowedFixture(t, staleCaps) | |||
| 203 | + | err := svc.verifyReusableCapabilities(context.Background(), "blog", eff) | |||
| 204 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |||
| 205 | + | assert.Contains(t, err.Error(), "NET_ADMIN") | |||
| 206 | + | assert.NotContains(t, err.Error(), "IPC_LOCK") | |||
| 207 | + | }) | |||
| 208 | + | t.Run("CAP_ prefixed engine names compare equal", func(t *testing.T) { | |||
| 209 | + | svc, _, eff := narrowedFixture(t, []string{"CAP_CHOWN", "CAP_NET_BIND_SERVICE", "CAP_IPC_LOCK"}) | |||
| 210 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |||
| 211 | + | }) | |||
| 212 | + | t.Run("fewer capabilities than authorized is allowed", func(t *testing.T) { | |||
| 213 | + | svc, _, eff := narrowedFixture(t, []string{"CHOWN"}) | |||
| 214 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |||
| 215 | + | }) | |||
| 216 | + | t.Run("prefixed revoked capability is still caught", func(t *testing.T) { | |||
| 217 | + | svc, _, eff := narrowedFixture(t, []string{"CAP_NET_ADMIN"}) | |||
| 218 | + | require.ErrorIs(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff), domain.ErrCapabilityPolicy) | |||
| 219 | + | }) | |||
| 220 | + | t.Run("uninspectable container fails closed", func(t *testing.T) { | |||
| 221 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 222 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c1").Return(nil, fmt.Errorf("daemon down at /run/podman.sock")).Once() | |||
| 223 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()). | |||
| 224 | + | WithCapabilityPolicies(map[string]domain.AppCapabilityPolicy{ | |||
| 225 | + | "net-admin": capabilityPolicyFor([]string{"blog"}, []string{"web"}), | |||
| 226 | + | }) | |||
| 227 | + | eff := domain.AppEffectiveService{Container: "c1", Spec: domain.AppService{Name: "web", Capabilities: []string{"net-admin"}}} | |||
| 228 | + | err := svc.verifyReusableCapabilities(context.Background(), "blog", eff) | |||
| 229 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |||
| 230 | + | assert.NotContains(t, err.Error(), "podman.sock") | |||
| 231 | + | }) | |||
| 232 | + | t.Run("missing container is left to the rebuild path", func(t *testing.T) { | |||
| 233 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 234 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c1").Return(nil, domain.ErrContainerNotFound).Once() | |||
| 235 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()). | |||
| 236 | + | WithCapabilityPolicies(map[string]domain.AppCapabilityPolicy{ | |||
| 237 | + | "net-admin": capabilityPolicyFor([]string{"blog"}, []string{"web"}), | |||
| 238 | + | }) | |||
| 239 | + | eff := domain.AppEffectiveService{Container: "c1", Spec: domain.AppService{Name: "web", Capabilities: []string{"net-admin"}}} | |||
| 240 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |||
| 241 | + | }) | |||
| 242 | + | t.Run("service without capabilities never inspects", func(t *testing.T) { | |||
| 243 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 244 | + | svc := NewService(Deps{Runtime: runtime}, zerowrap.Default()) | |||
| 245 | + | eff := domain.AppEffectiveService{Container: "c1", Spec: domain.AppService{Name: "web"}} | |||
| 246 | + | require.NoError(t, svc.verifyReusableCapabilities(context.Background(), "blog", eff)) | |||
| 247 | + | }) | |||
| 248 | + | } | |||
| 249 | + | ||||
| 250 | + | func TestRestartOneService_NarrowedCapabilityPolicyRefusesInPlaceRestart(t *testing.T) { | |||
| 251 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |||
| 252 | + | op := &domain.AppOperation{Op: "op-1", Steps: []domain.AppOperationStep{{ | |||
| 253 | + | ID: "service.web.restart", State: domain.AppStepPending, Before: eff.Container, | |||
| 254 | + | }}} | |||
| 255 | + | result := svc.restartOneService(context.Background(), "blog", "op-1", "web", eff, op, 0) | |||
| 256 | + | ||||
| 257 | + | assert.Equal(t, "failed", result.Result) | |||
| 258 | + | assert.Equal(t, domain.AppStepFailed, op.Steps[0].State) | |||
| 259 | + | assert.Contains(t, op.Steps[0].Error, "NET_ADMIN") | |||
| 260 | + | runtime.AssertNotCalled(t, "RestartContainer") | |||
| 261 | + | runtime.AssertNotCalled(t, "StartContainer") | |||
| 262 | + | } | |||
| 263 | + | ||||
| 264 | + | func TestEnsureServiceRunning_NarrowedCapabilityPolicyRefusesStart(t *testing.T) { | |||
| 265 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |||
| 266 | + | op := &domain.AppOperation{Op: "op-1", Steps: []domain.AppOperationStep{{ | |||
| 267 | + | ID: "service.web.start", State: domain.AppStepPending, Before: eff.Container, | |||
| 268 | + | }}} | |||
| 269 | + | result := &LifecycleResult{Services: map[string]ServiceResult{}} | |||
| 270 | + | ||||
| 271 | + | svc.ensureServiceRunning(context.Background(), "blog", "op-1", "web", eff, op, 0, result) | |||
| 272 | + | ||||
| 273 | + | assert.Equal(t, domain.AppStepFailed, op.Steps[0].State) | |||
| 274 | + | assert.Contains(t, op.Steps[0].Error, "NET_ADMIN") | |||
| 275 | + | runtime.AssertNotCalled(t, "StartContainer") | |||
| 276 | + | runtime.AssertNotCalled(t, "IsContainerRunning") | |||
| 277 | + | } | |||
| 278 | + | ||||
| 279 | + | func TestStartRecovered_NarrowedCapabilityPolicyRefusesStart(t *testing.T) { | |||
| 280 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |||
| 281 | + | started, err := svc.startRecovered(context.Background(), "blog", "web", eff, recoveryKey{app: "blog", service: "web", container: "c1"}) | |||
| 282 | + | ||||
| 283 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |||
| 284 | + | assert.False(t, started) | |||
| 285 | + | runtime.AssertNotCalled(t, "StartContainer") | |||
| 286 | + | } | |||
| 287 | + | ||||
| 288 | + | func TestRecoverHealth_NarrowedCapabilityPolicyRefusesRestart(t *testing.T) { | |||
| 289 | + | svc, runtime, eff := narrowedFixture(t, staleCaps) | |||
| 290 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, "c1").Return("unhealthy", true, nil).Once() | |||
| 291 | + | key := recoveryKey{app: "blog", service: "web", container: "c1"} | |||
| 292 | + | for i := 0; i < unhealthyThreshold; i++ { | |||
| 293 | + | _, _, err := svc.recoverHealth(context.Background(), "blog", "web", eff, key) | |||
| 294 | + | if err != nil { | |||
| 295 | + | require.ErrorIs(t, err, domain.ErrCapabilityPolicy) | |||
| 296 | + | runtime.AssertNotCalled(t, "RestartContainer") | |||
| 297 | + | return | |||
| 298 | + | } | |||
| 299 | + | if i < unhealthyThreshold-1 { | |||
| 300 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, "c1").Return("unhealthy", true, nil).Once() | |||
| 301 | + | } | |||
| 302 | + | } | |||
| 303 | + | t.Fatal("expected recoverHealth to refuse the restart") | |||
| 304 | + | } | |||
M internal/usecase/deployment/lifecycle.go +2 -2
| @@ -333,7 +333,7 @@ func (s *Service) restartOneService(ctx context.Context, app, opID, name string, | |||
| 333 | 333 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | |
| 334 | 334 | return fail(err.Error()) | |
| 335 | 335 | } | |
| 336 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | |
| 336 | + | if err := s.verifyReusableCapabilities(ctx, app, eff); err != nil { | |
| 337 | 337 | return fail(err.Error()) | |
| 338 | 338 | } | |
| 339 | 339 | if err := s.refuseInhibitedRestart(ctx, app, name, eff.Container); err != nil { | |
| @@ -711,7 +711,7 @@ func (s *Service) ensureServiceRunning(ctx context.Context, app, opID, name stri | |||
| 711 | 711 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | |
| 712 | 712 | return | |
| 713 | 713 | } | |
| 714 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | |
| 714 | + | if err := s.verifyReusableCapabilities(ctx, app, eff); err != nil { | |
| 715 | 715 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | |
| 716 | 716 | return | |
| 717 | 717 | } | |
| @@ -333,7 +333,7 @@ func (s *Service) restartOneService(ctx context.Context, app, opID, name string, | |||||
| 333 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | 333 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | ||
| 334 | return fail(err.Error()) | 334 | return fail(err.Error()) | ||
| 335 | } | 335 | } | ||
| 336 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | 336 | + | if err := s.verifyReusableCapabilities(ctx, app, eff); err != nil { |
| 337 | return fail(err.Error()) | 337 | return fail(err.Error()) | ||
| 338 | } | 338 | } | ||
| 339 | if err := s.refuseInhibitedRestart(ctx, app, name, eff.Container); err != nil { | 339 | if err := s.refuseInhibitedRestart(ctx, app, name, eff.Container); err != nil { | ||
| @@ -711,7 +711,7 @@ func (s *Service) ensureServiceRunning(ctx context.Context, app, opID, name stri | |||||
| 711 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | 711 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | ||
| 712 | return | 712 | return | ||
| 713 | } | 713 | } | ||
| 714 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | 714 | + | if err := s.verifyReusableCapabilities(ctx, app, eff); err != nil { |
| 715 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | 715 | s.failServiceStep(ctx, app, name, eff, step, result, err.Error()) | ||
| 716 | return | 716 | return | ||
| 717 | } | 717 | } | ||
M internal/usecase/deployment/monitor.go +2 -2
| @@ -739,7 +739,7 @@ func (s *Service) startRecovered(serviceCtx context.Context, app, name string, e | |||
| 739 | 739 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | |
| 740 | 740 | return false, err | |
| 741 | 741 | } | |
| 742 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | |
| 742 | + | if err := s.verifyReusableCapabilities(serviceCtx, app, eff); err != nil { | |
| 743 | 743 | return false, err | |
| 744 | 744 | } | |
| 745 | 745 | if err := s.deps.Runtime.StartContainer(serviceCtx, eff.Container); err != nil { | |
| @@ -798,7 +798,7 @@ func (s *Service) recoverHealth(serviceCtx context.Context, app, name string, ef | |||
| 798 | 798 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | |
| 799 | 799 | return false, false, err | |
| 800 | 800 | } | |
| 801 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | |
| 801 | + | if err := s.verifyReusableCapabilities(serviceCtx, app, eff); err != nil { | |
| 802 | 802 | return false, false, err | |
| 803 | 803 | } | |
| 804 | 804 | if err := s.deps.Runtime.RestartContainer(serviceCtx, eff.Container, serviceStopGrace(eff)); err != nil { | |
| @@ -739,7 +739,7 @@ func (s *Service) startRecovered(serviceCtx context.Context, app, name string, e | |||||
| 739 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | 739 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | ||
| 740 | return false, err | 740 | return false, err | ||
| 741 | } | 741 | } | ||
| 742 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | 742 | + | if err := s.verifyReusableCapabilities(serviceCtx, app, eff); err != nil { |
| 743 | return false, err | 743 | return false, err | ||
| 744 | } | 744 | } | ||
| 745 | if err := s.deps.Runtime.StartContainer(serviceCtx, eff.Container); err != nil { | 745 | if err := s.deps.Runtime.StartContainer(serviceCtx, eff.Container); err != nil { | ||
| @@ -798,7 +798,7 @@ func (s *Service) recoverHealth(serviceCtx context.Context, app, name string, ef | |||||
| 798 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | 798 | if _, err := s.resolveServiceDevices(app, eff.Spec); err != nil { | ||
| 799 | return false, false, err | 799 | return false, false, err | ||
| 800 | } | 800 | } | ||
| 801 | - | if _, err := s.resolveServiceCapabilities(app, eff.Spec); err != nil { | 801 | + | if err := s.verifyReusableCapabilities(serviceCtx, app, eff); err != nil { |
| 802 | return false, false, err | 802 | return false, false, err | ||
| 803 | } | 803 | } | ||
| 804 | if err := s.deps.Runtime.RestartContainer(serviceCtx, eff.Container, serviceStopGrace(eff)); err != nil { | 804 | if err := s.deps.Runtime.RestartContainer(serviceCtx, eff.Container, serviceStopGrace(eff)); err != nil { | ||
M internal/usecase/deployment/service.go +33 -0
| @@ -292,6 +292,39 @@ func (s *Service) resolveServiceCapabilities(app string, svc domain.AppService) | |||
| 292 | 292 | return caps, nil | |
| 293 | 293 | } | |
| 294 | 294 | ||
| 295 | + | // verifyReusableCapabilities gates every path that reuses an existing | |
| 296 | + | // capability-bearing container (restart, start, health recovery). Policy | |
| 297 | + | // resolution alone is not enough: the container keeps the capability set it | |
| 298 | + | // was created with, so a policy narrowed after creation would still run with | |
| 299 | + | // the revoked capability. The container's configured CapAdd must therefore | |
| 300 | + | // not exceed Gordon's defaults plus the currently authorized grant; | |
| 301 | + | // otherwise the reuse is refused and a deploy must recreate it. A container | |
| 302 | + | // holding fewer capabilities than authorized is fine (least privilege). | |
| 303 | + | // A container that no longer exists is left to the caller's existing | |
| 304 | + | // rebuild path, which re-resolves the policy. | |
| 305 | + | func (s *Service) verifyReusableCapabilities(ctx context.Context, app string, eff domain.AppEffectiveService) error { | |
| 306 | + | granted, err := s.resolveServiceCapabilities(app, eff.Spec) | |
| 307 | + | if err != nil { | |
| 308 | + | return err | |
| 309 | + | } | |
| 310 | + | if len(eff.Spec.Capabilities) == 0 || eff.Container == "" { | |
| 311 | + | return nil | |
| 312 | + | } | |
| 313 | + | ctr, err := s.deps.Runtime.InspectContainer(ctx, eff.Container) | |
| 314 | + | if err != nil { | |
| 315 | + | if errors.Is(err, domain.ErrContainerNotFound) { | |
| 316 | + | return nil | |
| 317 | + | } | |
| 318 | + | return fmt.Errorf("deployment: app %q service %q: cannot verify container capabilities: %w", app, eff.Spec.Name, domain.ErrCapabilityPolicy) | |
| 319 | + | } | |
| 320 | + | authorized := append(domain.DefaultAppCapAdd(), granted...) | |
| 321 | + | if excess := domain.ExcessCapabilities(ctr.CapAdd, authorized); len(excess) > 0 { | |
| 322 | + | return fmt.Errorf("deployment: app %q service %q: container holds capabilities beyond the current policy (%s); deploy to recreate it: %w", | |
| 323 | + | app, eff.Spec.Name, strings.Join(excess, ", "), domain.ErrCapabilityPolicy) | |
| 324 | + | } | |
| 325 | + | return nil | |
| 326 | + | } | |
| 327 | + | ||
| 295 | 328 | // resolveServiceGrants resolves the device and capability policies of a | |
| 296 | 329 | // service in one fail-closed step. | |
| 297 | 330 | func (s *Service) resolveServiceGrants(app string, svc domain.AppService) (devices, caps []string, err error) { | |
| @@ -292,6 +292,39 @@ func (s *Service) resolveServiceCapabilities(app string, svc domain.AppService) | |||||
| 292 | return caps, nil | 292 | return caps, nil | ||
| 293 | } | 293 | } | ||
| 294 | 294 | ||||
| 295 | + | // verifyReusableCapabilities gates every path that reuses an existing | |||
| 296 | + | // capability-bearing container (restart, start, health recovery). Policy | |||
| 297 | + | // resolution alone is not enough: the container keeps the capability set it | |||
| 298 | + | // was created with, so a policy narrowed after creation would still run with | |||
| 299 | + | // the revoked capability. The container's configured CapAdd must therefore | |||
| 300 | + | // not exceed Gordon's defaults plus the currently authorized grant; | |||
| 301 | + | // otherwise the reuse is refused and a deploy must recreate it. A container | |||
| 302 | + | // holding fewer capabilities than authorized is fine (least privilege). | |||
| 303 | + | // A container that no longer exists is left to the caller's existing | |||
| 304 | + | // rebuild path, which re-resolves the policy. | |||
| 305 | + | func (s *Service) verifyReusableCapabilities(ctx context.Context, app string, eff domain.AppEffectiveService) error { | |||
| 306 | + | granted, err := s.resolveServiceCapabilities(app, eff.Spec) | |||
| 307 | + | if err != nil { | |||
| 308 | + | return err | |||
| 309 | + | } | |||
| 310 | + | if len(eff.Spec.Capabilities) == 0 || eff.Container == "" { | |||
| 311 | + | return nil | |||
| 312 | + | } | |||
| 313 | + | ctr, err := s.deps.Runtime.InspectContainer(ctx, eff.Container) | |||
| 314 | + | if err != nil { | |||
| 315 | + | if errors.Is(err, domain.ErrContainerNotFound) { | |||
| 316 | + | return nil | |||
| 317 | + | } | |||
| 318 | + | return fmt.Errorf("deployment: app %q service %q: cannot verify container capabilities: %w", app, eff.Spec.Name, domain.ErrCapabilityPolicy) | |||
| 319 | + | } | |||
| 320 | + | authorized := append(domain.DefaultAppCapAdd(), granted...) | |||
| 321 | + | if excess := domain.ExcessCapabilities(ctr.CapAdd, authorized); len(excess) > 0 { | |||
| 322 | + | return fmt.Errorf("deployment: app %q service %q: container holds capabilities beyond the current policy (%s); deploy to recreate it: %w", | |||
| 323 | + | app, eff.Spec.Name, strings.Join(excess, ", "), domain.ErrCapabilityPolicy) | |||
| 324 | + | } | |||
| 325 | + | return nil | |||
| 326 | + | } | |||
| 327 | + | ||||
| 295 | // resolveServiceGrants resolves the device and capability policies of a | 328 | // resolveServiceGrants resolves the device and capability policies of a | ||
| 296 | // service in one fail-closed step. | 329 | // service in one fail-closed step. | ||
| 297 | func (s *Service) resolveServiceGrants(app string, svc domain.AppService) (devices, caps []string, err error) { | 330 | func (s *Service) resolveServiceGrants(app string, svc domain.AppService) (devices, caps []string, err error) { | ||