Re: kern/29360: vfs.generic.usermount and mount(8) general questions

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR kern/29360; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Elad Efrat <elad%NetBSD.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, tech-kern%NetBSD.org@localhost
Subject: Re: kern/29360: vfs.generic.usermount and mount(8) general questions
Date: Sun, 6 Sep 2009 11:20:09 +0200

 On Sun, Sep 06, 2009 at 02:01:44AM -0400, Elad Efrat wrote:
 > Hi,
 > 
 > I just came across this PR.
 > 
 > The check that a non-root user owns the mount-point directory was
 > introduced way before vfs.generic.usermount. In fact, it seems that it
 > actually removed the root check, and allowed non-root users to freely
 > mount file-systems:
 > 
 >     
 > http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/vfs_syscalls.c.diff?r1=1.42&r2=1.43&f=h
 
 Yes, vfs.generic.usermount was introduced later, because of security issues
 that usermounts could cause. AFAIK the know security issues with
 usermounts are fixed, but still it's better to have it disabled on systems
 where it's not needed.
 
 
 > 
 > In other words, I don't see a direct relation between the two.
 > 
 > I believe your (the submitter's) suggestion makes a lot of sense: if
 > we only care about read access to the device file when mounting as a
 > non-root user, why should we care about more than write access (i.e.,
 > ownership) for the mount-point in the same scenario?
 > 
 > What I suggest is that since we already have a check to ensure the
 > user is allowed to mount a file-system, we should replace the
 > following code in kern/vfs_syscalls.c:
 > 
 > 309: /*
 > 310:  * If the user is not root, ensure that they own the directory
 > 311:  * onto which we are attempting to mount.
 > 312:  */
 > 313: if ((error = VOP_GETATTR(vp, &va, l->l_cred)) != 0 ||
 > 314:     (va.va_uid != kauth_cred_geteuid(l->l_cred) &&
 > 315:     (error = kauth_authorize_generic(l->l_cred,
 > 316:     KAUTH_GENERIC_ISSUSER, NULL)) != 0)) {
 > 317:         return error;
 > 318: }
 > 
 > With something like the following:
 > 
 > /* Ensure that the user can write to the mount-point. */
 > if ((error = VOP_ACCESS(vp, VWRITE, l->l_cred)) != 0)
 >     return error;
 > 
 > Does anyone see any drawbacks to this approach? If not, I'll change
 > the relevant code.
 
 Yes, that would mean a user could mount his own FS over e.g. /tmp, or
 /var/mail. that's bad.
 I think that checking the user owns the mount point is the right thing to do.
 I think a sysctl to control whenever to check for group ownerchip instead
 of user ownerchip would work, though. It's up to the admin to carefully
 choose a group for devices and mount points :)
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --