Skip to content

Commit ac5731f

Browse files
committed
🔒 security(sweep): GA multi-lens findings — UID bypass, admin wide-open, bundle reload, config drift
Verified findings from a 6-lens adversarial sweep. One critical (HostConfig.Capabilities allowlist bypass) was investigated and REJECTED as a false positive — the field exists in no Docker release (v20.10/v24/master all lack it; PR #38380 reverted pre-release). - 🔒 security(filter): zero-padded numeric UIDs ("00","000:5") no longer bypass require_non_root_user / allow_root_user — parse the UID, treat any 0 as root - 🔒 security(config): wide-open dedicated admin listener (non-loopback plaintext, no CIDR allowlist) is now a validation error, gated by new admin.listen.insecure_allow_wide_open - 🐛 fix(reload): verifyBundle reads signature_path from the CANDIDATE config, not the last-applied one — rotating the bundle no longer wedges every subsequent reload - 🐛 fix(config): register allow_sysctls + image_trust.verify_timeout SetDefaults so their SOCKGUARD_* env vars are honored (Viper only unmarshals registered keys) - 🐛 fix(filter): oversized /nodes/*/update and /build bodies return 413, not 403 - ⚡ perf(proxy): drain upstream inspect responses before close (ownership + visibility) so daemon keep-alive connections are reused - 📝 docs(configs): drop non-existent allowed_users from cis + gitlab-runner exec comments - ✅ test: zero-padded UID boundaries, candidate signature_path, env-var defaults, node/build 413 - 📝 docs: admin.listen.insecure_allow_wide_open in configuration.mdx + env table; roadmap refresh Deferred to roadmap: stale per-profile in-flight gauge on profile removal; SecurityOpt label=/systempaths= passthrough (needs policy design).
1 parent 9c946a1 commit ac5731f

24 files changed

Lines changed: 278 additions & 27 deletions

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Security
11+
12+
- **Zero-padded numeric UIDs can no longer bypass root-user enforcement.** `require_non_root_user` (container create) and `allow_root_user: false` (exec) compared the user field to the exact string `"0"`, but Docker parses `Config.User` numerically — so `"00"`, `"000"`, or `"0000:5"` all run as root while slipping past the check. Both inspectors now parse the UID and treat any value resolving to 0 as root.
13+
- **A wide-open dedicated admin listener is now a validation error, not just a startup warning.** A non-loopback plaintext admin listener with no `clients.allowed_cidrs` exposes candidate-YAML submission and policy metadata with no authentication and no IP backstop. Validation now rejects that shape unless the operator sets `clients.allowed_cidrs`, configures `admin.listen.tls`, or explicitly acknowledges the exposure with the new `admin.listen.insecure_allow_wide_open: true`.
14+
1015
### Fixed
1116

17+
- **A hot reload that rotates `policy_bundle.signature_path` no longer wedges every subsequent reload.** Bundle verification resolved the signature file from the last-applied config instead of the candidate, so changing `signature_path` (which is intentionally reload-mutable) loaded the wrong bundle, failed the digest check, and — because the active config only advances on success — left the operator permanently stuck until a restart. Verification now reads the path from the candidate config.
18+
- **`SOCKGUARD_REQUEST_BODY_CONTAINER_CREATE_ALLOW_SYSCTLS` and the `image_trust.verify_timeout` env vars are no longer silently ignored.** Three `request_body` keys were missing their `SetDefault` registration, and Viper only unmarshals registered keys — so configuring those fields purely through their `SOCKGUARD_*` env vars (no config file) had no effect. All three are now registered.
19+
- **Oversized `POST /nodes/{id}/update` and `POST /build` request bodies now return `413` instead of `403`,** matching every other body-limited inspector (a policy `403` implied the request was forbidden by rule rather than too large).
1220
- **Release images now carry real `commit` and `built` metadata.** The release workflow's Docker build only passed the `VERSION` build arg, so `sockguard version` in published images reported `commit unknown / built unknown`. The workflow now passes `COMMIT` (the tagged SHA) and `BUILD_DATE` (UTC build timestamp) through to the existing ldflags wiring.
1321
- **The container-label ACL exclusivity warning now fires once per process instead of on every hot-reload.** It was wired into the handler-chain build, which reruns on each config reload, so long-running deployments with reload enabled accumulated duplicate warnings. The warning is now gated by a process-lifetime guard (and finally has test coverage for both the enable-check and the once semantics).
1422
- **Coalesced inspect-cache waiters now honor their own context.** A request that joined an in-flight upstream inspect blocked unconditionally until that inspect finished — a slow daemon could pin waiter goroutines past their own deadlines. Waiters now return `ctx.Err()` promptly on cancellation while the in-flight resolve (and its cache fill) continues for future callers.
@@ -17,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1725
### Changed
1826

1927
- **Test coverage for review-flagged gaps:** legacy (map-format) Docker filter encoding through the ownership and visibility rewrites end-to-end, the visibility meta-cache's one-inspect-per-TTL-window behavior, admin path-normalization variants on the policy-version interceptor (the validate interceptor already had them), and double-encoded `%252f` admin paths pinning the fail-closed pass-through. Corrected the visibility meta-cache code comment and the 1.3.0-rc.2 changelog wording: the cache accelerates repeated single-resource reads — list responses are filtered from their own payload and never inspect upstream.
28+
- **Upstream inspect responses are now drained before close** on the ownership and visibility owner-isolation paths, so idle keep-alive connections to the daemon are reused instead of discarded after each inspect.
29+
- **Corrected misleading exec comments in the `cis-docker-benchmark` and `gitlab-runner` presets** that referenced a non-existent `allowed_users` field; exec identity is controlled by `allow_root_user` and the argv `allowed_commands` allowlist.
2030

2131
## [1.3.0-rc.2] - 2026-06-10
2232

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,9 +461,9 @@ LinuxServer's socket-proxy env surface is already Tecnativa-compatible for the b
461461

462462
| Tier | Theme |
463463
|---|---|
464-
| Security hardening (v1.x) | Continued mutation-test hardening of the rule-evaluation core and config validators; swarm `ContainerSpec.User` / `Privileges` enforcement parity with container create; wide-open admin listener promoted from startup warning to error behind an explicit opt-in flag |
464+
| Security hardening (v1.x) | Continued mutation-test hardening of the rule-evaluation core and config validators; swarm `ContainerSpec.User` / `Privileges` enforcement parity with container create; `HostConfig.SecurityOpt` `label=`/`systempaths=` policy evaluation (currently passed through) |
465465
| Policy refinement (v1.x) | Multiple frontend listeners on the main proxy, named rule path aliases |
466-
| Internals (v1.x) | Code-review backlog: collapse the config → filter-options → policy translation layers behind a single source of truth (generated Viper defaults); allocation-free rate-limit bucket state (packed `atomic.Uint64`); profiling-gated JSON redaction fast path |
466+
| Internals (v1.x) | Code-review backlog: collapse the config → filter-options → policy translation layers behind a single source of truth (generated Viper defaults); allocation-free rate-limit bucket state (packed `atomic.Uint64`); profiling-gated JSON redaction fast path; clear per-profile in-flight gauges when a hot reload removes the profile |
467467
| Compliance (v1.x) | CIS Docker Benchmark control mapping, audit-ready policy templates |
468468
| Multi-host (v1.3) | Remote Docker TCP upstreams, multi-upstream fan-out, remote daemon health checking, connection pooling, automatic failover |
469469
| Extensibility (v1.x+) | Optional plugin extension points (WASM or Go plugins), OPA/Rego policy integration |

app/configs/cis-docker-benchmark.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ request_body:
115115
# 5.22 / 5.23 — no privileged exec, no root exec).
116116
exec:
117117
allow_privileged: false
118-
# allowed_users left empty = no `User:` override on exec. Combine
119-
# with allow_root_user: false below for full CIS 5.23 enforcement.
118+
# allow_root_user: false denies any exec whose User resolves to root
119+
# (empty, "root", or numeric 0 incl. zero-padded). For full CIS 5.23
120+
# enforcement, also pin the permitted argv with allowed_commands.
120121
allow_root_user: false
121122

122123
# Read responses: redact host paths, env, network topology, and other

app/configs/gitlab-runner.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ request_body:
147147
# is still denied; the runner never requests it in normal operation.
148148
allow_privileged: false
149149
# Most official CI images (alpine, ubuntu, node, python) run job scripts
150-
# as root. Set allow_root_user: false and provide allowed_users if your
151-
# pipeline enforces a non-root image standard.
150+
# as root. Set allow_root_user: false if your pipeline enforces a non-root
151+
# image standard; pair with allowed_commands to pin the permitted argv.
152152
allow_root_user: true
153153

154154
image_pull:

app/internal/cmd/serve.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,10 @@ func newAdminHTTPServer(handler http.Handler) *http.Server {
11861186
// for parsing and expose policy metadata — are reachable by any host that can
11871187
// route to the port, with neither authentication nor an IP backstop (#21). The
11881188
// CIDR guard in buildAdminHandlerChain closes the gap when clients.allowed_cidrs
1189-
// is set; this warning covers the remaining case where it is empty.
1189+
// is set. Config validation now rejects this shape outright unless
1190+
// admin.listen.insecure_allow_wide_open acknowledges it, so by the time this
1191+
// warning fires the operator has explicitly opted in — it keeps the exposure
1192+
// visible in the logs rather than gating startup.
11901193
func warnIfAdminListenerWideOpen(cfg *config.Config, logger *slog.Logger) {
11911194
listen := cfg.Admin.Listen
11921195
if listen.Address == "" || listen.TLS.Complete() {

app/internal/cmd/serve_policy_bundle_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,41 @@ func TestPolicyBundleReload_ParsesVerifiedBytesNotFile(t *testing.T) {
266266
}
267267
}
268268

269+
func TestPolicyBundleReload_ResolvesSignaturePathFromCandidate(t *testing.T) {
270+
// Regression: verifyBundle must load the bundle entity from the CANDIDATE
271+
// config's signature_path, not the last-applied config's. signature_path is
272+
// reload-mutable so an operator can rotate the bundle; reading it from
273+
// activeCfg loaded the wrong (old) bundle for the new YAML and — because
274+
// activeCfg only advances on success — wedged every subsequent reload.
275+
initial := policyBundleInitialConfig()
276+
initial.PolicyBundle.SignaturePath = "/old/sig.bundle.json"
277+
278+
verifier := &stubBundleVerifier{res: policybundle.VerifyResult{Signer: "keyed:x", DigestHex: "ab"}}
279+
f := newPolicyBundleFixture(t, initial, verifier)
280+
281+
// The candidate (parsed from the verified bytes) points at the rotated path.
282+
candidate := config.Defaults()
283+
candidate.PolicyBundle.Enabled = true
284+
candidate.PolicyBundle.SignaturePath = "/new/sig.bundle.json"
285+
candidate.PolicyBundle.AllowedSigningKeys = []config.PolicyBundleSigningKey{{PEM: "stub"}}
286+
f.loadCfg = &candidate
287+
288+
var gotPath string
289+
f.coordinator.deps.loadBundleEntity = func(p string) (verify.SignedEntity, error) {
290+
gotPath = p
291+
return &stubEntity{}, nil
292+
}
293+
294+
f.coordinator.reload()
295+
296+
if gotPath != "/new/sig.bundle.json" {
297+
t.Fatalf("loadBundleEntity path = %q, want the candidate's /new/sig.bundle.json (not active /old)", gotPath)
298+
}
299+
if got, ok := metricsReloadCount(t, f.registry, "ok"); !ok || got != 1 {
300+
t.Fatalf("ok count = %d (found=%v), want 1", got, ok)
301+
}
302+
}
303+
269304
// ---------------------------------------------------------------------------
270305
// verifyPolicyBundleAtStartup — exhaustive branch coverage.
271306
// Every failure branch of the signature gate aborts startup, so each one

app/internal/cmd/serve_reload.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cmd
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"io"
78
"log/slog"
89
"path/filepath"
@@ -273,14 +274,28 @@ func (c *reloadCoordinator) verifyBundle() (*policybundle.VerifyResult, []byte,
273274
if c.bundleVerifier == nil || !c.activeCfg.PolicyBundle.Enabled {
274275
return nil, nil, nil
275276
}
276-
if c.activeCfg.PolicyBundle.SignaturePath == "" {
277-
return nil, nil, errors.New("policy_bundle.signature_path is empty")
278-
}
279277
yamlBytes, err := c.deps.readConfigBytes(c.cfgFile)
280278
if err != nil {
281279
return nil, nil, err
282280
}
283-
entity, err := c.deps.loadBundleEntity(c.activeCfg.PolicyBundle.SignaturePath)
281+
// Resolve the bundle path from the CANDIDATE config, not c.activeCfg.
282+
// signature_path is intentionally reload-mutable (reload/diff.go) so an
283+
// operator can rotate the bundle file; reading it from the last-applied
284+
// config would load the wrong (old) bundle for the new YAML, fail the
285+
// digest check, and — because activeCfg only advances on success — leave
286+
// every subsequent reload permanently stuck on the stale path until a
287+
// restart. The trust root that actually authenticates the bundle is
288+
// reload-immutable, so taking the path from the unverified candidate adds
289+
// no trust: a forged path still needs a bundle signed by the pinned key.
290+
candidate, err := c.deps.loadConfigBytes(yamlBytes)
291+
if err != nil {
292+
return nil, nil, fmt.Errorf("parse candidate config for signature_path: %w", err)
293+
}
294+
signaturePath := candidate.PolicyBundle.SignaturePath
295+
if signaturePath == "" {
296+
return nil, nil, errors.New("policy_bundle.signature_path is empty")
297+
}
298+
entity, err := c.deps.loadBundleEntity(signaturePath)
284299
if err != nil {
285300
return nil, nil, err
286301
}

app/internal/config/admin_listener_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,14 @@ func TestValidateAdminListenAcceptsNonLoopbackWithBothOptIns(t *testing.T) {
9595
cfg.Admin.Listen.Address = "0.0.0.0:9000"
9696
cfg.Admin.Listen.InsecureAllowPlainTCP = true
9797
cfg.Admin.Listen.InsecureAllowUnauthenticatedClients = true
98+
// The plaintext opt-ins are necessary but no longer sufficient for the
99+
// admin listener: a wide-open admin surface (no CIDR allowlist) needs the
100+
// third explicit acknowledgment now that it is a validation error rather
101+
// than a startup warning. See validate_admin_wideopen_test.go.
102+
cfg.Admin.Listen.InsecureAllowWideOpen = true
98103

99104
if err := Validate(&cfg); err != nil {
100-
t.Fatalf("Validate() = %v, want nil with both admin insecure opt-ins", err)
105+
t.Fatalf("Validate() = %v, want nil with both admin insecure opt-ins + wide-open ack", err)
101106
}
102107
}
103108

app/internal/config/config.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,16 @@ type AdminConfig struct {
564564
// When false the admin endpoints fall back to riding the main listener.
565565
type AdminListenConfig struct {
566566
ListenConfig `mapstructure:",squash"`
567+
// InsecureAllowWideOpen is the third acknowledgment a wide-open admin
568+
// listener requires. The embedded insecure_allow_plain_tcp /
569+
// insecure_allow_unauthenticated_clients flags opt a non-loopback TCP
570+
// admin listener out of TLS, but unlike the main listener — where
571+
// unauthenticated requests still face the full policy filter chain — the
572+
// admin endpoints accept candidate YAML and expose policy metadata with
573+
// client CIDRs as their ONLY admission control. A non-loopback plaintext
574+
// admin listener with no clients.allowed_cidrs is therefore a validation
575+
// error unless this flag explicitly accepts that exposure.
576+
InsecureAllowWideOpen bool `mapstructure:"insecure_allow_wide_open"`
567577
}
568578

569579
// Configured reports whether an admin listener address has been requested.

app/internal/config/load.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ func setLoadDefaults(v *viper.Viper, defaults Config) {
107107
v.SetDefault("request_body.container_create.allowed_apparmor_profiles", defaults.RequestBody.ContainerCreate.AllowedAppArmorProfiles)
108108
v.SetDefault("request_body.container_create.deny_unconfined_apparmor", defaults.RequestBody.ContainerCreate.DenyUnconfinedAppArmor)
109109
v.SetDefault("request_body.container_create.allow_host_userns", defaults.RequestBody.ContainerCreate.AllowHostUserNS)
110+
v.SetDefault("request_body.container_create.allow_sysctls", defaults.RequestBody.ContainerCreate.AllowSysctls)
110111
v.SetDefault("request_body.container_create.required_labels", defaults.RequestBody.ContainerCreate.RequiredLabels)
111112
v.SetDefault("request_body.container_create.allowed_runtimes", defaults.RequestBody.ContainerCreate.AllowedRuntimes)
112113
v.SetDefault("request_body.exec.allow_privileged", defaults.RequestBody.Exec.AllowPrivileged)
@@ -159,7 +160,9 @@ func setLoadDefaults(v *viper.Viper, defaults Config) {
159160
v.SetDefault("request_body.service.allowed_capabilities", defaults.RequestBody.Service.AllowedCapabilities)
160161
v.SetDefault("request_body.service.allow_sysctls", defaults.RequestBody.Service.AllowSysctls)
161162
v.SetDefault("request_body.container_create.image_trust.require_rekor_inclusion", defaults.RequestBody.ContainerCreate.ImageTrust.RequireRekorInclusion)
163+
v.SetDefault("request_body.container_create.image_trust.verify_timeout", defaults.RequestBody.ContainerCreate.ImageTrust.VerifyTimeout)
162164
v.SetDefault("request_body.service.image_trust.require_rekor_inclusion", defaults.RequestBody.Service.ImageTrust.RequireRekorInclusion)
165+
v.SetDefault("request_body.service.image_trust.verify_timeout", defaults.RequestBody.Service.ImageTrust.VerifyTimeout)
163166
v.SetDefault("request_body.swarm.allow_force_new_cluster", defaults.RequestBody.Swarm.AllowForceNewCluster)
164167
v.SetDefault("request_body.swarm.allow_external_ca", defaults.RequestBody.Swarm.AllowExternalCA)
165168
v.SetDefault("request_body.swarm.allowed_join_remote_addrs", defaults.RequestBody.Swarm.AllowedJoinRemoteAddrs)

0 commit comments

Comments
 (0)