Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 03 Sep 2019 14:06:38 -0000
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r346088 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs
Message-ID:  <201904101731.x3AHV0jB050950@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: asomers
Date: Wed Apr 10 17:31:00 2019
New Revision: 346088
URL: https://svnweb.freebsd.org/changeset/base/346088

Log:
  fusefs: WIP supporting -o default_permissions
  
  Normally all permission checking is done in the fuse server.  But when -o
  default_permissions is used, it should be done in the kernel instead.  This
  commit adds appropriate permission checks through fusefs when -o
  default_permissions is used.  However, sticky bit checks aren't working yet.
  I'll handle those in a follow-up commit.
  
  There are no checks for file flags, because those aren't supported by our
  version of the FUSE protocol.  Nor is there any support for ACLs, though
  that could be added if there were any demand.
  
  PR:		216391
  Reported by:	hiyorin@gmail.com
  Sponsored by:	The FreeBSD Foundation

Modified:
  projects/fuse2/sys/fs/fuse/fuse_internal.c
  projects/fuse2/sys/fs/fuse/fuse_internal.h
  projects/fuse2/sys/fs/fuse/fuse_vnops.c
  projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc
  projects/fuse2/tests/sys/fs/fusefs/destroy.cc
  projects/fuse2/tests/sys/fs/fusefs/lookup.cc
  projects/fuse2/tests/sys/fs/fusefs/setattr.cc
  projects/fuse2/tests/sys/fs/fusefs/unlink.cc
  projects/fuse2/tests/sys/fs/fusefs/utils.cc
  projects/fuse2/tests/sys/fs/fusefs/utils.hh
  projects/fuse2/tests/sys/fs/fusefs/xattr.cc

Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.c	Wed Apr 10 16:48:45 2019	(r346087)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.c	Wed Apr 10 17:31:00 2019	(r346088)
@@ -109,7 +109,7 @@ static int isbzero(void *buf, size_t len);
 /* Synchronously send a FUSE_ACCESS operation */
 int
 fuse_internal_access(struct vnode *vp,
-    mode_t mode,
+    accmode_t mode,
     struct fuse_access_param *facp,
     struct thread *td,
     struct ucred *cred)
