diff --git a/usr/lib/python3/dist-packages/linuxmusterTools/linbo/image_sync.py b/usr/lib/python3/dist-packages/linuxmusterTools/linbo/image_sync.py index ac90ad8..4e27e52 100644 --- a/usr/lib/python3/dist-packages/linuxmusterTools/linbo/image_sync.py +++ b/usr/lib/python3/dist-packages/linuxmusterTools/linbo/image_sync.py @@ -29,6 +29,89 @@ IMAGE_EXTS = {".qcow2", ".qdiff", ".cloop"} INCOMING_DIR_NAME = ".incoming" +# The .macct file of an image holds the Samba secrets (unicodePwd, +# supplementalCredentials) of the client the image was taken from. The +# [linbo] rsync module serves the whole LINBO directory without login, as +# nobody, so this file has to stay root:root 0600, like linuxmuster-linbo7 +# keeps it (rsync-post-upload.sh, linbo-torrent). The staging directory lies +# in that module too, and is closed as a whole while an upload waits there. +MACCT_EXT = ".macct" +MACCT_MODE = 0o600 +STAGING_DIR_MODE = 0o700 + + +def _restrict(path: Path, mode: int) -> None: + """Set the mode of a path, and make it root's when running as root.""" + + + os.chmod(path, mode) + if os.geteuid() == 0: + os.chown(path, 0, 0) + + +def _staging_dir(images_dir: Path, image_name: str) -> Path: + """Create the staging directory of an image, closed to other users. + + Both .incoming and its image subdirectory are set on every call, so a + directory created before, or opened again by a recursive chown, is + closed again. + """ + + + incoming_dir = images_dir / INCOMING_DIR_NAME + staging_dir = incoming_dir / image_name + staging_dir.mkdir(mode=STAGING_DIR_MODE, parents=True, exist_ok=True) + for directory in (incoming_dir, staging_dir): + _restrict(directory, STAGING_DIR_MODE) + return staging_dir + + +def _open_for_write(file_path: Path, flags: int): + """Open a file for writing, a .macct one created and kept 0600. + + The mode is given to os.open() so a new .macct file never exists with a + wider one, then set again on the descriptor for a file that already + existed, or a umask that took the owner bits away. + """ + + + flags |= os.O_WRONLY + if file_path.suffix != MACCT_EXT: + return os.fdopen(os.open(file_path, flags, 0o666), "wb") + + fd = os.open(file_path, flags, MACCT_MODE) + try: + os.fchmod(fd, MACCT_MODE) + if os.geteuid() == 0: + os.fchown(fd, 0, 0) + except OSError: + os.close(fd) + raise + return os.fdopen(fd, "wb") + + +def _copy_file(source: str | Path, target: str | Path) -> None: + """Copy a file like shutil.copy2(), a .macct one without widening its mode. + + shutil.copy2() creates the copy with the umask default and copies the + mode of the source afterwards: a .macct would be readable while it is + written, and stay so if the source was. It is copied into a file opened + 0600 instead, keeping the times like copy2() does. Used for the backup, + and by shutil.move() when the staged file lies on another filesystem. + """ + + + source, target = Path(source), Path(target) + if source.suffix != MACCT_EXT: + shutil.copy2(str(source), str(target)) + return + + with open(source, "rb") as src, \ + _open_for_write(target, os.O_CREAT | os.O_TRUNC) as dst: + shutil.copyfileobj(src, dst) + stat = source.stat() + os.utime(target, ns=(stat.st_atime_ns, stat.st_mtime_ns)) + def resolve_image_file(images_dir: Path, image_name: str, filename: str) -> Path: """Resolve and validate an image file path. @@ -89,8 +172,7 @@ def receive_upload_chunk( image_name = name_checker.validate_linbo_image_name(image_name) filename = name_checker.validate_linbo_image_name(filename) - staging_dir = images_dir / INCOMING_DIR_NAME / image_name - staging_dir.mkdir(parents=True, exist_ok=True) + staging_dir = _staging_dir(images_dir, image_name) file_path = staging_dir / filename if offset is not None and offset > 0: @@ -99,11 +181,12 @@ def receive_upload_chunk( current_size = file_path.stat().st_size if current_size != offset: raise ValueError(f"Offset mismatch: expected {current_size}, got {offset}") - with open(file_path, "r+b") as f: + with _open_for_write(file_path, 0) as f: f.seek(offset) f.write(data) else: - file_path.write_bytes(data) + with _open_for_write(file_path, os.O_CREAT | os.O_TRUNC) as f: + f.write(data) return {"received": len(data), "offset": (offset or 0) + len(data)} @@ -158,7 +241,7 @@ def finalize_upload(images_dir: Path, image_name: str) -> dict: for f in target_dir.iterdir(): if f.is_file(): try: - shutil.copy2(str(f), str(backup_dir / f.name)) + _copy_file(f, backup_dir / f.name) except OSError as e: logger.warning("Backup failed for %s: %s", f.name, e) @@ -169,9 +252,18 @@ def finalize_upload(images_dir: Path, image_name: str) -> dict: for f in staging_dir.iterdir(): if f.is_file(): target = target_dir / f.name - shutil.move(str(f), str(target)) + # Before the move, which keeps the mode: a file staged before + # the staging directory was closed can still be wider. + if f.suffix == MACCT_EXT: + _restrict(f, MACCT_MODE) + shutil.move(str(f), str(target), copy_function=_copy_file) moved.append(f.name) + # A .macct left from an earlier upload or an older version is closed too. + for f in target_dir.iterdir(): + if f.is_file() and f.suffix == MACCT_EXT: + _restrict(f, MACCT_MODE) + try: staging_dir.rmdir() except OSError: diff --git a/usr/lib/python3/dist-packages/linuxmusterTools/linbo/pytests/test_image_sync_permissions.py b/usr/lib/python3/dist-packages/linuxmusterTools/linbo/pytests/test_image_sync_permissions.py new file mode 100644 index 0000000..bb22bf2 --- /dev/null +++ b/usr/lib/python3/dist-packages/linuxmusterTools/linbo/pytests/test_image_sync_permissions.py @@ -0,0 +1,234 @@ +""" +Tests for the permissions of the files the image serving helpers write. + +The .macct file of an image holds the Samba machine account secrets of the +client it was taken from, and the [linbo] rsync module serves the whole +LINBO directory without login as nobody. Every .macct the helpers stage, +move into place or back up has to be 0600, the staging directory closed +while an upload waits there, and the other image files left as they are. +""" + +import errno +import os +import stat + +import pytest + +import linuxmusterTools.linbo.image_sync as image_sync_module +from linuxmusterTools.linbo.image_sync import ( + finalize_upload, + receive_upload_chunk, +) + + +IMAGE = "ubuntu22" +MACCT = "ubuntu22.qcow2.macct" +QCOW2 = "ubuntu22.qcow2" + + +def mode(path): + return stat.S_IMODE(path.stat().st_mode) + + +@pytest.fixture(autouse=True) +def usual_umask(): + """The umask a service usually runs with, which leaves files 0644.""" + + previous = os.umask(0o022) + yield + os.umask(previous) + + +@pytest.fixture +def staging_dir(tmp_path): + return tmp_path / ".incoming" / IMAGE + + +def test_first_chunk_creates_the_macct_owner_only(tmp_path, staging_dir): + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"unicodePwd:: secret") + assert mode(staging_dir / MACCT) == 0o600 + + +def test_first_chunk_creates_the_macct_without_a_later_chmod(tmp_path, staging_dir, monkeypatch): + # Without chmod the file keeps the mode it was created with: 0600 here + # means it never existed readable by others, not even for a moment. + monkeypatch.setattr(image_sync_module.os, "chmod", lambda *args, **kwargs: None) + monkeypatch.setattr(image_sync_module.os, "fchmod", lambda *args, **kwargs: None) + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"unicodePwd:: secret") + assert mode(staging_dir / MACCT) == 0o600 + + +def test_resumed_chunk_closes_a_macct_staged_readable(tmp_path, staging_dir): + staging_dir.mkdir(parents=True) + staged = staging_dir / MACCT + staged.write_bytes(b"unicodePwd:: ") + staged.chmod(0o644) + + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"secret", offset=len(b"unicodePwd:: ")) + + assert staged.read_bytes() == b"unicodePwd:: secret" + assert mode(staged) == 0o600 + + +def test_first_chunk_replaces_a_macct_staged_readable(tmp_path, staging_dir): + staging_dir.mkdir(parents=True) + staged = staging_dir / MACCT + staged.write_bytes(b"old") + staged.chmod(0o644) + + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"new") + + assert staged.read_bytes() == b"new" + assert mode(staged) == 0o600 + + +def test_image_chunk_keeps_the_usual_mode(tmp_path, staging_dir): + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"qcow2") + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"-more", offset=5) + assert (staging_dir / QCOW2).read_bytes() == b"qcow2-more" + assert mode(staging_dir / QCOW2) == 0o644 + + +def test_staging_directories_are_closed(tmp_path, staging_dir): + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"qcow2") + assert mode(tmp_path / ".incoming") == 0o700 + assert mode(staging_dir) == 0o700 + + +def test_staging_directories_created_open_are_closed_again(tmp_path, staging_dir): + staging_dir.mkdir(parents=True) + (tmp_path / ".incoming").chmod(0o755) + staging_dir.chmod(0o755) + + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"qcow2") + + assert mode(tmp_path / ".incoming") == 0o700 + assert mode(staging_dir) == 0o700 + + +def test_finalize_moves_the_macct_owner_only(tmp_path): + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"qcow2") + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"secret") + + finalize_upload(tmp_path, IMAGE) + + assert (tmp_path / IMAGE / MACCT).read_bytes() == b"secret" + assert mode(tmp_path / IMAGE / MACCT) == 0o600 + assert mode(tmp_path / IMAGE / QCOW2) == 0o644 + + +def test_finalize_closes_a_macct_staged_readable(tmp_path, staging_dir): + staging_dir.mkdir(parents=True) + (staging_dir / MACCT).write_bytes(b"secret") + (staging_dir / MACCT).chmod(0o644) + + finalize_upload(tmp_path, IMAGE) + + assert mode(tmp_path / IMAGE / MACCT) == 0o600 + + +def test_finalize_closes_a_staged_macct_before_moving_it(tmp_path, staging_dir, monkeypatch): + staging_dir.mkdir(parents=True) + (staging_dir / MACCT).write_bytes(b"secret") + (staging_dir / MACCT).chmod(0o644) + moved_modes = {} + move = image_sync_module.shutil.move + + def recording_move(src, dst, **kwargs): + moved_modes[os.path.basename(src)] = stat.S_IMODE(os.stat(src).st_mode) + return move(src, dst, **kwargs) + + monkeypatch.setattr(image_sync_module.shutil, "move", recording_move) + + finalize_upload(tmp_path, IMAGE) + + assert moved_modes == {MACCT: 0o600} + + +def test_finalize_moves_the_macct_across_filesystems_owner_only(tmp_path, monkeypatch): + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"qcow2") + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"secret") + + # rename() fails like it does across filesystems, so shutil.move() + # copies. Without chmod the copy keeps the mode it was created with. + def rename(src, dst): + raise OSError(errno.EXDEV, os.strerror(errno.EXDEV)) + + monkeypatch.setattr(image_sync_module.os, "rename", rename) + monkeypatch.setattr(image_sync_module.os, "chmod", lambda *args, **kwargs: None) + monkeypatch.setattr(image_sync_module.os, "fchmod", lambda *args, **kwargs: None) + + finalize_upload(tmp_path, IMAGE) + + assert (tmp_path / IMAGE / MACCT).read_bytes() == b"secret" + assert mode(tmp_path / IMAGE / MACCT) == 0o600 + assert not (tmp_path / ".incoming" / IMAGE / MACCT).exists() + assert (tmp_path / IMAGE / QCOW2).read_bytes() == b"qcow2" + + +def test_finalize_backs_up_the_macct_owner_only(tmp_path): + target_dir = tmp_path / IMAGE + target_dir.mkdir() + (target_dir / QCOW2).write_bytes(b"old qcow2") + (target_dir / MACCT).write_bytes(b"old secret") + (target_dir / MACCT).chmod(0o644) + os.utime(target_dir / MACCT, (1_000_000_000, 1_000_000_000)) + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"new qcow2") + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"new secret") + + result = finalize_upload(tmp_path, IMAGE) + + backup_dir = tmp_path / result["backup"] + assert (backup_dir / MACCT).read_bytes() == b"old secret" + assert mode(backup_dir / MACCT) == 0o600 + assert (backup_dir / MACCT).stat().st_mtime == 1_000_000_000 + assert mode(backup_dir / QCOW2) == 0o644 + assert mode(target_dir / MACCT) == 0o600 + + +def test_finalize_closes_a_macct_the_upload_left_in_place(tmp_path): + target_dir = tmp_path / IMAGE + target_dir.mkdir() + (target_dir / MACCT).write_bytes(b"secret") + (target_dir / MACCT).chmod(0o644) + receive_upload_chunk(tmp_path, IMAGE, QCOW2, b"qcow2") + + finalize_upload(tmp_path, IMAGE) + + assert mode(target_dir / MACCT) == 0o600 + + +def test_macct_and_staging_become_root_owned_when_running_as_root(tmp_path, monkeypatch): + owned = [] + + def chown(path, uid, gid): + owned.append((os.path.relpath(path, tmp_path), uid, gid)) + + def fchown(fd, uid, gid): + chown(os.readlink(f"/proc/self/fd/{fd}"), uid, gid) + + monkeypatch.setattr(image_sync_module.os, "geteuid", lambda: 0) + monkeypatch.setattr(image_sync_module.os, "chown", chown) + monkeypatch.setattr(image_sync_module.os, "fchown", fchown) + + target_dir = tmp_path / IMAGE + target_dir.mkdir() + (target_dir / QCOW2).write_bytes(b"old qcow2") + (target_dir / MACCT).write_bytes(b"old secret") + receive_upload_chunk(tmp_path, IMAGE, MACCT, b"new secret") + + assert set(owned) == { + (".incoming", 0, 0), + (f".incoming/{IMAGE}", 0, 0), + (f".incoming/{IMAGE}/{MACCT}", 0, 0), + } + + owned.clear() + result = finalize_upload(tmp_path, IMAGE) + backup = os.path.relpath(result["backup"], tmp_path) + + assert set(owned) == { + (f".incoming/{IMAGE}/{MACCT}", 0, 0), + (f"{backup}/{MACCT}", 0, 0), + (f"{IMAGE}/{MACCT}", 0, 0), + }