From owner-freebsd-fs@freebsd.org Tue Apr 11 20:22:09 2017 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C71D9D3AD5E for ; Tue, 11 Apr 2017 20:22:09 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: from mail-io0-x22c.google.com (mail-io0-x22c.google.com [IPv6:2607:f8b0:4001:c06::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8684E638 for ; Tue, 11 Apr 2017 20:22:09 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: by mail-io0-x22c.google.com with SMTP id r16so15451359ioi.2 for ; Tue, 11 Apr 2017 13:22:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sippysoft-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=XhERijJ19H9pUjdYci7d7OMcYP2tTYZRVba7Dfv0y0s=; b=DrpyDuWIrfbRie8d8V8ar49nSkM2XMHzC94iGaJloxYOIFJPqJLhq55kDwz+TiMVkn PM4oIPBuzK+T41nM414CTSX+D4s9w+jrrFDC30kq76KOzr0th8cJSzwX2pRXL86TMeME zUPKn7tOHE2XhNF8eBJohvQis55mOmo/INJeTMhjgE0KcfQNUA7Gtz/OcZsKGgVki2og M0afpIJsBtjbYsz2Ld1pktjJxr7yVVsBfh0+Kn9UBZucYxNfUI663aiZFLYlGFOylimu B59EB1x3G6DHqyK4eI5zjp/HUuZ95C1dZDQAlxQpncwflkgCbdDRFQhH7piYVVHIyD0a HO5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=XhERijJ19H9pUjdYci7d7OMcYP2tTYZRVba7Dfv0y0s=; b=A1V3/IMmuu7R0SAj86qqCCK4ydttl673Sdnr6e1IMBuv7+iE7uGszydj/wXJeXei5n cl/0cHZ3BlYdecxOikjPwUAcWh8Y0tqGIgdbXpsqx9emlZe1LXuyWXRnd+PvNPzqTJHZ osobNy2VAX6EM0VsUv1omiaJub/DnXRuUAiUPdKv7zK+wofHMIh0lhGjL9t6NEzkBW8a UEGQ6TBw65BASS5SR9NOTlqYznOPoJ/j9XUDOYjR7+nSAO4STzb/OcpUjRryt41NyIiD BEMJoauxcAjEYO+ucPyS6oQitonfBRzuuitucknOljs8w9zt637c7wbgFeXkHlo8AO0i Ehbw== X-Gm-Message-State: AFeK/H35cLuu6may9Vuzb6W8pI5hkkMjpR+ZG64DdCD5UDqMIm0optYfz7pbGPn+7ocGbeS9fbc2RD+HpJQa85wt X-Received: by 10.107.171.67 with SMTP id u64mr57477142ioe.102.1491942128752; Tue, 11 Apr 2017 13:22:08 -0700 (PDT) MIME-Version: 1.0 Sender: sobomax@sippysoft.com Received: by 10.36.112.210 with HTTP; Tue, 11 Apr 2017 13:22:08 -0700 (PDT) In-Reply-To: References: <201704111807.v3BI7PGM001952@chez.mckusick.com> From: Maxim Sobolev Date: Tue, 11 Apr 2017 13:22:08 -0700 X-Google-Sender-Auth: vdYAvcJNgoiuu-tUdLqL6jMuAXg Message-ID: Subject: Re: mksnap_ffs(8) is not working while chrooted To: Kirk McKusick Cc: FreeBSD Filesystems Content-Type: multipart/mixed; boundary=94eb2c059e149cc4b8054ce9d80d X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Apr 2017 20:22:09 -0000 --94eb2c059e149cc4b8054ce9d80d Content-Type: text/plain; charset=UTF-8 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 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 > 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 >> wrote: >> >>> > From: Maxim Sobolev >>> > Date: Tue, 11 Apr 2017 10:40:58 -0700 >>> > Subject: mksnap_ffs(8) is not working while chrooted >>> > To: Kirk McKusick , >>> > FreeBSD Filesystems >>> > >>> > 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 >>> >>> >> > --94eb2c059e149cc4b8054ce9d80d Content-Type: text/plain; charset=US-ASCII; name="mksnap_ffs.diff" Content-Disposition: attachment; filename="mksnap_ffs.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_j1dzs84g0 ZGlmZiAtLWdpdCBhL3NiaW4vbWtzbmFwX2Zmcy9ta3NuYXBfZmZzLmMgYi9zYmluL21rc25hcF9m ZnMvbWtzbmFwX2Zmcy5jCmluZGV4IGVmZDljNmIzMy4uYjAzOWZhZGQyIDEwMDY0NAotLS0gYS9z YmluL21rc25hcF9mZnMvbWtzbmFwX2Zmcy5jCisrKyBiL3NiaW4vbWtzbmFwX2Zmcy9ta3NuYXBf ZmZzLmMKQEAgLTU4LDYgKzU4LDMzIEBAIHVzYWdlKHZvaWQpCiAJZXJyeChFWF9VU0FHRSwgInVz YWdlOiBta3NuYXBfZmZzIHNuYXBzaG90X25hbWUiKTsKIH0KIAorc3RhdGljIGludAoraXNkaXIo Y29uc3QgY2hhciAqcGF0aCkKK3sKKwlzdHJ1Y3Qgc3RhdCBzdGJ1ZjsKKworCWlmIChzdGF0KHBh dGgsICZzdGJ1ZikgPCAwKQorCQlyZXR1cm4gKC0xKTsKKyAgICAgICAgaWYgKCFTX0lTRElSKHN0 YnVmLnN0X21vZGUpKQorCQlyZXR1cm4gKDApOworCXJldHVybiAoMSk7Cit9CisKK3N0YXRpYyBp bnQKK2lzc2FtZWZzKGNvbnN0IGNoYXIgKnBhdGgsIHN0cnVjdCBzdGF0ZnMgKnN0ZnNwKQorewor CXN0cnVjdCBzdGF0ZnMgc3Rmc2J1ZjsKKworCWlmIChpc2RpcihwYXRoKSAhPSAxKQorCQlyZXR1 cm4gKC0xKTsKKwlpZiAoc3RhdGZzKHBhdGgsICZzdGZzYnVmKSA8IDApCisJCXJldHVybiAoLTEp OworCWlmICgoc3Rmc2J1Zi5mX2ZzaWQudmFsWzBdICE9IHN0ZnNwLT5mX2ZzaWQudmFsWzBdKSB8 fAorCSAgICAoc3Rmc2J1Zi5mX2ZzaWQudmFsWzFdICE9IHN0ZnNwLT5mX2ZzaWQudmFsWzFdKSkK KwkJcmV0dXJuICgwKTsKKwlyZXR1cm4gKDEpOworfQorCiBpbnQKIG1haW4oaW50IGFyZ2MsIGNo YXIgKiphcmd2KQogewpAQCAtOTYsMTYgKzEyMywzMyBAQCBtYWluKGludCBhcmdjLCBjaGFyICoq YXJndikKIAl9CiAJaWYgKHN0YXRmcyhwYXRoLCAmc3Rmc2J1ZikgPCAwKQogCQllcnIoMSwgIiVz IiwgcGF0aCk7Ci0JaWYgKHN0YXQocGF0aCwgJnN0YnVmKSA8IDApCisJc3dpdGNoIChpc2Rpcihw YXRoKSkgeworCWNhc2UgLTE6CiAJCWVycigxLCAiJXMiLCBwYXRoKTsKLQlpZiAoIVNfSVNESVIo c3RidWYuc3RfbW9kZSkpCisJY2FzZSAwOgogCQllcnJ4KDEsICIlczogTm90IGEgZGlyZWN0b3J5 IiwgcGF0aCk7CisJZGVmYXVsdDoKKwkJYnJlYWs7CisJfQogCWlmIChhY2Nlc3MocGF0aCwgV19P SykgPCAwKQogCQllcnIoMSwgIkxhY2sgd3JpdGUgcGVybWlzc2lvbiBpbiAlcyIsIHBhdGgpOwog CWlmICgoc3RidWYuc3RfbW9kZSAmIFNfSVNUWFQpICYmIHN0YnVmLnN0X3VpZCAhPSBnZXR1aWQo KSkKIAkJZXJyeCgxLCAiTGFjayB3cml0ZSBwZXJtaXNzaW9uIGluICVzOiBTdGlja3kgYml0IHNl dCIsIHBhdGgpOwogCiAJLyoKKwkgKiBXb3JrIGFyb3VuZCBhbiBpc3N1ZSB3aGVuIG1rc25hcF9m ZnMgaXMgc3RhcnRlZCBpbiBjaHJvb3QnZWQKKwkgKiBlbnZpcm9ubWVudCBhbmQgZl9tbnRvbm5h bWUgY29udGFpbnMgYWJzb2x1dGUgcGF0aCB3aXRoaW4KKwkgKiByZWFsIHJvb3QuCisJICovCisJ Zm9yIChjcCA9IHN0ZnNidWYuZl9tbnRvbm5hbWU7IGlzc2FtZWZzKGNwLCAmc3Rmc2J1ZikgIT0g MTsKKwkgICAgY3AgPSBzdHJjaHJudWwoY3AgKyAxLCAnLycpKSB7CisJCWlmIChjcFswXSA9PSAn XDAnKQorCQkJZXJyeCgxLCAiJXM6IE5vdCBhIG1vdW50IHBvaW50Iiwgc3Rmc2J1Zi5mX21udG9u bmFtZSk7CisJfQorCWlmIChjcCAhPSBzdGZzYnVmLmZfbW50b25uYW1lKQorCQlzdHJsY3B5KHN0 ZnNidWYuZl9tbnRvbm5hbWUsIGNwLCBzaXplb2Yoc3Rmc2J1Zi5mX21udG9ubmFtZSkpOworCisJ LyoKIAkgKiBIYXZpbmcgdmVyaWZpZWQgYWNjZXNzIHRvIHRoZSBkaXJlY3RvcnkgaW4gd2hpY2gg dGhlCiAJICogc25hcHNob3QgaXMgdG8gYmUgYnVpbHQsIHByb2NlZWQgd2l0aCBjcmVhdGluZyBp dC4KIAkgKi8K --94eb2c059e149cc4b8054ce9d80d--