e2fsprogs - could not preserve ACL permissions : The getxattr() returns with (EINVAL)

[Shrawan Kumar]

The original installed e2fsprogs did not support mke2fs -d /directory " option . So , git cloned from e2fsprogs packages from repository and installed it and followed below steps to reproduce this :

1. set the ACL rules as below to one of the binary :

    $ setfacl -m u:vipatil:r-- rootfs/usr/bin/helloworld
    $ getfacl rootfs/usr/bin/helloworld
    # file: rootfs/usr/bin/helloworld
    # owner: shkumar
    # group: hardev
    user::rwx
    user:vipatil:r--
    group::---
    mask::r--
    other::---

2. $ dd if=/dev/zero of=test.ext4 bs=1M count=60

3. $ mke2fs -t ext4 test.ext4 -d rootfs/
    mke2fs 1.43.3 (04-Sep-2016)
    Discarding device blocks: done
    Creating filesystem with 61440 1k blocks and 15360 inodes
    Filesystem UUID: 495713b3-5f1f-427a-8359-a736dfb2ece9
    Superblock backups stored on blocks:
        8193, 24577, 40961, 57345

Allocating group tables: done
Writing inode tables: done
Creating journal (4096 blocks): done
Copying files into the device: done
Writing superblocks and filesystem accounting information: done

4. sudo mount -o loop,acl,user_xattr,rw,sync test.ext4 mountpoint

5.@/mountpoint$ getfacl usr/bin/helloworld
    getfacl: usr/bin/helloworld: Invalid argument

[Tyler Hicks (tyhicks)]

Thanks for taking the time to report this bug and helping to make Ubuntu better. We appreciate the difficulties you are facing, but this appears to be a "regular" (non-security) bug. I have unmarked it as a security issue since this bug does not show evidence of allowing attackers to cross privilege boundaries nor directly cause loss of data/privacy. Please feel free to report any other bugs you may find.

[Tyler Hicks (tyhicks)]

Hello and thanks for the bug report!

I was tempted to mark this bug as invalid since you compiled e2fsprogs from git and aren't using Ubuntu's e2fsprogs. However, I see that yakkety (16.10) and zesty both have a new enough e2fsprogs to support the -d option. I tried your reproducer in yakkety, using e2fsprogs 1.43.3-1, and can confirm what you're seeing.

It looks like this is a known issue:

  https://www.spinics.net/lists/linux-ext4/msg43058.html

A note to anyone thinking about fixing this bug in Ubuntu: The Ubuntu Security team doesn't consider this to be a security issue and doesn't have plans to fix this issue. Feel free to take a fix through the SRU process.

[Shrawan Kumar (shkumar31)]

Hello All,

As an additional info , I can see that POSIX capabilities and SMACK rules are getting preserved. The problem is only with ACL rules.

Regards
Shrawan

[Sojan James (sojan-james)]

Hello,

I have a fix for this and the patch is attached. I couldn't find the right mailing list for e2fsprogs. Until I find that, this is a good place to upload the patch.

mke2fs was copying the ACL in the POSIX extended attribute format. The attached patch converts it to for format expected by the file-system.

Regards,
Sojan

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "0001-mke2fs-handle-ACL-in-xattrs-correctly.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Theodore Ts'o (tytso)]

Thanks for the patch; it definitely helps identify the problem. I don't think it is a completely correct change, however.

The problem is that it's doing the translation in write_xattrs_to_buffer(), but it is not making the translation in the opposite direction in read_xattrs_to_buffer(). This means if some other program using this library function --- for example, fuse2fs, reads in an existing xattr block with an ACL, it will read it the on-disk format, _not_ translate it to a standard acl format, and then write_xattrs_to_buffer() will take as input an acl encoded in the on-disk format, and then try to convert it from lgetxattr format to on-disk format again, and Much Hilarity will result.

I also wonder if write_xattrs_to_buffer() is the right place to make this conversion, or rather ext2fs_{read,write}_ext_attr3(). Then what is stored in the handle structure is the on-disk encoding, and we would do the translation at the higher layer function rather than the low-level read/write functions.

[Sojan James (sojan-james)]

Hello Theodore,

Thank you for the feedback. I only had mke2fs in mind when trying to look for a fix as this solves my immediate problem - to be able to preserve ACLs when creating a filesystem via the Yocto build system.