@@ -129,8 +129,17 @@ fuse_internal_access(struct vnode *vp,
 	data = fuse_get_mpdata(mp);
 	dataflags = data->dataflags;
 
-	if ((mode & VWRITE) && vfs_isrdonly(mp)) {
-		return EROFS;
+	if (mode & VMODIFY_PERMS && vfs_isrdonly(mp)) {
+		switch (vp->v_type) {
+		case VDIR:
+			/* FALLTHROUGH */
+		case VLNK:
+			/* FALLTHROUGH */
+		case VREG:
+			return EROFS;
+		default:
+			break;
+		}
 	}
 
 	/* Unless explicitly permitted, deny everyone except the fs owner. */
@@ -144,8 +153,11 @@ fuse_internal_access(struct vnode *vp,
 	}
 
 	if (dataflags & FSESS_DEFAULT_PERMISSIONS) {
-		/* TODO: Implement me!  Bug 216391 */
-		return 0;
+		struct vattr va;
+
+		fuse_internal_getattr(vp, &va, cred, td);
+		return vaccess(vp->v_type, va.va_mode, va.va_uid,
+		    va.va_gid, mode, cred, NULL);
 	}
 
 	if (!fsess_isimpl(mp, FUSE_ACCESS))

Modified: projects/fuse2/sys/fs/fuse/fuse_internal.h
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.h	Wed Apr 10 16:48:45 2019	(r346087)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.h	Wed Apr 10 17:31:00 2019	(r346088)
@@ -232,7 +232,7 @@ fuse_match_cred(struct ucred *basecred, struct ucred *
 	return (EPERM);
 }
 
-int fuse_internal_access(struct vnode *vp, mode_t mode,
+int fuse_internal_access(struct vnode *vp, accmode_t mode,
     struct fuse_access_param *facp, struct thread *td, struct ucred *cred);
 
 /* attributes */

Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_vnops.c	Wed Apr 10 16:48:45 2019	(r346087)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c	Wed Apr 10 17:31:00 2019	(r346088)
@@ -829,19 +829,12 @@ calldaemon:
 	/* lookup_err, if non-zero, must be ENOENT at this point */
 
 	if (lookup_err) {
-
-		if ((nameiop == CREATE || nameiop == RENAME) && islastcn
-		     /* && directory dvp has not been removed */ ) {
-
-			if (vfs_isrdonly(mp)) {
-				err = EROFS;
+		/* Entry not found */
+		if ((nameiop == CREATE || nameiop == RENAME) && islastcn) {
+			err = fuse_internal_access(dvp, VWRITE, &facp, td,
+				cred);
+			if (err)
 				goto out;
-			}
-#if 0 /* THINK_ABOUT_THIS */
-			if ((err = fuse_internal_access(dvp, VWRITE, cred, td, &facp))) {
-				goto out;
-			}
-#endif
 
 			/*
 	                 * Possibly record the position of a slot in the
@@ -860,16 +853,41 @@ calldaemon:
 		goto out;
 
 	} else {
-
-		/* !lookup_err */
-
+		/* Entry was found */
 		if (op == FUSE_GETATTR) {
 			fattr = &((struct fuse_attr_out *)fdi.answ)->attr;
 		} else {
 			feo = (struct fuse_entry_out *)fdi.answ;
 			fattr = &(feo->attr);
 		}
+		
+		/* TODO: check for ENOTDIR */
 
+		if ((nameiop == DELETE || nameiop == RENAME) && islastcn) {
+			err = fuse_internal_access(dvp, VWRITE, &facp,
+				td, cred);
+			if (err != 0)
+				goto out;
+			/* 
+			 * TODO: if the parent's sticky bit is set, check
+			 * whether we're allowed to remove the file.
+			 * Need to figure out the vnode locking to make this
+			 * work.
+			 */
+#if defined(NOT_YET)
+			struct vattr dvattr;
+			fuse_internal_getattr(dvp, &dvattr, cred, td);
+			if ((dvattr.va_mode & S_ISTXT) &&
+				fuse_internal_access(dvp, VADMIN, &facp,
+					cnp->cn_thread, cnp->cn_cred) &&
+				fuse_internal_access(*vpp, VADMIN, &facp,
+					cnp->cn_thread, cnp->cn_cred)) {
+				err = EPERM;
+				goto out;
+			}
+#endif
+		}
+
 		/*
 	         * If deleting, and at end of pathname, return parameters
 	         * which can be used to remove file.  If the wantparent flag
@@ -877,17 +895,6 @@ calldaemon:
 	         * and lock the inode, being careful with ".".
 	         */
 		if (nameiop == DELETE && islastcn) {
-			/*
-	                 * Check for write access on directory.
-	                 */
-			facp.xuid = fattr->uid;
-			facp.facc_flags |= FACCESS_STICKY;
-			err = fuse_internal_access(dvp, VWRITE, &facp, td, cred);
-			facp.facc_flags &= ~FACCESS_XQUERIES;
-
-			if (err) {
-				goto out;
-			}
 			if (nid == VTOI(dvp)) {
 				vref(dvp);
 				*vpp = dvp;
@@ -914,13 +921,6 @@ calldaemon:
 	         * regular file, or empty directory.
 	         */
 		if (nameiop == RENAME && wantparent && islastcn) {
-
-#if 0 /* THINK_ABOUT_THIS */
-			if ((err = fuse_internal_access(dvp, VWRITE, cred, td, &facp))) {
-				goto out;
-			}
-#endif
-
 			/*
 	                 * Check for "."
 	                 */
@@ -933,6 +933,10 @@ calldaemon:
 			if (err) {
 				goto out;
 			}
+			/*err = fuse_lookup_check(dvp, vp, &facp, td, cred,*/
+				/*nameiop, islastcn);*/
+			/*if (err)*/
+				/*goto out;*/
 			*vpp = vp;
 			/*
 	                 * Save the name for use in VOP_RENAME later.
@@ -1031,7 +1035,6 @@ calldaemon:
 				&fao->attr, fao->attr_valid,
 				fao->attr_valid_nsec, NULL);
 		} else {
-			feo = (struct fuse_entry_out*)fdi.answ;
 			fuse_internal_cache_attrs(*vpp,
 				&feo->attr, feo->attr_valid,
 				feo->attr_valid_nsec, NULL);
@@ -1532,6 +1535,7 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 	enum vtype vtyp;
 	int sizechanged = 0;
 	uint64_t newsize = 0;
+	accmode_t accmode = 0;
 
 	if (fuse_isdeadfs(vp)) {
 		return ENXIO;
@@ -1550,21 +1554,24 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 		facp.facc_flags |= FACCESS_CHOWN;
 		fsai->uid = vap->va_uid;
 		fsai->valid |= FATTR_UID;
+		accmode |= VADMIN;
 	}
 	if (vap->va_gid != (gid_t)VNOVAL) {
 		facp.facc_flags |= FACCESS_CHOWN;
 		fsai->gid = vap->va_gid;
 		fsai->valid |= FATTR_GID;
+		accmode |= VADMIN;
 	}
 	if (vap->va_size != VNOVAL) {
 
 		struct fuse_filehandle *fufh = NULL;
 
 		/*Truncate to a new value. */
-		    fsai->size = vap->va_size;
+		fsai->size = vap->va_size;
 		sizechanged = 1;
 		newsize = vap->va_size;
 		fsai->valid |= FATTR_SIZE;
+		accmode |= VWRITE;
 
 		fuse_filehandle_getrw(vp, FWRITE, &fufh, cred, pid);
 		if (fufh) {
@@ -1576,15 +1583,24 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 		fsai->atime = vap->va_atime.tv_sec;
 		fsai->atimensec = vap->va_atime.tv_nsec;
 		fsai->valid |= FATTR_ATIME;
+		accmode |= VADMIN;
+		/*
+		 * TODO: only require VWRITE if UTIMENS_NULL is set. PR 237181
+		 */
 	}
 	if (vap->va_mtime.tv_sec != VNOVAL) {
 		fsai->mtime = vap->va_mtime.tv_sec;
 		fsai->mtimensec = vap->va_mtime.tv_nsec;
 		fsai->valid |= FATTR_MTIME;
+		accmode |= VADMIN;
+		/*
+		 * TODO: only require VWRITE if UTIMENS_NULL is set. PR 237181
+		 */
 	}
 	if (vap->va_mode != (mode_t)VNOVAL) {
 		fsai->mode = vap->va_mode & ALLPERMS;
 		fsai->valid |= FATTR_MODE;
+		accmode |= VADMIN;
 	}
 	if (!fsai->valid) {
 		goto out;
@@ -1599,6 +1615,9 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
 		err = EROFS;
 		goto out;
 	}
+	err = fuse_internal_access(vp, accmode, &facp, td, cred);
+	if (err)
+		goto out;
 
 	if ((err = fdisp_wait_answ(&fdi)))
 		goto out;
@@ -2009,6 +2028,10 @@ fuse_vnop_getextattr(struct vop_getextattr_args *ap)
 	if (fuse_isdeadfs(vp))
 		return (ENXIO);
 
+	err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VREAD);
+	if (err)
+		return err;
+
 	/* Default to looking for user attributes. */
 	if (ap->a_attrnamespace == EXTATTR_NAMESPACE_SYSTEM)
 		prefix = EXTATTR_NAMESPACE_SYSTEM_STRING;
@@ -2086,6 +2109,14 @@ fuse_vnop_setextattr(struct vop_setextattr_args *ap)
 	if (fuse_isdeadfs(vp))
 		return (ENXIO);
 
+	/* Deleting xattrs must use VOP_DELETEEXTATTR instead */
+	if (ap->a_uio == NULL)
+		return (EINVAL);
+
+	err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VWRITE);
+	if (err)
+		return err;
+
 	/* Default to looking for user attributes. */
 	if (ap->a_attrnamespace == EXTATTR_NAMESPACE_SYSTEM)
 		prefix = EXTATTR_NAMESPACE_SYSTEM_STRING;
@@ -2213,6 +2244,10 @@ fuse_vnop_listextattr(struct vop_listextattr_args *ap)
 	if (fuse_isdeadfs(vp))
 		return (ENXIO);
 
+	err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VREAD);
+	if (err)
+		return err;
+
 	/*
 	 * Add space for a NUL and the period separator if enabled.
 	 * Default to looking for user attributes.
@@ -2316,6 +2351,10 @@ fuse_vnop_deleteextattr(struct vop_deleteextattr_args 
 
 	if (fuse_isdeadfs(vp))
 		return (ENXIO);
+
+	err = extattr_check_cred(vp, ap->a_attrnamespace, cred, td, VWRITE);
+	if (err)
+		return err;
 
 	/* Default to looking for user attributes. */
 	if (ap->a_attrnamespace == EXTATTR_NAMESPACE_SYSTEM)

Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc	Wed Apr 10 16:48:45 2019	(r346087)
+++ projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc	Wed Apr 10 17:31:00 2019	(r346088)
@@ -34,6 +34,9 @@
  */
 
 extern "C" {
+#include <sys/types.h>
+#include <sys/extattr.h>
+
 #include <fcntl.h>
 #include <unistd.h>
 }
@@ -46,34 +49,185 @@ using namespace testing;
 class DefaultPermissions: public FuseTest {
 
 virtual void SetUp() {
+	m_default_permissions = true;
 	FuseTest::SetUp();
+	if (HasFatalFailure() || IsSkipped())
+		return;
 
 	if (geteuid() == 0) {
 		GTEST_SKIP() << "This test requires an unprivileged user";
 	}
-	m_default_permissions = true;
+	
+	/* With -o default_permissions, FUSE_ACCESS should never be called */
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_ACCESS);
+		}, Eq(true)),
+		_)
+	).Times(0);
 }
 
 public:
-void expect_lookup(const char *relpath, uint64_t ino, mode_t mode)
+void expect_getattr(uint64_t ino, mode_t mode, uint64_t attr_valid, int times,
+	uid_t uid = 0)
 {
-	FuseTest::expect_lookup(relpath, ino, S_IFREG | mode, 0, 1);
+	/* Until the attr cache is working, we may send an additional GETATTR */
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_GETATTR &&
+				in->header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).Times(times)
+	.WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out->body.attr.attr.ino = ino;	// Must match nodeid
+		out->body.attr.attr.mode = mode;
+		out->body.attr.attr.size = 0;
+		out->body.attr.attr.uid = uid;
+		out->body.attr.attr_valid = attr_valid;
+	})));
 }
 
