On Wed, Jul 21, 2010 at 1:40 AM, Bruce Evans <b...@optusnet.com.au> wrote: > On Tue, 20 Jul 2010, Garrett Cooper wrote:
... > 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. Agreed (for all of the above). >> 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. Yes. Thankfully Linux also agrees on this point (I say this, because portability between Linux and BSD helps ease porting pains). > - 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. No, sadly it doesn't (and of course POSIX leaves that open in the File permissions section, for good reason: http://www.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap04.html#tag_04_04 ). That's the one thing that one of the folks on the bash list brought up that was a valid argument for using access(2), eaccess(2), or faccessat(2) vs stat(2). If you use a straight stat(2) call to determine whether or not a file is executable, the `hint' could be completely bogus as the file itself could be non-executable when the ACL or MAC denies the capability, whereas many implementations of access(2) support this additional permissions checking. > - 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? 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.. >> 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). ... > -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. Agreed. > % /* > % * 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. 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 system. ... Yes. The usual warning about the `hinting' being done by *access(2) is fine :). -Garrett _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
