chmod

chmod

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 680 bytes --]

Hi folks!

In my testing against with 6.11.5-300.fc41.x86_64 against Samba chmod is 
not working on a posix mount.

I don't see an expected set-sd call with the S-1-5-88-3-mode SID, it 
seems the client is not considering to do this.

mount options (all default beside explicitly requesting posix):

//localhost/posix on /mnt/smb3unix type cifs 
(rw,relatime,vers=3.1.1,cache=strict,username=slow,uid=0,noforceuid,gid=0,noforcegid,addr=127.0.0.1,file_mode=0755,dir_mode=0755,soft,posix,posixpaths,serverino,mapposix,reparse=nfs,rsize=4194304,wsize=4194304,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)

Is this supposed to work?

Thanks!
-slow

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: chmod

[Paulo Alcantara]

Hi Ralph,

Ralph Boehme <slow@samba.org> writes:

> In my testing against with 6.11.5-300.fc41.x86_64 against Samba chmod is 
> not working on a posix mount.
>
> I don't see an expected set-sd call with the S-1-5-88-3-mode SID, it 
> seems the client is not considering to do this.
>
> mount options (all default beside explicitly requesting posix):
>
> //localhost/posix on /mnt/smb3unix type cifs 
> (rw,relatime,vers=3.1.1,cache=strict,username=slow,uid=0,noforceuid,gid=0,noforcegid,addr=127.0.0.1,file_mode=0755,dir_mode=0755,soft,posix,posixpaths,serverino,mapposix,reparse=nfs,rsize=4194304,wsize=4194304,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)
>
> Is this supposed to work?

Yes, but this is broken for a while already.  Samuel reported such
problem at SDC 2023 but nobody fixed it yet.

Re: chmod

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 2041 bytes --]

Hi Paulo,

On 11/13/24 1:51 PM, Paulo Alcantara wrote:
> Ralph Boehme <slow@samba.org> writes:
> 
>> In my testing against with 6.11.5-300.fc41.x86_64 against Samba chmod is
>> not working on a posix mount.
>>
>> I don't see an expected set-sd call with the S-1-5-88-3-mode SID, it
>> seems the client is not considering to do this.
>>
>> mount options (all default beside explicitly requesting posix):
>>
>> //localhost/posix on /mnt/smb3unix type cifs
>> (rw,relatime,vers=3.1.1,cache=strict,username=slow,uid=0,noforceuid,gid=0,noforcegid,addr=127.0.0.1,file_mode=0755,dir_mode=0755,soft,posix,posixpaths,serverino,mapposix,reparse=nfs,rsize=4194304,wsize=4194304,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)
>>
>> Is this supposed to work?
> 
> Yes, but this is broken for a while already.  Samuel reported such
> problem at SDC 2023 but nobody fixed it yet.

ok, I got a bit farther. It seems the client needs the mount option 
modefromsid to use this. Why? It's not even documented in the manpage. 
For a posix mount the behaviour to send a chmod(mode) as 
SMB2-SETINFO(SD, S-1-5-88-3-mode) must be the default.

And then there's another problem. This commit from Ronny

0c6f4ebf8835d01866eb686d47578cde80097981
cifs: modefromsids must add an ACE for authenticated users

breaks this against Samba as Samba requires that this special SD has 
only a single ACE with the magic SID S-1-5-88-3-mode in 
check_smb2_posix_chmod_ace():

         if (psd->dacl->num_aces != 1) {
                 return false;
         }

I'm not sure I fully understand the reasoning in the commit messages, 
but I think a userspace chmod() should be mapped to an ACL with the 
single magic ACE and nothing else. Server should treat these SDs special 
int the way, that they will *only* apply the mode from the SID, the must 
not apply this as an SD (ACL) in the filesystem and hence to make this 
clear we assert psd->dacl->num_aces == 1.

Am I missing anything? Thoughts?

Thanks!
-slow

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: chmod

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 355 bytes --]

On 11/13/24 4:39 PM, Ralph Boehme wrote:
> Am I missing anything? Thoughts?

did some more research on what the option modefromsid actually does and 
I guess the problem is that the behaviour is likely correct for 
modefromsid, it just doesn't work for smb3 posix, so populate_new_aces() 
needs to be tweaked to not include sid_authusers.

-slow

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: chmod

[ronnie sahlberg]

On Thu, 14 Nov 2024 at 05:07, Ralph Boehme <slow@samba.org> wrote:
>
> On 11/13/24 4:39 PM, Ralph Boehme wrote:
> > Am I missing anything? Thoughts?
>
> did some more research on what the option modefromsid actually does and
> I guess the problem is that the behaviour is likely correct for
> modefromsid, it just doesn't work for smb3 posix, so populate_new_aces()
> needs to be tweaked to not include sid_authusers.

Remember, it also need to work for use-cases with normal Windows and
Azure servers where you do NOT
have a multiuser mount (i.e. all client access is using the
credentials from the mount)
and you basically have an inherited ACE to "allow all access to the
mount user" for the whole share.

>
> -slow

Re: chmod

[Jeremy Allison]

On Thu, Nov 14, 2024 at 06:52:14AM +1000, ronnie sahlberg wrote:
>On Thu, 14 Nov 2024 at 05:07, Ralph Boehme <slow@samba.org> wrote:
>>
>> On 11/13/24 4:39 PM, Ralph Boehme wrote:
>> > Am I missing anything? Thoughts?
>>
>> did some more research on what the option modefromsid actually does and
>> I guess the problem is that the behaviour is likely correct for
>> modefromsid, it just doesn't work for smb3 posix, so populate_new_aces()
>> needs to be tweaked to not include sid_authusers.
>
>Remember, it also need to work for use-cases with normal Windows and
>Azure servers where you do NOT
>have a multiuser mount (i.e. all client access is using the
>credentials from the mount)
>and you basically have an inherited ACE to "allow all access to the
>mount user" for the whole share.

This is different from the SMB3-POSIX chmod operation.

I think Ralph is suggesting the modefromsid is left alone,
and a new (separate) code path used for SMB3-POSIX that
sends the one-ACE entry defined in that spec.

Re: chmod

[Ralph Boehme]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On 11/13/24 10:49 PM, Jeremy Allison wrote:
> On Thu, Nov 14, 2024 at 06:52:14AM +1000, ronnie sahlberg wrote:
>> On Thu, 14 Nov 2024 at 05:07, Ralph Boehme <slow@samba.org> wrote:
>>>
>>> On 11/13/24 4:39 PM, Ralph Boehme wrote:
>>> > Am I missing anything? Thoughts?
>>>
>>> did some more research on what the option modefromsid actually does and
>>> I guess the problem is that the behaviour is likely correct for
>>> modefromsid, it just doesn't work for smb3 posix, so populate_new_aces()
>>> needs to be tweaked to not include sid_authusers.
>>
>> Remember, it also need to work for use-cases with normal Windows and
>> Azure servers where you do NOT
>> have a multiuser mount (i.e. all client access is using the
>> credentials from the mount)
>> and you basically have an inherited ACE to "allow all access to the
>> mount user" for the whole share.
> 
> This is different from the SMB3-POSIX chmod operation.
> 
> I think Ralph is suggesting the modefromsid is left alone,
> and a new (separate) code path used for SMB3-POSIX that
> sends the one-ACE entry defined in that spec.

exactly.

I've implemented a quick fix and am currently testing it.

Thanks!
-slow

