Date: Sun, 26 Nov 2000 20:34:07 -0800 From: Peter Wemm <peter@netplex.com.au> To: "Brian F. Feldman" <green@FreeBSD.org> Cc: 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: <200011270434.eAR4Y7D45315@mobile.wemm.org> In-Reply-To: <200011270405.eAR45H578642@green.dyndns.org>
next in thread | previous in thread | raw e-mail | index | archive | help
"Brian F. Feldman" wrote:
> 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 yo
u
> > 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.
It sounds like you're asking for getfh(2), fhstat(2), fhopen(2).. Sure, it
is intended for nfs lockd, but is in the base kernel, not part of NFS.
fhandle_t foo;
struct stat sb;
getfh("/home/foo/.fakeid", &foo); /* grab a "handle" on it */
fhstat(&foo, &sb); /* stat it */
if (!S_ISREG(sb.st_mode) || permission_check(&sb))
die();
fd = fhopen(&foo, O_RDONLY); /* we *know* it is a regular file */
fp = fdopen(fd, "r");
Granted, it is kinda perverse, but also should work. The file could be
moved or renamed etc but the handle would always identify the correct file.
It should also be race free and quite safe as you can stat() and check out
the file before you commit to actually opening it.
This may or may not be appropriate for the task at hand, but I thought the
technique might be worth mentioning since we were on the topic of safe file
opens.
Cheers,
-Peter
--
Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au
"All of this is for nothing if we don't go to the stars" - JMS/B5
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?200011270434.eAR4Y7D45315>