[GSOC] Extended File Attributes not preserved by most editors

32 views
Skip to first unread message

Andrew Morgan

unread,
Jun 9, 2017, 11:48:16 PM6/9/17
to qubes...@googlegroups.com
Hey all,

So I've run into a bit of a road-block with keeping track of untrusted
files. My original plan of marking files with an Extended File Attribute
[1] does not seem to be as practical due to many popular editor's (vim,
gedit, etc) behavior of deleting and writing a new file from their buffer.

What this means is that if a file is marked with the
"user.qubes.untrusted" extended file attribute, then edited in vim, that
file will no longer carry that attribute. I believe marking files as
always open-able in disposable VMs becomes quite useless when they come
back without that mark, forcing the user to re-mark them again.

Perhaps qvm-open-in-(d)vm could be modified to re-apply any file
attributes when returning files from the destination VM? That still
doesn't address the issue of editing the file locally destroying the
attribute, but maybe that isn't an issue since the file is untrusted and
shouldn't be opened locally anyways?

Another way to mark files is to just list and later read their
filepaths, line-by-line. However if one is marking a folder of thousands
of files as untrusted, that tracking file can quickly become very long.
Perhaps a database option would suffice, do any of our tools use a
database already? I'd like to keep these efficient as possible without
harming the UX.

Any thoughts or ideas are appreciated.

Thanks,
Andrew Morgan

[1]: https://www.freedesktop.org/wiki/CommonExtendedAttributes/

signature.asc

Patrik Hagara

unread,
Jun 10, 2017, 6:17:39 AM6/10/17
to Andrew Morgan, qubes...@googlegroups.com
On 06/10/2017 05:47 AM, Andrew Morgan wrote:
> Another way to mark files is to just list and later read their
> filepaths, line-by-line. However if one is marking a folder of thousands
> of files as untrusted, that tracking file can quickly become very long.
> Perhaps a database option would suffice, do any of our tools use a
> database already? I'd like to keep these efficient as possible without
> harming the UX.

How about using a DB (I'm not sure whether qubesdb would yield
reasonable performance, but SQLite should work just fine for VM-local
stuff) and monitoring files/folders via the inotify [0] API to keep
track of new files created in untrusted folders, files getting
deleted/renamed and generally avoiding race conditions?

The API even provides events for extended attribute changes, so you
could, theoretically, reapply them when discarded by text editors.
However, this would likely result in more race-y behavior.

One possible solution might be to create a FUSE [1] stub file system
driver that would proxy all the FS commands to a real driver (eg. ext4),
but discard (refuse with EPERM or similar error) all calls violating the
"only use in dispVM" per-file/folder policy as listed in its database.

Alternatively, it may be possible to use SELinux [2] object tags for
Mandatory Access Control (MAC) -- which should be, compared to extended
file attributes, sticky -- and write a policy that would refuse to let
those files be open()'ed. This approach would IMO work the best, as it
should (presumably, since SELinux is security-oriented) avoid all race
conditions. Plus, I'd imagine it would be easier to code than a custom
stub FUSE FS driver.


Cheers,
Patrik


[0] https://linux.die.net/man/7/inotify
[1] https://en.wikipedia.org/wiki/Filesystem_in_Userspace
[2] https://selinuxproject.org/page/Main_Page

signature.asc

Marek Marczykowski-Górecki

unread,
Jun 10, 2017, 6:53:16 AM6/10/17
to Patrik Hagara, Andrew Morgan, qubes...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Sat, Jun 10, 2017 at 12:17:29PM +0200, Patrik Hagara wrote:
> On 06/10/2017 05:47 AM, Andrew Morgan wrote:
> > Another way to mark files is to just list and later read their
> > filepaths, line-by-line. However if one is marking a folder of thousands
> > of files as untrusted, that tracking file can quickly become very long.
> > Perhaps a database option would suffice, do any of our tools use a
> > database already? I'd like to keep these efficient as possible without
> > harming the UX.
>
> How about using a DB (I'm not sure whether qubesdb would yield
> reasonable performance,

QubesDB is _not_ a persistent storage. It's a configuration exposed by
dom0 to a VM, re-created at each VM startup.

> but SQLite should work just fine for VM-local
> stuff) and monitoring files/folders via the inotify [0] API to keep
> track of new files created in untrusted folders, files getting
> deleted/renamed and generally avoiding race conditions?

This will easily desynchronize. And if that happens, a file would be
opened locally instead of in DispVM, which is a security issue.

> The API even provides events for extended attribute changes, so you
> could, theoretically, reapply them when discarded by text editors.
> However, this would likely result in more race-y behavior.
>
> One possible solution might be to create a FUSE [1] stub file system
> driver that would proxy all the FS commands to a real driver (eg. ext4),
> but discard (refuse with EPERM or similar error) all calls violating the
> "only use in dispVM" per-file/folder policy as listed in its database.

That's an interesting idea, but IMO great overkill. Proxying every fs
access, just to filter few files in Downloads directory (or such).

> Alternatively, it may be possible to use SELinux [2] object tags for
> Mandatory Access Control (MAC) -- which should be, compared to extended
> file attributes, sticky -- and write a policy that would refuse to let
> those files be open()'ed. This approach would IMO work the best, as it
> should (presumably, since SELinux is security-oriented) avoid all race
> conditions. Plus, I'd imagine it would be easier to code than a custom
> stub FUSE FS driver.