[-- Attachment #2: chmod-smb3-posix.patch --]
[-- Type: text/x-patch, Size: 4807 bytes --]

From 7b0aa527e693d1339122897c4cdec650d6826147 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 14 Nov 2024 11:05:13 +0100
Subject: [PATCH] fs/smb/client: implement chmod() for SMB3 POSIX Extensions

---
 fs/smb/client/cifsacl.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 1d294d53f662..e5237c62a440 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -933,7 +933,8 @@ static void populate_new_aces(char *nacl_base,
 		struct smb_sid *pownersid,
 		struct smb_sid *pgrpsid,
 		__u64 *pnmode, u32 *pnum_aces, u16 *pnsize,
-		bool modefromsid)
+		bool modefromsid,
+		bool posix)
 {
 	__u64 nmode;
 	u32 num_aces = 0;
@@ -950,13 +951,15 @@ static void populate_new_aces(char *nacl_base,
 	num_aces = *pnum_aces;
 	nsize = *pnsize;
 
-	if (modefromsid) {
+	if (modefromsid || posix) {
 		pnntace = (struct smb_ace *) (nacl_base + nsize);
 		nsize += setup_special_mode_ACE(pnntace, nmode);
 		num_aces++;
-		pnntace = (struct smb_ace *) (nacl_base + nsize);
-		nsize += setup_authusers_ACE(pnntace);
-		num_aces++;
+		if (modefromsid) {
+			pnntace = (struct smb_ace *) (nacl_base + nsize);
+			nsize += setup_authusers_ACE(pnntace);
+			num_aces++;
+		}
 		goto set_size;
 	}
 
@@ -1076,7 +1079,7 @@ static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *p
 
 static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 		struct smb_sid *pownersid,	struct smb_sid *pgrpsid,
-		__u64 *pnmode, bool mode_from_sid)
+		__u64 *pnmode, bool mode_from_sid, bool posix)
 {
 	int i;
 	u16 size = 0;
@@ -1098,7 +1101,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 		populate_new_aces(nacl_base,
 				pownersid, pgrpsid,
 				pnmode, &num_aces, &nsize,
-				mode_from_sid);
+				mode_from_sid, posix);
 		goto finalize_dacl;
 	}
 
@@ -1115,7 +1118,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 			populate_new_aces(nacl_base,
 					pownersid, pgrpsid,
 					pnmode, &num_aces, &nsize,
-					mode_from_sid);
+					mode_from_sid, posix);
 
 			new_aces_set = true;
 		}
@@ -1144,7 +1147,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 		populate_new_aces(nacl_base,
 				pownersid, pgrpsid,
 				pnmode, &num_aces, &nsize,
-				mode_from_sid);
+				mode_from_sid, posix);
 
 		new_aces_set = true;
 	}
@@ -1251,7 +1254,7 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
 /* Convert permission bits from mode to equivalent CIFS ACL */
 static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
 	__u32 secdesclen, __u32 *pnsecdesclen, __u64 *pnmode, kuid_t uid, kgid_t gid,
-	bool mode_from_sid, bool id_from_sid, int *aclflag)
+	bool mode_from_sid, bool id_from_sid, bool posix, int *aclflag)
 {
 	int rc = 0;
 	__u32 dacloffset;
@@ -1288,7 +1291,7 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
 		ndacl_ptr->num_aces = cpu_to_le32(0);
 
 		rc = set_chmod_dacl(dacl_ptr, ndacl_ptr, owner_sid_ptr, group_sid_ptr,
-				    pnmode, mode_from_sid);
+				    pnmode, mode_from_sid, posix);
 
 		sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size);
 		/* copy the non-dacl portion of secdesc */
@@ -1587,6 +1590,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 	struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
 	struct smb_version_operations *ops;
 	bool mode_from_sid, id_from_sid;
+	bool posix = tlink_tcon(tlink)->posix_extensions;
 	const u32 info = 0;
 
 	if (IS_ERR(tlink))
@@ -1622,12 +1626,13 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 		id_from_sid = false;
 
 	/* Potentially, five new ACEs can be added to the ACL for U,G,O mapping */
-	nsecdesclen = secdesclen;
 	if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */
-		if (mode_from_sid)
-			nsecdesclen += 2 * sizeof(struct smb_ace);
+		if (posix)
+			nsecdesclen = 1 * sizeof(struct smb_ace);
+		else if (mode_from_sid)
+			nsecdesclen = secdesclen + (2 * sizeof(struct smb_ace));
 		else /* cifsacl */
-			nsecdesclen += 5 * sizeof(struct smb_ace);
+			nsecdesclen = secdesclen + (5 * sizeof(struct smb_ace));
 	} else { /* chown */
 		/* When ownership changes, changes new owner sid length could be different */
 		nsecdesclen = sizeof(struct smb_ntsd) + (sizeof(struct smb_sid) * 2);
@@ -1657,7 +1662,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 	}
 
 	rc = build_sec_desc(pntsd, pnntsd, secdesclen, &nsecdesclen, pnmode, uid, gid,
-			    mode_from_sid, id_from_sid, &aclflag);
+			    mode_from_sid, id_from_sid, posix, &aclflag);
 
 	cifs_dbg(NOISY, "build_sec_desc rc: %d\n", rc);
 
