From: Ilya Dryomov <idryomov@gmail.com>
Date: Fri, 24 Jan 2025 19:47:11 +0000 (+0100)
Subject: mon/OSDMonitor: relax cap enforcement for unmanaged snapshots
X-Git-Tag: testing/wip-pdonnell-testing-20250130.221602-debug~7^2~1
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=5f3815e80028066fb3d6ae9b68d6b69045ab2bdf;p=ceph-ci.git

mon/OSDMonitor: relax cap enforcement for unmanaged snapshots

Since commit 4972e054b32c ("mon/OSDMonitor: enforce caps when
creating/deleting unmanaged snapshots"), a) write access to the MON
service, b) write access to the OSD service for a pool or c) permission
for "osd pool op unmanaged-snap" command for a pool is required.  For
"profile rbd" we configure read-only access to the MON service and rely
on write access to the OSD service, however the corresponding check in
is_osd_writable() is too strict.

A OSD cap like "profile rbd namespace=myns" or "allow w namespace=myns"
allows write access to myns namespace of any pool, but is_osd_writable()
disallows operations with unmanaged snapshots with such a cap because
its match.pool_namespace.pool_name.empty() is true.  This condition
appears to serve as the "doesn't include support for the application
tag" guard, but it should actually be match.pool_tag.is_match_all()
(or match.pool_tag.application.empty() if open-coded) -- no restriction
on the pool name doesn't automatically mean that there is a restriction
on the application tag.

Fixes: https://tracker.ceph.com/issues/69679
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/qa/workunits/rbd/permissions.sh b/qa/workunits/rbd/permissions.sh
index f8a9aaa7128..dfa33bf45f7 100755
--- a/qa/workunits/rbd/permissions.sh
+++ b/qa/workunits/rbd/permissions.sh
@@ -49,8 +49,13 @@ create_users() {
     ceph auth get-or-create client.snap_none mon 'allow r' >> $KEYRING
     ceph auth get-or-create client.snap_all mon 'allow r' osd 'allow w' >> $KEYRING
     ceph auth get-or-create client.snap_pool mon 'allow r' osd 'allow w pool=images' >> $KEYRING
+    ceph auth get-or-create client.snap_pool_namespace mon 'allow r' osd 'allow w pool=images namespace=foo' >> $KEYRING
+    ceph auth get-or-create client.snap_namespace mon 'allow r' osd 'allow w namespace=foo' >> $KEYRING
+    ceph auth get-or-create client.snap_tag mon 'allow r' osd 'allow w tag fooapp *=*' >> $KEYRING
     ceph auth get-or-create client.snap_profile_all mon 'allow r' osd 'profile rbd' >> $KEYRING
     ceph auth get-or-create client.snap_profile_pool mon 'allow r' osd 'profile rbd pool=images' >> $KEYRING
+    ceph auth get-or-create client.snap_profile_pool_namespace mon 'allow r' osd 'profile rbd pool=images namespace=foo' >> $KEYRING
+    ceph auth get-or-create client.snap_profile_namespace mon 'allow r' osd 'profile rbd namespace=foo' >> $KEYRING
 
     ceph auth get-or-create client.mon_write mon 'allow *' >> $KEYRING
 }
@@ -208,12 +213,27 @@ test_remove_self_managed_snapshots() {
     create_self_managed_snapshot snap_pool images
     expect 1 create_self_managed_snapshot snap_pool volumes
 
+    create_self_managed_snapshot snap_pool_namespace images
+    expect 1 create_self_managed_snapshot snap_pool_namespace volumes
+
+    create_self_managed_snapshot snap_namespace images
+    create_self_managed_snapshot snap_namespace volumes
+
+    expect 1 create_self_managed_snapshot snap_tag images
+    expect 1 create_self_managed_snapshot snap_tag volumes
+
     create_self_managed_snapshot snap_profile_all images
     create_self_managed_snapshot snap_profile_all volumes
 
     create_self_managed_snapshot snap_profile_pool images
     expect 1 create_self_managed_snapshot snap_profile_pool volumes
 
+    create_self_managed_snapshot snap_profile_pool_namespace images
+    expect 1 create_self_managed_snapshot snap_profile_pool_namespace volumes
+
+    create_self_managed_snapshot snap_profile_namespace images
+    create_self_managed_snapshot snap_profile_namespace volumes
+
     # Ensure users cannot delete self-managed snapshots w/o permissions
     expect 1 remove_self_managed_snapshot snap_none images
     expect 1 remove_self_managed_snapshot snap_none volumes
@@ -224,11 +244,26 @@ test_remove_self_managed_snapshots() {
     remove_self_managed_snapshot snap_pool images
     expect 1 remove_self_managed_snapshot snap_pool volumes
 
+    remove_self_managed_snapshot snap_pool_namespace images
+    expect 1 remove_self_managed_snapshot snap_pool_namespace volumes
+
+    remove_self_managed_snapshot snap_namespace images
+    remove_self_managed_snapshot snap_namespace volumes
+
+    expect 1 remove_self_managed_snapshot snap_tag images
+    expect 1 remove_self_managed_snapshot snap_tag volumes
+
     remove_self_managed_snapshot snap_profile_all images
     remove_self_managed_snapshot snap_profile_all volumes
 
     remove_self_managed_snapshot snap_profile_pool images
     expect 1 remove_self_managed_snapshot snap_profile_pool volumes
+
+    remove_self_managed_snapshot snap_profile_pool_namespace images
+    expect 1 remove_self_managed_snapshot snap_profile_pool_namespace volumes
+
+    remove_self_managed_snapshot snap_profile_namespace images
+    remove_self_managed_snapshot snap_profile_namespace volumes
 }
 
 test_rbd_support() {
diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 69be79b3a8f..a97f81c36b5 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -270,10 +270,14 @@ bool is_osd_writable(const OSDCapGrant& grant, const std::string* pool_name) {
     auto& match = grant.match;
     if (match.is_match_all()) {
       return true;
-    } else if (pool_name != nullptr &&
-               !match.pool_namespace.pool_name.empty() &&
-               match.pool_namespace.pool_name == *pool_name) {
-      return true;
+    } else if (pool_name != nullptr) {
+      if (!match.pool_namespace.pool_name.empty()) {
+        if (match.pool_namespace.pool_name == *pool_name) {
+          return true;
+        }
+      } else if (match.pool_tag.is_match_all()) {
+        return true;
+      }
     }
   }
   return false;