Skip to content

skrog serve and skrog proxy install no policy gate and no audit sink #257

Description

@zcsizmadia

Found by an independent security review; verified against the code.

Only the supervisor uses RewriteBindsGuarded(auditor, watcher) (cmd/skrog/supervise.go:245). cmd/skrog/serve.go:96 and cmd/skrog/proxy.go:108 use bare RewriteBinds.

So skrog serve --tcp — the surface whose own help text says it exists so "a teammate or a CI runner" can drive this engine — enforces none of the rules the machine owner wrote, and audits none of the calls.

The "hostile local user owns the machine anyway" disclaimer does not cover this one. The entire point of skrog serve is that the requester is not the machine owner and holds only a client certificate. docs/policy.md:131-138 scopes the feature by API endpoint and never by listener, so nothing tells an operator that the rules stop at the pipe.

skrog proxy is documented as a debug mode, which is a weaker case, but it drops auditing silently too.

Fix shape: the guarded rewriter should be the default for every listener, with any exception opted into explicitly rather than inherited by omission.

Lower-severity notes from the same pass, recorded here rather than as separate issues:

  • docker exec --privileged is never judged. docs/policy.md:131-133 scopes judgement to /containers/create "which is where --privileged, capabilities, namespaces, binds and the image reference all arrive". That parenthetical is wrong for exec, which carries its own Privileged field. The scope limit is disclosed; its justification is not accurate.
  • A missing host-side agent secret silently downgrades the vsock handshake. cmd/skrog/dialer.go:32-38 passes an empty secret on any read error, and the dialer has no memory that this install did have one — so removing one small file turns mutual authentication off with no error and no log line. vsockproto's package doc describes the scheme as downgrade-resistant, which is true of the protocol and not of the caller deciding whether it holds a secret. docs/security.md:179-180 already lists this boundary as unhardened, so the docs are conservative; the gap is between the code and its own claim.
  • export HTTP_PROXY=%q is sourced by sh as root (internal/provision/provision.go:338-353). %q does not neutralise command substitution inside a double-quoted shell string. Input is owner-supplied today, so impact is low; single-quoting removes the class.
  • cmd/skrog/proxy.go:23 help text still describes the pipe ACL as "SYSTEM, admins and interactive users". Pipe DACL grants GENERIC_ALL to all interactive users (cross-session access to the engine) #79 tightened that to the owning user. Harmless direction, but it contradicts docs/security.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Fold into v0.3 stagesbugSomething isn't workingreview-findingFrom the 2026-09 independent 4-agent review of v0.2.0securitySecurity boundary or hardening

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions