Implement basic working LXC+LVM snapshot functionality.

Still missing:
- proper exception handling
- stats

3 files changed, 359 insertions, 63 deletions

diff --git a/bupper/exceptions.py b/bupper/exceptions.py
index 8944fe9..69201ef 100644
--- a/bupper/exceptions.py
+++ b/bupper/exceptions.py
@@ -1,4 +1,7 @@
-class RemoteExecException(Exception):
+class BackupException(Exception):
+    pass
+
+class RemoteExecException(BackupException):
     retcode = None
     output  = None
     def __init__(self, explanation, retcode, output):
diff --git a/bupper/targets.py b/bupper/targets.py
index e56b71b..65fc22e 100644
--- a/bupper/targets.py
+++ b/bupper/targets.py
@@ -1,9 +1,14 @@
 
 from abc import ABC, abstractmethod
+import contextlib
+import errno
+import logging
+import os.path
 import re
+import socket
 import subprocess
 
-from .exceptions import RemoteExecException
+from .exceptions import BackupException, RemoteExecException
 from . import repository
 from . import ssh_remote
 from . import _ssh_client
@@ -32,7 +37,13 @@ class Target(ABC):
     name     = None
     dirs     = None
     excludes = None
-    def __init__(self, name, dirs, excludes = None):
+
+    _logger  = None
+
+    _index_opts = None
+    _save_opts  = None
+
+    def __init__(self, name, dirs, excludes = None, logger = None):
         if excludes is None:
             excludes = []
 
@@ -40,18 +51,23 @@ class Target(ABC):
         self.dirs     = dirs
         self.excludes = excludes
 
-    @abstractmethod
-    def save(self, data_dir):
-        pass
+        if logger is None:
+            self._logger = logging.getLogger(self.name)
+        else:
+            self._logger  = logger
 
-class TargetLocal(Target):
-    def save(self, data_dir):
-        cmd = ['bup', 'index', '--update', '--one-file-system']
+        self._index_opts = []
+        self._save_opts  = []
+
+    def _do_save(self, bup_exec):
+        cmd = bup_exec + ['index', '--update', '--one-file-system'] + self._index_opts
         cmd.extend(['--exclude=%s' % e for e in self.excludes])
         cmd.extend(self.dirs)
+        self._logger.debug('Executing index command: ' + str(cmd))
         res_idx = subprocess.run(cmd, capture_output = True)
 
-        cmd = ['bup', 'save', '-n', self.name] + self.dirs
+        cmd = bup_exec + ['save', '-n', self.name] + self._save_opts + self.dirs
+        self._logger.debug('Executing save command: ' + str(cmd))
         res_save = subprocess.run(cmd, capture_output = True)
 
         retcode = 0
@@ -67,13 +83,19 @@ class TargetLocal(Target):
 
         return result
 
+    @abstractmethod
+    def save(self, data_dir):
+        pass
+
+class TargetLocal(Target):
+    def save(self, data_dir):
+        return self._do_save(['bup'])
+
 class TargetSSH(Target):
     _remote = None
 
     def __init__(self, name, dirs, excludes = None,
                  remote = None):
-        super().__init__(name, dirs, excludes)
-
         if remote is None:
             remote = _parse_name(name)
         if remote.proxy_remote is not None:
@@ -82,36 +104,10 @@ class TargetSSH(Target):
             raise NotImplementedError('Specifying port not implemented')
         self._remote = remote
 
-    def save(self, data_dir):
-        cmd = ['bup', 'on', '%s@%s' % (self._remote.username, self._remote.host), 'index', '--update', '--one-file-system']
-        cmd.extend(['--exclude=%s' % e for e in self.excludes])
-        cmd.extend(self.dirs)
-        res_idx = subprocess.run(cmd, capture_output = True)
-
-        cmd = ['bup', 'on', '%s@%s' %(self._remote.username, self._remote.host), 'save', '-n', self.name] + self.dirs
-        res_save = subprocess.run(cmd, capture_output = True)
-
-        retcode = 0
-        output  = b''
-        if res_idx.returncode != 0:
-            retcode = res_idx.returncode
-            output += res_idx.stderr + res_idx.stdout
-        if res_save.returncode != 0:
-            retcode = res_save.returncode
-            output += res_save.stderr + res_save.stdout
-
-        result = repository.StepResult(retcode, output)
-
-        return result
+        super().__init__(name, dirs, excludes)
 
