From owner-freebsd-hackers@FreeBSD.ORG Wed Jul 21 10:47:30 2010 Return-Path: Delivered-To: hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4DBE1106566B; Wed, 21 Jul 2010 10:47:30 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx09.syd.optusnet.com.au (fallbackmx09.syd.optusnet.com.au [211.29.132.242]) by mx1.freebsd.org (Postfix) with ESMTP id C3CEF8FC0A; Wed, 21 Jul 2010 10:47:29 +0000 (UTC) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by fallbackmx09.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o6L8ehRa027766; Wed, 21 Jul 2010 18:40:43 +1000 Received: from c122-106-145-25.carlnfd1.nsw.optusnet.com.au (c122-106-145-25.carlnfd1.nsw.optusnet.com.au [122.106.145.25]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o6L8ebFA010071 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 21 Jul 2010 18:40:39 +1000 Date: Wed, 21 Jul 2010 18:40:37 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Garrett Cooper In-Reply-To: Message-ID: <20100721162044.N7348@delplex.bde.org> References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Mailman-Approved-At: Wed, 21 Jul 2010 11:16:20 +0000 Cc: standards@FreeBSD.org, hackers@FreeBSD.org Subject: Re: Chasing down bugs with access(2) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jul 2010 10:47:30 -0000 On Tue, 20 Jul 2010, Garrett Cooper wrote: > Hi Hackers, > I ran into an issue last night where apparently several apps make > faulty assumptions w.r.t. whether or not access(2) returns functional > data when running as a superuser. > > POSIX says: > In early proposals, some inadequacies in the access() function led > to the creation of an eaccess() function because: > > 1. Historical implementations of access() do not test file > access correctly when the process' real user ID is superuser. In > particular, they always return zero when testing execute permissions > without regard to whether the file is executable. > 2. The superuser has complete access to all files on a system. > As a consequence, programs started by the superuser and switched to > the effective user ID with lesser privileges cannot use access() to > test their file access permissions. > > However, the historical model of eaccess() does not resolve > problem (1), so this volume of IEEE Std 1003.1-2001 now allows > access() to behave in the desired way because several implementations > have corrected the problem. It was also argued that problem (2) is > more easily solved by using open(), chdir(), or one of the exec > functions as appropriate and responding to the error, rather than > creating a new function that would not be as reliable. Therefore, > eaccess() is not included in this volume of IEEE Std 1003.1-2001. > > The sentence concerning appropriate privileges and execute > permission bits reflects the two possibilities implemented by > historical implementations when checking superuser access for X_OK. > > New implementations are discouraged from returning X_OK unless at > least one execution permission bit is set. This seems wrong for directories. It should say "... unless the file is 'executable'". 'executable' means searchable for directories, and the above shouldn't apply. 'executable' actually means executable for regular files, and the above should only apply indirectly: it is executability that should be required to have an X perm bit set, and then access() should just track the capability. The usual weaseling with "appropriate privilege" allows the X perm bits to have any control on executablity including none, and at least the old POSIX spec doesn't get in the way of this, since it doesn't mention the X perm bits in connection with the exec functions. The spec goes too far in the other direction for the access function. > FreeBSD says: > > Even if a process's real or effective user has appropriate privileges and > indicates success for X_OK, the file may not actually have execute per- > mission bits set. Likewise for R_OK and W_OK. Perhaps it is time to fix this. The part about X_OK never applied to any version of FreeBSD. Perhaps it applied to the version of execve() in Net/2 and 4.4BSD, but FreeBSD had to reimplement execve() and it never had this bug. But^2, the access() syscall and man page weren't changed to match. See the end of this reply for more details on execve(). See the next paragraph about more bugs in the above paragraph. Other bugs: - R_OK and W_OK are far from likewise. Everone knows that root can read and write any file. - The permission bits are relatively uninteresting. access() should track the capability, not the bits. The bits used to map to the capability directly for non-root, but now with ACLs, MAC, etc. they don't even do that. - access(2) has no mention of ACLs, MAC, etc. - See a recent PR about unifdefed CAPABILITIES code in vaccess(). (The comment says that the code is always ifdefed out, but it now always unifdefed in.) I don't quite understand this code -- does it give all of ACLs, MAC and etc. at this level? > > This results in: > > sh/test - ok-ish (a guy on bash-bugs challenged the fact that the > syscall was buggy based on the details returned). > bash - broken (builtin test(1) always returns true) > csh - not really ok (uses whacky stat-like detection; doesn't > check for ACLs, or MAC info) > perl - ok (uses eaccess(2) for our OS). > python - broken (uses straight access(2), so os.access is broken). > > So now the question is how to fix this? Linux reports the correct > mode for the file (when operating as superuser or not), and there's a > lot of code outside of *BSD that builds upon that assumption because > stat(2) doesn't test for permissions via POSIX ACLs, MAC, etc. > I tried munging through the code to determine where VOP_ACCESS was > coming from but I got lost. Where should I look for this? Mostly it is in vaccess(9). But execve() mostly doesn't use VOP_ACCESS(). Here is the FreeBSD-1 version, which is a bit shorter and thus easier to understand than the current version, and shows that FreeBSD has always required 1 exec bit for execve(): % /* % * Check permissions of file to execute. % * Return 0 for success or error code on failure. % */ % int % exec_check_permissions(iparams) % struct image_params *iparams; % { % struct proc *p = iparams->proc; % struct vnode *vnodep = iparams->vnodep; % struct vattr *attr = iparams->attr; % int error; % % /* % * Check number of open-for-writes on the file and deny execution % * if there are any. % */ % if (vnodep->v_writecount) { % return (ETXTBSY); % } % % /* Get file attributes */ % error = VOP_GETATTR(vnodep, attr, p->p_ucred, p); % if (error) % return (error); % -current also has a MAC check here. I can't see how vaccess(9) does an equivalent check, or if it does, but if it did then we wouldn't need a special MAC check here. % /* % * 1) Check if file execution is disabled for the filesystem that this % * file resides on. % * 2) Insure that at least one execute bit is on - otherwise root % * will always succeed, and we don't want to happen unless the % * file really is executable. % * 3) Insure that the file is a regular file. % */ % if ((vnodep->v_mount->mnt_flag & MNT_NOEXEC) || % ((attr->va_mode & 0111) == 0) || % (attr->va_type != VREG)) { % return (EACCES); % } 0111 is an old spelling of the S_IX* bits. We check these directly since we know that VOP_ACCESS() is broken for root. It is also good to avoid calling VOP_ACCESS() first, since VOP_ACCESS() would record our use of suser() privilege when in fact we won't use it. Yet 2 more bugs: not just point 2, but points 1 and 3 in the above comment are undocumented in execve(2) and access(2). The usual weaseling with "appropriate privilege" should allow these too, but (as I forgot to mention above) I think "appropriate privilege" is supposed to be documented somewhere, so the man pages are still missing details. % % /* % * Zero length files can't be exec'd % */ % if (attr->va_size == 0) % return (ENOEXEC); % % /* % * Disable setuid/setgid if the filesystem prohibits it or if % * the process is being traced. % */ % if ((vnodep->v_mount->mnt_flag & MNT_NOSUID) || (p->p_flag & STRC)) % attr->va_mode &= ~(VSUID | VSGID); % % /* % * Check for execute permission to file based on current credentials. % * Then call filesystem specific open routine (which does nothing % * in the general case). % */ % error = VOP_ACCESS(vnodep, VEXEC, p->p_ucred, p); % if (error) % return (error); We still use VOP_ACCESS() to handle the access checking in the usual case where 1 X bit is set. But this is not quite access(2) -- it uses the effective credentials, so is close to eaccess(2). % % error = VOP_OPEN(vnodep, FREAD, p->p_ucred, p); % if (error) % return (error); % % return (0); % } FreeBSD's man page also says: % SECURITY CONSIDERATIONS % The access() system call is a potential security hole due to race condi- % tions and should never be used. Set-user-ID and set-group-ID applica- % tions should restore the effective user or group ID, and perform actions % directly rather than use access() to simulate access checks for the real % user or group ID. The eaccess() system call likewise may be subject to % races if used inappropriately. This covers more than POSIX's point 2. Bruce