-- 
2.47.0

Re: chmod

[Ralph Boehme]

[-- Attachment #1.1.1: Type: text/plain, Size: 158 bytes --]

On 11/14/24 12:07 PM, Ralph Boehme wrote:
> I've implemented a quick fix and am currently testing it.
this works.

Review appreciated!

Thanks!
-slow

[-- Attachment #1.1.2: chmod-smb3-posix.patch --]
[-- Type: text/x-patch, Size: 8833 bytes --]

From 2fa2838efcffbca5cdd2d7ce7449e36c51830ee4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 14 Nov 2024 11:05:13 +0100
Subject: [PATCH] fs/smb/client: implement chmod() for SMB3 POSIX Extensions

The NT ACL format for an SMB3 POSIX Extensions chmod() is a single ACE with the
magic S-1-5-88-3-mode SID:

  NT Security Descriptor
      Revision: 1
      Type: 0x8004, Self Relative, DACL Present
      Offset to owner SID: 56
      Offset to group SID: 124
      Offset to SACL: 0
      Offset to DACL: 20
      Owner: S-1-5-21-3177838999-3893657415-1037673384-1000
      Group: S-1-22-2-1000
      NT User (DACL) ACL
          Revision: NT4 (2)
          Size: 36
          Num ACEs: 1
          NT ACE: S-1-5-88-3-438, flags 0x00, Access Allowed, mask 0x00000000
              Type: Access Allowed
              NT ACE Flags: 0x00
              Size: 28
              Access required: 0x00000000
              SID: S-1-5-88-3-438

Owner and Group should be NULL, but the server is not required to fail the
request if they are present.

Signed-off-by: Ralph Boehme <slow@samba.org>
---
 fs/smb/client/cifsacl.c   | 50 +++++++++++++++++++++++----------------
 fs/smb/client/cifsproto.h |  4 +++-
 fs/smb/client/inode.c     |  4 +++-
 fs/smb/client/smb2pdu.c   |  2 +-
 4 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 1d294d53f662..c68ad526a4de 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -885,12 +885,17 @@ unsigned int setup_authusers_ACE(struct smb_ace *pntace)
  * Fill in the special SID based on the mode. See
  * https://technet.microsoft.com/en-us/library/hh509017(v=ws.10).aspx
  */
-unsigned int setup_special_mode_ACE(struct smb_ace *pntace, __u64 nmode)
+unsigned int setup_special_mode_ACE(struct smb_ace *pntace,
+				    bool posix,
+				    __u64 nmode)
 {
 	int i;
 	unsigned int ace_size = 28;
 
-	pntace->type = ACCESS_DENIED_ACE_TYPE;
+	if (posix)
+		pntace->type = ACCESS_ALLOWED_ACE_TYPE;
+	else
+		pntace->type = ACCESS_DENIED_ACE_TYPE;
 	pntace->flags = 0x0;
 	pntace->access_req = 0;
 	pntace->sid.num_subauth = 3;
@@ -933,7 +938,8 @@ static void populate_new_aces(char *nacl_base,
 		struct smb_sid *pownersid,
 		struct smb_sid *pgrpsid,
 		__u64 *pnmode, u32 *pnum_aces, u16 *pnsize,
-		bool modefromsid)
+		bool modefromsid,
+		bool posix)
 {
 	__u64 nmode;
 	u32 num_aces = 0;
@@ -950,13 +956,15 @@ static void populate_new_aces(char *nacl_base,
 	num_aces = *pnum_aces;
 	nsize = *pnsize;
 
-	if (modefromsid) {
-		pnntace = (struct smb_ace *) (nacl_base + nsize);
-		nsize += setup_special_mode_ACE(pnntace, nmode);
-		num_aces++;
+	if (modefromsid || posix) {
 		pnntace = (struct smb_ace *) (nacl_base + nsize);
-		nsize += setup_authusers_ACE(pnntace);
+		nsize += setup_special_mode_ACE(pnntace, posix, nmode);
 		num_aces++;
+		if (modefromsid) {
+			pnntace = (struct smb_ace *) (nacl_base + nsize);
+			nsize += setup_authusers_ACE(pnntace);
+			num_aces++;
+		}
 		goto set_size;
 	}
 
@@ -1076,7 +1084,7 @@ static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *p
 
 static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 		struct smb_sid *pownersid,	struct smb_sid *pgrpsid,
-		__u64 *pnmode, bool mode_from_sid)
+		__u64 *pnmode, bool mode_from_sid, bool posix)
 {
 	int i;
 	u16 size = 0;
@@ -1094,11 +1102,11 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 	nsize = sizeof(struct smb_acl);
 
 	/* If pdacl is NULL, we don't have a src. Simply populate new ACL. */
-	if (!pdacl) {
+	if (!pdacl || posix) {
 		populate_new_aces(nacl_base,
 				pownersid, pgrpsid,
 				pnmode, &num_aces, &nsize,
-				mode_from_sid);
+				mode_from_sid, posix);
 		goto finalize_dacl;
 	}
 
@@ -1115,7 +1123,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 			populate_new_aces(nacl_base,
 					pownersid, pgrpsid,
 					pnmode, &num_aces, &nsize,
-					mode_from_sid);
+					mode_from_sid, posix);
 
 			new_aces_set = true;
 		}
@@ -1144,7 +1152,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
 		populate_new_aces(nacl_base,
 				pownersid, pgrpsid,
 				pnmode, &num_aces, &nsize,
-				mode_from_sid);
+				mode_from_sid, posix);
 
 		new_aces_set = true;
 	}
