client: Don't write permission when the file is marked with SUID or SGID

[renhwztetecs]

… SUID or SGID

The problem appears in the ceph-fuse mount,there is no problem with the kernel mount.
we can add the identification of S_ISUID and S_ISGID in may_setattr.

Fixes: https://tracker.ceph.com/issues/45320
Signed-off-by: huanwen ren ren.huanwen@zte.com.cn

Checklist

• References tracker ticket
• Updates documentation if necessary
• Includes tests for new functionality or reproducer for bug

---

Show available Jenkins commands

• jenkins retest this please
• jenkins test classic perf
• jenkins test crimson perf
• jenkins test signed
• jenkins test make check
• jenkins test make check arm64
• jenkins test submodules
• jenkins test dashboard
• jenkins test dashboard backend
• jenkins test docs
• jenkins render docs
• jenkins test ceph-volume all
• jenkins test ceph-volume tox

[ukernel]

this allow all operation for file with setuid/setgid, which is incorrect.
In may_setattr CEPH_SETATTR_MODE backet, we should allow removing SUID/SGID if file write is allowed for uid.
A better fix is set FUSE_HANDLE_KILLPRIV fuse flag, and let libcephfs handle removing suid/sgid on write.

[ukernel]

something like:

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 84fcf91d0d..25a917c704 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -5394,8 +5394,16 @@ int Client::may_setattr(Inode *in, struct ceph_statx *stx, int mask,
   }
 
   if (mask & CEPH_SETATTR_MODE) {
-    if (perms.uid() != 0 && perms.uid() != in->uid)
-      goto out;
+    if (perms.uid() != 0 && perms.uid() != in->uid) {
+      auto to_clear = ~stx->stx_mode & in->mode & 07777;
+      if (to_clear && !(to_clear & ~(S_ISUID | S_ISGID)) && // only remove setuid/setgid
+         !(stx->stx_mode & ~in->mode & 07777) && // not add new bits
+         !inode_permission(in, perms, MAY_WRITE)) { // may write
+       // hack: kill setuid/setguid on write?
+      } else {
+       goto out;
+      }
+    }
 
     gid_t i_gid = (mask & CEPH_SETATTR_GID) ? stx->stx_gid : in->gid;
     if (perms.uid() != 0 && !perms.gid_in_groups(i_gid))

[renhwztetecs]

Thank you
FUSE_HANDLE_KILLPRIV fuse flag is the best way , but only supported in fuse3.0

[renhwztetecs]

！I first try to test it

[renhwztetecs]

The provided method cannot be verified, and the write function will clear out suid and sgid

[ukernel]

why do you check CEPH_CAP_AUTH_EXCL? It's possible that multiple clients open a setuid file for write at the same time.

[renhwztetecs]

This place is wrong, I modified it, please review again.
I use the pjdfstest tool for a test, the compatibility result is correct

[ukernel]

this means that any user can modify mode of file with setuid/setgid. I don't think it's correct

[ukernel]

the newest code still has this issue

[stale]

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

[stale]

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!