+void expect_lookup(const char *relpath, uint64_t ino, mode_t mode,
+	uint64_t attr_valid, uid_t uid = 0)
+{
+	FuseTest::expect_lookup(relpath, ino, mode, 0, 1, attr_valid, uid);
+}
+
 };
 
 class Access: public DefaultPermissions {};
+class Lookup: public DefaultPermissions {};
 class Open: public DefaultPermissions {};
+class Setattr: public DefaultPermissions {};
+class Unlink: public DefaultPermissions {};
 
-/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */
-TEST_F(Access, DISABLED_eaccess)
+/* 
+ * Test permission handling during create, mkdir, mknod, link, symlink, and
+ * rename vops (they all share a common path for permission checks in
+ * VOP_LOOKUP)
+ */
+class Create: public DefaultPermissions {
+public:
+
+void expect_create(const char *relpath, uint64_t ino)
 {
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			const char *name = (const char*)in->body.bytes +
+				sizeof(fuse_open_in);
+			return (in->header.opcode == FUSE_CREATE &&
+				(0 == strcmp(relpath, name)));
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
+		SET_OUT_HEADER_LEN(out, create);
+		out->body.create.entry.attr.mode = S_IFREG | 0644;
+		out->body.create.entry.nodeid = ino;
+		out->body.create.entry.entry_valid = UINT64_MAX;
+		out->body.create.entry.attr_valid = UINT64_MAX;
+	})));
+}
+
+};
+
+class Deleteextattr: public DefaultPermissions {
+public:
+void expect_removexattr()
+{
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_REMOVEXATTR);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(0)));
+}
+};
+
+class Getextattr: public DefaultPermissions {
+public:
+void expect_getxattr(ProcessMockerT r)
+{
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_GETXATTR);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(r));
+}
+};
+
+class Listextattr: public DefaultPermissions {
+public:
+void expect_listxattr()
+{
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_LISTXATTR);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([](auto i __unused, auto out) {
+		out->body.listxattr.size = 0;
+		SET_OUT_HEADER_LEN(out, listxattr);
+	})));
+}
+};
+
+class Rename: public DefaultPermissions {
+public:
+	/* 
+	 * Expect a rename and respond with the given error.  Don't both to
+	 * validate arguments; the tests in rename.cc do that.
+	 */
+	void expect_rename(int error)
+	{
+		EXPECT_CALL(*m_mock, process(
+			ResultOf([=](auto in) {
+				return (in->header.opcode == FUSE_RENAME);
+			}, Eq(true)),
+			_)
+		).WillOnce(Invoke(ReturnErrno(error)));
+	}
+};
+
+class Setextattr: public DefaultPermissions {
+public:
+void expect_setxattr(int error)
+{
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_SETXATTR);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(error)));
+}
+};
+
+TEST_F(Access, eacces)
+{
 	const char FULLPATH[] = "mountpoint/some_file.txt";
 	const char RELPATH[] = "some_file.txt";
 	uint64_t ino = 42;
 	mode_t	access_mode = X_OK;
 
-	expect_lookup(RELPATH, ino, S_IFREG | 0644);
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX);
+
+	ASSERT_NE(0, access(FULLPATH, access_mode));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F(Access, eacces_no_cached_attrs)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	mode_t	access_mode = X_OK;
+
+	expect_getattr(1, S_IFDIR | 0755, 0, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, 0);
+	expect_getattr(ino, S_IFREG | 0644, 0, 1);
 	/* 
 	 * Once default_permissions is properly implemented, there might be
 	 * another FUSE_GETATTR or something in here.  But there should not be
@@ -84,16 +238,15 @@ TEST_F(Access, DISABLED_eaccess)
 	ASSERT_EQ(EACCES, errno);
 }
 
-/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236291 */
-TEST_F(Access, DISABLED_ok)
+TEST_F(Access, ok)
 {
 	const char FULLPATH[] = "mountpoint/some_file.txt";
 	const char RELPATH[] = "some_file.txt";
 	uint64_t ino = 42;
 	mode_t	access_mode = R_OK;
 
-	expect_lookup(RELPATH, ino, S_IFREG | 0644);
-	expect_access(ino, access_mode, 0);
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX);
 	/* 
 	 * Once default_permissions is properly implemented, there might be
 	 * another FUSE_GETATTR or something in here.
@@ -102,6 +255,227 @@ TEST_F(Access, DISABLED_ok)
 	ASSERT_EQ(0, access(FULLPATH, access_mode)) << strerror(errno);
 }
 
+TEST_F(Create, ok)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int fd;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1);
+	EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT)));
+	expect_create(RELPATH, ino);
+
+	fd = open(FULLPATH, O_CREAT | O_EXCL, 0644);
+	EXPECT_LE(0, fd) << strerror(errno);
+	/* Deliberately leak fd.  close(2) will be tested in release.cc */
+}
+
+TEST_F(Create, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT)));
+
+	EXPECT_EQ(-1, open(FULLPATH, O_CREAT | O_EXCL, 0644));
+	EXPECT_EQ(EACCES, errno);
+}
+
+TEST_F(Deleteextattr, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, 0);
+
+	ASSERT_EQ(-1, extattr_delete_file(FULLPATH, ns, "foo"));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F(Deleteextattr, ok)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, geteuid());
+	expect_removexattr();
+
+	ASSERT_EQ(0, extattr_delete_file(FULLPATH, ns, "foo"))
+		<< strerror(errno);
+}
+
+/* Delete system attributes requires superuser privilege */
+TEST_F(Deleteextattr, system)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_SYSTEM;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0666, UINT64_MAX, geteuid());
+
+	ASSERT_EQ(-1, extattr_delete_file(FULLPATH, ns, "foo"));
+	ASSERT_EQ(EPERM, errno);
+}
+
+/* Deleting user attributes merely requires WRITE privilege */
+TEST_F(Deleteextattr, user)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0666, UINT64_MAX, 0);
+	expect_removexattr();
+
+	ASSERT_EQ(0, extattr_delete_file(FULLPATH, ns, "foo"))
+		<< strerror(errno);
+}
+
+TEST_F(Getextattr, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	char data[80];
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0600, UINT64_MAX, 0);
+
+	ASSERT_EQ(-1,
+		extattr_get_file(FULLPATH, ns, "foo", data, sizeof(data)));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F(Getextattr, ok)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	char data[80];
+	const char value[] = "whatever";
+	ssize_t value_len = strlen(value) + 1;
+	int ns = EXTATTR_NAMESPACE_USER;
+	ssize_t r;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	/* Getting user attributes only requires read access */
+	expect_lookup(RELPATH, ino, S_IFREG | 0444, UINT64_MAX, 0);
+	expect_getxattr(
+		ReturnImmediate([&](auto in __unused, auto out) {
+			memcpy((void*)out->body.bytes, value, value_len);
+			out->header.len = sizeof(out->header) + value_len;
+		})
+	);
+
+	r = extattr_get_file(FULLPATH, ns, "foo", data, sizeof(data));
+	ASSERT_EQ(value_len, r)  << strerror(errno);
+	EXPECT_STREQ(value, data);
+}
+
+/* Getting system attributes requires superuser privileges */
+TEST_F(Getextattr, system)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	char data[80];
+	int ns = EXTATTR_NAMESPACE_SYSTEM;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0666, UINT64_MAX, geteuid());
+
+	ASSERT_EQ(-1,
+		extattr_get_file(FULLPATH, ns, "foo", data, sizeof(data)));
+	ASSERT_EQ(EPERM, errno);
+}
+
+TEST_F(Listextattr, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0600, UINT64_MAX, 0);
+
+	ASSERT_EQ(-1, extattr_list_file(FULLPATH, ns, NULL, 0));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F(Listextattr, ok)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1);
+	/* Listing user extended attributes merely requires read access */
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, 0);
+	expect_listxattr();
+
+	ASSERT_EQ(0, extattr_list_file(FULLPATH, ns, NULL, 0))
+		<< strerror(errno);
+}
+
+/* Listing system xattrs requires superuser privileges */
+TEST_F(Listextattr, system)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int ns = EXTATTR_NAMESPACE_SYSTEM;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1);
+	/* Listing user extended attributes merely requires read access */
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, geteuid());
+
+	ASSERT_EQ(-1, extattr_list_file(FULLPATH, ns, NULL, 0));
+	ASSERT_EQ(EPERM, errno);
+}
+
+/* A component of the search path lacks execute permissions */
+TEST_F(Lookup, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_dir/some_file.txt";
+	const char RELDIRPATH[] = "some_dir";
+	//const char FINALPATH[] = "some_file.txt";
+	uint64_t dir_ino = 42;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELDIRPATH, dir_ino, S_IFDIR | 0700, UINT64_MAX, 0);
+
+	EXPECT_EQ(-1, access(FULLPATH, F_OK));
+	EXPECT_EQ(EACCES, errno);
+}
+
+TEST_F(Open, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX);
+
+	EXPECT_NE(0, open(FULLPATH, O_RDWR));
+	EXPECT_EQ(EACCES, errno);
+}
+
 TEST_F(Open, ok)
 {
 	const char FULLPATH[] = "mountpoint/some_file.txt";
@@ -109,7 +483,8 @@ TEST_F(Open, ok)
 	uint64_t ino = 42;
 	int fd;
 
-	expect_lookup(RELPATH, ino, S_IFREG | 0644);
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX);
 	expect_open(ino, 0, 1);
 
 	fd = open(FULLPATH, O_RDONLY);
