Quoting Miklos Szeredi ([email protected]):
> Here's a git tree of the unprivileged mounts patchset:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git unprivileged-mounts
>
> Could this be added to -mm (and dropped if it's in the way of
> something) for some testing and added visibility until it's reviewed
> by Christoph/Al?
>
> I'm not reposting the whole patchset, since it's essentially the same
> as the last submission, only updated to the latest git. But if
> somebody wants it I can post them.
>
> Thanks,
> Miklos
>
>
> Documentation/filesystems/fuse.txt | 88 ++++++++-
> Documentation/filesystems/proc.txt | 40 ++++
> fs/filesystems.c | 60 ++++++
> fs/fuse/inode.c | 21 ++
> fs/internal.h | 3 +-
> fs/namespace.c | 366 +++++++++++++++++++++++++++---------
> fs/pnode.c | 22 ++-
> fs/pnode.h | 2 +
> fs/super.c | 26 ---
> include/linux/fs.h | 7 +
> include/linux/mount.h | 4 +
> kernel/sysctl.c | 16 ++
> 12 files changed, 527 insertions(+), 128 deletions(-)
>
> Miklos Szeredi (10):
> unprivileged mounts: add user mounts to the kernel
> unprivileged mounts: allow unprivileged umount
> unprivileged mounts: propagate error values from clone_mnt
> unprivileged mounts: account user mounts
> unprivileged mounts: allow unprivileged bind mounts
> unprivileged mounts: allow unprivileged mounts
> unprivileged mounts: add sysctl tunable for "safe" property
> unprivileged mounts: make fuse safe
> unprivileged mounts: propagation: inherit owner from parent
> unprivileged mounts: add "no submounts" flag
Hi Miklos,
so on the bright side I pulled this tree today and it compiled and
passed ltp with no problems.
But then I played around a bit and found I could do the following:
(hmm, i'm trying to remember the exact order :)
as root:
mmount --bind -o user=500 /home/hallyn/etc/ /home/hallyn/etc/
mount --bind /mnt /mnt
mount --make-rshared /mnt
mount --bind /dev /mnt/dev
as hallyn:
mmount --bind /mnt /home/hallyn/etc/mnt
/usr/src/mmount-0.3/mmount --bind mnt/dev mnt/src
Now /mnt/src contained /dev.
Is this what we want? Do we want to tell the admin it's his fault for
not somehow forcing a slave relationship between /mnt and
/home/hallyn/etc/mnt? Except I don't think he can do that preemptively,
it has to be done after hallyn does the mmount.
So does that mean that if non-root user X does:
mount a b
where b is user=X but a is not, then if a is shared we should force it
to be mounted as slave at b?
-serge
"Serge E. Hallyn" <[email protected]> writes:
> Quoting Miklos Szeredi ([email protected]):
>> Here's a git tree of the unprivileged mounts patchset:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git
> unprivileged-mounts
>>
>> Could this be added to -mm (and dropped if it's in the way of
>> something) for some testing and added visibility until it's reviewed
>> by Christoph/Al?
>>
>> I'm not reposting the whole patchset, since it's essentially the same
>> as the last submission, only updated to the latest git. But if
>> somebody wants it I can post them.
>>
>> Thanks,
>> Miklos
>>
>>
>> Documentation/filesystems/fuse.txt | 88 ++++++++-
>> Documentation/filesystems/proc.txt | 40 ++++
>> fs/filesystems.c | 60 ++++++
>> fs/fuse/inode.c | 21 ++
>> fs/internal.h | 3 +-
>> fs/namespace.c | 366 +++++++++++++++++++++++++++---------
>> fs/pnode.c | 22 ++-
>> fs/pnode.h | 2 +
>> fs/super.c | 26 ---
>> include/linux/fs.h | 7 +
>> include/linux/mount.h | 4 +
>> kernel/sysctl.c | 16 ++
>> 12 files changed, 527 insertions(+), 128 deletions(-)
>>
>> Miklos Szeredi (10):
>> unprivileged mounts: add user mounts to the kernel
>> unprivileged mounts: allow unprivileged umount
>> unprivileged mounts: propagate error values from clone_mnt
>> unprivileged mounts: account user mounts
>> unprivileged mounts: allow unprivileged bind mounts
>> unprivileged mounts: allow unprivileged mounts
>> unprivileged mounts: add sysctl tunable for "safe" property
>> unprivileged mounts: make fuse safe
>> unprivileged mounts: propagation: inherit owner from parent
>> unprivileged mounts: add "no submounts" flag
>
> Hi Miklos,
>
> so on the bright side I pulled this tree today and it compiled and
> passed ltp with no problems.
>
> But then I played around a bit and found I could do the following:
>
> (hmm, i'm trying to remember the exact order :)
>
> as root:
> mmount --bind -o user=500 /home/hallyn/etc/ /home/hallyn/etc/
> mount --bind /mnt /mnt
> mount --make-rshared /mnt
> mount --bind /dev /mnt/dev
>
> as hallyn:
> mmount --bind /mnt /home/hallyn/etc/mnt
> /usr/src/mmount-0.3/mmount --bind mnt/dev mnt/src
You are using relative directory names here which makes it confusing.
I'm assuming you in /home/hallyn/etc ?
>
> Now /mnt/src contained /dev.
>
> Is this what we want?
I don't think so.
I think the simplest answer is to not allow mounting of shared
subtrees controlled by a different user.
Serge I think you are right downgrading the mount from shared to slave
looks like the sane thing to do if the mount owners match.
> Do we want to tell the admin it's his fault for
> not somehow forcing a slave relationship between /mnt and
> /home/hallyn/etc/mnt? Except I don't think he can do that preemptively,
> it has to be done after hallyn does the mmount.
>
> So does that mean that if non-root user X does:
>
> mount a b
>
> where b is user=X but a is not, then if a is shared we should force it
> to be mounted as slave at b?
>
> -serge
Quoting Eric W. Biederman ([email protected]):
> "Serge E. Hallyn" <[email protected]> writes:
>
> > Quoting Miklos Szeredi ([email protected]):
> >> Here's a git tree of the unprivileged mounts patchset:
> >>
> >> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git
> > unprivileged-mounts
> >>
> >> Could this be added to -mm (and dropped if it's in the way of
> >> something) for some testing and added visibility until it's reviewed
> >> by Christoph/Al?
> >>
> >> I'm not reposting the whole patchset, since it's essentially the same
> >> as the last submission, only updated to the latest git. But if
> >> somebody wants it I can post them.
> >>
> >> Thanks,
> >> Miklos
> >>
> >>
> >> Documentation/filesystems/fuse.txt | 88 ++++++++-
> >> Documentation/filesystems/proc.txt | 40 ++++
> >> fs/filesystems.c | 60 ++++++
> >> fs/fuse/inode.c | 21 ++
> >> fs/internal.h | 3 +-
> >> fs/namespace.c | 366 +++++++++++++++++++++++++++---------
> >> fs/pnode.c | 22 ++-
> >> fs/pnode.h | 2 +
> >> fs/super.c | 26 ---
> >> include/linux/fs.h | 7 +
> >> include/linux/mount.h | 4 +
> >> kernel/sysctl.c | 16 ++
> >> 12 files changed, 527 insertions(+), 128 deletions(-)
> >>
> >> Miklos Szeredi (10):
> >> unprivileged mounts: add user mounts to the kernel
> >> unprivileged mounts: allow unprivileged umount
> >> unprivileged mounts: propagate error values from clone_mnt
> >> unprivileged mounts: account user mounts
> >> unprivileged mounts: allow unprivileged bind mounts
> >> unprivileged mounts: allow unprivileged mounts
> >> unprivileged mounts: add sysctl tunable for "safe" property
> >> unprivileged mounts: make fuse safe
> >> unprivileged mounts: propagation: inherit owner from parent
> >> unprivileged mounts: add "no submounts" flag
> >
> > Hi Miklos,
> >
> > so on the bright side I pulled this tree today and it compiled and
> > passed ltp with no problems.
> >
> > But then I played around a bit and found I could do the following:
> >
> > (hmm, i'm trying to remember the exact order :)
> >
> > as root:
> > mmount --bind -o user=500 /home/hallyn/etc/ /home/hallyn/etc/
> > mount --bind /mnt /mnt
> > mount --make-rshared /mnt
> > mount --bind /dev /mnt/dev
> >
> > as hallyn:
> > mmount --bind /mnt /home/hallyn/etc/mnt
> > /usr/src/mmount-0.3/mmount --bind mnt/dev mnt/src
>
> You are using relative directory names here which makes it confusing.
> I'm assuming you in /home/hallyn/etc ?
Sorry, yeah.
> > Now /mnt/src contained /dev.
> >
> > Is this what we want?
>
> I don't think so.
>
> I think the simplest answer is to not allow mounting of shared
> subtrees controlled by a different user.
>
> Serge I think you are right downgrading the mount from shared to slave
> looks like the sane thing to do if the mount owners match.
I assume you mean "if the mount owners don't match"?
Miklos, what do you think?
The next question then becomes, how can we prove to ourselves that that
closes the last security hole with unprivileged mounts? So long as
we treat each mount event as a piece of information and look at it as an
information flow problem, maybe we can actually come up with a good
description of the logic that is implemented and show that there is no
way a user can "leak" info... (where a leak is a mount event, a
violation of intended DAC on open(file) or mkdir, etc)
-serge
On Thu, 7 Aug 2008, Serge E. Hallyn wrote:
> Quoting Eric W. Biederman ([email protected]):
> > "Serge E. Hallyn" <[email protected]> writes:
> > > so on the bright side I pulled this tree today and it compiled and
> > > passed ltp with no problems.
> > >
> > > But then I played around a bit and found I could do the following:
> > >
> > > (hmm, i'm trying to remember the exact order :)
> > >
> > > as root:
> > > mmount --bind -o user=500 /home/hallyn/etc/ /home/hallyn/etc/
> > > mount --bind /mnt /mnt
> > > mount --make-rshared /mnt
> > > mount --bind /dev /mnt/dev
> > >
> > > as hallyn:
> > > mmount --bind /mnt /home/hallyn/etc/mnt
> > > /usr/src/mmount-0.3/mmount --bind mnt/dev mnt/src
> >
> > You are using relative directory names here which makes it confusing.
> > I'm assuming you in /home/hallyn/etc ?
>
> Sorry, yeah.
>
> > > Now /mnt/src contained /dev.
> > >
> > > Is this what we want?
> >
> > I don't think so.
> >
> > I think the simplest answer is to not allow mounting of shared
> > subtrees controlled by a different user.
> >
> > Serge I think you are right downgrading the mount from shared to slave
> > looks like the sane thing to do if the mount owners match.
>
> I assume you mean "if the mount owners don't match"?
>
> Miklos, what do you think?
Sorry about the late reply: I was on a long summer vacation...
Serge, thanks for spotting this: it looks indeed a nasty hole! I also
agree about the solution.
> The next question then becomes, how can we prove to ourselves that that
> closes the last security hole with unprivileged mounts? So long as
> we treat each mount event as a piece of information and look at it as an
> information flow problem, maybe we can actually come up with a good
> description of the logic that is implemented and show that there is no
> way a user can "leak" info... (where a leak is a mount event, a
> violation of intended DAC on open(file) or mkdir, etc)
"Information flow problem" doesn't mean much to me (I'm actually an
electric engineer, who ended up doing programming for living ;)
But yeah, we should think this over very carefully. Especially
interaction with mount propagation, which has very complicated and
sometimes rather counter-intuitive semantics.
Thanks,
Miklos
Quoting Miklos Szeredi ([email protected]):
> On Thu, 7 Aug 2008, Serge E. Hallyn wrote:
> > Quoting Eric W. Biederman ([email protected]):
> > > "Serge E. Hallyn" <[email protected]> writes:
> > > > so on the bright side I pulled this tree today and it compiled and
> > > > passed ltp with no problems.
> > > >
> > > > But then I played around a bit and found I could do the following:
> > > >
> > > > (hmm, i'm trying to remember the exact order :)
> > > >
> > > > as root:
> > > > mmount --bind -o user=500 /home/hallyn/etc/ /home/hallyn/etc/
> > > > mount --bind /mnt /mnt
> > > > mount --make-rshared /mnt
> > > > mount --bind /dev /mnt/dev
> > > >
> > > > as hallyn:
> > > > mmount --bind /mnt /home/hallyn/etc/mnt
> > > > /usr/src/mmount-0.3/mmount --bind mnt/dev mnt/src
> > >
> > > You are using relative directory names here which makes it confusing.
> > > I'm assuming you in /home/hallyn/etc ?
> >
> > Sorry, yeah.
> >
> > > > Now /mnt/src contained /dev.
> > > >
> > > > Is this what we want?
> > >
> > > I don't think so.
> > >
> > > I think the simplest answer is to not allow mounting of shared
> > > subtrees controlled by a different user.
> > >
> > > Serge I think you are right downgrading the mount from shared to slave
> > > looks like the sane thing to do if the mount owners match.
> >
> > I assume you mean "if the mount owners don't match"?
> >
> > Miklos, what do you think?
>
> Sorry about the late reply: I was on a long summer vacation...
>
> Serge, thanks for spotting this: it looks indeed a nasty hole! I also
> agree about the solution.
Are you implementing it, or did you want me to?
> > The next question then becomes, how can we prove to ourselves that that
> > closes the last security hole with unprivileged mounts? So long as
> > we treat each mount event as a piece of information and look at it as an
> > information flow problem, maybe we can actually come up with a good
> > description of the logic that is implemented and show that there is no
> > way a user can "leak" info... (where a leak is a mount event, a
> > violation of intended DAC on open(file) or mkdir, etc)
>
> "Information flow problem" doesn't mean much to me (I'm actually an
> electric engineer, who ended up doing programming for living ;)
>
> But yeah, we should think this over very carefully. Especially
> interaction with mount propagation, which has very complicated and
> sometimes rather counter-intuitive semantics.
I know we discussed before about whether a propagated mount from a
non-user mount to a user mount should end up being owned by the user
or not. I don't recall (and am not checking the code at the moment
as your tree is sitting elsewhere) whether we mark the propagated
tree with the right nosuid and nodev flags, or whether we call it
a user mount or not.
-serge
On Wed, 27 Aug 2008, Serge E. Hallyn wrote:
> Quoting Miklos Szeredi ([email protected]):
> > Serge, thanks for spotting this: it looks indeed a nasty hole! I also
> > agree about the solution.
>
> Are you implementing it, or did you want me to?
I'll implement it.
> > But yeah, we should think this over very carefully. Especially
> > interaction with mount propagation, which has very complicated and
> > sometimes rather counter-intuitive semantics.
>
> I know we discussed before about whether a propagated mount from a
> non-user mount to a user mount should end up being owned by the user
> or not. I don't recall (and am not checking the code at the moment
> as your tree is sitting elsewhere) whether we mark the propagated
> tree with the right nosuid and nodev flags, or whether we call it
> a user mount or not.
If the destination is a user mount, then
- the propagated mount(s) will be owned by the same user as the destination
- the propagated mount(s) will inherit 'nosuid' from the destination
I remember also thinking about 'nodev' and why it doesn't need similar
treatment to 'nosuid'. The reasoning was that 'nodev' is safe as long
as permissions are enforced, namespace shuffling cannot make it
insecure. Does that sound correct?
Thanks,
Miklos
Quoting Miklos Szeredi ([email protected]):
> On Wed, 27 Aug 2008, Serge E. Hallyn wrote:
> > Quoting Miklos Szeredi ([email protected]):
> > > Serge, thanks for spotting this: it looks indeed a nasty hole! I also
> > > agree about the solution.
> >
> > Are you implementing it, or did you want me to?
>
> I'll implement it.
Ok, thanks. I look forward to playing around with it when you publish
the resulting git tree :)
> > > But yeah, we should think this over very carefully. Especially
> > > interaction with mount propagation, which has very complicated and
> > > sometimes rather counter-intuitive semantics.
> >
> > I know we discussed before about whether a propagated mount from a
> > non-user mount to a user mount should end up being owned by the user
> > or not. I don't recall (and am not checking the code at the moment
> > as your tree is sitting elsewhere) whether we mark the propagated
> > tree with the right nosuid and nodev flags, or whether we call it
> > a user mount or not.
>
> If the destination is a user mount, then
>
> - the propagated mount(s) will be owned by the same user as the destination
> - the propagated mount(s) will inherit 'nosuid' from the destination
>
> I remember also thinking about 'nodev' and why it doesn't need similar
> treatment to 'nosuid'. The reasoning was that 'nodev' is safe as long
> as permissions are enforced, namespace shuffling cannot make it
> insecure. Does that sound correct?
Yes that sounds correct, thanks for the refresher.
-serge