-
Notifications
You must be signed in to change notification settings - Fork 3
Remove the residual root escalation left after the sudoers narrowing (TASK-445) #471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
c0832b4
Add a fail-closed sudoers coverage check and prune 10 dead grants (TA…
KrasimirKralev d46657a
Replace the blanket sudoers grant on already-shipped devices (TASK-445)
KrasimirKralev e2277a2
Prove the narrowed grant on a device install.sh provisioned (TASK-445)
KrasimirKralev f7662e1
Install the ollama sudo grant from the step that always runs (TASK-445)
KrasimirKralev d6977f8
fix: prove the allow-list landed before quarantining the blanket sudo…
1843dfb
fix: restart the Hermes gateway through systemctl and stop faking the…
fc15707
docs: correct the EXEMPT_CALLS rationale for the sudo'd Hermes CLI
a273101
test: sharpen the Hermes-unit sudoers tripwire instead of removing it
5563de5
fix: stop the root steps running code the clawbox user can rewrite (T…
6be559a
fix: enumerate the root-update grants instead of matching a clawbox-*…
5a93fbf
fix: start ollama through sudo during factory reset (TASK-445)
1e7d9db
fix: parse the clawbox-writable env files instead of sourcing them as…
b71e383
fix: keep the root-exec record from refusing the device it protects (…
52c88c4
fix: do not fail the granted-path audit on a grant with no snapd behi…
a9d120e
fix: hash the copy root is about to run, not the path (TASK-445)
2e647c4
Merge pull request #495 from ID-Robots/fix/sudoers-445-followup
KrasimirKralev 86dc561
Merge pull request #502 from ID-Robots/fix/sudoers-445-root-paths
KrasimirKralev 1a7f7b2
Merge branch 'beta' into fix/hermes-sudoers-445-r2
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,204 @@ | ||
| #!/usr/bin/env bash | ||
| # | ||
| # Integrity manifest for the code root executes on the clawbox user's behalf. | ||
| # | ||
| # The privilege chain is: | ||
| # | ||
| # clawbox --sudo--> systemctl start clawbox-root-update@<step>.service | ||
| # --systemd--> /usr/local/libexec/clawbox/clawbox-root-step.sh (root:root) | ||
| # --exec--> /home/clawbox/clawbox/install.sh --step <step> | ||
| # | ||
| # Only the middle link is root-owned. install.sh is `clawbox:clawbox 0755` in a | ||
| # `clawbox:clawbox 0775` directory — install.sh itself hands the tree back with | ||
| # `chown -R clawbox:clawbox` on every root run — and the steps it dispatches go | ||
| # on to run more of the same tree as root (scripts/start-ap.sh, | ||
| # scripts/launch-browser.sh, scripts/setup-hermes-edition.sh, …). So anything | ||
| # with clawbox-level code execution — the web server, the in-UI terminal, the | ||
| # agent's shell — could rewrite the program root was about to run and then | ||
| # trigger a granted step. That is passwordless local root in two moves, and it | ||
| # is the defect TASK-445 was filed about. | ||
| # | ||
| # Moving the tree out of clawbox's reach is not an option: the updater has to be | ||
| # able to replace it, and the app has to be able to build in it. So instead the | ||
| # root side REFUSES to run code it did not record. This file writes and checks | ||
| # that record: | ||
| # | ||
| # * install.sh writes the manifest at the end of every root-side install and | ||
| # immediately after every successful `git reset --hard` to the update branch | ||
| # (install.sh's bootstrap block and sync_repo_to_update_target). Those are | ||
| # the only two ways the covered files are supposed to change. | ||
| # * clawbox-root-step.sh verifies it before exec'ing anything. A tampered or | ||
| # unrecorded tree fails the step instead of running as root. | ||
| # | ||
| # What this does and does not buy: | ||
| # | ||
| # * It closes the "rewrite install.sh, then start a granted unit" path — the | ||
| # dispatcher refuses before the exec. | ||
| # * It does NOT make the box safe against someone who can already run code as | ||
| # root, and it does not authenticate the UPDATE itself: an update legitimately | ||
| # replaces the covered files and re-records them. The update path is gated | ||
| # on the dashboard session instead (TASK-445's "require auth for update"). | ||
| # | ||
| # Usage (root only): | ||
| # clawbox-root-manifest.sh --write record the tree as it is now | ||
| # clawbox-root-manifest.sh --verify exit 0 if it still matches, 65 if not | ||
| # | ||
| # Installed by install.sh::install_root_libexec to | ||
| # /usr/local/libexec/clawbox/clawbox-root-manifest.sh, root:root 0755. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| # Hard-coded on purpose. Every value below selects WHICH code root executes, so | ||
| # none of them is overridable from the environment: this script runs from a | ||
| # systemd unit reached through a NOPASSWD sudoers grant, and an env escape hatch | ||
| # would be a second way to point root at a file the clawbox user chose. | ||
| PROJECT_DIR="/home/clawbox/clawbox" | ||
| MANIFEST_DIR="/etc/clawbox" | ||
| MANIFEST_FILE="/etc/clawbox/root-exec.manifest" | ||
|
|
||
| # Everything the clawbox-root-update@ chain can end up running as root: | ||
| # install.sh, the scripts it hands to bash, and the config/unit files it installs. | ||
| # Runtime state — data/, .next/, node_modules/, .git/ — is deliberately NOT | ||
| # covered: it is clawbox's to write and root never executes it, so covering it | ||
| # would turn every build into a manifest mismatch. | ||
| COVERED_PATHS="install.sh scripts config" | ||
|
|
||
| # Generated content that lives INSIDE a covered path, and must not be recorded. | ||
| # `scripts/__pycache__/` is the one that bites: gateway-pre-start.sh imports | ||
| # scripts/gateway_origins.py, so CPython writes a .pyc there the first time the | ||
| # gateway starts — after the manifest was written, and again under a different | ||
| # name after any python3 minor-version bump. Recording those would make an | ||
| # ordinary first boot, or an ordinary distro upgrade, refuse every root step. | ||
| PRUNE_DIRS="__pycache__ node_modules .venv venv" | ||
|
|
||
| die() { | ||
| echo "clawbox-root-manifest: $1" >&2 | ||
| exit "${2:-65}" | ||
| } | ||
|
|
||
| # Covered files, relative to PROJECT_DIR, NUL-delimited and byte-sorted. | ||
| # Callers must already be in PROJECT_DIR. | ||
| # | ||
| # `-type f` excludes symlinks deliberately: what gets RECORDED is a real file | ||
| # and its real content. Verification then re-opens the recorded path, so | ||
| # replacing one of these with a symlink to something else changes the hash and | ||
| # fails — which is the answer we want, rather than recording the link. | ||
| covered_files() { | ||
| local p | ||
| local -a args=() prune=() | ||
| for p in $COVERED_PATHS; do | ||
| [ -e "$p" ] && args+=("$p") | ||
| done | ||
| [ "${#args[@]}" -gt 0 ] || return 1 | ||
| for p in $PRUNE_DIRS; do | ||
| prune+=(-name "$p" -prune -o) | ||
| done | ||
| find "${args[@]}" "${prune[@]}" -type f -print0 | LC_ALL=C sort -z | ||
| } | ||
|
|
||
| write_manifest() { | ||
| cd "$PROJECT_DIR" || die "$PROJECT_DIR is missing" 66 | ||
|
|
||
| # ONE walk, so the names that are checked are exactly the names that are | ||
| # hashed. Walking twice — once to check, once to hash — leaves a window in | ||
| # which a file that appears in between is recorded without ever having been | ||
| # checked. | ||
| # | ||
| # The check itself: sha256sum ESCAPES a filename containing a backslash or a | ||
| # newline (it prefixes the line with `\` and re-encodes them), and | ||
| # verify_manifest reads the path column back with a fixed-width strip. Refuse | ||
| # to record such a name rather than record one this file cannot parse. | ||
| local f | ||
| local -a files=() | ||
| while IFS= read -r -d '' f; do | ||
| case "$f" in | ||
| *\\*|*$'\n'*) | ||
| die "refusing to record a path containing a backslash or a newline" | ||
| ;; | ||
| esac | ||
| files+=("$f") | ||
| done < <(covered_files) | ||
| [ "${#files[@]}" -gt 0 ] || die "nothing to record under $PROJECT_DIR" 66 | ||
|
|
||
| install -d -o root -g root -m 0755 "$MANIFEST_DIR" || die "cannot create $MANIFEST_DIR" 66 | ||
|
|
||
| # Staged inside the root-owned /etc/clawbox, never /tmp: a world-writable | ||
| # staging directory is one more place to race the file root ends up trusting. | ||
| local tmp | ||
| tmp="$(mktemp "$MANIFEST_FILE.XXXXXX")" || die "cannot stage a manifest" 66 | ||
| if ! printf '%s\0' "${files[@]}" | xargs -0 sha256sum > "$tmp"; then | ||
| rm -f "$tmp" | ||
| die "cannot hash $PROJECT_DIR" 66 | ||
| fi | ||
| if ! chmod 0644 "$tmp"; then | ||
| rm -f "$tmp" | ||
| die "cannot set the manifest mode" 66 | ||
| fi | ||
| if ! mv -f "$tmp" "$MANIFEST_FILE"; then | ||
| rm -f "$tmp" | ||
| die "cannot install $MANIFEST_FILE" 66 | ||
| fi | ||
| } | ||
|
|
||
| verify_manifest() { | ||
| [ -f "$MANIFEST_FILE" ] || die "no manifest at $MANIFEST_FILE" | ||
| cd "$PROJECT_DIR" || die "$PROJECT_DIR is missing" 66 | ||
|
|
||
| # Every recorded file must still be there and still hash to what was recorded. | ||
| # That covers the three things that matter: an edited file, a deleted file, and | ||
| # a file replaced by a symlink (sha256sum opens the path, so it hashes what the | ||
| # link resolves to and the content stops matching). | ||
| # | ||
| # A file ADDED under a covered path is deliberately NOT an error, even though | ||
| # `sha256sum -c` cannot see it. Root only ever executes files install.sh names | ||
| # explicitly, and all of those are recorded — so an unrecorded file is not | ||
| # something root can be made to run. Treating additions as tampering, on the | ||
| # other hand, turns any stray file under scripts/ into a device that refuses | ||
| # every root step for good: no password change, no hostname change, no hotspot | ||
| # restart, on an appliance with no console. That trade is the wrong way round. | ||
| sha256sum --status --strict -c "$MANIFEST_FILE" \ | ||
| || die "$PROJECT_DIR does not match $MANIFEST_FILE (a covered file changed or is gone)" | ||
| } | ||
|
|
||
| # Check ONE already-opened copy against what the manifest recorded for a path. | ||
| # | ||
| # `--verify` answers a question about the project tree, and the answer is stale | ||
| # the moment it returns: the clawbox user can replace a file between the check | ||
| # and the exec, and a tight rewrite loop wins that race. So the root dispatcher | ||
| # copies the file it is going to run into a root-only directory FIRST and then | ||
| # asks about the copy — which is the same bytes it will execute, and which | ||
| # clawbox cannot touch. | ||
| # | ||
| # clawbox-root-manifest.sh --verify-file <recorded path> <file to hash> | ||
| verify_file() { | ||
| local rel="$1" actual="$2" want="" got h p | ||
| [ -n "$rel" ] && [ -n "$actual" ] || die "usage: $0 --verify-file <recorded path> <file>" 64 | ||
| [ -f "$MANIFEST_FILE" ] || die "no manifest at $MANIFEST_FILE" | ||
| [ -f "$actual" ] || die "$actual is missing" 66 | ||
|
|
||
| # Read the recorded hash out of the sha256sum-format manifest by exact path | ||
| # match. write_manifest refuses names it would have to escape, so the path | ||
| # column is the plain name (with a leading `*` in binary mode). | ||
| while read -r h p; do | ||
| p="${p#\*}" | ||
| if [ "$p" = "$rel" ]; then | ||
| want="$h" | ||
| break | ||
| fi | ||
| done < "$MANIFEST_FILE" | ||
| [ -n "$want" ] || die "$rel is not in $MANIFEST_FILE" | ||
|
|
||
| got="$(sha256sum < "$actual")" | ||
| got="${got%% *}" | ||
| [ "$want" = "$got" ] || die "$actual does not match what $MANIFEST_FILE recorded for $rel" | ||
| } | ||
|
|
||
| case "${1:-}" in | ||
| --write) write_manifest ;; | ||
| --verify) verify_manifest ;; | ||
| --verify-file) verify_file "${2:-}" "${3:-}" ;; | ||
| *) | ||
| echo "usage: $0 --write|--verify|--verify-file <recorded path> <file>" >&2 | ||
| exit 64 | ||
| ;; | ||
| esac |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: ID-Robots/clawbox
Length of output: 3825
🏁 Script executed:
Repository: ID-Robots/clawbox
Length of output: 49648
🌐 Web query:
systemd service template instances Type=oneshot run concurrently serialization official documentation💡 Result:
By default, systemd does not serialize instances of a service template; all instances triggered at the same time will run concurrently [1]. There is no native flag or configuration option within a service template to limit concurrency or enforce serial execution across multiple instances of that template [2][1]. If you need to ensure that instances of a oneshot template run one after another rather than in parallel, you must implement explicit ordering dependencies [3][4]. While you cannot easily reference "all instances" of a template dynamically from within the template itself, you can achieve serialization through the following common approaches: 1. Explicit Dependency Ordering: You can define an After= dependency in the template to refer to a specific preceding instance [3]. However, this is often impractical for a dynamic or unknown number of instances [3]. 2. Wrapper Service: A common, recommended workaround is to create a "manager" or "wrapper" service (e.g., a oneshot service) that orchestrates the execution [5][6]. This wrapper can run a script that executes the logic or starts the necessary instances in a controlled, serial fashion, rather than relying on systemd to manage them as independent, concurrent units [1][5][6]. 3. External Coordination: Because systemd lacks a built-in semaphore or locking mechanism for service instances, advanced users often use external coordination (such as a lock file or a dedicated service that serializes requests) if strict concurrency control is required [2][4]. Note that Type=oneshot services are designed to execute commands and then transition to an inactive state (unless RemainAfterExit=yes is used) [7]. While this is appropriate for batch jobs, it does not inherently prevent concurrent execution of multiple instances if they are triggered simultaneously by systemd [1]. If you require sequential execution, you must structure your units or orchestration logic to enforce that order [3][4].
Citations:
Use a unique staged path for each invocation.
clawbox-root-update@.servicepermits concurrent template instances. They can share"$RUN_DIR/root-step-install.sh". If one instance replaces the file after another instance verifies it but beforeexecopens it, the first instance can execute unverified bytes.Use a per-invocation staged path. Remove only that invocation's file on verification failure. Do not remove all files for the same step, because concurrent invocations of that step would still collide.
🤖 Prompt for AI Agents