@@ -117,20 +492,292 @@ TEST_F(Open, ok)
 	/* Deliberately leak fd.  close(2) will be tested in release.cc */
 }
 
+TEST_F(Rename, eacces_on_srcdir)
+{
+	const char FULLDST[] = "mountpoint/d/dst";
+	const char RELDST[] = "d/dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	uint64_t ino = 42;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, 0);
+	expect_lookup(RELSRC, ino, S_IFREG | 0644, UINT64_MAX);
+	EXPECT_LOOKUP(1, RELDST)
+		.Times(AnyNumber())
+		.WillRepeatedly(Invoke(ReturnErrno(ENOENT)));
+
+	ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F(Rename, eacces_on_dstdir_for_creating)
+{
+	const char FULLDST[] = "mountpoint/d/dst";
+	const char RELDSTDIR[] = "d";
+	const char RELDST[] = "dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	uint64_t src_ino = 42;
+	uint64_t dstdir_ino = 43;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, 0);
+	expect_lookup(RELSRC, src_ino, S_IFREG | 0644, UINT64_MAX);
+	expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 0755, UINT64_MAX);
+	EXPECT_LOOKUP(dstdir_ino, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
+
+	ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F(Rename, eacces_on_dstdir_for_removing)
+{
+	const char FULLDST[] = "mountpoint/d/dst";
+	const char RELDSTDIR[] = "d";
+	const char RELDST[] = "dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	uint64_t src_ino = 42;
+	uint64_t dstdir_ino = 43;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, 0);
+	expect_lookup(RELSRC, src_ino, S_IFREG | 0644, UINT64_MAX);
+	expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 0755, UINT64_MAX);
+	EXPECT_LOOKUP(dstdir_ino, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
+
+	ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
+	ASSERT_EQ(EACCES, errno);
+}
+
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */
-TEST_F(Open, DISABLED_eperm)
+TEST_F(Rename, DISABLED_eperm_on_sticky_srcdir)
 {
+	const char FULLDST[] = "mountpoint/d/dst";
+	const char RELDSTDIR[] = "d";
+	const char RELDST[] = "dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	uint64_t ino = 42;
+	uint64_t dstdir_ino = 43;
+
+	expect_getattr(1, S_IFDIR | 01777, UINT64_MAX, 1, 0);
+	expect_lookup(RELSRC, ino, S_IFREG | 0644, UINT64_MAX);
+	expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 0777, UINT64_MAX);
+	EXPECT_LOOKUP(dstdir_ino, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
+
+	ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
+	ASSERT_EQ(EPERM, errno);
+}
+
+/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */
+TEST_F(Rename, DISABLED_eperm_on_sticky_dstdir)
+{
+	const char FULLDST[] = "mountpoint/d/dst";
+	const char RELDSTDIR[] = "d";
+	const char RELDST[] = "d/dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	uint64_t src_ino = 42;
+	uint64_t dstdir_ino = 43;
+	uint64_t dst_ino = 44;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, 0);
+	expect_lookup(RELSRC, src_ino, S_IFREG | 0644, UINT64_MAX);
+	expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 01777, UINT64_MAX);
+	expect_lookup(RELDST, dst_ino, S_IFREG | 0644, UINT64_MAX, 0);
+
+	ASSERT_EQ(-1, rename(FULLSRC, FULLDST));
+	ASSERT_EQ(EPERM, errno);
+}
+
+/* Successfully rename a file, overwriting the destination */
+TEST_F(Rename, ok)
+{
+	const char FULLDST[] = "mountpoint/dst";
+	const char RELDST[] = "dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	// The inode of the already-existing destination file
+	uint64_t dst_ino = 2;
+	uint64_t ino = 42;
+
+	expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, geteuid());
+	expect_lookup(RELSRC, ino, S_IFREG | 0644, UINT64_MAX);
+	expect_lookup(RELDST, dst_ino, S_IFREG | 0644, UINT64_MAX);
+	expect_rename(0);
+
+	ASSERT_EQ(0, rename(FULLSRC, FULLDST)) << strerror(errno);
+}
+
+TEST_F(Rename, ok_to_remove_src_because_of_stickiness)
+{
+	const char FULLDST[] = "mountpoint/dst";
+	const char RELDST[] = "dst";
+	const char FULLSRC[] = "mountpoint/src";
+	const char RELSRC[] = "src";
+	uint64_t ino = 42;
+
+	expect_getattr(1, S_IFDIR | 01777, UINT64_MAX, 1, 0);
+	expect_lookup(RELSRC, ino, S_IFREG | 0644, UINT64_MAX, geteuid());
+	EXPECT_LOOKUP(1, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT)));
+	expect_rename(0);
+
+	ASSERT_EQ(0, rename(FULLSRC, FULLDST)) << strerror(errno);
+}
+
+TEST_F(Setattr, ok)
+{
 	const char FULLPATH[] = "mountpoint/some_file.txt";
 	const char RELPATH[] = "some_file.txt";
-	uint64_t ino = 42;
+	const uint64_t ino = 42;
+	const mode_t oldmode = 0755;
+	const mode_t newmode = 0644;
 
-	expect_lookup(RELPATH, ino, S_IFREG | 0644);
-	/* 
-	 * Once default_permissions is properly implemented, there might be
-	 * another FUSE_GETATTR or something in here.  But there should not be
-	 * a FUSE_ACCESS
-	 */
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX, geteuid());
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([](auto in) {
+			return (in->header.opcode == FUSE_SETATTR &&
+				in->header.nodeid == ino &&
+				in->body.setattr.mode == newmode);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([](auto in __unused, auto out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out->body.attr.attr.mode = S_IFREG | newmode;
+	})));
 
