fusefs: fix some permission checks with -o default_permissions

When mounted with -o default_permissions fusefs is supposed to validate all
permissions in the kernel, not the file system.  This commit fixes two
permissions that I had previously overlooked.

* Only root may chown a file
* Non-root users may only chgrp a file to a group to which they belong

PR:		216391
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-05-01 00:00:49 +00:00
parent ede571e40a
commit 474ba6fa3b
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/projects/fuse2/; revision=346979
4 changed files with 132 additions and 6 deletions

View File

@ -1518,14 +1518,20 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
struct thread *td = curthread;
struct fuse_dispatcher fdi;
struct fuse_setattr_in *fsai;
struct mount *mp;
pid_t pid = td->td_proc->p_pid;
struct fuse_data *data;
int dataflags;
int err = 0;
enum vtype vtyp;
int sizechanged = 0;
uint64_t newsize = 0;
accmode_t accmode = 0;
mp = vnode_mount(vp);
data = fuse_get_mpdata(mp);
dataflags = data->dataflags;
if (fuse_isdeadfs(vp)) {
return ENXIO;
}
@ -1535,11 +1541,28 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
fsai->valid = 0;
if (vap->va_uid != (uid_t)VNOVAL) {
if (dataflags & FSESS_DEFAULT_PERMISSIONS) {
/* Only root may change a file's owner */
err = priv_check_cred(cred, PRIV_VFS_CHOWN);
if (err)
return err;
}
fsai->uid = vap->va_uid;
fsai->valid |= FATTR_UID;
accmode |= VADMIN;
}
if (vap->va_gid != (gid_t)VNOVAL) {
if (dataflags & FSESS_DEFAULT_PERMISSIONS &&
!groupmember(vap->va_gid, cred))
{
/*
* Non-root users may only chgrp to one of their own
* groups
*/
err = priv_check_cred(cred, PRIV_VFS_CHOWN);
if (err)
return err;
}
fsai->gid = vap->va_gid;
fsai->valid |= FATTR_GID;
accmode |= VADMIN;

View File

@ -69,7 +69,7 @@ virtual void SetUp() {
public:
void expect_getattr(uint64_t ino, mode_t mode, uint64_t attr_valid, int times,
uid_t uid = 0)
uid_t uid = 0, gid_t gid = 0)
{
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
@ -84,19 +84,22 @@ void expect_getattr(uint64_t ino, mode_t mode, uint64_t attr_valid, int times,
out->body.attr.attr.mode = mode;
out->body.attr.attr.size = 0;
out->body.attr.attr.uid = uid;
out->body.attr.attr.uid = gid;
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)
uint64_t attr_valid, uid_t uid = 0, gid_t gid = 0)
{
FuseTest::expect_lookup(relpath, ino, mode, 0, 1, attr_valid, uid);
FuseTest::expect_lookup(relpath, ino, mode, 0, 1, attr_valid, uid, gid);
}
};
class Access: public DefaultPermissions {};
class Chown: public DefaultPermissions {};
class Chgrp: public DefaultPermissions {};
class Lookup: public DefaultPermissions {};
class Open: public DefaultPermissions {};
class Setattr: public DefaultPermissions {};
@ -254,6 +257,105 @@ TEST_F(Access, ok)
ASSERT_EQ(0, access(FULLPATH, access_mode)) << strerror(errno);
}
/* Only root may change a file's owner */
TEST_F(Chown, eperm)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
const uint64_t ino = 42;
const mode_t mode = 0755;
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, geteuid());
expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, geteuid());
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).Times(0);
EXPECT_NE(0, chown(FULLPATH, 0, -1));
EXPECT_EQ(EPERM, errno);
}
/* non-root users may only chgrp a file to a group they belong to */
TEST_F(Chgrp, eperm)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
const uint64_t ino = 42;
const mode_t mode = 0755;
int ngroups = 64;
gid_t groups[ngroups];
uid_t uid;
gid_t gid, newgid;
int i;
uid = geteuid();
gid = getegid();
getgrouplist(getlogin(), getegid(), groups, &ngroups);
for (newgid = 0; newgid >= 0; newgid++) {
bool belongs = false;
for (i = 0; i < ngroups; i++) {
if (groups[i] == newgid)
belongs = true;
}
if (!belongs)
break;
}
/* newgid is now a group to which the current user does not belong */
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid, gid);
expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, uid, gid);
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).Times(0);
EXPECT_NE(0, chown(FULLPATH, -1, newgid));
EXPECT_EQ(EPERM, errno);
}
TEST_F(Chgrp, ok)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
const uint64_t ino = 42;
const mode_t mode = 0755;
uid_t uid;
gid_t gid, newgid;
uid = geteuid();
gid = 0;
newgid = getegid();
expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1, uid, gid);
expect_lookup(RELPATH, ino, S_IFREG | mode, UINT64_MAX, uid, gid);
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).Times(0);
EXPECT_CALL(*m_mock, process(
ResultOf([](auto in) {
return (in->header.opcode == FUSE_SETATTR &&
in->header.nodeid == ino);
}, Eq(true)),
_)
).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) {
SET_OUT_HEADER_LEN(out, attr);
out->body.attr.attr.mode = S_IFREG | mode;
out->body.attr.attr.uid = uid;
out->body.attr.attr.gid = newgid;
})));
EXPECT_EQ(0, chown(FULLPATH, -1, newgid)) << strerror(errno);
}
TEST_F(Create, ok)
{
const char FULLPATH[] = "mountpoint/some_file.txt";

View File

@ -175,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 attr_valid, uid_t uid)
uint64_t size, int times, uint64_t attr_valid, uid_t uid, gid_t gid)
{
EXPECT_LOOKUP(1, relpath)
.Times(times)
@ -187,6 +187,7 @@ void FuseTest::expect_lookup(const char *relpath, uint64_t ino, mode_t mode,
out->body.entry.attr_valid = attr_valid;
out->body.entry.attr.size = size;
out->body.entry.attr.uid = uid;
out->body.entry.attr.gid = gid;
})));
}

View File

@ -103,7 +103,7 @@ class FuseTest : public ::testing::Test {
*/
void expect_lookup(const char *relpath, uint64_t ino, mode_t mode,
uint64_t size, int times, uint64_t attr_valid = UINT64_MAX,
uid_t uid = 0);
uid_t uid = 0, gid_t gid = 0);
/*
* Create an expectation that FUSE_OPEN will be called for the given