Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:59

Jeff King [off-list ref] writes:
On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
quoted
quoted
quoted
  setfacl -m m:rwx .
  perl -MFcntl -e 'sysopen(X, "a", O_WRONLY|O_CREAT, 0444)'
  umask 077
  perl -MFcntl -e 'sysopen(X, "b", O_WRONLY|O_CREAT, 0444)'
  getfacl a b
[...]
quoted
Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
clear that if a default ACL is present, it should be used, and umask
consulted only if it is not (so the umask should not be making a
difference in this case).

The reproduction recipe above shows the minimum required to trigger it;
adding a more realistic default ACL (with actual entries for users) does
not seem to make a difference.
Thanks; so combining the above with your earlier patch to 1304 we
would have a good detection for SETFACL prerequisite?
Yes, I think we can detect it reliably. I'd like to hear back from
ecryptfs folks before making a final patch, though. It may be that there
is some subtle reason for their behavior, and I want to make sure before
we write it off as just buggy.
Sensible; thanks.

[PATCH] t1304: improve setfacl prerequisite setup

From: Jeff King <hidden>
Date: 2016-06-15 22:53:59

t1304 first runs setfacl as an experiment to see whether the
filesystem supports ACLs, and skips the remaining tests if
it does not. However, our setfacl run did not exercise the
ACLs very well, and some filesystems may support our initial
setfacl, but not the rest of the test.

In particular, some versions of ecryptfs will erroneously
apply the umask on top of an inherited directory ACL,
causing our tests to fail. Let's be more careful and make
sure both that we can read back the user ACL we set, and
that the inherited ACL is propagated correctly. The latter
catches the ecryptfs bug, but may also catch other bugs
(e.g., an implementation which does not handle inherited
ACLs at all).

Since we're making the setup more complex, let's move it
into its own test. This will hide the output for us unless
the user wants to run "-v" to see it (and we don't need to
bother printing anything about setfacl failing; the
remaining tests will properly print "skip" due to the
missing prerequisite).

Signed-off-by: Jeff King <redacted>
---
The ecryptfs response was that it is probably a bug, so I think we
should go with this (especially because it is general enough to
potentially catch other weird errors).

 t/t1304-default-acl.sh | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 2b962cf..79045ab 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -14,16 +14,15 @@ umask 077
 # We need an arbitrary other user give permission to using ACLs. root
 # is a good candidate: exists on all unices, and it has permission
 # anyway, so we don't create a security hole running the testsuite.
-
-setfacl_out="$(setfacl -m u:root:rwx . 2>&1)"
-setfacl_ret=$?
-
-if test $setfacl_ret != 0
-then
-	say "Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')"
-else
-	test_set_prereq SETFACL
-fi
+test_expect_success 'checking for a working acl setup' '
+	if setfacl -m d:m:rwx -m u:root:rwx . &&
+	   getfacl . | grep user:root:rwx &&
+	   touch should-have-readable-acl &&
+	   getfacl should-have-readable-acl | egrep "mask::?rw-"
+	then
+		test_set_prereq SETFACL
+	fi
+'
 
 if test -z "$LOGNAME"
 then
-- 
1.7.11.rc1.4.g0d3b9b3

Re: [PATCH] t1304: improve setfacl prerequisite setup

From: Stefan Beller <hidden>
Date: 2016-06-15 22:54:00

Applying that patch, the test t1304 succeeds on my machine.

Tested-by: Stefan Beller <redacted>

2012/6/6 Jeff King [off-list ref]:
quoted hunk
t1304 first runs setfacl as an experiment to see whether the
filesystem supports ACLs, and skips the remaining tests if
it does not. However, our setfacl run did not exercise the
ACLs very well, and some filesystems may support our initial
setfacl, but not the rest of the test.

In particular, some versions of ecryptfs will erroneously
apply the umask on top of an inherited directory ACL,
causing our tests to fail. Let's be more careful and make
sure both that we can read back the user ACL we set, and
that the inherited ACL is propagated correctly. The latter
catches the ecryptfs bug, but may also catch other bugs
(e.g., an implementation which does not handle inherited
ACLs at all).

Since we're making the setup more complex, let's move it
into its own test. This will hide the output for us unless
the user wants to run "-v" to see it (and we don't need to
bother printing anything about setfacl failing; the
remaining tests will properly print "skip" due to the
missing prerequisite).

Signed-off-by: Jeff King <redacted>
---
The ecryptfs response was that it is probably a bug, so I think we
should go with this (especially because it is general enough to
potentially catch other weird errors).

 t/t1304-default-acl.sh | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 2b962cf..79045ab 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -14,16 +14,15 @@ umask 077
 # We need an arbitrary other user give permission to using ACLs. root
 # is a good candidate: exists on all unices, and it has permission
 # anyway, so we don't create a security hole running the testsuite.
-
-setfacl_out="$(setfacl -m u:root:rwx . 2>&1)"
-setfacl_ret=$?
-
-if test $setfacl_ret != 0
-then
-       say "Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')"
-else
-       test_set_prereq SETFACL
-fi
+test_expect_success 'checking for a working acl setup' '
+       if setfacl -m d:m:rwx -m u:root:rwx . &&
+          getfacl . | grep user:root:rwx &&
+          touch should-have-readable-acl &&
+          getfacl should-have-readable-acl | egrep "mask::?rw-"
+       then
+               test_set_prereq SETFACL
+       fi
+'

 if test -z "$LOGNAME"
 then
--
1.7.11.rc1.4.g0d3b9b3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help