[PATCH] Fix for XDR Backend of NFS4ACL_XATTR module to get it working with NFS4.0 ACL Spec

[Sandeep Nashikkar]

> On Fri, Jul 20, 2018 at 12:47:16PM +0000, Sandeep Nashikkar via samba-technical wrote:
> > On Tue, Jul 21, 2018 Jeremy Allison wrote
> > > On Tue, Jul 24, 2018 Sandeep Nashikkar wrote
> 
> *         Changed XDR attribute name to "system.nfs4_acl" which works with native NFSv4 and nfs-ganesha.
> > 
> > Hmmm. I'm going to have to think about this some more
> > - this changes the read/write possibilities for the attibute to root-only. Is this correct ?
> > 
> > > <Sandeep> I used this attribute using local user as well as domain user login through samba and did not see any issue. As per man page of the xattr  
> > > "Read and write access permissions to system attributes depend on the policy implemented for each system attribute implemented by filesystems in 
> > > the kernel."
> +                             nfsace40 *nace40 = nfs40acl_get_ace(nacl40, i);
> +                             /* UTf-8 identifier strings are aligned */
> +                             if (nace40->who.utf8string_len % 4) {
> +                                             size_t id_size = nace40->who.utf8string_len;
> +                                             id_size += (4 - (nace40->who.utf8string_len % 4));
> +                                             identifiers_size += id_size;
> +                                             DBG_DEBUG("identifier[%d] size: %ld\n", i, id_size);
> +                             } else {
> +                                             identifiers_size += (nace40->who.utf8string_len);
> +                             }
> +             }
> > 
> > In the above, does utf8string_len come from the client ?
> > If so we need overflow checks on the additions of utf8string_len to prevent security problems.
> > 
> > As a rule, doing overflow checks on any additions from marshalling/unmarshalling code is a good idea anyway.
> > 
> > > <Sandeep> Can you please elaborate more on what kind of limitations to check the string length against? Are there any specific limits as per RFC which 
> > > restrict the security principal string length (or additions of it) to x number of bytes?

> > > Thanks for reviewing the patch Jeremy. I will submit the patch again with some more changes. There are cases when the security principal in the NFSv4 
> > > ACL is changed from numeric uid/gid to string by nfs-ganesha after restart. Will handle those cases as well and submit the patch with above fixes. Please > > > let me know what kind of limitations you were referring to while checking overflow conditions after unmarshalling data. Any pointer to existing code 
> > > with similar checks would be a great help. 

Please find attached the updated patch and kindly review it. I added code for handling those cases where security principals are converted to string identifiers by nfs-ganesha after restart. 


***************************Legal Disclaimer***************************
"This communication may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message by mistake, please advise the sender by reply email and delete the message. Thank you."
**********************************************************************


***************************Legal Disclaimer***************************
"This communication may contain confidential and privileged material for the
sole use of the intended recipient. Any unauthorized review, use or distribution
by others is strictly prohibited. If you have received the message by mistake,
please advise the sender by reply email and delete the message. Thank you."
**********************************************************************
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: update1-nfs4acl_xattr_patch_for_nfs40_acl_spec.patch.txt
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180725/59fddfb4/update1-nfs4acl_xattr_patch_for_nfs40_acl_spec.patch.txt>