Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1394399 > unrolled thread

[RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems

Back to article view | Back to linux.kernel

Contents

  [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-04 16:50 +0200
    [RFC v2 PATCH 1/8] VFS: add CLONE_MNTNS_SHIFT_UIDGID flag to allow mounts to shift their UIDs/GIDs Djalal Harouni <tixxdz@gmail.com> - 2016-05-04 16:50 +0200
    Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Josh Triplett <josh@joshtriplett.org> - 2016-05-04 18:40 +0200
    Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-05-04 23:10 +0200
      Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-05 09:40 +0200
        Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-05-05 14:00 +0200
          Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-06 00:00 +0200
            Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-05-06 00:10 +0200
              Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-05-11 01:40 +0200
                Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Al Viro <viro@ZenIV.linux.org.uk> - 2016-05-11 02:40 +0200
                Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Al Viro <viro@ZenIV.linux.org.uk> - 2016-05-11 03:00 +0200
                  Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-05-11 05:50 +0200
                Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-11 18:50 +0200
                  Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-05-11 20:40 +0200
    Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Serge Hallyn <serge.hallyn@ubuntu.com> - 2016-05-05 01:40 +0200
      Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-06 16:40 +0200
        Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Serge Hallyn <serge.hallyn@ubuntu.com> - 2016-05-09 18:30 +0200
          Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-10 12:40 +0200
    Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Dave Chinner <david@fromorbit.com> - 2016-05-05 02:30 +0200
      Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Andy Lutomirski <luto@amacapital.net> - 2016-05-05 03:50 +0200
        Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Dave Chinner <david@fromorbit.com> - 2016-05-05 04:30 +0200
          Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Andy Lutomirski <luto@amacapital.net> - 2016-05-05 05:40 +0200
        Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-06 00:40 +0200
      Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Djalal Harouni <tixxdz@gmail.com> - 2016-05-06 00:30 +0200
        Re: [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems Dave Chinner <david@fromorbit.com> - 2016-05-06 05:00 +0200

Page 1 of 2  [1] 2  Next page →

#1394399 — [RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems

This is version 2 of the VFS:userns support portable root filesystems
RFC. Changes since version 1:

* Update documentation and remove some ambiguity about the feature.
  Based on Josh Triplett comments.
* Use a new email address to send the RFC :-)


This RFC tries to explore how to support filesystem operations inside
user namespace using only VFS and a per mount namespace solution. This
allows to take advantage of user namespace separations without
introducing any change at the filesystems level. All this is handled
with the virtual view of mount namespaces.


1) Presentation:
================

The main aim is to support portable root filesystems and allow containers,
virtual machines and other cases to use the same root filesystem.
Due to security reasons, filesystems can't be mounted inside user
namespaces, and mounting them outside will not solve the problem since
they will show up with the wrong UIDs/GIDs. Read and write operations
will also fail and so on.

The current userspace solution is to automatically chown the whole root
filesystem before starting a container, example:
(host) init_user_ns  1000000:1065536  => (container) user_ns_X1 0:65535
(host) init_user_ns  2000000:2065536  => (container) user_ns_Y1 0:65535
(host) init_user_ns  3000000:3065536  => (container) user_ns_Z1 0:65535
...

Every time a chown is called, files are changed and so on... This
prevents to have portable filesystems where you can throw anywhere
and boot. Having an extra step to adapt the filesystem to the current
mapping and persist it will not allow to verify its integrity, it makes
snapshots and migration a bit harder, and probably other limitations...

It seems that there are multiple ways to allow user namespaces combine
nicely with filesystems, but none of them is that easy. The bind mount
and pin the user namespace during mount time will not work, bind mounts
share the same super block, hence you may endup working on the wrong
vfsmount context and there is no easy way to get out of that...

Using the user namespace in the super block seems the way to go, and
there is the "Support fuse mounts in user namespaces" [1] patches which
seem nice but perhaps too complex!? there is also the overlayfs solution,
and finaly the VFS layer solution.


We present here a simple VFS solution, everything is packed inside VFS,
filesystems don't need to know anything (except probably XFS, and special
operations inside union filesystems). Currently it supports ext4, btrfs
and overlayfs. Changes into filesystems are small, just parse the
vfs_shift_uids and vfs_shift_gids options during mount and set the
appropriate flags into the super_block structure.

1) Filesystems don't need the FS_USERNS_MOUNT flag, so no user
namespace mounting, they stay secure, nothing changes.

2) The solution is based on VFS and mount namespaces, we use the user
namespace of the containing mount namespace to check if we should shift
UIDs/GIDs from/to virtual <=> on-disk view.
If a filesystem was mounted with "vfs_shift_uids" and "vfs_shift_gids"
options, and if it shows up inside a mount namespace that supports VFS
UIDs/GIDs shifts then during each access we will remap UID/GID either
to virtual or to on-disk view using simple helper functions to allow the
access. In case the mount or current mount namespace do not support VFS
UID/GID shifts, we fallback to the old behaviour, no shift is performed.

3) The existing user namespace interface is the one used to do the
translation from virtual to on-disk mapping.

3) inodes will always keep their original values which reflect the
mapping inside init_user_ns which we consider the on-disk mapping.

 3.1) During access we map to the virtual view, and if the
      inode->{i_uid|i_gid} do not have a mapping in the mount namespace
      we construct one for them.

 3.2) For on-disk write we construct the appropriate kuid/kgid that
      should be stored on-disk. If they have a mapping in the mount
      namespace we use the corresponding uid_t/gid_t values of that
      mapping inside the mount namespace and construct the kuid from
      the pair init_user_ns and uid_t. This covers cases where the
      mapping inside should be the one stored into on-disk. Now If they
      don't have a mapping in the mount namespace, we fallback to the
      old behaviour, the global kuid inside init_user_ns is the one
      used to update the inode->i_uid.


As an example if the mapping 0:65535 inside mount namespace and outside
is 1000000:1065536, then 0:65535 will be the range that we use to
construct UIDs/GIDs mapping into init_user_ns and use it for on-disk
data. They represent the persistent values that we want to write to the
disk. Therefore, we don't keep track of any UID/GID shift that was applied
before, it gives portability and allows to use the previous mapping
which was freed for another root filesystem...

If the mapping inside the mount namespace is 1000:65535 and outside
is 2000:65535 then the range used to construct UIDs/GIDs mapping to
update inode->{i_uid|i_gid} will be the one inside the container, we
always use that one to construct the kuid/kgid from uid_t/gid_t and
init_user_ns.

$ cat /proc/self/uid_map 
      1000      2000      65536
$ stat -c '%u:%g' mountpoint/etc/fedora-release 
65534:65534
$ stat -c '%u:%g' mountpoint/home/tixxdz/
1000:1000
$ touch mountpoint/newuser  
touch: cannot touch ‘mountpoint/newuser’: Permission denied
$ stat -c '%u:%g' mountpoint/home/tixxdz/newuser
1000:1000
[ outside of namespaces] $ stat -c '%u:%g' mountpoint/home/tixxdz/newuser
1000:1000

Please note that the range here is not hardcoded to 65535, it can be any
value set by the creator of the user namespace. These patches use the
only interface user namespaces provide. 2**16 was used here to just show
how filesystems can be made portable by making the most used UIDs/GIDs
available inside containers.

Simple demo overlayfs, and  btrfs mounted with vfs_shift_uids and
vfs_shift_gids. The overlayfs mounts will share the same upperdir. We
create two user namesapces every one with its own mapping and where
container-uid-2000000 will pull changes from container-uid-1000000
upperdir automatically.

[tixxdz@fedora-kvm btrfs_root]$ mount | grep btrfs
/dev/mapper/fedora-btrfs_root on /mnt/btrfs_root type btrfs (rw,relatime,seclabel,space_cache,vfs_shift_uids,vfs_shift_gids,subvolid=5,subvol=/)
[tixxdz@fedora-kvm btrfs_root]$ sudo mount -t overlay overlay \
-o,lowerdir=/mnt/btrfs_root/rootfs/fedora-tree,upperdir=/mnt/btrfs_root/container-uid-1000000/upperdir,workdir=/mnt/btrfs_root/container-uid-1000000/workdir \
/mnt/btrfs_root/container-uid-1000000/merged
[tixxdz@fedora-kvm btrfs_root]$ sudo mount -t overlay overlay \
-o,lowerdir=/mnt/btrfs_root/rootfs/fedora-tree,upperdir=/mnt/btrfs_root/container-uid-1000000/upperdir,workdir=/mnt/btrfs_root/container-uid-2000000/workdir \
/mnt/btrfs_root/container-uid-2000000/merged
[tixxdz@fedora-kvm btrfs_root]$ sudo chown -R 1000000.1000000 /mnt/btrfs_root/container-uid-1000000/workdir/work/
[tixxdz@fedora-kvm btrfs_root]$ sudo chown -R 2000000.2000000 /mnt/btrfs_root/container-uid-2000000/workdir/work/

[ Term 1 ]
[tixxdz@fedora-kvm container-uid-1000000]$ sudo ~/bin/mountns-uidshift -u 1000000 
bash: /root/.bashrc: Permission denied
bash-4.3# cat /proc/self/uid_map 
         0    1000000      65536
bash-4.3# touch container-uid-1000000/merged/rootfile
bash-4.3# stat -c '%u:%g' container-uid-1000000/merged/rootfile
0:0

[ Term 2 ]
[tixxdz@fedora-kvm btrfs_root]$ sudo ~/bin/mountns-uidshift -u 2000000 
[sudo] password for tixxdz: 
bash: /root/.bashrc: Permission denied
bash-4.3# cat /proc/self/uid_map 
         0    2000000      65536
bash-4.3# stat -c '%u:%g' container-uid-2000000/merged/rootfile
0:0

[ Term 3 ] (outside of all namespaces)
[tixxdz@fedora-kvm btrfs_root]$ stat -c '%u:%g' container-uid-1000000/upperdir/rootfile 
0:0


This means that root in user namespace or inside containers is able to
write inodes with uid/gid == 0 into disk. This may sound strange and
dangerous, yes of course, care must be taken, this way we have added
the following:

1) Filesystems when mounted must explicitly support "vfs_shift_uids"
and "vfs_shift_gids", we don't require mounting inside user namespaces.

2) Containers or mounts can have their parent directory as 0700, and
even before mounting clean the mount namespace, set the appropriate
propagation flags and so on...

3) To be able to set the CLONE_MNTNS_SHIFT_UIDGID flag on the new mount
namespace either caller has to be real root in init_user_ns, or the parent
of the new mount namespace has already that flag set. This allows
nesting which I discussed briefly with Serge Hallyn, and he suggested
that this should be supported. Preventing nesting is doomed to fail. This
way we have security and nesting at the same time. Of course if you clean
that flag you won't be able to set it next time only if you are capable
in init_user_ns.

4) If the mount namespaces has the flag CLONE_MNTNS_SHIFT_UIDGID set but
the filesystem was mounted without "vfs_shift_uids" and "vfs_shift_gids"
or does not support these options, then  no shifting is performed. You
have to meet the two conditions at each access, otherwise we fallback to
current behaviour.

5) Only the creator of the mount namespace or one with similar
privileges is able to change the mapping rules of the user namespace of
that mount namespace. This ensures that only a more privileged is able
to change the mapping and at the same time it gives some flexibility
since the rules can be changed, and we never persist the virtual
UIDs/GIDs into disk, only the view in init_user_ns is always stored into
disk.


To complete this solution the current blocker is: since we need a way to
control mount namespaces we need a new flag, however all flags of current
clone() syscall are consumed, yes 32bits no luck! In this RFC I didn't
include a new syscall clone4() [2] which was already requested in the past,
and the patches for a new clone4() are already there. This way this RFC
stays minimal.

The flag we use here is just for demonstration, please see patch 0001
and the program mountns-uidshift.c [3] for that. Future versions
will include the new clone4() syscall.


2) TEST:
========

Apply on top of Linux 4.6-rc6 HEAD 04974df8049fc4240d2275, and use this
program mountns-uidshift.c to test the shifted mount namespaces.
https://raw.githubusercontent.com/OpenDZ/research/master/kernel/mountns-uidshift.c

With current mapping rules init_user_ns:
[1000000:1065536] => new_user_ns: [0:65536]
# cat /proc/self/uid_map 
         0    1000000      65536
# cat /proc/self/gid_map 
         0    1000000      65536


2.1) ext4:
==========

Setup:
/   		on ext4 without vfs_shift_uids, vfs_shift_gids
/mnt/ext4_root	on ext4 with vfs_shift_uids, vfs_shift_gids
/mnt/ext4_root/rootfs/fedore-tree		(Another fedora rootfs)
/mnt/ext4_root/container-uid-1000000		(container files with uid 1000000)
/mnt/ext4_root/container-uid-1000000/mountpoint	(bind mount of fedora-tree)

$ sudo mount -t ext4 -ovfs_shift_uids,vfs_shift_gids \
/dev/fedora/ext4_root /mnt/ext4_root/
$ mount | grep ext4 -
/dev/mapper/fedora-root on / type ext4 (rw,relatime,seclabel,data=ordered)
/dev/sda1 on /boot type ext4 (rw,relatime,seclabel,data=ordered)
/dev/mapper/fedora-ext4_root on /mnt/ext4_root type ext4 (rw,relatime,seclabel,data=ordered,vfs_shift_uids,vfs_shift_gids)
$ sudo mkdir /mnt/ext4_root/rootfs/
$ sudo yum -y --releasever=23 --installroot=/mnt/ext4_root/rootfs/fedora-tree \
--disablerepo='*' --enablerepo=fedora install systemd passwd yum fedora-release vim
$ sudo mkdir /mnt/ext4_root/container-uid-1000000/
$ sudo mkdir /mnt/ext4_root/container-uid-1000000/mountpoint
$ sudo chown -R 1000000.1000000 /mnt/ext4_root/container-uid-1000000/
$ sudo mount --bind -ovfs_shift_uids,vfs_shift_gids \
/mnt/ext4_root/rootfs/fedora-tree/ mountpoint/
$ mount | grep vfs_shift_uids -
/dev/mapper/fedora-ext4_root on /mnt/ext4_root type ext4 (rw,relatime,seclabel,data=ordered,vfs_shift_uids,vfs_shift_gids)
/dev/mapper/fedora-ext4_root on /mnt/ext4_root/container-uid-1000000/mountpoint type ext4 (rw,relatime,seclabel,data=ordered,vfs_shift_uids,vfs_shift_gids)
$ sudo ~/bin/mountns-uidshift -u 1000000
...
bash-4.3# id
uid=0(root) gid=0(root) groups=0(root) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
bash-4.3# cat /proc/self/uid_map 
         0    1000000      65536
bash-4.3# mount | grep shift -           
/dev/mapper/fedora-ext4_root on /mnt/ext4_root type ext4 (rw,relatime,seclabel,data=ordered,vfs_shift_uids,vfs_shift_gids)
/dev/mapper/fedora-ext4_root on /mnt/ext4_root/container-uid-1000000/mountpoint type ext4 (rw,relatime,seclabel,data=ordered,vfs_shift_uids,vfs_shift_gids)
bash-4.3# stat -c '%u:%g' /etc/motd 
65534:65534
bash-4.3# stat -c '%u:%g' /mnt/ext4_root/rootfs/fedora-tree/etc/motd 
0:0
bash-4.3# stat -c '%u:%g' mountpoint/etc/motd                        
0:0
bash-4.3# stat -c '%u:%g' /etc/machine-id 
65534:65534
bash-4.3# echo "blabla" > /etc/machine-id                                                                      
bash: /etc/machine-id: Permission denied
bash-4.3# stat -c '%u:%g' mountpoint/etc/machine-id 
0:0
bash-4.3# sha1sum mountpoint/etc/machine-id 
edb24591988f0f003cd397704f49e92208b3015f  mountpoint/etc/machine-id
bash-4.3# m=$(cat /dev/urandom | tr -cd 'a-f0-9' | head -c 32); echo $m | sha1sum; echo $m > mountpoint/etc/machine-id 
f256a796b1f2ed09c4107f1f5aff2568fb2d79cc  -
bash-4.3# sha1sum mountpoint/etc/machine-id                                                                            
f256a796b1f2ed09c4107f1f5aff2568fb2d79cc  mountpoint/etc/machine-id
bash-4.3# stat -c '%u:%g' mountpoint/etc/machine-id                                                                    
0:0
[outside of namespaces]$ stat -c '%u:%g' /mnt/ext4_root/container-uid-1000000/mountpoint/etc/machine-id
0:0