I agree that the ext2fs_{read,write}_ functions may be a better place to fix this. I did have an inkling that fixing this in write_xattrs_to_buffer might not be the ideal fix and I was hoping for exactly this - feedback from someone who knows the source better.

The biggest challenge was identifying the cause. Now I hope someone else can step up and put the fix in the right location. My problem is solved for now. :-)

Regards,
Sojan

[Theodore Ts'o (tytso)]

I was hoping you would be interested in further refining the patch which you put forward. But if you don't have the time or interest, that's fine, I'll put it on my queue of things on my todo list. (Or maybe someone from Canonical will be interested in picking this up.)

[Sojan James (sojan-james)]

Hi Theodore,

The write_xattrs_to_buffer is already doing other modifications to the xattrs, like sorting keys. (look for qsort). The changes I made are a logical extension to that.

Further refinement might need a larger bunch of changes. I don't have the bandwidth for this now unfortunately.

Saving ACLs with mke2fs is completely broken now. This patch fixes the problem and I think it will be useful to merge this in so that others can benefit this.

Regards,
Sojan

[Theodore Ts'o (tytso)]

I've committed a complete fix to the master branch of the e2fsprogs git tree.

One of the reasons why I don't want to commit a partial fix is because the potential for corruption of production file systems by programs such as fuse2fs was extremely high. Users of mke2fs -d are generally embedded developers. Users of fuse2fs are more likely to be civilians.

We've had this bug for a long time, and no one has complained. Taking some time to have a fix that doesn't corrupt file systems is far more important to me as the maintainer. For any distribution, but especially for Ubuntu, civilian users >>> embedded developers.

[Sojan James (sojan-james)]

Thank you Theodore.

[Shrawan Kumar (shkumar31)]

Thanks a lot !!

Regards
Shrawan

On Sun, Jan 29, 2017 at 11:27 AM, Sojan James <email address hidden> wrote:

> Thank you Theodore.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1645232
>
> Title:
> e2fsprogs - could not preserve ACL permissions : The getxattr()
> returns with (EINVAL)
>
> Status in e2fsprogs package in Ubuntu:
> Confirmed
>
> Bug description:
> The original installed e2fsprogs did not support mke2fs -d /directory
> " option . So , git cloned from e2fsprogs packages from repository
> and installed it and followed below steps to reproduce this :
>
> 1. set the ACL rules as below to one of the binary :
>
> $ setfacl -m u:vipatil:r-- rootfs/usr/bin/helloworld
> $ getfacl rootfs/usr/bin/helloworld
> # file: rootfs/usr/bin/helloworld
> # owner: shkumar
> # group: hardev
> user::rwx
> user:vipatil:r--
> group::---
> mask::r--
> other::---
>
> 2. $ dd if=/dev/zero of=test.ext4 bs=1M count=60
>
> 3. $ mke2fs -t ext4 test.ext4 -d rootfs/
> mke2fs 1.43.3 (04-Sep-2016)
> Discarding device blocks: done
> Creating filesystem with 61440 1k blocks and 15360 inodes
> Filesystem UUID: 495713b3-5f1f-427a-8359-a736dfb2ece9
> Superblock backups stored on blocks:
> 8193, 24577, 40961, 57345
>
> Allocating group tables: done
> Writing inode tables: done
> Creating journal (4096 blocks): done
> Copying files into the device: done
> Writing superblocks and filesystem accounting information: done
>
> 4. sudo mount -o loop,acl,user_xattr,rw,sync test.ext4 mountpoint
>
> 5.@/mountpoint$ getfacl usr/bin/helloworld
> getfacl: usr/bin/helloworld: Invalid argument
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/e2fsprogs/+bug/
> 1645232/+subscriptions
>

[Shrawan Kumar (shkumar31)]

Thanks Theodore & Sojan!!

Regards
Shrawan

[Theodore Ts'o (tytso)]

Fix is in e2fsprogs 1.43.4

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

I am getting this in bionic, with e2fsprogs 1.44.1-1. In fact, I have tried with e2fsprogs upstream and happens as well. I have simply followed the instructions in the bug description to reproduce.

So, somewhat, although part of the patch was included upstream, it did not fix completely the issue.

[Theodore Ts'o (tytso)]

This was a regression that was reported at:

https://bugs.launchpad.net/ubuntu/+source/e2fsprogs/+bug/1807288

... a fix has been applied uptream and the fix has been cherry picked for Ubuntu. See the above bug for details.

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

@tytso awesome, thanks for pointing out.