What about simply using file permissions? When a file is marked as "open
in Disposable VM", it gets extended attribute and loose read/write
access (`chmod 0 file`). Then qvm-open-in-dvm will check file permission
and extended attribute, and if file is marked as "open in Disposable
VM", it will temporarily add read access (for for the open() syscall),
read it, and send it to Disposable VM. And similarly when retrieving the
file back - it should adjust both extended attributes (preserve it) and
set file permissions to "0".

This will make user mistakes harder (not impossible, but at least
opening file by launching an editor and then selecting a file will not
work), and also allow better control over those extended attributes and
who access those files.

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJZO8+VAAoJENuP0xzK19csT/AH/RRTmq1zF5wBA7T1LFztAqGY
JZvV1OoAJ79Iv5qHmibGAstDgmVa6kffbhz3jQMCxyad7EDK735box/o1wbvFf+g
bgnXxNqIVoSFdtdunjpacpA4H/H3aXWpCmwP53b+6tTlkTrObrBRd7seB5hZgDAP
4D7rXfSK7yPP/UBrJ1DlDxqknzUXeQy0tikLXKBbA8cHvdgChDnwJXhxkVl81f0H
oPnwGsSR5xVVWz2/mDlcY7hP+MCIV9TfvcvMjolG3OkbZIcfxVUvbfM8k3XPWy03
mktnSaXxUL3TT4+icEnuSPi7WZKRZxie9nWOKdcJeEK4RDM6vW3i3OJ1k5rOe5E=
=Ic5i
-----END PGP SIGNATURE-----

Andrew Morgan

unread,
Jun 12, 2017, 10:33:42 PM6/12/17
to qubes...@googlegroups.com
Hey guys, I've implemented the `chmod 0` technique and it works quite
well compared to other methods of preventing modification as it doesn't
require root.

The one issue with this is that preventing read permissions additionally
denies reading the extended file permissions. As such our tools need to
'unlock' the file by applying the appropriate permissions before
checking the extended file attributes.

As far as I can tell there are no security issues with unlocking these
files temporarily. All I could perhaps think of is if there's a
theoretical exploit in the thumbnail-parsing functionality of the file
manager, which may be automatically asked to parse the file when it's
unlocked.

In the mean time, I'll go ahead with this method of locking and checking
files, as well as preserving these attributes from within qvm-open-in-(d)vm.

Thanks,
Andrew Morgan

signature.asc

Jean-Philippe Ouellet

unread,
Jun 13, 2017, 3:30:14 AM6/13/17
to Andrew Morgan, qubes-devel
On Mon, Jun 12, 2017 at 10:33 PM, Andrew Morgan <an...@openmailbox.org> wrote:
> As far as I can tell there are no security issues with unlocking these
> files temporarily. All I could perhaps think of is if there's a
> theoretical exploit in the thumbnail-parsing functionality of the file
> manager, which may be automatically asked to parse the file when it's
> unlocked.

All such "thumbnail-parsing functionality of the file manager" is
intended to be turned off. [1][2] If you've discovered a case where it
is not, please open an issue!

[1]: https://github.com/QubesOS/qubes-issues/issues/813
[2]: https://github.com/QubesOS/qubes-issues/issues/1108

Marek Marczykowski-Górecki

unread,
Jun 13, 2017, 3:40:09 AM6/13/17
to Andrew Morgan, qubes...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Mon, Jun 12, 2017 at 07:33:30PM -0700, Andrew Morgan wrote:
> Hey guys, I've implemented the `chmod 0` technique and it works quite
> well compared to other methods of preventing modification as it doesn't
> require root.
>
> The one issue with this is that preventing read permissions additionally
> denies reading the extended file permissions. As such our tools need to
> 'unlock' the file by applying the appropriate permissions before
> checking the extended file attributes.

Another issue is file copying (or moving in some cases) will be harder.
But maybe it's a feature?

> As far as I can tell there are no security issues with unlocking these
> files temporarily. All I could perhaps think of is if there's a
> theoretical exploit in the thumbnail-parsing functionality of the file
> manager, which may be automatically asked to parse the file when it's
> unlocked.

Yes, see Jean-Philippe's answer.

> In the mean time, I'll go ahead with this method of locking and checking
> files, as well as preserving these attributes from within qvm-open-in-(d)vm.

+1

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJZP5bPAAoJENuP0xzK19csDvYH/3wagwIz921AmeIF6IFvDiDb
AJZSXq48e1/mgYdOQCyToyQn1FtMUaICKrYSK0FDbM7JDRtAJ8f9TlF/DElmsOkn
5oUth3o7HEiqfsgydNDPaB+foQgWWOrE67E8LJzoJ7V1vqNPqcpr6N0yv0+8FBVH
W+1PgmM2ETW2eH1JIMLAfFNQgNdBOtJn/qd5zz8JJNMAJ2fR7g0FD280DMhtb0aX
sVsgtmvrmbN5Y4uNhpfOjgfOee+/vPomQjJKHGTd1XlYO9/d2WO8Wen/Gd7dFExn
z8q+0T7yiS1GrxCm8Q0glC6CSalNaq16fdT0r1E8AGI5RYHak+lWx1h1s7OB/sk=
=v8DU
-----END PGP SIGNATURE-----
Reply all
Reply to author
Forward
0 new messages