fix(firecracker): harden committed-snapshot resume against guest-controlled data
Address the codex review on #398: - P1: inject_guest_boot no longer follows a symlink at bb-init/bb-dropbear. A committed snapshot is guest-controlled and could plant those paths as symlinks aimed at a host file (e.g. bb-init -> ~/.bashrc); write_text / copy2 would then overwrite the target as the host user during resume. Replace any pre-existing entry and create the files with O_EXCL|O_NOFOLLOW so the write stays inside the staging tree. - P2: write the snapshot tar owner-only (0600). It can contain the bottle's private workspace; it was being created world-readable (0644). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit is contained in:
@@ -7,6 +7,7 @@ from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import stat
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
@@ -218,5 +219,59 @@ class TestBuildCommittedRootfsDir(unittest.TestCase):
|
||||
self.assertEqual(2, calls["n"])
|
||||
|
||||
|
||||
class TestInjectGuestBootSymlinkSafe(unittest.TestCase):
|
||||
"""A committed snapshot is guest-controlled: inject_guest_boot must not
|
||||
follow a planted symlink and overwrite a host file during resume."""
|
||||
|
||||
def test_planted_symlink_does_not_escape_staging_tree(self):
|
||||
with tempfile.TemporaryDirectory(prefix="fc-inject.") as d:
|
||||
tmp = Path(d)
|
||||
dropbear = tmp / "dropbear"
|
||||
dropbear.write_bytes(b"DROPBEAR")
|
||||
# A host file the malicious snapshot tries to clobber.
|
||||
victim = tmp / "victim"
|
||||
victim.write_text("original")
|
||||
|
||||
rootfs = tmp / "rootfs"
|
||||
rootfs.mkdir()
|
||||
# The snapshot planted bb-init/bb-dropbear as symlinks to it.
|
||||
(rootfs / "bb-init").symlink_to(victim)
|
||||
(rootfs / "bb-dropbear").symlink_to(victim)
|
||||
|
||||
with patch.object(util, "dropbear_path", return_value=dropbear):
|
||||
util.inject_guest_boot(rootfs, init_script="#!/bin/sh\nreal\n")
|
||||
|
||||
# Host file untouched; the staged paths are fresh regular files.
|
||||
self.assertEqual("original", victim.read_text())
|
||||
self.assertFalse((rootfs / "bb-init").is_symlink())
|
||||
self.assertFalse((rootfs / "bb-dropbear").is_symlink())
|
||||
self.assertEqual("#!/bin/sh\nreal\n", (rootfs / "bb-init").read_text())
|
||||
self.assertEqual(b"DROPBEAR", (rootfs / "bb-dropbear").read_bytes())
|
||||
|
||||
|
||||
class TestCommitRootfsPermissions(unittest.TestCase):
|
||||
"""The snapshot tar can hold the bottle's private workspace, so the
|
||||
freezer must write it owner-only (0600)."""
|
||||
|
||||
def test_snapshot_created_owner_only(self):
|
||||
with tempfile.TemporaryDirectory(prefix="fc-freeze.") as d:
|
||||
tmp = Path(d)
|
||||
key = tmp / "key"
|
||||
key.write_text("K")
|
||||
tar_path = tmp / "state" / "committed-rootfs.tar"
|
||||
|
||||
def fake_run(argv: list[str], *a: Any, **k: Any) -> Any:
|
||||
# Emulate the ssh|tar pipe streaming the snapshot to stdout.
|
||||
k["stdout"].write(b"TARDATA")
|
||||
return subprocess.CompletedProcess(argv, 0, b"", b"")
|
||||
|
||||
with patch.object(freezer.util, "ssh_base_argv", return_value=["ssh"]), \
|
||||
patch.object(freezer.subprocess, "run", side_effect=fake_run):
|
||||
freezer._commit_rootfs_via_ssh(key, "10.0.0.1", tar_path)
|
||||
|
||||
self.assertEqual(b"TARDATA", tar_path.read_bytes())
|
||||
self.assertEqual(0o600, stat.S_IMODE(tar_path.stat().st_mode))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user