client: disallow unprivileged users to escalate root privileges

An unprivileged user can `chmod 777` a directory owned by root
and gain access. Fix this bug and also add a test case for the
same.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 1cb58c815f9e3147a950408a752cba00cf7e4c22..49ca4322bbb2790819a51dbf46e3d1b4dffd10cb 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -5897,18 +5897,22 @@ int Client::may_setattr(Inode *in, struct ceph_statx *stx, int mask,
   }
 
   if (mask & CEPH_SETATTR_MODE) {
+    bool allowed = false;
+    /*
+     * Currently the kernel fuse and libfuse code is buggy and
+     * won't pass the ATTR_KILL_SUID/ATTR_KILL_SGID to ceph-fuse.
+     * But will just set the ATTR_MODE and at the same time by
+     * clearing the suid/sgid bits.
+     *
+     * Only allow unprivileged users to clear S_ISUID and S_ISUID.
+     */
+    if ((in->mode & (S_ISUID | S_ISGID)) != (stx->stx_mode & (S_ISUID | S_ISGID)) &&
+        (in->mode & ~(S_ISUID | S_ISGID)) == (stx->stx_mode & ~(S_ISUID | S_ISGID))) {
+      allowed = true;
+    }
     uint32_t m = ~stx->stx_mode & in->mode; // mode bits removed
     ldout(cct, 20) << __func__ << " " << *in << " = " << hex << m << dec <<  dendl;
-    if (perms.uid() != 0 && perms.uid() != in->uid &&
-       /*
-        * Currently the kernel fuse and libfuse code is buggy and
-        * won't pass the ATTR_KILL_SUID/ATTR_KILL_SGID to ceph-fuse.
-        * But will just set the ATTR_MODE and at the same time by
-        * clearing the suid/sgid bits.
-        *
-        * Only allow unprivileged users to clear S_ISUID and S_ISUID.
-        */
-       (m & ~(S_ISUID | S_ISGID)))
+    if (perms.uid() != 0 && perms.uid() != in->uid && !allowed)
       goto out;
 
     gid_t i_gid = (mask & CEPH_SETATTR_GID) ? stx->stx_gid : in->gid;

diff --git a/src/test/libcephfs/suidsgid.cc b/src/test/libcephfs/suidsgid.cc
index d750613ebd814626636504183928fe65c49a55d3..474795cc455d440d53b13576ebbda8735c67e440 100644 (file)
--- a/src/test/libcephfs/suidsgid.cc
+++ b/src/test/libcephfs/suidsgid.cc
@@ -134,6 +134,14 @@ void run_truncate_test_case(int mode, int result, size_t size, bool with_admin=f
   ceph_close(_cmount, fd);
 }
 
+void run_change_mode_test_case()
+{
+  char c_dir[1024];
+  sprintf(c_dir, "/mode_test_%d", getpid());
+  ASSERT_EQ(0, ceph_mkdirs(admin, c_dir, 0700));
+  ASSERT_EQ(ceph_chmod(cmount, c_dir, 0777), -CEPHFS_EPERM);
+}
+
 TEST(SuidsgidTest, WriteClearSetuid) {
   ASSERT_EQ(0, ceph_create(&admin, NULL));
   ASSERT_EQ(0, ceph_conf_read_file(admin, NULL));
@@ -206,6 +214,8 @@ TEST(SuidsgidTest, WriteClearSetuid) {
   // 14, Truncate by unprivileged user clears the suid and sgid
   run_truncate_test_case(06766, 0, 100);
 
+  run_change_mode_test_case();
+
   // clean up
   ceph_shutdown(cmount);
   ceph_shutdown(admin);