From owner-svn-src-all@freebsd.org Sun Feb 2 16:35:00 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B1F052332BE; Sun, 2 Feb 2020 16:35:00 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 489c4h50rHz4MdY; Sun, 2 Feb 2020 16:35:00 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 88B841AB4C; Sun, 2 Feb 2020 16:35:00 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 012GZ0WX091095; Sun, 2 Feb 2020 16:35:00 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 012GYvo2091063; Sun, 2 Feb 2020 16:34:57 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <202002021634.012GYvo2091063@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Sun, 2 Feb 2020 16:34:57 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357412 - in head: contrib/netbsd-tests/lib/libc/c063 lib/libc/sys lib/libc/tests/c063 sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/fs/devfs sys/fs/fuse sys/fs/nfsclient sys/fs/sm... X-SVN-Group: head X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: in head: contrib/netbsd-tests/lib/libc/c063 lib/libc/sys lib/libc/tests/c063 sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/fs/devfs sys/fs/fuse sys/fs/nfsclient sys/fs/smbfs sys/fs/tmpfs sys/kern... X-SVN-Commit-Revision: 357412 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Feb 2020 16:35:00 -0000 Author: kevans Date: Sun Feb 2 16:34:57 2020 New Revision: 357412 URL: https://svnweb.freebsd.org/changeset/base/357412 Log: 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 Modified: head/contrib/netbsd-tests/lib/libc/c063/t_o_search.c head/lib/libc/sys/open.2 head/lib/libc/tests/c063/Makefile head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c head/sys/fs/devfs/devfs_vnops.c head/sys/fs/fuse/fuse_vnops.c head/sys/fs/nfsclient/nfs_clvnops.c head/sys/fs/smbfs/smbfs_vnops.c head/sys/fs/tmpfs/tmpfs_vnops.c head/sys/kern/vfs_cache.c head/sys/kern/vfs_lookup.c head/sys/kern/vfs_subr.c head/sys/sys/fcntl.h head/sys/sys/namei.h head/sys/sys/vnode.h Modified: head/contrib/netbsd-tests/lib/libc/c063/t_o_search.c ============================================================================== --- head/contrib/netbsd-tests/lib/libc/c063/t_o_search.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/contrib/netbsd-tests/lib/libc/c063/t_o_search.c Sun Feb 2 16:34:57 2020 (r357412) @@ -33,9 +33,10 @@ __RCSID("$NetBSD: t_o_search.c,v 1.5 2017/01/10 22:25: #include -#include +#include #include +#include #include #include #include @@ -50,7 +51,7 @@ __RCSID("$NetBSD: t_o_search.c,v 1.5 2017/01/10 22:25: * 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(); } Modified: head/lib/libc/sys/open.2 ============================================================================== --- head/lib/libc/sys/open.2 Sun Feb 2 16:10:33 2020 (r357411) +++ head/lib/libc/sys/open.2 Sun Feb 2 16:34:57 2020 (r357412) @@ -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 illeg 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 Modified: head/lib/libc/tests/c063/Makefile ============================================================================== --- head/lib/libc/tests/c063/Makefile Sun Feb 2 16:10:33 2020 (r357411) +++ head/lib/libc/tests/c063/Makefile Sun Feb 2 16:34:57 2020 (r357412) @@ -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 Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Sun Feb 2 16:34:57 2020 (r357412) @@ -1543,10 +1543,14 @@ zfs_lookup(vnode_t *dvp, char *nm, vnode_t **vpp, stru * 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); + } } } Modified: head/sys/fs/devfs/devfs_vnops.c ============================================================================== --- head/sys/fs/devfs/devfs_vnops.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/fs/devfs/devfs_vnops.c Sun Feb 2 16:34:57 2020 (r357412) @@ -946,8 +946,8 @@ devfs_lookupx(struct vop_lookup_args *ap, int *dm_unlo 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 == '.') { Modified: head/sys/fs/fuse/fuse_vnops.c ============================================================================== --- head/sys/fs/fuse/fuse_vnops.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/fs/fuse/fuse_vnops.c Sun Feb 2 16:34:57 2020 (r357412) @@ -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) { Modified: head/sys/fs/nfsclient/nfs_clvnops.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clvnops.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/fs/nfsclient/nfs_clvnops.c Sun Feb 2 16:34:57 2020 (r357412) @@ -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) Modified: head/sys/fs/smbfs/smbfs_vnops.c ============================================================================== --- head/sys/fs/smbfs/smbfs_vnops.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/fs/smbfs/smbfs_vnops.c Sun Feb 2 16:34:57 2020 (r357412) @@ -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); Modified: head/sys/fs/tmpfs/tmpfs_vnops.c ============================================================================== --- head/sys/fs/tmpfs/tmpfs_vnops.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/fs/tmpfs/tmpfs_vnops.c Sun Feb 2 16:34:57 2020 (r357412) @@ -90,7 +90,7 @@ tmpfs_lookup1(struct vnode *dvp, struct vnode **vpp, s *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; Modified: head/sys/kern/vfs_cache.c ============================================================================== --- head/sys/kern/vfs_cache.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/kern/vfs_cache.c Sun Feb 2 16:34:57 2020 (r357412) @@ -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); Modified: head/sys/kern/vfs_lookup.c ============================================================================== --- head/sys/kern/vfs_lookup.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/kern/vfs_lookup.c Sun Feb 2 16:34:57 2020 (r357412) @@ -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, Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/kern/vfs_subr.c Sun Feb 2 16:34:57 2020 (r357412) @@ -6377,3 +6377,15 @@ __mnt_vnode_markerfree_lazy(struct vnode **mvp, struct 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)); +} Modified: head/sys/sys/fcntl.h ============================================================================== --- head/sys/sys/fcntl.h Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/sys/fcntl.h Sun Feb 2 16:34:57 2020 (r357412) @@ -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 Modified: head/sys/sys/namei.h ============================================================================== --- head/sys/sys/namei.h Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/sys/namei.h Sun Feb 2 16:34:57 2020 (r357412) @@ -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 Modified: head/sys/sys/vnode.h ============================================================================== --- head/sys/sys/vnode.h Sun Feb 2 16:10:33 2020 (r357411) +++ head/sys/sys/vnode.h Sun Feb 2 16:34:57 2020 (r357412) @@ -953,6 +953,8 @@ int vn_chown(struct file *fp, uid_t uid, gid_t gid, st 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); \