Test with unprivileged user inside the new mount and user namespaces:
---------------------------------------------------------------------

Test with uid tixxdz == 1000, the user exists on both:
(1) /
(2) /mnt/ext4_root/rootfs/fedore-tree which is bind mounted into /mnt/ext4_root/container-uid-1000000/mountpoint

-bash-4.3$ touch /home/tixxdz/newfile
touch: cannot touch /home/tixxdz/newfile: Permission denied
-bash-4.3$ stat -c '%u:%g' /home/tixxdz/
65534:65534
-bash-4.3$ stat -c '%u:%g' mountpoint/home/tixxdz/
1000:1000
-bash-4.3$ touch mountpoint/home/tixxdz/newfile
-bash-4.3$ stat -c '%u:%g' mountpoint/home/tixxdz/newfile 
1000:1000
[outside of namespaces]$ stat -c '%u:%g' /mnt/ext4_root/container-uid-1000000/mountpoint/home/tixxdz/newfile 
1000:1000


2.2) btrfs:
===========
Same steps as ext4.


2.3) overlayfs:
===============

2.3.1) Native support using VFS:

Overlayfs is natively supported if lowerdir, upperdir and workdir are all
on a mount that supports vfs_shift_uids and vfs_shift_gids flags and we
are in a mount namespace that also supports that.

$ mount | grep btrfs
/dev/mapper/fedora-btrfs_root on /mnt/btrfs_root type btrfs (rw,relatime,seclabel,space_cache,vfs_shift_uids,vfs_shift_gids,subvolid=5,subvol=/)
$ cd /mnt/btrfs_root/
$ sudo mkdir -p container-uid-2000000/{upperdir,workdir,merged}
$ sudo chown -R 2000000.2000000 container-uid-2000000/
$ cd container-uid-2000000/
$ sudo mount -t overlay overlay -o,lowerdir=/mnt/btrfs_root/rootfs/fedora-tree,upperdir=upperdir,workdir=workdir merged
$ sudo chown -R 2000000.2000000 workdir/work/
$ sudo ~/bin/mountns-uidshift -u 2000000
...
bash-4.3# stat -c '%u:%g' merged/etc/passwd
0:0
bash-4.3# touch merged/overlayfs-file
bash-4.3# stat -c '%u:%g' merged/overlayfs-file 
0:0
[outside of namespaces]# stat -c '%u:%g' /mnt/btrfs_root/container-uid-2000000/merged/overlayfs-file 
0:0
[outside of namespaces]# stat -c '%u:%g' /mnt/btrfs_root/container-uid-2000000/upperdir/overlayfs-file 
0:0


2.3.2) Complex support or union filesystems:

If overlayfs lowerdir and upperdir are not on a filesystem that supports
natively vfs_shift_uids and vfs_shift_gids then to support VFS UID/GID
shifts, we must adapt the helper functions that where introduced in this
series to take also a super_block struct and test if the appropriate flags
where set into overlayfs instead of the other filesystem which the inode
belongs to. The translation on-disk <=> virtual should happen then inside
overlayfs.

I think this will always be the case of union mounts which fetch an inode
from another mount. I think that solution (2.3.2) can also be implemented,
I had some ugly patches to implement this on top of overlayfs, but not
sure, better see what others think about VFS UID/GID shifts first.

IMO solution (2.3.1) if done correctly is the way to go, in the end all
this relates to the virtual view of UID/GID inside the kernel, and how
resources are translated to them, it's not related to overlayfs.


3) ROADMAP:
===========
* Confirm current design, and make sure that the mapping is done
  correctly.

* Add clone4() syscall [2]

* Investigate if current setns() checks to enter new mount namespaces
  are sufficient ?

* Add POSIX ACL support ?

* Check if all filesystem operations are correctly supported and recheck
  permissions access.

* Do filesystems provide some operations to control disk or host resources ?
  in other words are there some inodes on filesystems that allow to access
  host resources, if so then maybe these inodes either should be marked only
  safe in init_user_ns or get the appropriate capable() in init_user_ns if
  missing. Needs investigation.

* Add XFS support.



References:
===========
[1] https://www.redhat.com/archives/dm-devel/2016-April/msg00368.html
[2] https://lkml.org/lkml/2015/3/15/10
[3] https://raw.githubusercontent.com/OpenDZ/research/master/kernel/mountns-uidshift.c

Thanks!

Patches:
[RFC v2 PATCH 0/8] VFS:userns: support portable root filesystems
[RFC v2 PATCH 1/8] VFS: add CLONE_MNTNS_SHIFT_UIDGID flag to allow mounts to shift their UIDs/GIDs
[RFC v2 PATCH 2/8] VFS:uidshift: add flags and helpers to shift UIDs and GIDs to virtual view
[RFC v2 PATCH 3/8] fs: Treat foreign mounts as nosuid
[RFC v2 PATCH 4/8] VFS:userns: shift UID/GID to virtual view during permission access
[RFC v2 PATCH 5/8] VFS:userns: add helpers to shift UIDs and GIDs into on-disk view
[RFC v2 PATCH 6/8] VFS:userns: shift UID/GID to on-disk view before any write to disk
[RFC v2 PATCH 7/8] ext4: add support for vfs_shift_uids and vfs_shift_gids mount options
[RFC v2 PATCH 8/8] btrfs: add support for vfs_shift_uids and vfs_shift_gids mount options


Diffstat for this RFC
fs/attr.c                      |  44 +++++++++++++++++++++++--------
fs/btrfs/super.c               |  15 ++++++++++-
fs/exec.c                      |   2 +-
fs/ext4/super.c                |  14 ++++++++++
fs/inode.c                     |   9 ++++---
fs/mount.h                     |   1 +
fs/namei.c                     |   6 +++--
fs/namespace.c                 | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/stat.c                      |   4 +--
include/linux/fs.h             |  14 ++++++++++
include/linux/mount.h          |   1 +
include/linux/user_namespace.h |   8 ++++++
include/uapi/linux/sched.h     |   1 +
kernel/capability.c            |  14 ++++++++--
kernel/fork.c                  |   4 +++
kernel/user_namespace.c        |  13 ++++++++++
security/commoncap.c           |   2 +-
security/selinux/hooks.c       |   2 +-
18 files changed, 319 insertions(+), 25 deletions(-)

[toc] | [next] | [standalone]

#1394407 — [RFC v2 PATCH 1/8] VFS: add CLONE_MNTNS_SHIFT_UIDGID flag to allow mounts to shift their UIDs/GIDs

Add CLONE_MNTNS_SHIFT_UIDGID flag which is a mount namespace flag when
set mount points on filesystems that support UID/GID shifts will have
their UIDs and GIDs shifted by the VFS. The UID and GID mapping rules are per
mount namespace, they follow the rules of the user namespace of the containing
mount namespace. The UID/GID of inodes are supposed to always contain
the on-disk values, hence, the shift will be done inside VFS and it's a read
shift when we access the inodes.

This is a preparation patch.

Goal:

	/* (1) */
	clone4(CLONE_NEWNS|CLONE_MNTNS_SHIFT_UIDGID, ...)
	/*
		Setup container base mount namespace, rootfs and mount all
		necessary mount points and filesystems that can't be mounted
		in user namespaces. Filesystems that support uid/gid shifts
		should set the mount parameters.
		mount(..., mount_options=[vfs_shift_uids, vfs_shift_gids])
	*/

	/* (2) */
	/*
		Setup new mount and user namespaces and inherit the
		CLONE_MNTNS_SHIFT_UIDGID flag from (1) into the new mount
		namespace (2).
	*/
	clone4(CLONE_NEWUSER|CLONE_NEWNS|CLONE_MNTNS_SHIFT_UIDGID, ...)
	/*
	   inodes of mount points here that support UID/GID shifts will have
	   automatically their UID/GID shifted according to the user
	   namespace rules of the current mount namespace (2).
	*/

We create the new user and mount namespaces where:
1) The mount namespace allows mounts inside it that support UID and GID
   shifting to perform the shifts if the CLONE_MNTNS_SHIFT_UIDGID is set
   in the current mount namespace.

2) The UID and GID mapping is done according to the rules of the user
   namespace of the containing mount namespace. The CLONE_MNTNS_SHIFT_UIDGID
   follows the CLONE_NEWUSER|CLONE_NEWNS combination. This ensures that
   only the creator of the mount namespace is able to adjust the user
   namespace mapping rules.

The flag CLONE_MNTNS_SHIFT_UIDGID can be set on the mount namespace
only if:

1) The parent namespace has already CLONE_MNTNS_SHIFT_UIDGID set on
   its mount namespace.

2) The caller has CAP_SYS_ADMIN in the init_user_ns namespace, since we
   start from that namespace and we inherit some mount points we have to
   protect files from privileged userns doing:
   clone(CLONE_NEWUSER|CLONE_NEWNS|CLONE_MNTNS_SHIFT_UIDGID...)
   This is blocked.

If a filesystem was mounted with "vfs_shift_uids" and "vfs_shift_gids"
and shows up in a mount namespace that does not include the
CLONE_MNTNS_SHIFT_UIDGID, then no shift is done. UIDs and GIDs will
not be changed at all, and things will continue to work as they are now.

Signed-off-by: Dongsu Park <dongsu@endocode.com>
Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
---
 fs/mount.h                 |  1 +
 fs/namespace.c             | 20 ++++++++++++++++++++
 include/uapi/linux/sched.h |  1 +
 kernel/fork.c              |  4 ++++
 4 files changed, 26 insertions(+)

diff --git a/fs/mount.h b/fs/mount.h
index 14db05d..1e317eb 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -6,6 +6,7 @@
 
 struct mnt_namespace {
 	atomic_t		count;
+	int			flags;
 	struct ns_common	ns;
 	struct mount *	root;
 	struct list_head	list;
diff --git a/fs/namespace.c b/fs/namespace.c
index 4fb1691..940ecfc 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2774,6 +2774,7 @@ static struct mnt_namespace *alloc_mnt_ns(struct user_namespace *user_ns)
 	INIT_LIST_HEAD(&new_ns->list);
 	init_waitqueue_head(&new_ns->poll);
 	new_ns->event = 0;
+	new_ns->flags = 0;
 	new_ns->user_ns = get_user_ns(user_ns);
 	return new_ns;
 }
@@ -2801,6 +2802,25 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 	if (IS_ERR(new_ns))
 		return new_ns;
 
+	if (flags & CLONE_MNTNS_SHIFT_UIDGID) {
+		/*
+		 * If parent has the CLONE_MNTNS_SHIFT_UIDGID flag set
+		 * or current is capable in init_user_ns, then we set the
+		 * CLONE_MNTNS_SHIFT_UIDGID flag and allow mounts inside
+		 * this namespace to shift their UID and GID.
+		 *
+		 * We check the init_user_ns here since we always start from
+		 * that user namespace and mounts are by default available to all
+		 * users. In this regard, only CAP_SYS_ADMIN in init_user_ns is
+		 * allowed to start and propagate the CLONE_MNTNS_SHIFT_UIDGID
+		 * flag to new mount namespaces.
+		 */
+		if ((ns->flags & CLONE_MNTNS_SHIFT_UIDGID) || capable(CAP_SYS_ADMIN))
+			new_ns->flags |= CLONE_MNTNS_SHIFT_UIDGID;
+		else
+			return ERR_PTR(-EPERM);
+	}
+
 	namespace_lock();
 	/* First pass: copy the tree topology */
 	copy_flags = CL_COPY_UNBINDABLE | CL_EXPIRE;
diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index 5f0fe01..9ba2124 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -19,6 +19,7 @@
 #define CLONE_PARENT_SETTID	0x00100000	/* set the TID in the parent */
 #define CLONE_CHILD_CLEARTID	0x00200000	/* clear the TID in the child */
 #define CLONE_DETACHED		0x00400000	/* Unused, ignored */
+#define CLONE_MNTNS_SHIFT_UIDGID     0x00400000      /* If set allows to shift UID and GID for mounts that support it */
 #define CLONE_UNTRACED		0x00800000	/* set if the tracing process can't force CLONE_PTRACE on this clone */
 #define CLONE_CHILD_SETTID	0x01000000	/* set the TID in the child */
 #define CLONE_NEWCGROUP		0x02000000	/* New cgroup namespace */
diff --git a/kernel/fork.c b/kernel/fork.c
index d277e83..41223cd 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1264,6 +1264,10 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS))
 		return ERR_PTR(-EINVAL);
 
+	if ((clone_flags & CLONE_MNTNS_SHIFT_UIDGID) &&
+	    !(clone_flags & CLONE_NEWNS))
+		return ERR_PTR(-EINVAL);
+
 	if ((clone_flags & (CLONE_NEWUSER|CLONE_FS)) == (CLONE_NEWUSER|CLONE_FS))
 		return ERR_PTR(-EINVAL);
 
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1394535

On Wed, May 04, 2016 at 04:26:46PM +0200, Djalal Harouni wrote:
> This is version 2 of the VFS:userns support portable root filesystems
> RFC. Changes since version 1:
> 
> * Update documentation and remove some ambiguity about the feature.
>   Based on Josh Triplett comments.

Thanks for the clarifications.

> 3) The existing user namespace interface is the one used to do the
> translation from virtual to on-disk mapping.

This makes sense.  Even if in the future we had a way to supply an
arbitrary VFS UID/GID mapping for a mount, independent of the userns,
what you've proposed would still make sense as a shorthand for the
common case of using the same mapping for both userns and VFS.

- Josh Triplett

[toc] | [prev] | [next] | [standalone]

#1394722

On Wed, 2016-05-04 at 16:26 +0200, Djalal Harouni wrote:
> This is version 2 of the VFS:userns support portable root filesystems
> RFC. Changes since version 1:
> 
> * Update documentation and remove some ambiguity about the feature.
>   Based on Josh Triplett comments.
> * Use a new email address to send the RFC :-)
> 
> 
> This RFC tries to explore how to support filesystem operations inside
> user namespace using only VFS and a per mount namespace solution.
> This
> allows to take advantage of user namespace separations without
> introducing any change at the filesystems level. All this is handled
> with the virtual view of mount namespaces.
> 
> 
> 1) Presentation:
> ================
> 
> The main aim is to support portable root filesystems and allow 
> containers, virtual machines and other cases to use the same root 
> filesystem. Due to security reasons, filesystems can't be mounted 
> inside user namespaces, and mounting them outside will not solve the 
> problem since they will show up with the wrong UIDs/GIDs. Read and 
> write operations will also fail and so on.
> 
> The current userspace solution is to automatically chown the whole 
> root filesystem before starting a container, example:
> (host) init_user_ns  1000000:1065536  => (container) user_ns_X1
> 0:65535
> (host) init_user_ns  2000000:2065536  => (container) user_ns_Y1
> 0:65535
> (host) init_user_ns  3000000:3065536  => (container) user_ns_Z1
> 0:65535
> ...
> 
> Every time a chown is called, files are changed and so on... This
> prevents to have portable filesystems where you can throw anywhere
> and boot. Having an extra step to adapt the filesystem to the current
> mapping and persist it will not allow to verify its integrity, it 
> makes snapshots and migration a bit harder, and probably other
> limitations...
> 
> It seems that there are multiple ways to allow user namespaces 
> combine nicely with filesystems, but none of them is that easy. The 
> bind mount and pin the user namespace during mount time will not 
> work, bind mounts share the same super block, hence you may endup 
> working on the wrong vfsmount context and there is no easy way to get
> out of that...

So this option was discussed at the recent LSF/MM summit.  The most
supported suggestion was that you'd use a new internal fs type that had
a struct mount with a new superblock and would copy the underlying
inodes but substitute it's own with modified ->getatrr/->setattr calls
that did the uid shift.  In many ways it would be a remapping bind
which would look similar to overlayfs but be a lot simpler.

> Using the user namespace in the super block seems the way to go, and
> there is the "Support fuse mounts in user namespaces" [1] patches 
> which seem nice but perhaps too complex!?

