The current mapping behaviour in Samba 3.6 using NFS4 filesystem ACLs in the case where nfs4:mode special is as follows: - an entry for a named user "user1" is added from the client - if this is the same as the connected user, samba will write an acl entry for "@owner", and remove any existing entries for "user1" - if this is for a user other than the connected user, an entry is created for "user1" This can lead to odd behaviour - if "user1" grants "user2" full access to a folder owned by "user1", and "user2" creates a subfolder - this subfolder will not grant access to "user1", contrary to the users' expectations. This behaviour can be avoided by using "nfs4:mode simple", where the @owner acl is not used at all. Additionally, the "CREATOR OWNER" special SID from Windows maps to an arbitrary unix group (as per the configured group mapping configuration), rather than to the special ACL entry "@owner". Alexander Werth described this very well in his mail to samba-technical: https://lists.samba.org/archive/samba-technical/2012-July/085760.html and submitted a patch which unfortunately got scrubbed by the mailing list.
Created attachment 8284 [details] Patch to modify mapping behaviour of creator owner ACL Attaching original patch from Alexander Werth - see https://lists.samba.org/archive/samba-technical/2012-July/085760.html "The patch for adding creator owner support to nfs4:mode simple contains the following seperate commits: - Move params struct and reading of parameters up. - Change smbacl4_get_vfs_params to use connection_struct instead of fsp. - Add params parameter to smbacl4_nfs42win function - In nfs4:mode simple read nfs4 special owner@ and group@ ACEs as "creator owner" and "creator owner group". - In nfs4:mode simple write "creator owner" and "creator owner group" as nfs4 special owner@ and group@ ACEs."
I've updated the patch to work with Samba 4. There are actually four patch sets in total: The patch for adding creator owner support to nfs4:mode simple contains the following seperate commits: - Move params struct and reading of parameters up. - Change smbacl4_get_vfs_params to use connection_struct instead of fsp. - Add params parameter to smbacl4_nfs42win function - In nfs4:mode simple read nfs4 special owner@ and group@ ACEs as "creator owner" and "creator owner group". - In nfs4:mode simple write "creator owner" and "creator owner group" as nfs4 special owner@ and group@ ACEs. The patch for adding a new nfs4 mode that combines the ideas of mode special with creator owner support - Add new nfs4:mode specialcreator on reading parameters. - Add specialcreator to nfs4acls.txt readme - Add smbacl4_expand_special function for mode specialcreate - Add function smbacl4_substitute_special. - Remove inlined special substitution of mode special. - Rewrite ACL with special entries for the owner and group. - Only rewrite acl on mode specialcreator. - Add smb_create_file_nfs4 to vfs_fn_pointers. A patch to add a nfs4:readmode parameter to allow online migration of filesystems. - Add readmode to nfs4 readme. - Add readmode to smbacl4_get_vfs_params. - Use new readmode parameter in smb_create_file_nfs4. And a patch with an optimization that skips an unnecessary transformation from nfs4 to security descriptor and back. - Optimize adjustment of specialcreator ACLs within nfs4_acls.c. - Add create_file hook to rewrite ACL on file creation for GPFS module. To fully support the specialcreator mode a vfs module with support for an nfs filesystem based on nfs4_mode must hook into create_file similar to what has been done in the last patch to add the hook to the vfs_gpfs module.
Created attachment 8367 [details] Patches adding creator owner support to nfs4:mode simple The patch for adding creator owner support to nfs4:mode simple contains the following seperate commits: - Move params struct and reading of parameters up. - Change smbacl4_get_vfs_params to use connection_struct instead of fsp. - Add params parameter to smbacl4_nfs42win function - In nfs4:mode simple read nfs4 special owner@ and group@ ACEs as "creator owner" and "creator owner group". - In nfs4:mode simple write "creator owner" and "creator owner group" as nfs4 special owner@ and group@ ACEs. It turns out that while "creator owner" and "creator owner group" ACEs behave pretty much like nfs4 inheritonly special owner@ and group@ ACEs these nfs4 special id's are not used for that purpose by the current code. The current code uses these special id's in nfs4:mode special to encode the explicit user and group ACEs of the current file owner and group. I'd like to contribute the following patch which will use the special ids for the "creator" SIDs in nfs4:mode simple.
Created attachment 8368 [details] Patches adding a new nfs4:mode specialcreator The patch for adding a new nfs4 mode that combines the ideas of mode special with creator owner support - Add new nfs4:mode specialcreator on reading parameters. - Add specialcreator to nfs4acls.txt readme - Add smbacl4_expand_special function for mode specialcreate - Add function smbacl4_substitute_special. - Remove inlined special substitution of mode special. - Rewrite ACL with special entries for the owner and group. - Only rewrite acl on mode specialcreator. - Add smb_create_file_nfs4 to vfs_fn_pointers. The nfs4:mode special is already using the nfs4 special ids in a different way to set the expected unix mode bits for the owner and group. Unfortunately the way this is done results in inheriting ACEs of the owner behaving like "creator owner" entries without actually displaying them as creator owner entries. This is frequently not the intended behavior of user aces. While this mapping to special id's is needed to get sensible posix mode bits the resulting inheritance behavior seams arbitrary and broken from a user point of view. The proposed solution to this is to automatically split the inheriting user ACEs into a non inheriting part using the nfs4 special ids to get the unix mode bits right and an inheriting part using the user id to get the inheritance right. Replacing the existing nfs4:mode special could cause migration problems so a new option "specialcreator" has been added to the nfs4:mode option. To fully support the specialcreator mode a vfs module with support for an nfs filesystem based on nfs4_mode must hook into create_file vfs call. This has been done for the vfs_gpfs module already.
Created attachment 8369 [details] Patches adding a nfs4:readmode parameter for online migration. A patch to add a nfs4:readmode parameter to allow online migration of filesystems. - Add readmode to nfs4 readme. - Add readmode to smbacl4_get_vfs_params. - Use new readmode parameter in smb_create_file_nfs4. This adds a parameter that allows to set a different mode for reading and setting ACLs. This simplifies the migration of existing filesystems between the nfs4:modes.
Created attachment 8370 [details] Patches optimizing the rewriting of ACLs on creating files. And a patch with an optimization that skips an unnecessary transformation from nfs4 to security descriptor and back. - Optimize adjustment of specialcreator ACLs within nfs4_acls.c. - Add create_file hook to rewrite ACL on file creation for GPFS module. This patch speeds up the creation of files by skipping the step to transform all unix user and group IDs to SIDs and back.
I'll take a look at this patch in the new year. Cheers, Jeremy.
Jeremy, I'm also interested in this for work, and as I seem to be getting more and more into VFS ACLs, I'll also take a look at it.
(In reply to comment #3) > Created attachment 8367 [details] > Patches adding creator owner support to nfs4:mode simple > > The patch for adding creator owner support to nfs4:mode simple contains > the following seperate commits: > - Move params struct and reading of parameters up. > - Change smbacl4_get_vfs_params to use connection_struct instead of fsp. > - Add params parameter to smbacl4_nfs42win function > - In nfs4:mode simple read nfs4 special owner@ and group@ ACEs as > "creator owner" and "creator owner group". > - In nfs4:mode simple write "creator owner" and "creator owner group" as > nfs4 special owner@ and group@ ACEs. > > > It turns out that while "creator owner" and "creator owner group" ACEs > behave pretty much like nfs4 inheritonly special owner@ and group@ ACEs > these nfs4 special id's are not used for that purpose by the current code. > > The current code uses these special id's in nfs4:mode special to encode > the explicit user and group ACEs of the current file owner and group. > > I'd like to contribute the following patch which will use the special > ids for the "creator" SIDs in nfs4:mode simple. Thanks for posting the updated patches. My biggest concern in reviewing these patches is the number of options provided. I realise we have a situation where existing deployments use these modules, but with such an array of options, I have genuine fears that there is no way to test all the combinations, and no real indication to a new user or vendor as to what the 'right' options are. If at all possible, I would like to see us just have a 'correct' mode of operation, that can read and process a variety of input ACLs but just 'does the right thing' going forward. Given the restrictions of the 'make test' environment, I know testing this will be hard, but I think we really should try. POSIX ACLs are now tested, by writing the ACL into a xattr (mapped onto a tdb), and we need to do the same here. While it will remain difficult to test the kernel-side behaviours, we can at least ensure that the mapping code runs and works reasonably, and continues to pass our ACL tests. This will not be a small amount of work - we will have to start by changing the internal NFSv4 ACL representation into IDL, but I think it will be worth it.
(In reply to comment #9) > My biggest concern in reviewing these patches is the number of options > provided. I realise we have a situation where existing deployments use these > modules, but with such an array of options, I have genuine fears that there is > no way to test all the combinations, and no real indication to a new user or > vendor as to what the 'right' options are. > > If at all possible, I would like to see us just have a 'correct' mode of > operation, that can read and process a variety of input ACLs but just 'does the > right thing' going forward. The nfs4:mode special is never the right mode but unfortunately we can't remove it without causing severe backwards compatibility problems. Also I couldn't think of any indication in the ACL itself that would allow us to do 'the right thing' without this external information. However we can make this clearer in the man page of the nfs4 module that this mode is deprecated and not recommended. The nfs4:mode simple can be recommended for CIFS only shares. The nfs4:mode specialcreator has some advantages for mixed mode access (CIFS and NFS, FTP, etc.). The nfs4:readmode parameter defaults to the same as nfs4:mode and is only required for migration. In particular online migration away from the nfs4:mode special. So a normal user or implementer only has to chose between nfs4_mode simple and nfs4:mode specialcreator which could be pointed out in the man page. > Given the restrictions of the 'make test' environment, I know testing this will > be hard, but I think we really should try. POSIX ACLs are now tested, by > writing the ACL into a xattr (mapped onto a tdb), and we need to do the same > here. While it will remain difficult to test the kernel-side behaviours, we > can at least ensure that the mapping code runs and works reasonably, and > continues to pass our ACL tests. Good point. It should be possible to create a smbtorture test that points out where the nfs4:mode special produces wrong inheritance results and the new nfs4:mode specialcreator works. This could be done without looking directly at the NFSv4 ACL and just using raw CIFS access. > This will not be a small amount of work - we will have to start by changing the > internal NFSv4 ACL representation into IDL, but I think it will be worth it. I'm not sure I'm understood this idea. Is it to enable generic NFS4 ACL tests and simplify writing of NFSv4 related tests by moving some of the NFS4 ACL code into the common code base?
(In reply to comment #10) > (In reply to comment #9) > > My biggest concern in reviewing these patches is the number of options > > provided. I realise we have a situation where existing deployments use these > > modules, but with such an array of options, I have genuine fears that there is > > no way to test all the combinations, and no real indication to a new user or > > vendor as to what the 'right' options are. > > > > If at all possible, I would like to see us just have a 'correct' mode of > > operation, that can read and process a variety of input ACLs but just 'does the > > right thing' going forward. > > The nfs4:mode special is never the right mode but unfortunately we can't remove > it without causing severe backwards compatibility problems. Also I couldn't > think of any indication in the ACL itself that would allow us to do 'the right > thing' without this external information. What would be misinterpreted if we used the wrong option? > However we can make this clearer in the man page of the nfs4 module that this > mode is deprecated and not recommended. > > The nfs4:mode simple can be recommended for CIFS only shares. Could we also deprecate that? If we get this new mode right, is there any advantage to 'simple'? > The nfs4:mode specialcreator has some advantages for mixed mode access (CIFS > and NFS, FTP, etc.). > > The nfs4:readmode parameter defaults to the same as nfs4:mode and is only > required for migration. In particular online migration away from the nfs4:mode > special. > > So a normal user or implementer only has to chose between nfs4_mode simple and > nfs4:mode specialcreator which could be pointed out in the man page. Is there any way we could make this 'just work'? > > Given the restrictions of the 'make test' environment, I know testing this will > > be hard, but I think we really should try. POSIX ACLs are now tested, by > > writing the ACL into a xattr (mapped onto a tdb), and we need to do the same > > here. While it will remain difficult to test the kernel-side behaviours, we > > can at least ensure that the mapping code runs and works reasonably, and > > continues to pass our ACL tests. > > Good point. It should be possible to create a smbtorture test that points out > where the nfs4:mode special produces wrong inheritance results and the new > nfs4:mode specialcreator works. This could be done without looking directly at > the NFSv4 ACL and just using raw CIFS access. > > > This will not be a small amount of work - we will have to start by changing the > > internal NFSv4 ACL representation into IDL, but I think it will be worth it. > > I'm not sure I'm understood this idea. Is it to enable generic NFS4 ACL tests > and simplify writing of NFSv4 related tests by moving some of the NFS4 ACL code > into the common code base? To understand my comments, you might want to look at how we test POSIX ACLs in Samba 4.0 and master. What we do, is rather than writing the POSIX ACL to the OS, we write it into a xattr. This means we can read and write posix ACLs even on systems without ACLs, and without being root. We need to do the same for NFSv4. We need to write an ACL module that is backed not by gpfs or zfs, but by xattr blobs. Then we need to test the ACL behaviours using smbtorture, during Samba's make test. Andrew Bartlett
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > My biggest concern in reviewing these patches is the number of options > > > provided. I realise we have a situation where existing deployments use these > > > modules, but with such an array of options, I have genuine fears that there is > > > no way to test all the combinations, and no real indication to a new user or > > > vendor as to what the 'right' options are. > > > > > > If at all possible, I would like to see us just have a 'correct' mode of > > > operation, that can read and process a variety of input ACLs but just 'does the > > > right thing' going forward. > > > > The nfs4:mode special is never the right mode but unfortunately we can't remove > > it without causing severe backwards compatibility problems. Also I couldn't > > think of any indication in the ACL itself that would allow us to do 'the right > > thing' without this external information. > > What would be misinterpreted if we used the wrong option? The nfs4:mode special stores an ACE for a user A who's the file owner of the folder always as a special @owner ACE. Even whe that ACE is inheriting to subfolders and files. If a user B now creates a file in that folder the ACL for that file inherits that special @owner entry which now applies to user B instead of user A. That's the wrong behavior I'm trying to fix. If we use the specialcreator mode to read the ACL of that folder from user A then that special @owner ACE would get interpreted as a CREATOR OWNER ACE that inherits to subfolders and files and a non inheriting ACE for user A. That's the missinterpretation. The displayed ACL changes. It might match the effective behavior but it frequently doesn't match the intend of the user. > > However we can make this clearer in the man page of the nfs4 module that this > > mode is deprecated and not recommended. > > > > The nfs4:mode simple can be recommended for CIFS only shares. > > Could we also deprecate that? If we get this new mode right, is there any > advantage to 'simple'? Good point. At least for the cases I know the drawbacks described below wouldn't matter or be far outweighted by usefull mode bits. These are the remaining advantages of the mode simple I can think of: - It's very slightly faster since it has two system calls less to adjust the posix mode bits. - After the creation of a new file another system call is required to rewrite the special ACL. If an application is polling the file system itself for new files and is very quick and lucky it might be able to change the access mode of the file before the ACL got rewritten and that access mode change would get lost. Deliberately doing that doesn't cause a security issue though, since the rewritten ACL has the same permissions as the initial one but without the special entries for the mode bits. But to get perfect compatibility we will eventually have to write a new ACL anyway when we have to generate a default ACL when none inherited anything. The posix default ACL is slightly different from the windows one. - Since it expands the ACE of the owner and group the mode specialcreator will usually fit two ACEs less than mode simple. - A little easier to understand. - Code path essentially a subset of mode specialcreator so it's unlikely to break suddenly. - I guess it has a better behavior on file systems with an incomplete NFSv4 ACL implementation but right now But the mode simple doesn't really add code. It just skips some steps of the mode special and specialcreator. > > The nfs4:mode specialcreator has some advantages for mixed mode access (CIFS > > and NFS, FTP, etc.). > > > > The nfs4:readmode parameter defaults to the same as nfs4:mode and is only > > required for migration. In particular online migration away from the nfs4:mode > > special. > > > > So a normal user or implementer only has to chose between nfs4_mode simple and > > nfs4:mode specialcreator which could be pointed out in the man page. > > Is there any way we could make this 'just work'? I'm afraid not. At least not immediately. But we can deprecate the nfs4:mode special and remove it eventually. And we can make that mode specialcreator the default mode. Even if we add it as non default it will enjoy a lot of manual tests and hopefully soon some smbtorture tests. We should also add the hooks for the create file call to zfs, otherwise zfs would not see the mode bit's unless someone changes the acl manually. Once that's done we could also deprecate the mode simple. At least I don't know of a use case where we would need it. Switching from mode simple to mode specialcreator works fine without any migration of the file system. But, there's more to this: Claim based ACLs are around the corner. It's unclear how these ACL should be mapped. I guess we could store them via vfs_acl_xattr and only map the normal NT ACLs to NFSv4 ACLs. Then the above should work. But I'm not sure if that's all that can be done. Certainly some customers would look for a way to use claim based ACLs but still provide access via NFS/FTP etc. So a different mapping rule might be to map just the normal allow ACEs in absence of a claim based deny ACE. > > > Given the restrictions of the 'make test' environment, I know testing this will > > > be hard, but I think we really should try. POSIX ACLs are now tested, by > > > writing the ACL into a xattr (mapped onto a tdb), and we need to do the same > > > here. While it will remain difficult to test the kernel-side behaviours, we > > > can at least ensure that the mapping code runs and works reasonably, and > > > continues to pass our ACL tests. > > > > Good point. It should be possible to create a smbtorture test that points out > > where the nfs4:mode special produces wrong inheritance results and the new > > nfs4:mode specialcreator works. This could be done without looking directly at > > the NFSv4 ACL and just using raw CIFS access. > > > > > This will not be a small amount of work - we will have to start by changing the > > > internal NFSv4 ACL representation into IDL, but I think it will be worth it. > > > > I'm not sure I'm understood this idea. Is it to enable generic NFS4 ACL tests > > and simplify writing of NFSv4 related tests by moving some of the NFS4 ACL code > > into the common code base? > > To understand my comments, you might want to look at how we test POSIX ACLs in > Samba 4.0 and master. > > What we do, is rather than writing the POSIX ACL to the OS, we write it into a > xattr. This means we can read and write posix ACLs even on systems without > ACLs, and without being root. > > We need to do the same for NFSv4. We need to write an ACL module that is > backed not by gpfs or zfs, but by xattr blobs. Then we need to test the ACL > behaviours using smbtorture, during Samba's make test. Ok, that's cool. And it shouldn't be to hard. This could test the nfs4_acl.c code as well as the testcase as part of the samba regression tests. Investigating. Alexander Werth
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > (In reply to comment #9) > > > > My biggest concern in reviewing these patches is the number of options > > > > provided. I realise we have a situation where existing deployments use these > > > > modules, but with such an array of options, I have genuine fears that there is > > > > no way to test all the combinations, and no real indication to a new user or > > > > vendor as to what the 'right' options are. > > > > > > > > If at all possible, I would like to see us just have a 'correct' mode of > > > > operation, that can read and process a variety of input ACLs but just 'does the > > > > right thing' going forward. > > > > > > The nfs4:mode special is never the right mode but unfortunately we can't remove > > > it without causing severe backwards compatibility problems. Also I couldn't > > > think of any indication in the ACL itself that would allow us to do 'the right > > > thing' without this external information. > > > > What would be misinterpreted if we used the wrong option? > > The nfs4:mode special stores an ACE for a user A who's the file owner > of the folder always as a special @owner ACE. Even whe that ACE is > inheriting to subfolders and files. > If a user B now creates a file in that folder the ACL for that file > inherits that special @owner entry which now applies to user B instead > of user A. That's the wrong behavior I'm trying to fix. > > If we use the specialcreator mode to read the ACL of that folder from > user A then that special @owner ACE would get interpreted as a CREATOR > OWNER ACE that inherits to subfolders and files and a non inheriting > ACE for user A. That's the missinterpretation. The displayed ACL > changes. It might match the effective behavior but it frequently > doesn't match the intend of the user. Shouldn't the displayed ACL match the effective behaviour? Surely that is the most secure and sensible thing to do, even if the user wanted something slightly different? If we really do want to match exactly what the user set, the acl_xattr module allows that, rather than doing it at this layer. > > > However we can make this clearer in the man page of the nfs4 module that this > > > mode is deprecated and not recommended. > > > > > > The nfs4:mode simple can be recommended for CIFS only shares. > > > > Could we also deprecate that? If we get this new mode right, is there any > > advantage to 'simple'? > > Good point. At least for the cases I know the drawbacks described > below wouldn't matter or be far outweighted by usefull mode bits. OK. What concerns me is that you said: > But the mode simple doesn't really add code. It just skips some steps > of the mode special and specialcreator. However, in attachment #8367 [details] the patch to have 'simple' return @owner entries as creator-owner seems to add a special case for simple. Is there some way we can unify that with the special and specialcreator case, for reading? > > > The nfs4:mode specialcreator has some advantages for mixed mode access (CIFS > > > and NFS, FTP, etc.). > > > > > > The nfs4:readmode parameter defaults to the same as nfs4:mode and is only > > > required for migration. In particular online migration away from the nfs4:mode > > > special. > > > > > > So a normal user or implementer only has to chose between nfs4_mode simple and > > > nfs4:mode specialcreator which could be pointed out in the man page. > > > > Is there any way we could make this 'just work'? > > I'm afraid not. > At least not immediately. > But we can deprecate the nfs4:mode special and remove it eventually. > And we can make that mode specialcreator the default mode. So, the case that started me on this was an internal report complaining: ACL (good): root# ls -Vdl /myshare/ d---------+ 2 root root 2 Jan 2 07:49 /myshare/ user:administrator:rwxpdDaARWcCo-:fd-----:allow group:domain admins:rwxpdDaARWcCo-:fd-----:allow owner@:--------------:fd-----:allow group@:--------------:fd-----:allow everyone@:--------------:fd-----:allow 5. Add domain users entry from Windows SMB client ===ACL (bad, owner@ and group@ entries have been replaced with user:root, group:root)=== root# /usr/sun/bin/ls -Vdl /myshare/ d---------+ 2 root root 2 Jan 2 07:13 /myshare/ user:administrator:rwxpdDaARWcCo-:fd-----:allow group:domain admins:rwxpdDaARWcCo-:fd-----:allow user:root:--------------:fd-----:allow group:root:--------------:fd-----:allow everyone@:--------------:fd-----:allow group:domain users:r-x---a-R-c---:fd-----:allow The reporter tells me that adding "nfs4:mode = special" in /etc/smb.conf seemed to resolve this issue. What I'm wondering is, is there any reason we should ever return an ACL already having owner@ as anything other than CREATOR_ONWNER? Then, when the ACL is set back, with a new ACE for an unrelated group, wouldn't it naturally translate back into the correct owner@? If we got that right, we could narrow the difference for ACLs first created directly on the NFSv4-using filesystem backend, and so reduce the difference between the modes, making them easier to test, eliminate or deprecate in the longer term. If I read it right, your attachment #8367 [details] seems to be to address this case. Is correct? > Even if we add it as non default it will enjoy a lot of manual tests > and hopefully soon some smbtorture tests. > We should also add the hooks for the create file call to zfs, > otherwise zfs would not see the mode bit's unless someone changes the > acl manually. > Once that's done we could also deprecate the mode simple. At least I > don't know of a use case where we would need it. Switching from mode > simple to mode specialcreator works fine without any migration of the > file system. > > But, there's more to this: Claim based ACLs are around the corner. > It's unclear how these ACL should be mapped. I guess we could store > them via vfs_acl_xattr and only map the normal NT ACLs to NFSv4 > ACLs. Then the above should work. But I'm not sure if that's all that > can be done. Certainly some customers would look for a way to use > claim based ACLs but still provide access via NFS/FTP etc. So a > different mapping rule might be to map just the normal allow ACEs in > absence of a claim based deny ACE. I need to learn more about them, but I think claim based ACLs are going to be a totally different ACL modal, and it will (again) take years to get it into filesystems. Let's deal with hear and now of NFSv4/NT ACLs for the moment. I'll be back with access to my ZFS based system next week, I'll have more of a look at all of this then. I think we should at least get attachment #8367 [details] into master. Andrew Bartlett
I'm splitting the answer. (In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #11) > > > (In reply to comment #10) > > > > (In reply to comment #9) > > > > The nfs4:mode specialcreator has some advantages for mixed mode access (CIFS > > > > and NFS, FTP, etc.). > > > > > > > > The nfs4:readmode parameter defaults to the same as nfs4:mode and is only > > > > required for migration. In particular online migration away from the nfs4:mode > > > > special. > > > > > > > > So a normal user or implementer only has to chose between nfs4_mode simple and > > > > nfs4:mode specialcreator which could be pointed out in the man page. > > > > > > Is there any way we could make this 'just work'? > > > > I'm afraid not. > > At least not immediately. > > But we can deprecate the nfs4:mode special and remove it eventually. > > And we can make that mode specialcreator the default mode. > > > So, the case that started me on this was an internal report complaining: > > ACL (good): > root# ls -Vdl /myshare/ > d---------+ 2 root root 2 Jan 2 07:49 /myshare/ > user:administrator:rwxpdDaARWcCo-:fd-----:allow > group:domain admins:rwxpdDaARWcCo-:fd-----:allow > owner@:--------------:fd-----:allow > group@:--------------:fd-----:allow > everyone@:--------------:fd-----:allow > > 5. Add domain users entry from Windows SMB client > ===ACL (bad, owner@ and group@ entries have been replaced with user:root, > group:root)=== > root# /usr/sun/bin/ls -Vdl /myshare/ > d---------+ 2 root root 2 Jan 2 07:13 /myshare/ > user:administrator:rwxpdDaARWcCo-:fd-----:allow > group:domain admins:rwxpdDaARWcCo-:fd-----:allow > user:root:--------------:fd-----:allow > group:root:--------------:fd-----:allow > everyone@:--------------:fd-----:allow > group:domain users:r-x---a-R-c---:fd-----:allow > > The reporter tells me that adding "nfs4:mode = special" in /etc/smb.conf seemed > to resolve this issue. > > What I'm wondering is, is there any reason we should ever return an ACL already > having owner@ as anything other than CREATOR_ONWNER? Then, when the ACL is set > back, with a new ACE for an unrelated group, wouldn't it naturally translate > back into the correct owner@? > > If we got that right, we could narrow the difference for ACLs first created > directly on the NFSv4-using filesystem backend, and so reduce the difference > between the modes, making them easier to test, eliminate or deprecate in the > longer term. > > If I read it right, your attachment #8367 [details] seems to be to address this case. Is > correct? Interesting case. That's indeed what would happen if such an ACL would be rewritten through nfs4_mode simple and it's a reason why nfs4_mode simple is problematic for a mixed CIFS/NFS share access. With regards to your question first keep in mind that only the inheriting owner@ and group@ ACEs are behaving like creator owner entries. Second the mode special was made to address issues like this but fails by mapping the inherited owner@ and group@ entries wrongly to inherited entries of the file owner and group instead of creator owner. The suggested mode special would map the creator owner entries to these inherit only owner@ and group@ entries and back. And it would map the non inheriting onwer@ entries to non inheriting user entries and back. So in the example above the ACL on the file system would not change and still consist of owner@, group@, and everyone@. The reason why we should be keep a mode special is simply because of existing customers. At least we need some control over when the behavior changes on existing installations. Alexander Werth
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #11) > > > (In reply to comment #10) > > > > (In reply to comment #9) > > > > > My biggest concern in reviewing these patches is the number of options > > > > > provided. I realise we have a situation where existing deployments use these > > > > > modules, but with such an array of options, I have genuine fears that there is > > > > > no way to test all the combinations, and no real indication to a new user or > > > > > vendor as to what the 'right' options are. > > > > > > > > > > If at all possible, I would like to see us just have a 'correct' mode of > > > > > operation, that can read and process a variety of input ACLs but just 'does the > > > > > right thing' going forward. > > > > > > > > The nfs4:mode special is never the right mode but unfortunately we can't remove > > > > it without causing severe backwards compatibility problems. Also I couldn't > > > > think of any indication in the ACL itself that would allow us to do 'the right > > > > thing' without this external information. > > > > > > What would be misinterpreted if we used the wrong option? > > > > The nfs4:mode special stores an ACE for a user A who's the file owner > > of the folder always as a special @owner ACE. Even whe that ACE is > > inheriting to subfolders and files. > > If a user B now creates a file in that folder the ACL for that file > > inherits that special @owner entry which now applies to user B instead > > of user A. That's the wrong behavior I'm trying to fix. > > > > If we use the specialcreator mode to read the ACL of that folder from > > user A then that special @owner ACE would get interpreted as a CREATOR > > OWNER ACE that inherits to subfolders and files and a non inheriting > > ACE for user A. That's the missinterpretation. The displayed ACL > > changes. It might match the effective behavior but it frequently > > doesn't match the intend of the user. > > Shouldn't the displayed ACL match the effective behaviour? Surely that is the > most secure and sensible thing to do, even if the user wanted something > slightly different? Yes it should. And that is why mode special is problematic and needs improvement. But changing the behavior as a side effect of a samba update is very problematic for existing customers. > If we really do want to match exactly what the user set, the acl_xattr module > allows that, rather than doing it at this layer. Mapping as much behavior as possible into NFSv4 has benefits for a mixed share access through multiple protocols. For CIFS only access the acl_xattr module is nice and it might still be usefull on top of the nfs4 mapping for an extra boost of compatibility. But for mixed protocol access the mapped ACLs should at least work correctly. Alexander Werth
Here's another suggestion for the README.nfs4acls.txt help text that's more vocal about when to use what option. mode = [simple|special|specialcreator] - simple: Use inheriting OWNER@ and GROUP@ special IDs in ACEs for creator owner and creator owner group only. - default - special: use OWNER@ and GROUP@ special IDs in ACEs instead of simple user&group ids. - deprecated - specialcreator: use non inheriting OWNER@ and GROUP@ special IDs for mode bits and inheriting OWNER@ and GROUP@ special IDs for creator owner and creator owner group. Note: EVERYONE@ is always processed (if found such an ACE). Note2: special mode has side effects on inheritance when a different owner creates a file and when _only_ chown is performed. Note3: special mode can't store creator owner ACL entries. These show up as explicit entries for the current user. Use "simple" mode when the share is used mainly by windows users and unix side is not significant. You will loose unix bits in this case. Use "specialcreator" mode when the share is to be accessed through both unix and cifs systems. Files show up with unix mode bits. It's strongly advised setting "store dos attributes = yes" in smb.conf. readmode = [simple|special|specialcreator] Allows to interpret ACLs different while reading and writing them. Defaults to the same setting as nfs4:mode. This should only be used for online migration between nfs4:mode modes.
(In reply to comment #13) > What concerns me is that you said: > (In reply to comment #12) > > But the mode simple doesn't really add code. It just skips some steps > > of the mode special and specialcreator. > > However, in attachment #8367 [details] the patch to have 'simple' return @owner entries > as creator-owner seems to add a special case for simple. Is there some way we > can unify that with the special and specialcreator case, for reading? You found a bug in patchset #8368. There the patch should contain something like this: /* Mapping of special entries to creator owner. */ - if (params->mode == e_simple && + if ((params->mode == e_simple || + params->mode == e_specialcreator) && This in only part of the patchset #8369 to add the readmode: /* Mapping of special entries to creator owner. */ - if (params->mode == e_simple && + if ((params->readmode == e_simple || + params->readmode == e_specialcreator) && Btw. The readmode parameter would only be interesting for an online migration from mode special to one of the other modes. So it's only temporarily interesting for a very small subset of Samba users. If the number of parameters is an issue this could be easily omitted. Alexander Werth
*** Bug 5769 has been marked as a duplicate of this bug. ***
(In reply to comment #12) > (In reply to comment #11) > > We need to do the same for NFSv4. We need to write an ACL module that is > > backed not by gpfs or zfs, but by xattr blobs. Then we need to test the ACL > > behaviours using smbtorture, during Samba's make test. > > Ok, that's cool. And it shouldn't be to hard. This could test the > nfs4_acl.c code as well as the testcase as part of the samba > regression tests. Investigating. > > Alexander Werth Have you made any progress on the tests? Otherwise I may need to look into it, but I don't want to start if you are already actively working on this. Thanks,
Created attachment 8793 [details] Patches adding creator owner support to nfs4:mode simple Updated the patch to work against 4.0 after the patches to test nfs4_acls.c are applied.
Created attachment 8824 [details] Patches adding a new nfs4:mode specialcreator Patches adding a new nfs4:mode specialcreator. This patch takes the recent development for nfs4 acl tests into account.
Created attachment 8825 [details] Patches adding a new nfs4:mode specialcreator Fixed patch numbering [PATCH 1/7]
G'Day all, With the patches now in master for the improved simple (and specialcreator dropped), is someone already preparing a backport of these changes? It would be very helpful for other vendors if they were posted here. Finally, are there any other issues not addressed by the patches we got in? Thanks,
Created attachment 9048 [details] Backport to the v3-6-stable branch Backport of the patches to Samba 3.6.
plrease everybody, close the bug reports after they are fixed and you checked in the patch. This one was fixed 8 years ago.