Provide O_SEARCH

O_SEARCH is defined by POSIX [0] to open a directory for searching, skipping
permissions checks on the directory itself after the initial open(). This is
close to the semantics we've historically applied for O_EXEC on a directory,
which is UB according to POSIX. Conveniently, O_SEARCH on a file is also
explicitly undefined behavior according to POSIX, so O_EXEC would be a fine
choice. The spec goes on to state that O_SEARCH and O_EXEC need not be
distinct values, but they're not defined to be the same value.

This was pointed out as an incompatibility with other systems that had made
its way into libarchive, which had assumed that O_EXEC was an alias for
O_SEARCH.

This defines compatibility O_SEARCH/FSEARCH (equivalent to O_EXEC and FEXEC
respectively) and expands our UB for O_EXEC on a directory. O_EXEC on a
directory is checked in vn_open_vnode already, so for completeness we add a
NOEXECCHECK when O_SEARCH has been specified on the top-level fd and do not
re-check that when descending in namei.

[0] https://pubs.opengroup.org/onlinepubs/9699919799/

Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D23247
This commit is contained in:
kevans 2020-02-02 16:34:57 +00:00
parent 1f9321e091
commit 11e74d9fa2
15 changed files with 151 additions and 25 deletions

View File

@ -33,9 +33,10 @@ __RCSID("$NetBSD: t_o_search.c,v 1.5 2017/01/10 22:25:01 christos Exp $");
#include <atf-c.h>
#include <sys/param.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <limits.h>
@ -50,7 +51,7 @@ __RCSID("$NetBSD: t_o_search.c,v 1.5 2017/01/10 22:25:01 christos Exp $");
* until a decision is reached about the semantics of O_SEARCH and a
* non-broken implementation is available.
*/
#if (O_MASK & O_SEARCH) != 0
#if defined(__FreeBSD__) || (O_MASK & O_SEARCH) != 0
#define USE_O_SEARCH
#endif
@ -257,11 +258,79 @@ ATF_TC_BODY(o_search_notdir, tc)
int fd;
ATF_REQUIRE(mkdir(DIR, 0755) == 0);
#ifndef __FreeBSD__
ATF_REQUIRE((dfd = open(FILE, O_CREAT|O_RDWR|O_SEARCH, 0644)) != -1);
#else
ATF_REQUIRE((dfd = open(FILE, O_CREAT|O_SEARCH, 0644)) != -1);
#endif
ATF_REQUIRE((fd = openat(dfd, BASEFILE, O_RDWR, 0)) == -1);
ATF_REQUIRE(errno == ENOTDIR);
}
#ifdef USE_O_SEARCH
ATF_TC(o_search_nord);
ATF_TC_HEAD(o_search_nord, tc)
{
atf_tc_set_md_var(tc, "descr", "See that openat succeeds with no read permission");
atf_tc_set_md_var(tc, "require.user", "unprivileged");
}
ATF_TC_BODY(o_search_nord, tc)
{
int dfd, fd;
ATF_REQUIRE(mkdir(DIR, 0755) == 0);
ATF_REQUIRE((fd = open(FILE, O_CREAT|O_RDWR, 0644)) != -1);
ATF_REQUIRE(close(fd) == 0);
ATF_REQUIRE(chmod(DIR, 0100) == 0);
ATF_REQUIRE((dfd = open(DIR, O_SEARCH, 0)) != -1);
ATF_REQUIRE(faccessat(dfd, BASEFILE, W_OK, 0) != -1);
ATF_REQUIRE(close(dfd) == 0);
}
ATF_TC(o_search_getdents);
ATF_TC_HEAD(o_search_getdents, tc)
{
atf_tc_set_md_var(tc, "descr", "See that O_SEARCH forbids getdents");
}
ATF_TC_BODY(o_search_getdents, tc)
{
char buf[1024];
int dfd;
ATF_REQUIRE(mkdir(DIR, 0755) == 0);
ATF_REQUIRE((dfd = open(DIR, O_SEARCH, 0)) != -1);
ATF_REQUIRE(getdents(dfd, buf, sizeof(buf)) < 0);
ATF_REQUIRE(close(dfd) == 0);
}
ATF_TC(o_search_revokex);
ATF_TC_HEAD(o_search_revokex, tc)
{
atf_tc_set_md_var(tc, "descr", "See that *at behaves after chmod -x");
atf_tc_set_md_var(tc, "require.user", "unprivileged");
}
ATF_TC_BODY(o_search_revokex, tc)
{
int dfd, fd;
struct stat sb;
ATF_REQUIRE(mkdir(DIR, 0755) == 0);
ATF_REQUIRE((fd = open(FILE, O_CREAT|O_RDWR, 0644)) != -1);
ATF_REQUIRE(close(fd) == 0);
ATF_REQUIRE((dfd = open(DIR, O_SEARCH, 0)) != -1);
/* Drop permissions. The kernel must still not check the exec bit. */
ATF_REQUIRE(chmod(DIR, 0000) == 0);
ATF_REQUIRE(fstatat(dfd, BASEFILE, &sb, 0) == 0);
ATF_REQUIRE(close(dfd) == 0);
}
#endif /* USE_O_SEARCH */
ATF_TP_ADD_TCS(tp)
{
@ -276,6 +345,11 @@ ATF_TP_ADD_TCS(tp)
ATF_TP_ADD_TC(tp, o_search_unpriv_flag2);
#endif
ATF_TP_ADD_TC(tp, o_search_notdir);
#ifdef USE_O_SEARCH
ATF_TP_ADD_TC(tp, o_search_nord);
ATF_TP_ADD_TC(tp, o_search_getdents);
ATF_TP_ADD_TC(tp, o_search_revokex);
#endif
return atf_no_error();
}

