mds: pass full gid list to MDSAuthCap checks, when present

Signed-off-by: Greg Farnum <gfarnum@redhat.com>

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index 966a5f2c12fabba30a90e4f433409f10037f075d..51eff4cfb9460903a4dfff3abfa0c6077982290d 100644 (file)
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -3155,7 +3155,7 @@ bool Locker::_do_cap_update(CInode *in, Capability *cap,
 
   Session *session = static_cast<Session *>(m->get_connection()->get_priv());
   if (session->check_access(in, MAY_WRITE,
-                           m->caller_uid, m->caller_gid, 0, 0) < 0) {
+                           m->caller_uid, m->caller_gid, NULL, 0, 0) < 0) {
     session->put();
     dout(10) << "check_access failed, dropping cap update on " << *in << dendl;
     return false;

diff --git a/src/mds/MDSAuthCaps.cc b/src/mds/MDSAuthCaps.cc
index c774d3f2473ea3bfad90cd4304e06bf51bfac761..415142e983a1d08d5ae75d49999a49add20befe7 100644 (file)
--- a/src/mds/MDSAuthCaps.cc
+++ b/src/mds/MDSAuthCaps.cc
@@ -110,12 +110,24 @@ void MDSCapMatch::normalize_path()
 
 bool MDSCapMatch::match(const std::string &target_path,
                        const int caller_uid,
-                       const int caller_gid) const
+                       const int caller_gid,
+                       const vector<uint64_t> *caller_gid_list) const
 {
   if (uid != MDS_AUTH_UID_ANY) {
     if (uid != caller_uid)
       return false;
-    if (std::find(gids.begin(), gids.end(), caller_gid) == gids.end())
+    bool gid_matched = false;
+    if (std::find(gids.begin(), gids.end(), caller_gid) != gids.end())
+      gid_matched = true;
+    if (caller_gid_list) {
+      for (auto i = caller_gid_list->begin(); i != caller_gid_list->end(); ++i) {
+       if (std::find(gids.begin(), gids.end(), *i) != gids.end()) {
+         gid_matched = true;
+         break;
+       }
+      }
+    }
+    if (!gid_matched)
       return false;
   }
 
@@ -168,6 +180,7 @@ 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,
+                            const vector<uint64_t> *caller_gid_list,
                             unsigned mask,
                             uid_t new_uid, gid_t new_gid) const
 {
@@ -176,6 +189,7 @@ bool MDSAuthCaps::is_capable(const std::string &inode_path,
                   << " owner " << inode_uid << ":" << inode_gid
                   << " mode 0" << std::oct << inode_mode << std::dec
                   << ") by caller " << caller_uid << ":" << caller_gid
+// << "[" << caller_gid_list << "]";
                   << " mask " << mask
                   << " new " << new_uid << ":" << new_gid
                   << " cap: " << *this << dendl;
@@ -184,7 +198,7 @@ bool MDSAuthCaps::is_capable(const std::string &inode_path,
        i != grants.end();
        ++i) {
 
-    if (i->match.match(inode_path, caller_uid, caller_gid) &&
+    if (i->match.match(inode_path, caller_uid, caller_gid, caller_gid_list) &&
        i->spec.allows(mask & (MAY_READ|MAY_EXECUTE), mask & MAY_WRITE)) {
 
       // Spec is non-allowing if caller asked for set pool but spec forbids it

diff --git a/src/mds/MDSAuthCaps.h b/src/mds/MDSAuthCaps.h
index eee7707fe03cc1d7cac5881dc99c4623f760c632..19a62a6c936305afa3ceebebc8c3e16fd167c7db 100644 (file)
--- a/src/mds/MDSAuthCaps.h
+++ b/src/mds/MDSAuthCaps.h
@@ -93,7 +93,8 @@ struct MDSCapMatch {
   // check whether this grant matches against a given file and caller uid:gid
   bool match(const std::string &target_path,
             const int caller_uid,
-            const int caller_gid) const;
+            const int caller_gid,
+            const vector<uint64_t> *caller_gid_list) const;
 
   /**
    * Check whether this path *might* be accessible (actual permission
@@ -132,8 +133,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,
-                 uid_t new_uid, gid_t new_gid) const;
+                 uid_t uid, gid_t gid, const vector<uint64_t> *caller_gid_list,
+                 unsigned mask, uid_t new_uid, gid_t new_gid) const;
   bool path_capable(const std::string &inode_path) const;
 
   friend std::ostream &operator<<(std::ostream &out, const MDSAuthCaps &cap);

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 40d9fd8db3f7ce2cf14dcc38f9abb1cf7c106111..6aeb2c1aede9c37014f861ff709be990c44de5af 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -2147,6 +2147,7 @@ bool Server::check_access(MDRequestRef& mdr, CInode *in, unsigned mask)
       in, mask,
       mdr->client_request->get_caller_uid(),
       mdr->client_request->get_caller_gid(),
+      &mdr->client_request->get_caller_gid_list(),
       mdr->client_request->head.args.setattr.uid,
       mdr->client_request->head.args.setattr.gid);
     if (r < 0) {

diff --git a/src/mds/SessionMap.cc b/src/mds/SessionMap.cc
index 4cf3177473482228235c669dbf9508738a1d3818..8f3b3a643752b05116873a1bd67a54a72180dd14 100644 (file)
--- a/src/mds/SessionMap.cc
+++ b/src/mds/SessionMap.cc
@@ -862,6 +862,7 @@ void Session::decode(bufferlist::iterator &p)
 
 int Session::check_access(CInode *in, unsigned mask,
                          int caller_uid, int caller_gid,
+                         const vector<uint64_t> *caller_gid_list,
                          int new_uid, int new_gid)
 {
   string path;
@@ -887,7 +888,7 @@ int Session::check_access(CInode *in, unsigned mask,
   }
 
   if (!auth_caps.is_capable(path, in->inode.uid, in->inode.gid, in->inode.mode,
-                           caller_uid, caller_gid, mask,
+                           caller_uid, caller_gid, caller_gid_list, mask,
                            new_uid, new_gid)) {
     return -EACCES;
   }

diff --git a/src/mds/SessionMap.h b/src/mds/SessionMap.h
index ec28e4483bf9e1d6bbdb0d1144fe73fa9ea216e3..f3de92ab79d697dc38c785cd5b52de1be5fe3bc9 100644 (file)
--- a/src/mds/SessionMap.h
+++ b/src/mds/SessionMap.h
@@ -312,7 +312,7 @@ public:
   }
 
   int check_access(CInode *in, unsigned mask, int caller_uid, int caller_gid,
-                  int new_uid, int new_gid);
+                  const vector<uint64_t> *gid_list, int new_uid, int new_gid);
 
 
   Session() : 

diff --git a/src/test/mds/TestMDSAuthCaps.cc b/src/test/mds/TestMDSAuthCaps.cc
index 919095d9d2fb927c3aa410047691f96ac1bdee7c..004763e437884a79957bc9af4a19202118f61740 100644 (file)
--- a/src/test/mds/TestMDSAuthCaps.cc
+++ b/src/test/mds/TestMDSAuthCaps.cc
@@ -114,7 +114,7 @@ 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, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo/bar", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
 }
 
 TEST(MDSAuthCaps, AllowUid) {
@@ -123,75 +123,75 @@ TEST(MDSAuthCaps, AllowUid) {
   ASSERT_FALSE(cap.allow_all());
 
   // uid/gid must be valid
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 10, 0, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 9, 10, MAY_READ, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 10, 10, MAY_READ, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 12, 12, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 10, 12, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 10, 0, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 9, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 12, 12, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 10, 12, NULL, MAY_READ, 0, 0));
 
   // user
-  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0500, 10, 11, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0500, 10, 11, MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0500, 10, 11, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0700, 10, 11, MAY_READ, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0700, 10, 11, MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0700, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 10, 0, 0700, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 12, 0, 0700, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 12, 0, 0700, 12, 12, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0700, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0500, 10, 11, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0500, 10, 11, NULL, MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0500, 10, 11, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0700, 10, 11, NULL, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0700, 10, 11, NULL, MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0700, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 0, 0700, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 12, 0, 0700, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 12, 0, 0700, 12, 12, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0700, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
 
   // group
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0750, 10, 10, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0750, 10, 10, MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0770, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0770, 10, 11, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 11, 0770, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 11, 0770, 10, 11, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 12, 0770, 12, 12, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0770, 12, 12, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 12, 0770, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0750, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0750, 10, 10, NULL, MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0770, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 10, 0770, 10, 11, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 11, 0770, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 11, 0770, 10, 11, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 12, 0770, 12, 12, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0770, 12, 12, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 12, 0770, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
 
   // user > group
-  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0570, 10, 10, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0570, 10, 10, MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 10, 10, 0570, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 10, 10, 0570, 10, 10, NULL, MAY_WRITE, 0, 0));
 
   // other
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0775, 10, 10, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0770, 10, 10, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0775, 10, 10, MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0775, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 10, 10, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0773, 10, 10, MAY_READ, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0775, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0770, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0775, 10, 10, NULL, MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0775, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0777, 10, 10, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0773, 10, 10, NULL, MAY_READ, 0, 0));
 
   // group > other
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0557, 10, 10, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0557, 10, 10, MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0557, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 10, 0557, 10, 10, NULL, MAY_WRITE, 0, 0));
 
   // user > other
-  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0557, 10, 10, MAY_READ, 0, 0));
-  ASSERT_FALSE(cap.is_capable("foo", 10, 0, 0557, 10, 10, MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("foo", 0, 0, 0557, 10, 10, NULL, MAY_READ, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 10, 0, 0557, 10, 10, NULL, 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, 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));
+  ASSERT_TRUE(cap.is_capable("sandbox/foo", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(cap.is_capable("sandbox", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("sandboxed", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(cap.is_capable("foo", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
 }
 
 TEST(MDSAuthCaps, AllowPathChars) {
   MDSAuthCaps unquo_cap;
   ASSERT_TRUE(unquo_cap.parse(g_ceph_context, "allow * path=/sandbox-._foo", NULL));
   ASSERT_FALSE(unquo_cap.allow_all());
-  ASSERT_TRUE(unquo_cap.is_capable("sandbox-._foo/foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(unquo_cap.is_capable("sandbox", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(unquo_cap.is_capable("sandbox-._food", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(unquo_cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(unquo_cap.is_capable("sandbox-._foo/foo", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(unquo_cap.is_capable("sandbox", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(unquo_cap.is_capable("sandbox-._food", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(unquo_cap.is_capable("foo", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
 }
 
 
@@ -199,10 +199,10 @@ TEST(MDSAuthCaps, AllowPathCharsQuoted) {
   MDSAuthCaps quo_cap;
   ASSERT_TRUE(quo_cap.parse(g_ceph_context, "allow * path=\"/sandbox-._foo\"", NULL));
   ASSERT_FALSE(quo_cap.allow_all());
-  ASSERT_TRUE(quo_cap.is_capable("sandbox-._foo/foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(quo_cap.is_capable("sandbox", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(quo_cap.is_capable("sandbox-._food", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
-  ASSERT_FALSE(quo_cap.is_capable("foo", 0, 0, 0777, 0, 0, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_TRUE(quo_cap.is_capable("sandbox-._foo/foo", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(quo_cap.is_capable("sandbox", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(quo_cap.is_capable("sandbox-._food", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
+  ASSERT_FALSE(quo_cap.is_capable("foo", 0, 0, 0777, 0, 0, NULL, MAY_READ | MAY_WRITE, 0, 0));
 }
 
 TEST(MDSAuthCaps, OutputParsed) {