[PATCH] mount.cifs: properly check for mount being in fstab when running setuid root

[Jeff Layton]

On Sun, 31 May 2009 18:34:18 -0700
Steve Langasek <vorlon at debian.org> wrote:

Ouch. Apologies Steve, I somehow missed your response a few months ago
when we were discussing this. I've committed the patch to master and
it's now in 3.5-test. It's not in an official release yet though, so I
consider this to still be open for discussion.

> Hi Jeff,
> 
> On Tue, May 26, 2009 at 07:51:43PM -0400, Jeff Layton wrote:
> > Ordinarily, I'd consider this a security problem, but I'm not aware of
> > any distro that ships mount.cifs as setuid binary. Therefore, I'm going
> > to go ahead and post this publically for discussion.
> 
> Both Debian and Ubuntu ship mount.cifs setuid, so I guess you didn't look
> very far afield.  But in any event, I don't see why you're claiming that
> there's a security problem here - you seem to just be objecting that
> unprivileged users can mount CIFS shares on directories they own, but this
> is by design.  Or have I overlooked some other security hole?
> 

Mea culpa, I didn't look too far afield when I proposed this. I now
realize that Debian/Ubuntu ship this program setuid root. This old
behavior was by design, but I believe it was implemented without fully
considering the possibilities. Here's one potential way to exploit it:

Suppose I have an unprivileged user (Alice) who mounts a filesystem
that's listed in /etc/fstab via the "user" option using
standard /bin/mount setuid behavior. Could be a USB hard disk or
something...

This filesystem contains a directory that happens to be owned by Bob,
another unprivileged user. Bob then uses a setuid mount.cifs to mount a
cifs filesystem on top of that directory. Now Alice cannot unmount her
USB hard disk cleanly.

Here's another handwavy one...many of the /proc/pid directories are
owned by unprivileged users. Could I set up a cifs share on another
machine, mount it on top of a directory under /proc/pid and use that as
part of a scheme to compromise something?

Now, both of these are a bit contrived for security issues, but I think
that setuid programs require a bit more due dilligence than has been
given here. The "policy" that /bin/mount follows for user mounts is a
good one since it requires that the admin explicitly enumerate where
mounts can occur.

My assertion is simply that if mount.cifs is installed setuid root then
it should not violate this policy by default. Conscious action should
be taken.

> > This means that it's currently not possible to set up user mounts the
> > standard way (by the admin, in /etc/fstab) and simultaneously protect
> > from an unprivileged user calling mount.cifs directly to mount a share
> > on any directory that that user owns.
> 
> And as a result, my understanding is that the former usage is not
> supported while the latter usage is.  That may be considered a bug, but I
> don't see how it's a security bug.
> 

Fair enough. I don't have a specific attack vector (other than the
handwavy ones I've outlined above). We are however dealing with a
setuid program so we have to be a bit careful with what we allow it to
do.

> > Mount helpers are never intended to be called directly, and shouldn't
> > offer any "extra" privileges over what /bin/mount allows.
> 
> Well, I think this is an unsubstantiated assertion.  I think that would be
> *a* valid policy, but clearly not everyone agrees with it or we wouldn't
> have the current mount.cifs behavior in question.
> 
> I'm not particularly attached to the current behavior personally, but I know
> there are users who expect this to work, and none of the alternatives you
> propose are a complete replacement.
> 

True...there is no direct replacement for this functionality.

The final patch that I merged contains a preprocessor switch to turn on
the old behavior for people who need it. I did not, however add an
autoconf option for it. We can do that if it makes your life easier.

One possibility for making the old behavior more safe might be to allow
unprivileged users to only mount onto directories that live
under /home or some other top-level directory. Care would have to be
taken to make sure that symlinks don't allow them to circumvent it, but
that might be reasonable.

-- 
Jeff Layton <jlayton at samba.org>