View File

@ -28,7 +28,7 @@
.\" @(#)open.2 8.2 (Berkeley) 11/16/93
.\" $FreeBSD$
.\"
.Dd September 28, 2019
.Dd January 31, 2020
.Dt OPEN 2
.Os
.Sh NAME
@ -166,6 +166,7 @@ O_RDONLY open for reading only
O_WRONLY open for writing only
O_RDWR open for reading and writing
O_EXEC open for execute only
O_SEARCH open for search only, an alias for O_EXEC
O_NONBLOCK do not block on open
O_APPEND append on each write
O_CREAT create file if it does not exist
@ -326,6 +327,19 @@ If the specified path is absolute,
allows arbitrary prefix that ends up at the topping directory,
after which all further resolved components must be under it.
.Pp
When
.Fa fd
is opened with
.Dv O_SEARCH ,
execute permissions are checked at open time.
The
.Fa fd
may not be used for any read operations like
.Xr getdirentries 2 .
The primary use for this descriptor will be as the lookup descriptor for the
.Fn *at
family of functions.
.Pp
If successful,
.Fn open
returns a non-negative integer, termed a file descriptor.
@ -518,9 +532,12 @@ An attempt was made to open a descriptor with an illegal combination
of
.Dv O_RDONLY ,
.Dv O_WRONLY ,
.Dv O_RDWR
or
.Dv O_RDWR ,
and
.Dv O_EXEC .
.Dv O_EXEC
or
.Dv O_SEARCH .
.It Bq Er EBADF
The
.Fa path

View File

@ -1,7 +1,5 @@
# $FreeBSD$
#TODO: t_o_search
NETBSD_ATF_TESTS_C= faccessat_test
NETBSD_ATF_TESTS_C+= fchmodat_test
NETBSD_ATF_TESTS_C+= fchownat_test
@ -11,6 +9,7 @@ NETBSD_ATF_TESTS_C+= linkat_test
NETBSD_ATF_TESTS_C+= mkdirat_test
NETBSD_ATF_TESTS_C+= mkfifoat_test
NETBSD_ATF_TESTS_C+= mknodat_test
NETBSD_ATF_TESTS_C+= o_search_test
NETBSD_ATF_TESTS_C+= openat_test
NETBSD_ATF_TESTS_C+= readlinkat_test
NETBSD_ATF_TESTS_C+= renameat_test

View File

@ -1543,10 +1543,14 @@ zfs_lookup(vnode_t *dvp, char *nm, vnode_t **vpp, struct componentname *cnp,
* Check accessibility of directory.
*/
if (!cached) {
error = zfs_zaccess(zdp, ACE_EXECUTE, 0, B_FALSE, cr);
if (error != 0) {
ZFS_EXIT(zfsvfs);
return (error);
if ((cnp->cn_flags & NOEXECCHECK) != 0) {
cnp->cn_flags &= ~NOEXECCHECK;
} else {
error = zfs_zaccess(zdp, ACE_EXECUTE, 0, B_FALSE, cr);
if (error != 0) {
ZFS_EXIT(zfsvfs);
return (error);
}
}
}

View File

@ -946,8 +946,8 @@ devfs_lookupx(struct vop_lookup_args *ap, int *dm_unlock)
if ((flags & ISDOTDOT) && (dvp->v_vflag & VV_ROOT))
return (EIO);
error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td);
if (error)
error = vn_dir_check_exec(dvp, cnp);
if (error != 0)
return (error);
if (cnp->cn_namelen == 1 && *pname == '.') {

View File

@ -1006,7 +1006,9 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
if (islastcn && vfs_isrdonly(mp) && (nameiop != LOOKUP))
return EROFS;
if ((err = fuse_internal_access(dvp, VEXEC, td, cred)))
if ((cnp->cn_flags & NOEXECCHECK) != 0)
cnp->cn_flags &= ~NOEXECCHECK;
else if ((err = fuse_internal_access(dvp, VEXEC, td, cred)))
return err;
if (flags & ISDOTDOT) {

View File

@ -1195,7 +1195,8 @@ nfs_lookup(struct vop_lookup_args *ap)
}
NFSUNLOCKNODE(np);
if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0)
error = vn_dir_check_exec(dvp, cnp);
if (error != 0)
return (error);
error = cache_lookup(dvp, vpp, cnp, &nctime, &ncticks);
if (error > 0 && error != ENOENT)

View File

@ -1199,7 +1199,8 @@ smbfs_lookup(ap)
islastcn = flags & ISLASTCN;
if (islastcn && (mp->mnt_flag & MNT_RDONLY) && (nameiop != LOOKUP))
return EROFS;
if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0)
error = vn_dir_check_exec(dvp, cnp);
if (error != 0)
return error;
smp = VFSTOSMBFS(mp);
dnp = VTOSMB(dvp);

View File

@ -90,7 +90,7 @@ tmpfs_lookup1(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp)
*vpp = NULLVP;
/* Check accessibility of requested node as a first step. */
error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, cnp->cn_thread);
error = vn_dir_check_exec(dvp, cnp);
if (error != 0)
goto out;

View File

@ -2141,9 +2141,7 @@ vfs_cache_lookup(struct vop_lookup_args *ap)
int error;
struct vnode **vpp = ap->a_vpp;
struct componentname *cnp = ap->a_cnp;
struct ucred *cred = cnp->cn_cred;
int flags = cnp->cn_flags;
struct thread *td = cnp->cn_thread;
*vpp = NULL;
dvp = ap->a_dvp;
@ -2155,8 +2153,8 @@ vfs_cache_lookup(struct vop_lookup_args *ap)
(cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME))
return (EROFS);
error = VOP_ACCESS(dvp, VEXEC, cred, td);
if (error)
error = vn_dir_check_exec(dvp, cnp);
if (error != 0)
return (error);
error = cache_lookup(dvp, vpp, cnp, NULL, NULL);

View File

@ -308,6 +308,7 @@ namei(struct nameidata *ndp)
struct vnode *dp; /* the directory we are searching */
struct iovec aiov; /* uio for reading symbolic links */
struct componentname *cnp;
struct file *dfp;
struct thread *td;
struct proc *p;
cap_rights_t rights;
@ -445,10 +446,22 @@ namei(struct nameidata *ndp)
AUDIT_ARG_ATFD1(ndp->ni_dirfd);
if (cnp->cn_flags & AUDITVNODE2)
AUDIT_ARG_ATFD2(ndp->ni_dirfd);
error = fgetvp_rights(td, ndp->ni_dirfd,
&rights, &ndp->ni_filecaps, &dp);
if (error == EINVAL)
/*
* Effectively inlined fgetvp_rights, because we need to
* inspect the file as well as grabbing the vnode.
*/
error = fget_cap_locked(fdp, ndp->ni_dirfd, &rights,
&dfp, &ndp->ni_filecaps);
if (error != 0 || dfp->f_ops == &badfileops ||
dfp->f_vnode == NULL) {
error = ENOTDIR;
} else {
dp = dfp->f_vnode;
vrefact(dp);
if ((dfp->f_flag & FSEARCH) != 0)
cnp->cn_flags |= NOEXECCHECK;
}
#ifdef CAPABILITIES
/*
* If file descriptor doesn't have all rights,

View File

@ -6377,3 +6377,15 @@ __mnt_vnode_markerfree_lazy(struct vnode **mvp, struct mount *mp)
mtx_unlock(&mp->mnt_listmtx);
mnt_vnode_markerfree_lazy(mvp, mp);
}
int
vn_dir_check_exec(struct vnode *vp, struct componentname *cnp)
{
if ((cnp->cn_flags & NOEXECCHECK) != 0) {
cnp->cn_flags &= ~NOEXECCHECK;
return (0);
}
return (VOP_ACCESS(vp, VEXEC, cnp->cn_cred, cnp->cn_thread));
}

View File

@ -119,9 +119,11 @@ typedef __pid_t pid_t;
#if __POSIX_VISIBLE >= 200809
#define O_DIRECTORY 0x00020000 /* Fail if not directory */
#define O_EXEC 0x00040000 /* Open for execute only */
#define O_SEARCH O_EXEC
#endif
#ifdef _KERNEL
#define FEXEC O_EXEC
#define FSEARCH O_SEARCH
#endif
#if __POSIX_VISIBLE >= 200809

View File

@ -161,7 +161,8 @@ struct nameidata {
#define AUDITVNODE2 0x08000000 /* audit the looked up vnode information */
#define TRAILINGSLASH 0x10000000 /* path ended in a slash */
#define NOCAPCHECK 0x20000000 /* do not perform capability checks */
#define PARAMASK 0x3ffffe00 /* mask of parameter descriptors */
#define NOEXECCHECK 0x40000000 /* do not perform exec check on dir */
#define PARAMASK 0x7ffffe00 /* mask of parameter descriptors */
/*
* Namei results flags

View File

@ -953,6 +953,8 @@ int vn_chown(struct file *fp, uid_t uid, gid_t gid, struct ucred *active_cred,
void vn_fsid(struct vnode *vp, struct vattr *va);
int vn_dir_check_exec(struct vnode *vp, struct componentname *cnp);
#define VOP_UNLOCK_FLAGS(vp, flags) ({ \
struct vnode *_vp = (vp); \
int _flags = (flags); \