diff --git a/DEVELOPER.md b/DEVELOPER.md index d0b8124..69c8607 100644 --- a/DEVELOPER.md +++ b/DEVELOPER.md @@ -272,6 +272,9 @@ mode. There is no persisted boolean asserting that a policy is enforced. - Missing `disk` and process start time remain absent. Legacy `disk_ready` is inferred only for running/stopped records with a recorded private disk; start still validates the path, file, and provisioning state. +- Legacy records own their canonical runtime directory independently of whether + they had a private disk. Deletion never removes the shared rootfs template; + explicit `runtime_owned=False` on a new interrupted creation stays authoritative. - Records created before credential provisioning default to username `root` and password `null`; UVM does not apply a password retroactively. - `next_mac_index` is reconstructed from older locally administered MACs when @@ -384,7 +387,8 @@ merely because its PID was never persisted or its socket has not appeared. ### Policy and Bulk Operations -`set_ports()` persists the new allowlist with `operation=policy` before +`set_ports()` takes a frozen registry ID, not a reusable name/IP; the CLI resolves +the user's identifier before confirmation. It persists the new allowlist with `operation=policy` before applying it. Live updates apply atomically and retain conntrack; cleanly stopped edits are metadata-only. On apply/readback failure, preserve the new intent, attempt link-down/quarantine, and leave a failed pending record. Never diff --git a/README.md b/README.md index 34ed9bd..491a5da 100644 --- a/README.md +++ b/README.md @@ -308,6 +308,9 @@ forward rules for return traffic. Guest IPs, gateway, bridge name, and guest boot netmask derive from the configured CIDR. The existing `iptables` egress NAT/forwarding behavior is unchanged. Private +VM creation and start require an RFC 1918 subnet (`10.0.0.0/8`, +`172.16.0.0/12`, or `192.168.0.0/16`). Retained VMs prevent incompatible +gateway, subnet, or bridge changes within the same registry. Private port enforcement uses separate per-VM native nftables `bridge` tables, not DNAT or host listeners. Policy is installed and read back before an owned TAP is activated and before the guest starts. The software bridge must have STP @@ -325,6 +328,9 @@ There is no autostart service or daemon reconciliation. After externally lost or changed UVM rules, inspect policy status and explicitly run `ports sync`. Read-only commands do not repair rules; sync does not boot stopped VMs. +Use one authoritative registry per bridge/subnet. Independent `UVM_BASE` +directories do not coordinate IP or MAC allocation with each other. + ## Persistent State The default data directory is `/var/lib/uvm`: diff --git a/pyproject.toml b/pyproject.toml index e9d1c7b..0a5784c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,6 +9,7 @@ description = "Local Firecracker microVM command-line manager" requires-python = ">=3.12" dependencies = [ "fastapi>=0.115,<1", + "pydantic>=2,<3", "uvicorn>=0.30,<1", ] diff --git a/tests/test_firewall.py b/tests/test_firewall.py index 723c306..d84e5a3 100644 --- a/tests/test_firewall.py +++ b/tests/test_firewall.py @@ -442,6 +442,80 @@ class FirewallManagerTests(unittest.TestCase): with self.assertRaises(CommandError): self.manager.cleanup_connections(self.vm) + def test_legacy_cleanup_outside_current_network_uses_only_recorded_ip(self): + record = dict(id="vm-legacy", cpu=1, ram_mib=128, guest_ip="203.0.113.7", + gateway="203.0.113.1", tap="tap-legacy", mac=self.vm.mac, + socket="", config="", log="", status="stopped") + for stored_network in (None, "203.0.113.0/24"): + with self.subTest(network=stored_network): + vm = VmRecord.from_dict({**record, **({"network": stored_network} if stored_network else {})}) + self.assertIsNone(vm.ports) + self.runner.calls.clear() + self.manager.cleanup_connections(vm) + expected = [("ip", "-j", "-d", "link", "show")] + expected.extend(("conntrack", command, "--family", "ipv4", field, "203.0.113.7") + for field in ("--orig-src", "--orig-dst", "--reply-src", "--reply-dst") + for command in ("--delete", "--list")) + # No nft policy, publishing, or unrelated address/conntrack flush. + self.assertEqual([args for args, _ in self.runner.calls], expected) + + def test_cleanup_host_boundaries_use_stored_network_not_current_settings(self): + cases = ( + ("10.77.8.2", "10.77.0.0/16"), + ("100.64.3.4", "100.64.0.0/16"), + ("10.42.0.1", None), # Current gateway is not a cleanup constraint. + ("10.42.0.0", None), # Do not guess the old prefix from current settings. + ("10.42.0.255", None), + ("203.0.113.0", "203.0.0.0/16"), + ("203.0.113.255", "203.0.0.0/16"), + ("203.0.113.8", "203.0.113.8/31"), + ("203.0.113.9", "203.0.113.8/31"), + ("203.0.113.8", "203.0.113.8/32"), + ) + for address, network in cases: + with self.subTest(address=address, network=network): + self.runner.calls.clear() + self.manager.cleanup_connections(vm_record(guest_ip=address, network=network, ports=None)) + deletes = [args for args, _ in self.runner.calls if args[:2] == ("conntrack", "--delete")] + self.assertEqual(len(deletes), 4) + self.assertTrue(all(args[-1] == address for args in deletes)) + + def test_cleanup_rejects_invalid_or_non_host_persisted_addresses(self): + cases = [(address, None) for address in ( + "not-an-ip", "203.0.113.999", "203.0.113.7/24", "2001:db8::7", + "::ffff:203.0.113.7", None, True, 3405803783, + "0.0.0.0", "127.0.0.1", "127.12.2.3", "169.254.1.2", + "224.0.0.1", "239.255.255.255", "255.255.255.255", + )] + cases.extend(( + ("203.0.113.0", "203.0.113.0/24"), + ("203.0.113.255", "203.0.113.0/24"), + ("203.0.113.7", "203.0.114.0/24"), + ("203.0.113.7", "not-a-network"), + ("203.0.113.7", ""), + ("203.0.113.7", "2001:db8::/64"), + )) + for address, network in cases: + with self.subTest(address=address, network=network): + self.runner.calls.clear() + with self.assertRaises(ValidationError): + self.manager.cleanup_connections(vm_record(guest_ip=address, network=network, ports=None)) + self.assertEqual([args for args, _ in self.runner.calls], [("ip", "-j", "-d", "link", "show")]) + + def test_legacy_public_cleanup_still_requires_tap_absence(self): + self.runner.links = [{"ifname": "tap-legacy"}] + vm = vm_record(tap="tap-legacy", guest_ip="203.0.113.7", ports=None) + with self.assertRaisesRegex(UvmError, "still exists"): + self.manager.cleanup_connections(vm) + self.assertFalse(any(args[0] == "conntrack" for args, _ in self.runner.calls)) + + def test_cleanup_compatibility_does_not_allow_a_public_managed_policy(self): + vm = vm_record(guest_ip="203.0.113.7", network="203.0.113.0/24", ports=()) + with self.assertRaisesRegex(ValidationError, "private guest IP"): + self.manager.apply(vm) + self.assertEqual(self.runner.calls, []) + self.manager.cleanup_connections(vm) + def test_availability_checks_kernel_and_full_policy_without_committing(self): self.manager.check_available() self.assertEqual(self.runner.tables, {}) diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index b5b6eca..d3cd710 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -1,11 +1,12 @@ from __future__ import annotations +import json import tempfile import unittest from collections.abc import Callable, Iterable, Iterator from contextlib import contextmanager from dataclasses import replace -from ipaddress import IPv4Address +from ipaddress import IPv4Address, IPv4Network from pathlib import Path from unittest.mock import patch @@ -485,6 +486,81 @@ class LifecycleTests(LifecycleTestCase): self.provision.assert_not_called() self.copy_disk.assert_not_called() + def test_create_rejects_retained_topology_changes_before_allocation_or_bridge_work(self) -> None: + vm = self.create_vm() + self.copy_disk.reset_mock() + self.provision.reset_mock() + changes = ( + {"gateway": IPv4Address(vm.guest_ip)}, + {"gateway": IPv4Address("10.42.0.10")}, + {"network": IPv4Network("10.42.0.0/25")}, + {"bridge": "uvm-other"}, + {"gateway": IPv4Address(vm.guest_ip), "network": IPv4Network("10.42.0.0/25")}, + ) + for status in ("running", "stopped"): + if status == "stopped": + self.service.stop(vm.id) + before = self.settings.state_path.read_bytes() + before_directories = set(self.settings.vms_dir.iterdir()) + before_taps = self.network.taps.copy() + before_active = self.network.active_taps.copy() + before_tables = self.network.firewall.tables.copy() + for changed in changes: + with self.subTest(status=status, changed=changed): + settings = replace(self.settings, **changed) + service = LifecycleService( + settings, self.store, self.images, self.network, self.process, + client_factory=self.make_client, + ) + self.host.events.clear() + with ( + patch.object(self.network, "_settings", settings), + patch.object(self.network, "allocate_ip", wraps=self.network.allocate_ip) as allocate, + self.assertRaisesRegex(ConflictError, "topology conflicts with retained VMs"), + ): + service.create(VmSpec(cpu=1, ram_mib=512)) + allocate.assert_not_called() + self.assertEqual(self.settings.state_path.read_bytes(), before) + self.assertEqual(set(self.settings.vms_dir.iterdir()), before_directories) + self.assertEqual(self.network.taps, before_taps) + self.assertEqual(self.network.active_taps, before_active) + self.assertEqual(self.network.firewall.tables, before_tables) + self.assertEqual(self.host.events, [("firewall.check_available", None)]) + self.copy_disk.assert_not_called() + self.provision.assert_not_called() + self.assertEqual(len(self.process.started), 1) + self.assertEqual(Path(vm.disk).read_bytes(), b"rootfs") + + def test_non_rfc1918_network_fails_before_kvm_assets_or_registry_reservation(self) -> None: + for network, gateway in ( + ("203.0.113.0/24", "203.0.113.1"), + ("100.64.0.0/24", "100.64.0.1"), + ("8.8.8.0/24", "8.8.8.1"), + ("10.0.0.0/7", "10.0.0.1"), + ): + with self.subTest(network=network): + settings = replace(self.settings, network=IPv4Network(network), gateway=IPv4Address(gateway)) + service = LifecycleService( + settings, self.store, self.images, self.network, self.process, + client_factory=self.make_client, + ) + with ( + patch.object(self.network, "_settings", settings), + patch.object(self.images, "installed_assets", wraps=self.images.installed_assets) as assets, + patch.object(self.network, "allocate_ip", wraps=self.network.allocate_ip) as allocate, + self.assertRaisesRegex(ValidationError, "RFC 1918"), + ): + service.create(VmSpec(cpu=1, ram_mib=512)) + assets.assert_not_called() + allocate.assert_not_called() + self.check_kvm.assert_not_called() + self.assertFalse(self.settings.state_path.exists()) + self.assertEqual(self.store.load().vms, {}) + self.assertEqual(list(self.settings.vms_dir.iterdir()), []) + self.assertEqual(self.host.events, []) + self.provision.assert_not_called() + self.copy_disk.assert_not_called() + def test_create_cleans_up_if_policy_readback_is_not_enforced_after_boot(self) -> None: with patch.object(self.network.firewall, "status", return_value="drift"): with self.assertRaisesRegex(UvmError, "enforcement could not be confirmed after boot"): @@ -605,10 +681,54 @@ class LifecycleTests(LifecycleTestCase): self.copy_disk.assert_not_called() self.assertEqual(self.network.created_taps, []) self.assertEqual(self.process.started, []) - retained = self.store.load().vms.get("vm-existing") - if retained is not None: - self.assertFalse(retained.runtime_owned) - self.assertFalse(retained.disk_ready) + retained = self.store.get("vm-existing") + self.assertFalse(retained.runtime_owned) + self.assertFalse(retained.disk_ready) + with self.assertRaisesRegex(UvmError, "refusing to remove unowned VM directory"): + self.service.destroy(retained.id) + self.assertFalse(self.store.get(retained.id).runtime_owned) + self.assertEqual({path.name: (path.stat().st_ino, path.read_bytes()) for path in directory.iterdir()}, before) + + def test_delete_legacy_runtime_without_private_disk_preserves_shared_rootfs(self) -> None: + shared_rootfs = self.settings.rootfs_image + before = (shared_rootfs.stat().st_ino, shared_rootfs.read_bytes()) + for disk_field in ({}, {"disk": ""}): + with self.subTest(disk_field=disk_field): + directory = self.settings.vm_dir("vm-legacy") + directory.mkdir(parents=True, mode=0o700) + raw = { + "id": "vm-legacy", "cpu": 1, "ram_mib": 512, + "guest_ip": "10.42.0.2", "gateway": "10.42.0.1", + "tap": self.network.tap_name("vm-legacy"), "mac": self.network.mac_for(1), + "socket": str(directory / "firecracker.sock"), + "config": str(directory / "config.json"), + "log": str(directory / "firecracker.log"), + "status": "running", "pid": 12345, "process_start_time": "42", + **disk_field, + } + Path(raw["socket"]).touch() + Path(raw["log"]).write_bytes(b"legacy boot log") + Path(raw["config"]).write_text(json.dumps({ + "drives": [{"path_on_host": str(shared_rootfs), "is_read_only": False}], + }), encoding="utf-8") + self.settings.state_path.write_text(json.dumps({ + "schema_version": 1, "vms": {raw["id"]: raw}, + }), encoding="utf-8") + self.settings.state_path.chmod(0o600) + vm = self.store.get(raw["id"]) + self.assertTrue(vm.runtime_owned) + self.assertEqual(vm.disk, "") + self.assertFalse(vm.disk_ready) + self.assertEqual(self.process.observe(vm), "gone") + + self.service.destroy(vm.id) + + self.assertEqual(self.store.load().vms, {}) + self.assertFalse(directory.exists()) + self.assertEqual((shared_rootfs.stat().st_ino, shared_rootfs.read_bytes()), before) + self.copy_disk.assert_not_called() + self.provision.assert_not_called() + self.check_kvm.assert_not_called() def test_stale_stopping_and_deleting_records_can_finish_cleanup(self) -> None: for action, status, operation in (("stop", "stopping", "stop"), ("destroy", "terminating", "delete")): diff --git a/tests/test_lifecycle_commands.py b/tests/test_lifecycle_commands.py index e5eed4a..f7b6cde 100644 --- a/tests/test_lifecycle_commands.py +++ b/tests/test_lifecycle_commands.py @@ -43,7 +43,7 @@ class LifecycleLookupTests(LifecycleTestCase): before = self.settings.state_path.read_bytes() for kwargs, message in ( ({"name": vm.name}, "VM name already exists"), - ({"name": vm.id}, "VM name already exists"), + ({"name": vm.id}, "must not impersonate generated VM IDs"), ({"guest_ip": IPv4Address(vm.guest_ip)}, "IP already allocated"), ): with self.subTest(kwargs=kwargs), self.assertRaisesRegex(UvmError, message): @@ -53,6 +53,16 @@ class LifecycleLookupTests(LifecycleTestCase): self.assertEqual(self.copy_disk.call_count, 1) self.assertEqual(len(self.process.started), 1) + def test_generated_id_names_are_rejected_before_state_or_host_work(self) -> None: + with self.assertRaisesRegex(ValidationError, "must not impersonate generated VM IDs"): + self.create_vm(name="vm-0123456789abcdef0123456789abcdef") + self.assertFalse(self.settings.state_path.exists()) + self.assertEqual(self.store.operation_locks, 0) + self.assertEqual(self.host.events, []) + self.check_kvm.assert_not_called() + self.provision.assert_not_called() + self.copy_disk.assert_not_called() + def test_generated_id_retries_id_name_and_tap_collisions(self) -> None: with patch("uvm.lifecycle.new_vm_id", return_value="vm-first-same1234567"): first = self.create_vm(name="reserved-name") @@ -68,8 +78,9 @@ class LifecycleLookupTests(LifecycleTestCase): self.assertNotEqual(second.guest_ip, first.guest_ip) self.assertEqual(self.network.taps, {first.tap: first.id, second.tap: second.id}) - def test_ambiguous_persisted_name_ip_or_id_never_selects_an_arbitrary_vm(self) -> None: - first = self.create_vm(name="first") + def test_ambiguous_persisted_name_ip_or_legacy_id_never_selects_an_arbitrary_vm(self) -> None: + with patch("uvm.lifecycle.new_vm_id", return_value="vm-legacy"): + first = self.create_vm(name="first") second = self.create_vm(name="second") for identifier in (first.guest_ip, first.id, first.name): with self.subTest(identifier=identifier): @@ -85,6 +96,34 @@ class LifecycleLookupTests(LifecycleTestCase): self.assertEqual(self.settings.state_path.read_bytes(), before) self.assertEqual(self.host.events, []) + def test_exact_generated_id_wins_over_a_malformed_legacy_name(self) -> None: + first = self.create_vm(name="first") + second = self.create_vm(name="second") + self.persist(second, name=first.id) + before = self.settings.state_path.read_bytes() + second_record = self.store.get(second.id).to_dict() + self.host.events.clear() + self.assertEqual(self.service.inspect(first.id).vm.id, first.id) + self.assertEqual(self.service.find_for_ssh(first.id).id, first.id) + self.assertEqual(self.service.log_path(first.id), Path(first.log)) + self.assertEqual([vm.id for vm in self.service.resolve_targets([first.id])], [first.id]) + self.assertEqual(self.settings.state_path.read_bytes(), before) + self.assertEqual(self.service.stop(first.id).id, first.id) + self.service.destroy(first.id) + self.assertEqual(self.store.get(second.id).to_dict(), second_record) + self.assertEqual(self.host.actions(second.id), []) + + before = self.settings.state_path.read_bytes() + self.host.events.clear() + for method in (self.service.inspect, self.service.find_for_ssh, + self.service.log_path, self.service.stop): + with self.assertRaises(NotFoundError): + method(first.id) + with self.assertRaises(NotFoundError): + self.service.resolve_targets([first.id]) + self.assertEqual(self.settings.state_path.read_bytes(), before) + self.assertEqual(self.host.events, []) + def test_target_resolution_rejects_invalid_or_missing_targets_without_mutation(self) -> None: vm = self.create_vm(name="web") before = self.settings.state_path.read_bytes() @@ -102,6 +141,36 @@ class LifecycleLookupTests(LifecycleTestCase): class LifecyclePortTests(LifecycleTestCase): + def test_set_ports_requires_an_exact_registry_id_not_a_name_or_ip(self) -> None: + vm = self.create_vm(name="web") + before = self.settings.state_path.read_bytes() + self.host.events.clear() + for identifier in (vm.name, vm.guest_ip): + with self.subTest(identifier=identifier), self.assertRaises(NotFoundError): + self.service.set_ports(identifier, ()) + self.assertEqual(self.settings.state_path.read_bytes(), before) + self.assertEqual(self.host.events, []) + self.assertEqual(self.service.set_ports(vm.id, ()).ports, ()) + + def test_frozen_legacy_id_cannot_retarget_policy_after_name_reuse(self) -> None: + with patch("uvm.lifecycle.new_vm_id", return_value="vm-legacy"): + original = self.create_vm(name="original") + selected = self.service.inspect(original.name).vm.id + self.service.destroy(original.id) + replacement = self.create_vm(name=original.id, ports=(PortRule(22), PortRule(80))) + self.assertEqual(self.service.inspect(selected).vm.id, replacement.id) + before = self.settings.state_path.read_bytes() + before_tables = self.network.firewall.tables.copy() + self.host.events.clear() + for mode, ports in (("open", (PortRule(443),)), ("close", (PortRule(80),)), ("set", ())): + with self.subTest(mode=mode), self.assertRaises(NotFoundError): + self.service.set_ports(selected, ports, mode=mode) + self.assertEqual(self.settings.state_path.read_bytes(), before) + self.assertEqual(self.network.firewall.tables, before_tables) + self.assertEqual(self.host.events, []) + self.assertIn(replacement.tap, self.network.active_taps) + self.assertEqual(self.process.observe(replacement), "alive") + def test_stopped_port_edits_and_sync_are_metadata_only(self) -> None: vm = self.create_vm() stopped = self.service.stop(vm.id) @@ -195,8 +264,61 @@ class LifecyclePortTests(LifecycleTestCase): pending = self.store.get(vm.id) self.assertEqual(pending.ports, (PortRule(22),)) self.assertEqual(pending.operation, "policy") + self.assertIn("link-down failed", pending.last_error) + self.assertTrue(self.network.has_tap(pending)) + self.assertIn(vm.tap, self.network.active_taps) self.assertEqual(self.network.firewall.status(pending), "quarantined") - self.assertIn("firewall.quarantine", self.host.actions(vm.id)) + self.assert_call_order(vm, "firewall.apply", "network.deactivate_tap", "firewall.quarantine") + + def test_policy_failure_never_quarantines_a_missing_or_foreign_tap(self) -> None: + for owner in (None, "operator-owned"): + with self.subTest(owner=owner): + vm = self.create_vm(ports=(PortRule(22), PortRule(80))) + if owner is None: + del self.network.taps[vm.tap] + self.network.active_taps.discard(vm.tap) + else: + self.network.taps[vm.tap] = owner + with self.assertRaisesRegex(ConflictError, "unowned TAP"): + self.network.has_tap(vm) + before_taps = self.network.taps.copy() + before_active = self.network.active_taps.copy() + before_tables = self.network.firewall.tables.copy() + self.host.events.clear() + with self.assertRaisesRegex(UvmError, "policy pending"): + self.service.set_ports(vm.id, (PortRule(80),), mode="close") + pending = self.store.get(vm.id) + self.assertEqual((pending.ports, pending.operation), ((PortRule(22),), "policy")) + self.assertEqual(pending.pid, vm.pid) + self.assertEqual(self.network.taps, before_taps) + self.assertEqual(self.network.active_taps, before_active) + self.assertEqual(self.network.firewall.tables, before_tables) + self.assertFalse(set(self.host.actions(vm.id)) & { + "firewall.apply", "firewall.quarantine", "firewall.remove", + "firewall.cleanup_connections", "network.delete_vm_tap", "process.terminate", + }) + + def test_policy_fallback_rechecks_ownership_after_a_tap_is_replaced(self) -> None: + vm = self.create_vm(ports=(PortRule(22), PortRule(80))) + before_tables = self.network.firewall.tables.copy() + apply = self.network.firewall.apply + + def replace_tap_and_fail(record, *, quarantine=False): + if not quarantine: + self.network.taps[record.tap] = "operator-owned" + self.host.failures["firewall.apply", record.id] = UvmError("policy rejected") + return apply(record, quarantine=quarantine) + + self.host.events.clear() + with patch.object(self.network.firewall, "apply", side_effect=replace_tap_and_fail) as applied: + with self.assertRaisesRegex(UvmError, "policy pending.*unowned TAP"): + self.service.set_ports(vm.id, (PortRule(80),), mode="close") + self.assertEqual(applied.call_count, 1) + self.assertEqual(self.network.taps[vm.tap], "operator-owned") + self.assertIn(vm.tap, self.network.active_taps) + self.assertEqual(self.network.firewall.tables, before_tables) + self.assertNotIn("firewall.quarantine", self.host.actions(vm.id)) + self.assertEqual(self.store.get(vm.id).ports, (PortRule(22),)) def test_failed_close_keeps_tap_down_if_quarantine_also_fails(self) -> None: vm = self.create_vm(ports=(PortRule(22), PortRule(80))) @@ -348,10 +470,12 @@ class LifecycleBulkTests(LifecycleTestCase): self.assertEqual(self.store.get(retained.id).status, "stopped") def test_bulk_exact_ids_do_not_retarget_a_new_vm_named_after_a_deleted_id(self) -> None: - original = self.create_vm(name="original") + with patch("uvm.lifecycle.new_vm_id", return_value="vm-legacy"): + original = self.create_vm(name="original") selected = tuple(vm.id for vm in self.service.resolve_targets([original.name])) self.service.destroy(original.id) replacement = self.create_vm(name=original.id) + self.assertEqual(self.service.inspect(original.id).vm.id, replacement.id) before = self.store.get(replacement.id).to_dict() self.host.events.clear() results = self.service.run_many("delete", selected) diff --git a/uvm/firewall.py b/uvm/firewall.py index 7dca5fc..ba32df1 100644 --- a/uvm/firewall.py +++ b/uvm/firewall.py @@ -89,6 +89,8 @@ class FirewallManager: current = self._read_table(table["name"]) if current is not None: self._transaction([self._delete_owned(current, table)]) + if self._read_table(table["name"]) is not None: + raise UvmError("firewall table remains or was replaced after removal; keep the VM reservation") def status(self, vm: VmRecord) -> str: """Inspect actual hooks, ordered rules, identity and complete port set. @@ -137,9 +139,26 @@ class FirewallManager: All four tuple addresses are checked, including NAT reply tuples. A failed deletion is tolerated only if a successful list proves absence. The caller must retain the address reservation until this succeeds. + Legacy non-RFC1918 addresses are allowed here; only the recorded subnet, + when present, determines host-address boundaries, never current settings. """ self._require_offline(vm) - address = self._guest_ip(vm) + try: + address = IPv4Address(str(vm.guest_ip)) + stored_network = getattr(vm, "network", None) + network = IPv4Network(str(stored_network)) if stored_network is not None else None + except (ValueError, TypeError) as error: + raise ValidationError("conntrack cleanup requires a valid persisted IPv4 address/network") from error + if (address.is_unspecified or address.is_loopback or address.is_link_local + or address.is_multicast or address == IPv4Address("255.255.255.255")): + raise ValidationError("conntrack cleanup requires a usable persisted unicast IPv4 address") + # /31 point-to-point subnets and /32 host routes have no broadcast address. + if network is not None and ( + address not in network or (network.prefixlen < 31 + and address in {network.network_address, network.broadcast_address}) + ): + raise ValidationError("conntrack cleanup requires a usable guest IP inside its persisted network") + address = str(address) for field in ("--orig-src", "--orig-dst", "--reply-src", "--reply-dst"): selector = ("--family", "ipv4", field, address) self._run(("conntrack", "--delete", *selector), check=False) diff --git a/uvm/lifecycle.py b/uvm/lifecycle.py index 4dd7484..b21ce0e 100644 --- a/uvm/lifecycle.py +++ b/uvm/lifecycle.py @@ -104,6 +104,10 @@ class LifecycleService: for network in ("10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16")): raise ValidationError("private ports require a network inside an RFC 1918 IPv4 range") check_kvm() + self._verify_firecracker() + self._network.firewall.check_available() + + def _verify_firecracker(self) -> None: manifest = load_manifest(self._settings.integrity_manifest_path) checksum = self._settings.firecracker_binary_sha256 if manifest.verified or self._settings.allow_unverified_downloads: @@ -111,7 +115,6 @@ class LifecycleService: verify_file(self._settings.firecracker_binary, checksum, "Firecracker binary", "UVM_FIRECRACKER_BINARY_SHA256", allow_unverified=self._settings.allow_unverified_downloads) - self._network.firewall.check_available() def _boot(self, vm: VmRecord, kernel: Path) -> None: vm.status, vm.operation = "starting", "network" @@ -414,6 +417,7 @@ class LifecycleService: def diagnostics(self) -> list[dict]: checks = [("root", require_root), ("kvm", check_kvm), + ("Firecracker", self._verify_firecracker), ("guest assets", self._images.installed_assets), ("private firewall", self._network.firewall.check_available)] results = [] diff --git a/uvm/network.py b/uvm/network.py index 4a9e393..3d779d5 100644 --- a/uvm/network.py +++ b/uvm/network.py @@ -9,7 +9,7 @@ from ipaddress import IPv4Address from .config import Settings from .domain import VmRecord -from .errors import ConflictError, TapCreationError, UvmError, ValidationError +from .errors import ConflictError, UvmError, ValidationError from .firewall import FirewallManager from .system import CommandRunner @@ -166,27 +166,6 @@ class NetworkManager: raise ConflictError(f"refusing to modify unowned TAP {vm.tap}") return link - def create_tap(self, name: str) -> None: - existing = self._runner.run(("ip", "link", "show", name), check=False, capture=True) - if existing.returncode == 0: - raise UvmError(f"TAP device already exists: {name}") - created = False - try: - self._runner.run(("ip", "tuntap", "add", "dev", name, "mode", "tap")) - created = True - self._runner.run(("ip", "link", "set", name, "master", self._settings.bridge)) - self._runner.run(("ip", "link", "set", name, "up")) - except BaseException as error: - if created: - try: - self.delete_tap(name) - except UvmError as cleanup_error: - raise TapCreationError( - f"could not configure TAP device {name}; cleanup also failed: {cleanup_error}", - tap_created=True, - ) from error - raise - def delete_tap(self, name: str) -> None: deleted = self._runner.run(("ip", "link", "del", name), check=False, capture=True) if deleted.returncode == 0: