[PATCH] tar: extend numeric-owner to ACL entries

[PATCH] tar: extend numeric-owner to ACL entries

[Piotr Łobacz]

ACL entries store references to numeric uids/gids. on platforms that
have libacl, use `acl_to_any_text` to generate ACL strings that preserve
those numeric identifiers if `numeric-owner` is set (instead of doing a
conversion to user/group name, like the acl_to_text function does).

this fixes the following broken scenario (and similar ones, where a
user/group of the stored name exists, but has a different numeric
identifier).

system A with user foo with uid 1001
system B with no user foo
file with ACL referencing uid 1001 on system A

on A:
$ echo 'bar' > file
$ setfacl -m u:foo:r file
$ tar --acls --xattrs --numeric-owner -cf test.tar file
$ tar -vv --acls --xattrs -tf test.tar

expected output:
-rw-r--r--+ 0/0         4 2022-01-26 14:32 file
  a: user::rw-,user:1001:r--,group::r--,mask::r--,other::r--

actual output:
-rw-r--r--+ 0/0         4 2022-01-26 14:32 file
  a: user::rw-,user:fakeuser:r--,group::r--,mask::r--,other::r--

on B:
$ tar --acls --xattrs -xf test.tar
$ getfacl -n file

expected output (extraction) - none
expected output (getfacl):
 # file: file
 # owner: 0
 # group: 0
 user::rw-
 user:1001:r--
 group::r--
 other::r--

actual output (extraction):
tar: file: Warning: Cannot acl_from_text: Invalid argument

actual output (getfacl) - note the missing user entry:
 # file: file
 # owner: 0
 # group: 0
 user::rw-
 group::r--
 other::r--

Fixes: [YOCTO #15099]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Piotr Łobacz <p.lobacz@welotec.com>
---
 ...-extend-numeric-owner-to-ACL-entries.patch | 113 ++++++++++++++++++
 meta/recipes-extended/tar/tar_1.34.bb         |   1 +
 2 files changed, 114 insertions(+)
 create mode 100644 meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch

diff --git a/meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch b/meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch
new file mode 100644
index 0000000000..9acce2e90a
--- /dev/null
+++ b/meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch
@@ -0,0 +1,113 @@
+From e95db1b5315957181c0255f6ca9607959abac4c3 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
+Date: Wed, 26 Jan 2022 14:54:58 +0100
+Subject: [PATCH] extend numeric-owner to ACL entries
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+ACL entries store references to numeric uids/gids. on platforms that
+have libacl, use `acl_to_any_text` to generate ACL strings that preserve
+those numeric identifiers if `numeric-owner` is set (instead of doing a
+conversion to user/group name, like the acl_to_text function does).
+
+this fixes the following broken scenario (and similar ones, where a
+user/group of the stored name exists, but has a different numeric
+identifier).
+
+system A with user foo with uid 1001
+system B with no user foo
+file with ACL referencing uid 1001 on system A
+
+on A:
+$ echo 'bar' > file
+$ setfacl -m u:foo:r file
+$ tar --acls --xattrs --numeric-owner -cf test.tar file
+$ tar -vv --acls --xattrs -tf test.tar
+
+expected output:
+-rw-r--r--+ 0/0         4 2022-01-26 14:32 file
+  a: user::rw-,user:1001:r--,group::r--,mask::r--,other::r--
+
+actual output:
+-rw-r--r--+ 0/0         4 2022-01-26 14:32 file
+  a: user::rw-,user:fakeuser:r--,group::r--,mask::r--,other::r--
+
+on B:
+$ tar --acls --xattrs -xf test.tar
+$ getfacl -n file
+
+expected output (extraction) - none
+expected output (getfacl):
+ # file: file
+ # owner: 0
+ # group: 0
+ user::rw-
+ user:1001:r--
+ group::r--
+ other::r--
+
+actual output (extraction):
+tar: file: Warning: Cannot acl_from_text: Invalid argument
+
+actual output (getfacl) - note the missing user entry:
+ # file: file
+ # owner: 0
+ # group: 0
+ user::rw-
+ group::r--
+ other::r--
+
+Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
+---
+ src/xattrs.c | 20 ++++++++++++++++++--
+ 1 file changed, 18 insertions(+), 2 deletions(-)
+
+diff --git a/src/xattrs.c b/src/xattrs.c
+index 7c00527c..b319dc68 100644
+--- a/src/xattrs.c
++++ b/src/xattrs.c
+@@ -130,6 +130,10 @@ static struct
+ #ifdef HAVE_POSIX_ACLS
+ # include "acl.h"
+ # include <sys/acl.h>
++#ifdef HAVE_ACL_LIBACL_H
++/* needed for numeric-owner support */
++# include <acl/libacl.h>
++#endif
+ #endif
+ 
+ #ifdef HAVE_POSIX_ACLS
+@@ -362,7 +366,13 @@ xattrs__acls_get_a (int parentfd, const char *file_name,
+       return;
+     }
+ 
+-  val = acl_to_text (acl, NULL);
++#ifdef HAVE_ACL_LIBACL_H
++  if (numeric_owner_option)
++    val = acl_to_any_text(acl, NULL, '\n', TEXT_SOME_EFFECTIVE | TEXT_NUMERIC_IDS);
++  else
++#endif
++    val = acl_to_text (acl, NULL);
++
+   acl_free (acl);
+ 
+   if (!val)
+@@ -392,7 +402,13 @@ xattrs__acls_get_d (int parentfd, char const *file_name,
+       return;
+     }
+ 
+-  val = acl_to_text (acl, NULL);
++#ifdef HAVE_ACL_LIBACL_H
++  if (numeric_owner_option)
++    val = acl_to_any_text(acl, NULL, '\n', TEXT_SOME_EFFECTIVE | TEXT_NUMERIC_IDS);
++  else
++    val = acl_to_text (acl, NULL);
++#endif
++
+   acl_free (acl);
+ 
+   if (!val)
+-- 
+2.30.2
+
diff --git a/meta/recipes-extended/tar/tar_1.34.bb b/meta/recipes-extended/tar/tar_1.34.bb
index 1ef5fe221e..bf117f600a 100644
--- a/meta/recipes-extended/tar/tar_1.34.bb
+++ b/meta/recipes-extended/tar/tar_1.34.bb
@@ -8,6 +8,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=d32239bcb673463ab874e80d47fae504"
 
 SRC_URI = "${GNU_MIRROR}/tar/tar-${PV}.tar.bz2 \
            file://CVE-2022-48303.patch \
+           file://0001-extend-numeric-owner-to-ACL-entries.patch \
 "
 
 SRC_URI[sha256sum] = "b44cc67f8a1f6b0250b7c860e952b37e8ed932a90bd9b1862a511079255646ff"
-- 
2.34.1

Re: [OE-core] [PATCH] tar: extend numeric-owner to ACL entries

[Alexandre Belloni]

Hello,

On 07/04/2023 14:58:46+0200, Piotr Łobacz wrote:
> ACL entries store references to numeric uids/gids. on platforms that
> have libacl, use `acl_to_any_text` to generate ACL strings that preserve
> those numeric identifiers if `numeric-owner` is set (instead of doing a
> conversion to user/group name, like the acl_to_text function does).
> 
> this fixes the following broken scenario (and similar ones, where a
> user/group of the stored name exists, but has a different numeric
> identifier).
> 
> system A with user foo with uid 1001
> system B with no user foo
> file with ACL referencing uid 1001 on system A
> 
> on A:
> $ echo 'bar' > file
> $ setfacl -m u:foo:r file
> $ tar --acls --xattrs --numeric-owner -cf test.tar file
> $ tar -vv --acls --xattrs -tf test.tar
> 
> expected output:
> -rw-r--r--+ 0/0         4 2022-01-26 14:32 file
>   a: user::rw-,user:1001:r--,group::r--,mask::r--,other::r--
> 
> actual output:
> -rw-r--r--+ 0/0         4 2022-01-26 14:32 file
>   a: user::rw-,user:fakeuser:r--,group::r--,mask::r--,other::r--
> 
> on B:
> $ tar --acls --xattrs -xf test.tar
> $ getfacl -n file
> 
> expected output (extraction) - none
> expected output (getfacl):
>  # file: file
>  # owner: 0
>  # group: 0
>  user::rw-
>  user:1001:r--
>  group::r--
>  other::r--
> 
> actual output (extraction):
> tar: file: Warning: Cannot acl_from_text: Invalid argument
> 
> actual output (getfacl) - note the missing user entry:
>  # file: file
>  # owner: 0
>  # group: 0
>  user::rw-
>  group::r--
>  other::r--
> 
> Fixes: [YOCTO #15099]
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Piotr Łobacz <p.lobacz@welotec.com>
> ---
>  ...-extend-numeric-owner-to-ACL-entries.patch | 113 ++++++++++++++++++
>  meta/recipes-extended/tar/tar_1.34.bb         |   1 +
>  2 files changed, 114 insertions(+)
>  create mode 100644 meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch
> 
> diff --git a/meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch b/meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch
> new file mode 100644
> index 0000000000..9acce2e90a
> --- /dev/null
> +++ b/meta/recipes-extended/tar/tar/0001-extend-numeric-owner-to-ACL-entries.patch
> @@ -0,0 +1,113 @@
> +From e95db1b5315957181c0255f6ca9607959abac4c3 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
> +Date: Wed, 26 Jan 2022 14:54:58 +0100
> +Subject: [PATCH] extend numeric-owner to ACL entries
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +ACL entries store references to numeric uids/gids. on platforms that
> +have libacl, use `acl_to_any_text` to generate ACL strings that preserve
> +those numeric identifiers if `numeric-owner` is set (instead of doing a
> +conversion to user/group name, like the acl_to_text function does).
> +
> +this fixes the following broken scenario (and similar ones, where a
> +user/group of the stored name exists, but has a different numeric
> +identifier).
> +
> +system A with user foo with uid 1001
> +system B with no user foo
> +file with ACL referencing uid 1001 on system A
> +
> +on A:
> +$ echo 'bar' > file
> +$ setfacl -m u:foo:r file
> +$ tar --acls --xattrs --numeric-owner -cf test.tar file
> +$ tar -vv --acls --xattrs -tf test.tar
> +
> +expected output:
> +-rw-r--r--+ 0/0         4 2022-01-26 14:32 file
> +  a: user::rw-,user:1001:r--,group::r--,mask::r--,other::r--
> +
> +actual output:
> +-rw-r--r--+ 0/0         4 2022-01-26 14:32 file
> +  a: user::rw-,user:fakeuser:r--,group::r--,mask::r--,other::r--
> +
> +on B:
> +$ tar --acls --xattrs -xf test.tar
> +$ getfacl -n file
> +
> +expected output (extraction) - none
> +expected output (getfacl):
> + # file: file
> + # owner: 0
> + # group: 0
> + user::rw-
> + user:1001:r--
> + group::r--
> + other::r--
> +
> +actual output (extraction):
> +tar: file: Warning: Cannot acl_from_text: Invalid argument
> +
> +actual output (getfacl) - note the missing user entry:
> + # file: file
> + # owner: 0
> + # group: 0
> + user::rw-
> + group::r--
> + other::r--
> +

This patch is missing the Upstream-Status tag here

> +Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> +---
> + src/xattrs.c | 20 ++++++++++++++++++--
> + 1 file changed, 18 insertions(+), 2 deletions(-)
> +
> +diff --git a/src/xattrs.c b/src/xattrs.c
> +index 7c00527c..b319dc68 100644
> +--- a/src/xattrs.c
> ++++ b/src/xattrs.c
> +@@ -130,6 +130,10 @@ static struct
> + #ifdef HAVE_POSIX_ACLS
> + # include "acl.h"
> + # include <sys/acl.h>
> ++#ifdef HAVE_ACL_LIBACL_H
> ++/* needed for numeric-owner support */
> ++# include <acl/libacl.h>
> ++#endif
> + #endif
> + 
> + #ifdef HAVE_POSIX_ACLS
> +@@ -362,7 +366,13 @@ xattrs__acls_get_a (int parentfd, const char *file_name,
> +       return;
> +     }
> + 
> +-  val = acl_to_text (acl, NULL);
> ++#ifdef HAVE_ACL_LIBACL_H
> ++  if (numeric_owner_option)
> ++    val = acl_to_any_text(acl, NULL, '\n', TEXT_SOME_EFFECTIVE | TEXT_NUMERIC_IDS);
> ++  else
> ++#endif
> ++    val = acl_to_text (acl, NULL);
> ++
> +   acl_free (acl);
> + 
> +   if (!val)
> +@@ -392,7 +402,13 @@ xattrs__acls_get_d (int parentfd, char const *file_name,
> +       return;
> +     }
> + 
> +-  val = acl_to_text (acl, NULL);
> ++#ifdef HAVE_ACL_LIBACL_H
> ++  if (numeric_owner_option)
> ++    val = acl_to_any_text(acl, NULL, '\n', TEXT_SOME_EFFECTIVE | TEXT_NUMERIC_IDS);
> ++  else
> ++    val = acl_to_text (acl, NULL);
> ++#endif
> ++
> +   acl_free (acl);
> + 
> +   if (!val)
> +-- 
> +2.30.2
> +
> diff --git a/meta/recipes-extended/tar/tar_1.34.bb b/meta/recipes-extended/tar/tar_1.34.bb
> index 1ef5fe221e..bf117f600a 100644
> --- a/meta/recipes-extended/tar/tar_1.34.bb
> +++ b/meta/recipes-extended/tar/tar_1.34.bb
> @@ -8,6 +8,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=d32239bcb673463ab874e80d47fae504"
>  
>  SRC_URI = "${GNU_MIRROR}/tar/tar-${PV}.tar.bz2 \
>             file://CVE-2022-48303.patch \
> +           file://0001-extend-numeric-owner-to-ACL-entries.patch \
>  "
>  
>  SRC_URI[sha256sum] = "b44cc67f8a1f6b0250b7c860e952b37e8ed932a90bd9b1862a511079255646ff"
> -- 
> 2.34.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#179818): https://lists.openembedded.org/g/openembedded-core/message/179818
> Mute This Topic: https://lists.openembedded.org/mt/98123758/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

ODP: [OE-core] [PATCH] tar: extend numeric-owner to ACL entries

[Piotr Łobacz]

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

Hi Fabien,
That was my intention to leave you as an author as you have found and fixed this issue 🙂

Btw. this patch saved my time for applying stupid solutions...
Additionaly I have also created a bug report on that in ubuntu https://bugs.launchpad.net/ubuntu/+source/tar/+bug/2015539

BR
Piotr
________________________________
Od: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Wysłane: wtorek, 11 kwietnia 2023 09:44
Do: Alexandre Belloni <alexandre.belloni@bootlin.com>; Piotr Łobacz <p.lobacz@welotec.com>
DW: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org>
Temat: Re: [OE-core] [PATCH] tar: extend numeric-owner to ACL entries

On April 9, 2023 1:53 am, Alexandre Belloni wrote:
> Hello,
>
> On 07/04/2023 14:58:46+0200, Piotr Łobacz wrote:
>> ACL entries store references to numeric uids/gids. on platforms that
>> have libacl, use `acl_to_any_text` to generate ACL strings that preserve
>> those numeric identifiers if `numeric-owner` is set (instead of doing a
>> conversion to user/group name, like the acl_to_text function does).
>>
>> [..]
>>
>> +
>> +actual output (getfacl) - note the missing user entry:
>> + # file: file
>> + # owner: 0
>> + # group: 0
>> + user::rw-
>> + group::r--
>> + other::r--
>> +
>
> This patch is missing the Upstream-Status tag here
>
>> +Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

(Original patch author, thanks for the CC!)

FWIW, I got no response from upstream or the Debian tar maintainers[0].

But nice to see that other distros are interested as well :) maybe we
could try to ping the upstream report or sending the patch to some
relevant devel list?

The bug was not important enough for us to justify carrying our own,
custom tar packages, so we'd ideally still want to see this upstreamed.

0: https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.debian.org%2Fcgi-bin%2Fbugreport.cgi%3Fbug%3D1004430&data=05%7C01%7Cp.lobacz%40welotec.com%7Cd5846947d807488e650108db3a60b11e%7C25111a7f1d5a4c51a4ca7f8e44011b39%7C0%7C0%7C638167959195524735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=xYvcBDzx40TfoeX6dE2HQuj3tpP1u2SyX19WNqRWd5Q%3D&reserved=0<https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004430>


[-- Attachment #2: Type: text/html, Size: 4862 bytes --]