[PATCH - nfsv4-acl-tools] nfs4_ace_from_string: ignore inheritance ACEs on non-directories.

[PATCH - nfsv4-acl-tools] nfs4_ace_from_string: ignore inheritance ACEs on non-directories.

[NeilBrown]

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


If you try to use
     nfs4_setfacl -R -a A:d:........   directory

to recursively set an inheritance ACE on all directories in a tree, it
will fail on the first non-directory as setting an inheritance ACE
there is not permitted (and as it aborts on the first sign of an error).

So use the is_dir flag to avoid doing that, just as is done with the
DELETE_CHILD permission.

Signed-off-by: NeilBrown <neilb@suse.com>
---

Hi Bruce,
 are you still maintaining nfsv4-acl-tools?  Last commit was over
 a year ago!! I guess that means it is nearly perfect :-)
 
 A customer came across this problem and it seems simple to fix,
 but if I'm missing something important, please let me know.

NeilBrown


 libnfs4acl/nfs4_ace_from_string.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/libnfs4acl/nfs4_ace_from_string.c b/libnfs4acl/nfs4_ace_from_string.c
index 510ffee35d32..a7a30360d860 100644
--- a/libnfs4acl/nfs4_ace_from_string.c
+++ b/libnfs4acl/nfs4_ace_from_string.c
@@ -185,16 +185,20 @@ struct nfs4_ace * nfs4_ace_from_string(char *ace_buf, int is_dir)
 	for (buflen = strlen(field); buflen > 0; buflen--) {
 		switch (*field) {
 			case FLAG_FILE_INHERIT:
-				flags |= NFS4_ACE_FILE_INHERIT_ACE;
+				if (is_dir)
+					flags |= NFS4_ACE_FILE_INHERIT_ACE;
 				break;
 			case FLAG_DIR_INHERIT:
-				flags |= NFS4_ACE_DIRECTORY_INHERIT_ACE;
+				if (is_dir)
+					flags |= NFS4_ACE_DIRECTORY_INHERIT_ACE;
 				break;
 			case FLAG_NO_PROPAGATE_INHERIT:
-				flags |= NFS4_ACE_NO_PROPAGATE_INHERIT_ACE;
+				if (is_dir)
+					flags |= NFS4_ACE_NO_PROPAGATE_INHERIT_ACE;
 				break;
 			case FLAG_INHERIT_ONLY:
-				flags |= NFS4_ACE_INHERIT_ONLY_ACE;
+				if (is_dir)
+					flags |= NFS4_ACE_INHERIT_ONLY_ACE;
 				break;
 			case FLAG_SUCCESSFUL_ACCESS:
 				flags |= NFS4_ACE_SUCCESSFUL_ACCESS_ACE_FLAG;
-- 
2.7.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH - nfsv4-acl-tools] nfs4_ace_from_string: ignore inheritance ACEs on non-directories.

[J. Bruce Fields]

On Wed, Feb 24, 2016 at 02:54:18PM +1100, NeilBrown wrote:
> 
> If you try to use
>      nfs4_setfacl -R -a A:d:........   directory
> 
> to recursively set an inheritance ACE on all directories in a tree, it
> will fail on the first non-directory as setting an inheritance ACE
> there is not permitted (and as it aborts on the first sign of an error).
> 
> So use the is_dir flag to avoid doing that, just as is done with the
> DELETE_CHILD permission.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> 
> Hi Bruce,
>  are you still maintaining nfsv4-acl-tools?  Last commit was over
>  a year ago!! I guess that means it is nearly perfect :-)

Alas, it could probably use some love.  I'm hoping richacls take over,
though.  Eventually.

>  A customer came across this problem and it seems simple to fix,
>  but if I'm missing something important, please let me know.

I didn't trace carefully through the callers, but I suspect this'll also
mean that nfs4_setfacl also silently discards inheritable ACEs in some
cases where the user could know better, instead of erroring out?

But, honestly, I'm not necessarily even sure which is the better
behavior, and -R needs to work, so, applying.

Futher patches, or volunteers for maintenance, welcome....

--b.

> 
> NeilBrown
> 
> 
>  libnfs4acl/nfs4_ace_from_string.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/libnfs4acl/nfs4_ace_from_string.c b/libnfs4acl/nfs4_ace_from_string.c
> index 510ffee35d32..a7a30360d860 100644
> --- a/libnfs4acl/nfs4_ace_from_string.c
> +++ b/libnfs4acl/nfs4_ace_from_string.c
> @@ -185,16 +185,20 @@ struct nfs4_ace * nfs4_ace_from_string(char *ace_buf, int is_dir)
>  	for (buflen = strlen(field); buflen > 0; buflen--) {
>  		switch (*field) {
>  			case FLAG_FILE_INHERIT:
> -				flags |= NFS4_ACE_FILE_INHERIT_ACE;
> +				if (is_dir)
> +					flags |= NFS4_ACE_FILE_INHERIT_ACE;
>  				break;
>  			case FLAG_DIR_INHERIT:
> -				flags |= NFS4_ACE_DIRECTORY_INHERIT_ACE;
> +				if (is_dir)
> +					flags |= NFS4_ACE_DIRECTORY_INHERIT_ACE;
>  				break;
>  			case FLAG_NO_PROPAGATE_INHERIT:
> -				flags |= NFS4_ACE_NO_PROPAGATE_INHERIT_ACE;
> +				if (is_dir)
> +					flags |= NFS4_ACE_NO_PROPAGATE_INHERIT_ACE;
>  				break;
>  			case FLAG_INHERIT_ONLY:
> -				flags |= NFS4_ACE_INHERIT_ONLY_ACE;
> +				if (is_dir)
> +					flags |= NFS4_ACE_INHERIT_ONLY_ACE;
>  				break;
>  			case FLAG_SUCCESSFUL_ACCESS:
>  				flags |= NFS4_ACE_SUCCESSFUL_ACCESS_ACE_FLAG;
> -- 
> 2.7.1
> 

Re: [PATCH - nfsv4-acl-tools] nfs4_ace_from_string: ignore inheritance ACEs on non-directories.

[NeilBrown]

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

On Tue, Mar 15 2016, J. Bruce Fields wrote:

> On Wed, Feb 24, 2016 at 02:54:18PM +1100, NeilBrown wrote:
>> 
>> If you try to use
>>      nfs4_setfacl -R -a A:d:........   directory
>> 
>> to recursively set an inheritance ACE on all directories in a tree, it
>> will fail on the first non-directory as setting an inheritance ACE
>> there is not permitted (and as it aborts on the first sign of an error).
>> 
>> So use the is_dir flag to avoid doing that, just as is done with the
>> DELETE_CHILD permission.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> 
>> Hi Bruce,
>>  are you still maintaining nfsv4-acl-tools?  Last commit was over
>>  a year ago!! I guess that means it is nearly perfect :-)
>
> Alas, it could probably use some love.  I'm hoping richacls take over,
> though.  Eventually.
>
>>  A customer came across this problem and it seems simple to fix,
>>  but if I'm missing something important, please let me know.
>
> I didn't trace carefully through the callers, but I suspect this'll also
> mean that nfs4_setfacl also silently discards inheritable ACEs in some
> cases where the user could know better, instead of erroring out?

I guess so.  If you give a file on the command line then you still want
the error.  If you give a directory and "-R" you don't.  I wonder how
much work that would be....

>
> But, honestly, I'm not necessarily even sure which is the better
> behavior, and -R needs to work, so, applying.

Thanks.

>
> Futher patches, or volunteers for maintenance, welcome....

:-)  Patches, maybe.  The rest - not me!!

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]