@@ -1251,7 +1259,7 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
 /* Convert permission bits from mode to equivalent CIFS ACL */
 static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
 	__u32 secdesclen, __u32 *pnsecdesclen, __u64 *pnmode, kuid_t uid, kgid_t gid,
-	bool mode_from_sid, bool id_from_sid, int *aclflag)
+	bool mode_from_sid, bool id_from_sid, bool posix, int *aclflag)
 {
 	int rc = 0;
 	__u32 dacloffset;
@@ -1288,7 +1296,7 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
 		ndacl_ptr->num_aces = cpu_to_le32(0);
 
 		rc = set_chmod_dacl(dacl_ptr, ndacl_ptr, owner_sid_ptr, group_sid_ptr,
-				    pnmode, mode_from_sid);
+				    pnmode, mode_from_sid, posix);
 
 		sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size);
 		/* copy the non-dacl portion of secdesc */
@@ -1587,6 +1595,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 	struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
 	struct smb_version_operations *ops;
 	bool mode_from_sid, id_from_sid;
+	bool posix = tlink_tcon(tlink)->posix_extensions;
 	const u32 info = 0;
 
 	if (IS_ERR(tlink))
@@ -1622,12 +1631,13 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 		id_from_sid = false;
 
 	/* Potentially, five new ACEs can be added to the ACL for U,G,O mapping */
-	nsecdesclen = secdesclen;
 	if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */
-		if (mode_from_sid)
-			nsecdesclen += 2 * sizeof(struct smb_ace);
+		if (posix)
+			nsecdesclen = 1 * sizeof(struct smb_ace);
+		else if (mode_from_sid)
+			nsecdesclen = secdesclen + (2 * sizeof(struct smb_ace));
 		else /* cifsacl */
