* Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask
@ 2012-06-04 15:40 Stefan Beller
2012-06-04 20:18 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Stefan Beller @ 2012-06-04 15:40 UTC (permalink / raw)
To: git
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-04 15:40 Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Stefan Beller @ 2012-06-04 20:18 ` Junio C Hamano [not found] ` <CALbm-EatNCPjFRO4NyGfZuSa72-FXwZcd_7cFe-f_iMOdGL4MQ@mail.gmail.com> 0 siblings, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2012-06-04 20:18 UTC (permalink / raw) To: Stefan Beller; +Cc: git 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <CALbm-EatNCPjFRO4NyGfZuSa72-FXwZcd_7cFe-f_iMOdGL4MQ@mail.gmail.com>]
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask [not found] ` <CALbm-EatNCPjFRO4NyGfZuSa72-FXwZcd_7cFe-f_iMOdGL4MQ@mail.gmail.com> @ 2012-06-04 22:19 ` Junio C Hamano 2012-06-05 6:02 ` Matthieu Moy 0 siblings, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2012-06-04 22:19 UTC (permalink / raw) To: Stefan Beller; +Cc: git, Matthieu Moy 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... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-04 22:19 ` Junio C Hamano @ 2012-06-05 6:02 ` Matthieu Moy 2012-06-05 7:23 ` Stefan Beller 0 siblings, 1 reply; 26+ messages in thread From: Matthieu Moy @ 2012-06-05 6:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: Stefan Beller, git 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 6:02 ` Matthieu Moy @ 2012-06-05 7:23 ` Stefan Beller 2012-06-05 7:43 ` Stefan Beller 2012-06-05 7:56 ` Jeff King 0 siblings, 2 replies; 26+ messages in thread From: Stefan Beller @ 2012-06-05 7:23 UTC (permalink / raw) To: Matthieu Moy; +Cc: Junio C Hamano, git 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 7:23 ` Stefan Beller @ 2012-06-05 7:43 ` Stefan Beller 2012-06-05 7:56 ` Jeff King 1 sibling, 0 replies; 26+ messages in thread From: Stefan Beller @ 2012-06-05 7:43 UTC (permalink / raw) To: Matthieu Moy; +Cc: Junio C Hamano, git 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 7:23 ` Stefan Beller 2012-06-05 7:43 ` Stefan Beller @ 2012-06-05 7:56 ` Jeff King 2012-06-05 11:29 ` Matthieu Moy 1 sibling, 1 reply; 26+ messages in thread From: Jeff King @ 2012-06-05 7:56 UTC (permalink / raw) To: Stefan Beller; +Cc: Matthieu Moy, Junio C Hamano, git 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 7:56 ` Jeff King @ 2012-06-05 11:29 ` Matthieu Moy 2012-06-05 12:23 ` Stefan Beller 0 siblings, 1 reply; 26+ messages in thread From: Matthieu Moy @ 2012-06-05 11:29 UTC (permalink / raw) To: Jeff King; +Cc: Stefan Beller, Junio C Hamano, git 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 11:29 ` Matthieu Moy @ 2012-06-05 12:23 ` Stefan Beller 2012-06-05 13:28 ` Matthieu Moy 2012-06-05 13:29 ` Jeff King 0 siblings, 2 replies; 26+ messages in thread From: Stefan Beller @ 2012-06-05 12:23 UTC (permalink / raw) To: Matthieu Moy; +Cc: Jeff King, Junio C Hamano, git 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 12:23 ` Stefan Beller @ 2012-06-05 13:28 ` Matthieu Moy 2012-06-05 14:04 ` Jeff King 2012-06-05 13:29 ` Jeff King 1 sibling, 1 reply; 26+ messages in thread From: Matthieu Moy @ 2012-06-05 13:28 UTC (permalink / raw) To: Stefan Beller; +Cc: Jeff King, Junio C Hamano, git 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 13:28 ` Matthieu Moy @ 2012-06-05 14:04 ` Jeff King 2012-06-05 14:10 ` Jeff King 0 siblings, 1 reply; 26+ messages in thread From: Jeff King @ 2012-06-05 14:04 UTC (permalink / raw) To: Matthieu Moy; +Cc: Stefan Beller, Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 14:04 ` Jeff King @ 2012-06-05 14:10 ` Jeff King 2012-06-05 14:28 ` Jeff King 0 siblings, 1 reply; 26+ messages in thread From: Jeff King @ 2012-06-05 14:10 UTC (permalink / raw) To: Matthieu Moy; +Cc: Stefan Beller, Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 14:10 ` Jeff King @ 2012-06-05 14:28 ` Jeff King 2012-06-05 15:05 ` Jeff King 0 siblings, 1 reply; 26+ messages in thread From: Jeff King @ 2012-06-05 14:28 UTC (permalink / raw) To: Matthieu Moy; +Cc: Stefan Beller, Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 14:28 ` Jeff King @ 2012-06-05 15:05 ` Jeff King 2012-06-05 16:31 ` Junio C Hamano 0 siblings, 1 reply; 26+ messages in thread From: Jeff King @ 2012-06-05 15:05 UTC (permalink / raw) To: Matthieu Moy; +Cc: ecryptfs, Stefan Beller, Junio C Hamano, git [+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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 15:05 ` Jeff King @ 2012-06-05 16:31 ` Junio C Hamano 2012-06-05 16:44 ` Jeff King 0 siblings, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2012-06-05 16:31 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, ecryptfs, Stefan Beller, git 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? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 16:31 ` Junio C Hamano @ 2012-06-05 16:44 ` Jeff King 2012-06-05 17:27 ` Junio C Hamano 2012-06-05 19:16 ` Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Tyler Hicks 0 siblings, 2 replies; 26+ messages in thread From: Jeff King @ 2012-06-05 16:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: Matthieu Moy, ecryptfs, Stefan Beller, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 16:44 ` Jeff King @ 2012-06-05 17:27 ` Junio C Hamano 2012-06-06 13:28 ` [PATCH] t1304: improve setfacl prerequisite setup Jeff King 2012-06-05 19:16 ` Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Tyler Hicks 1 sibling, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2012-06-05 17:27 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, ecryptfs, Stefan Beller, git 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] t1304: improve setfacl prerequisite setup 2012-06-05 17:27 ` Junio C Hamano @ 2012-06-06 13:28 ` Jeff King 2012-06-06 16:11 ` Stefan Beller 2012-06-06 16:58 ` Junio C Hamano 0 siblings, 2 replies; 26+ messages in thread From: Jeff King @ 2012-06-06 13:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: Matthieu Moy, Stefan Beller, git 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] t1304: improve setfacl prerequisite setup 2012-06-06 13:28 ` [PATCH] t1304: improve setfacl prerequisite setup Jeff King @ 2012-06-06 16:11 ` Stefan Beller 2012-06-06 16:58 ` Junio C Hamano 1 sibling, 0 replies; 26+ messages in thread From: Stefan Beller @ 2012-06-06 16:11 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Matthieu Moy, git 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] t1304: improve setfacl prerequisite setup 2012-06-06 13:28 ` [PATCH] t1304: improve setfacl prerequisite setup Jeff King 2012-06-06 16:11 ` Stefan Beller @ 2012-06-06 16:58 ` Junio C Hamano 2012-06-07 9:02 ` Jeff King 1 sibling, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2012-06-06 16:58 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, Stefan Beller, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] t1304: improve setfacl prerequisite setup 2012-06-06 16:58 ` Junio C Hamano @ 2012-06-07 9:02 ` Jeff King 2012-06-07 16:16 ` Junio C Hamano 0 siblings, 1 reply; 26+ messages in thread From: Jeff King @ 2012-06-07 9:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: Matthieu Moy, Stefan Beller, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] t1304: improve setfacl prerequisite setup 2012-06-07 9:02 ` Jeff King @ 2012-06-07 16:16 ` Junio C Hamano 2012-06-08 1:18 ` Brandon Casey 0 siblings, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2012-06-07 16:16 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, Stefan Beller, git 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). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] t1304: improve setfacl prerequisite setup 2012-06-07 16:16 ` Junio C Hamano @ 2012-06-08 1:18 ` Brandon Casey 0 siblings, 0 replies; 26+ messages in thread From: Brandon Casey @ 2012-06-08 1:18 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Matthieu Moy, Stefan Beller, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 16:44 ` Jeff King 2012-06-05 17:27 ` Junio C Hamano @ 2012-06-05 19:16 ` Tyler Hicks 2012-06-05 20:45 ` Stefan Beller 1 sibling, 1 reply; 26+ messages in thread From: Tyler Hicks @ 2012-06-05 19:16 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Matthieu Moy, ecryptfs, Stefan Beller, git [-- 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 --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 19:16 ` Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Tyler Hicks @ 2012-06-05 20:45 ` Stefan Beller 0 siblings, 0 replies; 26+ messages in thread From: Stefan Beller @ 2012-06-05 20:45 UTC (permalink / raw) To: Tyler Hicks; +Cc: Jeff King, Junio C Hamano, Matthieu Moy, ecryptfs, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask 2012-06-05 12:23 ` Stefan Beller 2012-06-05 13:28 ` Matthieu Moy @ 2012-06-05 13:29 ` Jeff King 1 sibling, 0 replies; 26+ messages in thread From: Jeff King @ 2012-06-05 13:29 UTC (permalink / raw) To: Stefan Beller; +Cc: Matthieu Moy, Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2012-06-08 1:18 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-06-04 15:40 Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Stefan Beller 2012-06-04 20:18 ` Junio C Hamano [not found] ` <CALbm-EatNCPjFRO4NyGfZuSa72-FXwZcd_7cFe-f_iMOdGL4MQ@mail.gmail.com> 2012-06-04 22:19 ` Junio C Hamano 2012-06-05 6:02 ` Matthieu Moy 2012-06-05 7:23 ` Stefan Beller 2012-06-05 7:43 ` Stefan Beller 2012-06-05 7:56 ` Jeff King 2012-06-05 11:29 ` Matthieu Moy 2012-06-05 12:23 ` Stefan Beller 2012-06-05 13:28 ` Matthieu Moy 2012-06-05 14:04 ` Jeff King 2012-06-05 14:10 ` Jeff King 2012-06-05 14:28 ` Jeff King 2012-06-05 15:05 ` Jeff King 2012-06-05 16:31 ` Junio C Hamano 2012-06-05 16:44 ` Jeff King 2012-06-05 17:27 ` Junio C Hamano 2012-06-06 13:28 ` [PATCH] t1304: improve setfacl prerequisite setup Jeff King 2012-06-06 16:11 ` Stefan Beller 2012-06-06 16:58 ` Junio C Hamano 2012-06-07 9:02 ` Jeff King 2012-06-07 16:16 ` Junio C Hamano 2012-06-08 1:18 ` Brandon Casey 2012-06-05 19:16 ` Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask Tyler Hicks 2012-06-05 20:45 ` Stefan Beller 2012-06-05 13:29 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).