Merge pull request #13628 from jcsp/wip-19075

mds: extend 'p' auth cap to cover all vxattr stuff

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

PendingReleaseNotes

@@ -16,6 +16,11 @@
   object_list_begin (C++) API is only usable on clusters with the
   SORTBITWISE flag enabled (Jewel and later).  (Note that this flag is
   required to be set before upgrading beyond Jewel.)
+* CephFS clients without the 'p' flag in their authentication capability
+  string will no longer be able to set quotas or any layout fields.  This
+  flag previously only restricted modification of the pool and namespace
+  fields in layouts.
+
 
 12.0.0
 ------

doc/cephfs/client-auth.rst

@@ -100,14 +100,16 @@ You may also restrict clients from writing data by using 'r' instead of
 to update filesystem metadata for these files, but it will prevent them
 from persistently writing data in a way that would be visible to other clients.
 
-Layout modification restriction
-===============================
+Layout and Quota restriction (the 'p' flag)
+===========================================
 
-To prevent clients from modifying the data pool used for files or
-directories, use the 'p' modifier in MDS authentication capabilities.
+To set layouts or quotas, clients require the 'p' flag in addition to 'rw'.
+This restricts all the attributes that are set by special extended attributes
+with a "ceph." prefix, as well as restricting other means of setting
+these fields (such as openc operations with layouts).
 
-For example, in the following snippet client.0 can modify the pool used
-for files, but client.1 cannot.
+For example, in the following snippet client.0 can modify layouts and quotas, 
+but client.1 cannot.
 
 ::
 

src/mds/MDSAuthCaps.cc

