From: Rishabh Dave <ridave@redhat.com>
Date: Sun, 30 Apr 2023 13:01:00 +0000 (+0530)
Subject: mds: allow all types of MDS caps
X-Git-Tag: v19.0.0~866^2~2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=0c36929f258f2b32ded16bfaf40f99c27b668f44;p=ceph-ci.git

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 b2b1f0ed7be..e4287072475 100644
--- 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 5fcbb1f2fbb..6cfdf489f9a 100644
--- 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;