Bound credential input reads and verify registry lock recovery
This commit is contained in:
1 parent
12a03a1f97
commit
bf79a79342
4 files changed
+80
-3
No files matched your search
@@ -25,7 +25,9 @@ owner/profile-specific commands tagged `NOLOG_INPUT`, `NOLOG_OUTPUT` and
|
||||
`NOSETENV`. The frontend then queries the effective policy for the exact command
|
||||
using command-specific `sudo -n -ll` in the C locale. It requires one effective
|
||||
matching root rule, the exact command/arguments and explicit disabled I/O logging
|
||||
and environment overrides before sending any key. Generic Defaults and absence
|
||||
and environment overrides before sending any key. Helper input is limited to
|
||||
16384 bytes plus EOF with a five-second absolute deadline, so an open input pipe
|
||||
cannot hold the shared registry lock indefinitely. Generic Defaults and absence
|
||||
of logging flags are not accepted as evidence. Unknown formats fail closed.
|
||||
|
||||
On systemd 255, setup writes host-encrypted ciphertext and principal metadata under
|
||||
|
||||
@@ -17,9 +17,11 @@ import pwd
|
||||
import re
|
||||
import resource
|
||||
import secrets
|
||||
import select
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
|
||||
ROOT = Path("/var/lib/framebridge-expanded/credentials")
|
||||
SUDOERS = Path("/etc/sudoers.d")
|
||||
@@ -32,6 +34,7 @@ USER = re.compile(r"[A-Za-z_][A-Za-z0-9_-]{0,63}\$?\Z")
|
||||
ENV = {"PATH": "/usr/sbin:/usr/bin:/sbin:/bin", "LANG": "C", "LC_ALL": "C"}
|
||||
MAX_KEY = 16384
|
||||
MAX_CIPHER = 131072
|
||||
INPUT_TIMEOUT = 5
|
||||
|
||||
|
||||
def _fail():
|
||||
@@ -100,6 +103,24 @@ def _sync(directory):
|
||||
os.close(fd)
|
||||
|
||||
|
||||
def _read_key():
|
||||
# The frontend has already collected the key before launching this helper.
|
||||
# A caller holding stdin open must not retain the shared registry lock.
|
||||
deadline = time.monotonic() + INPUT_TIMEOUT
|
||||
fd = sys.stdin.fileno()
|
||||
value = bytearray()
|
||||
while True:
|
||||
remaining = deadline - time.monotonic()
|
||||
if remaining <= 0 or not select.select([fd], [], [], remaining)[0]:
|
||||
_fail()
|
||||
chunk = os.read(fd, MAX_KEY + 1 - len(value))
|
||||
if not chunk:
|
||||
return bytes(value)
|
||||
value.extend(chunk)
|
||||
if len(value) > MAX_KEY:
|
||||
_fail()
|
||||
|
||||
|
||||
def _prepare_root():
|
||||
# Only this fixed root-owned tree is created. No profile or HOME is opened.
|
||||
for directory in (ROOT.parent, ROOT):
|
||||
@@ -453,7 +474,7 @@ def main(argv=None):
|
||||
# sudo already selected this exact NOLOG policy. Consume its
|
||||
# encryption permission before accepting any key bytes.
|
||||
unauthorize(owner, args[1], "host", required=True)
|
||||
key = sys.stdin.buffer.read(MAX_KEY + 1)
|
||||
key = _read_key()
|
||||
result = protect(owner, args[1], key)
|
||||
key = b""
|
||||
elif operation == "decrypt":
|
||||
|
||||
@@ -216,12 +216,20 @@ class CredentialHelperTests(unittest.TestCase):
|
||||
def test_broken_receipt_pipe_rolls_back_published_protect_grant(self):
|
||||
helper.authorize(self.owner, PROFILE, "host")
|
||||
stdout = mock.Mock(); stdout.buffer.write.side_effect = BrokenPipeError("synthetic closed pipe")
|
||||
with mock.patch.object(helper, "_installation"), mock.patch.object(helper, "_caller", return_value=self.owner), mock.patch.object(helper, "_prepare_root"), mock.patch.object(helper.resource, "setrlimit"), mock.patch.object(helper.os, "umask"), mock.patch("sys.stdin", SimpleNamespace(buffer=io.BytesIO(KEY))), mock.patch("sys.stdout", stdout), mock.patch("sys.stderr", new_callable=io.StringIO) as stderr:
|
||||
with mock.patch.object(helper, "_installation"), mock.patch.object(helper, "_caller", return_value=self.owner), mock.patch.object(helper, "_prepare_root"), mock.patch.object(helper, "_read_key", return_value=KEY), mock.patch.object(helper.resource, "setrlimit"), mock.patch.object(helper.os, "umask"), mock.patch("sys.stdout", stdout), mock.patch("sys.stderr", new_callable=io.StringIO) as stderr:
|
||||
self.assertEqual(1, helper.main(["protect", PROFILE]))
|
||||
self.assertNotIn(KEY.decode(), stderr.getvalue())
|
||||
self.assertEqual([".lock"], [p.name for p in self.registry.iterdir()])
|
||||
self.assertEqual([], list(self.policies.iterdir()))
|
||||
|
||||
def test_open_stdin_has_a_deadline_and_excess_input_is_bounded(self):
|
||||
with mock.patch("sys.stdin", SimpleNamespace(fileno=lambda: 17)), mock.patch.object(helper.select, "select", return_value=([], [], [])), mock.patch.object(helper.os, "read") as read:
|
||||
with self.assertRaises(RuntimeError): helper._read_key()
|
||||
read.assert_not_called()
|
||||
with mock.patch("sys.stdin", SimpleNamespace(fileno=lambda: 17)), mock.patch.object(helper.select, "select", return_value=([17], [], [])), mock.patch.object(helper.os, "read", return_value=b"x" * (helper.MAX_KEY + 1)) as read:
|
||||
with self.assertRaises(RuntimeError): helper._read_key()
|
||||
read.assert_called_once_with(17, helper.MAX_KEY + 1)
|
||||
|
||||
def test_ids_reject_paths_flags_and_extra_arguments(self):
|
||||
for value in ("../secret", "--uid=0", "A" * 64, "a" * 63, PROFILE + "\n"):
|
||||
with self.subTest(value=value), self.assertRaises(RuntimeError):
|
||||
|
||||
@@ -27,6 +27,7 @@ spec.loader.exec_module(helper)
|
||||
helper.ROOT = Path(sys.argv[2])
|
||||
helper._installation = lambda: None
|
||||
helper._prepare_root = lambda: None
|
||||
helper._cleanup_temporaries = lambda: None
|
||||
helper._caller = lambda: SimpleNamespace(pw_uid=12345, pw_name="synthetic", pw_dir="/synthetic-only")
|
||||
helper.decrypt = lambda *args: b"X" * 16384
|
||||
# The only file is our own temporary lock; simulate its expected root metadata.
|
||||
@@ -37,7 +38,52 @@ raise SystemExit(helper.main(["decrypt", "a" * 64, "b" * 64]))
|
||||
"""
|
||||
|
||||
|
||||
PROTECT_CHILD = CHILD.replace(
|
||||
'raise SystemExit(helper.main(["decrypt", "a" * 64, "b" * 64]))',
|
||||
'\n'.join([
|
||||
'helper.INPUT_TIMEOUT = 0.1',
|
||||
'helper.unauthorize = lambda *args, **kwargs: None',
|
||||
'helper.protect = lambda *args: b"SYNTHETIC-CIPHER-ONLY" * 8',
|
||||
'raise SystemExit(helper.main(["protect", "a" * 64]))',
|
||||
]),
|
||||
)
|
||||
|
||||
|
||||
class CredentialHelperPipeTests(unittest.TestCase):
|
||||
def test_open_stdin_without_eof_times_out_and_releases_registry_lock(self):
|
||||
with tempfile.TemporaryDirectory(prefix="framebridge-input-canary-") as directory:
|
||||
child = None
|
||||
lock_fd = None
|
||||
try:
|
||||
started = time.monotonic()
|
||||
child = subprocess.Popen([sys.executable, "-I", "-S", "-c", PROTECT_CHILD,
|
||||
str(Path(credential_helper.__file__).resolve()), directory],
|
||||
stdin=subprocess.PIPE, stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE, close_fds=True)
|
||||
self.assertEqual(1, child.wait(timeout=1), "Protect must reject an input stream without EOF")
|
||||
self.assertLess(time.monotonic() - started, 1, "Synthetic input timeout exceeded its bound")
|
||||
# Do not close stdin or communicate(): both would supply EOF and
|
||||
# could hide an unbounded read or a leaked registry lock.
|
||||
self.assertFalse(child.stdin.closed)
|
||||
lock_fd = os.open(Path(directory) / ".lock", os.O_RDWR)
|
||||
try:
|
||||
fcntl.flock(lock_fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
except BlockingIOError:
|
||||
self.fail("Protect retained the registry lock after its input deadline")
|
||||
finally:
|
||||
fcntl.flock(lock_fd, fcntl.LOCK_UN)
|
||||
self.assertEqual(b"", child.stdout.read())
|
||||
self.assertIn(b"protected credential operation failed", child.stderr.read())
|
||||
finally:
|
||||
if child is not None:
|
||||
if child.poll() is None:
|
||||
child.kill()
|
||||
child.wait(timeout=3)
|
||||
for stream in (child.stdin, child.stdout, child.stderr):
|
||||
stream.close()
|
||||
if lock_fd is not None:
|
||||
os.close(lock_fd)
|
||||
|
||||
@unittest.skipUnless(hasattr(fcntl, "F_SETPIPE_SZ"), "Linux pipe-size controls required")
|
||||
def test_stalled_stdout_reader_does_not_hold_global_registry_lock(self):
|
||||
with tempfile.TemporaryDirectory(prefix="framebridge-pipe-canary-") as directory:
|
||||
|
||||
Reference in new issue
Block a user