mds: allow all types of MDS caps

MDS caps can contain 5 components: name of a CephFS, a path inside
CephFS, a flag for enabling root squashing mechanism, a UID and list of
GIDs. These 5 components result in 31 combinations, so there can be 31
types of MDS caps. Out of these, the current main branch only allows 11
combinations. This restriction is strange and inappropriate. Ideally,
all combinations should be allowed.

This strange restriction must've been created unintentionally by
previous developers while adding FS name and root squash to MDS caps. A
TODO for a allowing a subset of these combination was also left in
codebase:
https://github.com/ceph/ceph/blob/reef/src/mds/MDSAuthCaps.cc#L69

Fixes: https://tracker.ceph.com/issues/59388
Signed-off-by: Rishabh Dave <ridave@redhat.com>

diff --git a/src/mds/MDSAuthCaps.cc b/src/mds/MDSAuthCaps.cc
index b2b1f0ed7bea03a7602e42797fc3c8cd030e1d9a..e428707247569b66fcbb69db79b4db72612524d4 100644 (file)
--- a/src/mds/MDSAuthCaps.cc
+++ b/src/mds/MDSAuthCaps.cc
@@ -54,6 +54,8 @@ struct MDSCapParser : qi::grammar<Iterator, MDSAuthCaps()>
     using qi::_1;
     using qi::_2;
     using qi::_3;
+    using qi::_4;
+    using qi::_5;
     using qi::eps;
     using qi::lit;
 
@@ -66,25 +68,13 @@ struct MDSCapParser : qi::grammar<Iterator, MDSAuthCaps()>
     network_str %= +char_("/.:a-fA-F0-9][");
     fs_name_str %= +char_("a-zA-Z0-9_.-");
 
-    // match := [path=<path>] [uid=<uid> [gids=<gid>[,<gid>...]]
-    // TODO: allow fsname, and root_squash to be specified with uid, and gidlist
-    path %= (spaces >> lit("path") >> lit('=') >> (quoted_path | unquoted_path));
-    uid %= (spaces >> lit("uid") >> lit('=') >> uint_);
+    path %= -(spaces >> lit("path") >> lit('=') >> (quoted_path | unquoted_path));
+    uid %= -(spaces >> lit("uid") >> lit('=') >> uint_);
     uintlist %= (uint_ % lit(','));
     gidlist %= -(spaces >> lit("gids") >> lit('=') >> uintlist);
     fs_name %= -(spaces >> lit("fsname") >> lit('=') >> fs_name_str);
-    root_squash %= (spaces >> lit("root_squash") >> attr(true));
-    match = -(
-             (fs_name >> path >> root_squash)[_val = phoenix::construct<MDSCapMatch>(_2, _1, _3)] |
-            (uid >> gidlist)[_val = phoenix::construct<MDSCapMatch>(_1, _2)] |
-            (path >> uid >> gidlist)[_val = phoenix::construct<MDSCapMatch>(_1, _2, _3)] |
-             (fs_name >> path)[_val = phoenix::construct<MDSCapMatch>(_2, _1)] |
-             (fs_name >> root_squash)[_val = phoenix::construct<MDSCapMatch>(string(), _1, _2)] |
-             (path >> root_squash)[_val = phoenix::construct<MDSCapMatch>(_1, string(), _2)] |
-             (path)[_val = phoenix::construct<MDSCapMatch>(_1)] |
-             (root_squash)[_val = phoenix::construct<MDSCapMatch>(string(), string(), _1)] |
-             (fs_name)[_val = phoenix::construct<MDSCapMatch>(string(),
-                                                             _1)]);
+    root_squash %= -(spaces >> lit("root_squash") >> attr(true));
+    match = (fs_name >> path >> root_squash >> uid >> gidlist)[_val = phoenix::construct<MDSCapMatch>(_1, _2, _3, _4, _5)];
 
     // capspec = * | r[w][f][p][s]
     capspec = spaces >> (

diff --git a/src/mds/MDSAuthCaps.h b/src/mds/MDSAuthCaps.h
index 5fcbb1f2fbbf0d12cc6cd328b9d4e8f091a5b2dc..6cfdf489f9a15db172903b50bad523bcb80a298f 100644 (file)
--- a/src/mds/MDSAuthCaps.h
+++ b/src/mds/MDSAuthCaps.h
@@ -101,30 +101,17 @@ private:
 struct MDSCapMatch {
   static const int64_t MDS_AUTH_UID_ANY = -1;
 
-  MDSCapMatch() : uid(MDS_AUTH_UID_ANY), fs_name(std::string()) {}
+  MDSCapMatch() {}
 
-  MDSCapMatch(int64_t uid_, std::vector<gid_t>& gids_) :
-    uid(uid_), gids(gids_), fs_name(std::string()) {}
+  MDSCapMatch(const std::string& fsname_, const std::string& path_,
+             bool root_squash_, int64_t uid_=MDS_AUTH_UID_ANY,
+             const std::vector<gid_t>& gids_={}) {
+    fs_name = std::move(fsname_);
+    path = std::move(path_);
+    root_squash = root_squash_;
+    uid = (uid_ == 0) ? -1 : uid_;
+    gids = gids_;
 
-  explicit MDSCapMatch(const std::string &path_)
-    : uid(MDS_AUTH_UID_ANY), path(path_), fs_name(std::string()) {
-    normalize_path();
-  }
-
-  explicit MDSCapMatch(std::string path, std::string fs_name) :
-    uid(MDS_AUTH_UID_ANY), path(std::move(path)), fs_name(std::move(fs_name))
-  {
-    normalize_path();
-  }
-
-  explicit MDSCapMatch(std::string path, std::string fs_name, bool root_squash_) :
-    uid(MDS_AUTH_UID_ANY), path(std::move(path)), fs_name(std::move(fs_name)), root_squash(root_squash_)
-  {
-    normalize_path();
-  }
-
-  MDSCapMatch(const std::string& path_, int64_t uid_, std::vector<gid_t>& gids_)
-    : uid(uid_), gids(gids_), path(path_), fs_name(std::string()) {
     normalize_path();
   }
 
@@ -149,7 +136,8 @@ struct MDSCapMatch {
    */
   bool match_path(std::string_view target_path) const;
 
-  int64_t uid;       // Require UID to be equal to this, if !=MDS_AUTH_UID_ANY
+  // Require UID to be equal to this, if !=MDS_AUTH_UID_ANY
+  int64_t uid = MDS_AUTH_UID_ANY;
   std::vector<gid_t> gids;  // Use these GIDs
   std::string path;  // Require path to be child of this (may be "" or "/" for any)
   std::string fs_name;