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

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

[Stefan Beller]

Hello,

so I just pulled the new git v1.7.10.4 and tried to test it with
> make test
This yields this output:
stderr http://pastebin.com/V8yuZFfi
stdout http://dl.dropbox.com/u/6520164/git/maketest.txt

In Test 1304 there is
not ok - 2 Objects creation does not break ACLs with restrictive umask

I am running Ubuntu 12.04 with Linux sb 3.2.0-25-generic #40-Ubuntu
SMP Wed May 23 20:30:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

Best Regards,
Stefan Beller

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

[Junio C Hamano]

Stefan Beller <stefanbeller@googlemail.com> writes:

> so I just pulled the new git v1.7.10.4 and tried to test it with
>> make test
> This yields this output:
> stderr http://pastebin.com/V8yuZFfi
> stdout http://dl.dropbox.com/u/6520164/git/maketest.txt
>
> In Test 1304 there is
> not ok - 2 Objects creation does not break ACLs with restrictive umask
>
> I am running Ubuntu 12.04 with Linux sb 3.2.0-25-generic #40-Ubuntu
> SMP Wed May 23 20:30:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

Interesting. I've seen t1304 break from time to time.

I set the DEFAULT_TEST_TARGET to "prove", and when the test suite
finishes with this failure, I noticed t3600-rm and some other test I
do not recall also failed, with two extra files (actual and expect)
at the root of the TEST_OUTPUT_DIRECTORY (set to /dev/shm/testpen
via "--root=/dev/shm/testpen" option).

I the breakage does not happen reproducibly with any pattern other
than the above (I do not know if it never happens when test target
is set to "test", for example), so haven't looked (and will not
look) into it myself further than that.

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

[Junio C Hamano]

Stefan Beller <stefanbeller@googlemail.com> writes:

> I tried debugging into it:
> In git/t/t1304-default-acl.sh there is:
> check_perms_and_acl () {
>     test -r "$1" &&
>     getfacl "$1" > actual &&
>     grep -q "user:root:rwx" actual &&
>     grep -q "user:${LOGNAME}:rwx" actual &&
>     egrep "mask::?r--" actual > /dev/null 2>&1 &&
>     grep -q "group::---" actual || false
> }
>
> but when I do all the commands as in the test2:
> test_expect_success SETFACL 'Objects creation does not break ACLs with
> restrictive umask' '
>     # SHA1 for empty blob
>     check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
> '
> Now I run the second line in check_perms_and_acl ()  with
> .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
> there is
>
> getfacl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
> # file: .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
> # owner: sb
> # group: sb
> user::r--
> user:root:rwx			#effective:---
> user:sb:rwx			#effective:---
> group::---
> mask::---
> other::---
>
> This command seem to fail 'egrep "mask::?r--" actual' as there is
> mask::---
> but expected is
> mask::r--
>
> That's my understanding of the test case so far.

Hmph.  Running the test manually does not seem to fail for me and
gives the "r--" mask.

I am lost.  Matthieu, you introduced this as a failing test with
7aba618 (Add a testcase for ACL with restrictive umask.,
2010-02-22), as part of the series leading to 5256b00 (Use
git_mkstemp_mode instead of plain mkstemp to create object files,
2010-02-22).  Any ideas (other than "Your filesystem is broken",
that is)?  As far as I can tell, with 'mask::---', these specific
users who are given permissions to read from the objects wouldn't be
able to read from them, so...

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

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Stefan Beller <stefanbeller@googlemail.com> writes:
>
>> I tried debugging into it:
>> In git/t/t1304-default-acl.sh there is:
>> check_perms_and_acl () {
>>     test -r "$1" &&
>>     getfacl "$1" > actual &&
>>     grep -q "user:root:rwx" actual &&
>>     grep -q "user:${LOGNAME}:rwx" actual &&
>>     egrep "mask::?r--" actual > /dev/null 2>&1 &&
>>     grep -q "group::---" actual || false
>> }

Can you run it with --verbose and post the result?

> Any ideas (other than "Your filesystem is broken",
> that is)?

I'm very tempted to go for the "Your filesystem is broken" indeed.

