mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-20 18:09:05 +00:00
ChuckBuilds
fb0e869bf6
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:
<user> 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.
88 lines
3.4 KiB
Python
88 lines
3.4 KiB
Python
"""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:
|
|
|
|
<user> 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}"
|