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

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

[Jeff King]

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

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

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

Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
clear that if a default ACL is present, it should be used, and umask
consulted only if it is not (so the umask should not be making a
difference in this case).

The reproduction recipe above shows the minimum required to trigger it;
adding a more realistic default ACL (with actual entries for users) does
not seem to make a difference.

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> [+cc ecryptfs@vger, as I think this is an ecryptfs bug]
>
> On Tue, Jun 05, 2012 at 10:28:13AM -0400, Jeff King wrote:
>
>> OK, here's the _real_ issue. Git creates with mode 0444, which should
>> still allow read in the mask. But it's the restrictive umask at the top
>> of the test script that causes the problem. Try this:
>> 
>>   setfacl -m m:rwx .
>>   perl -MFcntl -e 'sysopen(X, "a", O_WRONLY|O_CREAT, 0444)'
>>   umask 077
>>   perl -MFcntl -e 'sysopen(X, "b", O_WRONLY|O_CREAT, 0444)'
>>   getfacl a b
>> 
>> On ext4, both files will have the read bit set in the mask. On ecryptfs,
>> "b" will have an empty mask. I think the wrong thing is that we should
>> not be respecting umask at all when default ACLs are in play, and
>> ecryptfs is getting that wrong. But I'm having trouble digging up an
>> authoritative source.
>
> Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
> clear that if a default ACL is present, it should be used, and umask
> consulted only if it is not (so the umask should not be making a
> difference in this case).
>
> The reproduction recipe above shows the minimum required to trigger it;
> adding a more realistic default ACL (with actual entries for users) does
> not seem to make a difference.

Thanks; so combining the above with your earlier patch to 1304 we
would have a good detection for SETFACL prerequisite?
 

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

[Jeff King]

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

> >>   setfacl -m m:rwx .
> >>   perl -MFcntl -e 'sysopen(X, "a", O_WRONLY|O_CREAT, 0444)'
> >>   umask 077
> >>   perl -MFcntl -e 'sysopen(X, "b", O_WRONLY|O_CREAT, 0444)'
> >>   getfacl a b
> [...]
> >
> > Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
> > clear that if a default ACL is present, it should be used, and umask
> > consulted only if it is not (so the umask should not be making a
> > difference in this case).
> >
> > The reproduction recipe above shows the minimum required to trigger it;
> > adding a more realistic default ACL (with actual entries for users) does
> > not seem to make a difference.
> 
> Thanks; so combining the above with your earlier patch to 1304 we
> would have a good detection for SETFACL prerequisite?

Yes, I think we can detect it reliably. I'd like to hear back from
ecryptfs folks before making a final patch, though. It may be that there
is some subtle reason for their behavior, and I want to make sure before
we write it off as just buggy.

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
>
>> >>   setfacl -m m:rwx .
>> >>   perl -MFcntl -e 'sysopen(X, "a", O_WRONLY|O_CREAT, 0444)'
>> >>   umask 077
>> >>   perl -MFcntl -e 'sysopen(X, "b", O_WRONLY|O_CREAT, 0444)'
>> >>   getfacl a b
>> [...]
>> >
>> > Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
>> > clear that if a default ACL is present, it should be used, and umask
>> > consulted only if it is not (so the umask should not be making a
>> > difference in this case).
>> >
>> > The reproduction recipe above shows the minimum required to trigger it;
>> > adding a more realistic default ACL (with actual entries for users) does
>> > not seem to make a difference.
>> 
>> Thanks; so combining the above with your earlier patch to 1304 we
>> would have a good detection for SETFACL prerequisite?
>
> Yes, I think we can detect it reliably. I'd like to hear back from
> ecryptfs folks before making a final patch, though. It may be that there
> is some subtle reason for their behavior, and I want to make sure before
> we write it off as just buggy.

Sensible; thanks.

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

[Tyler Hicks]

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

On 2012-06-05 12:44:39, Jeff King wrote:
> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
> 
> > >>   setfacl -m m:rwx .
> > >>   perl -MFcntl -e 'sysopen(X, "a", O_WRONLY|O_CREAT, 0444)'
> > >>   umask 077
> > >>   perl -MFcntl -e 'sysopen(X, "b", O_WRONLY|O_CREAT, 0444)'
> > >>   getfacl a b
> > [...]
> > >
> > > Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
> > > clear that if a default ACL is present, it should be used, and umask
> > > consulted only if it is not (so the umask should not be making a
> > > difference in this case).
> > >
> > > The reproduction recipe above shows the minimum required to trigger it;
> > > adding a more realistic default ACL (with actual entries for users) does
> > > not seem to make a difference.
> > 
> > Thanks; so combining the above with your earlier patch to 1304 we
> > would have a good detection for SETFACL prerequisite?
> 
> Yes, I think we can detect it reliably. I'd like to hear back from
> ecryptfs folks before making a final patch, though. It may be that there
> is some subtle reason for their behavior, and I want to make sure before
> we write it off as just buggy.

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

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

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

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

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

Tyler

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

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

[Stefan Beller]

Hi Tyler et all,

thanks for all your help :)

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

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

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

A happy git user,
Stefan



2012/6/5 Tyler Hicks <tyhicks@canonical.com>:
> On 2012-06-05 12:44:39, Jeff King wrote:
>> On Tue, Jun 05, 2012 at 09:31:54AM -0700, Junio C Hamano wrote:
>>
>> > >>   setfacl -m m:rwx .
>> > >>   perl -MFcntl -e 'sysopen(X, "a", O_WRONLY|O_CREAT, 0444)'
>> > >>   umask 077
>> > >>   perl -MFcntl -e 'sysopen(X, "b", O_WRONLY|O_CREAT, 0444)'
>> > >>   getfacl a b
>> > [...]
>> > >
>> > > Reading the withdrawn posix 1003.1e and "man 5 acl", it seems pretty
>> > > clear that if a default ACL is present, it should be used, and umask
>> > > consulted only if it is not (so the umask should not be making a
>> > > difference in this case).
>> > >
>> > > The reproduction recipe above shows the minimum required to trigger it;
>> > > adding a more realistic default ACL (with actual entries for users) does
>> > > not seem to make a difference.
>> >
>> > Thanks; so combining the above with your earlier patch to 1304 we
>> > would have a good detection for SETFACL prerequisite?
>>
>> Yes, I think we can detect it reliably. I'd like to hear back from
>> ecryptfs folks before making a final patch, though. It may be that there
>> is some subtle reason for their behavior, and I want to make sure before
>> we write it off as just buggy.
>
> It is likely a bug in the eCryptfs filesystem stacking code.
>
> However, using the above script, I get the same results on eCryptfs as I
> do on ext4 in the Ubuntu 12.04 (Precise) LTS:
>
> # file: a
> # owner: tyhicks
> # group: tyhicks
> user::r--
> group::r--
> other::r--
>
> # file: b
> # owner: tyhicks
> # group: tyhicks
> user::r--
> group::---
> other::---
>
> Stefan - can you specify which LTS release you're running as well as the
> output of `cat /proc/version_signature`? Thanks!
>
> Tyler