diff --git a/ci/Jenkinsfile.hermes-voice-image b/ci/Jenkinsfile.hermes-voice-image index cad379eb..af30b55f 100644 --- a/ci/Jenkinsfile.hermes-voice-image +++ b/ci/Jenkinsfile.hermes-voice-image @@ -18,6 +18,18 @@ spec: fsGroupChangePolicy: OnRootMismatch nodeSelector: kubernetes.io/arch: arm64 + tolerations: + # Jetson kubelet/network maintenance can briefly outlast Kubernetes' five + # minute default. Keep the disposable build workspace intact long enough + # for the node to recover instead of restarting a large image expansion. + - key: node.kubernetes.io/not-ready + operator: Exists + effect: NoExecute + tolerationSeconds: 600 + - key: node.kubernetes.io/unreachable + operator: Exists + effect: NoExecute + tolerationSeconds: 600 affinity: nodeAffinity: requiredDuringSchedulingIgnoredDuringExecution: diff --git a/services/hermes/scripts/node_account_hardening.py b/services/hermes/scripts/node_account_hardening.py index 29af19a8..c729ddff 100644 --- a/services/hermes/scripts/node_account_hardening.py +++ b/services/hermes/scripts/node_account_hardening.py @@ -22,6 +22,7 @@ from node_account_io import ( assert_unchanged, atomic_write, backup_once, + open_private_temporary, read_mounted_secret, read_regular, ) @@ -247,7 +248,15 @@ def _directory(path: Path, *, mode: int, uid: int, gid: int) -> None: def _without_key(value: bytes, identity: tuple[str, bytes]) -> bytes: kept = [] for line in value.splitlines(keepends=True): - if _key_identity(line) != identity: + try: + line_identity = _key_identity(line) + except HardeningError: + # authorized_keys is shared with human operators on the legacy + # accounts. An unrelated line that this reconciler cannot prove + # valid is not authority to delete it or block all reconciliation. + kept.append(line) + continue + if line_identity != identity: kept.append(line) return b"".join(kept) @@ -408,9 +417,9 @@ def _acl_backup_once(path: Path, value: bytes, backup_name: str | None = None) - _validate_acl_backup(existing) return - temporary = root / f".{path.name}.acl.hermes-{os.getpid()}" - flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) - descriptor = os.open(temporary, flags, 0o600) + temporary, descriptor = open_private_temporary( + root / f"{path.name}.acl", 0o600 + ) try: if os.write(descriptor, encoded) != len(encoded): raise HardeningError("short sensitive directory ACL backup write") diff --git a/services/hermes/scripts/node_account_io.py b/services/hermes/scripts/node_account_io.py index 6421e648..8359326f 100644 --- a/services/hermes/scripts/node_account_io.py +++ b/services/hermes/scripts/node_account_io.py @@ -5,6 +5,7 @@ from __future__ import annotations import fcntl import os +import secrets import stat import time from contextlib import contextmanager @@ -17,6 +18,7 @@ MAX_XATTR_NAME = 255 MAX_XATTR_VALUE = 256 * 1024 MAX_XATTR_TOTAL = 1024 * 1024 LOCK_TIMEOUT_SECONDS = 15.0 +TEMPORARY_CREATE_ATTEMPTS = 8 class HardeningError(RuntimeError): @@ -141,11 +143,29 @@ def assert_unchanged(path: Path, expected: FileSnapshot, maximum: int) -> None: raise HardeningError(f"concurrent host account change detected: {path.name}") +def open_private_temporary(path: Path, mode: int) -> tuple[Path, int]: + """Create an unpredictable peer temporary without trusting stale files. + + Reconciler containers commonly reuse the same low PID after a restart. A + PID-only name can therefore collide forever with a file left by a killed + prior run. The exclusive random suffix keeps old artifacts inert while + preserving the same-directory atomic-replace boundary. + """ + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) + for _attempt in range(TEMPORARY_CREATE_ATTEMPTS): + temporary = path.with_name( + f".{path.name}.hermes-{os.getpid()}-{secrets.token_hex(16)}" + ) + try: + return temporary, os.open(temporary, flags, mode) + except FileExistsError: + continue + raise HardeningError(f"could not allocate private temporary: {path.name}") + + def atomic_write(path: Path, value: bytes, metadata: FileSnapshot) -> None: """Atomically replace a file while retaining ACLs, labels, and xattrs.""" - temporary = path.with_name(f".{path.name}.hermes-{os.getpid()}") - flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) - descriptor = os.open(temporary, flags, metadata.mode) + temporary, descriptor = open_private_temporary(path, metadata.mode) try: if os.write(descriptor, value) != len(value): raise HardeningError(f"short atomic write: {path.name}") diff --git a/testing/tests/test_hermes_node_account_io.py b/testing/tests/test_hermes_node_account_io.py index 674897e4..103d2da9 100644 --- a/testing/tests/test_hermes_node_account_io.py +++ b/testing/tests/test_hermes_node_account_io.py @@ -54,6 +54,22 @@ def test_unrelated_modern_human_key_type_is_preserved(tmp_path: Path, monkeypatc assert path.read_text() == modern + "\n" +def test_malformed_unrelated_authorized_key_lines_are_preserved( + tmp_path: Path, monkeypatch +): + module, _originals, key, _other, public_key = _fixture(tmp_path, monkeypatch) + malformed = b"ssh-ed25519 YWJj human-key-that-must-survive\noperator-note-\xff\n" + for user in module.LEGACY_ACCOUNTS: + path = module.HOST_HOME / user / ".ssh/authorized_keys" + path.write_bytes(malformed + key.encode("ascii") + b"\n") + + module.reconcile(public_key) + + for user in module.LEGACY_ACCOUNTS: + path = module.HOST_HOME / user / ".ssh/authorized_keys" + assert path.read_bytes() == malformed + + def test_crash_during_legacy_key_removal_never_installs_duplicate_key( tmp_path: Path, monkeypatch ): diff --git a/testing/tests/test_hermes_node_io_coverage.py b/testing/tests/test_hermes_node_io_coverage.py index 23131927..6c6949b8 100644 --- a/testing/tests/test_hermes_node_io_coverage.py +++ b/testing/tests/test_hermes_node_io_coverage.py @@ -188,6 +188,50 @@ def test_atomic_write_short_write_unlinks_temporary(tmp_path, monkeypatch): assert not list(tmp_path.glob(".passwd.hermes-*")) +def test_atomic_write_ignores_stale_pid_temporary(tmp_path): + module = _io_module() + target = tmp_path / "passwd" + target.write_bytes(b"old") + snapshot = module.read_regular(target, 16) + stale = tmp_path / f".passwd.hermes-{os.getpid()}" + stale.write_bytes(b"interrupted prior run") + + module.atomic_write(target, b"new", snapshot) + + assert target.read_bytes() == b"new" + assert stale.read_bytes() == b"interrupted prior run" + + +def test_private_temporary_retries_untrusted_name_collisions(tmp_path, monkeypatch): + module = _io_module() + target = tmp_path / "passwd" + tokens = iter(("collision", "unused")) + stale = tmp_path / f".passwd.hermes-{os.getpid()}-collision" + stale.write_bytes(b"untrusted") + monkeypatch.setattr(module.secrets, "token_hex", lambda _size: next(tokens)) + + temporary, descriptor = module.open_private_temporary(target, 0o600) + os.close(descriptor) + try: + assert temporary.name.endswith("-unused") + assert stale.read_bytes() == b"untrusted" + finally: + temporary.unlink(missing_ok=True) + + +def test_private_temporary_fails_closed_after_collision_limit(tmp_path, monkeypatch): + module = _io_module() + target = tmp_path / "passwd" + stale = tmp_path / f".passwd.hermes-{os.getpid()}-collision" + stale.write_bytes(b"untrusted") + monkeypatch.setattr(module.secrets, "token_hex", lambda _size: "collision") + + with pytest.raises(module.HardeningError, match="allocate private temporary"): + module.open_private_temporary(target, 0o600) + + assert stale.read_bytes() == b"untrusted" + + def test_backup_existing_is_validated_and_short_write_is_removed(tmp_path, monkeypatch): module = _io_module() source = tmp_path / "passwd" diff --git a/testing/tests/test_hermes_voice_release.py b/testing/tests/test_hermes_voice_release.py index 15035f0a..a0c8eb31 100644 --- a/testing/tests/test_hermes_voice_release.py +++ b/testing/tests/test_hermes_voice_release.py @@ -22,6 +22,9 @@ def test_voice_pipeline_binds_component_source_digest_and_release() -> None: assert "archiveArtifacts(" in pipeline assert '"SETFCAP"' in pipeline assert "values: [titan-20]" in pipeline + assert "key: node.kubernetes.io/not-ready" in pipeline + assert "key: node.kubernetes.io/unreachable" in pipeline + assert pipeline.count("tolerationSeconds: 600") == 2 assert 'node-role.kubernetes.io/worker: "true"' not in pipeline assert "ephemeral-storage: 10Gi" in pipeline assert "ephemeral-storage: 20Gi" in pipeline