-def _paramiko_exec_cmd(client, cmd):
-    res = client.exec_command(cmd)
-    chan = res[0].channel
-    out, err = res[1].read(), res[2].read()
-    if chan.exit_status != 0:
-        raise RemoteExecException('Error executing "%s"' % cmd,
-                                  chan.exit_status, err + out)
-    return out.decode('utf-8', errors = 'backslashreplace')
+    def save(self, data_dir):
+        return self._do_save(['bup', 'on', '%s@%s' % (self._remote.username, self._remote.host)])
 
 class TargetSSHLXCLVM(TargetSSH):
     """
@@ -124,11 +120,18 @@ class TargetSSHLXCLVM(TargetSSH):
     _lxc_username      = None
     _lxc_containername = None
 
+    _container_mountpoint = '/mnt/bupper'
+
     def __init__(self, name, dirs, excludes = None,
                  target_remote = None, parent_remote = None,
                  lxc_username = None, lxc_containername = None,
                  snapshot_size = '20G'):
-        super().__init__(name, dirs, excludes, target_remote)
+        dirs_snapshot = [os.path.join(self._container_mountpoint, d) for d in dirs]
+        excludes_snapshot = None
+        if excludes is not None:
+            excludes_snapshot = [os.path.join(self._container_mountpoint, e) for e in excludes]
+
+        super().__init__(name, dirs_snapshot, excludes_snapshot, target_remote)
 
         if parent_remote is None:
             raise ValueError('parent_remote not specified')
@@ -140,47 +143,104 @@ class TargetSSHLXCLVM(TargetSSH):
         self._lxc_containername = lxc_containername
         self._snapshot_size     = snapshot_size
 
+    def _paramiko_exec_cmd(self, client, cmd):
+        self._logger.debug('Client %s: executing command: %s' % (client, cmd))
+
+        res = client.exec_command(cmd)
+
+        chan = res[0].channel
+        chan.settimeout(64)
+        try:
+            out, err = res[1].read(), res[2].read()
+        except socket.timeout as t:
+            raise RemoteExecException('Timeout waiting for command output',
+                                      errno.ETIMEDOUT, b'') from t
+
+        chan.recv_exit_status()
+        if chan.exit_status != 0:
+            raise RemoteExecException('Error executing "%s"' % cmd,
+                                      chan.exit_status, err + out)
+        return out.decode('utf-8', errors = 'backslashreplace')
+
+
     def save(self, data_dir):
-        with (_ssh_client.SSHConnection(self._parent_remote) as parent,
-              _ssh_client.SSHConnection(self._remote)        as container):
+        with contextlib.ExitStack() as stack:
+            parent    = stack.enter_context(_ssh_client.SSHConnection(self._parent_remote))
+            container = stack.enter_context(_ssh_client.SSHConnection(self._remote))
+
+            # create the mount directory
+            self._container_mountpoint = self._paramiko_exec_cmd(container,
+                'mktemp -d --tmpdir bupper.XXXXXXXX').rstrip('\n')
+            if len(self._container_mountpoint) <= 1 or self._container_mountpoint[0] != '/':
+                raise BackupException('Unexpected mount directory: %s' % self._container_mountpoint)
+            stack.callback(lambda: self._paramiko_exec_cmd(container,
+                'rmdir %s' % self._container_mountpoint))
+
+            self._save_opts.extend(['--strip-path', self._container_mountpoint])
+
             # get the PID of the container's init
             cmd_template = 'su -s /bin/sh -c "{command}" %s' % self._lxc_username
-            container_pid = _paramiko_exec_cmd(parent, cmd_template.format(
+            container_pid = self._paramiko_exec_cmd(parent, cmd_template.format(
                 command = 'lxc-info -H -p -n %s' % self._lxc_containername)).rstrip('\n')
+            if not re.fullmatch('[0-9]+', container_pid):
+                raise BackupException('Invalid container PID: %s' % container_pid)
 
             # get the LV/VG for the container's rootfs
-            container_rootfs = _paramiko_exec_cmd(parent, cmd_template.format(
-                    command = 'lxc-info -H -c lxc.rootfs -n %s' %
+            container_rootfs = self._paramiko_exec_cmd(parent, cmd_template.format(
+                    command = 'lxc-info -H -c lxc.rootfs.path -n %s' %
                     self._lxc_containername))\
                 .rstrip('\n')\
-                .translate({ord(' ') : r'\040', ord('\t') : r'\011',
-                           ord('\n') : r'\012', ord('\\') : r'\O134'})
-            mountline = _paramiko_exec_cmd(parent, 'grep "%s" /proc/mounts' %
+                .translate({ord(' ')  : r'\040', ord('\t') : r'\011',
+                            ord('\n') : r'\012', ord('\\') : r'\0134'})
+            if len(container_rootfs) <= 1 or container_rootfs[0] != '/':
+                raise BackupException('Unxpected container rootfs directory: %s' % container_rootfs)
+
+            mountline = self._paramiko_exec_cmd(parent, 'grep "%s" /proc/mounts' %
                                            container_rootfs).rstrip('\n').split()
             if len(mountline) < 2 or mountline[1] != container_rootfs:
-                raise RemoteExecException('Invalid mount line: %s' % mountline)
+                raise BackupException('Invalid mount line: %s' % mountline)
             lv_path = mountline[0]
-            lv_name, vg_name = _paramiko_exec_cmd(parent,
+            lv_fstype = mountline[2]
+            if len(lv_path) <= 1 or lv_path[0] != '/' or len(lv_fstype) < 1:
+                raise BackupException('Unexpected LV path/FS type: %s\t%s' % (lv_path, lv_fstype))
+
+            lv_name, vg_name = self._paramiko_exec_cmd(parent,
                 'lvdisplay -C --noheadings -o lv_name,vg_name ' + lv_path)\
                 .strip().split()
-
-            # 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 now create a temporary
-
+            if len(lv_name) < 1 or len(vg_name) < 1:
+                raise BackupException('Unexpected LV/VG name: %s\t%s' % (lv_name, vg_name))
 
             # create a read-only snapshot
             snapshot_name = 'bupper_' + lv_name
-            _paramiko_exec_cmd(parent,
+            self._paramiko_exec_cmd(parent,
                 'lvcreate --permission r --snapshot -L {size} -n {name} {origin}'
                 .format(size = self._snapshot_size, name = snapshot_name,
                         origin = lv_path))
+            stack.callback(lambda: self._paramiko_exec_cmd(parent,
+                'lvremove -f %s/%s' % (vg_name, snapshot_name)))
 
             # execute the backup
+            # wait for the new node to be created
+            self._paramiko_exec_cmd(parent, 'udevadm settle')
+
+            # 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(parent,
+                'nsmount m {pid} {mountpoint} {devpath} {fstype}'.format(
+                    pid = container_pid, mountpoint = self._container_mountpoint,
+                    devpath = '/dev/%s/%s' % (vg_name, snapshot_name),
+                    fstype = lv_fstype))
+
             try:
-                print(container_pid, vg_name, lv_path, snapshot_name)
+                ret = super().save(data_dir)
             finally:
-                # delete the snapshot
-                _paramiko_exec_cmd(parent, 'lvremove -f %s/%s' % (vg_name, snapshot_name))
+                self._paramiko_exec_cmd(parent,
+                    'nsmount u {pid} {mountpoint}'.format(
+                        pid = container_pid, mountpoint = self._container_mountpoint))
+
+            return ret
diff --git a/nsmount.c b/nsmount.c
new file mode 100644
index 0000000..9d1533a
--- /dev/null
+++ b/nsmount.c
@@ -0,0 +1,233 @@
+/**
+ *  nsmount - mount a block device into a mount/pid namespace
+ *  Copyright (C) 2019  Anton Khirnov <anton@khirnov.net>
+ *
+ *  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 <http://www.gnu.org/licenses/>.
+ */
+
+#define _XOPEN_SOURCE 700
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/mount.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+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 <PID> <mountpoint> <blkdev_path> <fstype>\n"
+            "   %s u <PID> <mountpoint>\n\n"
+            " <PID>: PID (in the namespace in which this program is executed)"
+                " of the process whose namespaces are to be entered into\n"
+            " <mountpoint>: path (in the destination mount namespace) to be mounted"
+                " or unmounted\n"
+            " <blkdev_path>: path (in the namespace in which this program is executed)"
+                " to the block device that shall be mounted\n"
+            " <fstype>: 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;
+}