diff --git a/AGENTS.md b/AGENTS.md index 79f3283..84a3532 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,17 +39,17 @@ deploy/ — Docker Compose + integration tests - Zero external deps where possible (Go: yaml.v3, Rust: tokio/hyper/serde/clap, TS: yaml) ## Test Coverage -- Go: 131 unit tests (main/listener: 24, policy: 14, middleware: 28, proxy: 61, audit: 4); the middleware count includes `TestHostConfigRows`, whose 131 #88 rows run as subtests -- Rust: 158 unit tests (main/listener: 23, policy: 16, middleware: 44, proxy: 52, handler: 17, audit: 4, transport: 2); `test_host_config_rows` runs the 131 #88 rows -- TypeScript: 312 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 165 incl. the 131 #88 rows, proxy: 40, policy: 15, handler: 21, shutdown: 5, transport: 5, audit: 4) -- Integration, per implementation: 108 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose) +- Go: 133 unit tests (main/listener: 24, policy: 15, middleware: 29, proxy: 61, audit: 4); the middleware count includes `TestHostConfigRows`, whose 139 #88 and #93 rows run as subtests, and `TestGroupAddGateRows` (9 #93 rows) +- Rust: 160 unit tests (main/listener: 23, policy: 17, middleware: 45, proxy: 52, handler: 17, audit: 4, transport: 2); `test_host_config_rows` runs the 139 #88 and #93 rows, `test_group_add_gate_rows` the 9 #93 gate rows +- TypeScript: 330 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 182 incl. the 139 #88 and #93 rows and the 9 #93 gate rows, proxy: 40, policy: 16, handler: 21, shutdown: 5, transport: 5, audit: 4) +- Integration, per implementation: 116 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose) - Quint: `make test-spec` runs the `spec/listener.qnt` `run` tests (instances `listener_locked`, `listener_unlocked`) and the `spec/router.qnt` `run` tests (instances `router`, `router_pre48`, `router_pre53`, `router_pre55`, `router_readonly`, whose `readonly*` tests run with `--match`, `router_pre76`, `router_pre78`, `router_pre70` and `router_pre86`; and, for the image pull, `pull`, `pull_pre83`, `pull_body_ignored` and `pull_pre84`), and the `spec/docker_socket_policy.qnt` `run` tests (instances `network_endpoints`, `network_endpoints_pre87`, `create` and `create_pre88`, and the policy-selection tests of the main module `docker_socket_policy`, #85) ## Test Conventions - Go: stdlib `testing` package, `go test ./...` - Rust: `#[cfg(test)]` inline modules, `cargo test` - TypeScript: `node:test` framework, `npm run build && node --test dist/*.test.js` -- Integration: `make test-integration` (108 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose +- Integration: `make test-integration` (116 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose ## Contribution Workflow diff --git a/README.md b/README.md index e30cd06..8a0e994 100644 --- a/README.md +++ b/README.md @@ -100,9 +100,9 @@ All three implementations expose the same API surface, share the same [Quint spe | Language | Directory | Tests | Stack | |----------|-----------|-------|-------| -| Go | [go/](go/) | 131 unit + 108 integration + 15 socket | stdlib net/http + yaml.v3 | -| Rust | [rs/](rs/) | 158 unit + 108 integration + 15 socket | tokio, hyper, serde, clap | -| TypeScript | [ts/](ts/) | 312 unit (1 skipped) + 108 integration + 15 socket | Node 22 ESM, built-in http | +| Go | [go/](go/) | 133 unit + 116 integration + 15 socket | stdlib net/http + yaml.v3 | +| Rust | [rs/](rs/) | 160 unit + 116 integration + 15 socket | tokio, hyper, serde, clap | +| TypeScript | [ts/](ts/) | 330 unit (1 skipped) + 116 integration + 15 socket | Node 22 ESM, built-in http | ### Build All @@ -157,6 +157,8 @@ container_config: security_options: - no-new-privileges:true user: '2001:2001' + group_add: # optional; forced as HostConfig.GroupAdd + - '64190' # host GID of the jwt group (quote GIDs) volumes: - host_path: /home/beacon container_path: /data @@ -182,6 +184,22 @@ the bind must carry `:ro`, or the mount `ReadOnly: true`, and a bind without a mode is read-write in Docker, so it is denied. See [Container create: HostConfig rules](#container-create-hostconfig-rules). +`container_config.group_add` is optional +([#93](https://github.com/ChainSafe/docker-socket-policy/issues/93)). `user` +sets only the primary UID and GID, and the daemon checks a host-mounted file +against the GIDs the process has, so a service that reads a file such as a +`root:jwt 0640` JWT secret needs the host group as a supplementary group. The +mutator overwrites `HostConfig.GroupAdd` with the list, as `user` overwrites +`User`: a caller's own list (`community.docker.docker_container`'s `groups:`, +`docker run --group-add`) is replaced, not denied, and a caller that sends the +same list sees no change. Each entry must be a quoted YAML string holding a +decimal GID that fits in 32 bits or a group name +(`[a-z_][a-z0-9_-]*$?`, at most 32 characters); `0` (in any decimal form) and +`root`, duplicates and anything else fail startup with an error that names the +file. An empty list is the same as leaving the key out. A group name is +resolved by the daemon against the container's `/etc/group` when it starts; +a GID needs no entry there. + `container_names` is optional. It lists the container names the policy manages, each a valid Docker name (`[a-zA-Z0-9][a-zA-Z0-9_.-]*`; anything else fails startup). It does two things @@ -246,7 +264,7 @@ docker run --mount type=bind,src=/,dst=/host chainsafe/lodestar:next # denied: | Middleware | Type | What it checks | |------------|------|----------------| -| ContainerConfigMutator | Mutator | Forces `network_mode`, `user`, `security_options`, `restart_policy`, `log_driver` from policy, `HostConfig.Privileged: false` on every create, and makes `NetworkingConfig.EndpointsConfig` match a forced `network_mode` | +| ContainerConfigMutator | Mutator | Forces `network_mode`, `user`, `group_add`, `security_options`, `restart_policy`, `log_driver` from policy, `HostConfig.Privileged: false` on every create, and makes `NetworkingConfig.EndpointsConfig` match a forced `network_mode` | | RegistryGate | Gate | Validates the normalised image ref against `allowed_image_prefixes`, then `image_tag_pattern` and `image_digest_allowed` | | MountSourceGate | Gate | Validates `HostConfig.Binds`, `HostConfig.Mounts` and top-level `Volumes` against `volumes`, including bind options and `read_write: false` | | EnvFileGate | Gate | Strips `Env` field from create body; env must come from locked `env_file` | @@ -268,6 +286,7 @@ The chain runs only on container create. The mutator runs first, so the gates ch | `Mounts` | `Type: bind`: `Source` must be a policy `volumes` entry; `ReadOnly: true` for a `read_write: false` volume; `BindOptions.Propagation` empty, `private` or `rprivate`; `BindOptions.ReadOnlyNonRecursive` must be zero. `Type: tmpfs`: allowed, without a `Source`. Every other type (`volume`, `image`, `npipe`, `cluster`) is denied. Other entry keys besides `Target`, `Consistency` and `TmpfsOptions` must be zero. | | `NetworkMode`, `RestartPolicy`, `LogConfig` | Any value; overwritten when the policy sets `network_mode`, `restart_policy` or `log_driver`. | | `SecurityOpt` | Each entry must be in the policy's `security_options` or be `no-new-privileges` / `no-new-privileges:true`. Overwritten when the policy sets `security_options`. | +| `GroupAdd` | Absent or zero, unless the policy sets `group_add`: the mutator then overwrites it with that list, and the gate allows exactly that list, in order ([#93](https://github.com/ChainSafe/docker-socket-policy/issues/93)). | | `Privileged` | Forced to `false` by the mutator on every create, so `true` is neutralised, not denied. | | `PidMode`, `UTSMode`, `UsernsMode`, `Cgroup` | `""` only. | | `IpcMode` | `""`, `private`, `shareable` or `none`. | @@ -276,7 +295,7 @@ The chain runs only on container create. The mutator runs first, so the gates ch | `OomScoreAdj` | `0` or more. | | `MaskedPaths`, `ReadonlyPaths` | Absent or `null` only: an empty list unmasks `/proc` (`--security-opt systempaths=unconfined`). | | `CapDrop`, `ReadonlyRootfs`, `Init`, `AutoRemove`, `ConsoleSize`, `ShmSize`, `PortBindings`, `PublishAllPorts`, `Dns`, `DnsOptions`, `DnsSearch`, `ExtraHosts`, `Tmpfs`, `Ulimits`, `Memory`, `MemoryReservation`, `MemorySwap`, `MemorySwappiness`, `NanoCpus`, `CpuShares`, `CpuPeriod`, `CpuQuota`, `CpusetCpus`, `CpusetMems`, `PidsLimit`, `BlkioWeight`, `ContainerIDFile` | Any value. | -| Everything else: `CapAdd`, `Devices`, `DeviceRequests`, `DeviceCgroupRules`, `GroupAdd`, `VolumesFrom`, `Links`, `Sysctls`, `CgroupParent`, `Runtime`, `VolumeDriver`, `OomKillDisable`, `Annotations`, `StorageOpt`, the other resource fields, and unknown keys | Absent or zero. | +| Everything else: `CapAdd`, `Devices`, `DeviceRequests`, `DeviceCgroupRules`, `VolumesFrom`, `Links`, `Sysctls`, `CgroupParent`, `Runtime`, `VolumeDriver`, `OomKillDisable`, `Annotations`, `StorageOpt`, the other resource fields, and unknown keys | Absent or zero. | The daemon matches JSON field names without regard to case, and the last match wins, so `"privileged": true` would set `Privileged` after the mutator's `"Privileged": false`, and `"securityopt": null` would clear a forced `SecurityOpt`. A key that the daemon would read as a known field but that is not spelled exactly like it (`privileged`, `hostConfig`, `user`, `image`, …) is denied, at the top level of the body, in `HostConfig`, in `Mounts` entries and their `BindOptions`, and in `NetworkingConfig`, whether or not the policy forces `network_mode`: `NetworkEndpointsGate` reads `EndpointsConfig` only, and the daemon would attach the networks under `endpointsconfig` too. Inside `RestartPolicy` and `LogConfig` the spelling does not matter: the mutator replaces the whole object when the policy sets it, and otherwise any value is allowed; no gate reads `Healthcheck` or the settings inside an endpoint entry. The policy's `ports` field is parsed but not enforced; `PortBindings` is not restricted by it. diff --git a/config/beacon.yaml b/config/beacon.yaml index 2fd96c2..7623125 100644 --- a/config/beacon.yaml +++ b/config/beacon.yaml @@ -24,6 +24,11 @@ container_config: security_options: - no-new-privileges:true user: '2001:2001' + # Optional (#93). Forced as HostConfig.GroupAdd: supplementary groups for + # reading host files, here the JWT secret (root:jwt 0640). Quote GIDs; 0 + # and root are refused. A caller's own list is replaced. + group_add: + - '64190' log_driver: json-file log_options: max-file: '5' diff --git a/deploy/config/dsp-it-a.yaml b/deploy/config/dsp-it-a.yaml index 15e5a4f..ada8ba7 100644 --- a/deploy/config/dsp-it-a.yaml +++ b/deploy/config/dsp-it-a.yaml @@ -13,3 +13,7 @@ allowed_image_prefixes: container_config: user: '1111:1111' + # Two forced supplementary groups for the #93 checks; dsp-it-b has none. + group_add: + - '64190' + - '64191' diff --git a/deploy/test.sh b/deploy/test.sh index fe47d75..32b9ad8 100755 --- a/deploy/test.sh +++ b/deploy/test.sh @@ -807,6 +807,54 @@ check_denied "DELETE /v1.45/containers/ -> 403 (uncovered, #86) S=$(delete_status "$PROXY/containers/no-such-container") check_denied "DELETE /containers/no-such-container -> 403 (daemon 404, #86)" "$S" "$BODY" 'container no-such-container is not managed by any policy' +# ─── Forced supplementary groups (#93) ─────────────────── + +# deploy/config/dsp-it-a.yaml forces group_add ['64190', '64191']; dsp-it-b +# has no group_add. community.docker.docker_container's groups: becomes +# HostConfig.GroupAdd, a list of strings. Each create is inspected through the +# proxy (reads pass) and removed. Before #93 the proxy forced no list and +# denied any non-empty GroupAdd. +echo "" +echo "--- Forced group_add (#93) ---" + +FORCED_GROUPS='"GroupAdd":["64190","64191"]' + +# create_groups NAME HOSTCONFIG_JSON: create NAME from this runner's image. +create_groups() { + delete_status "$PROXY/containers/$1?force=1" >/dev/null + post_json "{\"Image\":\"$IT_IMAGE\",\"Cmd\":[\"sleep\",\"300\"],\"HostConfig\":$2}" "$PROXY/containers/create?name=$1" +} + +# inspect_groups NAME: the container's HostConfig.GroupAdd as the daemon +# reports it, or "none"; then remove the container. +inspect_groups() { + : >"$BODY" + curl -s -o "$BODY" $TIMEOUT --unix-socket "$PROXY_SOCK" "$PROXY/containers/$1/json" 2>/dev/null || true + g=$(grep -oE '"GroupAdd":(null|\[[^]]*\])' "$BODY" | head -n 1) + delete_status "$PROXY/containers/$1?force=1" >/dev/null + echo "${g:-none}" +} + +S=$(create_groups dsp-it-a '{}') +check "create dsp-it-a without GroupAdd -> 201" "201" "$S" +check "dsp-it-a without GroupAdd: the daemon has the forced list" "$FORCED_GROUPS" "$(inspect_groups dsp-it-a)" + +S=$(create_groups dsp-it-a '{"GroupAdd":["64190","64191"]}') +check "create dsp-it-a with the forced list -> 201" "201" "$S" +check "dsp-it-a with the forced list: the daemon has it unchanged" "$FORCED_GROUPS" "$(inspect_groups dsp-it-a)" + +S=$(create_groups dsp-it-a '{"GroupAdd":["0","docker"]}') +check "create dsp-it-a with another GroupAdd -> 201 (replaced, not denied)" "201" "$S" +check "dsp-it-a with another GroupAdd: the daemon has the forced list" "$FORCED_GROUPS" "$(inspect_groups dsp-it-a)" + +S=$(create_groups dsp-it-a '{"groupadd":["0"]}') +remove_if_created "$S" +check_denied "create dsp-it-a with lower-case groupadd -> 403" "$S" "$BODY" 'HostConfig.groupadd is not allowed for service dsp-it-a' + +S=$(create_groups dsp-it-b '{"GroupAdd":["64190","64191"]}') +remove_if_created "$S" +check_denied "create dsp-it-b (no group_add) with GroupAdd -> 403" "$S" "$BODY" 'HostConfig.GroupAdd is not allowed for service dsp-it-b' + # ─── Read-only mode (#75) ──────────────────────────────── # The same config behind --readonly. Every request whose method is not GET or diff --git a/go/internal/middleware/containerconfig.go b/go/internal/middleware/containerconfig.go index aadff84..66fa221 100644 --- a/go/internal/middleware/containerconfig.go +++ b/go/internal/middleware/containerconfig.go @@ -8,7 +8,8 @@ import ( // ContainerConfigMutator overwrites the fields the policy's container_config // sets. It forces HostConfig.Privileged to false on every create, even when -// container_config is empty (#88). A HostConfig that is not an object is left +// container_config is empty (#88), and HostConfig.GroupAdd to the policy's +// group_add when set (#93). A HostConfig that is not an object is left // alone for HostConfigGate to deny. type ContainerConfigMutator struct{} @@ -49,6 +50,15 @@ func (m *ContainerConfigMutator) Mutate(r *http.Request, p *policy.Policy, body hostConfig["Privileged"] = false + if len(cc.GroupAdd) > 0 { + // Overwritten like User (#93); HostConfigGate allows exactly this list. + groups := make([]interface{}, len(cc.GroupAdd)) + for i, g := range cc.GroupAdd { + groups[i] = g + } + hostConfig["GroupAdd"] = groups + } + if cc.User != "" { body["User"] = cc.User } diff --git a/go/internal/middleware/hostconfig.go b/go/internal/middleware/hostconfig.go index 0670907..f7a4d25 100644 --- a/go/internal/middleware/hostconfig.go +++ b/go/internal/middleware/hostconfig.go @@ -123,6 +123,13 @@ func (g *HostConfigGate) Check(r *http.Request, p *policy.Policy, body map[strin if value != nil { return fmt.Errorf("HostConfig.%s is not allowed for service %s", key, p.ServiceName) } + case key == "GroupAdd": + // Zero, or exactly the policy's group_add, which the mutator + // forces (#93). Checked here too so that the gate does not open + // up if the mutator ever stops forcing it. + if !isZeroValue(value) && !equalsStringList(value, p.ContainerConfig.GroupAdd) { + return fmt.Errorf("HostConfig.GroupAdd is not allowed for service %s", p.ServiceName) + } case key == "SecurityOpt": if err := checkSecurityOpt(value, p); err != nil { return err @@ -238,6 +245,21 @@ func isZeroValue(v interface{}) bool { return false } +// equalsStringList reports whether v is a JSON list of strings equal to want, +// in order. An empty want matches nothing. +func equalsStringList(v interface{}, want []string) bool { + list, ok := v.([]interface{}) + if !ok || len(want) == 0 || len(list) != len(want) { + return false + } + for i, e := range list { + if s, isStr := e.(string); !isStr || s != want[i] { + return false + } + } + return true +} + func containsString(list []string, s string) bool { for _, x := range list { if x == s { diff --git a/go/internal/middleware/hostconfig_test.go b/go/internal/middleware/hostconfig_test.go index 052e54c..95275e1 100644 --- a/go/internal/middleware/hostconfig_test.go +++ b/go/internal/middleware/hostconfig_test.go @@ -14,7 +14,8 @@ import ( // // Policies: "std" has volumes /data (read_write) and /ro (read-only) and // forces network_mode host; "secopt" is "std" plus security_options -// [no-new-privileges:true]; "bare" has no volumes and no container_config. +// [no-new-privileges:true]; "bare" has no volumes and no container_config; +// "groups" is "std" plus group_add ["64190", "jwt"] (#93). // want is "" for an allowed create, or a substring of the deny reason. fwd, // when set, lists HostConfig fields the forwarded body must hold. type hostConfigRow struct { @@ -41,6 +42,9 @@ func hostConfigRowPolicy(name string) *policy.Policy { if name == "secopt" { p.ContainerConfig.SecurityOptions = []string{"no-new-privileges:true"} } + if name == "groups" { + p.ContainerConfig.GroupAdd = []string{"64190", "jwt"} + } return p } @@ -91,6 +95,14 @@ var hostConfigRows = []hostConfigRow{ {name: "deviceRequestsDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"DeviceRequests":[{"Driver":"","Count":-1,"Capabilities":[["gpu"]]}]}}`, want: "HostConfig.DeviceRequests is not allowed for service svc", fwd: ``}, {name: "deviceCgroupRulesDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"DeviceCgroupRules":["b 8:* rmw"]}}`, want: "HostConfig.DeviceCgroupRules is not allowed for service svc", fwd: ``}, {name: "groupAddDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["docker"]}}`, want: "HostConfig.GroupAdd is not allowed for service svc", fwd: ``}, + {name: "groupAddPolicyListWithoutGroupAddDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["64190","jwt"]}}`, want: "HostConfig.GroupAdd is not allowed for service svc", fwd: ``}, + {name: "groupAddForced", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1"}`, want: "", fwd: `{"GroupAdd":["64190","jwt"]}`}, + {name: "groupAddForcedNullHostConfig", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":null}`, want: "", fwd: `{"GroupAdd":["64190","jwt"]}`}, + {name: "groupAddCallerReplaced", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["docker","0"]}}`, want: "", fwd: `{"GroupAdd":["64190","jwt"]}`}, + {name: "groupAddCallerNullReplaced", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":null}}`, want: "", fwd: `{"GroupAdd":["64190","jwt"]}`}, + {name: "groupAddSameListUnchanged", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["64190","jwt"]}}`, want: "", fwd: `{"GroupAdd":["64190","jwt"]}`}, + {name: "groupAddCaseVariantDenied", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"groupadd":["0"]}}`, want: "HostConfig.groupadd is not allowed for service svc", fwd: ``}, + {name: "groupAddNonObjectHostConfigDenied", policy: "groups", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":["GroupAdd"]}`, want: "HostConfig must be an object for service svc", fwd: ``}, {name: "volumesFromDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"VolumesFrom":["other"]}}`, want: "HostConfig.VolumesFrom is not allowed for service svc", fwd: ``}, {name: "linksDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"Links":["other:alias"]}}`, want: "HostConfig.Links is not allowed for service svc", fwd: ``}, {name: "sysctlsDenied", policy: "std", body: `{"Image":"chainsafe/lodestar:v1","HostConfig":{"Sysctls":{"kernel.shm_rmid_forced":"1"}}}`, want: "HostConfig.Sysctls is not allowed for service svc", fwd: ``}, @@ -218,3 +230,43 @@ var hostConfigRows = []hostConfigRow{ {name: "dockerCliCreateBodyBareAllowed", policy: "bare", body: `{"Hostname":"","Domainname":"","User":"","AttachStdin":false,"AttachStdout":true,"AttachStderr":true,"Tty":false,"OpenStdin":false,"StdinOnce":false,"Env":null,"Cmd":["true"],"Image":"chainsafe/lodestar:v1","Volumes":{},"WorkingDir":"","Entrypoint":null,"Labels":{},"HostConfig":{"Binds":null,"ContainerIDFile":"","LogConfig":{"Type":"","Config":{}},"NetworkMode":"default","PortBindings":{},"RestartPolicy":{"Name":"no","MaximumRetryCount":0},"AutoRemove":false,"VolumeDriver":"","VolumesFrom":null,"ConsoleSize":[0,0],"CapAdd":null,"CapDrop":null,"CgroupnsMode":"","Dns":null,"DnsOptions":[],"DnsSearch":[],"ExtraHosts":null,"GroupAdd":null,"IpcMode":"","Cgroup":"","Links":null,"OomScoreAdj":0,"PidMode":"","Privileged":false,"PublishAllPorts":false,"ReadonlyRootfs":false,"SecurityOpt":null,"UTSMode":"","UsernsMode":"","ShmSize":0,"Isolation":"","CpuShares":0,"Memory":0,"NanoCpus":0,"CgroupParent":"","BlkioWeight":0,"BlkioWeightDevice":[],"BlkioDeviceReadBps":[],"BlkioDeviceWriteBps":[],"BlkioDeviceReadIOps":[],"BlkioDeviceWriteIOps":[],"CpuPeriod":0,"CpuQuota":0,"CpuRealtimePeriod":0,"CpuRealtimeRuntime":0,"CpusetCpus":"","CpusetMems":"","Devices":[],"DeviceCgroupRules":null,"DeviceRequests":null,"MemoryReservation":0,"MemorySwap":0,"MemorySwappiness":-1,"OomKillDisable":false,"PidsLimit":0,"Ulimits":null,"CpuCount":0,"CpuPercent":0,"IOMaximumIOps":0,"IOMaximumBandwidth":0,"MaskedPaths":null,"ReadonlyPaths":null},"NetworkingConfig":{"EndpointsConfig":{"default":{"IPAMConfig":null,"Links":null,"Aliases":null,"DriverOpts":null,"GwPriority":0,"NetworkID":"","EndpointID":"","Gateway":"","IPAddress":"","MacAddress":"","IPPrefixLen":0,"IPv6Gateway":"","GlobalIPv6Address":"","GlobalIPv6PrefixLen":0,"DNSNames":null}}}}`, want: "", fwd: ``}, {name: "ansibleCreateBodyAllowed", policy: "secopt", body: `{"Image":"chainsafe/lodestar:v1","Cmd":["beacon"],"Env":[],"Labels":{"app":"beacon"},"HostConfig":{"Binds":["/data:/data:rw","/ro:/ro:ro"],"NetworkMode":"host","RestartPolicy":{"Name":"unless-stopped","MaximumRetryCount":0},"LogConfig":{"Type":"json-file","Config":{"max-size":"100m"}},"SecurityOpt":["no-new-privileges:true"],"Init":false,"AutoRemove":false,"Privileged":false,"ReadonlyRootfs":false,"CapDrop":["ALL"],"Memory":0}}`, want: "", fwd: ``}, } + +// HostConfigGate alone, on the "groups" policy (#93): as if the mutator +// stopped forcing the list. Only the policy's list, exactly, or a zero value +// passes. The same rows, by name, are in rs/src/middleware.rs and +// ts/src/middleware.test.ts. +var groupAddGateRows = []struct { + name string + body string + want string +}{ + {"groupAddGateForcedListAllowed", `{"HostConfig":{"GroupAdd":["64190","jwt"]}}`, ""}, + {"groupAddGateEmptyAllowed", `{"HostConfig":{"GroupAdd":[]}}`, ""}, + {"groupAddGateNullAllowed", `{"HostConfig":{"GroupAdd":null}}`, ""}, + {"groupAddGateOtherListDenied", `{"HostConfig":{"GroupAdd":["docker"]}}`, "HostConfig.GroupAdd is not allowed for service svc"}, + {"groupAddGateReorderedDenied", `{"HostConfig":{"GroupAdd":["jwt","64190"]}}`, "HostConfig.GroupAdd is not allowed for service svc"}, + {"groupAddGateSubsetDenied", `{"HostConfig":{"GroupAdd":["64190"]}}`, "HostConfig.GroupAdd is not allowed for service svc"}, + {"groupAddGateSupersetDenied", `{"HostConfig":{"GroupAdd":["64190","jwt","0"]}}`, "HostConfig.GroupAdd is not allowed for service svc"}, + {"groupAddGateNumberDenied", `{"HostConfig":{"GroupAdd":[64190,"jwt"]}}`, "HostConfig.GroupAdd is not allowed for service svc"}, + {"groupAddGateStringDenied", `{"HostConfig":{"GroupAdd":"64190"}}`, "HostConfig.GroupAdd is not allowed for service svc"}, +} + +func TestGroupAddGateRows(t *testing.T) { + for _, row := range groupAddGateRows { + t.Run(row.name, func(t *testing.T) { + var body map[string]interface{} + if err := json.Unmarshal([]byte(row.body), &body); err != nil { + t.Fatalf("bad row body: %v", err) + } + err := (&HostConfigGate{}).Check(testRequest("POST", "/containers/create"), hostConfigRowPolicy("groups"), body) + switch { + case row.want == "" && err != nil: + t.Fatalf("expected allow, got: %v", err) + case row.want != "" && err == nil: + t.Fatalf("expected deny with %q, got allow", row.want) + case row.want != "" && !strings.Contains(err.Error(), row.want): + t.Fatalf("expected deny with %q, got: %v", row.want, err) + } + }) + } +} diff --git a/go/internal/policy/manager.go b/go/internal/policy/manager.go index 93b7ded..3be9e0c 100644 --- a/go/internal/policy/manager.go +++ b/go/internal/policy/manager.go @@ -7,6 +7,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" "gopkg.in/yaml.v3" @@ -16,6 +17,45 @@ import ( // slash (#85). var validContainerName = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]*$`) +var ( + groupGIDRe = regexp.MustCompile(`^[0-9]+$`) + groupNameRe = regexp.MustCompile(`^[a-z_][a-z0-9_-]*\$?$`) +) + +// checkGroupAdd validates container_config.group_add (#93): each entry is a +// decimal GID that fits in 32 bits or a group name of at most 32 characters; +// the root group (0 in any decimal form, or root) and duplicates (by GID +// value for numbers) are refused. +func checkGroupAdd(file string, groups []string) error { + seen := make(map[string]bool) + for _, g := range groups { + var key string + switch { + case groupGIDRe.MatchString(g): + gid, err := strconv.ParseUint(g, 10, 32) + if err != nil { + return fmt.Errorf("policy file %s: invalid group_add entry %q (want a decimal GID or a group name)", file, g) + } + if gid == 0 { + return fmt.Errorf("policy file %s: group_add entry %q is the root group, which is not allowed", file, g) + } + key = "gid:" + strconv.FormatUint(gid, 10) + case len(g) <= 32 && groupNameRe.MatchString(g): + if g == "root" { + return fmt.Errorf("policy file %s: group_add entry %q is the root group, which is not allowed", file, g) + } + key = "name:" + g + default: + return fmt.Errorf("policy file %s: invalid group_add entry %q (want a decimal GID or a group name)", file, g) + } + if seen[key] { + return fmt.Errorf("policy file %s: duplicate group_add entry %q", file, g) + } + seen[key] = true + } + return nil +} + // ValidContainerName reports whether name is a valid Docker container name. func ValidContainerName(name string) bool { return validContainerName.MatchString(name) @@ -121,6 +161,14 @@ func loadPolicy(filePath string) (*Policy, error) { } } + if err := checkGroupAdd(filepath.Base(filePath), p.ContainerConfig.GroupAdd); err != nil { + return nil, err + } + // An empty group_add is the same as an unset one. + if len(p.ContainerConfig.GroupAdd) == 0 { + p.ContainerConfig.GroupAdd = nil + } + // Prefixes are compared in normalised form (#84). for i, prefix := range p.AllowedImagePrefixes { normalized, err := NormalizeImagePrefix(prefix) diff --git a/go/internal/policy/manager_test.go b/go/internal/policy/manager_test.go index 60c7519..d1db3da 100644 --- a/go/internal/policy/manager_test.go +++ b/go/internal/policy/manager_test.go @@ -333,6 +333,49 @@ func TestNewManager_LoadTimeValidation(t *testing.T) { {"empty container_names", map[string]string{ "a.yaml": "service_name: a\ncontainer_names: []\nallowed_image_prefixes: [alpine]\n", }, `container_names must not be empty`}, + // container_config.group_add (#93). + {"group_add GIDs and names", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '4294967295', jwt, _x-1, 'samba$', abcdefghijklmnopqrstuvwxyz012345]\n", + }, ""}, + {"group_add empty", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: []\n", + }, ""}, + {"group_add 0", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '0']\n", + }, `policy file a.yaml: group_add entry "0" is the root group, which is not allowed`}, + {"group_add 00", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['00']\n", + }, `policy file a.yaml: group_add entry "00" is the root group, which is not allowed`}, + {"group_add root", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [root]\n", + }, `policy file a.yaml: group_add entry "root" is the root group, which is not allowed`}, + {"group_add duplicate", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', jwt, '64190']\n", + }, `policy file a.yaml: duplicate group_add entry "64190"`}, + {"group_add duplicate GID with leading zero", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '064190']\n", + }, `policy file a.yaml: duplicate group_add entry "064190"`}, + {"group_add GID too large", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['4294967296']\n", + }, `policy file a.yaml: invalid group_add entry "4294967296"`}, + {"group_add upper-case name", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [Jwt]\n", + }, `policy file a.yaml: invalid group_add entry "Jwt"`}, + {"group_add name with a space", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['a b']\n", + }, `policy file a.yaml: invalid group_add entry "a b"`}, + {"group_add name starting with a dash", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['-x']\n", + }, `policy file a.yaml: invalid group_add entry "-x"`}, + {"group_add name of 33 characters", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [abcdefghijklmnopqrstuvwxyz0123456]\n", + }, `policy file a.yaml: invalid group_add entry "abcdefghijklmnopqrstuvwxyz0123456"`}, + {"group_add empty string", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['']\n", + }, `policy file a.yaml: invalid group_add entry ""`}, + {"group_add unquoted number", map[string]string{ + "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [64190]\n", + }, `group_add entries must be strings`}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -395,3 +438,17 @@ func TestCovering(t *testing.T) { } } } + +// An empty group_add is the same as an unset one (#93). +func TestNewManager_EmptyGroupAddIsUnset(t *testing.T) { + dir := t.TempDir() + writePolicyFile(t, dir, "a.yaml", "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: []\n") + m, err := NewManager(dir) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + p, _ := m.Get("a") + if p.ContainerConfig.GroupAdd != nil { + t.Fatalf("group_add = %q, want unset", p.ContainerConfig.GroupAdd) + } +} diff --git a/go/internal/policy/types.go b/go/internal/policy/types.go index 6564e13..fd5ed32 100644 --- a/go/internal/policy/types.go +++ b/go/internal/policy/types.go @@ -1,6 +1,11 @@ package policy -import "encoding/json" +import ( + "encoding/json" + "fmt" + + "gopkg.in/yaml.v3" +) type Policy struct { ServiceName string `yaml:"service_name"` @@ -50,6 +55,28 @@ type ContainerConfig struct { User string `yaml:"user"` LogDriver string `yaml:"log_driver"` LogOptions map[string]string `yaml:"log_options"` + // GroupAdd is forced as HostConfig.GroupAdd (#93). Empty means unset. + GroupAdd GroupList `yaml:"group_add"` +} + +// GroupList is a YAML list of strings. Unlike a plain []string, it refuses +// entries that are not YAML strings, such as an unquoted 64190 or 0x10, so +// Go, Rust and TypeScript read the same text for every entry (#93). +type GroupList []string + +func (g *GroupList) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.SequenceNode { + return fmt.Errorf("line %d: group_add must be a list", node.Line) + } + out := make(GroupList, 0, len(node.Content)) + for _, item := range node.Content { + if item.Kind != yaml.ScalarNode || item.Tag != "!!str" { + return fmt.Errorf("line %d: group_add entries must be strings (quote GIDs: '64190')", item.Line) + } + out = append(out, item.Value) + } + *g = out + return nil } type Volume struct { diff --git a/rs/src/handler.rs b/rs/src/handler.rs index 5fc4c45..6b78240 100644 --- a/rs/src/handler.rs +++ b/rs/src/handler.rs @@ -352,6 +352,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }), volumes: None, ports: None, @@ -447,6 +448,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }), volumes: None, ports: None, @@ -504,6 +506,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }), volumes: None, ports: None, diff --git a/rs/src/middleware.rs b/rs/src/middleware.rs index b282653..a06cc6e 100644 --- a/rs/src/middleware.rs +++ b/rs/src/middleware.rs @@ -587,6 +587,18 @@ fn is_zero_value(v: &serde_json::Value) -> bool { } } +/// Whether `value` is a JSON list of strings equal, in order, to the +/// policy's non-empty `group_add` (#93). +fn equals_forced_groups(value: &serde_json::Value, policy: &Policy) -> bool { + let forced = match policy.container_config.as_ref().and_then(|c| c.group_add.as_ref()) { + Some(g) if !g.is_empty() => g, + _ => return false, + }; + value.as_array().is_some_and(|list| { + list.len() == forced.len() && list.iter().zip(forced).all(|(v, f)| v.as_str() == Some(f.as_str())) + }) +} + fn check_security_opt(value: &serde_json::Value, policy: &Policy) -> std::result::Result<(), String> { if value.is_null() { return Ok(()); @@ -685,6 +697,17 @@ impl Gate for HostConfigGate { } } "SecurityOpt" => check_security_opt(value, policy)?, + // Zero, or exactly the policy's group_add, which the mutator + // forces (#93). Checked here too so that the gate does not + // open up if the mutator ever stops forcing it. + "GroupAdd" => { + if !is_zero_value(value) && !equals_forced_groups(value, policy) { + return Err(format!( + "HostConfig.GroupAdd is not allowed for service {}", + policy.service_name + )); + } + } "OomScoreAdj" => { if !value.is_null() && !value.as_f64().is_some_and(|n| n >= 0.0) { return Err(format!( @@ -929,7 +952,9 @@ fn force_endpoints(mode: &str, body: &mut HashMap) { /// ContainerConfigMutator overwrites the fields the policy's /// `container_config` sets. It forces `HostConfig.Privileged` to false on -/// every create, even without a `container_config` (#88). A `HostConfig` that +/// every create, even without a `container_config` (#88), and +/// `HostConfig.GroupAdd` to the policy's `group_add` when set (#93). A +/// `HostConfig` that /// is not an object is left alone for HostConfigGate to deny. pub struct ContainerConfigMutator; impl Mutator for ContainerConfigMutator { @@ -970,6 +995,13 @@ impl Mutator for ContainerConfigMutator { serde_json::json!({"Type": driver, "Config": cc.log_options.clone().unwrap_or_default()}), ); } + // Overwritten like User (#93); HostConfigGate allows exactly this list. + if let Some(groups) = cc.group_add.as_ref().filter(|g| !g.is_empty()) { + hc.insert( + "GroupAdd".to_string(), + serde_json::Value::Array(groups.iter().map(|g| serde_json::Value::String(g.clone())).collect()), + ); + } if let Some(u) = &cc.user { body.insert("User".to_string(), serde_json::Value::String(u.clone())); } @@ -1208,6 +1240,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }); let mut body: HashMap = HashMap::new(); mutator.mutate(&policy, &mut body); @@ -1227,6 +1260,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }); let mut body = body_from_json(serde_json::json!({"HostConfig": {}})); mutator.mutate(&policy, &mut body); @@ -1246,6 +1280,7 @@ mod tests { user: Some("appuser".into()), log_driver: None, log_options: None, + group_add: None, }); let mut body = HashMap::new(); mutator.mutate(&policy, &mut body); @@ -1298,6 +1333,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }); let body = body_from_json(serde_json::json!({"Image": "alpine"})); let result = chain.execute("POST", "/containers/create", &policy, &body); @@ -1689,6 +1725,7 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: None, }); let mut body = body_from_json(serde_json::json!({"Image": "alpine:3.20"})); if !req.is_empty() { @@ -1785,7 +1822,8 @@ mod tests { // Policies: "std" has volumes /data (read_write) and /ro (read-only) and // forces network_mode host; "secopt" is "std" plus security_options // [no-new-privileges:true]; "bare" has no volumes and no - // container_config. `want` is "" for an allowed create, or a substring of + // container_config; "groups" is "std" plus group_add ["64190", "jwt"] + // (#93). `want` is "" for an allowed create, or a substring of // the deny reason. `fwd`, when set, lists HostConfig fields the forwarded // body must hold. struct HostConfigRow { @@ -1824,6 +1862,11 @@ mod tests { user: None, log_driver: None, log_options: None, + group_add: if name == "groups" { + Some(vec!["64190".into(), "jwt".into()]) + } else { + None + }, }); p } @@ -1874,6 +1917,71 @@ mod tests { ); } + // HostConfigGate alone, on the "groups" policy (#93): as if the mutator + // stopped forcing the list. Only the policy's list, exactly, or a zero + // value passes. The same rows, by name, are in + // go/internal/middleware/hostconfig_test.go and ts/src/middleware.test.ts. + #[test] + fn test_group_add_gate_rows() { + let rows: &[(&str, &str, &str)] = &[ + ( + "groupAddGateForcedListAllowed", + r##"{"HostConfig":{"GroupAdd":["64190","jwt"]}}"##, + "", + ), + ("groupAddGateEmptyAllowed", r##"{"HostConfig":{"GroupAdd":[]}}"##, ""), + ("groupAddGateNullAllowed", r##"{"HostConfig":{"GroupAdd":null}}"##, ""), + ( + "groupAddGateOtherListDenied", + r##"{"HostConfig":{"GroupAdd":["docker"]}}"##, + "HostConfig.GroupAdd is not allowed for service svc", + ), + ( + "groupAddGateReorderedDenied", + r##"{"HostConfig":{"GroupAdd":["jwt","64190"]}}"##, + "HostConfig.GroupAdd is not allowed for service svc", + ), + ( + "groupAddGateSubsetDenied", + r##"{"HostConfig":{"GroupAdd":["64190"]}}"##, + "HostConfig.GroupAdd is not allowed for service svc", + ), + ( + "groupAddGateSupersetDenied", + r##"{"HostConfig":{"GroupAdd":["64190","jwt","0"]}}"##, + "HostConfig.GroupAdd is not allowed for service svc", + ), + ( + "groupAddGateNumberDenied", + r##"{"HostConfig":{"GroupAdd":[64190,"jwt"]}}"##, + "HostConfig.GroupAdd is not allowed for service svc", + ), + ( + "groupAddGateStringDenied", + r##"{"HostConfig":{"GroupAdd":"64190"}}"##, + "HostConfig.GroupAdd is not allowed for service svc", + ), + ]; + let policy = host_config_row_policy("groups"); + let mut failures = Vec::new(); + for (name, body, want) in rows { + let body: HashMap = serde_json::from_str(body).unwrap(); + match ( + HostConfigGate.check("POST", "/containers/create", &policy, &body), + *want, + ) { + (Ok(()), "") => {} + (Ok(()), want) => failures.push(format!("{name}: expected deny with {want:?}, got allow")), + (Err(e), "") => failures.push(format!("{name}: expected allow, got: {e}")), + (Err(e), want) if !e.contains(want) => { + failures.push(format!("{name}: expected deny with {want:?}, got: {e}")) + } + _ => {} + } + } + assert!(failures.is_empty(), "{}", failures.join("\n")); + } + const HOST_CONFIG_ROWS: &[HostConfigRow] = &[ HostConfigRow { name: "capAddDenied", @@ -1910,6 +2018,62 @@ mod tests { want: "HostConfig.GroupAdd is not allowed for service svc", fwd: r##""##, }, + HostConfigRow { + name: "groupAddPolicyListWithoutGroupAddDenied", + policy: "std", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["64190","jwt"]}}"##, + want: "HostConfig.GroupAdd is not allowed for service svc", + fwd: r##""##, + }, + HostConfigRow { + name: "groupAddForced", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1"}"##, + want: "", + fwd: r##"{"GroupAdd":["64190","jwt"]}"##, + }, + HostConfigRow { + name: "groupAddForcedNullHostConfig", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":null}"##, + want: "", + fwd: r##"{"GroupAdd":["64190","jwt"]}"##, + }, + HostConfigRow { + name: "groupAddCallerReplaced", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["docker","0"]}}"##, + want: "", + fwd: r##"{"GroupAdd":["64190","jwt"]}"##, + }, + HostConfigRow { + name: "groupAddCallerNullReplaced", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":null}}"##, + want: "", + fwd: r##"{"GroupAdd":["64190","jwt"]}"##, + }, + HostConfigRow { + name: "groupAddSameListUnchanged", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["64190","jwt"]}}"##, + want: "", + fwd: r##"{"GroupAdd":["64190","jwt"]}"##, + }, + HostConfigRow { + name: "groupAddCaseVariantDenied", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":{"groupadd":["0"]}}"##, + want: "HostConfig.groupadd is not allowed for service svc", + fwd: r##""##, + }, + HostConfigRow { + name: "groupAddNonObjectHostConfigDenied", + policy: "groups", + body: r##"{"Image":"chainsafe/lodestar:v1","HostConfig":["GroupAdd"]}"##, + want: "HostConfig must be an object for service svc", + fwd: r##""##, + }, HostConfigRow { name: "volumesFromDenied", policy: "std", diff --git a/rs/src/policy.rs b/rs/src/policy.rs index ac76fcb..cc064a6 100644 --- a/rs/src/policy.rs +++ b/rs/src/policy.rs @@ -61,6 +61,85 @@ pub struct ContainerConfig { pub user: Option, pub log_driver: Option, pub log_options: Option>, + /// Forced as `HostConfig.GroupAdd` (#93). An empty list is unset. + #[serde(default, deserialize_with = "group_list")] + pub group_add: Option>, +} + +/// Reads `group_add` as a list of YAML strings. serde_yaml would turn an +/// unquoted 64190 into "64190", and 0x10 into "0x10" where TypeScript reads +/// 16, so only string entries are taken, in all three (#93). +fn group_list<'de, D>(d: D) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, +{ + let values: Option> = Option::deserialize(d)?; + let Some(values) = values else { + return Ok(None); + }; + values + .into_iter() + .map(|v| match v { + serde_yaml::Value::String(s) => Ok(s), + _ => Err(serde::de::Error::custom( + "group_add entries must be strings (quote GIDs: '64190')", + )), + }) + .collect::, _>>() + .map(Some) +} + +/// A group name as useradd accepts it: `[a-z_][a-z0-9_-]*[$]?`, at most 32 +/// characters (#93). +fn valid_group_name(name: &str) -> bool { + let body = name.strip_suffix('$').unwrap_or(name); + let mut bytes = body.bytes(); + match bytes.next() { + Some(b) if b.is_ascii_lowercase() || b == b'_' => {} + _ => return false, + } + name.len() <= 32 && bytes.all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'_' || b == b'-') +} + +/// Validates `container_config.group_add` (#93): each entry is a decimal GID +/// that fits in 32 bits or a group name; the root group (0 in any decimal +/// form, or root) and duplicates (by GID value for numbers) are refused. +fn check_group_add(file: &str, groups: &[String]) -> Result<(), String> { + let mut seen = std::collections::HashSet::new(); + for g in groups { + let key = if !g.is_empty() && g.bytes().all(|b| b.is_ascii_digit()) { + let gid: u32 = g.parse().map_err(|_| { + format!( + "policy file {}: invalid group_add entry {:?} (want a decimal GID or a group name)", + file, g + ) + })?; + if gid == 0 { + return Err(format!( + "policy file {}: group_add entry {:?} is the root group, which is not allowed", + file, g + )); + } + format!("gid:{}", gid) + } else if valid_group_name(g) { + if g == "root" { + return Err(format!( + "policy file {}: group_add entry {:?} is the root group, which is not allowed", + file, g + )); + } + format!("name:{}", g) + } else { + return Err(format!( + "policy file {}: invalid group_add entry {:?} (want a decimal GID or a group name)", + file, g + )); + }; + if !seen.insert(key) { + return Err(format!("policy file {}: duplicate group_add entry {:?}", file, g)); + } + } + Ok(()) } #[derive(Debug, Clone, Deserialize)] @@ -123,7 +202,7 @@ impl Manager { .map(|f| f.to_string_lossy().into_owned()) .unwrap_or_default(); let data = std::fs::read_to_string(&path)?; - let policy: Policy = serde_yaml::from_str(&data)?; + let mut policy: Policy = serde_yaml::from_str(&data).map_err(|e| format!("policy file {}: {}", file, e))?; if policy.service_name.is_empty() { return Err("missing service_name".into()); } @@ -142,6 +221,13 @@ impl Manager { .into()); } } + if let Some(cc) = policy.container_config.as_mut() { + check_group_add(&file, cc.group_add.as_deref().unwrap_or_default())?; + // An empty group_add is the same as an unset one. + if cc.group_add.as_ref().is_some_and(|g| g.is_empty()) { + cc.group_add = None; + } + } // A second file with the same service_name used to replace the // first silently (#85). if let Some(prev) = file_of.get(&policy.service_name) { @@ -153,7 +239,6 @@ impl Manager { } file_of.insert(policy.service_name.clone(), file); // Prefixes are compared in normalised form (#84). - let mut policy = policy; let mut prefixes = Vec::with_capacity(policy.allowed_image_prefixes.len()); for prefix in &policy.allowed_image_prefixes { let normalized = normalize_image_prefix(prefix).map_err(|_| { @@ -942,6 +1027,119 @@ value_pattern: ^\d+[kmg]$ )], "container_names must not be empty", ), + // container_config.group_add (#93). + ( + "group_add GIDs and names", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '4294967295', jwt, _x-1, 'samba$', abcdefghijklmnopqrstuvwxyz012345]\n", + )], + "", + ), + ( + "group_add empty", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: []\n", + )], + "", + ), + ( + "group_add 0", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '0']\n", + )], + r#"policy file a.yaml: group_add entry "0" is the root group, which is not allowed"#, + ), + ( + "group_add 00", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['00']\n", + )], + r#"policy file a.yaml: group_add entry "00" is the root group, which is not allowed"#, + ), + ( + "group_add root", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [root]\n", + )], + r#"policy file a.yaml: group_add entry "root" is the root group, which is not allowed"#, + ), + ( + "group_add duplicate", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', jwt, '64190']\n", + )], + r#"policy file a.yaml: duplicate group_add entry "64190""#, + ), + ( + "group_add duplicate GID with leading zero", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '064190']\n", + )], + r#"policy file a.yaml: duplicate group_add entry "064190""#, + ), + ( + "group_add GID too large", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['4294967296']\n", + )], + r#"policy file a.yaml: invalid group_add entry "4294967296""#, + ), + ( + "group_add upper-case name", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [Jwt]\n", + )], + r#"policy file a.yaml: invalid group_add entry "Jwt""#, + ), + ( + "group_add name with a space", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['a b']\n", + )], + r#"policy file a.yaml: invalid group_add entry "a b""#, + ), + ( + "group_add name starting with a dash", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['-x']\n", + )], + r#"policy file a.yaml: invalid group_add entry "-x""#, + ), + ( + "group_add name of 33 characters", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [abcdefghijklmnopqrstuvwxyz0123456]\n", + )], + r#"policy file a.yaml: invalid group_add entry "abcdefghijklmnopqrstuvwxyz0123456""#, + ), + ( + "group_add empty string", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['']\n", + )], + r#"policy file a.yaml: invalid group_add entry """#, + ), + ( + "group_add unquoted number", + vec![( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [64190]\n", + )], + "group_add entries must be strings", + ), ]; let mut mismatches = Vec::new(); for (name, files, want) in cases { @@ -956,6 +1154,18 @@ value_pattern: ^\d+[kmg]$ assert!(mismatches.is_empty(), "{}", mismatches.join("\n")); } + /// An empty group_add is the same as an unset one (#93). + #[test] + fn test_manager_empty_group_add_is_unset() { + let m = load_dir(&[( + "a.yaml", + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: []\n", + )]) + .unwrap(); + let cc = m.get("a").unwrap().container_config.as_ref().unwrap(); + assert_eq!(cc.group_add, None); + } + /// #85 selection: the one policy matching the image and accepting the /// name, the nameless create, and ambiguity. #[test] diff --git a/spec/README.md b/spec/README.md index 80e8f7d..429c975 100644 --- a/spec/README.md +++ b/spec/README.md @@ -6,7 +6,7 @@ This directory contains a [Quint](https://quint-lang.org/) formal specification | File | Purpose | |------|---------| -| `docker_socket_policy.qnt` | Request-handling spec: policy types, state machine, endpoint routing table, policy selection by image and container name (#85), 12 invariants (9 P0 / 3 P1), 6 attack scenario simulations, and `run` tests for selection. Module `network_endpoints_model` models `NetworkingConfig.EndpointsConfig` under a forced `network_mode` (#87), one `run` test per table row, instances `network_endpoints` (all three after #87) and `network_endpoints_pre87`. Also the create check on `HostConfig` (module `create_model`, #88): the default-deny field rules, `Mounts`, bind options, read-only volumes and case-insensitive field names, with one `run` test per table row. Instances `create` (all three after #88) and `create_pre88` (all three before #88); the state machine uses the `create` rules | +| `docker_socket_policy.qnt` | Request-handling spec: policy types, state machine, endpoint routing table, policy selection by image and container name (#85), 12 invariants (9 P0 / 3 P1), 6 attack scenario simulations, and `run` tests for selection. Module `network_endpoints_model` models `NetworkingConfig.EndpointsConfig` under a forced `network_mode` (#87), one `run` test per table row, instances `network_endpoints` (all three after #87) and `network_endpoints_pre87`. Also the create check on `HostConfig` (module `create_model`, #88): the default-deny field rules, `Mounts`, bind options, read-only volumes and case-insensitive field names, and a policy-forced `GroupAdd` list (#93), with one `run` test per table row. Instances `create` (all three after #88) and `create_pre88` (all three before #88); the state machine uses the `create` rules | | `listener.qnt` | Listening-socket startup: flag/group selection, existing-path checks, single-instance lock, 6 invariants, one `run` test per design-table row. Instances `listener_locked` (Go, Rust) and `listener_unlocked` (TypeScript) | | `listener-design.md` | Design of the listening socket (dockerd parity) that `listener.qnt` models | | `router.qnt` | The percent-encoded path deny, the empty interior segment deny, the dot segment deny, the segment-exact exec deny and `POST /containers/create` route, the segment-exact attach deny, the GET/HEAD-only read passthrough with `/_ping` standing for the read endpoints, container-name extraction and the container-lifecycle branch of the router only, not the full router, plus the handler's daemon lookup that limits lifecycle actions to covered containers (#86). The daemon's percent-decoding as a fixed table. One `run` test per table row. Instances `router` (Go, TypeScript, and Rust after #48, #53, #49, #55, #76, #78 and #70), `router_pre48` (Rust's extraction rule before #48), `router_pre53` (Rust and TypeScript before #53, routing on the raw path), `router_pre55` (all three before #55, letting empty interior segments through), `router_readonly` (as `router`, with `--readonly`, #75), `router_pre76` (all three before #76, letting attach through), `router_pre78` (all three before #78, allowing the read endpoints for every method), `router_pre70` (all three before #70, letting `.` and `..` segments through) and `router_pre86` (all three before #86, forwarding lifecycle actions on uncovered containers). Every instance includes the #49 exec and create rule, so the pre instances isolate one earlier rule each rather than being full snapshots. Module `pull_model` models the `POST /images/create` branch: which of the body and the query names the image, and image-name normalisation as a fixed table. Instances `pull` (all three after #83 and #84), `pull_pre83` (all three before #83, reading `fromImage` from the body), `pull_body_ignored` (the first #83 fix, which forwarded a pull's body) and `pull_pre84` (all three before #84, matching the spelling as sent) | @@ -266,22 +266,30 @@ Selecting from every image match instead (the pre-#85 `oneOf`) makes the last tw | `namedOwnOverDefault` | `mynet` | `{"default": {Aliases}, "mynet": {}}` | `{"mynet": {}}` | ### Container create: HostConfig (`create_model` in `docker_socket_policy.qnt`) -The create check of [#88](https://github.com/ChainSafe/docker-socket-policy/issues/88): ContainerConfigMutator, then MountSourceGate and HostConfigGate on the mutated body. `LEGACY_CREATE = true` models all three before #88; `false` all three after. Four properties are checked exhaustively over both model policies (`BEACON`: volumes `/home/beacon` and read-only `/etc/jwt_dir`, a full `container_config`; `TOOLS`: no volumes, no `container_config`) and every single-field, single-bind and single-mount request over the model's atoms, plus every `NetworkingConfig` holding the CLI's `default` entry and one more entry, under `EndpointsConfig` or `endpointsconfig`: +The create check of [#88](https://github.com/ChainSafe/docker-socket-policy/issues/88): ContainerConfigMutator, then MountSourceGate and HostConfigGate on the mutated body. `LEGACY_CREATE = true` models all three before #88; `false` all three after. Five properties are checked exhaustively over the three model policies (`BEACON`: volumes `/home/beacon` and read-only `/etc/jwt_dir`, a full `container_config`; `TOOLS`: no volumes, no `container_config`; `GROUPS`: `BEACON` plus a forced `group_add`, #93) and every single-field, single-bind and single-mount request over the model's atoms, plus every `NetworkingConfig` holding the CLI's `default` entry and one more entry, under `EndpointsConfig` or `endpointsconfig`: | Property | Claim | |----------|-------| | `createChecksEveryField` | The rule: an allowed create has only exactly spelled field names, allowed values after the mutator, and allowed binds and mounts. Close to a tautology on `create`. | -| `daemonSeesOnlyAllowlistedHostConfig` | What the daemon reads (fields under their canonical names) is allowlisted with allowed values, `Privileged` is zero, and every value the policy forces (`NetworkMode`, `SecurityOpt`) is the forced one. Does not mention the gate's rules or the flag. | +| `daemonSeesOnlyAllowlistedHostConfig` | What the daemon reads (fields under their canonical names) is allowlisted with allowed values, `Privileged` is zero, and every value the policy forces (`NetworkMode`, `SecurityOpt`, `GroupAdd`) is the forced one. Does not mention the gate's rules or the flag. | | `daemonMountsOnlyWhitelisted` | Every host path the daemon mounts, from `Binds` or `Mounts`, is a policy volume, is not writable when the volume is read-only, and does not propagate. | +| `daemonGroupsOnlyForced` | The daemon reads `GroupAdd`, under any spelling, only as the selected policy's forced list or as an empty one ([#93](https://github.com/ChainSafe/docker-socket-policy/issues/93)). Mentions neither the mutator nor the gate. | | `daemonAttachesOnlyForcedNetwork` | When the policy forces `network_mode`, the daemon attaches no network but the mode (or the CLI's `default`), reading `EndpointsConfig` under any spelling. `NetworkEndpointsGate` (#87, in both instances) reads the exact key only. | -`create` runs `createSoundTest` (all four hold) and the rows below; `create_pre88` runs `pre88UnsoundTest`, which asserts that the three security properties fail and that the #88 reproductions were allowed: a `Mounts` bind of `/`, a `:rw` and a mode-less bind of the read-only volume, `CapAdd`, `PidMode: host`, any bind on a policy with no volumes, `Privileged: true` reaching the daemon on a policy with no `container_config`, a zero-valued `securityopt` clearing a forced `SecurityOpt`, and an `endpointsconfig` entry attaching `evil-net` under a forced `host`. The deny rows run on `create` only. +`create` runs `createSoundTest` (all five hold) and the rows below; `create_pre88` runs `pre88UnsoundTest`, which asserts that the three security properties fail and that the #88 reproductions were allowed: a `Mounts` bind of `/`, a `:rw` and a mode-less bind of the read-only volume, `CapAdd`, `PidMode: host`, any bind on a policy with no volumes, `Privileged: true` reaching the daemon on a policy with no `container_config`, a zero-valued `securityopt` clearing a forced `SecurityOpt`, and an `endpointsconfig` entry attaching `evil-net` under a forced `host`. The deny rows run on `create` only. + +`group_add` (#93) is a single atom in the model (`GROUP_LIST`): the mutator writes the policy's list whole and HostConfigGate compares it whole, so its entries do not matter to the properties, and the load-time checks on them (decimal GID or group name, no root group, no duplicates) are left to the language tables. There is no pre-93 instance. Before #93 `GroupAdd` had to be zero for every policy, which is the rule `create` still runs for `BEACON` and `TOOLS`, and is strictly more restrictive: `daemonGroupsOnlyForced` held then, so there is no gap to witness. Breaking the change instead fails the properties: letting the gate accept any `GroupAdd` fails `createSoundTest` and the `groupAddDenied`, `groupAddPolicyListWithoutGroupAddDenied` and `groupAddGateExactOnly` rows; a mutator that keeps the caller's `GroupAdd` fails `createSoundTest` and `groupAddCallerReplaced`. Rows, with the names the language tables use (the language tables have more rows, one per field): | Row (`run Test`) | Request | Outcome | |-----|---------|---------| | `capAddDenied`, `devicesDenied`, `groupAddDenied`, `unknownFieldDenied` | a non-zero field outside the allowlist | deny | +| `groupAddPolicyListWithoutGroupAddDenied` | `GroupAdd` equal to another policy's forced list, on `BEACON` (no `group_add`) | deny | +| `groupAddForced` | no `HostConfig`, on `GROUPS` | allow; the daemon reads the forced list | +| `groupAddCallerReplaced` / `groupAddSameListUnchanged` | `GroupAdd` with another list / the forced list, on `GROUPS` | allow; the daemon reads the forced list only | +| `groupAddCaseVariantDenied` | `groupadd` on `GROUPS` | deny | +| `groupAddGateExactOnly` | HostConfigGate alone on `GROUPS`: the forced list, `[]`, another list; and the forced list on `BEACON` | allow, allow, deny, deny | | `maskedPathsEmptyDenied` | `MaskedPaths: []` | deny | | `pidModeHostDenied`, `pidModeContainerDenied`, `ipcModeHostDenied` | `host` / `container:` namespace modes | deny | | `ipcModePrivateAllowed` | `IpcMode: private` | allow | @@ -300,7 +308,7 @@ Rows, with the names the language tables use (the language tables have more rows | `bindReadOnlyVolumeRoAllowed`, `bindReadOnlyVolumeRoZAllowed`, `bindAllowed` | `:ro`, `:ro,Z`, a read-write volume | allow | | `bindRsharedDenied`, `bindUnlistedDenied`, `bindNoVolumesDenied` | `Binds` | deny | -The state machine imports `create_model(LEGACY_CREATE = false)`: `createContainer` takes the request's `HostConfig`, guards on `createAllowed`, and records what the daemon read; the P0 invariants `hostConfigInAllowlist` and `hostMountsInWhitelist` check it on every step of `make verify`, and `noPrivilegedAccess` now reads `Privileged` from it instead of rejecting privileged requests (the implementations neutralise them). +The state machine imports `create_model(LEGACY_CREATE = false)`: `createContainer` takes the request's `HostConfig`, guards on `createAllowed`, and records what the daemon read; the P0 invariants `hostConfigInAllowlist` and `hostMountsInWhitelist` check it on every step of `make verify` (the state machine's `beacon` policy forces a `group_add`, so `hostConfigInAllowlist` also checks the forced list, #93), and `noPrivilegedAccess` now reads `Privileged` from it instead of rejecting privileged requests (the implementations neutralise them). ### Image pull (`router.qnt`, `pull_model`) diff --git a/spec/docker_socket_policy.qnt b/spec/docker_socket_policy.qnt index 4ce2236..b67ea0c 100644 --- a/spec/docker_socket_policy.qnt +++ b/spec/docker_socket_policy.qnt @@ -7,7 +7,10 @@ module policy { networkMode: str, restartPolicy: str, securityOptions: Set[str], - user: str + user: str, + // container_config.group_add (#93): "" when unset, otherwise an atom + // standing for the whole forced list. + groupAdd: str } // containerNames is the policy's container_names (#85). The empty set @@ -224,6 +227,13 @@ module network_endpoints_pre87 { // `networking` entries are its NetworkingConfig keys, each with one network // name as value, and NetworkEndpointsGate (#87) reads the exact key only. // +// container_config.group_add (#93) forces HostConfig.GroupAdd. The policy's +// list is one atom (GROUP_LIST): the mutator writes it whole and +// HostConfigGate compares it whole, so the model does not need its entries. +// There is no flag for the behaviour before #93: without group_add the rule +// is the #88 one (GroupAdd must be zero), and every policy then was one +// without group_add, so `create` with BEACON already models it. +// // LEGACY_CREATE = true models all three before #88: the only check was that // each Binds host path is a policy volume, skipped when the policy has no // volumes; Mounts, bind options and every other field went through; and the @@ -254,6 +264,7 @@ module create_model { "securityopt" -> "SecurityOpt", "networkmode" -> "NetworkMode", "endpointsconfig" -> "EndpointsConfig", + "groupadd" -> "GroupAdd", ) pure def canon(k: str): str = if (CASE_FOLD.keys().contains(k)) CASE_FOLD.get(k) else k pure def isVariant(k: str): bool = canon(k) != k @@ -267,6 +278,9 @@ module create_model { pure val NO_NEW_PRIVILEGES = Set("no-new-privileges", "no-new-privileges:true") // The value the mutator writes for SecurityOpt: the policy's list. pure val POLICY_SECURITY_OPT = "policySecurityOptions" + // A forced group_add list (#93), as GROUPS sets it. + pure val GROUP_LIST = "jwtGroups" + pure def forcesGroups(p: Policy): bool = p.containerConfig.groupAdd != "" pure val BIND_OPTS = Set("ro", "rw", "Z", "rprivate") pure val PRIVATE_PROPAGATION = Set("", "private", "rprivate") @@ -282,6 +296,8 @@ module create_model { else if (k == "OomScoreAdj") v != "-1000" else if (k == "SecurityOpt") NO_NEW_PRIVILEGES.contains(v) or p.containerConfig.securityOptions.contains(v) or v == POLICY_SECURITY_OPT + // #93: the policy's own list, exactly; any other non-zero list is denied. + else if (k == "GroupAdd") forcesGroups(p) and v == p.containerConfig.groupAdd else false pure def hasContainerConfig(p: Policy): bool = or { @@ -289,6 +305,7 @@ module create_model { p.containerConfig.restartPolicy != "", p.containerConfig.securityOptions.size() > 0, p.containerConfig.user != "", + forcesGroups(p), } pure def forcesPrivileged(p: Policy): bool = not(LEGACY_CREATE) or hasContainerConfig(p) @@ -301,11 +318,13 @@ module create_model { f.key == "Privileged" and forcesPrivileged(p), f.key == "NetworkMode" and nm != "", f.key == "SecurityOpt" and forcesSecOpt, + f.key == "GroupAdd" and forcesGroups(p), })) kept .union(if (forcesPrivileged(p)) Set({ key: "Privileged", value: "zero" }) else Set()) .union(if (nm != "") Set({ key: "NetworkMode", value: nm }) else Set()) .union(if (forcesSecOpt) Set({ key: "SecurityOpt", value: POLICY_SECURITY_OPT }) else Set()) + .union(if (forcesGroups(p)) Set({ key: "GroupAdd", value: p.containerConfig.groupAdd }) else Set()) } // HostConfigGate, on the mutated fields. @@ -388,6 +407,7 @@ module create_model { restartPolicy: "unless-stopped", securityOptions: Set("no-new-privileges:true"), user: "2001:2001", + groupAdd: "", }, volumes: Set("/home/beacon", "/etc/jwt_dir"), readOnlyVolumes: Set("/etc/jwt_dir"), @@ -405,7 +425,7 @@ module create_model { allowedImagePrefixes: Set("example/tools"), imageTagPattern: "", imageDigestAllowed: false, - containerConfig: { networkMode: "", restartPolicy: "", securityOptions: Set(), user: "" }, + containerConfig: { networkMode: "", restartPolicy: "", securityOptions: Set(), user: "", groupAdd: "" }, volumes: Set(), readOnlyVolumes: Set(), envFile: "", @@ -415,16 +435,19 @@ module create_model { deniedFlags: Set(), } - pure val MODEL_POLICIES = Set(BEACON, TOOLS) + // BEACON with a forced group_add (#93). + pure val GROUPS: Policy = { ...BEACON, containerConfig: { ...BEACON.containerConfig, groupAdd: GROUP_LIST } } + + pure val MODEL_POLICIES = Set(BEACON, TOOLS, GROUPS) pure val KEYS = Set( "Privileged", "privileged", "SecurityOpt", "securityopt", "NetworkMode", "networkmode", - "CapAdd", "Devices", "GroupAdd", "FutureField", "MaskedPaths", "CapDrop", + "CapAdd", "Devices", "GroupAdd", "groupadd", "FutureField", "MaskedPaths", "CapDrop", "MemorySwappiness", "PidMode", "IpcMode", "OomScoreAdj", ) pure val VALUES = Set( "null", "zero", "[]", "true", "host", "private", "container:x", "SYS_ADMIN", - "-1000", "500", "no-new-privileges", "seccomp=unconfined", "bridge", + "-1000", "500", "no-new-privileges", "seccomp=unconfined", "bridge", GROUP_LIST, ) pure val SOURCES = Set("/home/beacon", "/etc/jwt_dir", "/") @@ -477,9 +500,20 @@ module create_model { implies f.value == p.containerConfig.networkMode, (f.key == "SecurityOpt" and p.containerConfig.securityOptions.size() > 0) implies f.value == POLICY_SECURITY_OPT, + (f.key == "GroupAdd" and forcesGroups(p)) implies f.value == p.containerConfig.groupAdd, }) )) + // The security claim on supplementary groups (#93): the daemon reads + // GroupAdd, under any spelling, only as the selected policy's forced list + // or as an empty one. It mentions neither the mutator nor the gate. + pure val daemonGroupsOnlyForced: bool = + MODEL_POLICIES.forall(p => REQS.forall(r => + createAllowed(p, r) implies daemonFields(p, r).forall(f => + f.key == "GroupAdd" implies (isZero(f.value) or (forcesGroups(p) and f.value == p.containerConfig.groupAdd)) + ) + )) + // The security claim on mounts: every host path the daemon mounts is a // policy volume, a read-only volume is never writable, and no mount // propagates. With no volumes, nothing is mounted. @@ -519,6 +553,36 @@ module create { run capAddDeniedTest = assert(not(createAllowed(BEACON, one("CapAdd", "SYS_ADMIN")))) run devicesDeniedTest = assert(not(createAllowed(BEACON, one("Devices", "/dev/sda")))) run groupAddDeniedTest = assert(not(createAllowed(BEACON, one("GroupAdd", "true")))) + // #93: a policy with group_add forces its list; the caller's is replaced. + run groupAddForcedTest = + assert(and { + createAllowed(GROUPS, NO_HOST_CONFIG), + daemonFields(GROUPS, NO_HOST_CONFIG).contains({ key: "GroupAdd", value: GROUP_LIST }), + }) + run groupAddCallerReplacedTest = + assert(and { + createAllowed(GROUPS, one("GroupAdd", "true")), + daemonFields(GROUPS, one("GroupAdd", "true")).filter(f => f.key == "GroupAdd") + == Set({ key: "GroupAdd", value: GROUP_LIST }), + }) + run groupAddSameListUnchangedTest = + assert(and { + createAllowed(GROUPS, one("GroupAdd", GROUP_LIST)), + daemonFields(GROUPS, one("GroupAdd", GROUP_LIST)).filter(f => f.key == "GroupAdd") + == Set({ key: "GroupAdd", value: GROUP_LIST }), + }) + // A policy without group_add cannot take a caller's list as if forced. + run groupAddPolicyListWithoutGroupAddDeniedTest = assert(not(createAllowed(BEACON, one("GroupAdd", GROUP_LIST)))) + run groupAddCaseVariantDeniedTest = assert(not(createAllowed(GROUPS, one("groupadd", "true")))) + // HostConfigGate alone (as if the mutator stopped forcing the list): + // the forced list and zero pass, any other list does not. + run groupAddGateExactOnlyTest = + assert(and { + fieldOk(GROUPS, { key: "GroupAdd", value: GROUP_LIST }), + fieldOk(GROUPS, { key: "GroupAdd", value: "[]" }), + not(fieldOk(GROUPS, { key: "GroupAdd", value: "true" })), + not(fieldOk(BEACON, { key: "GroupAdd", value: GROUP_LIST })), + }) run unknownFieldDeniedTest = assert(not(createAllowed(BEACON, one("FutureField", "true")))) run maskedPathsEmptyDeniedTest = assert(not(createAllowed(BEACON, one("MaskedPaths", "[]")))) run pidModeHostDeniedTest = assert(not(createAllowed(BEACON, one("PidMode", "host")))) @@ -585,6 +649,7 @@ module create { daemonSeesOnlyAllowlistedHostConfig, daemonMountsOnlyWhitelisted, daemonAttachesOnlyForcedNetwork, + daemonGroupsOnlyForced, }) } @@ -682,7 +747,9 @@ module docker_socket_policy { networkMode: "host", restartPolicy: "unless-stopped", securityOptions: Set("no-new-privileges:true"), - user: "2001:2001" + user: "2001:2001", + // The beacon reads the JWT secret through a forced group (#93). + groupAdd: GROUP_LIST }, volumes: Set("/home/beacon", "/etc/jwt_dir"), readOnlyVolumes: Set("/etc/jwt_dir"), @@ -709,7 +776,8 @@ module docker_socket_policy { networkMode: "host", restartPolicy: "unless-stopped", securityOptions: Set("no-new-privileges:true"), - user: "2002:2002" + user: "2002:2002", + groupAdd: "" }, volumes: Set("/home/validator"), readOnlyVolumes: Set(), diff --git a/ts/src/middleware.test.ts b/ts/src/middleware.test.ts index 6c7b06a..663e779 100644 --- a/ts/src/middleware.test.ts +++ b/ts/src/middleware.test.ts @@ -1,6 +1,14 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; -import { Chain, RegistryGate, MountSourceGate, EnvFileGate, CmdGate, ContainerConfigMutator } from "./middleware.js"; +import { + Chain, + RegistryGate, + MountSourceGate, + EnvFileGate, + CmdGate, + ContainerConfigMutator, + HostConfigGate, +} from "./middleware.js"; import type { Policy } from "./policy.js"; function makePolicy(overrides: Partial = {}): Policy { @@ -377,7 +385,8 @@ describe("RegistryGate reference parsing (#84)", () => { // // Policies: "std" has volumes /data (read_write) and /ro (read-only) and // forces network_mode host; "secopt" is "std" plus security_options -// [no-new-privileges:true]; "bare" has no volumes and no container_config. +// [no-new-privileges:true]; "bare" has no volumes and no container_config; +// "groups" is "std" plus group_add ["64190", "jwt"] (#93). // want is "" for an allowed create, or a substring of the deny reason. fwd, // when set, lists HostConfig fields the forwarded body must hold. interface HostConfigRow { @@ -397,6 +406,7 @@ function hostConfigRowPolicy(name: string): Policy { ]; p.container_config = { network_mode: "host" }; if (name === "secopt") p.container_config.security_options = ["no-new-privileges:true"]; + if (name === "groups") p.container_config.group_add = ["64190", "jwt"]; return p; } @@ -436,6 +446,63 @@ const hostConfigRows: HostConfigRow[] = [ want: "HostConfig.GroupAdd is not allowed for service svc", fwd: String.raw``, }, + { + name: "groupAddPolicyListWithoutGroupAddDenied", + policy: "std", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["64190","jwt"]}}`, + want: "HostConfig.GroupAdd is not allowed for service svc", + fwd: String.raw``, + }, + { + name: "groupAddForced", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1"}`, + want: "", + fwd: String.raw`{"GroupAdd":["64190","jwt"]}`, + }, + { + name: "groupAddForcedNullHostConfig", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":null}`, + want: "", + fwd: String.raw`{"GroupAdd":["64190","jwt"]}`, + }, + { + name: "groupAddCallerReplaced", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["docker","0"]}}`, + want: "", + fwd: String.raw`{"GroupAdd":["64190","jwt"]}`, + }, + { + name: "groupAddCallerNullReplaced", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":null}}`, + want: "", + fwd: String.raw`{"GroupAdd":["64190","jwt"]}`, + }, + { + name: "groupAddSameListUnchanged", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":{"GroupAdd":["64190","jwt"]}}`, + want: "", + fwd: String.raw`{"GroupAdd":["64190","jwt"]}`, + }, + { + name: "groupAddCaseVariantDenied", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":{"groupadd":["0"]}}`, + want: "HostConfig.groupadd is not allowed for service svc", + fwd: String.raw``, + }, + { + name: "groupAddNonObjectHostConfigDenied", + policy: "groups", + body: String.raw`{"Image":"chainsafe/lodestar:v1","HostConfig":["GroupAdd"]}`, + want: "HostConfig must be an object for service svc", + fwd: String.raw``, + }, + { name: "volumesFromDenied", policy: "std", @@ -1320,6 +1387,64 @@ const hostConfigRows: HostConfigRow[] = [ }, ]; +// HostConfigGate alone, on the "groups" policy (#93): as if the mutator +// stopped forcing the list. Only the policy's list, exactly, or a zero value +// passes. The same rows, by name, are in +// go/internal/middleware/hostconfig_test.go and rs/src/middleware.rs. +const groupAddGateRows: [string, string, string][] = [ + ["groupAddGateForcedListAllowed", String.raw`{"HostConfig":{"GroupAdd":["64190","jwt"]}}`, ""], + ["groupAddGateEmptyAllowed", String.raw`{"HostConfig":{"GroupAdd":[]}}`, ""], + ["groupAddGateNullAllowed", String.raw`{"HostConfig":{"GroupAdd":null}}`, ""], + [ + "groupAddGateOtherListDenied", + String.raw`{"HostConfig":{"GroupAdd":["docker"]}}`, + "HostConfig.GroupAdd is not allowed for service svc", + ], + [ + "groupAddGateReorderedDenied", + String.raw`{"HostConfig":{"GroupAdd":["jwt","64190"]}}`, + "HostConfig.GroupAdd is not allowed for service svc", + ], + [ + "groupAddGateSubsetDenied", + String.raw`{"HostConfig":{"GroupAdd":["64190"]}}`, + "HostConfig.GroupAdd is not allowed for service svc", + ], + [ + "groupAddGateSupersetDenied", + String.raw`{"HostConfig":{"GroupAdd":["64190","jwt","0"]}}`, + "HostConfig.GroupAdd is not allowed for service svc", + ], + [ + "groupAddGateNumberDenied", + String.raw`{"HostConfig":{"GroupAdd":[64190,"jwt"]}}`, + "HostConfig.GroupAdd is not allowed for service svc", + ], + [ + "groupAddGateStringDenied", + String.raw`{"HostConfig":{"GroupAdd":"64190"}}`, + "HostConfig.GroupAdd is not allowed for service svc", + ], +]; + +describe("GroupAdd gate rows (#93)", () => { + for (const [name, body, want] of groupAddGateRows) { + it(name, () => { + const reason = new HostConfigGate().check( + "POST", + "/containers/create", + hostConfigRowPolicy("groups"), + JSON.parse(body) as Record, + ); + if (want === "") { + assert.equal(reason, null); + } else { + assert.ok(reason !== null && reason.includes(want), "expected deny with " + want + ", got: " + reason); + } + }); + } +}); + describe("HostConfig rows (#88)", () => { for (const row of hostConfigRows) { it(row.name, () => { diff --git a/ts/src/middleware.ts b/ts/src/middleware.ts index dd00d2e..96441bf 100644 --- a/ts/src/middleware.ts +++ b/ts/src/middleware.ts @@ -442,6 +442,14 @@ const HOST_CONFIG_MODES = new Map([ const NO_NEW_PRIVILEGES = new Set(["no-new-privileges", "no-new-privileges:true"]); +// Whether value is a JSON list of strings equal, in order, to the policy's +// non-empty group_add (#93). +function equalsForcedGroups(value: unknown, policy: Policy): boolean { + const forced = policy.container_config?.group_add; + if (!forced?.length || !Array.isArray(value) || value.length !== forced.length) return false; + return value.every((v, i) => v === forced[i]); +} + function checkSecurityOpt(value: unknown, policy: Policy): string | null { if (value === null) return null; if (!Array.isArray(value)) return `HostConfig.SecurityOpt must be a list for service ${policy.service_name}`; @@ -500,6 +508,15 @@ export class HostConfigGate implements Gate { if (err !== null) return err; continue; } + // Zero, or exactly the policy's group_add, which the mutator forces + // (#93). Checked here too so that the gate does not open up if the + // mutator ever stops forcing it. + if (key === "GroupAdd") { + if (!isZeroValue(value) && !equalsForcedGroups(value, policy)) { + return `HostConfig.GroupAdd is not allowed for service ${policy.service_name}`; + } + continue; + } if (key === "OomScoreAdj") { if (value !== null && !(typeof value === "number" && value >= 0)) { return `HostConfig.OomScoreAdj value ${JSON.stringify(value)} is not allowed for service ${policy.service_name}`; @@ -665,6 +682,8 @@ export class ContainerConfigMutator implements Mutator { if (cc.network_mode) hostConfig["NetworkMode"] = cc.network_mode; if (cc.restart_policy) hostConfig["RestartPolicy"] = { Name: cc.restart_policy }; if (cc.security_options?.length) hostConfig["SecurityOpt"] = cc.security_options; + // Overwritten like User (#93); HostConfigGate allows exactly this list. + if (cc.group_add?.length) hostConfig["GroupAdd"] = [...cc.group_add]; if (cc.user) body["User"] = cc.user; if (cc.log_driver) { hostConfig["LogConfig"] = { Type: cc.log_driver, Config: cc.log_options ?? {} }; diff --git a/ts/src/policy.test.ts b/ts/src/policy.test.ts index 96e64fc..cdd0467 100644 --- a/ts/src/policy.test.ts +++ b/ts/src/policy.test.ts @@ -233,6 +233,95 @@ describe("Manager container names", () => { { "a.yaml": "service_name: a\ncontainer_names: []\nallowed_image_prefixes: [alpine]\n" }, "container_names must not be empty", ], + // container_config.group_add (#93). + [ + "group_add GIDs and names", + { + "a.yaml": + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '4294967295', jwt, _x-1, 'samba$', abcdefghijklmnopqrstuvwxyz012345]\n", + }, + "", + ], + [ + "group_add empty", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: []\n" }, + "", + ], + [ + "group_add 0", + { + "a.yaml": + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '0']\n", + }, + 'policy file a.yaml: group_add entry "0" is the root group, which is not allowed', + ], + [ + "group_add 00", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['00']\n" }, + 'policy file a.yaml: group_add entry "00" is the root group, which is not allowed', + ], + [ + "group_add root", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [root]\n" }, + 'policy file a.yaml: group_add entry "root" is the root group, which is not allowed', + ], + [ + "group_add duplicate", + { + "a.yaml": + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', jwt, '64190']\n", + }, + 'policy file a.yaml: duplicate group_add entry "64190"', + ], + [ + "group_add duplicate GID with leading zero", + { + "a.yaml": + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['64190', '064190']\n", + }, + 'policy file a.yaml: duplicate group_add entry "064190"', + ], + [ + "group_add GID too large", + { + "a.yaml": + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['4294967296']\n", + }, + 'policy file a.yaml: invalid group_add entry "4294967296"', + ], + [ + "group_add upper-case name", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [Jwt]\n" }, + 'policy file a.yaml: invalid group_add entry "Jwt"', + ], + [ + "group_add name with a space", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['a b']\n" }, + 'policy file a.yaml: invalid group_add entry "a b"', + ], + [ + "group_add name starting with a dash", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['-x']\n" }, + 'policy file a.yaml: invalid group_add entry "-x"', + ], + [ + "group_add name of 33 characters", + { + "a.yaml": + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [abcdefghijklmnopqrstuvwxyz0123456]\n", + }, + 'policy file a.yaml: invalid group_add entry "abcdefghijklmnopqrstuvwxyz0123456"', + ], + [ + "group_add empty string", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: ['']\n" }, + 'policy file a.yaml: invalid group_add entry ""', + ], + [ + "group_add unquoted number", + { "a.yaml": "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: [64190]\n" }, + "group_add entries must be strings", + ], ]; const mismatches: string[] = []; for (const [name, files, want] of cases) { @@ -252,6 +341,16 @@ describe("Manager container names", () => { assert.deepEqual(mismatches, []); }); + // An empty group_add is the same as an unset one (#93). + it("treats an empty group_add as unset", () => { + writeFileSync( + join(tmpDir, "a.yaml"), + "service_name: a\nallowed_image_prefixes: [alpine]\ncontainer_config:\n group_add: []\n", + ); + const m = new Manager(tmpDir); + assert.equal(m.get("a")!.container_config!.group_add, undefined); + }); + // #85 selection: the one policy matching the image and accepting the name, // the nameless create, and ambiguity. it("selects the create policy by image and name", () => { diff --git a/ts/src/policy.ts b/ts/src/policy.ts index 00b9af8..af32e0c 100644 --- a/ts/src/policy.ts +++ b/ts/src/policy.ts @@ -26,6 +26,8 @@ export interface ContainerConfig { user?: string; log_driver?: string; log_options?: Record; + // Forced as HostConfig.GroupAdd (#93). An empty list is unset. + group_add?: string[]; } export interface Volume { @@ -39,6 +41,45 @@ export interface FlagRule { value_pattern: string; } +const GROUP_GID = /^[0-9]+$/; +const GROUP_NAME = /^[a-z_][a-z0-9_-]*\$?$/; + +// Validates container_config.group_add (#93): each entry is a YAML string +// holding a decimal GID that fits in 32 bits or a group name of at most 32 +// characters; the root group (0 in any decimal form, or root) and duplicates +// (by GID value for numbers) are refused. Only strings are taken: the YAML +// parser reads an unquoted 0x10 as 16, where Go and Rust would read "0x10". +function checkGroupAdd(file: string, groups: unknown): void { + if (!Array.isArray(groups)) throw new Error(`policy file ${file}: group_add must be a list`); + const q = JSON.stringify; + const seen = new Set(); + for (const g of groups) { + if (typeof g !== "string") { + throw new Error(`policy file ${file}: group_add entries must be strings (quote GIDs: '64190')`); + } + let key: string; + if (GROUP_GID.test(g)) { + const gid = BigInt(g); + if (gid > 0xffffffffn) { + throw new Error(`policy file ${file}: invalid group_add entry ${q(g)} (want a decimal GID or a group name)`); + } + if (gid === 0n) { + throw new Error(`policy file ${file}: group_add entry ${q(g)} is the root group, which is not allowed`); + } + key = `gid:${gid}`; + } else if (g.length <= 32 && GROUP_NAME.test(g)) { + if (g === "root") { + throw new Error(`policy file ${file}: group_add entry ${q(g)} is the root group, which is not allowed`); + } + key = `name:${g}`; + } else { + throw new Error(`policy file ${file}: invalid group_add entry ${q(g)} (want a decimal GID or a group name)`); + } + if (seen.has(key)) throw new Error(`policy file ${file}: duplicate group_add entry ${q(g)}`); + seen.add(key); + } +} + // A Docker container name without the optional leading slash (#85). const VALID_CONTAINER_NAME = /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/; @@ -112,6 +153,14 @@ export class Manager { throw new Error(`policy file ${entry}: invalid container name ${JSON.stringify(bad)} in container_names`); } } + const cc = policy.container_config; + if (cc && cc.group_add !== undefined && cc.group_add !== null) { + checkGroupAdd(entry, cc.group_add); + // An empty group_add is the same as an unset one. + if (cc.group_add.length === 0) delete cc.group_add; + } else if (cc) { + delete cc.group_add; + } // A second file with the same service_name used to replace the first // silently (#85). const prev = fileOf.get(policy.service_name);