[Bug-tar] ACL support, extract and default ACLs

[Juan J. Martínez]

Hello,

I'm experiencing a possible problem with ACL support with the --acls
flag and tar 1.27:

when a tarball is extracted using --acls, a default ACL is created for
every file/dir extracted from the tarball (with no archived ACLs I think).

I've run some tests and I've found that:

- a tar without ACLs (using GNU tar formar -that doesn't support ACLs-)
- a POSIX tar without ACLs
- a POSIX tar with ACLs

...all of these cases add a default ACL when extracting files with no
archived ACLs.

I was expecting the stored ACLs to be restored when extracting contents,
but not the addition of that default ACLs that was not present in the
archived contents (see the case of a tarball in GNU tar format).

This behaviour causes problems when creating and restoring a tarball
with a complete filesystem because lots of applications/tools can't
handle ACLs properly.

I'd like to use the --acls flag to preserve *existing* ACLs when
extracting files/dirs and not add a default ACL.

Markus Steinborn wrote about ACL support and in his list of points that

Optional (3): Do not store ACLs iff ACL contains just the normal
user, group and other permissions, i. .e. the ACL is an compatibility
ACL.

(I wasn't subscribed to the list back then)

I don't know if it is related or not, I'd appreciate comments about this.

Regards,

Juan

[Juan J. Martínez]

Hello,

I'm experiencing a possible problem with ACL support with the --acls
flag and tar 1.27:

when a tarball is extracted using --acls, a default ACL is created for
every file/dir extracted from the tarball (with no archived ACLs I think).

I've run some tests and I've found that:

- a tar without ACLs (using GNU tar formar -that doesn't support ACLs-)
- a POSIX tar without ACLs
- a POSIX tar with ACLs

...all of these cases add a default ACL when extracting files with no
archived ACLs.

I was expecting the stored ACLs to be restored when extracting contents,
but not the addition of that default ACLs that was not present in the
archived contents (see the case of a tarball in GNU tar format).

This behaviour causes problems when creating and restoring a tarball
with a complete filesystem because lots of applications/tools can't
handle ACLs properly.

I'd like to use the --acls flag to preserve *existing* ACLs when
extracting files/dirs and not add a default ACL.

Markus Steinborn wrote about ACL support and in his list of points that

Optional (3): Do not store ACLs iff ACL contains just the normal
user, group and other permissions, i. .e. the ACL is an compatibility
ACL.

(I wasn't subscribed to the list back then)

I don't know if it is related or not, I'd appreciate comments about this.

Regards,

Juan

[Markus Steinborn]

Hi Juan,

Post by Juan J. Martínez
...all of these cases add a default ACL when extracting files with no
archived ACLs.

Good catch.

Post by Juan J. Martínez
I was expecting the stored ACLs to be restored when extracting contents,
but not the addition of that default ACLs that was not present in the
archived contents (see the case of a tarball in GNU tar format).

This is my expection, too.

Post by Juan J. Martínez
Markus Steinborn wrote about ACL support and in his list of points that

Optional (3): Do not store ACLs iff ACL contains just the normal
user, group and other permissions, i. .e. the ACL is an compatibility
ACL.

I don't know if it is related or not, I'd appreciate comments about this.

It is about creating archives, not about extracting. Currently I cannot
produce a case where such an ACL is stored in the tar archive. Well, if
the archive does not contain these trivial ACLs, the extractor can
decide, so this may help for extracting. But that's not the cause of
above problems.


Some patch like the following may help:


diff --git a/src/xattrs.c b/src/xattrs.c
index e669adb..b90ef7a 100644
--- a/src/xattrs.c
+++ b/src/xattrs.c
@@ -454,8 +454,19 @@ xattrs__acls_set (struct tar_stat_info const *st,
free(tmp);
acls_option = 1;
}
- else if (acls_option > 0)
+ else if (acls_option > 0 && !def)
acl = perms2acl (st->stat.st_mode);
+ else if (acls_option > 0 /* && def */ )
+ {
+ if (acl_delete_def_file(file_name) == -1)
+ {
+ WARNOPT (WARN_XATTR_WRITE,
+ (0, errno,
+ _ ("acl_set_file_at: Cannot remove POSIX default ACLs for
file '%s'"),
+ file_name));
+ }
+ return;
+ }
else
return; /* don't call acl functions unless we first hit an ACL, or
--acls was passed explicitly */






Greetings from Germany

Markus Steimnborn
GNU gv maintainer

[Joerg Schilling]

Post by Markus Steinborn
It is about creating archives, not about extracting. Currently I cannot
produce a case where such an ACL is stored in the tar archive. Well, if
the archive does not contain these trivial ACLs, the extractor can
decide, so this may help for extracting. But that's not the cause of
above problems.

I cannot verify this as GNU tar does not compile with ACL support, ... but

Post by Markus Steinborn
+ (0, errno,
+ _ ("acl_set_file_at: Cannot remove POSIX default ACLs for
file '%s'"),

There is no "POSIX ACL".

What you refer to is a POSIX draft that was based on the Solaris ACL
implementation that was available since 1992/1993, but this draft was
withdrawn in 1997.

Jörg

[Pavel Raiskup]

Good points, thanks! Default ACLs for directories are currently badly
restored which seems to be a clear bug. Could we push something similar
to attached files? (I try to propose testcase as a separate commit to
allow you `make check` without fix — to observe that the testcase actually
catches something).

OT: I'd like to mention also one forgotten patch:
http://www.mail-archive.com/bug-***@gnu.org/msg04052.html

Pavel

[Juan J. Martínez]

Post by Pavel Raiskup
Good points, thanks! Default ACLs for directories are currently badly
restored which seems to be a clear bug. Could we push something similar
to attached files? (I try to propose testcase as a separate commit to
allow you `make check` without fix — to observe that the testcase actually
catches something).

I applied the patches to the repository code and it fixes the behaviour
I was reporting (setting a default ACL when there's no archived ACL).

The patch and the test looks good to me.

Also I run `make check` with the following results:

161 tests behaved as expected.
20 tests were skipped.

Thanks a lot for the quick response.

Regards,

Juan

[Juan J. Martínez]

Post by Pavel Raiskup
Good points, thanks! Default ACLs for directories are currently badly
restored which seems to be a clear bug. Could we push something similar
to attached files? (I try to propose testcase as a separate commit to
allow you `make check` without fix — to observe that the testcase actually
catches something).

Hello,

Any update on this? Sergey?

I'm happy with Pavel's patch but I'd prefer if there was an upstream fix
so I can track tar releases in our production systems.

Regards,

Juan

[Sergey Poznyakoff]

Hi,

Post by Juan J. Martínez
Any update on this? Sergey?

Yes, I've pushed both patches. The acls03.at test contained a bug,
which I fixed as well.

Regards,
Sergey

[Juan J. Martínez]

Post by Sergey Poznyakoff
Hi,

Post by Juan J. Martínez
Any update on this? Sergey?

Yes, I've pushed both patches. The acls03.at test contained a bug,
which I fixed as well.

Great! Thanks a lot Sergey.

Regards,

Juan

[Joerg Schilling]

Post by Juan J. Martínez
I was expecting the stored ACLs to be restored when extracting contents,
but not the addition of that default ACLs that was not present in the
archived contents (see the case of a tarball in GNU tar format).

I recommend you to use star.

Star treats ACLs as people expect - since 12 years.

There are other known problems in the gnu tar implementation that have been
discussed here, e.g. not exluding linux kernel specific exattrs that just code
the ACLs.

Jörg

[Markus Steinborn]

That's actually the patch that makes my series of patches from
https://bugzilla.redhat.com/show_bug.cgi?id=1052876 (I mentioned them on
Jan 14th, 2014 in this list) to apply on tar 1.27.1 and current git master.

My Capabilities patch from the other redhat bug report will apply with
the above mentioned patches also on tar 1.17.1, but for git master it
needs trivial manual merging on configure.ac.


Greetings

Markus Steinborn