mds: fix chown/chgrp check and tests

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/mds/MDSAuthCaps.cc b/src/mds/MDSAuthCaps.cc
index c9475560162158c324e566b2776eeeba8543bf51..483e0a18774d27503bb59c5f303c03e041241046 100644 (file)
--- a/src/mds/MDSAuthCaps.cc
+++ b/src/mds/MDSAuthCaps.cc
@@ -137,7 +137,8 @@ bool MDSAuthCaps::is_capable(const std::string &inode_path,
                             uid_t inode_uid, gid_t inode_gid,
                             unsigned inode_mode,
                             uid_t caller_uid, gid_t caller_gid,
-                            unsigned mask) const
+                            unsigned mask,
+                            uid_t new_uid, gid_t new_gid) const
 {
   if (cct)
     ldout(cct, 10) << __func__ << " inode(path /" << inode_path
@@ -145,6 +146,7 @@ bool MDSAuthCaps::is_capable(const std::string &inode_path,
                   << " mode 0" << std::oct << inode_mode << std::dec
                   << ") by caller " << caller_uid << ":" << caller_gid
                   << " mask " << mask
+                  << " new " << new_uid << ":" << new_gid
                   << " cap: " << *this << dendl;
 
   for (std::vector<MDSCapGrant>::const_iterator i = grants.begin();
@@ -165,6 +167,21 @@ bool MDSAuthCaps::is_capable(const std::string &inode_path,
        continue;
       }
 
+      // chown/chgrp
+      if (mask & MAY_CHOWN) {
+       if (new_uid != caller_uid ||   // you can't chown to someone else
+           inode_uid != caller_uid) { // you can't chown from someone else
+         continue;
+       }
+      }
+      if (mask & MAY_CHGRP) {
+       // you can only chgrp *to* one of your groups... if you own the file.
+       if (inode_uid != caller_uid ||
+           std::find(i->match.gids.begin(), i->match.gids.end(), new_gid) ==
+           i->match.gids.end()) {
+         continue;
+       }
+      }
 
       if (inode_uid == caller_uid) {
         if ((!(mask & MAY_READ) || (inode_mode & S_IRUSR)) &&

diff --git a/src/mds/MDSAuthCaps.h b/src/mds/MDSAuthCaps.h
index 88beeff1e6fd455fc13b92058eab2ce743a440b7..7f361ecaefdde25a23121d4024588e36848bf40d 100644 (file)
--- a/src/mds/MDSAuthCaps.h
+++ b/src/mds/MDSAuthCaps.h
@@ -111,7 +111,8 @@ public:
   bool allow_all() const;
   bool is_capable(const std::string &inode_path,
                  uid_t inode_uid, gid_t inode_gid, unsigned inode_mode,
-                 uid_t uid, gid_t gid, unsigned mask) const;
+                 uid_t uid, gid_t gid, unsigned mask,
+                 uid_t new_uid, gid_t new_gid) const;
 
   friend std::ostream &operator<<(std::ostream &out, const MDSAuthCaps &cap);
 };

diff --git a/src/mds/SessionMap.cc b/src/mds/SessionMap.cc
index 7f6b016ea721fe8566cee8dfef9aea353c305c18..e2034f89f2ca21c7bf8db2f138f7e71c70474efb 100644 (file)
--- a/src/mds/SessionMap.cc
+++ b/src/mds/SessionMap.cc
@@ -837,7 +837,7 @@ void Session::decode(bufferlist::iterator &p)
 
 bool Session::check_access(CInode *in, unsigned mask,
                           int caller_uid, int caller_gid,
-                          int setattr_uid, int setattr_gid)
+                          int new_uid, int new_gid)
 {
   string path;
   CInode *diri = in->get_parent_inode();
@@ -857,21 +857,15 @@ bool Session::check_access(CInode *in, unsigned mask,
     if (!(auth_caps.is_capable(path, caller_uid, caller_gid,
                               0 /* irrelevant */,
                               caller_uid, caller_gid,
-                              MAY_CREATE))) {
+                              MAY_CREATE, 0, 0))) {
       return false;
     }
     mask &= ~MAY_CREATE;
   }
 
-  if ((mask & (MAY_CHOWN|MAY_CHGRP)) &&
-      !(auth_caps.is_capable(path, in->inode.uid, in->inode.gid, in->inode.mode,
-                            caller_uid, caller_gid, mask))) {
-    return false;
-  }
-
-  // normal unix MAY_{READ,WRITE} checks
   if (auth_caps.is_capable(path, in->inode.uid, in->inode.gid, in->inode.mode,
-                          caller_uid, caller_gid, mask)) {
+                          caller_uid, caller_gid, mask,
+                          new_uid, new_gid)) {
     return true;
   }
   return false;

diff --git a/src/mds/SessionMap.h b/src/mds/SessionMap.h
index 6a2a840b635b0061ca83955fde870a9e6aa7c2c9..5dc459a9d6b5ee3b7dd61f10b229559f9eea899c 100644 (file)
--- a/src/mds/SessionMap.h
+++ b/src/mds/SessionMap.h
@@ -301,7 +301,8 @@ public:
     completed_requests_dirty = false;
   }
 
