From b877c3546f5e523df641e3d237cde34c31265c51 Mon Sep 17 00:00:00 2001 From: Anton Khirnov Date: Thu, 12 Oct 2023 19:07:02 +0200 Subject: targets: remove TargetSSHLXCLVM It is complex and fragile and has been replaced by idmapping/TargetSSHLVM/strip_mountpoint=True. Also drop nsmount.c, which was to be used with this target. --- lbup/targets.py | 105 ++----------------------- nsmount.c | 233 -------------------------------------------------------- 2 files changed, 5 insertions(+), 333 deletions(-) delete mode 100644 nsmount.c diff --git a/lbup/targets.py b/lbup/targets.py index 37a58f0..d6d44ff 100644 --- a/lbup/targets.py +++ b/lbup/targets.py @@ -241,11 +241,6 @@ class TargetSSHLVM(TargetSSH): """ _snapshot_size = None - """ - PID of the process whose mount namespace we use. - """ - _mntns_pid = None - _strip_mountpoint = None def __init__(self, name, dirs, excludes = None, logger = None, @@ -259,22 +254,6 @@ class TargetSSHLVM(TargetSSH): def __str__(self): return "%s{LVM:%s}" % (super().__str__(), self._snapshot_size) - def _root_remote(self): - """ - The the remote for root operations, such as manipulating LVM. - - Overridden in subclasses. - """ - return SSHRemote(self._remote.host, self._remote.port, - 'root', self._remote.proxy_remote) - - def _resolve_mntns(self, ssh): - """ - Get the PID of the process whose mount namespace we use. - Always 1 here, overridden in subclasses. - """ - return 1 - def _resolve_lv(self, ssh, devnum): """ Find the logical volume for the given device number. @@ -347,8 +326,11 @@ class TargetSSHLVM(TargetSSH): def save(self, dry_run = False): with contextlib.ExitStack() as stack: + root_remote = SSHRemote(self._remote.host, self._remote.port, + 'root', self._remote.proxy_remote) + conn_tgt = stack.enter_context(_ssh_client.SSHConnection(self._remote)) - conn_root = stack.enter_context(_ssh_client.SSHConnection(self._root_remote())) + conn_root = stack.enter_context(_ssh_client.SSHConnection(root_remote)) # resolve the remote paths bupdir = self._resolve_remote_path(conn_tgt, self._remote_bupdir) @@ -362,12 +344,9 @@ class TargetSSHLVM(TargetSSH): snapshot_mount = '%s/%s' % (bupdir, 'lbup_mount') self._paramiko_exec_cmd(conn_tgt, 'mkdir -p -m 700 ' + snapshot_mount) - self._mntns_pid = self._resolve_mntns(conn_root) - # read and parse the mountinfo table mntinfo = MountInfo( - self._paramiko_exec_cmd(conn_root, 'cat /proc/%d/mountinfo' % self._mntns_pid, - decode = False)) + self._paramiko_exec_cmd(conn_root, 'cat /proc/1/mountinfo', decode = False)) devnum, mountpoint, fstype = _resolve_mntdev(mntinfo, dirs) @@ -392,77 +371,3 @@ class TargetSSHLVM(TargetSSH): return self._do_save(bup_exec, dry_run, dirs = dirs, excludes = excludes, save_opts = save_opts, index_opts = index_opts) - -class TargetSSHLXCLVM(TargetSSHLVM): - """ - This target backs up an LXC container that lives on its own LVM logical - volume. Requires root-capable login on the container's host. - - :param SSHRemote parent_remote: - """ - # The container is treated as untrusted, so all code here needs to be - # careful to avoid giving it access to anything it would not have otherwise. - # Specifically, any information obtained by running binaries in the - # container should be assumed to have been potentially maliciously - # manipulated. No binaries from the container should be run as the (host) root. - _parent_remote = None - _lxc_username = None - _lxc_containername = None - - def __init__(self, name, dirs, excludes = None, logger = None, - target_remote = None, target_remote_bupdir = None, - parent_remote = None, - lxc_username = None, lxc_containername = None, - snapshot_size = '20G'): - if parent_remote is None: - raise ValueError('parent_remote not specified') - if lxc_username is None: - lxc_username = parent_remote.usename - - self._parent_remote = parent_remote - self._lxc_username = lxc_username - self._lxc_containername = lxc_containername - - super().__init__(name, dirs, excludes, logger, - target_remote, target_remote_bupdir, - snapshot_size) - - def __str__(self): - return "%s{LXC:%s/%s@[%s]}{LVM:%s}" % (super().__str__(), self._lxc_containername, - self._lxc_username, str(self._parent_remote), self._snapshot_size) - - def _resolve_mntns(self, ssh): - # get the PID of the container's init - cmd = 'su -s /bin/sh -c "lxc-info -H -p -n {container}" {user}'.format( - container = self._lxc_containername, user = self._lxc_username) - container_pid = self._paramiko_exec_cmd(ssh, cmd).rstrip('\n') - return int(container_pid) - - def _root_remote(self): - return self._parent_remote - - @contextlib.contextmanager - def _mount_snapshot(self, ssh, devnum, mount_path, fstype): - """ - Return a context manager that creates a read-only LVM snapshot - for the specified LV device number and mounts it at mount_path - inside the container, then unmounts and destroys it at exit. - """ - with self._snapshot_lv(ssh, devnum) as lv_path: - # we cannot trust any binaries located inside the container, since a - # compromised container could use them to execute arbitrary code - # with real root privileges, thus nullifying the point of - # unprivileged containers) - # so we ship a special tool, 'nsmount', which has to be - # installed on the parent, to mount the snapshot into the - # container mount namespace - self._paramiko_exec_cmd(ssh, - 'nsmount m {pid} {mount_path} {lv_path} {fstype}'.format( - pid = self._mntns_pid, mount_path = mount_path, - lv_path = lv_path, fstype = fstype)) - try: - yield None - finally: - self._paramiko_exec_cmd(ssh, - 'nsmount u {pid} {mount_path}'.format( - pid = self._mntns_pid, mount_path = mount_path)) diff --git a/nsmount.c b/nsmount.c deleted file mode 100644 index 9d1533a..0000000 --- a/nsmount.c +++ /dev/null @@ -1,233 +0,0 @@ -/** - * nsmount - mount a block device into a mount/pid namespace - * Copyright (C) 2019 Anton Khirnov - * - * nsmount is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * nsmount is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with nsmount. If not, see . - */ - -#define _XOPEN_SOURCE 700 -#define _GNU_SOURCE - -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include - -enum { - OP_MOUNT, - OP_UMOUNT, -}; - -static void print_usage(int argc, const char * const *argv) -{ - fprintf(stderr, - "%s: mount/unmount a block device in a mount/PID namespace\n\n" - "Usage:\n" - " %s m \n" - " %s u \n\n" - " : PID (in the namespace in which this program is executed)" - " of the process whose namespaces are to be entered into\n" - " : path (in the destination mount namespace) to be mounted" - " or unmounted\n" - " : path (in the namespace in which this program is executed)" - " to the block device that shall be mounted\n" - " : type of the filesystem to be mounted\n", - argv[0], argv[0], argv[0]); -} - -int main(int argc, const char * const *argv) -{ - char pathbuf[128]; - int blockdev_fd = -1, pidns_fd = -1, mountns_fd = -1; - const char *blockdev, *mountpoint, *fstype; - pid_t tgt_pid, child_pid; - int op; - int ret; - - /* parse the commandline */ - if (argc < 2) { - print_usage(argc, argv); - return 1; - } - - if (argv[1][0] == 'm') { - op = OP_MOUNT; - if (argc < 6) { - print_usage(argc, argv); - return 1; - } - } else if (argv[1][0] == 'u') { - op = OP_UMOUNT; - if (argc < 4) { - print_usage(argc, argv); - return 1; - } - } else { - fprintf(stderr, "Invalid operation: %s\n", - argv[1]); - print_usage(argc, argv); - return 1; - } - - tgt_pid = strtol(argv[2], NULL, 0); - mountpoint = argv[3]; - if (op == OP_MOUNT) { - blockdev = argv[4]; - fstype = argv[5]; - } - - /* open the files */ - if (op == OP_MOUNT) { - blockdev_fd = open(blockdev, O_RDONLY); - if (blockdev_fd == -1) { - fprintf(stderr, "Error opening %s: %s\n", - blockdev, strerror(errno)); - return 2; - } - } - - ret = snprintf(pathbuf, sizeof(pathbuf), "/proc/%d/ns/pid", - tgt_pid); - if (ret < 0 || ret >= sizeof(pathbuf)) { - fprintf(stderr, "Error constructing the PID namespace path\n"); - ret = 2; - goto finish; - } - - pidns_fd = open(pathbuf, O_RDONLY | O_CLOEXEC); - if (pidns_fd == -1) { - fprintf(stderr, "Error opening %s: %s\n", - pathbuf, strerror(errno)); - ret = 2; - goto finish; - } - - ret = snprintf(pathbuf, sizeof(pathbuf), "/proc/%d/ns/mnt", - tgt_pid); - if (ret < 0 || ret >= sizeof(pathbuf)) { - fprintf(stderr, "Error constructing the mount namespace path\n"); - ret = 2; - goto finish; - } - - mountns_fd = open(pathbuf, O_RDONLY | O_CLOEXEC); - if (mountns_fd == -1) { - fprintf(stderr, "Error opening %s: %s\n", - pathbuf, strerror(errno)); - ret = 2; - goto finish; - } - - /* enter the namespaces */ - ret = setns(pidns_fd, CLONE_NEWPID); - if (ret == -1) { - fprintf(stderr, "Error entering the PID namespace: %s\n", - strerror(errno)); - ret = 3; - goto finish; - } - - ret = setns(mountns_fd, CLONE_NEWNS); - if (ret == -1) { - fprintf(stderr, "Error entering the mount namespace: %s\n", - strerror(errno)); - ret = 3; - goto finish; - } - - /* fork to actually enter the PID namespace */ - child_pid = fork(); - if (child_pid == -1) { - fprintf(stderr, "fork() failed: %s\n", - strerror(errno)); - ret = 4; - goto finish; - } - - if (child_pid) { - /* we are the parent */ - ret = wait(NULL); - if (ret == -1) { - fprintf(stderr, "Error waiting for the child: %s\n", - strerror(errno)); - ret = 4; - goto finish; - } - } else { - /* we are the child */ - if (op == OP_MOUNT) { - /* we use /proc/self/fd to mount the device - * Since the container controls its own filesystem hierarchy, it - * could trick us into mounting an arbitrary node located in the - * filesystem. This is not considered a major security problem, - * since - * - the container should not have access to mknod() or nodes that - * it is not meant to read - * - we mount the filesystem read-only, with nosuid flag - * - since the container will typically live in its own user - * namespace, it will not have the right permissions to access a - * filesystem that is not intended for it - * - * Ideally, there would be something like a mountfd() syscall that - * would allow mounting an fd. - */ - ret = snprintf(pathbuf, sizeof(pathbuf), - "/proc/self/fd/%d", blockdev_fd); - if (ret < 0 || ret >= sizeof(pathbuf)) { - fprintf(stderr, "Error constructing the mount path\n"); - ret = 4; - goto finish; - } - - ret = mount(pathbuf, mountpoint, fstype, MS_RDONLY | MS_NOSUID, NULL); - if (ret == -1) { - fprintf(stderr, "mount(%s, %s) failed: %s\n", - pathbuf, mountpoint, strerror(errno)); - ret = 5; - goto finish; - } - } else if (op == OP_UMOUNT) { - /** - * As above, a malicious container can trick us into unmounting a - * filesystem in its tree. This should not cause any issues other - * than disrupting the container (which a compromised container can - * already do without our help). - */ - ret = umount(mountpoint); - if (ret == -1) { - fprintf(stderr, "umount() failed: %s\n", strerror(errno)); - ret = 5; - goto finish; - } - } - } - - ret = 0; -finish: - if (blockdev_fd >= 0) - close(blockdev_fd); - if (pidns_fd >= 0) - close(pidns_fd); - if (mountns_fd >= 0) - close(mountns_fd); - - return ret; -} -- cgit v1.2.3