-			nsecdesclen += 5 * sizeof(struct smb_ace);
+			nsecdesclen = secdesclen + (5 * sizeof(struct smb_ace));
 	} else { /* chown */
 		/* When ownership changes, changes new owner sid length could be different */
 		nsecdesclen = sizeof(struct smb_ntsd) + (sizeof(struct smb_sid) * 2);
@@ -1657,7 +1667,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 	}
 
 	rc = build_sec_desc(pntsd, pnntsd, secdesclen, &nsecdesclen, pnmode, uid, gid,
-			    mode_from_sid, id_from_sid, &aclflag);
+			    mode_from_sid, id_from_sid, posix, &aclflag);
 
 	cifs_dbg(NOISY, "build_sec_desc rc: %d\n", rc);
 
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 1d3470bca45e..d312ea9776ce 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -244,7 +244,9 @@ extern int cifs_set_acl(struct mnt_idmap *idmap,
 extern int set_cifs_acl(struct smb_ntsd *pntsd, __u32 len, struct inode *ino,
 				const char *path, int flag);
 extern unsigned int setup_authusers_ACE(struct smb_ace *pace);
-extern unsigned int setup_special_mode_ACE(struct smb_ace *pace, __u64 nmode);
+extern unsigned int setup_special_mode_ACE(struct smb_ace *pace,
+					   bool posix,
+					   __u64 nmode);
 extern unsigned int setup_special_user_owner_ACE(struct smb_ace *pace);
 
 extern void dequeue_mid(struct mid_q_entry *mid, bool malformed);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index eff3f57235ee..72ebd72dd02b 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3062,6 +3062,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 	int rc = -EACCES;
 	__u32 dosattr = 0;
 	__u64 mode = NO_CHANGE_64;
+	bool posix = cifs_sb_master_tcon(cifs_sb)->posix_extensions;
 
 	xid = get_xid();
 
@@ -3152,7 +3153,8 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 		mode = attrs->ia_mode;
 		rc = 0;
 		if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) ||
-		    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MODE_FROM_SID)) {
+		    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MODE_FROM_SID) ||
+		    posix) {
 			rc = id_mode_to_cifs_acl(inode, full_path, &mode,
 						INVALID_UID, INVALID_GID);
 			if (rc) {
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 6584b5cddc28..ab3a2ca66be3 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2683,7 +2683,7 @@ create_sd_buf(umode_t mode, bool set_owner, unsigned int *len)
 	ptr += sizeof(struct smb3_acl);
 
 	/* create one ACE to hold the mode embedded in reserved special SID */
-	acelen = setup_special_mode_ACE((struct smb_ace *)ptr, (__u64)mode);
+	acelen = setup_special_mode_ACE((struct smb_ace *)ptr, false, (__u64)mode);
 	ptr += acelen;
 	acl_size = acelen + sizeof(struct smb3_acl);
 	ace_count = 1;
-- 
2.47.0


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: chmod

[ronnie sahlberg]

On Thu, 14 Nov 2024 at 01:42, Ralph Boehme <slow@samba.org> wrote:
> ok, I got a bit farther. It seems the client needs the mount option
> modefromsid to use this. Why? It's not even documented in the manpage.
> For a posix mount the behaviour to send a chmod(mode) as
> SMB2-SETINFO(SD, S-1-5-88-3-mode) must be the default.
>
> And then there's another problem. This commit from Ronny
>
> 0c6f4ebf8835d01866eb686d47578cde80097981
> cifs: modefromsids must add an ACE for authenticated users
>
> breaks this against Samba as Samba requires that this special SD has
> only a single ACE with the magic SID S-1-5-88-3-mode in
> check_smb2_posix_chmod_ace():
>
>          if (psd->dacl->num_aces != 1) {
>                  return false;
>          }
>
> I'm not sure I fully understand the reasoning in the commit messages,
> but I think a userspace chmod() should be mapped to an ACL with the
> single magic ACE and nothing else. Server should treat these SDs special
> int the way, that they will *only* apply the mode from the SID, the must
> not apply this as an SD (ACL) in the filesystem and hence to make this
> clear we assert psd->dacl->num_aces == 1.
>
> Am I missing anything? Thoughts?

I think I recall this was because the old behavior in modefromsids
completely broke
Windows and Azure servers (or in hindsight it probably broke EVERY
SINGLE non-samba server)
due to it while setting the magic -88-3 ACE but removing all other ACEs.
So, you can set the mode and never access the file again, ever, well
not until you go to the windows
server and repair the ACL.

I recall one reasoning for this flag, as well as its sibling idsfromsid,
is also for situations where you do NOT have multiuser mounts
but you want to make it still look like you can set uid/gid and mode
from linux clientside.


These options are not well defined and could benefit from better documentation
and possibly restrictions when they can be used and when they can not.
For the commit in question I think I recall we had to do it this way because
"we want these options to work against normal windows and azure shares"
and the old behavior was an easy to use "dos attack on share data".