{"thread":{"id":"30701","subject":"Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","startedAt":"2012-06-04T15:40:35Z","lastAt":"2012-06-08T01:18:31Z","messageCount":26,"participants":["Stefan Beller","Junio C Hamano","Matthieu Moy","Jeff King","Tyler Hicks","Brandon Casey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"192809","messageId":"CALbm-Ea5ZkAGFyB2OETqe7vK7LE+yO0zSaa_+kFMXOhO-nMwMQ@mail.gmail.com","threadId":"30701","inReplyTo":null,"subject":"Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2012-06-04T15:40:35Z","receivedAt":"2012-06-04T15:40:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hello,\n\nso I just pulled the new git v1.7.10.4 and tried to test it with\n> make test\nThis yields this output:\nstderr http://pastebin.com/V8yuZFfi\nstdout http://dl.dropbox.com/u/6520164/git/maketest.txt\n\nIn Test 1304 there is\nnot ok - 2 Objects creation does not break ACLs with restrictive umask\n\nI am running Ubuntu 12.04 with Linux sb 3.2.0-25-generic #40-Ubuntu\nSMP Wed May 23 20:30:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux\n\nBest Regards,\nStefan Beller\n"},{"id":"192848","messageId":"7vhauqsue3.fsf@alter.siamese.dyndns.org","threadId":"30701","inReplyTo":"CALbm-Ea5ZkAGFyB2OETqe7vK7LE+yO0zSaa_+kFMXOhO-nMwMQ@mail.gmail.com","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-04T20:18:44Z","receivedAt":"2012-06-04T20:18:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <stefanbeller@googlemail.com> writes:\n\n> so I just pulled the new git v1.7.10.4 and tried to test it with\n>> make test\n> This yields this output:\n> stderr http://pastebin.com/V8yuZFfi\n> stdout http://dl.dropbox.com/u/6520164/git/maketest.txt\n>\n> In Test 1304 there is\n> not ok - 2 Objects creation does not break ACLs with restrictive umask\n>\n> I am running Ubuntu 12.04 with Linux sb 3.2.0-25-generic #40-Ubuntu\n> SMP Wed May 23 20:30:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux\n\nInteresting. I've seen t1304 break from time to time.\n\nI set the DEFAULT_TEST_TARGET to \"prove\", and when the test suite\nfinishes with this failure, I noticed t3600-rm and some other test I\ndo not recall also failed, with two extra files (actual and expect)\nat the root of the TEST_OUTPUT_DIRECTORY (set to /dev/shm/testpen\nvia \"--root=/dev/shm/testpen\" option).\n\nI the breakage does not happen reproducibly with any pattern other\nthan the above (I do not know if it never happens when test target\nis set to \"test\", for example), so haven't looked (and will not\nlook) into it myself further than that.\n"},{"id":"192859","messageId":"7vy5o2ra7w.fsf@alter.siamese.dyndns.org","threadId":"30701","inReplyTo":"CALbm-EatNCPjFRO4NyGfZuSa72-FXwZcd_7cFe-f_iMOdGL4MQ@mail.gmail.com","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-04T22:19:47Z","receivedAt":"2012-06-04T22:19:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <stefanbeller@googlemail.com> writes:\n\n> I tried debugging into it:\n> In git/t/t1304-default-acl.sh there is:\n> check_perms_and_acl () {\n>     test -r \"$1\" &&\n>     getfacl \"$1\" > actual &&\n>     grep -q \"user:root:rwx\" actual &&\n>     grep -q \"user:${LOGNAME}:rwx\" actual &&\n>     egrep \"mask::?r--\" actual > /dev/null 2>&1 &&\n>     grep -q \"group::---\" actual || false\n> }\n>\n> but when I do all the commands as in the test2:\n> test_expect_success SETFACL 'Objects creation does not break ACLs with\n> restrictive umask' '\n>     # SHA1 for empty blob\n>     check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n> '\n> Now I run the second line in check_perms_and_acl ()  with\n> .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n> there is\n>\n> getfacl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n> # file: .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n> # owner: sb\n> # group: sb\n> user::r--\n> user:root:rwx\t\t\t#effective:---\n> user:sb:rwx\t\t\t#effective:---\n> group::---\n> mask::---\n> other::---\n>\n> This command seem to fail 'egrep \"mask::?r--\" actual' as there is\n> mask::---\n> but expected is\n> mask::r--\n>\n> That's my understanding of the test case so far.\n\nHmph.  Running the test manually does not seem to fail for me and\ngives the \"r--\" mask.\n\nI am lost.  Matthieu, you introduced this as a failing test with\n7aba618 (Add a testcase for ACL with restrictive umask.,\n2010-02-22), as part of the series leading to 5256b00 (Use\ngit_mkstemp_mode instead of plain mkstemp to create object files,\n2010-02-22).  Any ideas (other than \"Your filesystem is broken\",\nthat is)?  As far as I can tell, with 'mask::---', these specific\nusers who are given permissions to read from the objects wouldn't be\nable to read from them, so...\n"},{"id":"192868","messageId":"vpqpq9ejnxs.fsf@bauges.imag.fr","threadId":"30701","inReplyTo":"7vy5o2ra7w.fsf@alter.siamese.dyndns.org","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-05T06:02:55Z","receivedAt":"2012-06-05T06:02:55Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Stefan Beller <stefanbeller@googlemail.com> writes:\n>\n>> I tried debugging into it:\n>> In git/t/t1304-default-acl.sh there is:\n>> check_perms_and_acl () {\n>>     test -r \"$1\" &&\n>>     getfacl \"$1\" > actual &&\n>>     grep -q \"user:root:rwx\" actual &&\n>>     grep -q \"user:${LOGNAME}:rwx\" actual &&\n>>     egrep \"mask::?r--\" actual > /dev/null 2>&1 &&\n>>     grep -q \"group::---\" actual || false\n>> }\n\nCan you run it with --verbose and post the result?\n\n> Any ideas (other than \"Your filesystem is broken\",\n> that is)?\n\nI'm very tempted to go for the \"Your filesystem is broken\" indeed.\n\n> As far as I can tell, with 'mask::---', these specific users who are\n> given permissions to read from the objects wouldn't be able to read\n> from them, so...\n\nThat's my understanding too.\n\nStefan, which filesystem are you using in the directory where you run\ntests (type \"mount\" if you don't know)? Are you running on a virtual\nmachine?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"192875","messageId":"CALbm-EZrKGaj1Q7gbmPmG0wQHxksnJqaS3bz3tMDsego7Zm2ZQ@mail.gmail.com","threadId":"30701","inReplyTo":"vpqpq9ejnxs.fsf@bauges.imag.fr","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2012-06-05T07:23:47Z","receivedAt":"2012-06-05T07:23:47Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\n\nsb@sb:~/OSS/git/t$ ./t1304-default-acl.sh --verbose\nInitialized empty Git repository in /home/sb/OSS/git/t/trash\ndirectory.t1304-default-acl/.git/\nexpecting success:\n    setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n    setfacl -m m:rwx               $dirs_to_set &&\n    setfacl -m u:root:rwx          $dirs_to_set &&\n    setfacl -m d:u:\"${LOGNAME}\":rwx  $dirs_to_set &&\n    setfacl -m d:u:root:rwx        $dirs_to_set &&\n\n    touch file.txt &&\n    git add file.txt &&\n    git commit -m \"init\"\n\n[master (root-commit) 47f54f4] init\n Author: A U Thor <author@example.com>\n 0 files changed\n create mode 100644 file.txt\nok 1 - Setup test repo\n\nexpecting success:\n    # SHA1 for empty blob\n    check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n\n/home/sb/OSS/git/t/trash directory.t1304-default-acl\nnot ok - 2 Objects creation does not break ACLs with restrictive umask\n#\t\n#\t    # SHA1 for empty blob\n#\t    check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n#\t\n\nexpecting success:\n    git gc &&\n    check_perms_and_acl .git/objects/pack/*.pack\n\nCounting objects: 3, done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 0 (delta 0)\n/home/sb/OSS/git/t/trash directory.t1304-default-acl\nnot ok - 3 git gc does not break ACLs with restrictive umask\n#\t\n#\t    git gc &&\n#\t    check_perms_and_acl .git/objects/pack/*.pack\n#\t\n\n# failed 2 among 3 test(s)\n1..3\nsb@sb:~/OSS/git/t$ cat trash\\ directory.t1304-default-acl/actual\n# file: .git/objects/pack/pack-ee77696bcc9be7ef581005ee3706bc17fcba376d.pack\n# owner: sb\n# group: sb\nuser::r--\nuser:root:rwx\t#effective:---\nuser:sb:rwx\t#effective:---\ngroup::---\nmask::---\nother::---\n\nsb@sb:~/OSS/git/t$ mount\n/dev/sda3 on / type ext4 (rw,errors=remount-ro)\nproc on /proc type proc (rw,noexec,nosuid,nodev)\nsysfs on /sys type sysfs (rw,noexec,nosuid,nodev)\nnone on /sys/fs/fuse/connections type fusectl (rw)\nnone on /sys/kernel/debug type debugfs (rw)\nnone on /sys/kernel/security type securityfs (rw)\nudev on /dev type devtmpfs (rw,mode=0755)\ndevpts on /dev/pts type devpts (rw,noexec,nosuid,gid=5,mode=0620)\ntmpfs on /run type tmpfs (rw,noexec,nosuid,size=10%,mode=0755)\nnone on /run/lock type tmpfs (rw,noexec,nosuid,nodev,size=5242880)\nnone on /run/shm type tmpfs (rw,nosuid,nodev)\n/dev/sda4 on /home type ext4 (rw)\nbinfmt_misc on /proc/sys/fs/binfmt_misc type binfmt_misc\n(rw,noexec,nosuid,nodev)\n/home/sb/.Private on /home/sb type ecryptfs\n(ecryptfs_check_dev_ruid,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs,ecryptfs_sig=3b9d213e5c5d5780,ecryptfs_fnek_sig=77bec2da523c0338)\ngvfs-fuse-daemon on /home/sb/.gvfs type fuse.gvfs-fuse-daemon\n(rw,nosuid,nodev,user=sb)\n\n\nSo I am using /dev/sda4 on /home type ext4 (rw), but my user account\nhas its home directory encrypted via ecryptfs.\nThis is not run inside a virtual machine, it's a native computer.\n\n\n\n2012/6/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Stefan Beller <stefanbeller@googlemail.com> writes:\n>>\n>>> I tried debugging into it:\n>>> In git/t/t1304-default-acl.sh there is:\n>>> check_perms_and_acl () {\n>>>     test -r \"$1\" &&\n>>>     getfacl \"$1\" > actual &&\n>>>     grep -q \"user:root:rwx\" actual &&\n>>>     grep -q \"user:${LOGNAME}:rwx\" actual &&\n>>>     egrep \"mask::?r--\" actual > /dev/null 2>&1 &&\n>>>     grep -q \"group::---\" actual || false\n>>> }\n>\n> Can you run it with --verbose and post the result?\n>\n>> Any ideas (other than \"Your filesystem is broken\",\n>> that is)?\n>\n> I'm very tempted to go for the \"Your filesystem is broken\" indeed.\n>\n>> As far as I can tell, with 'mask::---', these specific users who are\n>> given permissions to read from the objects wouldn't be able to read\n>> from them, so...\n>\n> That's my understanding too.\n>\n> Stefan, which filesystem are you using in the directory where you run\n> tests (type \"mount\" if you don't know)? Are you running on a virtual\n> machine?\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n"},{"id":"192878","messageId":"CALbm-EZKBw_r6DXzqP7qcYe3tuAYLsy-7YF6tQFR9hJV5Q6mUQ@mail.gmail.com","threadId":"30701","inReplyTo":"CALbm-EZrKGaj1Q7gbmPmG0wQHxksnJqaS3bz3tMDsego7Zm2ZQ@mail.gmail.com","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2012-06-05T07:43:19Z","receivedAt":"2012-06-05T07:43:19Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Just did a short google search and found this:\nhttp://serverfault.com/questions/294158/enable-acl-for-ecryptfs-mounted-home-directory\n\nSo I am convinced my filesystem is just broken as you said ;)\n\n2012/6/5 Stefan Beller <stefanbeller@googlemail.com>:\n> Hi,\n>\n> sb@sb:~/OSS/git/t$ ./t1304-default-acl.sh --verbose\n> Initialized empty Git repository in /home/sb/OSS/git/t/trash\n> directory.t1304-default-acl/.git/\n> expecting success:\n>    setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n>    setfacl -m m:rwx               $dirs_to_set &&\n>    setfacl -m u:root:rwx          $dirs_to_set &&\n>    setfacl -m d:u:\"${LOGNAME}\":rwx  $dirs_to_set &&\n>    setfacl -m d:u:root:rwx        $dirs_to_set &&\n>\n>    touch file.txt &&\n>    git add file.txt &&\n>    git commit -m \"init\"\n>\n> [master (root-commit) 47f54f4] init\n>  Author: A U Thor <author@example.com>\n>  0 files changed\n>  create mode 100644 file.txt\n> ok 1 - Setup test repo\n>\n> expecting success:\n>    # SHA1 for empty blob\n>    check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n>\n> /home/sb/OSS/git/t/trash directory.t1304-default-acl\n> not ok - 2 Objects creation does not break ACLs with restrictive umask\n> #\n> #           # SHA1 for empty blob\n> #           check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n> #\n>\n> expecting success:\n>    git gc &&\n>    check_perms_and_acl .git/objects/pack/*.pack\n>\n> Counting objects: 3, done.\n> Writing objects: 100% (3/3), done.\n> Total 3 (delta 0), reused 0 (delta 0)\n> /home/sb/OSS/git/t/trash directory.t1304-default-acl\n> not ok - 3 git gc does not break ACLs with restrictive umask\n> #\n> #           git gc &&\n> #           check_perms_and_acl .git/objects/pack/*.pack\n> #\n>\n> # failed 2 among 3 test(s)\n> 1..3\n> sb@sb:~/OSS/git/t$ cat trash\\ directory.t1304-default-acl/actual\n> # file: .git/objects/pack/pack-ee77696bcc9be7ef581005ee3706bc17fcba376d.pack\n> # owner: sb\n> # group: sb\n> user::r--\n> user:root:rwx   #effective:---\n> user:sb:rwx     #effective:---\n> group::---\n> mask::---\n> other::---\n>\n> sb@sb:~/OSS/git/t$ mount\n> /dev/sda3 on / type ext4 (rw,errors=remount-ro)\n> proc on /proc type proc (rw,noexec,nosuid,nodev)\n> sysfs on /sys type sysfs (rw,noexec,nosuid,nodev)\n> none on /sys/fs/fuse/connections type fusectl (rw)\n> none on /sys/kernel/debug type debugfs (rw)\n> none on /sys/kernel/security type securityfs (rw)\n> udev on /dev type devtmpfs (rw,mode=0755)\n> devpts on /dev/pts type devpts (rw,noexec,nosuid,gid=5,mode=0620)\n> tmpfs on /run type tmpfs (rw,noexec,nosuid,size=10%,mode=0755)\n> none on /run/lock type tmpfs (rw,noexec,nosuid,nodev,size=5242880)\n> none on /run/shm type tmpfs (rw,nosuid,nodev)\n> /dev/sda4 on /home type ext4 (rw)\n> binfmt_misc on /proc/sys/fs/binfmt_misc type binfmt_misc\n> (rw,noexec,nosuid,nodev)\n> /home/sb/.Private on /home/sb type ecryptfs\n> (ecryptfs_check_dev_ruid,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs,ecryptfs_sig=3b9d213e5c5d5780,ecryptfs_fnek_sig=77bec2da523c0338)\n> gvfs-fuse-daemon on /home/sb/.gvfs type fuse.gvfs-fuse-daemon\n> (rw,nosuid,nodev,user=sb)\n>\n>\n> So I am using /dev/sda4 on /home type ext4 (rw), but my user account\n> has its home directory encrypted via ecryptfs.\n> This is not run inside a virtual machine, it's a native computer.\n>\n>\n>\n> 2012/6/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Stefan Beller <stefanbeller@googlemail.com> writes:\n>>>\n>>>> I tried debugging into it:\n>>>> In git/t/t1304-default-acl.sh there is:\n>>>> check_perms_and_acl () {\n>>>>     test -r \"$1\" &&\n>>>>     getfacl \"$1\" > actual &&\n>>>>     grep -q \"user:root:rwx\" actual &&\n>>>>     grep -q \"user:${LOGNAME}:rwx\" actual &&\n>>>>     egrep \"mask::?r--\" actual > /dev/null 2>&1 &&\n>>>>     grep -q \"group::---\" actual || false\n>>>> }\n>>\n>> Can you run it with --verbose and post the result?\n>>\n>>> Any ideas (other than \"Your filesystem is broken\",\n>>> that is)?\n>>\n>> I'm very tempted to go for the \"Your filesystem is broken\" indeed.\n>>\n>>> As far as I can tell, with 'mask::---', these specific users who are\n>>> given permissions to read from the objects wouldn't be able to read\n>>> from them, so...\n>>\n>> That's my understanding too.\n>>\n>> Stefan, which filesystem are you using in the directory where you run\n>> tests (type \"mount\" if you don't know)? Are you running on a virtual\n>> machine?\n>>\n>> --\n>> Matthieu Moy\n>> http://www-verimag.imag.fr/~moy/\n"},{"id":"192879","messageId":"20120605075614.GE25809@sigill.intra.peff.net","threadId":"30701","inReplyTo":"CALbm-EZrKGaj1Q7gbmPmG0wQHxksnJqaS3bz3tMDsego7Zm2ZQ@mail.gmail.com","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T07:56:15Z","receivedAt":"2012-06-05T07:56:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 05, 2012 at 09:23:47AM +0200, Stefan Beller wrote:\n\n> So I am using /dev/sda4 on /home type ext4 (rw), but my user account\n> has its home directory encrypted via ecryptfs.\n\nIt's very likely that ecryptfs is the problem, then. Googling around, I\nfind conflicting reports on whether it actually supports ACLs or not.\n\nAt the top of the test script we check that \"setfacl\" doesn't report an\nerror, and assume that ACLs work if it doesn't. Maybe that test needs to\nbe more comprehensive, like the patch below. What happens when you run\nt1304 with this patch (my expectation is that it will just skip the acl\ntests)?\n\n-- >8 --\nSubject: t1304: improve setfacl prerequisite setup\n\nSome filesystems will cause \"setfacl\" to report success, even\nthough they do not seem to behave sanely enough for our\ntests. Let's make sure that not only does setfacl work, but\nthat we can read back the result.\n\nSince we're making the setup more complex, let's move it\ninto its own test. This will hide the output for us unless\nthe user wants to run \"-v\" to see it (and we don't need to\nbother printing anything about setfacl failing; the\nremaining tests will properly print \"skip\" due to the\nmissing prerequisite).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1304-default-acl.sh | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 2b962cf..be954f0 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -14,16 +14,13 @@ umask 077\n # We need an arbitrary other user give permission to using ACLs. root\n # is a good candidate: exists on all unices, and it has permission\n # anyway, so we don't create a security hole running the testsuite.\n-\n-setfacl_out=\"$(setfacl -m u:root:rwx . 2>&1)\"\n-setfacl_ret=$?\n-\n-if test $setfacl_ret != 0\n-then\n-\tsay \"Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')\"\n-else\n-\ttest_set_prereq SETFACL\n-fi\n+test_expect_success 'checking for a working acl setup' '\n+\tif setfacl -m u:root:rwx . &&\n+\t   getfacl . | grep user:root:rwx\n+\tthen\n+\t\ttest_set_prereq SETFACL\n+\tfi\n+'\n \n if test -z \"$LOGNAME\"\n then\n"},{"id":"192893","messageId":"vpq4nqqj8ss.fsf@bauges.imag.fr","threadId":"30701","inReplyTo":"20120605075614.GE25809@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-05T11:29:55Z","receivedAt":"2012-06-05T11:29:55Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n> Subject: t1304: improve setfacl prerequisite setup\n\n> +test_expect_success 'checking for a working acl setup' '\n> +\tif setfacl -m u:root:rwx . &&\n> +\t   getfacl . | grep user:root:rwx\n> +\tthen\n> +\t\ttest_set_prereq SETFACL\n> +\tfi\n> +'\n\nExcellent. Thanks.\n\nTested-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"192897","messageId":"CALbm-EbGoaxkvBXphAPF8rRkS=VFeeFHXQSFdWVrZUJJ8DYovw@mail.gmail.com","threadId":"30701","inReplyTo":"vpq4nqqj8ss.fsf@bauges.imag.fr","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2012-06-05T12:23:32Z","receivedAt":"2012-06-05T12:23:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\n\nso I applied that patch with git apply, but here 2 out of 4 tests now\nstill fail.\n\n\nsb@sb:~/OSS/git/t$ cat t1304-default-acl.sh\n#!/bin/sh\n#\n# Copyright (c) 2010 Matthieu Moy\n#\n\ntest_description='Test repository with default ACL'\n\n# Create the test repo with restrictive umask\n# => this must come before . ./test-lib.sh\numask 077\n\n. ./test-lib.sh\n\n# We need an arbitrary other user give permission to using ACLs. root\n# is a good candidate: exists on all unices, and it has permission\n# anyway, so we don't create a security hole running the testsuite.\ntest_expect_success 'checking for a working acl setup' '\n       if setfacl -m u:root:rwx . &&\n          getfacl . | grep user:root:rwx\n       then\n               test_set_prereq SETFACL\n       fi\n'\n\nif test -z \"$LOGNAME\"\nthen\n\tLOGNAME=$USER\nfi\n\ncheck_perms_and_acl () {\n\ttest -r \"$1\" &&\n\tgetfacl \"$1\" > actual &&\n\tgrep -q \"user:root:rwx\" actual &&\n\tgrep -q \"user:${LOGNAME}:rwx\" actual &&\n\tegrep \"mask::?r--\" actual > /dev/null 2>&1 &&\n\tgrep -q \"group::---\" actual || false\n}\n\ndirs_to_set=\"./ .git/ .git/objects/ .git/objects/pack/\"\n\ntest_expect_success SETFACL 'Setup test repo' '\n\tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n\tsetfacl -m m:rwx               $dirs_to_set &&\n\tsetfacl -m u:root:rwx          $dirs_to_set &&\n\tsetfacl -m d:u:\"$LOGNAME\":rwx  $dirs_to_set &&\n\tsetfacl -m d:u:root:rwx        $dirs_to_set &&\n\n\ttouch file.txt &&\n\tgit add file.txt &&\n\tgit commit -m \"init\"\n'\n\ntest_expect_success SETFACL 'Objects creation does not break ACLs with\nrestrictive umask' '\n\t# SHA1 for empty blob\n\tcheck_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n'\n\ntest_expect_success SETFACL 'git gc does not break ACLs with\nrestrictive umask' '\n\tgit gc &&\n\tcheck_perms_and_acl .git/objects/pack/*.pack\n'\n\ntest_done\nsb@sb:~/OSS/git/t$ ./t1304-default-acl.sh --verbose\nInitialized empty Git repository in /home/sb/OSS/git/t/trash\ndirectory.t1304-default-acl/.git/\nexpecting success:\n       if setfacl -m u:root:rwx . &&\n          getfacl . | grep user:root:rwx\n       then\n               test_set_prereq SETFACL\n       fi\n\nuser:root:rwx\nok 1 - checking for a working acl setup\n\nexpecting success:\n\tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n\tsetfacl -m m:rwx               $dirs_to_set &&\n\tsetfacl -m u:root:rwx          $dirs_to_set &&\n\tsetfacl -m d:u:\"$LOGNAME\":rwx  $dirs_to_set &&\n\tsetfacl -m d:u:root:rwx        $dirs_to_set &&\n\n\ttouch file.txt &&\n\tgit add file.txt &&\n\tgit commit -m \"init\"\n\n[master (root-commit) 011f91a] init\n Author: A U Thor <author@example.com>\n 0 files changed\n create mode 100644 file.txt\nok 2 - Setup test repo\n\nexpecting success:\n\t# SHA1 for empty blob\n\tcheck_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n\nnot ok - 3 Objects creation does not break ACLs with restrictive umask\n#\t\n#\t\t# SHA1 for empty blob\n#\t\tcheck_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391\n#\t\n\nexpecting success:\n\tgit gc &&\n\tcheck_perms_and_acl .git/objects/pack/*.pack\n\nCounting objects: 3, done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 0 (delta 0)\nnot ok - 4 git gc does not break ACLs with restrictive umask\n#\t\n#\t\tgit gc &&\n#\t\tcheck_perms_and_acl .git/objects/pack/*.pack\n#\t\n\n# failed 2 among 4 test(s)\n1..4\n\n2012/6/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:\n> Jeff King <peff@peff.net> writes:\n>\n>> Subject: t1304: improve setfacl prerequisite setup\n>\n>> +test_expect_success 'checking for a working acl setup' '\n>> +     if setfacl -m u:root:rwx . &&\n>> +        getfacl . | grep user:root:rwx\n>> +     then\n>> +             test_set_prereq SETFACL\n>> +     fi\n>> +'\n>\n> Excellent. Thanks.\n>\n> Tested-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n"},{"id":"192904","messageId":"vpqk3zlhorc.fsf@bauges.imag.fr","threadId":"30701","inReplyTo":"CALbm-EbGoaxkvBXphAPF8rRkS=VFeeFHXQSFdWVrZUJJ8DYovw@mail.gmail.com","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-05T13:28:07Z","receivedAt":"2012-06-05T13:28:07Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Stefan Beller <stefanbeller@googlemail.com> writes:\n\n> Hi,\n>\n> so I applied that patch with git apply, but here 2 out of 4 tests now\n> still fail.\n\nYes, Jeff's patch checks for a \"working\" setfacl/getfacl, but does not\ncheck for subtle brokenness of it.\n\nI'd say a filesystem that accepts setfacl, returns the right line in\ngetfacl, and then does not really work is broken, and it's a feature\nthat the testsuite shows test failures. A simple fix for you would be to\ndisable ACLs, either on your system or in your $HOME.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"192905","messageId":"20120605132919.GA13054@sigill.intra.peff.net","threadId":"30701","inReplyTo":"CALbm-EbGoaxkvBXphAPF8rRkS=VFeeFHXQSFdWVrZUJJ8DYovw@mail.gmail.com","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T13:29:19Z","receivedAt":"2012-06-05T13:29:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 05, 2012 at 02:23:32PM +0200, Stefan Beller wrote:\n\n> so I applied that patch with git apply, but here 2 out of 4 tests now\n> still fail.\n\nHmm. So it seems that the test setfacl does actually work, but for some\nreason what git does is making ecryptfs unhappy (but not any other\nfilesystem).\n\nI just made a test ecryptfs filesystem, and I can easily replicate your\nproblem. I'll see if I can find out what git is doing to create this\nweird state.\n\n-Peff\n"},{"id":"192906","messageId":"20120605140449.GA15640@sigill.intra.peff.net","threadId":"30701","inReplyTo":"vpqk3zlhorc.fsf@bauges.imag.fr","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T14:04:49Z","receivedAt":"2012-06-05T14:04:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 05, 2012 at 03:28:07PM +0200, Matthieu Moy wrote:\n\n> Stefan Beller <stefanbeller@googlemail.com> writes:\n> \n> > Hi,\n> >\n> > so I applied that patch with git apply, but here 2 out of 4 tests now\n> > still fail.\n> \n> Yes, Jeff's patch checks for a \"working\" setfacl/getfacl, but does not\n> check for subtle brokenness of it.\n> \n> I'd say a filesystem that accepts setfacl, returns the right line in\n> getfacl, and then does not really work is broken, and it's a feature\n> that the testsuite shows test failures. A simple fix for you would be to\n> disable ACLs, either on your system or in your $HOME.\n\nI found the problem. If you create a file on an ecryptfs filesystem with\na restrictive mode, then the inherited mask bits get cleared. For\nexample:\n\n  # if you don't have one already, create an ecryptfs mount; you\n  # will need the ecryptfs-utils package on Debian and Ubuntu\n  ecryptfs-setup-private\n  ecryptfs-mount-private\n  cd Private\n\n  # now set up an inherited mask\n  setfacl -m d:m:rwx .\n\n  # and create some files with different permissions\n  perl -MFcntl -e '\n    sysopen(X, \"a\", O_WRONLY|O_CREAT, 0666);\n    sysopen(X, \"b\", O_WRONLY|O_CREAT, 0600);\n  '\n\n  # and then check the permissions\n  getfacl a b\n\nwhich yields:\n\n  # file: a\n  # owner: peff\n  # group: peff\n  user::rw-\n  group::---\n  mask::r--\n  other::---\n\n  # file: b\n  # owner: peff\n  # group: peff\n  user::rw-\n  group::---\n  mask::---\n  other::---\n\nThat just seems wrong to me.\n\n-Peff\n"},{"id":"192907","messageId":"20120605141039.GB15640@sigill.intra.peff.net","threadId":"30701","inReplyTo":"20120605140449.GA15640@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T14:10:39Z","receivedAt":"2012-06-05T14:10:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 05, 2012 at 10:04:49AM -0400, Jeff King wrote:\n\n>   # and create some files with different permissions\n>   perl -MFcntl -e '\n>     sysopen(X, \"a\", O_WRONLY|O_CREAT, 0666);\n>     sysopen(X, \"b\", O_WRONLY|O_CREAT, 0600);\n>   '\n> \n>   # and then check the permissions\n>   getfacl a b\n> \n> which yields:\n> \n>   # file: a\n>   # owner: peff\n>   # group: peff\n>   user::rw-\n>   group::---\n>   mask::r--\n>   other::---\n> \n>   # file: b\n>   # owner: peff\n>   # group: peff\n>   user::rw-\n>   group::---\n>   mask::---\n>   other::---\n> \n> That just seems wrong to me.\n\nHmm, no I'm wrong. That is OK, as the mask should be restricted by the\ngroup permission (and it is on ext4, too). But when I traced through\ngit, the tmp_obj_* files are created with the right permissions on ext4,\nbut not on ecryptfs. Let me trace through it again...<sigh>\n\n-Peff\n"},{"id":"192908","messageId":"20120605142813.GA17238@sigill.intra.peff.net","threadId":"30701","inReplyTo":"20120605141039.GB15640@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T14:28:13Z","receivedAt":"2012-06-05T14:28:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 05, 2012 at 10:10:39AM -0400, Jeff King wrote:\n\n> On Tue, Jun 05, 2012 at 10:04:49AM -0400, Jeff King wrote:\n> \n> >   # and create some files with different permissions\n> >   perl -MFcntl -e '\n> >     sysopen(X, \"a\", O_WRONLY|O_CREAT, 0666);\n> >     sysopen(X, \"b\", O_WRONLY|O_CREAT, 0600);\n> >   '\n> Hmm, no I'm wrong. That is OK, as the mask should be restricted by the\n> group permission (and it is on ext4, too). But when I traced through\n> git, the tmp_obj_* files are created with the right permissions on ext4,\n> but not on ecryptfs. Let me trace through it again...<sigh>\n\nOK, here's the _real_ issue. Git creates with mode 0444, which should\nstill allow read in the mask. But it's the restrictive umask at the top\nof the test script that causes the problem. Try this:\n\n  setfacl -m m:rwx .\n  perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n  umask 077\n  perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n  getfacl a b\n\nOn ext4, both files will have the read bit set in the mask. On ecryptfs,\n\"b\" will have an empty mask. I think the wrong thing is that we should\nnot be respecting umask at all when default ACLs are in play, and\necryptfs is getting that wrong. But I'm having trouble digging up an\nauthoritative source.\n\n-Peff\n"},{"id":"192911","messageId":"20120605150550.GA19843@sigill.intra.peff.net","threadId":"30701","inReplyTo":"20120605142813.GA17238@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T15:05:50Z","receivedAt":"2012-06-05T15:05:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[+cc ecryptfs@vger, as I think this is an ecryptfs bug]\n\nOn Tue, Jun 05, 2012 at 10:28:13AM -0400, Jeff King wrote:\n\n> OK, here's the _real_ issue. Git creates with mode 0444, which should\n> still allow read in the mask. But it's the restrictive umask at the top\n> of the test script that causes the problem. Try this:\n> \n>   setfacl -m m:rwx .\n>   perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n>   umask 077\n>   perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n>   getfacl a b\n> \n> On ext4, both files will have the read bit set in the mask. On ecryptfs,\n> \"b\" will have an empty mask. I think the wrong thing is that we should\n> not be respecting umask at all when default ACLs are in play, and\n> ecryptfs is getting that wrong. But I'm having trouble digging up an\n> authoritative source.\n\nReading the withdrawn posix 1003.1e and \"man 5 acl\", it seems pretty\nclear that if a default ACL is present, it should be used, and umask\nconsulted only if it is not (so the umask should not be making a\ndifference in this case).\n\nThe reproduction recipe above shows the minimum required to trigger it;\nadding a more realistic default ACL (with actual entries for users) does\nnot seem to make a difference.\n\n-Peff\n"},{"id":"192920","messageId":"7vpq9dpvnp.fsf@alter.siamese.dyndns.org","threadId":"30701","inReplyTo":"20120605150550.GA19843@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-05T16:31:54Z","receivedAt":"2012-06-05T16:31:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> [+cc ecryptfs@vger, as I think this is an ecryptfs bug]\n>\n> On Tue, Jun 05, 2012 at 10:28:13AM -0400, Jeff King wrote:\n>\n>> OK, here's the _real_ issue. Git creates with mode 0444, which should\n>> still allow read in the mask. But it's the restrictive umask at the top\n>> of the test script that causes the problem. Try this:\n>> \n>>   setfacl -m m:rwx .\n>>   perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n>>   umask 077\n>>   perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n>>   getfacl a b\n>> \n>> On ext4, both files will have the read bit set in the mask. On ecryptfs,\n>> \"b\" will have an empty mask. I think the wrong thing is that we should\n>> not be respecting umask at all when default ACLs are in play, and\n>> ecryptfs is getting that wrong. But I'm having trouble digging up an\n>> authoritative source.\n>\n> Reading the withdrawn posix 1003.1e and \"man 5 acl\", it seems pretty\n> clear that if a default ACL is present, it should be used, and umask\n> consulted only if it is not (so the umask should not be making a\n> difference in this case).\n>\n> The reproduction recipe above shows the minimum required to trigger it;\n> adding a more realistic default ACL (with actual entries for users) does\n> not seem to make a difference.\n\nThanks; so combining the above with your earlier patch to 1304 we\nwould have a good detection for SETFACL prerequisite?\n \n"},{"id":"192923","messageId":"20120605164439.GA2694@sigill.intra.peff.net","threadId":"30701","inReplyTo":"7vpq9dpvnp.fsf@alter.siamese.dyndns.org","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-05T16:44:39Z","receivedAt":"2012-06-05T16:44:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:\n\n> >>   setfacl -m m:rwx .\n> >>   perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n> >>   umask 077\n> >>   perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n> >>   getfacl a b\n> [...]\n> >\n> > Reading the withdrawn posix 1003.1e and \"man 5 acl\", it seems pretty\n> > clear that if a default ACL is present, it should be used, and umask\n> > consulted only if it is not (so the umask should not be making a\n> > difference in this case).\n> >\n> > The reproduction recipe above shows the minimum required to trigger it;\n> > adding a more realistic default ACL (with actual entries for users) does\n> > not seem to make a difference.\n> \n> Thanks; so combining the above with your earlier patch to 1304 we\n> would have a good detection for SETFACL prerequisite?\n\nYes, I think we can detect it reliably. I'd like to hear back from\necryptfs folks before making a final patch, though. It may be that there\nis some subtle reason for their behavior, and I want to make sure before\nwe write it off as just buggy.\n\n-Peff\n"},{"id":"192931","messageId":"7v62b5pt2s.fsf@alter.siamese.dyndns.org","threadId":"30701","inReplyTo":"20120605164439.GA2694@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-05T17:27:39Z","receivedAt":"2012-06-05T17:27:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:\n>\n>> >>   setfacl -m m:rwx .\n>> >>   perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n>> >>   umask 077\n>> >>   perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n>> >>   getfacl a b\n>> [...]\n>> >\n>> > Reading the withdrawn posix 1003.1e and \"man 5 acl\", it seems pretty\n>> > clear that if a default ACL is present, it should be used, and umask\n>> > consulted only if it is not (so the umask should not be making a\n>> > difference in this case).\n>> >\n>> > The reproduction recipe above shows the minimum required to trigger it;\n>> > adding a more realistic default ACL (with actual entries for users) does\n>> > not seem to make a difference.\n>> \n>> Thanks; so combining the above with your earlier patch to 1304 we\n>> would have a good detection for SETFACL prerequisite?\n>\n> Yes, I think we can detect it reliably. I'd like to hear back from\n> ecryptfs folks before making a final patch, though. It may be that there\n> is some subtle reason for their behavior, and I want to make sure before\n> we write it off as just buggy.\n\nSensible; thanks.\n"},{"id":"192943","messageId":"20120605191637.GC25709@boyd","threadId":"30701","inReplyTo":"20120605164439.GA2694@sigill.intra.peff.net","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Tyler Hicks","fromEmail":"tyhicks@canonical.com","sentAt":"2012-06-05T19:16:38Z","receivedAt":"2012-06-05T19:16:38Z","isPatch":false,"sender":{"key":"tyhicks@canonical.com","avatar":null},"body":"On 2012-06-05 12:44:39, Jeff King wrote:\n> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:\n> \n> > >>   setfacl -m m:rwx .\n> > >>   perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n> > >>   umask 077\n> > >>   perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n> > >>   getfacl a b\n> > [...]\n> > >\n> > > Reading the withdrawn posix 1003.1e and \"man 5 acl\", it seems pretty\n> > > clear that if a default ACL is present, it should be used, and umask\n> > > consulted only if it is not (so the umask should not be making a\n> > > difference in this case).\n> > >\n> > > The reproduction recipe above shows the minimum required to trigger it;\n> > > adding a more realistic default ACL (with actual entries for users) does\n> > > not seem to make a difference.\n> > \n> > Thanks; so combining the above with your earlier patch to 1304 we\n> > would have a good detection for SETFACL prerequisite?\n> \n> Yes, I think we can detect it reliably. I'd like to hear back from\n> ecryptfs folks before making a final patch, though. It may be that there\n> is some subtle reason for their behavior, and I want to make sure before\n> we write it off as just buggy.\n\nIt is likely a bug in the eCryptfs filesystem stacking code.\n\nHowever, using the above script, I get the same results on eCryptfs as I\ndo on ext4 in the Ubuntu 12.04 (Precise) LTS:\n\n# file: a\n# owner: tyhicks\n# group: tyhicks\nuser::r--\ngroup::r--\nother::r--\n\n# file: b\n# owner: tyhicks\n# group: tyhicks\nuser::r--\ngroup::---\nother::---\n\nStefan - can you specify which LTS release you're running as well as the\noutput of `cat /proc/version_signature`? Thanks!\n\nTyler\n"},{"id":"192952","messageId":"CALbm-Ea2e=zYnzmvUrzbmJwXBD8j5EizbcLaZgD=5T4gJsYvdA@mail.gmail.com","threadId":"30701","inReplyTo":"20120605191637.GC25709@boyd","subject":"Re: Bugreport on Ubuntu LTS: not ok - 2 Objects creation does not break ACLs with restrictive umask","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2012-06-05T20:45:25Z","receivedAt":"2012-06-05T20:45:25Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Tyler et all,\n\nthanks for all your help :)\n\ncat /proc/version_signature\nUbuntu 3.2.0-25.40-generic 3.2.18\n\nI filed a bug at launchpad, which contains all my OS versions etc,\nplease see https://bugs.launchpad.net/ubuntu/+source/ecryptfs-utils/+bug/1009207\nI marked it as security issue as it deals with ACL.\n\nThanks for all the help here on the git mailing list.\n\nA happy git user,\nStefan\n\n\n\n2012/6/5 Tyler Hicks <tyhicks@canonical.com>:\n> On 2012-06-05 12:44:39, Jeff King wrote:\n>> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:\n>>\n>> > >>   setfacl -m m:rwx .\n>> > >>   perl -MFcntl -e 'sysopen(X, \"a\", O_WRONLY|O_CREAT, 0444)'\n>> > >>   umask 077\n>> > >>   perl -MFcntl -e 'sysopen(X, \"b\", O_WRONLY|O_CREAT, 0444)'\n>> > >>   getfacl a b\n>> > [...]\n>> > >\n>> > > Reading the withdrawn posix 1003.1e and \"man 5 acl\", it seems pretty\n>> > > clear that if a default ACL is present, it should be used, and umask\n>> > > consulted only if it is not (so the umask should not be making a\n>> > > difference in this case).\n>> > >\n>> > > The reproduction recipe above shows the minimum required to trigger it;\n>> > > adding a more realistic default ACL (with actual entries for users) does\n>> > > not seem to make a difference.\n>> >\n>> > Thanks; so combining the above with your earlier patch to 1304 we\n>> > would have a good detection for SETFACL prerequisite?\n>>\n>> Yes, I think we can detect it reliably. I'd like to hear back from\n>> ecryptfs folks before making a final patch, though. It may be that there\n>> is some subtle reason for their behavior, and I want to make sure before\n>> we write it off as just buggy.\n>\n> It is likely a bug in the eCryptfs filesystem stacking code.\n>\n> However, using the above script, I get the same results on eCryptfs as I\n> do on ext4 in the Ubuntu 12.04 (Precise) LTS:\n>\n> # file: a\n> # owner: tyhicks\n> # group: tyhicks\n> user::r--\n> group::r--\n> other::r--\n>\n> # file: b\n> # owner: tyhicks\n> # group: tyhicks\n> user::r--\n> group::---\n> other::---\n>\n> Stefan - can you specify which LTS release you're running as well as the\n> output of `cat /proc/version_signature`? Thanks!\n>\n> Tyler\n"},{"id":"192989","messageId":"20120606132824.GA2597@sigill.intra.peff.net","threadId":"30701","inReplyTo":"7v62b5pt2s.fsf@alter.siamese.dyndns.org","subject":"[PATCH] t1304: improve setfacl prerequisite setup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-06T13:28:24Z","receivedAt":"2012-06-06T13:28:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"t1304 first runs setfacl as an experiment to see whether the\nfilesystem supports ACLs, and skips the remaining tests if\nit does not. However, our setfacl run did not exercise the\nACLs very well, and some filesystems may support our initial\nsetfacl, but not the rest of the test.\n\nIn particular, some versions of ecryptfs will erroneously\napply the umask on top of an inherited directory ACL,\ncausing our tests to fail. Let's be more careful and make\nsure both that we can read back the user ACL we set, and\nthat the inherited ACL is propagated correctly. The latter\ncatches the ecryptfs bug, but may also catch other bugs\n(e.g., an implementation which does not handle inherited\nACLs at all).\n\nSince we're making the setup more complex, let's move it\ninto its own test. This will hide the output for us unless\nthe user wants to run \"-v\" to see it (and we don't need to\nbother printing anything about setfacl failing; the\nremaining tests will properly print \"skip\" due to the\nmissing prerequisite).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe ecryptfs response was that it is probably a bug, so I think we\nshould go with this (especially because it is general enough to\npotentially catch other weird errors).\n\n t/t1304-default-acl.sh | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 2b962cf..79045ab 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -14,16 +14,15 @@ umask 077\n # We need an arbitrary other user give permission to using ACLs. root\n # is a good candidate: exists on all unices, and it has permission\n # anyway, so we don't create a security hole running the testsuite.\n-\n-setfacl_out=\"$(setfacl -m u:root:rwx . 2>&1)\"\n-setfacl_ret=$?\n-\n-if test $setfacl_ret != 0\n-then\n-\tsay \"Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')\"\n-else\n-\ttest_set_prereq SETFACL\n-fi\n+test_expect_success 'checking for a working acl setup' '\n+\tif setfacl -m d:m:rwx -m u:root:rwx . &&\n+\t   getfacl . | grep user:root:rwx &&\n+\t   touch should-have-readable-acl &&\n+\t   getfacl should-have-readable-acl | egrep \"mask::?rw-\"\n+\tthen\n+\t\ttest_set_prereq SETFACL\n+\tfi\n+'\n \n if test -z \"$LOGNAME\"\n then\n-- \n1.7.11.rc1.4.g0d3b9b3\n"},{"id":"192995","messageId":"CALbm-EZJeNCts80sA6cxGHgGLUhAHREWRd7pUqZ0nUo22ixu0Q@mail.gmail.com","threadId":"30701","inReplyTo":"20120606132824.GA2597@sigill.intra.peff.net","subject":"Re: [PATCH] t1304: improve setfacl prerequisite setup","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2012-06-06T16:11:06Z","receivedAt":"2012-06-06T16:11:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Applying that patch, the test t1304 succeeds on my machine.\n\nTested-by: Stefan Beller <stefanbeller@googlemail.com>\n\n2012/6/6 Jeff King <peff@peff.net>:\n> t1304 first runs setfacl as an experiment to see whether the\n> filesystem supports ACLs, and skips the remaining tests if\n> it does not. However, our setfacl run did not exercise the\n> ACLs very well, and some filesystems may support our initial\n> setfacl, but not the rest of the test.\n>\n> In particular, some versions of ecryptfs will erroneously\n> apply the umask on top of an inherited directory ACL,\n> causing our tests to fail. Let's be more careful and make\n> sure both that we can read back the user ACL we set, and\n> that the inherited ACL is propagated correctly. The latter\n> catches the ecryptfs bug, but may also catch other bugs\n> (e.g., an implementation which does not handle inherited\n> ACLs at all).\n>\n> Since we're making the setup more complex, let's move it\n> into its own test. This will hide the output for us unless\n> the user wants to run \"-v\" to see it (and we don't need to\n> bother printing anything about setfacl failing; the\n> remaining tests will properly print \"skip\" due to the\n> missing prerequisite).\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> The ecryptfs response was that it is probably a bug, so I think we\n> should go with this (especially because it is general enough to\n> potentially catch other weird errors).\n>\n>  t/t1304-default-acl.sh | 19 +++++++++----------\n>  1 file changed, 9 insertions(+), 10 deletions(-)\n>\n> diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\n> index 2b962cf..79045ab 100755\n> --- a/t/t1304-default-acl.sh\n> +++ b/t/t1304-default-acl.sh\n> @@ -14,16 +14,15 @@ umask 077\n>  # We need an arbitrary other user give permission to using ACLs. root\n>  # is a good candidate: exists on all unices, and it has permission\n>  # anyway, so we don't create a security hole running the testsuite.\n> -\n> -setfacl_out=\"$(setfacl -m u:root:rwx . 2>&1)\"\n> -setfacl_ret=$?\n> -\n> -if test $setfacl_ret != 0\n> -then\n> -       say \"Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')\"\n> -else\n> -       test_set_prereq SETFACL\n> -fi\n> +test_expect_success 'checking for a working acl setup' '\n> +       if setfacl -m d:m:rwx -m u:root:rwx . &&\n> +          getfacl . | grep user:root:rwx &&\n> +          touch should-have-readable-acl &&\n> +          getfacl should-have-readable-acl | egrep \"mask::?rw-\"\n> +       then\n> +               test_set_prereq SETFACL\n> +       fi\n> +'\n>\n>  if test -z \"$LOGNAME\"\n>  then\n> --\n> 1.7.11.rc1.4.g0d3b9b3\n>\n"},{"id":"192997","messageId":"7vvcj4ml6a.fsf@alter.siamese.dyndns.org","threadId":"30701","inReplyTo":"20120606132824.GA2597@sigill.intra.peff.net","subject":"Re: [PATCH] t1304: improve setfacl prerequisite setup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-06T16:58:53Z","receivedAt":"2012-06-06T16:58:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> t1304 first runs setfacl as an experiment to see whether the\n> filesystem supports ACLs, and skips the remaining tests if\n> it does not. However, our setfacl run did not exercise the\n> ACLs very well, and some filesystems may support our initial\n> setfacl, but not the rest of the test.\n>\n> In particular, some versions of ecryptfs will erroneously\n> apply the umask on top of an inherited directory ACL,\n> causing our tests to fail. Let's be more careful and make\n> sure both that we can read back the user ACL we set, and\n> that the inherited ACL is propagated correctly. The latter\n> catches the ecryptfs bug, but may also catch other bugs\n> (e.g., an implementation which does not handle inherited\n> ACLs at all).\n>\n> Since we're making the setup more complex, let's move it\n> into its own test. This will hide the output for us unless\n> the user wants to run \"-v\" to see it (and we don't need to\n> bother printing anything about setfacl failing; the\n> remaining tests will properly print \"skip\" due to the\n> missing prerequisite).\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> The ecryptfs response was that it is probably a bug, so I think we\n> should go with this (especially because it is general enough to\n> potentially catch other weird errors).\n>\n>  t/t1304-default-acl.sh | 19 +++++++++----------\n>  1 file changed, 9 insertions(+), 10 deletions(-)\n>\n> diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\n> index 2b962cf..79045ab 100755\n> --- a/t/t1304-default-acl.sh\n> +++ b/t/t1304-default-acl.sh\n> @@ -14,16 +14,15 @@ umask 077\n>  # We need an arbitrary other user give permission to using ACLs. root\n>  # is a good candidate: exists on all unices, and it has permission\n>  # anyway, so we don't create a security hole running the testsuite.\n> -\n> -setfacl_out=\"$(setfacl -m u:root:rwx . 2>&1)\"\n> -setfacl_ret=$?\n> -\n> -if test $setfacl_ret != 0\n> -then\n> -\tsay \"Unable to use setfacl (output: '$setfacl_out'; return code: '$setfacl_ret')\"\n> -else\n> -\ttest_set_prereq SETFACL\n> -fi\n> +test_expect_success 'checking for a working acl setup' '\n> +\tif setfacl -m d:m:rwx -m u:root:rwx . &&\n> +\t   getfacl . | grep user:root:rwx &&\n> +\t   touch should-have-readable-acl &&\n> +\t   getfacl should-have-readable-acl | egrep \"mask::?rw-\"\n\nAt this point of the test, I do not think there is anything that can\ncause this check to be fooled by a user whose name is 'mask', but\nplease make it a habit to anchor the pattern at the left.  The tests\ndone in check_perms_and_acl do risk getting fooled by a user whose\nname is 'mask', and need to be updated, I think, but that is a\nseparate issue.\n\n> +\tthen\n> +\t\ttest_set_prereq SETFACL\n> +\tfi\n> +'\n>  \n>  if test -z \"$LOGNAME\"\n>  then\n"},{"id":"193043","messageId":"20120607090200.GA6087@sigill.intra.peff.net","threadId":"30701","inReplyTo":"7vvcj4ml6a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t1304: improve setfacl prerequisite setup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-07T09:02:00Z","receivedAt":"2012-06-07T09:02:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 06, 2012 at 09:58:53AM -0700, Junio C Hamano wrote:\n\n> > +test_expect_success 'checking for a working acl setup' '\n> > +\tif setfacl -m d:m:rwx -m u:root:rwx . &&\n> > +\t   getfacl . | grep user:root:rwx &&\n> > +\t   touch should-have-readable-acl &&\n> > +\t   getfacl should-have-readable-acl | egrep \"mask::?rw-\"\n> \n> At this point of the test, I do not think there is anything that can\n> cause this check to be fooled by a user whose name is 'mask', but\n> please make it a habit to anchor the pattern at the left.  The tests\n> done in check_perms_and_acl do risk getting fooled by a user whose\n> name is 'mask', and need to be updated, I think, but that is a\n> separate issue.\n\nI intentionally did not anchor it because the tests themselves did not,\nand I worried that that there was some subtle portability issue there\n(especially because of the magic \"let there be two colons\" part of the\npattern).\n\nIf it was just sloppiness in the original test, then yes, they should\nall be fixed.\n\n-Peff\n"},{"id":"193073","messageId":"7vlijzjdx6.fsf@alter.siamese.dyndns.org","threadId":"30701","inReplyTo":"20120607090200.GA6087@sigill.intra.peff.net","subject":"Re: [PATCH] t1304: improve setfacl prerequisite setup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-07T16:16:05Z","receivedAt":"2012-06-07T16:16:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I intentionally did not anchor it because the tests themselves did not,\n> and I worried that that there was some subtle portability issue there\n> (especially because of the magic \"let there be two colons\" part of the\n> pattern).\n\nOK, fair enough (the \"two colons\" one I think was for Solaris).\n"},{"id":"193120","messageId":"CA+sFfMd09Cp7qQdnhhQSTOp2p7jpOspf4HTEcbGihRZzMaz6Yg@mail.gmail.com","threadId":"30701","inReplyTo":"7vlijzjdx6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t1304: improve setfacl prerequisite setup","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-06-08T01:18:31Z","receivedAt":"2012-06-08T01:18:31Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, Jun 7, 2012 at 9:16 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jeff King <peff@peff.net> writes:\n>\n>> I intentionally did not anchor it because the tests themselves did not,\n>> and I worried that that there was some subtle portability issue there\n>> (especially because of the magic \"let there be two colons\" part of the\n>> pattern).\n>\n> OK, fair enough (the \"two colons\" one I think was for Solaris).\n\nIt was indeed for Solaris.  I don't recall any other differences in\nthe output between Solaris and Linux, so you could add the anchor and\nsee if anyone makes noise.  I no longer have access to Solaris to\ntest.\n\n-Brandon\n"}]}