Date: Wed, 21 Jul 2010 23:13:52 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Garrett Cooper <yanegomi@gmail.com> Cc: standards@FreeBSD.org, hackers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: Chasing down bugs with access(2) Message-ID: <20100721221551.B7582@delplex.bde.org> In-Reply-To: <AANLkTin8wsMnttQdqyMAqmYAPjFvfvQ5fbX-mGM9z3w8@mail.gmail.com> References: <AANLkTilXPg03r3eMJQKUeFIDhabA634lYu5K03Xue-kE@mail.gmail.com> <20100721162044.N7348@delplex.bde.org> <AANLkTin8wsMnttQdqyMAqmYAPjFvfvQ5fbX-mGM9z3w8@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--0-468481727-1279718032=:7582
Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
On Wed, 21 Jul 2010, Garrett Cooper wrote:
> On Wed, Jul 21, 2010 at 1:40 AM, Bruce Evans <brde@optusnet.com.au> wrote=
:
>> - See a recent PR about unifdefed CAPABILITIES code in vaccess(). =A0(Th=
e
>> =A0comment says that the code is always ifdefed out, but it now always
>> =A0unifdefed in.) =A0 I don't quite understand this code -- does it give
>> =A0all of ACLs, MAC and etc. at this level?
>
> Interestingly standard permissions bypass ACLs/MAC if standard
> permissions on the file/directory allow the requested permissions to
> succeed; note the return (0) vs the priv_check_cred calls -- this is
> where the the ACL/MAC for the inode is checked. This seems backwards,
> but I could be missing something..
I was wrong to say that vaccess() does most of the checking. This is only
correct if there are no ACLs, etc. Otherwise, e.g., for ffs, the layering
is more like VOP_ACCESS() -> ufs_access() =3D
check r/o ffs
check quota (bogusly in the clause whose comment says that it checks
=09for a r/o fs, deep in the case statement for that clause. This
=09was readable when that clause was only 16 lines long, but now
=09it has messy locking for quotas, and large comments about this,
=09so it is 44 lines long, with the number of lines for locking
=09up from 4 to 32)
check immutability. Another bug in access()'s man page is that it
=09doesn't mention either immutability or the errno EPERM that at
=09least ufs_access() returns for it.
check acls
call vaccess(). The MAC checks seem to be at the end of this and
are unrelated to acls. But for exec_check_permissions(), the
MAC checks are first.
> ...
>> % =A0 =A0 =A0 /*
>> % =A0 =A0 =A0 =A0* 1) Check if file execution is disabled for the filesy=
stem that
>> this
>> % =A0 =A0 =A0 =A0* =A0 =A0 =A0file resides on.
>> % =A0 =A0 =A0 =A0* 2) Insure that at least one execute bit is on - other=
wise root
>> % =A0 =A0 =A0 =A0* =A0 =A0 =A0will always succeed, and we don't want to =
happen unless the
>> % =A0 =A0 =A0 =A0* =A0 =A0 =A0file really is executable.
>> % =A0 =A0 =A0 =A0* 3) Insure that the file is a regular file.
>> % =A0 =A0 =A0 =A0*/
>> % =A0 =A0 =A0 if ((vnodep->v_mount->mnt_flag & MNT_NOEXEC) ||
>> % =A0 =A0 =A0 =A0 =A0 ((attr->va_mode & 0111) =3D=3D 0) ||
>> % =A0 =A0 =A0 =A0 =A0 (attr->va_type !=3D VREG)) {
>> % =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (EACCES);
>> % =A0 =A0 =A0 }
Your mail program seems to be responsible for making the above unreadable
by changing tabs to hard \xa0's (which are displayed as tabs by my mail
client(s?), but soft \xa0's followed by a space by my editor.
>> 0111 is an old spelling of the S_IX* bits. =A0We check these directly
Maybe the changes weren't of tabs. In this paragraphs, sentence breaks of
2 spaces got changed to 1 space followed by a hard \xa0.
>> 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). =A0The usual weasel=
ing
>> 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.
>
> Agreed on the former statement, and I understand the reasoning for the
> latter statement, but at least for 1., this is a feature of mount(2)
> (of course):
>
> MNT_NOEXEC Do not allow files to be executed from the file syst=
em.
How is a newbie supposed to know to look in mount(2) to find extra failure
cases? execve(2) even cross-references mount(8), but this was in connectio=
n
with the nosuid mount option in a wrong man page:
% Index: execve.2
% =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
% RCS file: /home/ncvs/src/lib/libc/sys/execve.2,v
% retrieving revision 1.11
% retrieving revision 1.12
% diff -u -2 -r1.11 -r1.12
% --- execve.2=0911 Jan 1998 21:43:38 -0000=091.11
% +++ execve.2=0927 Apr 1999 03:56:10 -0000=091.12
% @@ -31,5 +31,5 @@
% .\"
% .\" @(#)execve.2=098.5 (Berkeley) 6/1/94
% -.\" $Id$
% +.\" $Id: execve.2,v 1.11 1998/01/11 21:43:38 alex Exp $
% .\"
% .Dd June 1, 1994
% @@ -144,4 +144,9 @@
% .ne 1i
% .Pp
% +The set-ID bits are not honored if the respective file system has the
% +.Ar nosuid
% +option enabled or if the new process file is an interpreter file. Sysca=
ll
% +tracing is disabled if effective IDs are changed.
% +.Pp
% The new process also inherits the following attributes from
% the calling process:
% @@ -274,4 +279,6 @@
% .Xr exit 3 ,
% .Xr sysctl 3 ,
% +.Xr mount 1 ,
This dangling pointer was fixed to mount(8) in the next commit. But it
should have been to mount(2) for MNT_NOSUID.
% +.Xr ktrace 1 ,
% .Xr environ 7
% .Sh HISTORY
I strongly dislike general references to man pages for 1 detail. There
should be an Xr where each of the nosuid and tracing details is described
and none at the end.
There are actually many details of interest here, but how is a newbie
going to notice this when the committers didn't? Details of interest
are at least:
- MNT_RDONLY (related to EROFS error)
- MNT_NOSUID
- MNT_NOEXEC
- MNT_ACLS (new)
- MNT_QUOTA
Better yet, nmount() allows any number of new mount options that might
affect access(), and you should have to read all the section 5 or 7 man
pages for file systems to find them all (unless access() documents them
all), but these man pages mostly don't exist and/or have few details,
so you would have to read all section 8 man pages for mount utilities.
Bruce
--0-468481727-1279718032=:7582--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100721221551.B7582>