So I don't think that does what you want.  The fuse project I've used
before to do uid/gid shifts for build containers is bindfs

https://github.com/mpartel/bindfs/

It allows a --map argument where you specify pairs of uids/gids to map
(tedious for large ranges, but the map can be fixed to use uid:range
instead of individual).

>  there is also the overlayfs solution, and finaly the VFS layer 
> solution.
> 
> We present here a simple VFS solution, everything is packed inside 
> VFS, filesystems don't need to know anything (except probably XFS, 
> and special operations inside union filesystems). Currently it 
> supports ext4, btrfs and overlayfs. Changes into filesystems are 
> small, just parse the vfs_shift_uids and vfs_shift_gids options 
> during mount and set the appropriate flags into the super_block
> structure.

So this looks a little daunting.  It sprays the VFS with knowledge
about the shifts and requires support from every underlying filesystem.
 A simple remapping bind filesystem would be a lot simpler and require
no underlying filesystem support.

James

[toc] | [prev] | [next] | [standalone]

#1394910

On Wed, May 04, 2016 at 05:06:19PM -0400, James Bottomley wrote:
> On Wed, 2016-05-04 at 16:26 +0200, Djalal Harouni wrote:
> > This is version 2 of the VFS:userns support portable root filesystems
> > RFC. Changes since version 1:
> > 
> > * Update documentation and remove some ambiguity about the feature.
> >   Based on Josh Triplett comments.
> > * Use a new email address to send the RFC :-)
> > 
> > 
> > This RFC tries to explore how to support filesystem operations inside
> > user namespace using only VFS and a per mount namespace solution.
> > This
> > allows to take advantage of user namespace separations without
> > introducing any change at the filesystems level. All this is handled
> > with the virtual view of mount namespaces.
> > 
> > 
> > 1) Presentation:
> > ================
> > 
> > The main aim is to support portable root filesystems and allow 
> > containers, virtual machines and other cases to use the same root 
> > filesystem. Due to security reasons, filesystems can't be mounted 
> > inside user namespaces, and mounting them outside will not solve the 
> > problem since they will show up with the wrong UIDs/GIDs. Read and 
> > write operations will also fail and so on.
> > 
> > The current userspace solution is to automatically chown the whole 
> > root filesystem before starting a container, example:
> > (host) init_user_ns  1000000:1065536  => (container) user_ns_X1
> > 0:65535
> > (host) init_user_ns  2000000:2065536  => (container) user_ns_Y1
> > 0:65535
> > (host) init_user_ns  3000000:3065536  => (container) user_ns_Z1
> > 0:65535
> > ...
> > 
> > Every time a chown is called, files are changed and so on... This
> > prevents to have portable filesystems where you can throw anywhere
> > and boot. Having an extra step to adapt the filesystem to the current
> > mapping and persist it will not allow to verify its integrity, it 
> > makes snapshots and migration a bit harder, and probably other
> > limitations...
> > 
> > It seems that there are multiple ways to allow user namespaces 
> > combine nicely with filesystems, but none of them is that easy. The 
> > bind mount and pin the user namespace during mount time will not 
> > work, bind mounts share the same super block, hence you may endup 
> > working on the wrong vfsmount context and there is no easy way to get
> > out of that...
> 
> So this option was discussed at the recent LSF/MM summit.  The most
> supported suggestion was that you'd use a new internal fs type that had
> a struct mount with a new superblock and would copy the underlying
> inodes but substitute it's own with modified ->getatrr/->setattr calls
> that did the uid shift.  In many ways it would be a remapping bind
> which would look similar to overlayfs but be a lot simpler.

Hmm, it's not only about ->getattr and ->setattr, you have all the other
file system operations that need access too... which brings two points:

1) This new internal fs may end up doing what this RFC does...

2) or by quoting "new internal fs + its own super block + copy underlying
inodes..." it seems like another overlayfs where you also need some
decisions to copy what, etc. So, will this be really
that light compared to current overlayfs ? not to mention that you need
to hook up basically the same logic or something else inside overlayfs..

> > Using the user namespace in the super block seems the way to go, and
> > there is the "Support fuse mounts in user namespaces" [1] patches 
> > which seem nice but perhaps too complex!?
> 
> So I don't think that does what you want.  The fuse project I've used
> before to do uid/gid shifts for build containers is bindfs
> 
> https://github.com/mpartel/bindfs/
> 
> It allows a --map argument where you specify pairs of uids/gids to map
> (tedious for large ranges, but the map can be fixed to use uid:range
> instead of individual).

Ok, thanks for the link, will try to take a deep look but bindfs seem
really big!

> >  there is also the overlayfs solution, and finaly the VFS layer 
> > solution.
> > 
> > We present here a simple VFS solution, everything is packed inside 
> > VFS, filesystems don't need to know anything (except probably XFS, 
> > and special operations inside union filesystems). Currently it 
> > supports ext4, btrfs and overlayfs. Changes into filesystems are 
> > small, just parse the vfs_shift_uids and vfs_shift_gids options 
> > during mount and set the appropriate flags into the super_block
> > structure.
> 
> So this looks a little daunting.  It sprays the VFS with knowledge
> about the shifts and requires support from every underlying filesystem.
Well, from my angle, shifts are just user namespace mappings which
follow certain rules, and currently VFS and all filesystems are *already*
doing some kind of shifting... This RFC uses mount namespaces which are
the standard way to deal with mounts, now the mapping inside mount
namespace can just be "inside: 0:1000" => "outside: 0:1000"
and current implementation will just use it, at the same time I'm not
sure if this mapping qualifies to be named "shift". I think that some
folks here came up with the "shift" name to describe one of the use cases
from a user interface that's it... maybe I should do
s/vfs_shift_*/vfs_remap_*/ ?


>  A simple remapping bind filesystem would be a lot simpler and require
> no underlying filesystem support.
Yes probably, you still need to parse parameters but not at the
filesystem level, and sure this RFC can do the same of course, but maybe
it's not safe to shift/remap filesystems and their inodes on behalf of
filesystems... and virtual filesystems which can share inodes ?


> James
> 

Thank you!

-- 
Djalal Harouni
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1395060

On Thu, 2016-05-05 at 08:36 +0100, Djalal Harouni wrote:
> On Wed, May 04, 2016 at 05:06:19PM -0400, James Bottomley wrote:
> > On Wed, 2016-05-04 at 16:26 +0200, Djalal Harouni wrote:
> > > This is version 2 of the VFS:userns support portable root
> > > filesystems
> > > RFC. Changes since version 1:
> > > 
> > > * Update documentation and remove some ambiguity about the
> > > feature.
> > >   Based on Josh Triplett comments.
> > > * Use a new email address to send the RFC :-)
> > > 
> > > 
> > > This RFC tries to explore how to support filesystem operations 
> > > inside user namespace using only VFS and a per mount namespace 
> > > solution. This allows to take advantage of user namespace 
> > > separations without introducing any change at the filesystems 
> > > level. All this is handled with the virtual view of mount
> > > namespaces.
> > > 
> > > 
> > > 1) Presentation:
> > > ================
> > > 
> > > The main aim is to support portable root filesystems and allow 
> > > containers, virtual machines and other cases to use the same root
> > > filesystem. Due to security reasons, filesystems can't be mounted
> > > inside user namespaces, and mounting them outside will not solve 
> > > the problem since they will show up with the wrong UIDs/GIDs. 
> > > Read and write operations will also fail and so on.
> > > 
> > > The current userspace solution is to automatically chown the 
> > > whole root filesystem before starting a container, example:
> > > (host) init_user_ns  1000000:1065536  => (container) user_ns_X1
> > > 0:65535
> > > (host) init_user_ns  2000000:2065536  => (container) user_ns_Y1
> > > 0:65535
> > > (host) init_user_ns  3000000:3065536  => (container) user_ns_Z1
> > > 0:65535
> > > ...
> > > 
> > > Every time a chown is called, files are changed and so on... This
> > > prevents to have portable filesystems where you can throw 
> > > anywhere and boot. Having an extra step to adapt the filesystem 
> > > to the current mapping and persist it will not allow to verify 
> > > its integrity, it makes snapshots and migration a bit harder, and 
> > > probably other limitations...
> > > 
> > > It seems that there are multiple ways to allow user namespaces 
> > > combine nicely with filesystems, but none of them is that easy. 
> > > The bind mount and pin the user namespace during mount time will 
> > > not work, bind mounts share the same super block, hence you may 
> > > endup working on the wrong vfsmount context and there is no easy 
> > > way to get out of that...
> > 
> > So this option was discussed at the recent LSF/MM summit.  The most
> > supported suggestion was that you'd use a new internal fs type that 
> > had a struct mount with a new superblock and would copy the 
> > underlying inodes but substitute it's own with modified ->getatrr/
> > ->setattr calls that did the uid shift.  In many ways it would be a 
> > remapping bind which would look similar to overlayfs but be a lot
> > simpler.
> 
> Hmm, it's not only about ->getattr and ->setattr, you have all the 
> other file system operations that need access too...

Why?  Or perhaps we should more cogently define the actual problem.  My
problem is simply mounting image volumes that were created with real
uids at user namespace shifted uids because I'm downshifting the
privileged ids in the container.  I actually *only* need the uid/gids
on the attributes shifted because that's what I need to manipulate the
volumes.  I actually think that other operations, like the file ioctl
ones should, for security reasons, not be uid shifted.  For instance
with xfs you could set the panic mask and error tags and bring down the
whole host.  What extra things do you need access to and why?

>  which brings two points:
> 
> 1) This new internal fs may end up doing what this RFC does...

Well that was why I brought it up, yes.

> 2) or by quoting "new internal fs + its own super block + copy
> underlying inodes..." it seems like another overlayfs where you also
> need some decisions to copy what, etc. So, will this be really
> that light compared to current overlayfs ? not to mention that you 
> need to hook up basically the same logic or something else inside
> overlayfs..

OK, so forget overlayfs, perhaps that was a bad example.  It's like a
uid shifting bind.  The way it works is to use shadow inodes (unlike
bind, but because you have to intercept the operations, so it's not a
simple subtree operation) but there's no file copying.  The shadow
points to the real inode.

> > > Using the user namespace in the super block seems the way to go, 
> > > and there is the "Support fuse mounts in user namespaces" [1] 
> > > patches which seem nice but perhaps too complex!?
> > 
> > So I don't think that does what you want.  The fuse project I've 
> > used before to do uid/gid shifts for build containers is bindfs
> > 
> > https://github.com/mpartel/bindfs/
> > 
> > It allows a --map argument where you specify pairs of uids/gids to 
> > map (tedious for large ranges, but the map can be fixed to use
> > uid:range instead of individual).
> 
> Ok, thanks for the link, will try to take a deep look but bindfs seem
> really big!

Well, it does a lot more than just uid/gid shift.

> > >  there is also the overlayfs solution, and finaly the VFS layer 
> > > solution.
> > > 
> > > We present here a simple VFS solution, everything is packed 
> > > inside VFS, filesystems don't need to know anything (except 
> > > probably XFS, and special operations inside union filesystems). 
> > > Currently it supports ext4, btrfs and overlayfs. Changes into 
> > > filesystems are small, just parse the vfs_shift_uids and 
> > > vfs_shift_gids options during mount and set the appropriate flags 
> > > into the super_block structure.
> > 
> > So this looks a little daunting.  It sprays the VFS with knowledge
> > about the shifts and requires support from every underlying 
> > filesystem. 

> Well, from my angle, shifts are just user namespace mappings which
> follow certain rules, and currently VFS and all filesystems are 
> *already* doing some kind of shifting... This RFC uses mount 
> namespaces which are the standard way to deal with mounts, now the 
> mapping inside mount namespace can just be "inside: 0:1000" => 
> "outside: 0:1000" and current implementation will just use it, at the 
> same time I'm not sure if this mapping qualifies to be named "shift". 
> I think that some folks here came up with the "shift" name to 
> describe one of the use cases from a user interface that's it... 
> maybe I should do s/vfs_shift_*/vfs_remap_*/ ?

I don't think the naming is the issue ... it's the spread inside the
vfs code (and in the underlying fs code).  The vfs is very well
layered, so touching all that code makes it look like there's a
layering problem with the patch.  Touching the underlying fs code looks
even more problematic, but that may be necessary if you have a reason
for wanting the file ioctls, because they're pass through and usually
where the from_kuid() calls are in filesystems.

> >  A simple remapping bind filesystem would be a lot simpler and 
> > require no underlying filesystem support. 
>  
> Yes probably, you still need to parse parameters but not at the
> filesystem level,

They'd just be mount options.  Basically instead of mount --bind source
target, you'd do mount -t uidshift -o <shift options> source target.

>  and sure this RFC can do the same of course, but maybe it's not safe
> to shift/remap filesystems and their inodes on behalf of
> filesystems... and virtual filesystems which can share inodes ?

That depends who you allow to do the shift.  Each fstype in the kernel
decides access to mount.  For the uidshift, I was planning to allow
only a capable admin in the initial namespace, meaning that only the
admin in the host could set up the shifts.  As long as the shifted
filesystem is present, the container can then bind it wherever it wants
in its mount namespace.

James

[toc] | [prev] | [next] | [standalone]

#1395393

On Thu, May 05, 2016 at 07:56:28AM -0400, James Bottomley wrote:
> On Thu, 2016-05-05 at 08:36 +0100, Djalal Harouni wrote:
> > On Wed, May 04, 2016 at 05:06:19PM -0400, James Bottomley wrote:
> > > On Wed, 2016-05-04 at 16:26 +0200, Djalal Harouni wrote:
> > > > This is version 2 of the VFS:userns support portable root
> > > > filesystems
> > > > RFC. Changes since version 1:
> > > > 
> > > > * Update documentation and remove some ambiguity about the
> > > > feature.
> > > >   Based on Josh Triplett comments.
> > > > * Use a new email address to send the RFC :-)
> > > > 
> > > > 
> > > > This RFC tries to explore how to support filesystem operations 
> > > > inside user namespace using only VFS and a per mount namespace 
> > > > solution. This allows to take advantage of user namespace 
> > > > separations without introducing any change at the filesystems 
> > > > level. All this is handled with the virtual view of mount
> > > > namespaces.
> > > > 
> > > > 
> > > > 1) Presentation:
> > > > ================
> > > > 
> > > > The main aim is to support portable root filesystems and allow 
> > > > containers, virtual machines and other cases to use the same root
> > > > filesystem. Due to security reasons, filesystems can't be mounted
> > > > inside user namespaces, and mounting them outside will not solve 
> > > > the problem since they will show up with the wrong UIDs/GIDs. 
> > > > Read and write operations will also fail and so on.
> > > > 
> > > > The current userspace solution is to automatically chown the 
> > > > whole root filesystem before starting a container, example:
> > > > (host) init_user_ns  1000000:1065536  => (container) user_ns_X1
> > > > 0:65535
> > > > (host) init_user_ns  2000000:2065536  => (container) user_ns_Y1
> > > > 0:65535
> > > > (host) init_user_ns  3000000:3065536  => (container) user_ns_Z1
> > > > 0:65535
> > > > ...
> > > > 
> > > > Every time a chown is called, files are changed and so on... This
> > > > prevents to have portable filesystems where you can throw 
> > > > anywhere and boot. Having an extra step to adapt the filesystem 
> > > > to the current mapping and persist it will not allow to verify 
> > > > its integrity, it makes snapshots and migration a bit harder, and 
> > > > probably other limitations...
> > > > 
> > > > It seems that there are multiple ways to allow user namespaces 
> > > > combine nicely with filesystems, but none of them is that easy. 
> > > > The bind mount and pin the user namespace during mount time will 
> > > > not work, bind mounts share the same super block, hence you may 
> > > > endup working on the wrong vfsmount context and there is no easy 
> > > > way to get out of that...
> > > 
> > > So this option was discussed at the recent LSF/MM summit.  The most
> > > supported suggestion was that you'd use a new internal fs type that 
> > > had a struct mount with a new superblock and would copy the 
> > > underlying inodes but substitute it's own with modified ->getatrr/
> > > ->setattr calls that did the uid shift.  In many ways it would be a 
> > > remapping bind which would look similar to overlayfs but be a lot
> > > simpler.
> > 
> > Hmm, it's not only about ->getattr and ->setattr, you have all the 
> > other file system operations that need access too...
> 
> Why?  Or perhaps we should more cogently define the actual problem.  My
> problem is simply mounting image volumes that were created with real
> uids at user namespace shifted uids because I'm downshifting the
> privileged ids in the container.  I actually *only* need the uid/gids
> on the attributes shifted because that's what I need to manipulate the

