Project

General

Profile

Bug #18254

path restricted cephx caps not working correctly

Added by Jeff Layton over 7 years ago. Updated over 7 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
% Done:

0%

Source:
Tags:
Backport:
jewel
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
Client
Labels (FS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

Ramana noticed this first while testing my ganesha patches to allow restricting exports. It appears that attempting to restrict a particular cephx user to a subtree of the whole cephfs is not working correctly. To reproduce:

1) Set up a cephfs cluster with cephx enabled (I used vstart).

2) Mount up the share using ceph-fuse and create a directory within it called "/export".

3) Create a user named "alice" and give it wide open permissions first:

$ ./bin/ceph auth add client.alice mon 'allow *' mds 'allow *' osd 'allow rw'

4) take the attached program and build it vs. libcephfs:

$ gcc -Wall -o ./ceph_submount ./ceph_submount.c -lcephfs

5) Run the program. You should see "Mount successful!" output.

6) now, restrict the mds caps for alice:

$ ceph auth caps client.alice mds "allow rw path=/export" mon "allow *" osd "allow rw"

7) run the program again:

$ ./bin/ceph_submount 
mount: -1

That's -EPERM. So either I'm not restricting the caps correctly by path, or something is broken...

ceph_submount.c View (880 Bytes) Jeff Layton, 12/14/2016 07:56 PM

0001-ceph-add-ceph_submount-test-program.patch View - Revised test program (as a patch) (2.62 KB) Jeff Layton, 12/14/2016 09:18 PM


Related issues

Copied to CephFS - Backport #18307: path restricted cephx caps not working correctly Resolved

History

#1 Updated by Jeff Layton over 7 years ago

Oh, and you will need to overwrite the key in the reproducer program with the one for "alice".

#2 Updated by Jeff Layton over 7 years ago

The program logs this in the MDS logs when run. I'm definitely passing in a real path there:

2016-12-14 15:18:13.726226 7f18536c8700 -1 mds.0.server handle_client_session forbidden path claimed as mount root: / by client.4142
2016-12-14 15:18:13.726231 7f18536c8700  1 -- 192.168.1.3:6812/2591989918 --> 192.168.1.3:0/3287633120 -- client_session(reject) v2 -- 0x56155bcea240 con 0
2016-12-14 15:18:13.726255 7f18536c8700  0 log_channel(cluster) log [WRN] : client session with invalid root '/' denied (client.4142 192.168.1.3:0/3287633120)

Mounting with these creds with ceph-fuse works however and you don't see these messages there. I suspect the userland client code is trying to walk down from the root, instead of doing what ceph-fuse does.

#3 Updated by Greg Farnum over 7 years ago

Did you check the client log to see where it's failing out at?
I'd check the code flow from Client::mount() to the MDS' Server::handle_client_session(). Looks to me like the MDS is referencing the client's "root" metadata but the Client is only setting it based on the config value, not ceph_mount()...

#4 Updated by Jeff Layton over 7 years ago

Thanks Greg, I'll take a look at how all of that stuff gets set. FWIW, here's the log with the client debugging cranked up to 20:

