On Thu, Jun 15, 2023 at 2:54 PM Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@xxxxxxxxxxxxx> wrote: > > On Thu, Jun 15, 2023 at 2:29 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > > [...] > > > > > > > > > > > > I thought about this too and came to the same conclusion, that > > UID/GID > > > > > based > > > > > restriction can be applied dynamically, so detecting it on mount-time > > > > > helps not so much. > > > > > > > > > For this you please raise one PR to ceph first to support this, and in > > > > the PR we can discuss more for the MDS auth caps. And after the PR > > > > getting merged then in this patch series you need to check the > > > > corresponding option or flag to determine whether could the idmap > > > > mounting succeed. > > > > > > I'm sorry but I don't understand what we want to support here. Do we > > want to > > > add some new ceph request that allows to check if UID/GID-based > > > permissions are applied for > > > a particular ceph client user? > > > > IMO we should prevent users to set UID/GID-based MDS auth caps from ceph > > side. And users should know what has happened. > > ok, we want to restrict setting of UID/GID-based permissions if there is an > idmapped mount on the client. IMHO, idmapping mounts is truly a > client-side feature > and server modification looks a bit strange to me. > > > > > Once users want to support the idmap mounts they should know that the > > MDS auth caps won't work anymore. > > They will work, but permission rule configuration should include > non-mapped UID/GID-s. > As I mentioned here [1] it's already the case even without mount idmappings. > > It would be great to discuss this thing as a concept and synchronize > our understanding of this > before going into modification of a server side. Hi everyone, I've spent some extra time analyzing this issue with UID/GID-based path restriction feature and idmapped mounts one more time and am still fully sure that we have two ways here: I. Extend Cephfs protocol (add new fields to request arguments in the "union ceph_mds_request_args") There should be 2 new fields for the file/directory owner's UID and GID respectively. With the help of these new fields, we will be able to split the permission check logic (that is based on the caller's UID/GID and should not be affected by the mounts idmapping at all!) and file owner concept, which involves mounts' idmapping. II. ignore this issue as non-critical, because: - idmapped mounts can be created only by privileged users (CAP_SYS_ADMIN in the superblock owner's user namespace (currently, it's always the initial user namespace!)) - the surface of the problem is really small (combination of idmapped mount + UID/GID path-based restriction) - problem *can* be workarounded by appropriate permission configuration (UID/GID permissions should be configured to include both the mount's idmapping UIDs/GIDs and the host ones). Before that I've highlighted some existing problems of this UID/GID path-based restriction feature: - [kernel client] UID/GIDs are sent to the server always from the initial user namespace (while the caller can be from inside the container with a non-initial user namespace) - [fuse client] UID/GIDs are always mapped to the fuse mount's superblock user namespace (https://github.com/ceph/ceph-client/blob/409e873ea3c1fd3079909718bbeb06ac1ec7f38b/fs/fuse/dev.c#L138) It means that we already have analogical inconsistency between clients (userspace one and kernel). - [kernel client] We always take current user credentials instead of using (struct file)->f_cred as it has usually done for other filesystems Please understand me in the right way, I'm not trying to say that we need to be lazy and ignore the issue at all, but I'm just trying to say that this issue is not local and is not caused by an idmapped mounts, but it there is something to do on the cephfs side, we need to extend protocol and it's not obvious that it is worth it. My understanding is that we need to clarify this limitation in cephfs kernel client documentation and explain how to configure UID/GID path-based permissions with idmapped mounts to work around this. And if we get requests from our users that this is interesting to someone to support it in the right way then we can do all of this crazy stuff by extending ceph protocol. Btw, I've checked when "union ceph_mds_request_args" was extended last time. It was 6 (!) years ago :) Kind regards, Alex > > [1] https://lore.kernel.org/lkml/CAEivzxcBBJV6DOGzy5S7=TUjrXZfVaGaJX5z7WFzYq1w4MdtiA@xxxxxxxxxxxxxx/ > > Kind regards, > Alex > > > > > Thanks > > > > - Xiubo > >
