diff --git a/DEVELOPER.md b/DEVELOPER.md index ea4720f..d2a0108 100644 --- a/DEVELOPER.md +++ b/DEVELOPER.md @@ -425,10 +425,10 @@ shadow entry and OpenSSH password policy with `debugfs`. Firecracker only sees the resulting private copy. Never change that to a shared writable rootfs. Guest images must provide `/etc/passwd`, `/etc/shadow`, OpenSSH's -`/etc/ssh/sshd_config`, the requested account, and their own first-boot SSH -host-key generation. UVM does not create missing users. The known public demo -key in Firecracker's bionic image is removed from private VM disks. A shared -template must never contain reusable SSH host private keys. +`/etc/ssh/sshd_config`, and the requested account. UVM does not create missing +users. It replaces conventional RSA, ECDSA, and Ed25519 host keys in each +private disk and removes the known public login key from Firecracker's bionic +image. A VM must never rely on reusable SSH private keys from a shared template. ### Integrity Model diff --git a/README.md b/README.md index fd3510b..7eaad26 100644 --- a/README.md +++ b/README.md @@ -197,11 +197,11 @@ defaults are username `root` and password `root`. A custom image must provide: - `sshd` - `/etc/passwd`, `/etc/shadow`, and `/etc/ssh/sshd_config` - The requested user account; UVM does not create missing users -- Unique SSH host keys generated during first boot The known public demo key bundled in Firecracker's default bionic image is -removed from each private disk. Never bake SSH host private keys into a rootfs -template shared by multiple VMs. +removed from each private disk. UVM also replaces the conventional RSA, ECDSA, +and Ed25519 SSH host keys in every private disk so cloned VMs have distinct +host identities. Never rely on SSH private keys baked into a shared template. ## Host Networking diff --git a/USAGE.md b/USAGE.md index ebb7f3d..d9b9792 100644 --- a/USAGE.md +++ b/USAGE.md @@ -306,7 +306,9 @@ UVM provisions a password for an existing account. The default credentials are - An SSH server. - The requested login user plus standard passwd and shadow files. -- Unique SSH host keys generated at first boot. + +UVM replaces conventional RSA, ECDSA, and Ed25519 SSH host keys in the private +disk during creation and removes Firecracker's publicly known demo login key. This is applied only while creating a new VM. Existing VM disks and legacy registry entries are not changed retroactively. @@ -747,7 +749,7 @@ Before relying on a VM for useful work: 1. Use strict checksums for installed artifacts. 2. Confirm `/dev/kvm` and nested virtualization are available. -3. Use a guest image with unique first-boot SSH host keys and set a non-default password. +3. Confirm UVM generated unique guest SSH host keys and set a non-default password. 4. Test guest egress and SSH on the actual host network. 5. Back up guest data before destroy operations. 6. Keep the API loopback-only unless you have a strong remote-access need. diff --git a/tests/test_images.py b/tests/test_images.py index a92dcb6..0bf8565 100644 --- a/tests/test_images.py +++ b/tests/test_images.py @@ -4,6 +4,7 @@ import hashlib import tempfile import unittest from pathlib import Path +from unittest.mock import patch from uvm.config import Settings from uvm.errors import UvmError @@ -21,12 +22,15 @@ from uvm.system import CommandResult class PasswordHashRunner: def __init__(self) -> None: self.command: tuple[str, ...] | None = None + self.calls: list[tuple[str, ...]] = [] self.options: dict[str, object] = {} def run(self, command, **options) -> CommandResult: self.command = tuple(str(part) for part in command) + self.calls.append(self.command) self.options = options - return CommandResult(self.command, 0, stdout="$6$salt$password-hash\n") + stdout = "$6$salt$password-hash\n" if self.command[0] == "openssl" else "" + return CommandResult(self.command, 0, stdout=stdout) class ImageStoreTests(unittest.TestCase): @@ -79,15 +83,27 @@ class ImageStoreTests(unittest.TestCase): def test_enables_root_password_login_before_existing_sshd_settings(self) -> None: updated = _enable_ssh_password_authentication( - "PasswordAuthentication no\nPermitRootLogin prohibit-password\n", + "HostKey /insecure/shared-key\n" + "PasswordAuthentication no\n" + "PermitRootLogin prohibit-password\n", "root", ) self.assertTrue( updated.startswith( - "# Managed by uvm\nPasswordAuthentication yes\nPermitRootLogin yes\n" + "# Managed by uvm\n" + "HostKey /etc/ssh/ssh_host_rsa_key\n" + "HostKey /etc/ssh/ssh_host_ecdsa_key\n" + "HostKey /etc/ssh/ssh_host_ed25519_key\n" + "PasswordAuthentication no\n" + "PermitRootLogin yes\n" + "Match User root\n" + " PasswordAuthentication yes\n" + "Match all\n" ) ) + self.assertNotIn("/insecure/shared-key", updated) + self.assertNotIn("PermitRootLogin prohibit-password", updated) def test_removes_only_the_public_firecracker_demo_key(self) -> None: own_key = "ssh-ed25519 AAAA-own-key developer@example" @@ -97,3 +113,40 @@ class ImageStoreTests(unittest.TestCase): ) self.assertEqual(updated, f"{own_key}\n") + + def test_generates_three_unique_guest_host_key_types(self) -> None: + runner = PasswordHashRunner() + store = ImageStore(Settings(), runner=runner) # type: ignore[arg-type] + with ( + tempfile.TemporaryDirectory() as temporary_directory, + patch.object(store, "_read_guest_file", return_value=None), + patch.object(store, "_write_guest_file") as write_guest_file, + ): + store._replace_guest_host_keys( + Path("/tmp/rootfs.ext4"), + Path(temporary_directory), + ) + + keygen_calls = [call for call in runner.calls if call[0] == "ssh-keygen"] + self.assertEqual([call[3] for call in keygen_calls], ["rsa", "ecdsa", "ed25519"]) + self.assertEqual(write_guest_file.call_count, 6) + + def test_optional_guest_file_inspection_fails_closed_on_debugfs_errors(self) -> None: + class FailedRunner: + @staticmethod + def run(_command, **_options) -> CommandResult: + return CommandResult( + args=("debugfs",), + returncode=0, + stderr="filesystem checksum failure", + ) + + store = ImageStore(Settings(), runner=FailedRunner()) # type: ignore[arg-type] + with tempfile.TemporaryDirectory() as temporary_directory: + with self.assertRaises(UvmError): + store._read_guest_file( + Path("/tmp/rootfs.ext4"), + "/root/.ssh/authorized_keys", + Path(temporary_directory) / "authorized_keys", + required=False, + ) diff --git a/tests/test_state.py b/tests/test_state.py index 3e0add2..dcb5cfe 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -88,3 +88,12 @@ class StateStoreTests(unittest.TestCase): self.store.initialize() self.assertEqual(stat.S_IMODE(self.settings.state_path.stat().st_mode), 0o600) + + def test_load_protects_registry_credentials_without_initialization(self) -> None: + self.base.mkdir(parents=True) + self.settings.state_path.write_text('{"vms": {}}', encoding="utf-8") + self.settings.state_path.chmod(0o644) + + self.store.load() + + self.assertEqual(stat.S_IMODE(self.settings.state_path.stat().st_mode), 0o600) diff --git a/uvm/images.py b/uvm/images.py index 190cd58..013bd24 100644 --- a/uvm/images.py +++ b/uvm/images.py @@ -24,6 +24,11 @@ _FIRECRACKER_DEMO_PUBLIC_KEY = ( ) _MODE_PATTERN = re.compile(r"Mode:\s+0*([0-7]+)") _OWNER_PATTERN = re.compile(r"User:\s+(\d+)\s+Group:\s+(\d+)") +_HOST_KEY_TYPES = ( + ("rsa", "3072"), + ("ecdsa", "256"), + ("ed25519", None), +) @dataclass(frozen=True, slots=True) @@ -144,6 +149,7 @@ class ImageStore: self._write_guest_file(disk, "/etc/shadow", shadow) self._write_guest_file(disk, "/etc/ssh/sshd_config", sshd_config) + self._replace_guest_host_keys(disk, temporary) if authorized_keys is not None: self._remove_firecracker_demo_key(disk, authorized_keys) except (OSError, UnicodeError) as error: @@ -179,9 +185,12 @@ class ImageStore: mode_match = _MODE_PATTERN.search(stat_result.stdout) owner_match = _OWNER_PATTERN.search(stat_result.stdout) if mode_match is None or owner_match is None: - if required: + output = f"{stat_result.stdout}\n{stat_result.stderr}".lower() + if not required and "file not found" in output: + return None + if required and "file not found" in output: raise UvmError(f"guest rootfs is missing required file: {guest_path}") - return None + raise UvmError(f"could not inspect guest rootfs file: {guest_path}") self._runner.run( ("debugfs", "-R", f"dump {guest_path} {destination}", disk), @@ -229,6 +238,43 @@ class ImageStore: ): raise UvmError(f"could not verify updated guest rootfs file: {guest_path}") + def _replace_guest_host_keys(self, disk: Path, temporary: Path) -> None: + for key_type, bits in _HOST_KEY_TYPES: + name = f"ssh_host_{key_type}_key" + private_path = temporary / name + command: list[str | Path] = [ + "ssh-keygen", + "-q", + "-t", + key_type, + "-N", + "", + "-C", + "", + "-f", + private_path, + ] + if bits is not None: + command[4:4] = ["-b", bits] + self._runner.run(command, capture=True) + + for suffix, default_mode in (("", 0o600), (".pub", 0o644)): + guest_path = f"/etc/ssh/{name}{suffix}" + existing = self._read_guest_file( + disk, + guest_path, + temporary / f"existing-{name}{suffix}", + required=False, + ) + generated = private_path.with_name(f"{name}{suffix}") + source = _GuestFile( + path=generated, + mode=existing.mode if existing is not None else default_mode, + uid=existing.uid if existing is not None else 0, + gid=existing.gid if existing is not None else 0, + ) + self._write_guest_file(disk, guest_path, source) + def _remove_firecracker_demo_key(self, disk: Path, authorized_keys: _GuestFile) -> None: contents = authorized_keys.path.read_text(encoding="utf-8") updated = _without_firecracker_demo_key(contents) @@ -269,10 +315,29 @@ def _set_shadow_password(contents: str, username: str, password_hash: str) -> st def _enable_ssh_password_authentication(contents: str, username: str) -> str: - directives = ["PasswordAuthentication yes"] + managed_directives = {"hostkey", "passwordauthentication", "permitrootlogin"} + unmanaged_lines = [] + for line in contents.splitlines(): + stripped = line.lstrip() + directive = stripped.split(None, 1)[0].lower() if stripped else "" + if stripped.startswith("#") or directive not in managed_directives: + unmanaged_lines.append(line) + + directives = [ + "HostKey /etc/ssh/ssh_host_rsa_key", + "HostKey /etc/ssh/ssh_host_ecdsa_key", + "HostKey /etc/ssh/ssh_host_ed25519_key", + "PasswordAuthentication no", + ] if username == "root": directives.append("PermitRootLogin yes") - return "# Managed by uvm\n" + "\n".join(directives) + "\n" + contents + return ( + "# Managed by uvm\n" + + "\n".join(directives) + + f"\nMatch User {username}\n PasswordAuthentication yes\nMatch all\n" + + "\n".join(unmanaged_lines).rstrip() + + "\n" + ) def _without_firecracker_demo_key(contents: str) -> str | None: diff --git a/uvm/state.py b/uvm/state.py index dce4e14..c1c342a 100644 --- a/uvm/state.py +++ b/uvm/state.py @@ -94,6 +94,8 @@ class StateStore: if not path.exists(): return State() try: + if path.stat().st_mode & 0o077: + os.chmod(path, 0o600) with path.open(encoding="utf-8") as state_file: raw = json.load(state_file) except (OSError, json.JSONDecodeError) as error: