GNU bug report logs - #33644
[PATCH] cp --preserve=xattr: preserve NFSv4 ACL extended attributes

Previous Next

Package: coreutils;

Reported by: Kamil Dudka <kdudka <at> redhat.com>

Date: Thu, 6 Dec 2018 13:09:02 UTC

Severity: normal

Tags: notabug, patch

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 33644 in the body.
You can then email your comments to 33644 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Thu, 06 Dec 2018 13:09:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Kamil Dudka <kdudka <at> redhat.com>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Thu, 06 Dec 2018 13:09:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Kamil Dudka <kdudka <at> redhat.com>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL extended attributes
Date: Thu,  6 Dec 2018 14:08:09 +0100
... which cannot be preserved by other means

Bug: https://bugzilla.redhat.com/1031423#c4
---
 src/copy.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 3221b9997..754c5e1aa 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -640,6 +640,17 @@ copy_attr_free (struct error_context *ctx _GL_UNUSED,
 {
 }
 
+/* Include NFSv4 ACL extended attributes, which cannot be preserved by
+   other means.  Otherwise honor attributes configured for exclusion
+   in /etc/xattr.conf.  Return zero to skip.  */
+static int
+check_not_nfs4_acl (const char *name, struct error_context *ctx)
+{
+  return attr_copy_check_permissions(name, ctx)
+         || !STRNCMP_LIT (name, "system.nfs4_acl")
+         || !STRNCMP_LIT (name, "system.nfs4acl");
+}
+
 /* Exclude SELinux extended attributes that are otherwise handled,
    and are problematic to copy again.  Also honor attributes
    configured for exclusion in /etc/xattr.conf.
@@ -649,7 +660,7 @@ static int
 check_selinux_attr (const char *name, struct error_context *ctx)
 {
   return STRNCMP_LIT (name, "security.selinux")
-         && attr_copy_check_permissions (name, ctx);
+         && check_not_nfs4_acl (name, ctx);
 }
 
 /* If positive SRC_FD and DST_FD descriptors are passed,
@@ -663,6 +674,9 @@ copy_attr (char const *src_path, int src_fd,
   bool all_errors = (!x->data_copy_required || x->require_preserve_xattr);
   bool some_errors = (!all_errors && !x->reduce_diagnostics);
   bool selinux_done = (x->preserve_security_context || x->set_security_context);
+  int (*check) (const char *, struct error_context *) = (selinux_done)
+    ? check_selinux_attr
+    : check_not_nfs4_acl;
   struct error_context ctx =
   {
     .error = all_errors ? copy_attr_allerror : copy_attr_error,
@@ -670,12 +684,10 @@ copy_attr (char const *src_path, int src_fd,
     .quote_free = copy_attr_free
   };
   if (0 <= src_fd && 0 <= dst_fd)
-    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd,
-                        selinux_done ? check_selinux_attr : NULL,
+    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd, check,
                         (all_errors || some_errors ? &ctx : NULL));
   else
-    ret = attr_copy_file (src_path, dst_path,
-                          selinux_done ? check_selinux_attr : NULL,
+    ret = attr_copy_file (src_path, dst_path, check,
                           (all_errors || some_errors ? &ctx : NULL));
 
   return ret == 0;
-- 
2.17.2





Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Mon, 11 Feb 2019 05:08:02 GMT) Full text and rfc822 format available.

Message #8 received at 33644 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Kamil Dudka <kdudka <at> redhat.com>, 33644 <at> debbugs.gnu.org
Subject: Re: bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL
 extended attributes
Date: Sun, 10 Feb 2019 21:07:18 -0800
On 06/12/18 05:08, Kamil Dudka wrote:
> ... which cannot be preserved by other means
> 
> Bug: https://bugzilla.redhat.com/1031423#c4
> ---
>  src/copy.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/src/copy.c b/src/copy.c
> index 3221b9997..754c5e1aa 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -640,6 +640,17 @@ copy_attr_free (struct error_context *ctx _GL_UNUSED,
>  {
>  }
>  
> +/* Include NFSv4 ACL extended attributes, which cannot be preserved by
> +   other means.  Otherwise honor attributes configured for exclusion
> +   in /etc/xattr.conf.  Return zero to skip.  */
> +static int
> +check_not_nfs4_acl (const char *name, struct error_context *ctx)
> +{
> +  return attr_copy_check_permissions(name, ctx)
> +         || !STRNCMP_LIT (name, "system.nfs4_acl")
> +         || !STRNCMP_LIT (name, "system.nfs4acl");
> +}
> +
>  /* Exclude SELinux extended attributes that are otherwise handled,
>     and are problematic to copy again.  Also honor attributes
>     configured for exclusion in /etc/xattr.conf.
> @@ -649,7 +660,7 @@ static int
>  check_selinux_attr (const char *name, struct error_context *ctx)
>  {
>    return STRNCMP_LIT (name, "security.selinux")
> -         && attr_copy_check_permissions (name, ctx);
> +         && check_not_nfs4_acl (name, ctx);
>  }
>  
>  /* If positive SRC_FD and DST_FD descriptors are passed,
> @@ -663,6 +674,9 @@ copy_attr (char const *src_path, int src_fd,
>    bool all_errors = (!x->data_copy_required || x->require_preserve_xattr);
>    bool some_errors = (!all_errors && !x->reduce_diagnostics);
>    bool selinux_done = (x->preserve_security_context || x->set_security_context);
> +  int (*check) (const char *, struct error_context *) = (selinux_done)
> +    ? check_selinux_attr
> +    : check_not_nfs4_acl;
>    struct error_context ctx =
>    {
>      .error = all_errors ? copy_attr_allerror : copy_attr_error,
> @@ -670,12 +684,10 @@ copy_attr (char const *src_path, int src_fd,
>      .quote_free = copy_attr_free
>    };
>    if (0 <= src_fd && 0 <= dst_fd)
> -    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd,
> -                        selinux_done ? check_selinux_attr : NULL,
> +    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd, check,
>                          (all_errors || some_errors ? &ctx : NULL));
>    else
> -    ret = attr_copy_file (src_path, dst_path,
> -                          selinux_done ? check_selinux_attr : NULL,
> +    ret = attr_copy_file (src_path, dst_path, check,
>                            (all_errors || some_errors ? &ctx : NULL));
>  
>    return ret == 0;
> 

This patch is confusing to read, though looks functional.
It's clearer of you rename check_not_nfs4_acl() to check_but_allow_nfs4_acl().

So in summary, any xattr in /etc/xattr.conf is _not_ copied.
You want to essentially ignore the nfs4 entries in that config file.
So why not just remove the entries from that file?
Is that something that could be done in attr.git?
Why would one want to treat nfs4 attrs differently to the posix_acl_access attrs?

thanks,
Pádraig.




Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Mon, 11 Feb 2019 11:50:02 GMT) Full text and rfc822 format available.

Message #11 received at 33644 <at> debbugs.gnu.org (full text, mbox):

From: Kamil Dudka <kdudka <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 33644 <at> debbugs.gnu.org
Subject: Re: bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL
 extended attributes
Date: Mon, 11 Feb 2019 12:50:11 +0100
On Monday, February 11, 2019 6:07:18 AM CET Pádraig Brady wrote:
> On 06/12/18 05:08, Kamil Dudka wrote:
> > ... which cannot be preserved by other means
> > 
> > Bug: https://bugzilla.redhat.com/1031423#c4
> > ---
> > 
> >  src/copy.c | 22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/copy.c b/src/copy.c
> > index 3221b9997..754c5e1aa 100644
> > --- a/src/copy.c
> > +++ b/src/copy.c
> > @@ -640,6 +640,17 @@ copy_attr_free (struct error_context *ctx _GL_UNUSED,
> > 
> >  {
> >  }
> > 
> > +/* Include NFSv4 ACL extended attributes, which cannot be preserved by
> > +   other means.  Otherwise honor attributes configured for exclusion
> > +   in /etc/xattr.conf.  Return zero to skip.  */
> > +static int
> > +check_not_nfs4_acl (const char *name, struct error_context *ctx)
> > +{
> > +  return attr_copy_check_permissions(name, ctx)
> > +         || !STRNCMP_LIT (name, "system.nfs4_acl")
> > +         || !STRNCMP_LIT (name, "system.nfs4acl");
> > +}
> > +
> > 
> >  /* Exclude SELinux extended attributes that are otherwise handled,
> >  
> >     and are problematic to copy again.  Also honor attributes
> >     configured for exclusion in /etc/xattr.conf.
> > 
> > @@ -649,7 +660,7 @@ static int
> > 
> >  check_selinux_attr (const char *name, struct error_context *ctx)
> >  {
> >  
> >    return STRNCMP_LIT (name, "security.selinux")
> > 
> > -         && attr_copy_check_permissions (name, ctx);
> > +         && check_not_nfs4_acl (name, ctx);
> > 
> >  }
> >  
> >  /* If positive SRC_FD and DST_FD descriptors are passed,
> > 
> > @@ -663,6 +674,9 @@ copy_attr (char const *src_path, int src_fd,
> > 
> >    bool all_errors = (!x->data_copy_required ||
> >    x->require_preserve_xattr);
> >    bool some_errors = (!all_errors && !x->reduce_diagnostics);
> >    bool selinux_done = (x->preserve_security_context ||
> >    x->set_security_context);> 
> > +  int (*check) (const char *, struct error_context *) = (selinux_done)
> > +    ? check_selinux_attr
> > +    : check_not_nfs4_acl;
> > 
> >    struct error_context ctx =
> >    {
> >    
> >      .error = all_errors ? copy_attr_allerror : copy_attr_error,
> > 
> > @@ -670,12 +684,10 @@ copy_attr (char const *src_path, int src_fd,
> > 
> >      .quote_free = copy_attr_free
> >    
> >    };
> >    if (0 <= src_fd && 0 <= dst_fd)
> > 
> > -    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd,
> > -                        selinux_done ? check_selinux_attr : NULL,
> > +    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd, check,
> > 
> >                          (all_errors || some_errors ? &ctx : NULL));
> >    
> >    else
> > 
> > -    ret = attr_copy_file (src_path, dst_path,
> > -                          selinux_done ? check_selinux_attr : NULL,
> > +    ret = attr_copy_file (src_path, dst_path, check,
> > 
> >                            (all_errors || some_errors ? &ctx : NULL));
> >    
> >    return ret == 0;
> 
> This patch is confusing to read, though looks functional.

I can submit deduplication of the `selinux_done ? check_selinux_attr : NULL` 
code as a separate patch if you prefer it.

> It's clearer of you rename check_not_nfs4_acl() to
> check_but_allow_nfs4_acl().

Fine by me.

> So in summary, any xattr in /etc/xattr.conf is _not_ copied.
> You want to essentially ignore the nfs4 entries in that config file.
> So why not just remove the entries from that file?

See how xattr.conf is documented:

    # Actions:
    #   permissions - copy when trying to preserve permissions.
    #   skip - do not copy.

The fact that coreutils handles `persmissions` equally as `skip` is IMO a 
problem of coreutils, not a problem of xattr.conf.

> Is that something that could be done in attr.git?

I think that the information in xattr.conf is correct.  system.nfs4_acl is 
really an attribute one wants to copy when trying to preserve permissions.

> Why would one want to treat nfs4 attrs differently to the posix_acl_access
> attrs?

It was written in the commit message.  One can use `cp --preserve=mode`
to preserve POSIX ACLs whereas the only way to preserve NFSv4 ACLs was
`cp --preserve=xattr`.

> thanks,
> Pádraig.

On Monday, February 11, 2019 6:21:49 AM CET Pádraig Brady wrote:
> BTW is there anything interesting behind this paywall I can't access?
> https://access.redhat.com/solutions/115043

It just says that `cp a b` does not preserve NFSv4 ACLs whereas `cp -a a b`,
`cp --preserve=all a b`, or `cp --preserve=xattr a b` does.  Unfortunately, 
this is currently true only for Red Hat Enterprise Linux.

Kamil






Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Mon, 11 Feb 2019 18:31:01 GMT) Full text and rfc822 format available.

Message #14 received at 33644 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Kamil Dudka <kdudka <at> redhat.com>
Cc: 33644 <at> debbugs.gnu.org
Subject: Re: bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL
 extended attributes
Date: Mon, 11 Feb 2019 10:30:42 -0800
On 11/02/19 03:50, Kamil Dudka wrote:
> On Monday, February 11, 2019 6:07:18 AM CET Pádraig Brady wrote:
>> On 06/12/18 05:08, Kamil Dudka wrote:
>>> ... which cannot be preserved by other means
>>>
>>> Bug: https://bugzilla.redhat.com/1031423#c4
>>> ---
>>>
>>>  src/copy.c | 22 +++++++++++++++++-----
>>>  1 file changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/copy.c b/src/copy.c
>>> index 3221b9997..754c5e1aa 100644
>>> --- a/src/copy.c
>>> +++ b/src/copy.c
>>> @@ -640,6 +640,17 @@ copy_attr_free (struct error_context *ctx _GL_UNUSED,
>>>
>>>  {
>>>  }
>>>
>>> +/* Include NFSv4 ACL extended attributes, which cannot be preserved by
>>> +   other means.  Otherwise honor attributes configured for exclusion
>>> +   in /etc/xattr.conf.  Return zero to skip.  */
>>> +static int
>>> +check_not_nfs4_acl (const char *name, struct error_context *ctx)
>>> +{
>>> +  return attr_copy_check_permissions(name, ctx)
>>> +         || !STRNCMP_LIT (name, "system.nfs4_acl")
>>> +         || !STRNCMP_LIT (name, "system.nfs4acl");
>>> +}
>>> +
>>>
>>>  /* Exclude SELinux extended attributes that are otherwise handled,
>>>  
>>>     and are problematic to copy again.  Also honor attributes
>>>     configured for exclusion in /etc/xattr.conf.
>>>
>>> @@ -649,7 +660,7 @@ static int
>>>
>>>  check_selinux_attr (const char *name, struct error_context *ctx)
>>>  {
>>>  
>>>    return STRNCMP_LIT (name, "security.selinux")
>>>
>>> -         && attr_copy_check_permissions (name, ctx);
>>> +         && check_not_nfs4_acl (name, ctx);
>>>
>>>  }
>>>  
>>>  /* If positive SRC_FD and DST_FD descriptors are passed,
>>>
>>> @@ -663,6 +674,9 @@ copy_attr (char const *src_path, int src_fd,
>>>
>>>    bool all_errors = (!x->data_copy_required ||
>>>    x->require_preserve_xattr);
>>>    bool some_errors = (!all_errors && !x->reduce_diagnostics);
>>>    bool selinux_done = (x->preserve_security_context ||
>>>    x->set_security_context);> 
>>> +  int (*check) (const char *, struct error_context *) = (selinux_done)
>>> +    ? check_selinux_attr
>>> +    : check_not_nfs4_acl;
>>>
>>>    struct error_context ctx =
>>>    {
>>>    
>>>      .error = all_errors ? copy_attr_allerror : copy_attr_error,
>>>
>>> @@ -670,12 +684,10 @@ copy_attr (char const *src_path, int src_fd,
>>>
>>>      .quote_free = copy_attr_free
>>>    
>>>    };
>>>    if (0 <= src_fd && 0 <= dst_fd)
>>>
>>> -    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd,
>>> -                        selinux_done ? check_selinux_attr : NULL,
>>> +    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd, check,
>>>
>>>                          (all_errors || some_errors ? &ctx : NULL));
>>>    
>>>    else
>>>
>>> -    ret = attr_copy_file (src_path, dst_path,
>>> -                          selinux_done ? check_selinux_attr : NULL,
>>> +    ret = attr_copy_file (src_path, dst_path, check,
>>>
>>>                            (all_errors || some_errors ? &ctx : NULL));
>>>    
>>>    return ret == 0;
>>
>> This patch is confusing to read, though looks functional.
> 
> I can submit deduplication of the `selinux_done ? check_selinux_attr : NULL` 
> code as a separate patch if you prefer it.
> 
>> It's clearer of you rename check_not_nfs4_acl() to
>> check_but_allow_nfs4_acl().
> 
> Fine by me.
> 
>> So in summary, any xattr in /etc/xattr.conf is _not_ copied.
>> You want to essentially ignore the nfs4 entries in that config file.
>> So why not just remove the entries from that file?
> 
> See how xattr.conf is documented:
> 
>     # Actions:
>     #   permissions - copy when trying to preserve permissions.
>     #   skip - do not copy.
> 
> The fact that coreutils handles `persmissions` equally as `skip` is IMO a 
> problem of coreutils, not a problem of xattr.conf.
> 
>> Is that something that could be done in attr.git?
> 
> I think that the information in xattr.conf is correct.  system.nfs4_acl is 
> really an attribute one wants to copy when trying to preserve permissions.

Right. What I was getting at was attr_copy_file() from libattr seems
to skip all entries in xattr.conf by default. I need to dig in to
see what's preserving system.posix_acl_access (these might be
implicitly generated upon attr reading for example).
My question was why does coreutils need to explicitly handle
the nfs4 acls if it doesn't need to handle the posix ones.

I'm not saying the patch is wrong at all,
I'm just not seeing the full picture.

> 
>> Why would one want to treat nfs4 attrs differently to the posix_acl_access
>> attrs?
> 
> It was written in the commit message.  One can use `cp --preserve=mode`
> to preserve POSIX ACLs whereas the only way to preserve NFSv4 ACLs was
> `cp --preserve=xattr`.
> 
>> thanks,
>> Pádraig.
> 
> On Monday, February 11, 2019 6:21:49 AM CET Pádraig Brady wrote:
>> BTW is there anything interesting behind this paywall I can't access?
>> https://access.redhat.com/solutions/115043
> 
> It just says that `cp a b` does not preserve NFSv4 ACLs whereas `cp -a a b`,
> `cp --preserve=all a b`, or `cp --preserve=xattr a b` does.  Unfortunately, 
> this is currently true only for Red Hat Enterprise Linux.

I'll dig in some more.
thanks,
Pádraig





Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Tue, 12 Feb 2019 12:03:02 GMT) Full text and rfc822 format available.

Message #17 received at 33644 <at> debbugs.gnu.org (full text, mbox):

From: Kamil Dudka <kdudka <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 33644 <at> debbugs.gnu.org
Subject: Re: bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL
 extended attributes
Date: Tue, 12 Feb 2019 13:03:34 +0100
On Monday, February 11, 2019 7:30:42 PM CET Pádraig Brady wrote:
> On 11/02/19 03:50, Kamil Dudka wrote:
> > I think that the information in xattr.conf is correct.  system.nfs4_acl is
> > really an attribute one wants to copy when trying to preserve permissions.
> 
> Right. What I was getting at was attr_copy_file() from libattr seems
> to skip all entries in xattr.conf by default. I need to dig in to
> see what's preserving system.posix_acl_access (these might be
> implicitly generated upon attr reading for example).

I do not know the reasoning behind the default behavior of attr_copy_file().
There is a comment before the function definition but it does not talk about
NFSv4 ACLs:

http://git.savannah.nongnu.org/cgit/attr.git/tree/libattr/attr_copy_file.c?id=cb4786f1#n54

> My question was why does coreutils need to explicitly handle
> the nfs4 acls if it doesn't need to handle the posix ones.

I think the answer is obvious.  cp is able preserve POSIX ACLs at a higher
level (using gnulib's acl module, which uses libacl internally on Linux).
There is, unfortunately, no such module (neither library) for NFSv4 ACLs.
So copying the value of the low-level attribute is currently the only way
to make cp preserve NFSv4 ACLs.

Kamil






Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Sun, 03 Mar 2019 02:08:02 GMT) Full text and rfc822 format available.

Message #20 received at 33644 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Kamil Dudka <kdudka <at> redhat.com>
Cc: 33644 <at> debbugs.gnu.org
Subject: Re: bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL
 extended attributes
Date: Sat, 2 Mar 2019 18:07:53 -0800
tag 33644 notabug
close 33644
stop

rationale below...

On 12/02/19 04:03, Kamil Dudka wrote:
> On Monday, February 11, 2019 7:30:42 PM CET Pádraig Brady wrote:
>> On 11/02/19 03:50, Kamil Dudka wrote:
>>> I think that the information in xattr.conf is correct.  system.nfs4_acl is
>>> really an attribute one wants to copy when trying to preserve permissions.
>>
>> Right. What I was getting at was attr_copy_file() from libattr seems
>> to skip all entries in xattr.conf by default. I need to dig in to
>> see what's preserving system.posix_acl_access (these might be
>> implicitly generated upon attr reading for example).
> 
> I do not know the reasoning behind the default behavior of attr_copy_file().
> There is a comment before the function definition but it does not talk about
> NFSv4 ACLs:
> 
> http://git.savannah.nongnu.org/cgit/attr.git/tree/libattr/attr_copy_file.c?id=cb4786f1#n54
> 
>> My question was why does coreutils need to explicitly handle
>> the nfs4 acls if it doesn't need to handle the posix ones.
> 
> I think the answer is obvious.  cp is able preserve POSIX ACLs at a higher
> level (using gnulib's acl module, which uses libacl internally on Linux).
> There is, unfortunately, no such module (neither library) for NFSv4 ACLs.
> So copying the value of the low-level attribute is currently the only way
> to make cp preserve NFSv4 ACLs.

You used "obvious" and "ACLs" in the same email :)

Looking a bit more...

So attr_copy_file() copies all except those defined in /etc/xattr.conf

ACL xattrs are listed in that file with the rationale from a comment in libattr being:

 "ACLs are excluded by default because copying them between
  file systems with and without ACL support needs some
  additional logic so that no unexpected permissions result."

So the ACL handling specifically is deferred to libacl.
Now system.posix_acl_access is handled by libacl,
but system.nfs4_acl is not.
So I think the correct fix here is to remove the
nfs entries from /etc/xattr.conf, and then cp will copy.
This has the advantage of being configurable,
and also removes nfs4 specific handling from cp.
Any nfs4 specific handling should be in libacl.

thanks,
Pádraig




Added tag(s) notabug. Request was from Pádraig Brady <P <at> draigBrady.com> to control <at> debbugs.gnu.org. (Sun, 03 Mar 2019 02:08:05 GMT) Full text and rfc822 format available.

bug closed, send any further explanations to 33644 <at> debbugs.gnu.org and Kamil Dudka <kdudka <at> redhat.com> Request was from Pádraig Brady <P <at> draigBrady.com> to control <at> debbugs.gnu.org. (Sun, 03 Mar 2019 02:08:05 GMT) Full text and rfc822 format available.

Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Mon, 04 Mar 2019 09:38:02 GMT) Full text and rfc822 format available.

Message #27 received at 33644 <at> debbugs.gnu.org (full text, mbox):

From: Kamil Dudka <kdudka <at> redhat.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: bug-coreutils <at> gnu.org, 33644 <at> debbugs.gnu.org
Subject: Re: bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL
 extended attributes
Date: Mon, 04 Mar 2019 10:38:27 +0100
On Sunday, March 3, 2019 3:07:53 AM CET Pádraig Brady wrote:
> So attr_copy_file() copies all except those defined in /etc/xattr.conf

... which is, however, not how xattr.conf is currently documented:

    # /etc/xattr.conf
    #
    # Format:
    # <pattern> <action>
    #
    # Actions:
    #   permissions - copy when trying to preserve permissions.
    #   skip - do not copy.

> ACL xattrs are listed in that file with the rationale from a comment in
> libattr being:
> 
>  "ACLs are excluded by default because copying them between
>   file systems with and without ACL support needs some
>   additional logic so that no unexpected permissions result."
> 
> So the ACL handling specifically is deferred to libacl.
> Now system.posix_acl_access is handled by libacl,
> but system.nfs4_acl is not.

True.

> So I think the correct fix here is to remove the
> nfs entries from /etc/xattr.conf, and then cp will copy.

OK, I will propose it on the acl-devel mailing list, together with updating 
the documentation of xattr.conf.  We will see what attr/acl upstream thinks 
about it.

> This has the advantage of being configurable,
> and also removes nfs4 specific handling from cp.
> Any nfs4 specific handling should be in libacl.

So you think it should.  Nevertheless, in reality, libacl is not aware
of NFSv4 ACLs at all.  And I am not aware of any plans to change this.

Kamil

> thanks,
> Pádraig






Information forwarded to bug-coreutils <at> gnu.org:
bug#33644; Package coreutils. (Mon, 04 Mar 2019 09:44:01 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 01 Apr 2019 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 5 years and 26 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.