$ cat client.alice.25775.log
2016-12-14 15:51:09.877732 7fd117c2ef00 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-12-14 15:51:09.879919 7fd117c2ef00 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-12-14 15:51:09.882589 7fd117c2ef00 20 client.0 populate_metadata read hostname 'tleilax.poochiereds.net'
2016-12-14 15:51:09.882807 7fd0f1ffb700 10 client.0 ms_handle_connect on 192.168.1.3:6789/0
2016-12-14 15:51:09.884355 7fd117c2ef00 10 client.4154 Subscribing to map 'mdsmap'
2016-12-14 15:51:09.884395 7fd117c2ef00 20 client.4154 trim_cache size 0 max 16384
2016-12-14 15:51:09.884435 7fd117c2ef00 10 client.4154 did not get mds through better means, so chose random mds -1
2016-12-14 15:51:09.884452 7fd117c2ef00 20 client.4154 mds is -1
2016-12-14 15:51:09.884456 7fd117c2ef00 10 client.4154  target mds.-1 not active, waiting for new mdsmap
2016-12-14 15:51:09.884704 7fd0f1ffb700  1 client.4154 handle_mds_map epoch 5
2016-12-14 15:51:09.884737 7fd117c2ef00 10 client.4154 did not get mds through better means, so chose random mds 0
2016-12-14 15:51:09.884743 7fd117c2ef00 20 client.4154 mds is 0
2016-12-14 15:51:09.884746 7fd117c2ef00 10 client.4154 _open_mds_session mds.0
2016-12-14 15:51:09.884803 7fd117c2ef00 10 client.4154 waiting for session to mds.0 to open
2016-12-14 15:51:09.885398 7fd0f1ffb700 10 client.4154 ms_handle_connect on 192.168.1.3:6812/2591989918
2016-12-14 15:51:09.885680 7fd0f1ffb700 10 client.4154 handle_client_session client_session(reject) v1 from mds.0
2016-12-14 15:51:09.885712 7fd0f1ffb700 10 client.4154 remove_session_caps mds.0
2016-12-14 15:51:09.885715 7fd0f1ffb700 10 client.4154 kick_requests_closed for mds.0
2016-12-14 15:51:09.885728 7fd117c2ef00  1 client.4154 shutdown
2016-12-14 15:51:09.886459 7fd117c2ef00 20 client.4154 trim_cache size 0 max 0

Looks like it tries to issue a CEPH_SESSION_REQUEST_OPEN to the mds and the mds sent CEPH_SESSION_REJECT back.

#5 Updated by Jeff Layton over 7 years ago

Revised test program here, in patch format so it can build in tree. We should probably roll this up into a regression test once this bug is squashed.

With this program, it succeeds -- so yeah the issue is that the code seems to expect that the client_mountpoint option is set to the base of the export we want to mount (that option has a mighty confusing name, btw).

I'll play with a patch that has ->mount set this option, which seems like the right fix. I'll also need to verify that fixing this correctly won't break pathwalking, or whether pathwalking has been broken all along when this option isn't set by programs using libcephfs that aren't mounting "/".

#6 Updated by Jeff Layton over 7 years ago

The patch turns out to be pretty trivial:

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 5220715dad67..2e0adec83e35 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -5628,8 +5628,10 @@ int Client::mount(const std::string &mount_root, const UserPerm& perms,
   }

   filepath fp(CEPH_INO_ROOT);
-  if (!mount_root.empty())
+  if (!mount_root.empty()) {
+    metadata["root"] = mount_root.c_str();
     fp = filepath(mount_root.c_str());
+  }
   while (true) {
     MetaRequest *req = new MetaRequest(CEPH_MDS_OP_GETATTR);
     req->set_filepath(fp);

This metadata option only has any bearing on how the session gets opened...which seems a little sketchy, TBH. If you did some "guessing" could you craft packets such that you could escape the path to which you should be confined?

I guess is_capable() is what checks for that, so I guess maybe this is all that's needed?

#7 Updated by Jeff Layton over 7 years ago

Patch merged. We'll also want this backported to jewel.

#8 Updated by Jeff Layton over 7 years ago

  • Status changed from New to Pending Backport

#9 Updated by Jeff Layton over 7 years ago

  • Copied to Backport #18307: path restricted cephx caps not working correctly added

#10 Updated by Nathan Cutler over 7 years ago

  • Backport set to jewel

#11 Updated by Nathan Cutler over 7 years ago

@Jeff: We have a system/service in place for backporting bugfixes to our stable releases. Patches backported via this system undergo regression testing before being merged, in addition to the QE testing that takes place before each point release.

This service is also designed to make life easier for developers. Just:

1. make sure the fix has a tracker
2. enter the PR# or commit SHA1 of the master fix in the tracker
3. fill out the Backport field (e.g. "jewel", "jewel,hammer")
4. when the master PR is merged, change status to "Pending Backport"

That's all. Members of the backport team have scripting that automatically generate the backport tracker issues, and members of the backport team also do the cherry-picking, regression testing, etc. They only contact you if there's a problem.

#13 Updated by Nathan Cutler over 7 years ago

  • Status changed from Pending Backport to Resolved

Also available in: Atom PDF