We need them obviously for read/write/creation... ?! We want to handle
also stock filesystems that were never edited without depending on any
module or third party solution, mounting them outside user namespaces,
and access inside.

> volumes.  I actually think that other operations, like the file ioctl
> ones should, for security reasons, not be uid shifted.  For instance
> with xfs you could set the panic mask and error tags and bring down the
> whole host.  What extra things do you need access to and why?

That's why precisely I said that mounting options not *inside*
filesystems which means on their back, and on behalf of container
managers, etc then you are exposed to such scenarios... some virtual
file systems can also be mounted by unprivileged, how you will deal
with something like a bind mount on them ?


> >  which brings two points:
> > 
> > 1) This new internal fs may end up doing what this RFC does...
> 
> Well that was why I brought it up, yes.

yes but *with* extra code! that was my point. I'm not sure we need to
bother with any *new* internal fs type nor hack around dir, file
operations... yet that has to be shown, defined, coded ... ?


> > 2) or by quoting "new internal fs + its own super block + copy
> > underlying inodes..." it seems like another overlayfs where you also
> > need some decisions to copy what, etc. So, will this be really
> > that light compared to current overlayfs ? not to mention that you 
> > need to hook up basically the same logic or something else inside
> > overlayfs..
> 
> OK, so forget overlayfs, perhaps that was a bad example.  It's like a
> uid shifting bind.  The way it works is to use shadow inodes (unlike
> bind, but because you have to intercept the operations, so it's not a
> simple subtree operation) but there's no file copying.  The shadow
> points to the real inode.

For that you need a super block struct for every mount... now if you
also need a new internal fs + super block + shadowing inodes... it
seems like you are going into overlayfs direction... 

I'm taking overlayfs as an example here, cause it's just nice and
really dead simple!

At the same time I'm not at all sure about what you are describing!
and how you will deal with current mount and bind mounts tree and all
the internals...



> > > > Using the user namespace in the super block seems the way to go, 
> > > > and there is the "Support fuse mounts in user namespaces" [1] 
> > > > patches which seem nice but perhaps too complex!?
> > > 
> > > So I don't think that does what you want.  The fuse project I've 
> > > used before to do uid/gid shifts for build containers is bindfs
> > > 
> > > https://github.com/mpartel/bindfs/
> > > 
> > > It allows a --map argument where you specify pairs of uids/gids to 
> > > map (tedious for large ranges, but the map can be fixed to use
> > > uid:range instead of individual).
> > 
> > Ok, thanks for the link, will try to take a deep look but bindfs seem
> > really big!
> 
> Well, it does a lot more than just uid/gid shift.
> 
> > > >  there is also the overlayfs solution, and finaly the VFS layer 
> > > > solution.
> > > > 
> > > > We present here a simple VFS solution, everything is packed 
> > > > inside VFS, filesystems don't need to know anything (except 
> > > > probably XFS, and special operations inside union filesystems). 
> > > > Currently it supports ext4, btrfs and overlayfs. Changes into 
> > > > filesystems are small, just parse the vfs_shift_uids and 
> > > > vfs_shift_gids options during mount and set the appropriate flags 
> > > > into the super_block structure.
> > > 
> > > So this looks a little daunting.  It sprays the VFS with knowledge
> > > about the shifts and requires support from every underlying 
> > > filesystem. 
> 
> > Well, from my angle, shifts are just user namespace mappings which
> > follow certain rules, and currently VFS and all filesystems are 
> > *already* doing some kind of shifting... This RFC uses mount 
> > namespaces which are the standard way to deal with mounts, now the 
> > mapping inside mount namespace can just be "inside: 0:1000" => 
> > "outside: 0:1000" and current implementation will just use it, at the 
> > same time I'm not sure if this mapping qualifies to be named "shift". 
> > I think that some folks here came up with the "shift" name to 
> > describe one of the use cases from a user interface that's it... 
> > maybe I should do s/vfs_shift_*/vfs_remap_*/ ?
> 
> I don't think the naming is the issue ... it's the spread inside the
> vfs code (and in the underlying fs code).  The vfs is very well

Currently the underlying file systems just parse vfs_shift_uids and
vfs_shif_gids


> layered, so touching all that code makes it look like there's a
> layering problem with the patch.  Touching the underlying fs code looks

Hmm, not sure I follow here ? We make use of the mount namespace which
is part of the whole layer. Actually it's the *standard* way to control
mounts. What do you mean here please ?


> even more problematic, but that may be necessary if you have a reason
> for wanting the file ioctls, because they're pass through and usually
> where the from_kuid() calls are in filesystems.

Hmm sorry, I'm not sure I'm following you here ?


> > >  A simple remapping bind filesystem would be a lot simpler and 
> > > require no underlying filesystem support. 
> >  
> > Yes probably, you still need to parse parameters but not at the
> > filesystem level,
> 
> They'd just be mount options.  Basically instead of mount --bind source
> target, you'd do mount -t uidshift -o <shift options> source target.
> 
> >  and sure this RFC can do the same of course, but maybe it's not safe
> > to shift/remap filesystems and their inodes on behalf of
> > filesystems... and virtual filesystems which can share inodes ?
> 
> That depends who you allow to do the shift.  Each fstype in the kernel
> decides access to mount.  For the uidshift, I was planning to allow
> only a capable admin in the initial namespace, meaning that only the
> admin in the host could set up the shifts.  As long as the shifted
> filesystem is present, the container can then bind it wherever it wants
> in its mount namespace.

Ah I see admin in initial namespace, yes sounds reasonable for security
reasons, and how you will be able to achieve the user namespace shift ?


> James
>

Thank you!


-- 
Djalal Harouni
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1395396

On Thu, 2016-05-05 at 22:49 +0100, Djalal Harouni wrote:
> On Thu, May 05, 2016 at 07:56:28AM -0400, James Bottomley wrote:
> > On Thu, 2016-05-05 at 08:36 +0100, Djalal Harouni wrote:
> > > On Wed, May 04, 2016 at 05:06:19PM -0400, James Bottomley wrote:
> > > > On Wed, 2016-05-04 at 16:26 +0200, Djalal Harouni wrote:
> > > > > This is version 2 of the VFS:userns support portable root
> > > > > filesystems
> > > > > RFC. Changes since version 1:
> > > > > 
> > > > > * Update documentation and remove some ambiguity about the
> > > > > feature.   Based on Josh Triplett comments.
> > > > > * Use a new email address to send the RFC :-)
> > > > > 
> > > > > 
> > > > > This RFC tries to explore how to support filesystem 
> > > > > operations inside user namespace using only VFS and a per 
> > > > > mount namespace solution. This allows to take advantage of 
> > > > > user namespace separations without introducing any change at 
> > > > > the filesystems level. All this is handled with the virtual 
> > > > > view of mount namespaces.
> > > > > 
> > > > > 
> > > > > 1) Presentation:
> > > > > ================
> > > > > 
> > > > > The main aim is to support portable root filesystems and 
> > > > > allow containers, virtual machines and other cases to use the 
> > > > > same root filesystem. Due to security reasons, filesystems 
> > > > > can't be mounted inside user namespaces, and mounting them 
> > > > > outside will not solve the problem since they will show up 
> > > > > with the wrong UIDs/GIDs. Read and write operations will also
> > > > > fail and so on.
> > > > > 
> > > > > The current userspace solution is to automatically chown the 
> > > > > whole root filesystem before starting a container, example:
> > > > > (host) init_user_ns  1000000:1065536  => (container)
> > > > > user_ns_X1
> > > > > 0:65535
> > > > > (host) init_user_ns  2000000:2065536  => (container)
> > > > > user_ns_Y1
> > > > > 0:65535
> > > > > (host) init_user_ns  3000000:3065536  => (container)
> > > > > user_ns_Z1
> > > > > 0:65535
> > > > > ...
> > > > > 
> > > > > Every time a chown is called, files are changed and so on... 
> > > > > This prevents to have portable filesystems where you can 
> > > > > throw anywhere and boot. Having an extra step to adapt the
> > > > > filesystem to the current mapping and persist it will not 
> > > > > allow to verify its integrity, it makes snapshots and 
> > > > > migration a bit harder, and probably other limitations...
> > > > > 
> > > > > It seems that there are multiple ways to allow user 
> > > > > namespaces combine nicely with filesystems, but none of them 
> > > > > is that easy. The bind mount and pin the user namespace 
> > > > > during mount time will not work, bind mounts share the same 
> > > > > super block, hence you may endup working on the wrong 
> > > > > vfsmount context and there is no easy way to get out of
> > > > > that...
> > > > 
> > > > So this option was discussed at the recent LSF/MM summit.  The 
> > > > most supported suggestion was that you'd use a new internal fs 
> > > > type that had a struct mount with a new superblock and would 
> > > > copy the underlying inodes but substitute it's own with 
> > > >  modified  ->getatrr/->setattr calls that did the uid shift. 
> > > >  In many ways it would be a remapping bind which would look 
> > > > similar to overlayfs but be a lot simpler.
> > > 
> > > Hmm, it's not only about ->getattr and ->setattr, you have all 
> > > the other file system operations that need access too...
> > 
> > Why?  Or perhaps we should more cogently define the actual problem.
> >   My problem is simply mounting image volumes that were created 
> > with real uids at user namespace shifted uids because I'm
> >  downshifting the privileged ids in the container.  I actually 
> > *only* need the uid/gids on the attributes shifted because that's 
> > what I need to manipulate the
> >   
> We need them obviously for read/write/creation... ?!

OK, so the way attributes are populated on an inode is via getattr. 
 You intercept that, you change the inode owner and group that are
installed on the inode.  That means that when you list the directory,
you see the shift and the shifted uid/gid are used to check permissions
for vfs_open().

>  We want to handle also stock filesystems that were never edited
> without depending on any module or third party solution, mounting
> them outside user namespaces, and access inside.

OK, but that's basically my requirements ... you didn't mention any of
the esoteric filesystem ioctls, so I assume from the below you're not
interested in shifting the uids there either?

> > volumes.  I actually think that other operations, like the file 
> > ioctl ones should, for security reasons, not be uid shifted.  For
> > instance with xfs you could set the panic mask and error tags and 
> > bring down the whole host.  What extra things do you need access to
> > and why?
> 
> That's why precisely I said that mounting options not *inside*
> filesystems which means on their back, and on behalf of container
> managers, etc then you are exposed to such scenarios... some virtual
> file systems can also be mounted by unprivileged, how you will deal
> with something like a bind mount on them ?
> 
> 
> > >  which brings two points:
> > > 
> > > 1) This new internal fs may end up doing what this RFC does...
> > 
> > Well that was why I brought it up, yes.
> 
> yes but *with* extra code! that was my point. I'm not sure we need to
> bother with any *new* internal fs type nor hack around dir, file
> operations... yet that has to be shown, defined, coded ... ?

Either way requires patching the kernel.  The question I was asking is
is it better to confine the patch to a new fs type or directly change
the vfs.

> > > 2) or by quoting "new internal fs + its own super block + copy
> > > underlying inodes..." it seems like another overlayfs where you 
> > > also need some decisions to copy what, etc. So, will this be 
> > > really that light compared to current overlayfs ? not to mention 
> > > that you need to hook up basically the same logic or something 
> > > else inside overlayfs..
> > 
> > OK, so forget overlayfs, perhaps that was a bad example.  It's like 
> > a uid shifting bind.  The way it works is to use shadow inodes
> > (unlike bind, but because you have to intercept the operations, so 
> > it's not a simple subtree operation) but there's no file copying. 
> >  The shadow points to the real inode.
> 
> For that you need a super block struct for every mount... now if you
> also need a new internal fs + super block + shadowing inodes... it
> seems like you are going into overlayfs direction... 

Well, that's the way you build a shadowing fs.  I'm not sure you need
one sb per struct vfs mount, but it's certainly possible to code it
that way.

> I'm taking overlayfs as an example here, cause it's just nice and
> really dead simple!
> 
> At the same time I'm not at all sure about what you are describing!
> and how you will deal with current mount and bind mounts tree and all
> the internals...

You mean would MS_REC functionality be supported?  There's no reason
why not, but there's no reason you have to either (it could even be
optional, like it is for bind).

> > > > Using the user namespace in the super block seems the way to 
> > > > go, and there is the "Support fuse mounts in user namespaces" 
> > > > [1] patches which seem nice but perhaps too complex!?
> > > > 
> > > > So I don't think that does what you want.  The fuse project 
> > > > I've used before to do uid/gid shifts for build containers is 
> > > > bindfs https://github.com/mpartel/bindfs/
> > > > 
> > > > It allows a --map argument where you specify pairs of uids/gids 
> > > > to map (tedious for large ranges, but the map can be fixed to 
> > > > use uid:range instead of individual).
> > > 
> > > Ok, thanks for the link, will try to take a deep look but bindfs 
> > > seem really big!
> > 
> > Well, it does a lot more than just uid/gid shift.
> > 
> > > > >  there is also the overlayfs solution, and finaly the VFS 
> > > > > layer solution.
> > > > > 
> > > > > We present here a simple VFS solution, everything is packed 
> > > > > inside VFS, filesystems don't need to know anything (except 
> > > > > probably XFS, and special operations inside union 
> > > > > filesystems). Currently it supports ext4, btrfs and 
> > > > > overlayfs. Changes into filesystems are small, just parse the 
> > > > > vfs_shift_uids and vfs_shift_gids options during mount and 
> > > > > set the appropriate flags into the super_block structure.
> > > > 
> > > > So this looks a little daunting.  It sprays the VFS with 
> > > > knowledge about the shifts and requires support from every 
> > > > underlying filesystem.
> > 
> > > Well, from my angle, shifts are just user namespace mappings 
> > > which follow certain rules, and currently VFS and all filesystems 
> > > are *already* doing some kind of shifting... This RFC uses mount
> > > namespaces which are the standard way to deal with mounts, now 
> > > the mapping inside mount namespace can just be "inside: 0:1000" 
> > > => "outside: 0:1000" and current implementation will just use it, 
> > > at the same time I'm not sure if this mapping qualifies to be 
> > > named "shift". I think that some folks here came up with the 
> > > "shift" name to describe one of the use cases from a user 
> > > interface that's it... maybe I should do
> > > s/vfs_shift_*/vfs_remap_*/ ?
> > 
> > I don't think the naming is the issue ... it's the spread inside 
> > the vfs code (and in the underlying fs code).  The vfs is very well
> 
> Currently the underlying file systems just parse vfs_shift_uids and
> vfs_shif_gids 
> 
> > layered, so touching all that code makes it look like there's a
> > layering problem with the patch.  Touching the underlying fs code 
> > looks
> >  
> Hmm, not sure I follow here ? We make use of the mount namespace 
> which is part of the whole layer. Actually it's the *standard* way to
> control mounts. What do you mean here please ?

The patch touches a lot of the vfs.

> > even more problematic, but that may be necessary if you have a 
> > reason for wanting the file ioctls, because they're pass through 
> > and usually where the from_kuid() calls are in filesystems.
> 
> Hmm sorry, I'm not sure I'm following you here ?

An ideal solution, given both our requirements, shouldn't require
touching any underlying fs code.

> > > >  A simple remapping bind filesystem would be a lot simpler and 
> > > > require no underlying filesystem support. 
> > >  
> > > Yes probably, you still need to parse parameters but not at the
> > > filesystem level,
> > 
> > They'd just be mount options.  Basically instead of mount --bind 
> > source target, you'd do mount -t uidshift -o <shift options> source
> > target.
> > 
> > >  and sure this RFC can do the same of course, but maybe it's not 
> > > safe to shift/remap filesystems and their inodes on behalf of
> > > filesystems... and virtual filesystems which can share inodes ?
> > 
> > That depends who you allow to do the shift.  Each fstype in the 
> > kernel decides access to mount.  For the uidshift, I was planning 
> > to allow only a capable admin in the initial namespace, meaning 
> > that only the admin in the host could set up the shifts.  As long 
> > as the shifted filesystem is present, the container can then bind 
> > it wherever it wants in its mount namespace.
> 
> Ah I see admin in initial namespace, yes sounds reasonable for 
> security reasons, and how you will be able to achieve the user 
> namespace shift?

