diff --git a/source/docs/CREDENTIAL-BACKENDS.md b/source/docs/CREDENTIAL-BACKENDS.md index e17d69d..e685a78 100644 --- a/source/docs/CREDENTIAL-BACKENDS.md +++ b/source/docs/CREDENTIAL-BACKENDS.md @@ -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 diff --git a/source/expanded/framebridge_expanded/credential_helper.py b/source/expanded/framebridge_expanded/credential_helper.py index c7f583a..1566f93 100644 --- a/source/expanded/framebridge_expanded/credential_helper.py +++ b/source/expanded/framebridge_expanded/credential_helper.py @@ -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": diff --git a/source/tests/test_credential_helper.py b/source/tests/test_credential_helper.py index 04a36bd..261fe7f 100644 --- a/source/tests/test_credential_helper.py +++ b/source/tests/test_credential_helper.py @@ -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): diff --git a/source/tests/test_credential_helper_pipe.py b/source/tests/test_credential_helper_pipe.py index 82829d3..9954d27 100644 --- a/source/tests/test_credential_helper_pipe.py +++ b/source/tests/test_credential_helper_pipe.py @@ -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: