diff mbox series

Two cifsacl related patches to more accurately emulate mode bits

Message ID CAH2r5mufg02Y=xpDSzBZLLDPB3CBzW8yak2tUVWO7cTqU6=6Vg@mail.gmail.com
State New
Headers show
Series Two cifsacl related patches to more accurately emulate mode bits | expand

Commit Message

Steve French Dec. 9, 2020, 8:11 p.m. UTC 
Any thoughts on Shyam's two cifsacl related improvements?

First does:
"With the "cifsacl" mount option, the mode bits set on the file/dir
is converted to corresponding ACEs in DACL. However, only the
ALLOWED ACEs were being set for "owner" and "group" SIDs. Since
owner is a subset of group, and group is a subset of
everyone/world SID, in order to properly emulate unix perm groups,
we need to add DENIED ACEs. If we don't do that, "owner" and "group"
SIDs could get more access rights than they should. Which is what
was happening. This fixes it.
We try to keep the "preferred" order of ACEs, i.e. DENYs followed
by ALLOWs. However, for a small subset of cases we cannot
maintain the preferred order. In that case, we'll end up with the
DENY ACE for group after the ALLOW for the owner.
If owner SID == group SID, use the more restrictive
among the two perm bits and convert them to ACEs.
Also, for reverse mapping, i.e. to convert ACL to unix perm bits,
for the "others" bits, we needed to add the masked bits of the
owner and group masks to others mask."

Second is:
"For the cifsacl mount option, we did not support sticky bits.
With this patch, we do support it, by setting the DELETE_CHILD perm
on the directory only for the owner user. When sticky bit is not
enabled, allow DELETE_CHILD perm for everyone."
diff mbox series

Patch

From d24e661920cbd8c06b552a85a35b72fdb2009a78 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Mon, 9 Nov 2020 06:12:49 -0800
Subject: [PATCH 2/2] cifs: Enable sticky bit with cifsacl mount option.

For the cifsacl mount option, we did not support sticky bits.
With this patch, we do support it, by setting the DELETE_CHILD perm
on the directory only for the owner user. When sticky bit is not
enabled, allow DELETE_CHILD perm for everyone.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
 fs/cifs/cifsacl.c | 41 ++++++++++++++++++++++++++++++-----------
 fs/cifs/cifsacl.h |  4 ++++
 fs/cifs/cifspdu.h |  2 +-
 3 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index c8604d9b9958..d7a6d0f533bf 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -617,6 +617,17 @@  static void access_flags_to_mode(__le32 ace_flags, int type, umode_t *pmode,
 			!(*pdenied & mask & 0111))
 		*pmode |= mask & 0111;
 
+	/* If DELETE_CHILD is set only on an owner ACE, set sticky bit */
+	if (flags & FILE_DELETE_CHILD) {
+		if (mask == ACL_OWNER_MASK) {
+			if (!(*pdenied & 01000))
+				*pmode |= 01000;
+		} else if (!(*pdenied & 01000)) {
+			*pmode &= ~01000;
+			*pdenied |= 01000;
+		}
+	}
+
 	cifs_dbg(NOISY, "access flags 0x%x mode now %04o\n", flags, *pmode);
 	return;
 }
@@ -652,7 +663,9 @@  static void mode_to_access_flags(umode_t mode, umode_t bits_to_use,
 }
 
 static __u16 fill_ace_for_sid(struct cifs_ace *pntace,
-			const struct cifs_sid *psid, __u64 nmode, umode_t bits, __u8 access_type)
+			const struct cifs_sid *psid, __u64 nmode,
+			umode_t bits, __u8 access_type,
+			bool allow_delete_child)
 {
 	int i;
 	__u16 size = 0;
@@ -661,10 +674,15 @@  static __u16 fill_ace_for_sid(struct cifs_ace *pntace,
 	pntace->type = access_type;
 	pntace->flags = 0x0;
 	mode_to_access_flags(nmode, bits, &access_req);
+
+	if (access_type == ACCESS_ALLOWED && allow_delete_child)
+		access_req |= FILE_DELETE_CHILD;
+
 	if (access_type == ACCESS_ALLOWED && !access_req)
 		access_req = SET_MINIMUM_RIGHTS;
 	else if (access_type == ACCESS_DENIED)
 		access_req &= ~SET_MINIMUM_RIGHTS;
+
 	pntace->access_req = cpu_to_le32(access_req);
 
 	pntace->sid.revision = psid->revision;
@@ -717,10 +735,6 @@  static void dump_ace(struct cifs_ace *pace, char *end_of_acl)
 }
 #endif
 
-#define ACL_OWNER_MASK 0700
-#define ACL_GROUP_MASK 0770
-#define ACL_EVERYONE_MASK 0777
-
 static void parse_dacl(struct cifs_acl *pdacl, char *end_of_acl,
 		       struct cifs_sid *pownersid, struct cifs_sid *pgrpsid,
 		       struct cifs_fattr *fattr, bool mode_from_special_sid)
@@ -904,6 +918,7 @@  static int set_chmod_dacl(struct cifs_acl *pndacl, struct cifs_sid *pownersid,
 	__u64 other_mode;
 	__u64 deny_user_mode = 0;
 	__u64 deny_group_mode = 0;
+	bool sticky_set = false;
 
 	pnndacl = (struct cifs_acl *)((char *)pndacl + sizeof(struct cifs_acl));
 
@@ -944,31 +959,35 @@  static int set_chmod_dacl(struct cifs_acl *pndacl, struct cifs_sid *pownersid,
 
 	*pnmode = user_mode | group_mode | other_mode | (nmode & ~0777);
 
+	/* This tells if we should allow delete child for group and everyone. */
+	if (nmode & 01000)
+		sticky_set = true;
+
 	if (deny_user_mode) {
 		size += fill_ace_for_sid((struct cifs_ace *)((char *)pnndacl + size),
-				pownersid, deny_user_mode, 0700, ACCESS_DENIED);
+				pownersid, deny_user_mode, 0700, ACCESS_DENIED, false);
 		num_aces++;
 	}
 	/* Group DENY ACE does not conflict with owner ALLOW ACE. Keep in preferred order*/
 	if (deny_group_mode && !(deny_group_mode & (user_mode >> 3))) {
 		size += fill_ace_for_sid((struct cifs_ace *)((char *)pnndacl + size),
-				pgrpsid, deny_group_mode, 0070, ACCESS_DENIED);
+				pgrpsid, deny_group_mode, 0070, ACCESS_DENIED, false);
 		num_aces++;
 	}
 	size += fill_ace_for_sid((struct cifs_ace *) ((char *)pnndacl + size),
-			pownersid, user_mode, 0700, ACCESS_ALLOWED);
+			pownersid, user_mode, 0700, ACCESS_ALLOWED, true);
 	num_aces++;
 	/* Group DENY ACE conflicts with owner ALLOW ACE. So keep it after. */
 	if (deny_group_mode && (deny_group_mode & (user_mode >> 3))) {
 		size += fill_ace_for_sid((struct cifs_ace *)((char *)pnndacl + size),
-				pgrpsid, deny_group_mode, 0070, ACCESS_DENIED);
+				pgrpsid, deny_group_mode, 0070, ACCESS_DENIED, false);
 		num_aces++;
 	}
 	size += fill_ace_for_sid((struct cifs_ace *)((char *)pnndacl + size),
-			pgrpsid, group_mode, 0070, ACCESS_ALLOWED);
+			pgrpsid, group_mode, 0070, ACCESS_ALLOWED, !sticky_set);
 	num_aces++;
 	size += fill_ace_for_sid((struct cifs_ace *)((char *)pnndacl + size),
-			&sid_everyone, other_mode, 0007, ACCESS_ALLOWED);
+			&sid_everyone, other_mode, 0007, ACCESS_ALLOWED, !sticky_set);
 	num_aces++;
 
 set_size:
diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
index 45665ff87b64..ff7fd0862e28 100644
--- a/fs/cifs/cifsacl.h
+++ b/fs/cifs/cifsacl.h
@@ -30,6 +30,10 @@ 
 #define WRITE_BIT       0x2
 #define EXEC_BIT        0x1
 
+#define ACL_OWNER_MASK 0700
+#define ACL_GROUP_MASK 0770
+#define ACL_EVERYONE_MASK 0777
+
 #define UBITSHIFT	6
 #define GBITSHIFT	3
 
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index 593d826820c3..ce51183ecaf4 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -262,7 +262,7 @@ 
 				| WRITE_OWNER | SYNCHRONIZE)
 #define SET_FILE_WRITE_RIGHTS (FILE_WRITE_DATA | FILE_APPEND_DATA \
 				| FILE_READ_EA | FILE_WRITE_EA \
-				| FILE_DELETE_CHILD | FILE_READ_ATTRIBUTES \
+				| FILE_READ_ATTRIBUTES \
 				| FILE_WRITE_ATTRIBUTES \
 				| DELETE | READ_CONTROL | WRITE_DAC \
 				| WRITE_OWNER | SYNCHRONIZE)
-- 
2.27.0