mksnap_ffs(8) is not working while chrooted
Maxim Sobolev
sobomax at freebsd.org
Tue Apr 11 20:22:09 UTC 2017
Kirk, actually it turns out this is not too difficult to work around this
issue completely in userland. Please see the patch attached, should be
relatively bulletproof as it actually verifies fsid of whatever it finds
with the fsid of the target FS. Please let me know if there are any
objections to include it into the base system.
Thanks!
-Maxim
On Tue, Apr 11, 2017 at 11:55 AM, Maxim Sobolev <sobomax at freebsd.org> wrote:
> Kirk, yes, indeed, it works just fine if I chop out the chroot prefix
> manually here:
>
> [/builder.trunk/usr/src/sbin/mksnap_ffs]$ sudo chroot /builder.trunk sh
> # truncate -s 100m /tmp/fsimage
> # mdconfig -a -f /tmp/fsimage
> md0
> # newfs /dev/md0
> /dev/md0: 100.0MB (204800 sectors) block size 32768, fragment size 4096
> using 4 cylinder groups of 25.03MB, 801 blks, 3328 inodes.
> super-block backups (for fsck_ffs -b #) at:
> 192, 51456, 102720, 153984
> # mount /dev/md0 /mnt
> # /usr/src/sbin/mksnap_ffs/mksnap_ffs /mnt/mysnap
> # ls -l /mnt/mysnap
> -r--r----- 1 root operator 104857672 Apr 11 18:42 /mnt/mysnap
> # ^D
> [/builder.trunk/usr/src/sbin/mksnap_ffs]$ git diff
> diff --git a/sbin/mksnap_ffs/mksnap_ffs.c b/sbin/mksnap_ffs/mksnap_ffs.c
> index efd9c6b33..b1fba5e16 100644
> --- a/sbin/mksnap_ffs/mksnap_ffs.c
> +++ b/sbin/mksnap_ffs/mksnap_ffs.c
> @@ -116,7 +116,7 @@ main(int argc, char **argv)
> iovlen = 0;
> build_iovec(&iov, &iovlen, "fstype", "ffs", 4);
> build_iovec(&iov, &iovlen, "from", snapname, (size_t)-1);
> - build_iovec(&iov, &iovlen, "fspath", stfsbuf.f_mntonname,
> (size_t)-1);
> + build_iovec(&iov, &iovlen, "fspath", stfsbuf.f_mntonname + 14,
> (size_t)-1);
> build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
> build_iovec(&iov, &iovlen, "update", NULL, 0);
> build_iovec(&iov, &iovlen, "snapshot", NULL, 0);
>
> So my point is that by making this scenario working OOB I don't think we
> would add any new security issue. This already works if you pass proper
> path into the nmount(2) and the mount path is inside chroot. In principle I
> can possibly do some clever trick on the userland only to strip offending
> prefix f_mntonname here component by component until we can locate the
> mount dir, but it's likely to be somewhat fuzzy and clearly not suitable
> for the basesrc repo.
>
> -Max
>
> On Tue, Apr 11, 2017 at 11:23 AM, Maxim Sobolev <sobomax at freebsd.org>
> wrote:
>
>> Kirk, what you are saying in not applying in our case. The whole FS is
>> mounted *inside* the chroot. The reason why we are trying to use mksnap_ffs
>> is to take a clean snapshot to make a compressed version of it without the
>> need to forcefully zero out free space.
>>
>> So it looks like the following:
>>
>> parent# chroot /mnt/chroot /bin/sh
>> chroot# truncate /tmp/diskimage
>> chroot# mdconfig -a -f /tmp/diskimage
>> md0
>> chroot# newfs md0
>> chroot# mount /dev/md0 /mnt
>> [...do stuff with /mnt...]
>> chroot# mksnap_ffs /mnt/snap
>> mksnap_ffs: No such file or directory
>>
>> Perhaps, to work around your concern is to add some flag for the
>> statfs(1) to normalize f_mntonname for use inside chroot then? I have
>> not tested it here, but I believe that if I'd strip "/mnt/chroot" prefix
>> inside mksnap_ffs(8) that would work in our scenario just fine.
>>
>> -Max
>>
>> On Tue, Apr 11, 2017 at 11:07 AM, Kirk McKusick <mckusick at mckusick.com>
>> wrote:
>>
>>> > From: Maxim Sobolev <sobomax at freebsd.org>
>>> > Date: Tue, 11 Apr 2017 10:40:58 -0700
>>> > Subject: mksnap_ffs(8) is not working while chrooted
>>> > To: Kirk McKusick <mckusick at mckusick.com>,
>>> > FreeBSD Filesystems <freebsd-fs at freebsd.org>
>>> >
>>> > Hi Kirk et al,
>>> >
>>> > I've stumbled upon problem that it is impossible to use mksnap_ffs(8)
>>> while
>>> > in the chrooted environment. Other utilities that manipulate fs'es
>>> (i.e.
>>> > mount(8) / umount(8)) work just fine. Quick glance through the code
>>> shows
>>> > that the problem stems from the fact that mksnap_ffs uses f_mntonname
>>> > returned by the statfs(2) system call to fill fspath parameter for the
>>> > nmount call. And the statfs() returns f_mntonname path outside chroot.
>>> As
>>> > far as I can see, there are two options to address this issue.
>>> >
>>> > 1. Adjust statfs(2) system call to substract chroot prefix while
>>> > returning f_mntonname. Similar to what prison_enforce_statfs() function
>>> > does for jails.
>>> >
>>> > 2. Enhance nmount(2) to allow taking FSID in place of mount path to do
>>> > resolution using existing flag MNT_BYFSID and adjust mksnap_ffs to use
>>> that
>>> > instead. This is what umount(8) does to work around the problem.
>>> >
>>> > Which of two approaches would be preferred solution if any? The second
>>> one
>>> > seems a bit simpler to me. Please advise. Thanks!
>>> >
>>> > -Max
>>>
>>> It is not secure to allow mksnap_ffs(8) to work inside a jail. The issue
>>> is that mksnap_ffs takes a snapshot of the entire filesystem. Based on
>>> the
>>> way that snapshots work in FFS, it is not possible to snapshot only the
>>> part of the filesystem that is in the jail. Thus, if you take a snapshot
>>> of the entire filesystem and then mount it inside the jail, you will
>>> expose parts of the filesystem from outside of the jail to the jail.
>>> As such, you should only be able to snapshot a filesystem if it is
>>> entirely contained within the jail.
>>>
>>> If snapshots within jails are important to you, I recommend that you use
>>> ZFS which allows you to create per-jail filesystems which you can then
>>> snapshot.
>>>
>>> Kirk McKusick
>>>
>>>
>>
>
-------------- next part --------------
diff --git a/sbin/mksnap_ffs/mksnap_ffs.c b/sbin/mksnap_ffs/mksnap_ffs.c
index efd9c6b33..b039fadd2 100644
--- a/sbin/mksnap_ffs/mksnap_ffs.c
+++ b/sbin/mksnap_ffs/mksnap_ffs.c
@@ -58,6 +58,33 @@ usage(void)
errx(EX_USAGE, "usage: mksnap_ffs snapshot_name");
}
+static int
+isdir(const char *path)
+{
+ struct stat stbuf;
+
+ if (stat(path, &stbuf) < 0)
+ return (-1);
+ if (!S_ISDIR(stbuf.st_mode))
+ return (0);
+ return (1);
+}
+
+static int
+issamefs(const char *path, struct statfs *stfsp)
+{
+ struct statfs stfsbuf;
+
+ if (isdir(path) != 1)
+ return (-1);
+ if (statfs(path, &stfsbuf) < 0)
+ return (-1);
+ if ((stfsbuf.f_fsid.val[0] != stfsp->f_fsid.val[0]) ||
+ (stfsbuf.f_fsid.val[1] != stfsp->f_fsid.val[1]))
+ return (0);
+ return (1);
+}
+
int
main(int argc, char **argv)
{
@@ -96,16 +123,33 @@ main(int argc, char **argv)
}
if (statfs(path, &stfsbuf) < 0)
err(1, "%s", path);
- if (stat(path, &stbuf) < 0)
+ switch (isdir(path)) {
+ case -1:
err(1, "%s", path);
- if (!S_ISDIR(stbuf.st_mode))
+ case 0:
errx(1, "%s: Not a directory", path);
+ default:
+ break;
+ }
if (access(path, W_OK) < 0)
err(1, "Lack write permission in %s", path);
if ((stbuf.st_mode & S_ISTXT) && stbuf.st_uid != getuid())
errx(1, "Lack write permission in %s: Sticky bit set", path);
/*
+ * Work around an issue when mksnap_ffs is started in chroot'ed
+ * environment and f_mntonname contains absolute path within
+ * real root.
+ */
+ for (cp = stfsbuf.f_mntonname; issamefs(cp, &stfsbuf) != 1;
+ cp = strchrnul(cp + 1, '/')) {
+ if (cp[0] == '\0')
+ errx(1, "%s: Not a mount point", stfsbuf.f_mntonname);
+ }
+ if (cp != stfsbuf.f_mntonname)
+ strlcpy(stfsbuf.f_mntonname, cp, sizeof(stfsbuf.f_mntonname));
+
+ /*
* Having verified access to the directory in which the
* snapshot is to be built, proceed with creating it.
*/
More information about the freebsd-fs
mailing list