client: refactor code that gets group list

define a new class UserGroups. The class is used to check if a gid
is in user's group list.

Signed-off-by: Yan, Zheng <zyan@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index bfc2bb509143f434712cfe9e546f400a98e9b344..5324c4881ab629cd93d6508530a168f06e48f5eb 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -4773,56 +4773,64 @@ void Client::handle_cap_grant(MetaSession *session, Inode *in, Cap *cap, MClient
   m->put();
 }
 
-int Client::check_permissions(Inode *in, int flags, int uid, int gid)
+int Client::_getgrouplist(gid_t** sgids, int uid, int gid)
 {
-  gid_t *sgids = NULL;
-  int sgid_count = 0;
+  int sgid_count;
+  gid_t *sgid_buf;
+
   if (getgroups_cb) {
-    sgid_count = getgroups_cb(callback_handle, uid, &sgids);
-    if (sgid_count < 0) {
-      ldout(cct, 3) << "getgroups failed!" << dendl;
+    sgid_count = getgroups_cb(callback_handle, uid, &sgid_buf);
+    if (sgid_count > 0) {
+      *sgids = sgid_buf;
       return sgid_count;
     }
   }
+
 #if HAVE_GETGROUPLIST
-  else {
-    //use PAM to get the group list
-    // initial number of group entries, defaults to posix standard of 16
-    // PAM implementations may provide more than 16 groups....
-    sgid_count = 16;
-    sgids = (gid_t*)malloc(sgid_count * sizeof(gid_t));
-    if (sgids == NULL) {
-      ldout(cct, 3) << "allocating group memory failed" << dendl;
-      return -EACCES;
-    }
-    struct passwd *pw;
-    pw = getpwuid(uid);
-    if (pw == NULL) {
-      ldout(cct, 3) << "getting user entry failed" << dendl;
-      free(sgids); 
-      return -EACCES;
-    }
-    while (1) {
+  struct passwd *pw;
+  pw = getpwuid(uid);
+  if (pw == NULL) {
+    ldout(cct, 3) << "getting user entry failed" << dendl;
+    return -errno;
+  }
+  //use PAM to get the group list
+  // initial number of group entries, defaults to posix standard of 16
+  // PAM implementations may provide more than 16 groups....
+  sgid_count = 16;
+  sgid_buf = (gid_t*)malloc(sgid_count * sizeof(gid_t));
+  if (sgid_buf == NULL) {
+    ldout(cct, 3) << "allocating group memory failed" << dendl;
+    return -ENOMEM;
+  }
+
+  while (1) {
 #if defined(__APPLE__)
-      if (getgrouplist(pw->pw_name, gid, (int *)sgids, &sgid_count) == -1) {
+    if (getgrouplist(pw->pw_name, gid, (int*)sgid_buf, &sgid_count) == -1) {
 #else
-      if (getgrouplist(pw->pw_name, gid, sgids, &sgid_count) == -1) {
+    if (getgrouplist(pw->pw_name, gid, sgid_buf, &sgid_count) == -1) {
 #endif
-        // we need to resize the group list and try again
-       void *_realloc = NULL;
-        if ((_realloc = realloc(sgids, sgid_count * sizeof(gid_t))) == NULL) {
-          ldout(cct, 3) << "allocating group memory failed" << dendl;
-         free(sgids);
-          return -EACCES;
-        }
-       sgids = (gid_t*)_realloc;
-        continue;
+      // we need to resize the group list and try again
+      void *_realloc = NULL;
+      if ((_realloc = realloc(sgid_buf, sgid_count * sizeof(gid_t))) == NULL) {
+       ldout(cct, 3) << "allocating group memory failed" << dendl;
+       free(sgid_buf);
+       return -ENOMEM;
       }
-      // list was successfully retrieved
-      break;
-    }    
+      sgid_buf = (gid_t*)_realloc;
+      continue;
+    }
+    // list was successfully retrieved
+    break;
   }
+  *sgids = sgid_buf;
+  return sgid_count;
+#else
+  return 0;
 #endif
+}
+
+int Client::check_permissions(Inode *in, int flags, int uid, int gid)
+{
   unsigned want = 0;
   if ((flags & O_ACCMODE) == O_WRONLY)
     want = MAY_WRITE;
@@ -4831,18 +4839,18 @@ int Client::check_permissions(Inode *in, int flags, int uid, int gid)
   else if ((flags & O_ACCMODE) == O_RDONLY)
     want = MAY_READ;
 
-  int ret = _posix_acl_permission(in, uid, gid, sgids, sgid_count, want);
+  RequestUserGroups groups(this, uid, gid);
+
+  int ret = _posix_acl_permission(in, uid, groups, want);
   if (ret != -EAGAIN)
     return ret;
 
   // check permissions before doing anything else
-  if (uid == 0 || in->check_mode(uid, gid, sgids, sgid_count, want))
+  if (uid == 0 || in->check_mode(uid, groups, want))
     ret = 0;
   else
     ret = -EACCES;
 
-  if (sgids)
-    free(sgids);
   return ret;
 }
 
@@ -11577,12 +11585,11 @@ int Client::check_pool_perm(Inode *in, int need)
   return 0;
 }
 
-int Client::_posix_acl_permission(Inode *in, int uid, int gid,
-                                 gid_t *sgids, int sgid_count, unsigned want)
+int Client::_posix_acl_permission(Inode *in, uid_t uid, UserGroups& groups, unsigned want)
 {
   if (acl_type == POSIX_ACL) {
     int r = _getattr(in, CEPH_STAT_CAP_XATTR | CEPH_STAT_CAP_MODE,
-                    uid, gid, in->xattr_version == 0);
+                    uid, groups.get_gid(), in->xattr_version == 0);
     if (r < 0)
       return r;
 
@@ -11590,7 +11597,7 @@ int Client::_posix_acl_permission(Inode *in, int uid, int gid,
       const bufferptr& access_acl = in->xattrs[ACL_EA_ACCESS];
       if (in->mode & S_IRWXG)
        return posix_acl_permits(access_acl, in->uid, in->gid,
-                                uid, gid, sgids, sgid_count, want);
+                                uid, groups, want);
     }
   }
   return -EAGAIN;
@@ -11722,6 +11729,32 @@ void Client::handle_conf_change(const struct md_config_t *conf,
   }
 }
 
+bool Client::RequestUserGroups::is_in(gid_t id)
+{
+  if (id == gid)
+    return true;
+  if (sgid_count < 0)
+    init();
+  for (int i = 0; i < sgid_count; ++i) {
+    if (id == sgids[i])
+      return true;
+  }
+  return false;
+}
+
+int Client::RequestUserGroups::get_gids(const gid_t **out)
+{
+  if (sgid_count < 0)
+    init();
+  if (sgid_count > 0) {
+    *out = sgids;
+    return sgid_count;
+  } else {
+    *out = &gid;
+    return 1;
+  }
+}
+
 void intrusive_ptr_add_ref(Inode *in)
 {
   in->get();

diff --git a/src/client/Client.h b/src/client/Client.h
index 4dfd2b4dceb0efa2ea1b77558fa2ed5f7ce11a28..4323922ce0f4b8f864de6c1c3333e61ec2fe598f 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -54,6 +54,7 @@ using std::fstream;
 #include "osdc/ObjectCacher.h"
 
 #include "InodeRef.h"
+#include "UserGroups.h"
 
 class MDSMap;
 class MonClient;
@@ -761,7 +762,28 @@ private:
     MAY_READ = 4,
   };
 
+  class RequestUserGroups : public UserGroups {
+    Client *client;
+    uid_t uid;
+    gid_t gid;
+    int sgid_count;
+    gid_t *sgids;
+    void init() {
+      sgid_count = client->_getgrouplist(&sgids, uid, gid);
+    }
+    public:
+    RequestUserGroups(Client *c, uid_t u, gid_t g) :
+      client(c), uid(u), gid(g), sgid_count(-1), sgids(NULL) {}
+    ~RequestUserGroups() {
+      free(sgids);
+    }
+    gid_t get_gid() { return gid; }
+    bool is_in(gid_t id);
+    int get_gids(const gid_t **out);
+  };
+
   int check_permissions(Inode *in, int flags, int uid, int gid);
+  int _getgrouplist(gid_t **sgids, int uid=-1, int gid=-1);
 
   int check_data_pool_exist(string name, string value, const OSDMap *osdmap);
 
@@ -825,8 +847,7 @@ private:
 
   int _posix_acl_create(Inode *dir, mode_t *mode, bufferlist& xattrs_bl, int uid, int gid);
   int _posix_acl_chmod(Inode *in, mode_t mode, int uid, int gid);
-  int _posix_acl_permission(Inode *in, int uid, int gid,
-                           gid_t *sgids, int sgid_count, unsigned want);
+  int _posix_acl_permission(Inode *in, uid_t uid, UserGroups& groups, unsigned want);
 public:
   int mount(const std::string &mount_root, bool require_mds=false);
   void unmount();

diff --git a/src/client/Inode.cc b/src/client/Inode.cc
index ec9dad1472bfc26138b41128d72cc7e61e25c2ec..14ff341e0d09c58c1bf1a2c66392cf5ac0268101 100644 (file)
--- a/src/client/Inode.cc
+++ b/src/client/Inode.cc
@@ -7,6 +7,7 @@
 #include "Dir.h"
 #include "MetaSession.h"
 #include "ClientSnapRealm.h"
+#include "UserGroups.h"
 
 ostream& operator<<(ostream &out, Inode &in)
 {
@@ -279,22 +280,14 @@ Dir *Inode::open_dir()
   return dir;
 }
 
-bool Inode::check_mode(uid_t ruid, gid_t rgid, gid_t *sgids, int sgids_count, unsigned want)
+bool Inode::check_mode(uid_t ruid, UserGroups& groups, unsigned want)
 {
-  // if uid is owner, owner entry determines access
   if (uid == ruid) {
+    // if uid is owner, owner entry determines access
     want = want << 6;
-  } else if (gid == rgid) {
+  } else if (groups.is_in(gid)) {
     // if a gid or sgid matches the owning group, group entry determines access
     want = want << 3;
-  } else {
-    int i = 0;
-    for (; i < sgids_count; ++i) {
-      if (sgids[i] == gid) {
-       want = want << 3;
-       break;
-      }
-    }
   }
 
   return (mode & want) == want;

diff --git a/src/client/Inode.h b/src/client/Inode.h
index 52e578aaf9e53b5d0328881418f244f8079609db..bd10ad7626c8e0da86176bed9596952c62d4877c 100644 (file)
--- a/src/client/Inode.h
+++ b/src/client/Inode.h
@@ -23,6 +23,7 @@ struct SnapRealm;
 struct Inode;
 class ceph_lock_state_t;
 class MetaRequest;
+class UserGroups;
 
 struct Cap {
   MetaSession *session;
@@ -334,7 +335,7 @@ struct Inode {
     }
   };
 
-  bool check_mode(uid_t uid, gid_t gid, gid_t *sgids, int sgid_count, unsigned want);
+  bool check_mode(uid_t uid, UserGroups& groups, unsigned want);
 
   // CAPS --------
   void get_open_ref(int mode);

diff --git a/src/client/Makefile.am b/src/client/Makefile.am
index 5d9624754a8340aea7b647067dbbb3b1c3dd5de0..31ca60c935dbb3360aa863900261a0fe1aeda177 100644 (file)
--- a/src/client/Makefile.am
+++ b/src/client/Makefile.am
@@ -25,7 +25,8 @@ noinst_HEADERS += \
        client/Trace.h \
        client/ioctl.h \
        client/ObjecterWriteback.h \
-       client/posix_acl.h
+       client/posix_acl.h \
+       client/UserGroups.h
 
 if WITH_FUSE
 libclient_fuse_la_SOURCES = client/fuse_ll.cc

diff --git a/src/client/UserGroups.h b/src/client/UserGroups.h
new file mode 100644 (file)
index 0000000..3a3966f
--- /dev/null
+++ b/src/client/UserGroups.h
@@ -0,0 +1,12 @@
+#ifndef CEPH_CLIENT_USERGROUPS_H
+#define CEPH_CLIENT_USERGROUPS_H
+
+class UserGroups {
+public:
+  virtual bool is_in(gid_t gid) = 0;
+  virtual gid_t get_gid() = 0;
+  virtual int get_gids(const gid_t **gids) = 0;
+  virtual ~UserGroups() {};
+};
+
+#endif

diff --git a/src/client/posix_acl.cc b/src/client/posix_acl.cc
index 36e8562628f19a1706fe8fb402d0c2e484db24d9..50cb85176bc72381d64bc45d43d41ca02c9687d9 100644 (file)
--- a/src/client/posix_acl.cc
+++ b/src/client/posix_acl.cc
@@ -1,6 +1,7 @@
 #include "include/types.h"
 #include <sys/stat.h>
 #include "posix_acl.h"
+#include "UserGroups.h"
 
 int posix_acl_check(const void *xattr, size_t size)
 {
@@ -211,18 +212,8 @@ int posix_acl_access_chmod(bufferptr& acl, mode_t mode)
   return 0;
 }
 
-static int in_grouplist(gid_t gid, gid_t *sgids, int sgid_count)
-{
-  for (int i = 0; i < sgid_count; i++) {
-    if (sgids[i] == gid)
-      return 1;
-  }
-  return 0;
-}
-
 int posix_acl_permits(const bufferptr& acl, uid_t i_uid, gid_t i_gid,
-                     uid_t uid, gid_t gid, gid_t *sgids, int sgid_count,
-                     unsigned want)
+                        uid_t uid, UserGroups& groups, unsigned want)
 {
   if (!posix_acl_check(acl.c_str(), acl.length()))
     return -EIO;
@@ -252,7 +243,7 @@ int posix_acl_permits(const bufferptr& acl, uid_t i_uid, gid_t i_gid,
        /* fall through */
       case ACL_GROUP:
        id = (tag == ACL_GROUP_OBJ) ? i_gid : entry->e_id;
-       if (id == gid || in_grouplist(id, sgids, sgid_count)) {
+       if (groups.is_in(id)) {
          group_found = 1;
          if ((perm & want) == want)
            goto check_mask;

diff --git a/src/client/posix_acl.h b/src/client/posix_acl.h
index 739bb6551b61b90758cad7d0c55e2ceb058c43c0..d9c5cc854bd90599c485788ada9194fd2d466d34 100644 (file)
--- a/src/client/posix_acl.h
+++ b/src/client/posix_acl.h
@@ -24,11 +24,12 @@ typedef struct {
   acl_ea_entry    a_entries[0];
 } acl_ea_header;
 
+class UserGroups;
+
 int posix_acl_check(const void *xattr, size_t size);
 int posix_acl_equiv_mode(const void *xattr, size_t size, mode_t *mode_p);
 int posix_acl_inherit_mode(bufferptr& acl, mode_t *mode_p);
 int posix_acl_access_chmod(bufferptr& acl, mode_t mode);
 int posix_acl_permits(const bufferptr& acl, uid_t i_uid, gid_t i_gid,
-                     uid_t uid, gid_t gid, gid_t *sgids, int sgid_count,
-                     unsigned want);
+                     uid_t uid, UserGroups& groups, unsigned want);
 #endif