From owner-cvs-all Sun Nov 26 20: 5:32 2000 Delivered-To: cvs-all@freebsd.org Received: from green.dyndns.org (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 4300137B4C5; Sun, 26 Nov 2000 20:05:24 -0800 (PST) Received: from localhost (yezxk7@localhost [127.0.0.1]) by green.dyndns.org (8.11.0/8.11.0) with ESMTP id eAR45H578642; Sun, 26 Nov 2000 23:05:22 -0500 (EST) (envelope-from green@FreeBSD.org) Message-Id: <200011270405.eAR45H578642@green.dyndns.org> X-Mailer: exmh version 2.2 06/23/2000 with nmh-1.0.4 To: Peter Wemm Cc: "Brian F. Feldman" , Alfred Perlstein , obrien@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/inetd builtins.c In-Reply-To: Message from Peter Wemm of "Sun, 26 Nov 2000 19:10:03 PST." <200011270310.eAR3A3D44621@mobile.wemm.org> From: "Brian F. Feldman" Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Sun, 26 Nov 2000 23:05:15 -0500 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Peter Wemm wrote: > "Brian F. Feldman" wrote: > > Alfred Perlstein 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