-	EXPECT_NE(0, open(FULLPATH, O_RDWR));
+	EXPECT_EQ(0, chmod(FULLPATH, newmode)) << strerror(errno);
+}
+
+TEST_F(Setattr, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const uint64_t ino = 42;
+	const mode_t oldmode = 0755;
+	const mode_t newmode = 0644;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX, 0);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([](auto in) {
+			return (in->header.opcode == FUSE_SETATTR);
+		}, Eq(true)),
+		_)
+	).Times(0);
+
+	EXPECT_NE(0, chmod(FULLPATH, newmode));
 	EXPECT_EQ(EPERM, errno);
+}
+
+TEST_F(Setextattr, ok)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	const char value[] = "whatever";
+	ssize_t value_len = strlen(value) + 1;
+	int ns = EXTATTR_NAMESPACE_USER;
+	ssize_t r;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, geteuid());
+	expect_setxattr(0);
+
+	r = extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len);
+	ASSERT_EQ(value_len, r) << strerror(errno);
+}
+
+TEST_F(Setextattr, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	const char value[] = "whatever";
+	ssize_t value_len = strlen(value) + 1;
+	int ns = EXTATTR_NAMESPACE_USER;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, 0);
+
+	ASSERT_EQ(-1,
+		extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len));
+	ASSERT_EQ(EACCES, errno);
+}
+
+// Setting system attributes requires superuser privileges
+TEST_F(Setextattr, system)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	const char value[] = "whatever";
+	ssize_t value_len = strlen(value) + 1;
+	int ns = EXTATTR_NAMESPACE_SYSTEM;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | 0666, UINT64_MAX, geteuid());
+
+	ASSERT_EQ(-1,
+		extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len));
+	ASSERT_EQ(EPERM, errno);

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201904101731.x3AHV0jB050950>