From 79e82cb45e8f20f93dbb73cb9835f3d85773fe98 Mon Sep 17 00:00:00 2001 From: asomers Date: Wed, 10 Apr 2019 17:31:00 +0000 Subject: [PATCH] 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 --- sys/fs/fuse/fuse_internal.c | 22 +- sys/fs/fuse/fuse_internal.h | 2 +- sys/fs/fuse/fuse_vnops.c | 109 ++-- tests/sys/fs/fusefs/default_permissions.cc | 687 ++++++++++++++++++++- tests/sys/fs/fusefs/destroy.cc | 13 - tests/sys/fs/fusefs/lookup.cc | 2 + tests/sys/fs/fusefs/setattr.cc | 2 + tests/sys/fs/fusefs/unlink.cc | 12 - tests/sys/fs/fusefs/utils.cc | 32 +- tests/sys/fs/fusefs/utils.hh | 19 +- tests/sys/fs/fusefs/xattr.cc | 2 + 11 files changed, 811 insertions(+), 91 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 98413d8a50e..97ddc35cb19 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -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)) diff --git a/sys/fs/fuse/fuse_internal.h b/sys/fs/fuse/fuse_internal.h index e878901ef96..fcb8c0e35d0 100644 --- a/sys/fs/fuse/fuse_internal.h +++ b/sys/fs/fuse/fuse_internal.h @@ -232,7 +232,7 @@ fuse_match_cred(struct ucred *basecred, struct ucred *usercred) 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 */ diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 3efa2f4a6cf..517e9891320 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -829,19 +829,12 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) /* 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; - goto out; - } -#if 0 /* THINK_ABOUT_THIS */ - if ((err = fuse_internal_access(dvp, VWRITE, cred, td, &facp))) { + /* Entry not found */ + if ((nameiop == CREATE || nameiop == RENAME) && islastcn) { + err = fuse_internal_access(dvp, VWRITE, &facp, td, + cred); + if (err) goto out; - } -#endif /* * Possibly record the position of a slot in the @@ -860,15 +853,40 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) 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 @@ -877,17 +895,6 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) * 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 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) * 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 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) 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 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) &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. @@ -2317,6 +2352,10 @@ fuse_vnop_deleteextattr(struct vop_deleteextattr_args *ap) 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) prefix = EXTATTR_NAMESPACE_SYSTEM_STRING; diff --git a/tests/sys/fs/fusefs/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc index 0097e058dfa..f639d8aabeb 100644 --- a/tests/sys/fs/fusefs/default_permissions.cc +++ b/tests/sys/fs/fusefs/default_permissions.cc @@ -34,6 +34,9 @@ */ extern "C" { +#include +#include + #include #include } @@ -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_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_lookup(RELPATH, ino, S_IFREG | 0644); + 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(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(Open, DISABLED_eperm) +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); +} + +// Setting user attributes merely requires write privileges +TEST_F(Setextattr, user) +{ + 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 | 0666, UINT64_MAX, 0); + expect_setxattr(0); + + r = extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len); + ASSERT_EQ(value_len, r) << strerror(errno); +} + +TEST_F(Unlink, ok) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + + expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1); + expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, geteuid()); + expect_unlink(1, RELPATH, 0); + + ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno); +} + +TEST_F(Unlink, unwritable_directory) +{ + 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, geteuid()); + + ASSERT_EQ(-1, unlink(FULLPATH)); + ASSERT_EQ(EACCES, errno); +} + +/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216391 */ +TEST_F(Unlink, DISABLED_sticky_directory) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + + expect_getattr(1, S_IFDIR | 01777, UINT64_MAX, 1); + expect_lookup(RELPATH, ino, S_IFREG | 0644, UINT64_MAX, 0); + + ASSERT_EQ(-1, unlink(FULLPATH)); + ASSERT_EQ(EPERM, errno); +} diff --git a/tests/sys/fs/fusefs/destroy.cc b/tests/sys/fs/fusefs/destroy.cc index da3647ffff6..168e82eb212 100644 --- a/tests/sys/fs/fusefs/destroy.cc +++ b/tests/sys/fs/fusefs/destroy.cc @@ -45,19 +45,6 @@ void expect_destroy(int error) ).WillOnce(Invoke(ReturnErrno(error))); } -void expect_forget(uint64_t ino, uint64_t nlookup) -{ - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in->header.opcode == FUSE_FORGET && - in->header.nodeid == ino && - in->body.forget.nlookup == nlookup); - }, Eq(true)), - _) - ).WillOnce(Invoke([](auto in __unused, auto &out __unused) { - /* FUSE_FORGET has no response! */ - })); -} }; /* diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc index 988f734673d..bc0150b61c6 100644 --- a/tests/sys/fs/fusefs/lookup.cc +++ b/tests/sys/fs/fusefs/lookup.cc @@ -144,6 +144,8 @@ TEST_F(Lookup, enoent) EXPECT_EQ(ENOENT, errno); } +//TODO: test ENOTDIR + /* * If lookup returns a non-zero entry timeout, then subsequent VOP_LOOKUPs * should use the cached inode rather than requery the daemon diff --git a/tests/sys/fs/fusefs/setattr.cc b/tests/sys/fs/fusefs/setattr.cc index 8a413032059..c397632c7d5 100644 --- a/tests/sys/fs/fusefs/setattr.cc +++ b/tests/sys/fs/fusefs/setattr.cc @@ -553,3 +553,5 @@ TEST_F(Setattr, utimensat_mtime_only) { EXPECT_EQ(0, utimensat(AT_FDCWD, FULLPATH, &newtimes[0], 0)) << strerror(errno); } + +// TODO: test for erofs diff --git a/tests/sys/fs/fusefs/unlink.cc b/tests/sys/fs/fusefs/unlink.cc index b7516f192d5..a9beb73a6a4 100644 --- a/tests/sys/fs/fusefs/unlink.cc +++ b/tests/sys/fs/fusefs/unlink.cc @@ -43,18 +43,6 @@ void expect_lookup(const char *relpath, uint64_t ino, int times) FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, times); } -void expect_unlink(uint64_t parent, const char *path, int error) -{ - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in->header.opcode == FUSE_UNLINK && - 0 == strcmp(path, in->body.unlink) && - in->header.nodeid == parent); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(error))); -} - }; TEST_F(Unlink, eperm) diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index 27f8ca00db5..29b6a751444 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -142,6 +142,21 @@ FuseTest::expect_flush(uint64_t ino, int times, ProcessMockerT r) .WillRepeatedly(Invoke(r)); } +void +FuseTest::expect_forget(uint64_t ino, uint64_t nlookup) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_FORGET && + in->header.nodeid == ino && + in->body.forget.nlookup == nlookup); + }, Eq(true)), + _) + ).WillOnce(Invoke([](auto in __unused, auto &out __unused) { + /* FUSE_FORGET has no response! */ + })); +} + void FuseTest::expect_getattr(uint64_t ino, uint64_t size) { EXPECT_CALL(*m_mock, process( @@ -160,7 +175,7 @@ void FuseTest::expect_getattr(uint64_t ino, uint64_t size) } void FuseTest::expect_lookup(const char *relpath, uint64_t ino, mode_t mode, - uint64_t size, int times) + uint64_t size, int times, uint64_t attr_valid, uid_t uid) { EXPECT_LOOKUP(1, relpath) .Times(times) @@ -169,8 +184,9 @@ void FuseTest::expect_lookup(const char *relpath, uint64_t ino, mode_t mode, out->body.entry.attr.mode = mode; out->body.entry.nodeid = ino; out->body.entry.attr.nlink = 1; - out->body.entry.attr_valid = UINT64_MAX; + out->body.entry.attr_valid = attr_valid; out->body.entry.attr.size = size; + out->body.entry.attr.uid = uid; }))); } @@ -258,6 +274,18 @@ void FuseTest::expect_releasedir(uint64_t ino, ProcessMockerT r) ).WillOnce(Invoke(r)); } +void FuseTest::expect_unlink(uint64_t parent, const char *path, int error) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_UNLINK && + 0 == strcmp(path, in->body.unlink) && + in->header.nodeid == parent); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(error))); +} + void FuseTest::expect_write(uint64_t ino, uint64_t offset, uint64_t isize, uint64_t osize, uint32_t flags, const void *contents) { diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index b9114ab1618..c1967a38822 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -83,6 +83,12 @@ class FuseTest : public ::testing::Test { */ void expect_flush(uint64_t ino, int times, ProcessMockerT r); + /* + * Create an expectation that FUSE_FORGET will be called for the given + * inode. There will be no response + */ + void expect_forget(uint64_t ino, uint64_t nlookup); + /* * Create an expectation that FUSE_GETATTR will be called for the given * inode any number of times. It will respond with a few basic @@ -92,11 +98,12 @@ class FuseTest : public ::testing::Test { /* * Create an expectation that FUSE_LOOKUP will be called for the given - * path exactly times times. It will respond with inode ino, mode - * mode, filesize size, and cache validity forever. + * path exactly times times and cache validity period. It will respond + * with inode ino, mode mode, filesize size. */ void expect_lookup(const char *relpath, uint64_t ino, mode_t mode, - uint64_t size, int times); + uint64_t size, int times, uint64_t attr_valid = UINT64_MAX, + uid_t uid = 0); /* * Create an expectation that FUSE_GETATTR will be called for the given @@ -131,6 +138,12 @@ class FuseTest : public ::testing::Test { */ void expect_releasedir(uint64_t ino, ProcessMockerT r); + /* + * Create an expectation that FUSE_UNLINK will be called exactly once + * for the given path, returning an errno + */ + void expect_unlink(uint64_t parent, const char *path, int error); + /* * Create an expectation that FUSE_WRITE will be called exactly once * for the given inode, at offset offset, with write_flags flags, diff --git a/tests/sys/fs/fusefs/xattr.cc b/tests/sys/fs/fusefs/xattr.cc index 6cd5263d7ec..c42e9a03318 100644 --- a/tests/sys/fs/fusefs/xattr.cc +++ b/tests/sys/fs/fusefs/xattr.cc @@ -603,3 +603,5 @@ TEST_F(Setxattr, system) r = extattr_set_file(FULLPATH, ns, "foo", (void*)value, value_len); ASSERT_EQ(value_len, r) << strerror(errno); } + +// TODO: EROFS tests -- 2.45.0