> As far as I can tell, with 'mask::---', these specific users who are
> given permissions to read from the objects wouldn't be able to read
> from them, so...

That's my understanding too.

Stefan, which filesystem are you using in the directory where you run
tests (type "mount" if you don't know)? Are you running on a virtual
machine?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Stefan Beller]

Hi,

sb@sb:~/OSS/git/t$ ./t1304-default-acl.sh --verbose
Initialized empty Git repository in /home/sb/OSS/git/t/trash
directory.t1304-default-acl/.git/
expecting success:
    setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&
    setfacl -m m:rwx               $dirs_to_set &&
    setfacl -m u:root:rwx          $dirs_to_set &&
    setfacl -m d:u:"${LOGNAME}":rwx  $dirs_to_set &&
    setfacl -m d:u:root:rwx        $dirs_to_set &&

    touch file.txt &&
    git add file.txt &&
    git commit -m "init"

[master (root-commit) 47f54f4] init
 Author: A U Thor <author@example.com>
 0 files changed
 create mode 100644 file.txt
ok 1 - Setup test repo

expecting success:
    # SHA1 for empty blob
    check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391

/home/sb/OSS/git/t/trash directory.t1304-default-acl
not ok - 2 Objects creation does not break ACLs with restrictive umask
#	
#	    # SHA1 for empty blob
#	    check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
#	

