Date: Wed, 21 Jul 2010 18:40:37 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Garrett Cooper <yanegomi@gmail.com> Cc: standards@FreeBSD.org, hackers@FreeBSD.org Subject: Re: Chasing down bugs with access(2) Message-ID: <20100721162044.N7348@delplex.bde.org> In-Reply-To: <AANLkTilXPg03r3eMJQKUeFIDhabA634lYu5K03Xue-kE@mail.gmail.com> References: <AANLkTilXPg03r3eMJQKUeFIDhabA634lYu5K03Xue-kE@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <body deleted> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100721162044.N7348>