@@ -215,8 +215,8 @@ bool MDSAuthCaps::is_capable(const std::string &inode_path,
 
 
       // Spec is non-allowing if caller asked for set pool but spec forbids it
-      if (mask & MAY_SET_POOL) {
-        if (!i->spec.allows_set_pool()) {
+      if (mask & MAY_SET_VXATTR) {
+        if (!i->spec.allows_set_vxattr()) {
           continue;
         }
       }

src/mds/MDSAuthCaps.h

@@ -29,7 +29,7 @@ enum {
   MAY_EXECUTE = 4,
   MAY_CHOWN = 16,
   MAY_CHGRP = 32,
-  MAY_SET_POOL = 64,
+  MAY_SET_VXATTR = 64,
 };
 
 class CephContext;
@@ -38,12 +38,12 @@ class CephContext;
 struct MDSCapSpec {
   bool read, write, any;
 
-  // True if the capability permits modifying the pool on file layouts
-  bool layout_pool;
+  // True if the capability permits setting vxattrs (layout, quota, etc)
+  bool set_vxattr;
 
-  MDSCapSpec() : read(false), write(false), any(false), layout_pool(false) {}
+  MDSCapSpec() : read(false), write(false), any(false), set_vxattr(false) {}
   MDSCapSpec(bool r, bool w, bool a, bool lop)
-    : read(r), write(w), any(a), layout_pool(lop) {}
+    : read(r), write(w), any(a), set_vxattr(lop) {}
 
   bool allow_all() const {
     return any;
@@ -59,8 +59,8 @@ struct MDSCapSpec {
     return true;
   }
 
-  bool allows_set_pool() const {
-    return layout_pool;
+  bool allows_set_vxattr() const {
+    return set_vxattr;
   }
 };
 

src/mds/Server.cc

@@ -3206,6 +3206,8 @@ void Server::handle_client_openc(MDRequestRef& mdr)
   // What kind of client caps are required to complete this operation
   uint64_t access = MAY_WRITE;
 
+  const auto default_layout = layout;
+
   // fill in any special params from client
   if (req->head.args.open.stripe_unit)
     layout.stripe_unit = req->head.args.open.stripe_unit;
@@ -3217,23 +3219,20 @@ void Server::handle_client_openc(MDRequestRef& mdr)
       (__s32)req->head.args.open.pool >= 0) {
     layout.pool_id = req->head.args.open.pool;
 
-    // If client doesn't have capability to modify layout pools, then
-    // only permit this request if the requested pool matches what the
-    // file would have inherited anyway from its parent.
-    CDir *parent = dn->get_dir();
-    CInode *parent_in = parent->get_inode();
-    if (layout.pool_id != parent_in->inode.layout.pool_id
-        || layout.pool_ns != parent_in->inode.layout.pool_ns) {
-      access |= MAY_SET_POOL;
-    }
-
     // make sure we have as new a map as the client
     if (req->get_mdsmap_epoch() > mds->mdsmap->get_epoch()) {
       mds->wait_for_mdsmap(req->get_mdsmap_epoch(), new C_MDS_RetryRequest(mdcache, mdr));
       return;
     }
   }
 
+  // If client doesn't have capability to modify layout pools, then
+  // only permit this request if the requested pool matches what the
+  // file would have inherited anyway from its parent.
+  if (default_layout != layout) {
+    access |= MAY_SET_VXATTR;
+  }
+
   if (!layout.is_valid()) {
     dout(10) << " invalid initial file layout" << dendl;
     respond_to_request(mdr, -EINVAL);
@@ -3983,7 +3982,7 @@ void Server::handle_client_setlayout(MDRequestRef& mdr)
   // validate layout
   file_layout_t layout = cur->get_projected_inode()->layout;
   // save existing layout for later
-  int64_t old_pool = layout.pool_id;
+  const auto old_layout = layout;
 
   int access = MAY_WRITE;
 
@@ -3996,16 +3995,18 @@ void Server::handle_client_setlayout(MDRequestRef& mdr)
   if (req->head.args.setlayout.layout.fl_pg_pool > 0) {
     layout.pool_id = req->head.args.setlayout.layout.fl_pg_pool;
 
-    if (layout.pool_id != old_pool) {
-      access |= MAY_SET_POOL;
-    }
-
     // make sure we have as new a map as the client
     if (req->get_mdsmap_epoch() > mds->mdsmap->get_epoch()) {
       mds->wait_for_mdsmap(req->get_mdsmap_epoch(), new C_MDS_RetryRequest(mdcache, mdr));
       return;
     }
   }
+
+  // Don't permit layout modifications without 'p' caps
+  if (layout != old_layout) {
+    access |= MAY_SET_VXATTR;
+  }
+
   if (!layout.is_valid()) {
     dout(10) << "bad layout" << dendl;
     respond_to_request(mdr, -EINVAL);
@@ -4028,7 +4029,7 @@ void Server::handle_client_setlayout(MDRequestRef& mdr)
   inode_t *pi = cur->project_inode();
   pi->layout = layout;
   // add the old pool to the inode
-  pi->add_old_pool(old_pool);
+  pi->add_old_pool(old_layout.pool_id);
   pi->version = cur->pre_dirty();
   pi->ctime = mdr->get_op_stamp();
   pi->change_attr++;
@@ -4079,23 +4080,27 @@ void Server::handle_client_setdirlayout(MDRequestRef& mdr)
   // Level of access required to complete
   int access = MAY_WRITE;
 
+  const auto old_layout = layout;
+
   if (req->head.args.setlayout.layout.fl_object_size > 0)
     layout.object_size = req->head.args.setlayout.layout.fl_object_size;
   if (req->head.args.setlayout.layout.fl_stripe_unit > 0)
     layout.stripe_unit = req->head.args.setlayout.layout.fl_stripe_unit;
   if (req->head.args.setlayout.layout.fl_stripe_count > 0)
     layout.stripe_count=req->head.args.setlayout.layout.fl_stripe_count;
   if (req->head.args.setlayout.layout.fl_pg_pool > 0) {
-    if (req->head.args.setlayout.layout.fl_pg_pool != layout.pool_id) {
-      access |= MAY_SET_POOL;
-    }
     layout.pool_id = req->head.args.setlayout.layout.fl_pg_pool;
     // make sure we have as new a map as the client
     if (req->get_mdsmap_epoch() > mds->mdsmap->get_epoch()) {
       mds->wait_for_mdsmap(req->get_mdsmap_epoch(), new C_MDS_RetryRequest(mdcache, mdr));
       return;
     }  
   }
+
+  if (layout != old_layout) {
+    access |= MAY_SET_VXATTR;
+  }
+
   if (!layout.is_valid()) {
     dout(10) << "bad layout" << dendl;
     respond_to_request(mdr, -EINVAL);
@@ -4325,6 +4330,10 @@ void Server::handle_set_vxattr(MDRequestRef& mdr, CInode *cur,
   inode_t *pi = NULL;
   string rest;
 
+  if (!check_access(mdr, cur, MAY_SET_VXATTR)) {
+    return;
+  }
+
   if (name.compare(0, 15, "ceph.dir.layout") == 0) {
     if (!cur->is_dir()) {
       respond_to_request(mdr, -EINVAL);
@@ -4347,13 +4356,6 @@ void Server::handle_set_vxattr(MDRequestRef& mdr, CInode *cur,
     if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks))
       return;
 
-    if (cur->inode.layout.pool_id != layout.pool_id
-        || cur->inode.layout.pool_ns != layout.pool_ns) {
-      if (!check_access(mdr, cur, MAY_SET_POOL)) {
-        return;
-      }
-    }
-
     pi = cur->project_inode();
     pi->layout = layout;
   } else if (name.compare(0, 16, "ceph.file.layout") == 0) {
@@ -4375,13 +4377,6 @@ void Server::handle_set_vxattr(MDRequestRef& mdr, CInode *cur,
     if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks))
       return;
 
-    if (cur->inode.layout.pool_id != layout.pool_id
-        || cur->inode.layout.pool_ns != layout.pool_ns) {
-      if (!check_access(mdr, cur, MAY_SET_POOL)) {
-        return;
-      }
-    }
-
     pi = cur->project_inode();
     int64_t old_pool = pi->layout.pool_id;
     pi->add_old_pool(old_pool);