From 58345df35a413175d0e6b4c3aa072c18fd2d0782 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 7 Dec 2016 11:52:58 -0500 Subject: [PATCH 1/4] test: remove improper casts from SetSize test Signed-off-by: Jeff Layton --- src/test/libcephfs/test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/libcephfs/test.cc b/src/test/libcephfs/test.cc index e8eaef4ba52..6052fe12457 100644 --- a/src/test/libcephfs/test.cc +++ b/src/test/libcephfs/test.cc @@ -1694,10 +1694,10 @@ TEST(LibCephFS, SetSize) { struct ceph_statx stx; uint64_t size = 8388608; - stx.stx_size = (off_t)size; + stx.stx_size = size; ASSERT_EQ(ceph_fsetattrx(cmount, fd, &stx, CEPH_SETATTR_SIZE), 0); ASSERT_EQ(ceph_fstatx(cmount, fd, &stx, CEPH_STATX_SIZE, 0), 0); - ASSERT_EQ(stx.stx_size, (off_t)size); + ASSERT_EQ(stx.stx_size, size); ceph_close(cmount, fd); ceph_shutdown(cmount); From 3d7fa898030ba6a52346e8576435ce04b6f592c1 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 6 Dec 2016 14:10:31 -0500 Subject: [PATCH 2/4] test: add new testcase for clearing setuid/setgid bits on chown/chgrp Signed-off-by: Jeff Layton --- src/test/libcephfs/test.cc | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/test/libcephfs/test.cc b/src/test/libcephfs/test.cc index 6052fe12457..5f43f4ce2e8 100644 --- a/src/test/libcephfs/test.cc +++ b/src/test/libcephfs/test.cc @@ -1702,3 +1702,40 @@ TEST(LibCephFS, SetSize) { ceph_close(cmount, fd); ceph_shutdown(cmount); } + +TEST(LibCephFS, ClearSetuid) { + struct ceph_mount_info *cmount; + ASSERT_EQ(ceph_create(&cmount, NULL), 0); + ASSERT_EQ(ceph_conf_read_file(cmount, NULL), 0); + ASSERT_EQ(0, ceph_conf_parse_env(cmount, NULL)); + ASSERT_EQ(ceph_mount(cmount, "/"), 0); + + Inode *root; + ASSERT_EQ(ceph_ll_lookup_root(cmount, &root), 0); + + char filename[32]; + sprintf(filename, "clearsetuid%x", getpid()); + + Fh *fh; + Inode *in; + struct ceph_statx stx; + + ceph_ll_unlink(cmount, root, filename, ceph_mount_perms(cmount)); + ASSERT_EQ(ceph_ll_create(cmount, root, filename, 06555, + O_RDWR|O_CREAT|O_EXCL, &in, &fh, &stx, + CEPH_STATX_UID|CEPH_STATX_GID|CEPH_STATX_MODE, 0, + ceph_mount_perms(cmount)), 0); + + ASSERT_EQ(stx.stx_mode & 07777, 06555); + + UserPerm *rootcred = ceph_userperm_new(0, 0, 0, NULL); + ASSERT_TRUE(rootcred); + stx.stx_uid++; + stx.stx_gid++; + ASSERT_EQ(ceph_ll_setattr(cmount, in, &stx, CEPH_SETATTR_UID|CEPH_SETATTR_GID, rootcred), 0); + ASSERT_EQ(ceph_ll_getattr(cmount, in, &stx, CEPH_STATX_MODE, 0, ceph_mount_perms(cmount)), 0); + ASSERT_TRUE(stx.stx_mask & CEPH_STATX_MODE); + ASSERT_FALSE(stx.stx_mode & (S_ISUID|S_ISGID)); + + ceph_shutdown(cmount); +} From 6da72500882d9749cb2be6eaa2568e6fe6e5ff4d Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 5 Dec 2016 14:19:23 -0500 Subject: [PATCH 3/4] mds: clear setuid/setgid bits on ownership changes If we get a ownership change, POSIX mandates that you clear the setuid and setgid bits unless you are "appropriately privileged", in which case the OS is allowed to leave them intact. Linux however always clears those bits, regardless of the process privileges, as that makes it simpler to close some potential races. Have ceph do the same. Signed-off-by: Jeff Layton --- src/mds/Server.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 4f1057bf754..1293d7d0bc2 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -3793,13 +3793,20 @@ void Server::handle_client_setattr(MDRequestRef& mdr) pi = cur->project_inode(); - if (mask & CEPH_SETATTR_MODE) - pi->mode = (pi->mode & ~07777) | (req->head.args.setattr.mode & 07777); if (mask & CEPH_SETATTR_UID) pi->uid = req->head.args.setattr.uid; if (mask & CEPH_SETATTR_GID) pi->gid = req->head.args.setattr.gid; + if (mask & CEPH_SETATTR_MODE) + pi->mode = (pi->mode & ~07777) | (req->head.args.setattr.mode & 07777); + else if ((mask & (CEPH_SETATTR_UID|CEPH_SETATTR_GID)) && + S_ISREG(pi->mode)) { + pi->mode &= ~S_ISUID; + if ((pi->mode & (S_ISGID|S_IXGRP)) == (S_ISGID|S_IXGRP)) + pi->mode &= ~S_ISGID; + } + if (mask & CEPH_SETATTR_MTIME) pi->mtime = req->head.args.setattr.mtime; if (mask & CEPH_SETATTR_ATIME) From 18d2499d6c85a10b4b54f3b8c335cddf86c4588f Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 5 Dec 2016 14:11:44 -0500 Subject: [PATCH 4/4] client: drop setuid/setgid bits on ownership change When we hold exclusive auth caps, then the client is responsible for handling changes to the mode. Make sure we remove any setuid/setgid bits on an ownership change. Signed-off-by: Jeff Layton --- src/client/Client.cc | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index a8dd3a2af57..3638863e81e 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -6494,15 +6494,8 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, } if (in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { - if (mask & CEPH_SETATTR_MODE) { - in->ctime = ceph_clock_now(cct); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); - in->mode = (in->mode & ~07777) | (stx->stx_mode & 07777); - mark_caps_dirty(in, CEPH_CAP_AUTH_EXCL); - mask &= ~CEPH_SETATTR_MODE; - ldout(cct,10) << "changing mode to " << stx->stx_mode << dendl; - } + bool kill_sguid = false; + if (mask & CEPH_SETATTR_UID) { in->ctime = ceph_clock_now(cct); in->cap_dirtier_uid = perms.uid(); @@ -6510,6 +6503,7 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, in->uid = stx->stx_uid; mark_caps_dirty(in, CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_UID; + kill_sguid = true; ldout(cct,10) << "changing uid to " << stx->stx_uid << dendl; } if (mask & CEPH_SETATTR_GID) { @@ -6519,8 +6513,26 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, in->gid = stx->stx_gid; mark_caps_dirty(in, CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_GID; + kill_sguid = true; ldout(cct,10) << "changing gid to " << stx->stx_gid << dendl; } + + if (mask & CEPH_SETATTR_MODE) { + in->ctime = ceph_clock_now(cct); + in->cap_dirtier_uid = perms.uid(); + in->cap_dirtier_gid = perms.gid(); + in->mode = (in->mode & ~07777) | (stx->stx_mode & 07777); + mark_caps_dirty(in, CEPH_CAP_AUTH_EXCL); + mask &= ~CEPH_SETATTR_MODE; + ldout(cct,10) << "changing mode to " << stx->stx_mode << dendl; + } else if (kill_sguid && S_ISREG(in->mode)) { + /* Must squash the any setuid/setgid bits with an ownership change */ + in->mode &= ~S_ISUID; + if ((in->mode & (S_ISGID|S_IXGRP)) == (S_ISGID|S_IXGRP)) + in->mode &= ~S_ISGID; + mark_caps_dirty(in, CEPH_CAP_AUTH_EXCL); + } + if (mask & CEPH_SETATTR_BTIME) { in->ctime = ceph_clock_now(cct); in->cap_dirtier_uid = perms.uid();