As I said, it would be in the mount options of the command.  The <shift
options> above. Probably parametrised the way uid_map and gid_map are
today.

James

[toc] | [prev] | [next] | [standalone]

#1398590

On Thu, 2016-05-05 at 18:08 -0400, James Bottomley wrote:
> On Thu, 2016-05-05 at 22:49 +0100, Djalal Harouni wrote:
> > On Thu, May 05, 2016 at 07:56:28AM -0400, James Bottomley wrote:
> > > On Thu, 2016-05-05 at 08:36 +0100, Djalal Harouni wrote:
> > > > On Wed, May 04, 2016 at 05:06:19PM -0400, James Bottomley
> > > > wrote:
[...]
> > > > > So this option was discussed at the recent LSF/MM summit. 
> > > > >  The 
> > > > > most supported suggestion was that you'd use a new internal
> > > > > fs 
> > > > > type that had a struct mount with a new superblock and would 
> > > > > copy the underlying inodes but substitute it's own with 
> > > > >  modified  ->getatrr/->setattr calls that did the uid shift. 
> > > > >  In many ways it would be a remapping bind which would look 
> > > > > similar to overlayfs but be a lot simpler.
> > > > 
> > > > Hmm, it's not only about ->getattr and ->setattr, you have all 
> > > > the other file system operations that need access too...
> > > 
> > > Why?  Or perhaps we should more cogently define the actual 
> > > problem.   My problem is simply mounting image volumes that were 
> > > created with real uids at user namespace shifted uids because I'm
> > >  downshifting the privileged ids in the container.  I actually 
> > > *only* need the uid/gids on the attributes shifted because that's
> > > what I need to manipulate the
> > >   
> > We need them obviously for read/write/creation... ?!
> 
> OK, so the way attributes are populated on an inode is via getattr. 
>  You intercept that, you change the inode owner and group that are
> installed on the inode.  That means that when you list the directory,
> you see the shift and the shifted uid/gid are used to check 
> permissions for vfs_open().

Just to illustrate how this could be done, here's a functional proof of
concept for a uid/gid shifting bind mount equivalent.  It's not
actually a proper bind mount because it has to manufacture its own
inodes.  As you can see, it can only be used by root, it will shift all
the uid/gid bits as well as the permission comparisons.  It operates on
subtrees, so it can shift the uids/gids on any filesystem or part of
one and because the shifts are per superblock, it could actually shift
the same subtree for multiple users on different shifts.  Best of all,
it requires no vfs changes at all, being entirely implemented inside
its own filesystem type.

You use it just like bind mount:

mount -t shiftfs <source> <target>

except that it takes uidshift=x:y:z and gidshift=x:y:z multiple times
as options.  It's currently not recursive and it definitely needs
polishing to show things like mount options and be properly Kconfig
using.

There's a bit of an open question of whether it should have vfs
changes: the way the struct file f_inode and f_ops are hijacked is a
bit nasty and perhaps d_select_inode() could be made a bit cleverer to
help us here instead.

James

---

 fs/Makefile                |   1 +
 fs/shiftfs.c               | 790 +++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/magic.h |   2 +
 3 files changed, 793 insertions(+)

diff --git a/fs/Makefile b/fs/Makefile
index 85b6e13..bad03b2 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -128,3 +128,4 @@ obj-y				+= exofs/ # Multiple modules
 obj-$(CONFIG_CEPH_FS)		+= ceph/
 obj-$(CONFIG_PSTORE)		+= pstore/
 obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
