| Message ID | 217a387d55b5828a82fadad98dd88a959e7a13ed.1429008167.git.lauri.vosandi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 14, 2015 at 01:44:32PM +0300, Lauri Võsandi wrote: > This patch forces btrfs receive to issue chroot before > parsing the btrfs stream to confine the process and > minimize damage that could be done via malicious > btrfs stream. Thanks. As we've discussed, there are possibly some things to resolve: * chdir("/") after chroot * commandline options to enable/disable chroot, choose the default Receive should work for a non-root user so chroot should be conditional, but I'm not sure if this should be guessed from the UID or if this would be better to specify only by the commandline options. I'll put the patch into a separate branch for now. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015-04-14 08:28, David Sterba wrote: > On Tue, Apr 14, 2015 at 01:44:32PM +0300, Lauri Võsandi wrote: >> This patch forces btrfs receive to issue chroot before >> parsing the btrfs stream to confine the process and >> minimize damage that could be done via malicious >> btrfs stream. > > Thanks. > > As we've discussed, there are possibly some things to resolve: > > * chdir("/") after chroot > * commandline options to enable/disable chroot, choose the default > > Receive should work for a non-root user so chroot should be conditional, > but I'm not sure if this should be guessed from the UID or if this would > be better to specify only by the commandline options. > > I'll put the patch into a separate branch for now. Personally, I would expect it to default to not using chroot(), provide a commandline option to tell it to do so, and then just catch the error from trying to chroot as a non-root user.
On Tue, Apr 14, 2015 at 09:19:12AM -0400, Austin S Hemmelgarn wrote: > On 2015-04-14 08:28, David Sterba wrote: > > On Tue, Apr 14, 2015 at 01:44:32PM +0300, Lauri Võsandi wrote: > >> This patch forces btrfs receive to issue chroot before > >> parsing the btrfs stream to confine the process and > >> minimize damage that could be done via malicious > >> btrfs stream. > > > > Thanks. > > > > As we've discussed, there are possibly some things to resolve: > > > > * chdir("/") after chroot > > * commandline options to enable/disable chroot, choose the default > > > > Receive should work for a non-root user so chroot should be conditional, > > but I'm not sure if this should be guessed from the UID or if this would > > be better to specify only by the commandline options. > > > > I'll put the patch into a separate branch for now. > > Personally, I would expect it to default to not using chroot(), provide > a commandline option to tell it to do so, and then just catch the error > from trying to chroot as a non-root user. Thanks, I agree with that. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-receive.c b/cmds-receive.c index 44ef27e..8be92ea 100644 --- a/cmds-receive.c +++ b/cmds-receive.c @@ -867,15 +867,17 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd, goto out; } - /* - * find_mount_root returns a root_path that is a subpath of - * dest_dir_full_path. Now get the other part of root_path, - * which is the destination dir relative to root_path. - */ - r->dest_dir_path = dest_dir_full_path + strlen(r->root_path); - while (r->dest_dir_path[0] == '/') - r->dest_dir_path++; + if (chroot(dest_dir_full_path)) { + ret = -errno; + fprintf(stderr, + "ERROR: failed to chroot to %s, %s\n", + dest_dir_full_path, + strerror(-ret)); + goto out; + } + r->root_path = r->dest_dir_path = strdup("/"); + ret = subvol_uuid_search_init(r->mnt_fd, &r->sus); if (ret < 0) goto out;
This patch forces btrfs receive to issue chroot before parsing the btrfs stream to confine the process and minimize damage that could be done via malicious btrfs stream. Signed-off-by: Lauri Võsandi <lauri.vosandi@gmail.com> --- cmds-receive.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)