expecting success:
    git gc &&
    check_perms_and_acl .git/objects/pack/*.pack

Counting objects: 3, done.
Writing objects: 100% (3/3), done.
Total 3 (delta 0), reused 0 (delta 0)
/home/sb/OSS/git/t/trash directory.t1304-default-acl
not ok - 3 git gc does not break ACLs with restrictive umask
#	
#	    git gc &&
#	    check_perms_and_acl .git/objects/pack/*.pack
#	

# failed 2 among 3 test(s)
1..3
sb@sb:~/OSS/git/t$ cat trash\ directory.t1304-default-acl/actual
# file: .git/objects/pack/pack-ee77696bcc9be7ef581005ee3706bc17fcba376d.pack
# owner: sb
# group: sb
user::r--
user:root:rwx	#effective:---
user:sb:rwx	#effective:---
group::---
mask::---
other::---

sb@sb:~/OSS/git/t$ mount
/dev/sda3 on / type ext4 (rw,errors=remount-ro)
proc on /proc type proc (rw,noexec,nosuid,nodev)
sysfs on /sys type sysfs (rw,noexec,nosuid,nodev)
none on /sys/fs/fuse/connections type fusectl (rw)
none on /sys/kernel/debug type debugfs (rw)
none on /sys/kernel/security type securityfs (rw)
udev on /dev type devtmpfs (rw,mode=0755)
devpts on /dev/pts type devpts (rw,noexec,nosuid,gid=5,mode=0620)
tmpfs on /run type tmpfs (rw,noexec,nosuid,size=10%,mode=0755)
none on /run/lock type tmpfs (rw,noexec,nosuid,nodev,size=5242880)
none on /run/shm type tmpfs (rw,nosuid,nodev)
/dev/sda4 on /home type ext4 (rw)
binfmt_misc on /proc/sys/fs/binfmt_misc type binfmt_misc
(rw,noexec,nosuid,nodev)
/home/sb/.Private on /home/sb type ecryptfs
(ecryptfs_check_dev_ruid,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs,ecryptfs_sig=3b9d213e5c5d5780,ecryptfs_fnek_sig=77bec2da523c0338)
gvfs-fuse-daemon on /home/sb/.gvfs type fuse.gvfs-fuse-daemon
(rw,nosuid,nodev,user=sb)


So I am using /dev/sda4 on /home type ext4 (rw), but my user account
has its home directory encrypted via ecryptfs.
This is not run inside a virtual machine, it's a native computer.



2012/6/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Stefan Beller <stefanbeller@googlemail.com> writes:
>>
>>> I tried debugging into it:
>>> In git/t/t1304-default-acl.sh there is:
>>> check_perms_and_acl () {
>>>     test -r "$1" &&
>>>     getfacl "$1" > actual &&
>>>     grep -q "user:root:rwx" actual &&
>>>     grep -q "user:${LOGNAME}:rwx" actual &&
>>>     egrep "mask::?r--" actual > /dev/null 2>&1 &&
>>>     grep -q "group::---" actual || false
>>> }
>
> Can you run it with --verbose and post the result?
>
>> Any ideas (other than "Your filesystem is broken",
>> that is)?
>
> I'm very tempted to go for the "Your filesystem is broken" indeed.
>
>> As far as I can tell, with 'mask::---', these specific users who are
>> given permissions to read from the objects wouldn't be able to read
>> from them, so...
>
> That's my understanding too.
>
> Stefan, which filesystem are you using in the directory where you run
> tests (type "mount" if you don't know)? Are you running on a virtual
> machine?
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

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

[Stefan Beller]

Just did a short google search and found this:
http://serverfault.com/questions/294158/enable-acl-for-ecryptfs-mounted-home-directory

So I am convinced my filesystem is just broken as you said ;)

2012/6/5 Stefan Beller <stefanbeller@googlemail.com>:
> Hi,
>
> sb@sb:~/OSS/git/t$ ./t1304-default-acl.sh --verbose
> Initialized empty Git repository in /home/sb/OSS/git/t/trash
> directory.t1304-default-acl/.git/
> expecting success:
>    setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&
>    setfacl -m m:rwx               $dirs_to_set &&
>    setfacl -m u:root:rwx          $dirs_to_set &&
>    setfacl -m d:u:"${LOGNAME}":rwx  $dirs_to_set &&
>    setfacl -m d:u:root:rwx        $dirs_to_set &&
>
>    touch file.txt &&
>    git add file.txt &&
>    git commit -m "init"
>
> [master (root-commit) 47f54f4] init
>  Author: A U Thor <author@example.com>
>  0 files changed
>  create mode 100644 file.txt
> ok 1 - Setup test repo
>
> expecting success:
>    # SHA1 for empty blob
>    check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
>
> /home/sb/OSS/git/t/trash directory.t1304-default-acl
> not ok - 2 Objects creation does not break ACLs with restrictive umask
> #
> #           # SHA1 for empty blob
> #           check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
> #
>
> expecting success:
>    git gc &&
>    check_perms_and_acl .git/objects/pack/*.pack
>
> Counting objects: 3, done.
> Writing objects: 100% (3/3), done.
> Total 3 (delta 0), reused 0 (delta 0)
> /home/sb/OSS/git/t/trash directory.t1304-default-acl
> not ok - 3 git gc does not break ACLs with restrictive umask
> #
> #           git gc &&
> #           check_perms_and_acl .git/objects/pack/*.pack
> #
>
> # failed 2 among 3 test(s)
> 1..3
> sb@sb:~/OSS/git/t$ cat trash\ directory.t1304-default-acl/actual
> # file: .git/objects/pack/pack-ee77696bcc9be7ef581005ee3706bc17fcba376d.pack
> # owner: sb
> # group: sb
> user::r--
> user:root:rwx   #effective:---
> user:sb:rwx     #effective:---
> group::---
> mask::---
> other::---
>
> sb@sb:~/OSS/git/t$ mount
> /dev/sda3 on / type ext4 (rw,errors=remount-ro)
> proc on /proc type proc (rw,noexec,nosuid,nodev)
> sysfs on /sys type sysfs (rw,noexec,nosuid,nodev)
> none on /sys/fs/fuse/connections type fusectl (rw)
> none on /sys/kernel/debug type debugfs (rw)
> none on /sys/kernel/security type securityfs (rw)
> udev on /dev type devtmpfs (rw,mode=0755)
> devpts on /dev/pts type devpts (rw,noexec,nosuid,gid=5,mode=0620)
> tmpfs on /run type tmpfs (rw,noexec,nosuid,size=10%,mode=0755)
> none on /run/lock type tmpfs (rw,noexec,nosuid,nodev,size=5242880)
> none on /run/shm type tmpfs (rw,nosuid,nodev)
> /dev/sda4 on /home type ext4 (rw)
> binfmt_misc on /proc/sys/fs/binfmt_misc type binfmt_misc
> (rw,noexec,nosuid,nodev)
> /home/sb/.Private on /home/sb type ecryptfs
> (ecryptfs_check_dev_ruid,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs,ecryptfs_sig=3b9d213e5c5d5780,ecryptfs_fnek_sig=77bec2da523c0338)
> gvfs-fuse-daemon on /home/sb/.gvfs type fuse.gvfs-fuse-daemon
> (rw,nosuid,nodev,user=sb)
>
>
> So I am using /dev/sda4 on /home type ext4 (rw), but my user account
> has its home directory encrypted via ecryptfs.
> This is not run inside a virtual machine, it's a native computer.
>
>
>
> 2012/6/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Stefan Beller <stefanbeller@googlemail.com> writes:
>>>
>>>> I tried debugging into it:
>>>> In git/t/t1304-default-acl.sh there is:
>>>> check_perms_and_acl () {
>>>>     test -r "$1" &&
>>>>     getfacl "$1" > actual &&
>>>>     grep -q "user:root:rwx" actual &&
>>>>     grep -q "user:${LOGNAME}:rwx" actual &&
>>>>     egrep "mask::?r--" actual > /dev/null 2>&1 &&
>>>>     grep -q "group::---" actual || false
>>>> }
>>
>> Can you run it with --verbose and post the result?
>>
>>> Any ideas (other than "Your filesystem is broken",
>>> that is)?
>>
>> I'm very tempted to go for the "Your filesystem is broken" indeed.
>>
>>> As far as I can tell, with 'mask::---', these specific users who are
>>> given permissions to read from the objects wouldn't be able to read
>>> from them, so...
>>
>> That's my understanding too.
>>
>> Stefan, which filesystem are you using in the directory where you run
>> tests (type "mount" if you don't know)? Are you running on a virtual
>> machine?
>>
>> --
>> Matthieu Moy
>> http://www-verimag.imag.fr/~moy/

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

[Jeff King]

On Tue, Jun 05, 2012 at 09:23:47AM +0200, Stefan Beller wrote:

> So I am using /dev/sda4 on /home type ext4 (rw), but my user account
> has its home directory encrypted via ecryptfs.

It's very likely that ecryptfs is the problem, then. Googling around, I
find conflicting reports on whether it actually supports ACLs or not.

At the top of the test script we check that "setfacl" doesn't report an
error, and assume that ACLs work if it doesn't. Maybe that test needs to
be more comprehensive, like the patch below. What happens when you run
t1304 with this patch (my expectation is that it will just skip the acl
tests)?

-- >8 --
Subject: t1304: improve setfacl prerequisite setup

Some filesystems will cause "setfacl" to report success, even
though they do not seem to behave sanely enough for our
tests. Let's make sure that not only does setfacl work, but
that we can read back the result.

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 <peff@peff.net>
---
 t/t1304-default-acl.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 2b962cf..be954f0 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -14,16 +14,13 @@ 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 u:root:rwx . &&
+	   getfacl . | grep user:root:rwx
+	then
+		test_set_prereq SETFACL
+	fi
+'
 
 if test -z "$LOGNAME"
 then

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

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> Subject: t1304: improve setfacl prerequisite setup

> +test_expect_success 'checking for a working acl setup' '
> +	if setfacl -m u:root:rwx . &&
> +	   getfacl . | grep user:root:rwx
> +	then
> +		test_set_prereq SETFACL
> +	fi
> +'

Excellent. Thanks.

Tested-by: Matthieu Moy <Matthieu.Moy@imag.fr>

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Stefan Beller]

Hi,

so I applied that patch with git apply, but here 2 out of 4 tests now
still fail.


sb@sb:~/OSS/git/t$ cat t1304-default-acl.sh
#!/bin/sh
#
# Copyright (c) 2010 Matthieu Moy
#

test_description='Test repository with default ACL'

# Create the test repo with restrictive umask
# => this must come before . ./test-lib.sh
umask 077

. ./test-lib.sh

# 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.
test_expect_success 'checking for a working acl setup' '
       if setfacl -m u:root:rwx . &&
          getfacl . | grep user:root:rwx
       then
               test_set_prereq SETFACL
       fi
'

if test -z "$LOGNAME"
then
	LOGNAME=$USER
fi

check_perms_and_acl () {
	test -r "$1" &&
	getfacl "$1" > actual &&
	grep -q "user:root:rwx" actual &&
	grep -q "user:${LOGNAME}:rwx" actual &&
	egrep "mask::?r--" actual > /dev/null 2>&1 &&
	grep -q "group::---" actual || false
}

dirs_to_set="./ .git/ .git/objects/ .git/objects/pack/"

test_expect_success SETFACL 'Setup test repo' '
	setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&
	setfacl -m m:rwx               $dirs_to_set &&
	setfacl -m u:root:rwx          $dirs_to_set &&
	setfacl -m d:u:"$LOGNAME":rwx  $dirs_to_set &&
	setfacl -m d:u:root:rwx        $dirs_to_set &&

	touch file.txt &&
	git add file.txt &&
	git commit -m "init"
'

test_expect_success SETFACL 'Objects creation does not break ACLs with
restrictive umask' '
	# SHA1 for empty blob
	check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
'

test_expect_success SETFACL 'git gc does not break ACLs with
restrictive umask' '
	git gc &&
	check_perms_and_acl .git/objects/pack/*.pack
'

test_done
sb@sb:~/OSS/git/t$ ./t1304-default-acl.sh --verbose
Initialized empty Git repository in /home/sb/OSS/git/t/trash
directory.t1304-default-acl/.git/
expecting success:
       if setfacl -m u:root:rwx . &&
          getfacl . | grep user:root:rwx
       then
               test_set_prereq SETFACL
       fi

user:root:rwx
ok 1 - checking for a working acl setup

expecting success:
	setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&
	setfacl -m m:rwx               $dirs_to_set &&
	setfacl -m u:root:rwx          $dirs_to_set &&
	setfacl -m d:u:"$LOGNAME":rwx  $dirs_to_set &&
	setfacl -m d:u:root:rwx        $dirs_to_set &&

	touch file.txt &&
	git add file.txt &&
	git commit -m "init"

[master (root-commit) 011f91a] init
 Author: A U Thor <author@example.com>
 0 files changed
 create mode 100644 file.txt
ok 2 - Setup test repo

expecting success:
	# SHA1 for empty blob
	check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391

not ok - 3 Objects creation does not break ACLs with restrictive umask
#	
#		# SHA1 for empty blob
#		check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
#	

expecting success:
	git gc &&
	check_perms_and_acl .git/objects/pack/*.pack

Counting objects: 3, done.
Writing objects: 100% (3/3), done.
Total 3 (delta 0), reused 0 (delta 0)
not ok - 4 git gc does not break ACLs with restrictive umask
#	
#		git gc &&
#		check_perms_and_acl .git/objects/pack/*.pack
#	

# failed 2 among 4 test(s)
1..4

2012/6/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Jeff King <peff@peff.net> writes:
>
>> Subject: t1304: improve setfacl prerequisite setup
>
>> +test_expect_success 'checking for a working acl setup' '
>> +     if setfacl -m u:root:rwx . &&
>> +        getfacl . | grep user:root:rwx
>> +     then
>> +             test_set_prereq SETFACL
>> +     fi
>> +'
>
> Excellent. Thanks.
>
> Tested-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

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

[Matthieu Moy]

Stefan Beller <stefanbeller@googlemail.com> writes:

> Hi,
>
> so I applied that patch with git apply, but here 2 out of 4 tests now
> still fail.

Yes, Jeff's patch checks for a "working" setfacl/getfacl, but does not
check for subtle brokenness of it.

I'd say a filesystem that accepts setfacl, returns the right line in
getfacl, and then does not really work is broken, and it's a feature
that the testsuite shows test failures. A simple fix for you would be to
disable ACLs, either on your system or in your $HOME.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Jeff King]

On Tue, Jun 05, 2012 at 03:28:07PM +0200, Matthieu Moy wrote:

> Stefan Beller <stefanbeller@googlemail.com> writes:
> 
> > Hi,
> >
> > so I applied that patch with git apply, but here 2 out of 4 tests now
> > still fail.
> 
> Yes, Jeff's patch checks for a "working" setfacl/getfacl, but does not
> check for subtle brokenness of it.
> 
> I'd say a filesystem that accepts setfacl, returns the right line in
> getfacl, and then does not really work is broken, and it's a feature
> that the testsuite shows test failures. A simple fix for you would be to
> disable ACLs, either on your system or in your $HOME.

I found the problem. If you create a file on an ecryptfs filesystem with
a restrictive mode, then the inherited mask bits get cleared. For
example:

  # if you don't have one already, create an ecryptfs mount; you
  # will need the ecryptfs-utils package on Debian and Ubuntu
  ecryptfs-setup-private
  ecryptfs-mount-private
  cd Private

  # now set up an inherited mask
  setfacl -m d:m:rwx .

  # and create some files with different permissions
  perl -MFcntl -e '
    sysopen(X, "a", O_WRONLY|O_CREAT, 0666);
    sysopen(X, "b", O_WRONLY|O_CREAT, 0600);
  '

  # and then check the permissions
  getfacl a b

which yields:

  # file: a
  # owner: peff
  # group: peff
  user::rw-
  group::---
  mask::r--
  other::---

  # file: b
  # owner: peff
  # group: peff
  user::rw-
  group::---
  mask::---
  other::---

That just seems wrong to me.

-Peff

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

[Jeff King]

On Tue, Jun 05, 2012 at 10:04:49AM -0400, Jeff King wrote:

>   # and create some files with different permissions
>   perl -MFcntl -e '
>     sysopen(X, "a", O_WRONLY|O_CREAT, 0666);
>     sysopen(X, "b", O_WRONLY|O_CREAT, 0600);
>   '
> 
>   # and then check the permissions
>   getfacl a b
> 
> which yields:
> 
>   # file: a
>   # owner: peff
>   # group: peff
>   user::rw-
>   group::---
>   mask::r--
>   other::---
> 
>   # file: b
>   # owner: peff
>   # group: peff
>   user::rw-
>   group::---
>   mask::---
>   other::---
> 
> That just seems wrong to me.

Hmm, no I'm wrong. That is OK, as the mask should be restricted by the
group permission (and it is on ext4, too). But when I traced through
git, the tmp_obj_* files are created with the right permissions on ext4,
but not on ecryptfs. Let me trace through it again...<sigh>

-Peff

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

[Jeff King]

On Tue, Jun 05, 2012 at 10:10:39AM -0400, Jeff King wrote:

> On Tue, Jun 05, 2012 at 10:04:49AM -0400, Jeff King wrote:
> 
> >   # and create some files with different permissions
> >   perl -MFcntl -e '
> >     sysopen(X, "a", O_WRONLY|O_CREAT, 0666);
> >     sysopen(X, "b", O_WRONLY|O_CREAT, 0600);
> >   '
> Hmm, no I'm wrong. That is OK, as the mask should be restricted by the
> group permission (and it is on ext4, too). But when I traced through
> git, the tmp_obj_* files are created with the right permissions on ext4,
> but not on ecryptfs. Let me trace through it again...<sigh>

OK, here's the _real_ issue. Git creates with mode 0444, which should
still allow read in the mask. But it's the restrictive umask at the top
of the test script that causes the problem. Try this:

  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

On ext4, both files will have the read bit set in the mask. On ecryptfs,
"b" will have an empty mask. I think the wrong thing is that we should
not be respecting umask at all when default ACLs are in play, and
ecryptfs is getting that wrong. But I'm having trouble digging up an
authoritative source.

-Peff

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

[Jeff King]

[+cc ecryptfs@vger, as I think this is an ecryptfs bug]

On Tue, Jun 05, 2012 at 10:28:13AM -0400, Jeff King wrote:

> OK, here's the _real_ issue. Git creates with mode 0444, which should
> still allow read in the mask. But it's the restrictive umask at the top
> of the test script that causes the problem. Try this:
> 
>   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
> 
> On ext4, both files will have the read bit set in the mask. On ecryptfs,
> "b" will have an empty mask. I think the wrong thing is that we should
> not be respecting umask at all when default ACLs are in play, and
> ecryptfs is getting that wrong. But I'm having trouble digging up an
> authoritative source.

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.

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> [+cc ecryptfs@vger, as I think this is an ecryptfs bug]
>
> On Tue, Jun 05, 2012 at 10:28:13AM -0400, Jeff King wrote:
>
>> OK, here's the _real_ issue. Git creates with mode 0444, which should
>> still allow read in the mask. But it's the restrictive umask at the top
>> of the test script that causes the problem. Try this:
>> 
>>   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
>> 
>> On ext4, both files will have the read bit set in the mask. On ecryptfs,
>> "b" will have an empty mask. I think the wrong thing is that we should
>> not be respecting umask at all when default ACLs are in play, and
>> ecryptfs is getting that wrong. But I'm having trouble digging up an
>> authoritative source.
>
> 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?
 

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

[Jeff King]

On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:

> >>   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
> [...]
> >
> > 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.

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
>
>> >>   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
>> [...]
>> >
>> > 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

[Jeff King]

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 <peff@peff.net>
---
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

[Stefan Beller]

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

Tested-by: Stefan Beller <stefanbeller@googlemail.com>

2012/6/6 Jeff King <peff@peff.net>:
> 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 <peff@peff.net>
> ---
> 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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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 <peff@peff.net>
> ---
> 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-"

At this point of the test, I do not think there is anything that can
cause this check to be fooled by a user whose name is 'mask', but
please make it a habit to anchor the pattern at the left.  The tests
done in check_perms_and_acl do risk getting fooled by a user whose
name is 'mask', and need to be updated, I think, but that is a
separate issue.

> +	then
> +		test_set_prereq SETFACL
> +	fi
> +'
>  
>  if test -z "$LOGNAME"
>  then

Re: [PATCH] t1304: improve setfacl prerequisite setup

[Jeff King]

On Wed, Jun 06, 2012 at 09:58:53AM -0700, Junio C Hamano wrote:

> > +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-"
> 
> At this point of the test, I do not think there is anything that can
> cause this check to be fooled by a user whose name is 'mask', but
> please make it a habit to anchor the pattern at the left.  The tests
> done in check_perms_and_acl do risk getting fooled by a user whose
> name is 'mask', and need to be updated, I think, but that is a
> separate issue.

I intentionally did not anchor it because the tests themselves did not,
and I worried that that there was some subtle portability issue there
(especially because of the magic "let there be two colons" part of the
pattern).

If it was just sloppiness in the original test, then yes, they should
all be fixed.

-Peff

Re: [PATCH] t1304: improve setfacl prerequisite setup

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I intentionally did not anchor it because the tests themselves did not,
> and I worried that that there was some subtle portability issue there
> (especially because of the magic "let there be two colons" part of the
> pattern).

OK, fair enough (the "two colons" one I think was for Solaris).

Re: [PATCH] t1304: improve setfacl prerequisite setup

[Brandon Casey]

On Thu, Jun 7, 2012 at 9:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> I intentionally did not anchor it because the tests themselves did not,
>> and I worried that that there was some subtle portability issue there
>> (especially because of the magic "let there be two colons" part of the
>> pattern).
>
> OK, fair enough (the "two colons" one I think was for Solaris).

It was indeed for Solaris.  I don't recall any other differences in
the output between Solaris and Linux, so you could add the anchor and
see if anyone makes noise.  I no longer have access to Solaris to
test.

-Brandon

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

[Tyler Hicks]

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

On 2012-06-05 12:44:39, Jeff King wrote:
> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
> 
> > >>   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
> > [...]
> > >
> > > 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.

It is likely a bug in the eCryptfs filesystem stacking code.

However, using the above script, I get the same results on eCryptfs as I
do on ext4 in the Ubuntu 12.04 (Precise) LTS:

# file: a
# owner: tyhicks
# group: tyhicks
user::r--
group::r--
other::r--

# file: b
# owner: tyhicks
# group: tyhicks
user::r--
group::---
other::---

Stefan - can you specify which LTS release you're running as well as the
output of `cat /proc/version_signature`? Thanks!

Tyler

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Stefan Beller]

Hi Tyler et all,

thanks for all your help :)

cat /proc/version_signature
Ubuntu 3.2.0-25.40-generic 3.2.18

I filed a bug at launchpad, which contains all my OS versions etc,
please see https://bugs.launchpad.net/ubuntu/+source/ecryptfs-utils/+bug/1009207
I marked it as security issue as it deals with ACL.

Thanks for all the help here on the git mailing list.

A happy git user,
Stefan



2012/6/5 Tyler Hicks <tyhicks@canonical.com>:
> On 2012-06-05 12:44:39, Jeff King wrote:
>> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
>>
>> > >>   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
>> > [...]
>> > >
>> > > 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.
>
> It is likely a bug in the eCryptfs filesystem stacking code.
>
> However, using the above script, I get the same results on eCryptfs as I
> do on ext4 in the Ubuntu 12.04 (Precise) LTS:
>
> # file: a
> # owner: tyhicks
> # group: tyhicks
> user::r--
> group::r--
> other::r--
>
> # file: b
> # owner: tyhicks
> # group: tyhicks
> user::r--
> group::---
> other::---
>
> Stefan - can you specify which LTS release you're running as well as the
> output of `cat /proc/version_signature`? Thanks!
>
> Tyler

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

[Jeff King]

On Tue, Jun 05, 2012 at 02:23:32PM +0200, Stefan Beller wrote:

> so I applied that patch with git apply, but here 2 out of 4 tests now
> still fail.

Hmm. So it seems that the test setfacl does actually work, but for some
reason what git does is making ecryptfs unhappy (but not any other
filesystem).

I just made a test ecryptfs filesystem, and I can easily replicate your
problem. I'll see if I can find out what git is doing to create this
weird state.

-Peff

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

[Stefan Beller]

Hi,

I tried debugging into it:
In git/t/t1304-default-acl.sh there is:
check_perms_and_acl () {
   test -r "$1" &&
   getfacl "$1" > actual &&
   grep -q "user:root:rwx" actual &&
   grep -q "user:${LOGNAME}:rwx" actual &&
   egrep "mask::?r--" actual > /dev/null 2>&1 &&
   grep -q "group::---" actual || false
}

but when I do all the commands as in the test2:
test_expect_success SETFACL 'Objects creation does not break ACLs with
restrictive umask' '
   # SHA1 for empty blob
   check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
'
Now I run the second line in check_perms_and_acl ()  with
.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
there is

getfacl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
# file: .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
# owner: sb
# group: sb
user::r--
user:root:rwx                   #effective:---
user:sb:rwx                     #effective:---
group::---
mask::---
other::---

This command seem to fail 'egrep "mask::?r--" actual' as there is
mask::---
but expected is
mask::r--

That's my understanding of the test case so far.


2012/6/4 Junio C Hamano <gitster@pobox.com>:

> Stefan Beller <stefanbeller@googlemail.com> writes:
>
>> so I just pulled the new git v1.7.10.4 and tried to test it with
>>> make test
>> This yields this output:
>> stderr http://pastebin.com/V8yuZFfi
>> stdout http://dl.dropbox.com/u/6520164/git/maketest.txt
>>
>> In Test 1304 there is
>> not ok - 2 Objects creation does not break ACLs with restrictive umask
>>
>> I am running Ubuntu 12.04 with Linux sb 3.2.0-25-generic #40-Ubuntu
>> SMP Wed May 23 20:30:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux
>
> Interesting. I've seen t1304 break from time to time.
>
> I set the DEFAULT_TEST_TARGET to "prove", and when the test suite
> finishes with this failure, I noticed t3600-rm and some other test I
> do not recall also failed, with two extra files (actual and expect)
> at the root of the TEST_OUTPUT_DIRECTORY (set to /dev/shm/testpen
> via "--root=/dev/shm/testpen" option).
>
> I the breakage does not happen reproducibly with any pattern other
> than the above (I do not know if it never happens when test target
> is set to "test", for example), so haven't looked (and will not
> look) into it myself further than that.