Date: Tue, 2 Oct 2001 10:11:51 -0400 (EDT) From: Robert Watson <rwatson@FreeBSD.org> To: arch@FreeBSD.org Subject: nfs_lock.c and rpc.lockd: request for enlightenment on apparent evil Message-ID: <Pine.NEB.3.96L.1011002095551.98303A-100000@fledge.watson.org>
next in thread | raw e-mail | index | archive | help
As part of the on-going capabilities development work, all kernel access
control checks are being reviewed for correctness and consistency. Right
now, there's some un-pretty code in the new NFS locking code:
/*
* XXX Hack to temporarily allow this process (regardless of it's creds)
* to open the fifo we need to write to. vn_open() really should
* take a ucred (and once it does, this code should be fixed to use
* proc0's ucred.
*/
saved_uid = p->p_ucred->cr_uid;
p->p_ucred->cr_uid = 0; /* temporarly run the vn_open as root */
That's pretty ugly (especially because that ucred is shared). As the
comment indicates, that's probably relatively fixable -- either we can
make vn_open.c use a more appropriate credential (perhaps cred0, perhaps
one cached from the source of the fifo, or perhaps even keeping the fifo
open so that it doesn't have to be reopened -- we can find something).
But it gets worse:
/* Let root, or someone who once was root (lockd generally
* switches to the daemon uid once it is done setting up) make
* this call.
*
* XXX This authorization check is probably not right.
*/
if ((error = suser(p)) != 0 && p->p_ucred->cr_svuid != 0)
return (error);
It should be noted that this is probably entirely the wrong approach to
the problem. In general, only the effective uid should be used for
authorization. If the process has an svuid set to something other than
its ruid or euid, it can always swap them around -- probably rpc.lockd
should simply set its effective uid back to 0 when it wants to perform
this action, rather than remaining using daemon_uid. There are certainly
advantages to running as 'daemon', including reducing the risk of using
root privilege in an un-anticipated way. However, if the goal was to
prevent a compromise of rpc.lockd from resulting in root compromise, that
fails, since the saved uid is set (and relied on :-).
However, I'm not very familiar with the implementation here, and am only
just starting to get a grasp on it. It would be much faster for someone
whos familiar with it (Alfred?) to look at my comments and tell me if
they're off-mark. Specifically, is the following proposal going to cause
problems due to other use of credentials:
Remove the above svuid clause, relying purely on suser() (or in the
capabilities case, appropriate privilege), and modify rpc.lockd to
restore the saved uid using seteuid(), and then restore the daemon uid
using setuid() on either side of the call.
For the other case above, I have do do a bit more reading before
suggesting a course of action, but in general, setting root privileges on
a ucred to perform an action in this way is a bad idea. It's even worse
because it's a shared ucred, which sets root privilege for all other
processes sharing the ucred for the duration of the call, as well as any
open files with that ucred cached.
(people really need to figure out that ucred's are shared, or just learn
not to touch ucreds)
Robert N M Watson FreeBSD Core Team, TrustedBSD Project
robert@fledge.watson.org NAI Labs, Safeport Network Services
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1011002095551.98303A-100000>