-  bool check_access(CInode *in, unsigned mask, int caller_uid, int caller_gid, int setattr_uid, int setattr_gid);
+  bool check_access(CInode *in, unsigned mask, int caller_uid, int caller_gid,
+                   int new_uid, int new_gid);
 
 
   Session() : 

diff --git a/src/test/libcephfs/access.cc b/src/test/libcephfs/access.cc
index 7dc8e4bc49cccd47585936e01e945a20057ee5f2..5dfacea767dbc1887fc7b3a5ffb970fae9bde914 100644 (file)
--- a/src/test/libcephfs/access.cc
+++ b/src/test/libcephfs/access.cc
@@ -303,6 +303,30 @@ TEST(AccessTest, User) {
   ASSERT_EQ(0, ceph_chown(admin, dir.c_str(), 123, 456));
   ASSERT_EQ(-EACCES, ceph_mkdir(cmount, string(dir + "/no").c_str(), 0755));
 
+  // chown and chgrp
+  ASSERT_EQ(0, ceph_chmod(admin, dir.c_str(), 0700));
+  ASSERT_EQ(0, ceph_chown(admin, dir.c_str(), 123, 456));
+  ASSERT_EQ(0, ceph_chown(cmount, dir.c_str(), 123, 789));
+  ASSERT_EQ(0, ceph_chown(cmount, dir.c_str(), 123, 456));
+  ASSERT_EQ(0, ceph_chown(cmount, dir.c_str(), -1, 789));
+  ASSERT_EQ(0, ceph_chown(cmount, dir.c_str(), -1, 456));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), 123, 1));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), 1, 456));
+
+  ASSERT_EQ(0, ceph_chown(admin, dir.c_str(), 1, 1));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), 123, 456));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), 123, -1));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), -1, 456));
+
+  ASSERT_EQ(0, ceph_chown(admin, dir.c_str(), 1, 456));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), 123, 456));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), 123, -1));
+  ASSERT_EQ(-EACCES, ceph_chown(cmount, dir.c_str(), -1, 456));
+
+  ASSERT_EQ(0, ceph_chown(admin, dir.c_str(), 123, 1));
+  ASSERT_EQ(0, ceph_chown(cmount, dir.c_str(), -1, 456));
+  ASSERT_EQ(0, ceph_chown(cmount, dir.c_str(), 123, 789));
+
   ceph_shutdown(cmount);
 
   // clean up

diff --git a/src/test/mds/TestMDSAuthCaps.cc b/src/test/mds/TestMDSAuthCaps.cc
index 0cd0e8ef093640ca02e645b4f97b06c86ce3cd92..49d91f21c03f05555f06d1c3dfa2490dbfeb9cf1 100644 (file)
--- a/src/test/mds/TestMDSAuthCaps.cc
+++ b/src/test/mds/TestMDSAuthCaps.cc
@@ -110,37 +110,37 @@ TEST(MDSAuthCaps, AllowAll) {
 
   ASSERT_TRUE(cap.parse(g_ceph_context, "allow *", NULL));
   ASSERT_TRUE(cap.allow_all());
-  ASSERT_TRUE(cap.is_capable("foo/bar", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE));
+  ASSERT_TRUE(cap.is_capable("foo/bar", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
 }
 
 TEST(MDSAuthCaps, AllowUid) {
   MDSAuthCaps cap;
   ASSERT_TRUE(cap.parse(g_ceph_context, "allow * uid=10", NULL));
   ASSERT_FALSE(cap.allow_all());
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 10, 0, MAY_READ | MAY_WRITE));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, -1, 0, MAY_READ | MAY_WRITE));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0775, 10, 10, MAY_READ));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0777, 10, 10, MAY_WRITE));
-  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0755, 10, 10, MAY_WRITE));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 10, MAY_READ|MAY_CREATE));
-  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0755, 0, 0, MAY_READ));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0777, 10, 10, MAY_READ));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0777, 10, 10, MAY_READ|MAY_CREATE));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0557, 10, 10, MAY_READ));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0557, 10, 10, MAY_READ));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0557, 10, 10, MAY_CREATE));
-  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0577, 10, 10, MAY_WRITE));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 10, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, -1, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0775, 10, 10, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0777, 10, 10, MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0755, 10, 10, MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 10, MAY_READ|MAY_CREATE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0755, 0, 0, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0777, 10, 10, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0777, 10, 10, MAY_READ|MAY_CREATE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0557, 10, 10, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0557, 10, 10, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0557, 10, 10, MAY_CREATE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0577, 10, 10, MAY_WRITE, 0, 0));
 }
 
 TEST(MDSAuthCaps, AllowPath) {
   MDSAuthCaps cap;
   ASSERT_TRUE(cap.parse(g_ceph_context, "allow * path=/sandbox", NULL));
   ASSERT_FALSE(cap.allow_all());
-  ASSERT_TRUE(cap.is_capable("sandbox/foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE));
-  ASSERT_TRUE(cap.is_capable("sandbox", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE));
-  ASSERT_FALSE(cap.is_capable("sandboxed", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE));
+  ASSERT_TRUE(cap.is_capable("sandbox/foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("sandbox", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("sandboxed", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
 }
 
 TEST(MDSAuthCaps, OutputParsed) {