From fe5a3aa99ddec3acbd84ad6d680a2d6933377885 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:03:04 -0400 Subject: [PATCH] harden(install): tag the journalctl sudo grants NOEXEC (#472) journalctl starts a pager when its output is a terminal, and from less a "!sh" is a shell with whatever privileges journalctl was given. That is the standard journalctl escalation, and these rules end in a wildcard: ALL=(ALL) NOPASSWD: /usr/bin/journalctl -u ledmatrix * Nothing this project runs needs the pager -- both call sites pass --no-pager, in web_interface/app.py and api_v3.py. But a sudoers rule cannot require a flag that sits in the middle of a command line, and reasoning about what a trailing wildcard does and does not admit is exactly the kind of subtlety that produces a hole. sudo's NOEXEC tag stops the command executing another program at all, which closes it without depending on that reasoning. NOEXEC works by LD_PRELOAD, so it applies to dynamically linked binaries. Checked on the target hardware: journalctl there is dynamically linked. The generated rules were run through `visudo -c` -- parsed OK. Found while auditing the pre-existing wildcard grants, prompted by review catching a far worse one I had added myself in the same area: `iptables *`, where --modprobe runs an arbitrary path as root. Reachability, stated plainly: on a stock Raspberry Pi image none of this matters, because 010_pi-nopasswd already grants the default user `ALL=(ALL) NOPASSWD: ALL`. It matters on a hardened install, or where the service runs as a user without that blanket rule. Two mutation checks: dropping NOEXEC from a rule fails, and deleting the rules rather than tagging them fails too -- that second one matters, since "make the test pass" and "remove the feature" would otherwise look the same. --- first_time_install.sh | 13 +++- test/test_sudoers_noexec_on_pagers.py | 87 +++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 test/test_sudoers_noexec_on_pagers.py diff --git a/first_time_install.sh b/first_time_install.sh index 1a25bb25..c7ec7cf1 100644 --- a/first_time_install.sh +++ b/first_time_install.sh @@ -1419,9 +1419,16 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/ EOF if [ -n "$JOURNALCTL_PATH" ]; then cat >> /tmp/ledmatrix_web_sudoers << EOF -$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix.service * -$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix * -$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -t ledmatrix * +# NOEXEC, because these rules end in a wildcard and journalctl starts a pager +# when its output is a terminal. From that pager (less) a "!sh" is a root +# shell -- the standard journalctl escalation. The web interface always passes +# --no-pager, so nothing here needs it, but the rule cannot require a flag that +# sits in the middle of the command line. NOEXEC stops the command executing +# another program at all, which closes the hole without depending on wildcard +# matching subtleties. +$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service * +$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix * +$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix * EOF fi diff --git a/test/test_sudoers_noexec_on_pagers.py b/test/test_sudoers_noexec_on_pagers.py new file mode 100644 index 00000000..623622d7 --- /dev/null +++ b/test/test_sudoers_noexec_on_pagers.py @@ -0,0 +1,87 @@ +"""Wildcard grants to commands that start a pager must carry NOEXEC. + +`journalctl` runs a pager when its output is a terminal, and from `less` a +`!sh` is a shell with the privileges journalctl was given. That is the standard +journalctl privilege escalation, and the installer's rules end in a wildcard: + + ALL=(ALL) NOPASSWD: /usr/bin/journalctl -u ledmatrix * + +The web interface always passes --no-pager -- both call sites do, in app.py and +api_v3.py -- so nothing the project runs needs the pager. But a sudoers rule +cannot require a flag that sits in the middle of the command line, and reasoning +about what a trailing `*` does or does not admit is exactly the kind of +subtlety that produces a hole. + +sudo's NOEXEC tag stops the command executing another program at all, which +closes it without depending on that reasoning. It works by LD_PRELOAD, so it +applies to dynamically linked binaries; journalctl is one. + +On a stock Raspberry Pi image none of this is reachable, because +/etc/sudoers.d/010_pi-nopasswd already grants the default user +`ALL=(ALL) NOPASSWD: ALL`. It matters on a hardened install, or where the +service runs as a user without that blanket rule. +""" +import re +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +INSTALLERS = ( + ROOT / "first_time_install.sh", + ROOT / "scripts" / "install" / "configure_wifi_permissions.sh", +) + +#: Commands that will start another program of their own accord -- a pager, an +#: editor, a shell -- and so must not be granted the ability to do so. +SPAWNS_A_PROGRAM = ("journalctl", "systemctl", "less", "more", "man", "git") + + +def _grant_lines(): + lines = [] + for installer in INSTALLERS: + if not installer.is_file(): + continue + for line in installer.read_text(encoding="utf-8", errors="replace").splitlines(): + stripped = line.strip() + if "NOPASSWD" in stripped and not stripped.startswith("#"): + lines.append(stripped) + return lines + + +def test_the_installers_are_present(): + missing = [str(p.relative_to(ROOT)) for p in INSTALLERS if not p.is_file()] + assert not missing, f"installer(s) missing: {missing}" + + +def test_wildcard_pager_grants_carry_noexec(): + offenders = [] + for rule in _grant_lines(): + command = rule.split("NOPASSWD", 1)[1] + if not command.rstrip().endswith("*"): + continue + tool = command.replace("_PATH", "").replace("$", "").lower() + for name in SPAWNS_A_PROGRAM: + if re.search(rf"(^|/|\s){name}(\s|$)", tool): + if "NOEXEC" not in rule: + offenders.append(rule) + break + assert not offenders, ( + "wildcard grant to a command that can start a pager or shell, without " + "NOEXEC:\n " + "\n ".join(offenders)) + + +def test_journalctl_is_granted_at_all(): + """Guard against 'fixing' the above by deleting the rules.""" + text = "\n".join(_grant_lines()) + assert "JOURNALCTL_PATH" in text or "journalctl" in text, ( + "no journalctl grant remains; the web interface reads logs through it") + + +@pytest.mark.parametrize("unit", ["ledmatrix.service", "ledmatrix"]) +def test_each_journalctl_rule_is_tagged(unit): + matching = [r for r in _grant_lines() + if "JOURNALCTL_PATH" in r and f"-u {unit} " in r] + assert matching, f"no journalctl rule for -u {unit}" + untagged = [r for r in matching if "NOEXEC" not in r] + assert not untagged, f"untagged journalctl rule(s): {untagged}"