+obj-m				+= shiftfs.o
diff --git a/fs/shiftfs.c b/fs/shiftfs.c
new file mode 100644
index 0000000..b40cdfe
--- /dev/null
+++ b/fs/shiftfs.c
@@ -0,0 +1,790 @@
+#include <linux/cred.h>
+#include <linux/mount.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/magic.h>
+#include <linux/parser.h>
+#include <linux/statfs.h>
+#include <linux/slab.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+
+struct shiftfs_super_info {
+	struct vfsmount *mnt;
+	struct uid_gid_map uid_map, gid_map;
+};
+
+static struct inode *shiftfs_new_inode(struct super_block *sb, umode_t mode,
+				       struct dentry *dentry);
+
+enum {
+	OPT_UIDMAP,
+	OPT_GIDMAP,
+	OPT_LAST,
+};
+
+/* global filesystem options */
+static const match_table_t tokens = {
+	{ OPT_UIDMAP, "uidmap=%u:%u:%u" },
+	{ OPT_GIDMAP, "gidmap=%u:%u:%u" },
+	{ OPT_LAST, NULL }
+};
+
+/*
+ * code stolen from user_namespace.c ... except that these functions
+ * return the same id back if unmapped ... should probably have a
+ * library?
+ */
+static u32 map_id_down(struct uid_gid_map *map, u32 id)
+{
+	unsigned idx, extents;
+	u32 first, last;
+
+	/* Find the matching extent */
+	extents = map->nr_extents;
+	smp_rmb();
+	for (idx = 0; idx < extents; idx++) {
+		first = map->extent[idx].first;
+		last = first + map->extent[idx].count - 1;
+		if (id >= first && id <= last)
+			break;
+	}
+	/* Map the id or note failure */
+	if (idx < extents)
+		id = (id - first) + map->extent[idx].lower_first;
+
+	return id;
+}
+
+static u32 map_id_up(struct uid_gid_map *map, u32 id)
+{
+	unsigned idx, extents;
+	u32 first, last;
+
+	/* Find the matching extent */
+	extents = map->nr_extents;
+	smp_rmb();
+	for (idx = 0; idx < extents; idx++) {
+		first = map->extent[idx].lower_first;
+		last = first + map->extent[idx].count - 1;
+		if (id >= first && id <= last)
+			break;
+	}
+	/* Map the id or note failure */
+	if (idx < extents)
+		id = (id - first) + map->extent[idx].first;
+
+	return id;
+}
+
+static bool mappings_overlap(struct uid_gid_map *new_map,
+			     struct uid_gid_extent *extent)
+{
+	u32 upper_first, lower_first, upper_last, lower_last;
+	unsigned idx;
+
+	upper_first = extent->first;
+	lower_first = extent->lower_first;
+	upper_last = upper_first + extent->count - 1;
+	lower_last = lower_first + extent->count - 1;
+
+	for (idx = 0; idx < new_map->nr_extents; idx++) {
+		u32 prev_upper_first, prev_lower_first;
+		u32 prev_upper_last, prev_lower_last;
+		struct uid_gid_extent *prev;
+
+		prev = &new_map->extent[idx];
+
+		prev_upper_first = prev->first;
+		prev_lower_first = prev->lower_first;
+		prev_upper_last = prev_upper_first + prev->count - 1;
+		prev_lower_last = prev_lower_first + prev->count - 1;
+
+		/* Does the upper range intersect a previous extent? */
+		if ((prev_upper_first <= upper_last) &&
+		    (prev_upper_last >= upper_first))
+			return true;
+
+		/* Does the lower range intersect a previous extent? */
+		if ((prev_lower_first <= lower_last) &&
+		    (prev_lower_last >= lower_first))
+			return true;
+	}
+	return false;
+}
+/* end code stolen from user_namespace.c */
+
+static const struct cred *shiftfs_get_up_creds(struct super_block *sb)
+{
+	struct cred *cred = prepare_creds();
+	struct shiftfs_super_info *ssi = sb->s_fs_info;
+
+	if (!cred)
+		return NULL;
+
+	cred->fsuid = KUIDT_INIT(map_id_up(&ssi->uid_map, __kuid_val(cred->fsuid)));
+	cred->fsgid = KGIDT_INIT(map_id_up(&ssi->gid_map, __kgid_val(cred->fsgid)));
+
+	return cred;
+}
+
+static const struct cred *shiftfs_new_creds(const struct cred **newcred,
+					    struct super_block *sb)
+{
+	const struct cred *cred = shiftfs_get_up_creds(sb);
+
+	*newcred = cred;
+
+	if (cred)
+		cred = override_creds(cred);
+	else
+		printk(KERN_ERR "Credential override failed: no memory\n");
+
+	return cred;
+}
+
+static void shiftfs_old_creds(const struct cred *oldcred,
+			      const struct cred **newcred)
+{
+	if (!*newcred)
+		return;
+
+	revert_creds(oldcred);
+	put_cred(*newcred);
+}
+
+static int shiftfs_parse_options(struct shiftfs_super_info *ssi, char *options)
+{
+	char *p;
+	substring_t args[MAX_OPT_ARGS];
+	int from, to, count;
+	struct uid_gid_map *map, *maps[2] = {
+		[OPT_UIDMAP] = &ssi->uid_map,
+		[OPT_GIDMAP] = &ssi->gid_map,
+	};
+
+	while ((p = strsep(&options, ",")) != NULL) {
+		int token;
+		struct uid_gid_extent ext;
+
+		if (!*p)
+			continue;
+
+		token = match_token(p, tokens, args);
+		if (token != OPT_UIDMAP && token != OPT_GIDMAP)
+			return -EINVAL;
+		if (match_int(&args[0], &from) ||
+		    match_int(&args[1], &to) ||
+		    match_int(&args[2], &count))
+			return -EINVAL;
+		map = maps[token];
+		if (map->nr_extents >= UID_GID_MAP_MAX_EXTENTS)
+			return -EINVAL;
+		ext.first = from;
+		ext.lower_first = to;
+		ext.count = count;
+		if (mappings_overlap(map, &ext))
+			return -EINVAL;
+		map->extent[map->nr_extents++] = ext;
+	}
+	return 0;
+}
+
+static void shiftfs_d_iput(struct dentry *dentry, struct inode *inode)
+{
+	struct dentry *real = inode->i_private;
+
+	dput(real);
+	iput(inode);
+}
+
+static const struct dentry_operations shiftfs_dentry_ops = {
+	.d_iput		= shiftfs_d_iput,
+};
+
+static int shiftfs_readlink(struct dentry *dentry, char __user *data,
+			    int flags)
+{
+	struct dentry *real = dentry->d_inode->i_private;
+	const struct inode_operations *iop = real->d_inode->i_op;
+
+	if (iop->readlink)
+		return iop->readlink(real, data, flags);
+
+	return -EINVAL;
+}
+
+static const char *shiftfs_get_link(struct dentry *dentry, struct inode *inode,
+				    struct delayed_call *done)
+{
+	if (dentry) {
+		struct dentry *real = dentry->d_inode->i_private;
+		struct inode *reali = real->d_inode;
+		const struct inode_operations *iop = reali->i_op;
+		const char *res = ERR_PTR(-EPERM);
+
+		if (iop->get_link)
+			res = iop->get_link(real, reali, done);
+
+		return res;
+	} else {
+		/* RCU lookup not supported */
+		return ERR_PTR(-ECHILD);
+	}
+}
+
+static int shiftfs_setxattr(struct dentry *dentry, const char *name,
+			    const void *value, size_t size, int flags)
+{
+	struct dentry *real = dentry->d_inode->i_private;
+	const struct inode_operations *iop = real->d_inode->i_op;
+	int err = -EOPNOTSUPP;
+
+	if (iop->setxattr) {
+		const struct cred *oldcred, *newcred;
+
+		oldcred = shiftfs_new_creds(&newcred, dentry->d_sb);
+		err = iop->setxattr(real, name, value, size, flags);
+		shiftfs_old_creds(oldcred, &newcred);
+	}
+
+	return err;
+}
+
+static ssize_t shiftfs_getxattr(struct dentry *dentry, const char *name,
+				void *value, size_t size)
+{
+	struct dentry *real = dentry->d_inode->i_private;
+	const struct inode_operations *iop = real->d_inode->i_op;
+	int err = -EOPNOTSUPP;
+
+	if (iop->getxattr) {
+		const struct cred *oldcred, *newcred;
+
+		oldcred = shiftfs_new_creds(&newcred, dentry->d_sb);
+		err = iop->getxattr(real, name, value, size);
+		shiftfs_old_creds(oldcred, &newcred);
+	}
+
+	return err;
+}
+
+static ssize_t shiftfs_listxattr(struct dentry *dentry, char *list,
+				 size_t size)
+{
+	struct dentry *real = dentry->d_inode->i_private;
+	const struct inode_operations *iop = real->d_inode->i_op;
+
+	if (iop->listxattr)
+		return iop->listxattr(real, list, size);
+
+	return -EINVAL;
+}
+
+static int shiftfs_removexattr(struct dentry *dentry, const char *name)
+{
+	struct dentry *real = dentry->d_inode->i_private;
+	const struct inode_operations *iop = real->d_inode->i_op;
+
+	if (iop->removexattr)
+		return iop->removexattr(real, name);
+
+	return -EINVAL;
+}
+
+static void shiftfs_fill_inode(struct inode *inode, struct dentry *dentry)
+{
+	struct inode *reali;
+	struct shiftfs_super_info *ssi = inode->i_sb->s_fs_info;
+
+	if (!dentry)
+		return;
+
+	reali = dentry->d_inode;
+
+	if (!reali->i_op->get_link)
+		inode->i_opflags |= IOP_NOFOLLOW;
+
+	inode->i_mapping = reali->i_mapping;
+	inode->i_private = dentry;
+
+	inode->i_uid = KUIDT_INIT(map_id_down(&ssi->uid_map, __kuid_val(reali->i_uid)));
+	inode->i_gid = KGIDT_INIT(map_id_down(&ssi->gid_map, __kgid_val(reali->i_gid)));
+}
+
+static int shiftfs_make_object(struct inode *dir, struct dentry *dentry,
+			       umode_t mode, const char *symlink,
+			       struct dentry *hardlink, bool excl)
+{
+	struct dentry *real = dir->i_private, *new;
+	struct inode *reali = real->d_inode, *newi;
+	const struct inode_operations *iop = reali->i_op;
+	int err;
+	const struct cred *oldcred, *newcred;
+	bool op_ok = false;
+
+	if (hardlink) {
+		op_ok = iop->link;
+	} else {
+		switch (mode & S_IFMT) {
+		case S_IFDIR:
+			op_ok = iop->mkdir;
+			break;
+		case S_IFREG:
+			op_ok = iop->create;
+			break;
+		case S_IFLNK:
+			op_ok = iop->symlink;
+		}
+	}
+	if (!op_ok)
+		return -EINVAL;
+
+
+	newi = shiftfs_new_inode(dentry->d_sb, mode, NULL);
+	if (!newi)
+		return -ENOMEM;
+
+	oldcred = shiftfs_new_creds(&newcred, dentry->d_sb);
+
+	inode_lock_nested(reali, I_MUTEX_PARENT);
+	new = lookup_one_len(dentry->d_name.name, real, dentry->d_name.len);
+	err = PTR_ERR(new);
+	if (IS_ERR(new))
+		goto out_unlock;
+
+	if (hardlink) {
+		struct dentry *realhardlink = hardlink->d_inode->i_private;
+
+		err = vfs_link(new, reali, realhardlink, NULL);
+	} else {
+		switch (mode & S_IFMT) {
+		case S_IFDIR:
+			err = vfs_mkdir(reali, new, mode);
+			break;
+		case S_IFREG:
+			err = vfs_create(reali, new, mode, excl);
+			break;
+		case S_IFLNK:
+			err = vfs_symlink(reali, new, symlink);
+		}
+	}
+
+	shiftfs_old_creds(oldcred, &newcred);
+
+	if (err)
+		goto out_dput;
+
+	shiftfs_fill_inode(newi, new);
+
+	d_instantiate(dentry, newi);
+
+	new = NULL;
+	newi = NULL;
+
+ out_dput:
+	dput(new);
+ out_unlock:
+	iput(newi);
+	inode_unlock(reali);
+
+	return err;
+}
+
+static int shiftfs_create(struct inode *dir, struct dentry *dentry,
+			  umode_t mode,  bool excl)
+{
+	mode |= S_IFREG;
+
+	return shiftfs_make_object(dir, dentry, mode, NULL, NULL, excl);
+}
+
+static int shiftfs_mkdir(struct inode *dir, struct dentry *dentry,
+			 umode_t mode)
+{
+	mode |= S_IFDIR;
+
+	return shiftfs_make_object(dir, dentry, mode, NULL, NULL, false);
+}
+
+static int shiftfs_link(struct dentry *dentry, struct inode *dir,
+			struct dentry *hardlink)
+{
+	return shiftfs_make_object(dir, dentry, 0, NULL, hardlink, false);
+}
+
+static int shiftfs_symlink(struct inode *dir, struct dentry *dentry,
+			   const char *symlink)
+{
+	return shiftfs_make_object(dir, dentry, S_IFLNK, symlink, NULL, false);
+}
+
+static int shiftfs_rm(struct inode *dir, struct dentry *dentry, bool rmdir)
+{
+	struct dentry *real = dir->i_private, *new;
+	struct inode *reali = real->d_inode;
+	int err;
+	const struct cred *oldcred, *newcred;
+
+	inode_lock_nested(reali, I_MUTEX_PARENT);
+
+	oldcred = shiftfs_new_creds(&newcred, dentry->d_sb);
+
+	new = lookup_one_len(dentry->d_name.name, real, dentry->d_name.len);
+	err = PTR_ERR(new);
+	if (IS_ERR(new))
+		goto out_unlock;
+
+	if (rmdir)
+		err = vfs_rmdir(reali, new);
+	else
+		err = vfs_unlink(reali, new, NULL);
+
+	dput(new);
+
+ out_unlock:
+	shiftfs_old_creds(oldcred, &newcred);
+	inode_unlock(reali);
+
+	return err;
+}
+
+static int shiftfs_unlink(struct inode *dir, struct dentry *dentry)
+{
+	return shiftfs_rm(dir, dentry, false);
+}
+
+static int shiftfs_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	return shiftfs_rm(dir, dentry, true);
+}
+
+static int shiftfs_rename2(struct inode *olddir, struct dentry *old,
+			   struct inode *newdir, struct dentry *new,
+			   unsigned int flags)
+{
+	struct dentry *rodd = olddir->i_private, *rndd = newdir->i_private,
+		*realold = old->d_inode->i_private,
+		*realnew = new->d_inode->i_private;
+	struct inode *realolddir = rodd->d_inode, *realnewdir = rndd->d_inode;
+	const struct inode_operations *iop = realolddir->i_op;
+	int err;
+	const struct cred *oldcred, *newcred;
+
+	oldcred = shiftfs_new_creds(&newcred, old->d_sb);
+	err = iop->rename2(realolddir, realold, realnewdir, realnew, flags);
+	shiftfs_old_creds(oldcred, &newcred);
+
+	return err;
+}
+
+static struct dentry *shiftfs_lookup(struct inode *dir, struct dentry *dentry,
+				     unsigned int flags)
+{
+	struct dentry *real = dir->i_private, *new;
+	struct inode *reali = real->d_inode, *newi;
+	const struct cred *oldcred, *newcred;
+
+	/* note: violation of usual fs rules here: dentries are never
+	 * added with d_add.  This is because we want no dentry cache
+	 * for shiftfs.  All lookups proceed through the dentry cache
+	 * of the underlying filesystem, meaning we always see any
+	 * changes in the underlying */
+
+	inode_lock(reali);
+	oldcred = shiftfs_new_creds(&newcred, dentry->d_sb);
+	new = lookup_one_len(dentry->d_name.name, real, dentry->d_name.len);
+	shiftfs_old_creds(oldcred, &newcred);
+	inode_unlock(reali);
+
+	if (IS_ERR(new) || !new)
+		return new;
+
+	if (!new->d_inode) {
+		dput(new);
+		return NULL;
+	}
+
+	newi = shiftfs_new_inode(dentry->d_sb, new->d_inode->i_mode, new);
+	if (!newi) {
+		dput(new);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	d_instantiate(dentry, newi);
+
+	return NULL;
+}
+
+static int shiftfs_permission(struct inode *inode, int mask)
+{
+	struct dentry *real = inode->i_private;
+	struct inode *reali = real->d_inode;
+	const struct inode_operations *iop = reali->i_op;
+	int err;
+	const struct cred *oldcred, *newcred;
+
+	oldcred = shiftfs_new_creds(&newcred, inode->i_sb);
+	if (iop->permission)
+		err = iop->permission(reali, mask);
+	else
+		err = generic_permission(reali, mask);
+	shiftfs_old_creds(oldcred, &newcred);
+
+	return err;
+}
+
+static int shiftfs_setattr(struct dentry *dentry, struct iattr *attr)
+{
+	struct dentry *real = dentry->d_inode->i_private;
+	struct inode *reali = real->d_inode;
+	const struct inode_operations *iop = reali->i_op;
+	struct iattr newattr = *attr;
+	const struct cred *oldcred, *newcred;
+	struct shiftfs_super_info *ssi = dentry->d_sb->s_fs_info;
+	int err;
+
+	newattr.ia_uid = KUIDT_INIT(map_id_up(&ssi->uid_map, __kuid_val(attr->ia_uid)));
+	newattr.ia_gid = KGIDT_INIT(map_id_up(&ssi->gid_map, __kgid_val(attr->ia_gid)));
+
+	oldcred = shiftfs_new_creds(&newcred, dentry->d_sb);
+	if (iop->setattr)
+		err = iop->setattr(real, &newattr);
+	else
+		err = simple_setattr(real, &newattr);
+	shiftfs_old_creds(oldcred, &newcred);
+
+	return err;
+}
+
+static int shiftfs_getattr(struct vfsmount *mnt, struct dentry *dentry,
+			   struct kstat *stat)
+{
+	struct inode *inode = dentry->d_inode;
+	struct dentry *real = inode->i_private;
+	struct inode *reali = real->d_inode;
+	const struct inode_operations *iop = reali->i_op;
+	int err = 0;
+
+	mnt = dentry->d_sb->s_fs_info;
+
+	if (iop->getattr)
+		err = iop->getattr(mnt, real, stat);
+	else
+		generic_fillattr(reali, stat);
+
+	if (err)
+		return err;
+
+	stat->uid = inode->i_uid;
+	stat->gid = inode->i_gid;
+	return 0;
+}
+
+struct shiftfs_fop_carrier {
+	struct inode *inode;
+	int (*release)(struct inode *, struct file *);
+	struct file_operations fop;
+};
+
+static int shiftfs_release(struct inode *inode, struct file *file)
+{
+	struct shiftfs_fop_carrier *sfc;
+	int err = 0;
+
+	sfc = container_of(file->f_op, struct shiftfs_fop_carrier, fop);
+
+	if (sfc->release)
+		err = sfc->release(inode, file);
+
+	file->f_inode = sfc->inode;
+	file->f_op = sfc->inode->i_fop;
+
+	kfree(sfc);
+
+	return err;
+}
+
+static int shiftfs_open(struct inode *inode, struct file *file)
+{
+	struct dentry *real = inode->i_private;
+	struct inode *reali = real->d_inode;
+	const struct file_operations *fop;
+	struct shiftfs_fop_carrier *sfc;
+	int err = 0;
+
+	sfc = kmalloc(sizeof(*sfc), GFP_KERNEL);
+	if (!sfc)
+		return -ENOMEM;
+
+	if (real->d_flags & DCACHE_OP_SELECT_INODE)
+		reali = real->d_op->d_select_inode(real, file->f_flags);
+
+	fop = reali->i_fop;
+	sfc->inode = inode;
+	memcpy(&sfc->fop, fop, sizeof(*fop));
+	sfc->release = sfc->fop.release;
+	sfc->fop.release = shiftfs_release;
+
+	file->f_op = &sfc->fop;
+	file->f_inode = reali;
+
+	if (fop->open)
+		err = fop->open(reali, file);
+
+	return err;
+}
+
+static const struct inode_operations shiftfs_inode_ops = {
+	/* intercepted */
+	.lookup		= shiftfs_lookup,
+	.getattr	= shiftfs_getattr,
+	.setattr	= shiftfs_setattr,
+	.permission	= shiftfs_permission,
+
+	/*pass though */
+	.mkdir		= shiftfs_mkdir,
+	.symlink	= shiftfs_symlink,
+	.get_link	= shiftfs_get_link,
+	.readlink	= shiftfs_readlink,
+	.unlink		= shiftfs_unlink,
+	.rmdir		= shiftfs_rmdir,
+	.rename2	= shiftfs_rename2,
+	.link		= shiftfs_link,
+	.create		= shiftfs_create,
+	.mknod		= NULL,	/* no special files currently */
+	.setxattr	= shiftfs_setxattr,
+	.getxattr	= shiftfs_getxattr,
+	.listxattr	= shiftfs_listxattr,
+	.removexattr	= shiftfs_removexattr,
+};
+
+static const struct file_operations shiftfs_file_ops = {
+	.open		= shiftfs_open,
+};
+
+static struct inode *shiftfs_new_inode(struct super_block *sb, umode_t mode,
+				       struct dentry *dentry)
+{
+	struct inode *inode;
+
+	inode = new_inode(sb);
+	if (!inode)
+		return NULL;
+
+	mode &= S_IFMT;
+
+	inode->i_ino = get_next_ino();
+	inode->i_mode = mode;
+	inode->i_flags |= S_NOATIME | S_NOCMTIME;
+
+	inode->i_op = &shiftfs_inode_ops;
+	inode->i_fop = &shiftfs_file_ops;
+
+	shiftfs_fill_inode(inode, dentry);
+
+	return inode;
+}
+
+static void shiftfs_put_super(struct super_block *sb)
+{
+	struct shiftfs_super_info *ssi = sb->s_fs_info;
+
+	mntput(ssi->mnt);
+	kfree(ssi);
+}
+
+static const struct super_operations shiftfs_super_ops = {
+	.put_super	= shiftfs_put_super,
+};
+
+struct shiftfs_data {
+	void *data;
+	const char *path;
+};
+
+static int shiftfs_fill_super(struct super_block *sb, void *raw_data,
+			      int silent)
+{
+	struct shiftfs_data *data = raw_data;
+	char *name = kstrdup(data->path, GFP_KERNEL);
+	int err = -ENOMEM;
+	struct shiftfs_super_info *ssi = NULL;
+	struct path path;
+
+	if (!name)
+		goto out;
+
+	ssi = kzalloc(sizeof(*ssi), GFP_KERNEL);
+	if (!ssi)
+		goto out;
+
+	err = -EPERM;
+	if (!capable(CAP_SYS_ADMIN))
+		goto out;
+
+	err = shiftfs_parse_options(ssi, data->data);
+	if (err)
+		goto out;
+
+	err = kern_path(name, LOOKUP_FOLLOW, &path);
+	if (err)
+		goto out;
+
+	if (!S_ISDIR(path.dentry->d_inode->i_mode)) {
+		err = -ENOTDIR;
+		goto out_put;
+	}
+	ssi->mnt = path.mnt;
+
+	sb->s_fs_info = ssi;
+	sb->s_magic = SHIFTFS_MAGIC;
+	sb->s_op = &shiftfs_super_ops;
+	sb->s_d_op = &shiftfs_dentry_ops;
+	sb->s_root = d_make_root(shiftfs_new_inode(sb, S_IFDIR, path.dentry));
+
+	return 0;
+
+ out_put:
+	path_put(&path);
+ out:
+	kfree(name);
+	if (err)
+		kfree(ssi);
+	return err;
+}
+
+static struct dentry *shiftfs_mount(struct file_system_type *fs_type,
+				    int flags, const char *dev_name, void *data)
+{
+	struct shiftfs_data d = { data, dev_name };
+
+	return mount_nodev(fs_type, flags, &d, shiftfs_fill_super);
+}
+
+static struct file_system_type shiftfs_type = {
+	.owner		= THIS_MODULE,
+	.name		= "shiftfs",
+	.mount		= shiftfs_mount,
+	.kill_sb	= kill_anon_super,
+};
+
+static int __init shiftfs_init(void)
+{
+	return register_filesystem(&shiftfs_type);
+}
+
+static void __exit shiftfs_exit(void)
+{
+	unregister_filesystem(&shiftfs_type);
+}
+
+MODULE_ALIAS_FS("shiftfs");
+MODULE_AUTHOR("James Bottomley");
+MODULE_DESCRIPTION("uid/gid shifting bind filesystem");
+MODULE_LICENSE("GPL v2");
+module_init(shiftfs_init)
+module_exit(shiftfs_exit)
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index 0de181a..d7992f5 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -79,4 +79,6 @@
 #define NSFS_MAGIC		0x6e736673
 #define BPF_FS_MAGIC		0xcafe4a11
 
+#define SHIFTFS_MAGIC		0x6a656a62
+
 #endif /* __LINUX_MAGIC_H__ */

[toc] | [prev] | [next] | [standalone]

#1398615

On Tue, May 10, 2016 at 04:36:56PM -0700, James Bottomley wrote:

> mount -t shiftfs <source> <target>

Note to self: do not eat while reading l-k...

[toc] | [prev] | [next] | [standalone]

#1398619

On Tue, May 10, 2016 at 04:36:56PM -0700, James Bottomley wrote:
> +static int shiftfs_rename2(struct inode *olddir, struct dentry *old,
> +			   struct inode *newdir, struct dentry *new,
> +			   unsigned int flags)
> +{
> +	struct dentry *rodd = olddir->i_private, *rndd = newdir->i_private,
> +		*realold = old->d_inode->i_private,
> +		*realnew = new->d_inode->i_private;
> +	struct inode *realolddir = rodd->d_inode, *realnewdir = rndd->d_inode;
> +	const struct inode_operations *iop = realolddir->i_op;
> +	int err;
> +	const struct cred *oldcred, *newcred;
> +
> +	oldcred = shiftfs_new_creds(&newcred, old->d_sb);
> +	err = iop->rename2(realolddir, realold, realnewdir, realnew, flags);
> +	shiftfs_old_creds(oldcred, &newcred);

... and you've just violated all locking rules for ->rename2().

> +static struct dentry *shiftfs_lookup(struct inode *dir, struct dentry *dentry,
> +				     unsigned int flags)
> +{
> +	struct dentry *real = dir->i_private, *new;
> +	struct inode *reali = real->d_inode, *newi;
> +	const struct cred *oldcred, *newcred;
> +
> +	/* note: violation of usual fs rules here: dentries are never
> +	 * added with d_add.  This is because we want no dentry cache
> +	 * for shiftfs.  All lookups proceed through the dentry cache
> +	 * of the underlying filesystem, meaning we always see any
> +	 * changes in the underlying */

Bloody wonderful.  So
	* we lose caching the negative lookups
	* we've got buggered hardlinks (different inodes for those)
	* it has never, ever been tried on -next (would do rather nasty
things on that d_instantiate())

> +
> +	kfree(sfc);
> +
> +	return err;
> +}

> +	file->f_op = &sfc->fop;

Lovely - now try that with underlying fs something built modular.

Or try to use it on top of something with non-trivial dentry_operations
(hell, on top of itself, for starters).

[toc] | [prev] | [next] | [standalone]

#1398663

On Wed, 2016-05-11 at 01:53 +0100, Al Viro wrote:
> On Tue, May 10, 2016 at 04:36:56PM -0700, James Bottomley wrote:
> > +static int shiftfs_rename2(struct inode *olddir, struct dentry
> > *old,
> > +			   struct inode *newdir, struct dentry
> > *new,
> > +			   unsigned int flags)
> > +{
> > +	struct dentry *rodd = olddir->i_private, *rndd = newdir
> > ->i_private,
> > +		*realold = old->d_inode->i_private,
> > +		*realnew = new->d_inode->i_private;
> > +	struct inode *realolddir = rodd->d_inode, *realnewdir =
> > rndd->d_inode;
> > +	const struct inode_operations *iop = realolddir->i_op;
> > +	int err;
> > +	const struct cred *oldcred, *newcred;
> > +
> > +	oldcred = shiftfs_new_creds(&newcred, old->d_sb);
> > +	err = iop->rename2(realolddir, realold, realnewdir,
> > realnew, flags);
> > +	shiftfs_old_creds(oldcred, &newcred);
> 
> ... and you've just violated all locking rules for ->rename2().

Yes, sorry, somehow I missed that when I converted everything else to
the vfs_ functions.

> > +static struct dentry *shiftfs_lookup(struct inode *dir, struct
> > dentry *dentry,
> > +				     unsigned int flags)
> > +{
> > +	struct dentry *real = dir->i_private, *new;
> > +	struct inode *reali = real->d_inode, *newi;
> > +	const struct cred *oldcred, *newcred;
> > +
> > +	/* note: violation of usual fs rules here: dentries are
> > never
> > +	 * added with d_add.  This is because we want no dentry
> > cache
> > +	 * for shiftfs.  All lookups proceed through the dentry
> > cache
> > +	 * of the underlying filesystem, meaning we always see any
> > +	 * changes in the underlying */
> 
> Bloody wonderful.  So
> 	* we lose caching the negative lookups

We do?  They should be cached in the underlying layer's dcache. If
that's not enough, I can hash them, but I was trying to avoid doubling
the dcache size.

> 	* we've got buggered hardlinks (different inodes for those)

Yes, had a note to do the lookup, but forgot.

> 	* it has never, ever been tried on -next (would do rather nasty
> things on that d_instantiate())

So this is just a proof of concept; I figured it was best to do it
against current rather than have people who wanted to try it pull in
your tree.  I can respin it after the merge window closes.

> 
> > +
> > +	kfree(sfc);
> > +
> > +	return err;
> > +}
> 
> > +	file->f_op = &sfc->fop;
> 
> Lovely - now try that with underlying fs something built modular.
> 
> Or try to use it on top of something with non-trivial
> dentry_operations
> (hell, on top of itself, for starters).

So if I add the missing fops_get/put, you're happy with the way this
hijacks f_op and f_inode?

James

[toc] | [prev] | [next] | [standalone]

#1399352

On Tue, May 10, 2016 at 04:36:56PM -0700, James Bottomley wrote:
> On Thu, 2016-05-05 at 18:08 -0400, James Bottomley wrote:
[...]
> > 
> > OK, so the way attributes are populated on an inode is via getattr. 
> >  You intercept that, you change the inode owner and group that are
> > installed on the inode.  That means that when you list the directory,
> > you see the shift and the shifted uid/gid are used to check 
> > permissions for vfs_open().
> 
> Just to illustrate how this could be done, here's a functional proof of
> concept for a uid/gid shifting bind mount equivalent.  It's not
> actually a proper bind mount because it has to manufacture its own
> inodes.  As you can see, it can only be used by root, it will shift all
> the uid/gid bits as well as the permission comparisons.  It operates on
> subtrees, so it can shift the uids/gids on any filesystem or part of
> one and because the shifts are per superblock, it could actually shift
> the same subtree for multiple users on different shifts.  Best of all,
> it requires no vfs changes at all, being entirely implemented inside
> its own filesystem type.

First, I guess this should be in a separate thread.. this way this RFC
was just hijacked!

Obviously as you say later in your response it may require a VFS
change... 

You have just consumed all inodes... what about containers or small apps
that are spawned quickly... it can even used maybe as a DoS...  maybe you
endup reporting different inode numbers... ?


> You use it just like bind mount:
> 
> mount -t shiftfs <source> <target>
> 
> except that it takes uidshift=x:y:z and gidshift=x:y:z multiple times
> as options.  It's currently not recursive and it definitely needs
> polishing to show things like mount options and be properly Kconfig
> using.

why it's not recursive ? and what if you have circular bind mounts ? 

Hmm anyway you are mounting this on behalf of filesystems, so if you add
the recursive thing, you will just probably make everything worse, by
making any /proc, /sys dentry that's under that path shiftable, and
unprivileged users can just create user namespaces and read /proc/*
and all the other stuff that doesn't have capable() related to the
init_user_ns host...

  what if you have paths like /filesystem0/uidshiftedY/dir,
/filesystem0/uidshiftedX/dir , /filesystem0/notshifted/dir 
where some of them are also bind mounts that point to same dentry ?


Also, you create a totally new user namespace interface here! by making
your own new interface we just lose the notion of init_user_ns and its
children and mapping ?

I'm not sure of the implication of all this... your user namespace
mapping is not related at all to init_user_ns! it seems that it has
its own init_user_ns ?   does a capable() check now on a shifted
filesystem relates to that and hence to your mapping or to the real
init_user_ns ?


> There's a bit of an open question of whether it should have vfs
> changes: the way the struct file f_inode and f_ops are hijacked is a
> bit nasty and perhaps d_select_inode() could be made a bit cleverer to
> help us here instead.

I'm not sure if this PoC works... but you sure you didn't introduce
a serious vulnerability here ? you use a new mapping and you update
current_fsuid() creds up, which is global on any fs operation, so may
be: lets operate on any inode, update our current_fsuid()... and
access the rest of *unshifted filesystems*... !?

The worst thing is that current_fsuid() does not follow now the
/proc/self/uid_map interface! this is a serious vulnerability and a mix
of the current semantics... it's updated but using other rules...?

For overlayfs I did write an expriment but for me it's not an overlayfs
or another new filesystem problem... we are manipulating UID/GID
identities...

It would have been better if you did send this as a separate thread.
It was a vfs:userns RFC fix which if we continue we turn it into a
complicated thing! implement another new light filesystem with
userns... (overlayfs...)

Will follow up if the appropriate thread is created, not here, I guess
it's ok ?

> James
> 

Thank you for your feedback!


-- 
Djalal Harouni
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1399413

On Wed, 2016-05-11 at 17:42 +0100, Djalal Harouni wrote:
> On Tue, May 10, 2016 at 04:36:56PM -0700, James Bottomley wrote:
> > On Thu, 2016-05-05 at 18:08 -0400, James Bottomley wrote:
> [...]
> > > 
> > > OK, so the way attributes are populated on an inode is via 
> > > getattr.  You intercept that, you change the inode owner and 
> > > group that are installed on the inode.  That means that when you 
> > > list the directory, you see the shift and the shifted uid/gid are 
> > > used to check permissions for vfs_open().
> > 
> > Just to illustrate how this could be done, here's a functional 
> > proof of concept for a uid/gid shifting bind mount equivalent. 
> >  It's not actually a proper bind mount because it has to 
> > manufacture its own inodes.  As you can see, it can only be used by 
> > root, it will shift all the uid/gid bits as well as the permission 
> > comparisons.  It operates on subtrees, so it can shift the 
> > uids/gids on any filesystem or part of one and because the shifts 
> > are per superblock, it could actually shift the same subtree for 
> > multiple users on different shifts.  Best of all, it requires no 
> > vfs changes at all, being entirely implemented inside its own
> > filesystem type.
> 
> First, I guess this should be in a separate thread.. this way this 
> RFC was just hijacked!
> 
> Obviously as you say later in your response it may require a VFS
> change... 

I thought it may but viro didn't rip my head off for shifting the file
operations and inode, so perhaps it's OK as is.

> You have just consumed all inodes... what about containers or small 
> apps that are spawned quickly... it can even used maybe as a DoS... 
>  maybe you endup reporting different inode numbers... ?

Please explain?  Shiftfs deliberately doesn't populate its dentry
cache, so it basically has the same number inodes and dentries in use
as the lower filesystem would ordinarily have.

> 
> > You use it just like bind mount:
> > 
> > mount -t shiftfs <source> <target>
> > 
> > except that it takes uidshift=x:y:z and gidshift=x:y:z multiple
> > times
> > as options.  It's currently not recursive and it definitely needs
> > polishing to show things like mount options and be properly Kconfig
> > using.
> 
> why it's not recursive ? and what if you have circular bind mounts ? 

Because, as I said, it's a proof of concept.  It can easily have MS_REC
semantics added.

> Hmm anyway you are mounting this on behalf of filesystems, so if you 
> add the recursive thing, you will just probably make everything 
> worse, by making any /proc, /sys dentry that's under that path 
> shiftable, and unprivileged users can just create user namespaces and 
> read /proc/* and all the other stuff that doesn't have capable() 
> related to the init_user_ns host...

That's up to the admin who does the shifting.  Recursive would be an
option if added.

>   what if you have paths like /filesystem0/uidshiftedY/dir,
> /filesystem0/uidshiftedX/dir , /filesystem0/notshifted/dir 
> where some of them are also bind mounts that point to same dentry ?

Without recursive semantics, you see the underlying inode.  With them,
you see the upper vfsmnts.  Shiftfs isn't idempotent, so you would need
to be careful about nesting.  However, that's an admin problem.

> Also, you create a totally new user namespace interface here! by 
> making your own new interface we just lose the notion of init_user_ns 
> and its children and mapping ?

I don't quite understand this; the only use of the init_user_ns is the
capable(CAP_SYS_ADMIN) in fill_super which is how only the real admin
can mount at a shifted uid/gid.  Otherwise, there's no need to see into
the userns because filesystems see the kuid_t/kgid_t which is what I'm
shifting.

> I'm not sure of the implication of all this... your user namespace
> mapping is not related at all to init_user_ns! it seems that it has
> its own init_user_ns ?   does a capable() check now on a shifted
> filesystem relates to that and hence to your mapping or to the real
> init_user_ns ?

capable(CAP_SYS_ADMIN) == ns_capable(&init_user_ns, CAP_SYS_ADMIN)

Or is there a misunderstanding here about how user namespaces work
inside the kernel?  The design is that the ID shift is done as you
cross the kernel boundary, so a filesystem, being usually all in-kernel
operating via the VFS interfaces, ideally never needs to make any
from_kuid/make_kuid calls.  However, there are ways filesystems can
send data across the kernel/user bounary outside of the usual vfs
interfaces (ioctls being the most usual one) so in that specific code,
they have to do the kuid_t to uid_t changes themselves.  Shiftfs never
sends data to the user outside of the VFS so it never needs to do this
and can operate entirely on kuid_ts.

> > There's a bit of an open question of whether it should have vfs
> > changes: the way the struct file f_inode and f_ops are hijacked is 
> > a bit nasty and perhaps d_select_inode() could be made a bit 
> > cleverer to help us here instead.
> 
> I'm not sure if this PoC works... but you sure you didn't introduce
> a serious vulnerability here ? you use a new mapping and you update
> current_fsuid() creds up, which is global on any fs operation, so may
> be: lets operate on any inode, update our current_fsuid()... and
> access the rest of *unshifted filesystems*... !?

The credentials are per thread, so it's a standard way of doing
credential shifting and no other threads of execution in the same task
get access. As long as you bound the override_creds()/revert_creds()
pairs within the kernel, you're safe.

> The worst thing is that current_fsuid() does not follow now the
> /proc/self/uid_map interface! this is a serious vulnerability and a 
> mix of the current semantics... it's updated but using other
> rules...?

current_fsuid() is aready mapped via the userns; it's already a kuid_t
at its final value.  Shifting that is what you want to remap underlying
volume uid/gid's.  The uidmap/gidmap inputs to this are shifts on the
final underlying uid/gids.

So, if I've got a uid_map in a userns of 0:100000:1000 which remaps all
the privileged ids down to 100000, but I have a volume which still has
realids, I can mount that volume using shiftfs with
uidmap=0:100000:1000 and it will allow this userns to read and write
the volume through its remapped ids.

> For overlayfs I did write an expriment but for me it's not an 
> overlayfs or another new filesystem problem... we are manipulating 
> UID/GID identities...
> 
> It would have been better if you did send this as a separate thread.
> It was a vfs:userns RFC fix which if we continue we turn it into a
> complicated thing! implement another new light filesystem with
> userns... (overlayfs...)
> 
> Will follow up if the appropriate thread is created, not here, I 
> guess it's ok ?

Well, I can resend the patch as a separate thread when I've fixed some
of the problems viro pointed out.

James

> > James
> > 
> 
> Thank you for your feedback!
> 
> 

[toc] | [prev] | [next] | [standalone]

#1394789

Quoting Djalal Harouni (tixxdz@gmail.com):
> This is version 2 of the VFS:userns support portable root filesystems
> RFC. Changes since version 1:
> 
> * Update documentation and remove some ambiguity about the feature.
>   Based on Josh Triplett comments.
> * Use a new email address to send the RFC :-)
> 
> 
> This RFC tries to explore how to support filesystem operations inside
> user namespace using only VFS and a per mount namespace solution. This
> allows to take advantage of user namespace separations without
> introducing any change at the filesystems level. All this is handled
> with the virtual view of mount namespaces.

Given your use case, is there any way we could work in some tradeoffs
to protect the host?  What I'm thinking is that containers can all
share devices uid-mapped at will, however any device mounted with
uid shifting cannot be used by the inital user namespace.  Or maybe
just non-executable in that case, as you'll need enough access to
the fs to set up the containers you want to run.

So if /dev/sda1 is your host /, you have to use /dev/sda2 as the
container rootfs source.  Mount it under /containers with uid
shifting.  Now all containers regardless of uid mappings see
the shifted fs contents.  But the host root cannot be tricked by
files on it, as /dev/sda2 is non-executable as far as it is
concerned.

Just a thought.

[toc] | [prev] | [next] | [standalone]

#1395900

Hi,

On Wed, May 04, 2016 at 11:30:09PM +0000, Serge Hallyn wrote:
> Quoting Djalal Harouni (tixxdz@gmail.com):
> > This is version 2 of the VFS:userns support portable root filesystems
> > RFC. Changes since version 1:
> > 
> > * Update documentation and remove some ambiguity about the feature.
> >   Based on Josh Triplett comments.
> > * Use a new email address to send the RFC :-)
> > 
> > 
> > This RFC tries to explore how to support filesystem operations inside
> > user namespace using only VFS and a per mount namespace solution. This
> > allows to take advantage of user namespace separations without
> > introducing any change at the filesystems level. All this is handled
> > with the virtual view of mount namespaces.
> 
> Given your use case, is there any way we could work in some tradeoffs
> to protect the host?  What I'm thinking is that containers can all
> share devices uid-mapped at will, however any device mounted with
> uid shifting cannot be used by the inital user namespace.  Or maybe
> just non-executable in that case, as you'll need enough access to
> the fs to set up the containers you want to run.
> 
> So if /dev/sda1 is your host /, you have to use /dev/sda2 as the
> container rootfs source.  Mount it under /containers with uid
> shifting.  Now all containers regardless of uid mappings see
> the shifted fs contents.  But the host root cannot be tricked by
> files on it, as /dev/sda2 is non-executable as far as it is
> concerned.
Of course the whole setup is based on the container manager to setup
the right mount namespace, clean mounts, etc then pivot root, boot or
whatever...

Now I guess we can achieve what you want with MS_SLAVE|MS_REC on / ?

You create a new mount/pid... namespaces with shift flags, but you are still
in init_user_ns, you remount your / with MS_SLAVE|MS_REC, then you
create new mount/pid namespaces with shift flag (two mount namespaces
here if you don't want to race setting MS_SLAVE flag and creating mount
namespace and you don't trust other processes... or you want the same nested
setup...)

This second new secure mount namespace will be the one that you will use
to setup the container, device nodes, loops...  fs that you want into the
container (probably with shift options) and also filesystems that you can't
mount inside user namespaces nor want them to show up or propagate into
host, you may also want to umount stuff too or remount to change mount
options too.., etc anyway here call it the cleaning of the mount namespace.

Now during this phase, when you mount and prepare these file systems,
mount them with noexec flag first, then remount later with exec, or delay
the mounting just before you do a new clone(CLONE_NEWUSER...). During this
phase the container manager should get the device that you want to be
shared from input or argument, and it will only mount it and prepare
it inside new mount namespaces or containers and make sure that it will
never be propagated back...

After clone(CLONE_NEWUSER|CLONE_NEWNS|CLONE_MNTNS_SHIFT_UIDGID), setup
the user namespace mapping, I guess you drop capabilities, do setuid()
or whatever and start the PID 1 or the app of the container.

Now and to not confuse more Dave, since he doesn't like the idea of
a shared backing device, and me neither for obvious reasons! the shared
device should not be used for a rootfs, maybe for read-only user shared
data, or shared config, that's it... but for real rootfs they should have
their own *different* backing device! unless you know what you are doing
hehe I don't want to confuse people, and I just lack time, will also
respond to Dave email.


> Just a thought.

You think it will solve the case ?


Thanks for your comments!

-- 
Djalal Harouni
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1397167

Quoting Djalal Harouni (tixxdz@gmail.com):
> Hi,
> 
> On Wed, May 04, 2016 at 11:30:09PM +0000, Serge Hallyn wrote:
> > Quoting Djalal Harouni (tixxdz@gmail.com):
> > > This is version 2 of the VFS:userns support portable root filesystems
> > > RFC. Changes since version 1:
> > > 
> > > * Update documentation and remove some ambiguity about the feature.
> > >   Based on Josh Triplett comments.
> > > * Use a new email address to send the RFC :-)
> > > 
> > > 
> > > This RFC tries to explore how to support filesystem operations inside
> > > user namespace using only VFS and a per mount namespace solution. This
> > > allows to take advantage of user namespace separations without
> > > introducing any change at the filesystems level. All this is handled
> > > with the virtual view of mount namespaces.
> > 
> > Given your use case, is there any way we could work in some tradeoffs
> > to protect the host?  What I'm thinking is that containers can all
> > share devices uid-mapped at will, however any device mounted with
> > uid shifting cannot be used by the inital user namespace.  Or maybe
> > just non-executable in that case, as you'll need enough access to
> > the fs to set up the containers you want to run.
> > 
> > So if /dev/sda1 is your host /, you have to use /dev/sda2 as the
> > container rootfs source.  Mount it under /containers with uid
> > shifting.  Now all containers regardless of uid mappings see
> > the shifted fs contents.  But the host root cannot be tricked by
> > files on it, as /dev/sda2 is non-executable as far as it is
> > concerned.
> Of course the whole setup is based on the container manager to setup
> the right mount namespace, clean mounts, etc then pivot root, boot or
> whatever...
> 
> Now I guess we can achieve what you want with MS_SLAVE|MS_REC on / ?
> 
> You create a new mount/pid... namespaces with shift flags, but you are still
> in init_user_ns, you remount your / with MS_SLAVE|MS_REC, then you
> create new mount/pid namespaces with shift flag (two mount namespaces
> here if you don't want to race setting MS_SLAVE flag and creating mount
> namespace and you don't trust other processes... or you want the same nested
> setup...)
> 
> This second new secure mount namespace will be the one that you will use
> to setup the container, device nodes, loops...  fs that you want into the
> container (probably with shift options) and also filesystems that you can't
> mount inside user namespaces nor want them to show up or propagate into
> host, you may also want to umount stuff too or remount to change mount
> options too.., etc anyway here call it the cleaning of the mount namespace.
> 
> Now during this phase, when you mount and prepare these file systems,
> mount them with noexec flag first, then remount later with exec, or delay
> the mounting just before you do a new clone(CLONE_NEWUSER...). During this
> phase the container manager should get the device that you want to be
> shared from input or argument, and it will only mount it and prepare
> it inside new mount namespaces or containers and make sure that it will
> never be propagated back...
> 
> After clone(CLONE_NEWUSER|CLONE_NEWNS|CLONE_MNTNS_SHIFT_UIDGID), setup
> the user namespace mapping, I guess you drop capabilities, do setuid()
> or whatever and start the PID 1 or the app of the container.
> 
> Now and to not confuse more Dave, since he doesn't like the idea of
> a shared backing device, and me neither for obvious reasons! the shared
> device should not be used for a rootfs, maybe for read-only user shared
> data, or shared config, that's it... but for real rootfs they should have
> their own *different* backing device! unless you know what you are doing
> hehe I don't want to confuse people, and I just lack time, will also
> respond to Dave email.

Yes.  We're saying slightly different things.  You're saying that the admin
should assign different backing stores for containers.  I'm saying perhaps
the kernel should enforce that, because $leaks.  Let's say the host admin
did a perfect setup of a container with shifted uids.  Now he wants to
run a quick ps in the container...  he does it in a way that leaks a
/proc/pid reference into the container so that (evil) container root can
use /proc/pid/root/ to get a toehold into the host /.  Does he now have
shifted access to that?

I think if we say "this blockdev will have shifted uids in /proc/$pid/ns/user",
then immediately that blockdev becomes not-readable (or not-executable)
in any namespace which does not have /proc/$pid/ns/user as an ancestor.
With obvious check as in write-versus-execute exclusion that you cannot
mark the blockdev shifted if ancestor user_ns already has a file open for
execute.

BTW, perhaps I should do this in a separate email, but here is how I would
expect to use this:

1. Using zfs: I create a bare (unshifted) rootfs fs1.   When I want to
create a new container, I zfs clone fs1 to fs2, and let the container
use fs2 shifted.  No danger to fs1 since fs2 is cow.  Same with btrfs.

2. Using overlay: I create a bare (unshifted) rootfs fs1.  When I want
to create a new container, I I mount fs1 read-only and shifted as base
layer, then fs2 as the rw layer.

The point here is that the zfs clone plus container start takes (for a
600-800M rootfs) about .5 seconds on my laptop, while the act of shifting
all the uids takes another 2 seconds.  So being able do this without
manually shifting would be a huge improvement for cases (i.e. docker)
where you do lots and lots of quick deploys.

> > Just a thought.
> 
> You think it will solve the case ?
> 
> 
> Thanks for your comments!
> 
> -- 
> Djalal Harouni
> http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1397973

On Mon, May 09, 2016 at 04:26:30PM +0000, Serge Hallyn wrote:
> Quoting Djalal Harouni (tixxdz@gmail.com):
> > Hi,
[...]
> > 
> > After clone(CLONE_NEWUSER|CLONE_NEWNS|CLONE_MNTNS_SHIFT_UIDGID), setup
> > the user namespace mapping, I guess you drop capabilities, do setuid()
> > or whatever and start the PID 1 or the app of the container.
> > 
> > Now and to not confuse more Dave, since he doesn't like the idea of
> > a shared backing device, and me neither for obvious reasons! the shared
> > device should not be used for a rootfs, maybe for read-only user shared
> > data, or shared config, that's it... but for real rootfs they should have
> > their own *different* backing device! unless you know what you are doing
> > hehe I don't want to confuse people, and I just lack time, will also
> > respond to Dave email.
> 
> Yes.  We're saying slightly different things.  You're saying that the admin
> should assign different backing stores for containers.  I'm saying perhaps
> the kernel should enforce that, because $leaks.  Let's say the host admin
> did a perfect setup of a container with shifted uids.  Now he wants to
> run a quick ps in the container...  he does it in a way that leaks a
> /proc/pid reference into the container so that (evil) container root can
> use /proc/pid/root/ to get a toehold into the host /.  Does he now have
> shifted access to that?

No. Assuming host / or its other mount points are not mounted with
vfs_shift_uids and vfs_shift_gids options. In this case no shift is
performed at all.

1) If you mount host / with vfs_shift_uids and vfs_shift_gids it's
like real root in init_user_ns does "chmod -R o+rwx /"... It does not make
sense and since no one can edit/remount mounts to change their options in
the mount namespace of init_user_ns, it's safe, and not available by
default.

2) That's why also filsystems must support this explicitly and not on
their behalf.

IMO the kernel is already enforcing this, so even if you assign different
backing stores to containers, you can't have shifted access there, unless
you explicitly tell the kernel that the mount is mean to be shifted by
adding vfs_shift_uids and vfs_shift_gids mount options.


> I think if we say "this blockdev will have shifted uids in /proc/$pid/ns/user",
> then immediately that blockdev becomes not-readable (or not-executable)
> in any namespace which does not have /proc/$pid/ns/user as an ancestor.

Hmm,

(1) This won't work since to do that you have to know in advance
/proc/$pid/ns/user and since file systems can't be mounted inside user
namespace this brings us to the same blocker ... ! and in our use case
we do want to shift UIDs/GIDs to just access inodes, no need to expose
the whole filesystem, root is responsible and filesystems stay safe.

(2)  Why complicate ? the kernel already supports this! and it's a
generic solution.

As said you can just create new mount namespaces, mount things there
private, slave... mount your blockdev that will be shifted by processes
that inherits that mount, you can even have intermediate mount namespaces
that you will forget/unref at any moment and where they are only used to
perform setup, and no other process/code can enter... You don't have
any leaks nothing! you control that piece of code.

If you want that blockdev to become not-readable or noexec in any
namespace which does not have /proc/$pid/ns/user as an ancestor,
the kernel allows a better interface, it allows that blockdev to not
even show up in any ancestor, by making use of mount namespaces and
MS_PRIVATE, MS_SLAVE... no one will even notice if the mount exists.

However if you want to access that blockdev for whatever reason, then
create a new mount namespace and use MS_PRIVATE, MS_SLAVE and all the
noexec flags and mount it.

Yes slightly different things, but I don't want to add complexity where
the interface already exists in the kernel...


> With obvious check as in write-versus-execute exclusion that you cannot
> mark the blockdev shifted if ancestor user_ns already has a file open for
> execute.

Please note here, that it's the same ancestor who will mark the blockdev
to be shifted, but  why the ancestor will keep at the same time a file
open in that filesystem that is mean to be shifted and later execute
through that fd a program that was just crafted by untrusted container ?!


For me the kernel already offers the interfaces no need to complicate
things or enforce it... As said in other responses, the design of these
patches is to just use what the kernel already provides.



> BTW, perhaps I should do this in a separate email, but here is how I would
> expect to use this:
> 
> 1. Using zfs: I create a bare (unshifted) rootfs fs1.   When I want to
> create a new container, I zfs clone fs1 to fs2, and let the container
> use fs2 shifted.  No danger to fs1 since fs2 is cow.  Same with btrfs.

Yes that would work, since fs1 is unshifted, the only requirement is
that fs2 should not reside on the same backing store of fs1 to not share
quota with fs1 (I'm not a ZFS user...) and you can always make the parent
of mount point fs2 or containers directories 0700... and root should not
go there and exec programs like it's not safe to go /hom/$user... and
exec...


> 2. Using overlay: I create a bare (unshifted) rootfs fs1.  When I want
> to create a new container, I I mount fs1 read-only and shifted as base
> layer, then fs2 as the rw layer.

Yes here you may share quota if all the fs2 rw layers of all containers
reside on the same backing store... but here the requirement is that fs1
should be mounted the first time with shifted uids/gids where fs1 resides
on ext4, btrfs, xfs or anyother filesystem that supports shifting. This
means you may have to mount fs1 on a different backing store say on
/root-fs0/lib/container-image-fs1/  with vfs_shit_uids/gids then use it
as a shared read-only lower layer.

Of course you may just use your host / as a readonly layer where you
mount it the first time with vfs_shift_uids/gids but as discussed above
that's not really safe unless that's not a shared user system, or you
know what you are doing...

These patches do not edit overlayfs. overlayfs support is transparent if
the underlaying filesystems, the upper and lower directories are on
filesystems that support vfs_shift_uids/vfs_shift_gids.

If we go with overlayfs, we make it an overlayfs problem where it needs
different approache related to union mounts which I noted in the cover
letter of this patches.


> The point here is that the zfs clone plus container start takes (for a
> 600-800M rootfs) about .5 seconds on my laptop, while the act of shifting
> all the uids takes another 2 seconds.  So being able do this without
> manually shifting would be a huge improvement for cases (i.e. docker)
> where you do lots and lots of quick deploys.
> 

That's one of the use cases of course! you can also verify the
integrity... and able to really make containers fs read-only without
the recursive chown...

Thank you for your feedback!

-- 
Djalal Harouni
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1394806

On Wed, May 04, 2016 at 04:26:46PM +0200, Djalal Harouni wrote:
> This is version 2 of the VFS:userns support portable root filesystems
> RFC. Changes since version 1:
> 
> * Update documentation and remove some ambiguity about the feature.
>   Based on Josh Triplett comments.
> * Use a new email address to send the RFC :-)
> 
> 
> This RFC tries to explore how to support filesystem operations inside
> user namespace using only VFS and a per mount namespace solution. This
> allows to take advantage of user namespace separations without
> introducing any change at the filesystems level. All this is handled
> with the virtual view of mount namespaces.

[...]

> As an example if the mapping 0:65535 inside mount namespace and outside
> is 1000000:1065536, then 0:65535 will be the range that we use to
> construct UIDs/GIDs mapping into init_user_ns and use it for on-disk
> data. They represent the persistent values that we want to write to the
> disk. Therefore, we don't keep track of any UID/GID shift that was applied
> before, it gives portability and allows to use the previous mapping
> which was freed for another root filesystem...

So let me get this straight. Two /isolated/ containers, different
UID/GID mappings, sharing the same files and directories. Create a
new file in a writeable directory in container 1, namespace
information gets stripped from on-disk uid/gid representation.

Container 2 then reads that shared directory, finds the file written
by container 1. As there is no no namespace component to the uid:gid
stored in the inode, we apply the current namespace shift to the VFS
inode uid/gid and so it maps to root in container 2 and we are
allowed to read it?

Unless I've misunderstood something in this crazy mapping scheme,
isn't this just a vector for unintentional containment breaches?

[...]

> Simple demo overlayfs, and  btrfs mounted with vfs_shift_uids and
> vfs_shift_gids. The overlayfs mounts will share the same upperdir. We
> create two user namesapces every one with its own mapping and where
> container-uid-2000000 will pull changes from container-uid-1000000
> upperdir automatically.

Ok, forget I asked - it's clearly intentional. This is beyond
crazy, IMO.

> 3) ROADMAP:
> ===========
> * Confirm current design, and make sure that the mapping is done
>   correctly.

How are you going to ensure that all filesystems behave the same,
and it doesn't get broken by people who really don't care about this
sort of crazy?

FWIW, having the VFS convert things to "on-disk format" is an
oxymoron - the "V" in VFS means "virtual" and has nothing to do with
disks or persistent storage formats. Indeed, let's convert the UID
to "on-disk" format for a network filesystem client....

.....
> * Add XFS support.

What is the problem here?

Next question: how does this work with uid/gid based quotas?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1394821

On Wed, May 4, 2016 at 5:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Wed, May 04, 2016 at 04:26:46PM +0200, Djalal Harouni wrote:
>> This is version 2 of the VFS:userns support portable root filesystems
>> RFC. Changes since version 1:
>>
>> * Update documentation and remove some ambiguity about the feature.
>>   Based on Josh Triplett comments.
>> * Use a new email address to send the RFC :-)
>>
>>
>> This RFC tries to explore how to support filesystem operations inside
>> user namespace using only VFS and a per mount namespace solution. This
>> allows to take advantage of user namespace separations without
>> introducing any change at the filesystems level. All this is handled
>> with the virtual view of mount namespaces.
>
> [...]
>
>> As an example if the mapping 0:65535 inside mount namespace and outside
>> is 1000000:1065536, then 0:65535 will be the range that we use to
>> construct UIDs/GIDs mapping into init_user_ns and use it for on-disk
>> data. They represent the persistent values that we want to write to the
>> disk. Therefore, we don't keep track of any UID/GID shift that was applied
>> before, it gives portability and allows to use the previous mapping
>> which was freed for another root filesystem...
>
> So let me get this straight. Two /isolated/ containers, different
> UID/GID mappings, sharing the same files and directories. Create a
> new file in a writeable directory in container 1, namespace
> information gets stripped from on-disk uid/gid representation.

I think the intent is a totally separate superblock for each
container.  Djalal, am I right?

The feature that seems to me to be missing is the ability to squash
uids.  I can imagine desktop distros wanting to mount removable
storage such that everything shows up (to permission checks and
stat()) as the logged-in user's uid but that the filesystem sees 0:0.
That can be done by shifting, but the distro would want everything
else on the filesystem to show up as the logged-in user as well.

That use case could also be handled by adding a way to tell a given
filesystem to completely opt out of normal access control rules and
just let a given user act as root wrt that filesystem (and be nosuid,
of course).  This would be a much greater departure from current
behavior, but would let normal users chown things on a removable
device, which is potentially nice.

--Andy

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web