Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 26 Nov 2000 23:05:15 -0500
From:      "Brian F. Feldman" <green@FreeBSD.org>
To:        Peter Wemm <peter@netplex.com.au>
Cc:        "Brian F. Feldman" <green@FreeBSD.org>, Alfred Perlstein <bright@wintelcom.net>, obrien@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/usr.sbin/inetd builtins.c 
Message-ID:  <200011270405.eAR45H578642@green.dyndns.org>
In-Reply-To: Message from Peter Wemm <peter@netplex.com.au>  of "Sun, 26 Nov 2000 19:10:03 PST." <200011270310.eAR3A3D44621@mobile.wemm.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
Peter Wemm <peter@netplex.com.au> wrote:
> "Brian F. Feldman" wrote:
> > Alfred Perlstein <bright@wintelcom.net> wrote:
> > > Because your "fix" was a gross hack on top of the gross hack already
> > > in place.
> > 
> > Here, you can review this, then:
> 
> How about the O_NOFOLLOW flag?  It avoids the worst of the races because you
> can open and lstat and be immune to symlink races.
> 
> >  		/*
> > -		 * If we were to lstat() here, it would do no good, since it
> > -		 * would introduce a race condition and could be defeated.
> > +		 * We can't stat() here since that would be a race
> > +		 * condition.
> >  		 * Therefore, we open the file we have permissions to open
> >  		 * and if it's not a regular file, we close it and end up
> >  		 * returning the user's real username.
> >  		 */
> >  		fakeid_fd = open(p, O_RDONLY | O_NONBLOCK);

I've decided the comment needed to be changed because the race condition to 
be worried about is stat(), "okay, it's VREG", open() -> "Hey, it's not!".
Whether it's a symlink or not doesn't matter since the user's credentials 
are being used in the permission checks.  Now, the problem with this, is 
that if the user is allowed to access a file (device?  weird file system?) 
that does not correctly respect O_NONBLOCK, it can be still made to block.

There aren't many good solutions, but luckily this doesn't really seem to be 
a problem.  An open with O_NOFOLLOW prevents hapless symlink problems, but 
since it doesn't prevent hapless file-type problems...  I'd like it if there 
was something like this:

	* lstat() is used to verify permissions, in conjunction with 
	  getgroups()/initgroups() and seteuid().
	* the stat structure can be verified to be okay.  Normally, the next 
	  step would be to open the file and fstat() and check if it's the 
	  same -- but that is only alright for some things.  What if you 
	  don't want to have called open() at all?
	* the program calls int stathash(struct stat *sb) which returns a
	  reasonably-secure hash of the "telling" contents of the structure.
	* the program then calls
	  open(name, O_RDONLY | O_NONBLOCK | O_STATHASH, hash).
	* open(2) recognizes the overloading (O_STATHASH being mutex with
	  O_CREAT) and, after locking the vnode, VOP_STAT()s and checks the
	  hashes of the stats and returns an error if they do not match.

This relies on the hash being secure and the right fields of stat being used 
in the hash... I really think it would be nice to have something like this.
It's a pity that there is not a perfect way to solve the "check file info, 
open only if correct" race.  The poor-man's way out would be something like:

open(name, O_RDONLY | O_NONBLOCK | O_NOFOLLOW | O_ASSERTMODE, S_IFREG);

I wouldn't really mind this; at least you REALLY know what kind of file 
you're opening.  Comments?  Robert, I know you'll have something to say on 
the subject :)

--
 Brian Fundakowski Feldman           \  FreeBSD: The Power to Serve!  /
 green@FreeBSD.org                    `------------------------------'




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200011270405.eAR45H578642>