Enhance security by ensuring unique SSH host keys for VMs and protecting registry credentials. Update documentation to reflect changes in SSH key management and improve state file permissions.
This commit is contained in:
+4
-4
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
+56
-3
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
+69
-4
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user