Skip to content

SECURITY.md says the enforcement layers are independent; the sudoers file says they are not #483

Description

@vladimirrott

Two files in this repository describe the same trust boundary and disagree
about it.

SECURITY.md:43-44:

SysKnife uses a layered enforcement model. Every layer is independent; a
bypass of one does not bypass the others.

packaging/sysknife-sudoers:9-12:

# No shell or general runuser grant is permitted. Multi-step and user-scoped
# actions use independently validated, operation-restricted helpers. Other
# grants still cover powerful administrative tools: the daemon's authorization,
# validation and audit remain essential. See each grant's scope below.

The sudoers header has it right, and I would keep its wording over the
sentence in SECURITY.md.

Why the layers are not independent

All five layers in the Security Model run inside one process, and that
process runs as one account:

$ grep -nE '^(User|Group)=' packaging/sysknife-daemon.service
8:User=sysknife
9:Group=sysknife

That account holds grants that reach root with no shell grant involved:

$ grep -nE 'NOPASSWD: (/usr/sbin/useradd|/usr/bin/systemctl|/usr/bin/env DEBIAN)' packaging/sysknife-sudoers
31:sysknife ALL=(root) NOPASSWD: /usr/sbin/useradd
59:sysknife ALL=(root) NOPASSWD: /usr/bin/systemctl
123:sysknife ALL=(root) NOPASSWD: /usr/bin/env DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a /usr/bin/apt-get *

useradd with a free argument list creates a second uid-0 account, which I
checked rather than assumed:

$ podman run --rm debian:12-slim sh -c 'useradd -o -u 0 -g 0 -M backdoor; echo "rc=$?"; grep ^backdoor: /etc/passwd'
useradd warning: backdoor's uid 0 outside of the UID_MIN 1000 and UID_MAX 60000 range.
rc=0
backdoor:x:0:0::/home/backdoor:/bin/sh

apt-get behind a trailing wildcard accepts -o APT::Update::Pre-Invoke::=,
which runs an arbitrary command as root. Layers 1 through 5 all sit above
these grants, so one bypass of the daemon is a bypass of all five at once.

The case a reader will hit

ROOT_SHELL_UNITS reads like a containment boundary:

$ sed -n '185,186p;207,209p' crates/sysknife-daemon/src/actions/validate.rs
pub(crate) const ROOT_SHELL_UNITS: &[&str] =
    &["debug-shell", "emergency", "rescue", "runlevel1", "single"];
    if ROOT_SHELL_UNITS.contains(&bare) {
        return Err(ExecutorError::InvalidParam(param));
    }

A typed action naming rescue.target is refused. The same account runs
sudo -n /usr/bin/systemctl start rescue.target with no validator anywhere in
the path, because the grant on line 59 carries no argument restriction.

The denylist does the job it was written for: it stops a unit name arriving
from the LLM or from an MCP client, and the doc comment above it says so. It
was never containment, and the independence sentence invites a reader to
treat it as containment.

Why it matters

An operator sizing the blast radius of a daemon compromise reads SECURITY.md,
not a comment in the sudoers fragment. Today that reader concludes the daemon
is held by the layers under it. The accurate answer is that compromise of the
daemon process is compromise of root on that host, and the layers are
sequential gates on one request path rather than independent walls around it.

Three grants (systemctl, kill, useradd) carry no argument restriction,
and narrowing them is a separate piece of work. I am not asking for it here.
Narrowing them one at a time while apt-get * stands buys nothing, and that
apt grant is load-bearing for the package actions.

Scope

  • Rewrite the Security Model preamble to say what the model gives: each layer
    gates a different stage of one request path, and the sysknife service
    account is root-equivalent by design.
  • Put the daemon-compromise case in the preamble where an operator will read
    it, rather than leaving it to packaging/sysknife-sudoers.
  • Add the ROOT_SHELL_UNITS example, so the reason the denylist exists and
    the reason it is not containment are both written down once.
  • Read the rest of SECURITY.md for sentences leaning on independence. Layer 4
    already carries the right caveat ("This boundary protects against an
    untrusted MCP agent, not against arbitrary malware already running as the
    same Linux user"), and that is the register the preamble needs.

scripts/check_public_claims.sh screens prose in this repository. Say in the
PR whether the replacement wording can be pinned there, and pin it if it can.

Difficulty

easy. It is a documentation change. The work is choosing the wording; every
fact above is quoted from the tree, and the commands that produced the quotes
are in the blocks.

Getting started

CONTRIBUTING.md
has the build and test commands. No CLA and no copyright waiver. The project
is MIT.

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

    documentationImprovements or additions to documentationeasyDifficulty: self-contained, no deep context neededhelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions