Bug 95487 - data: Set GIO_USE_VFS=local in the .service file environment
Summary: data: Set GIO_USE_VFS=local in the .service file environment
Status: RESOLVED FIXED
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-19 09:09 UTC by Philip Withnall
Modified: 2016-06-06 16:34 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
data: Set GIO_USE_VFS=local in the .service file environment (1006 bytes, patch)
2016-05-19 09:10 UTC, Philip Withnall
Details | Splinter Review
data: Set GIO_USE_VFS=local in the environment (3.44 KB, patch)
2016-06-06 16:30 UTC, Philip Withnall
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Withnall 2016-05-19 09:09:59 UTC
Trivial patch attached.
Comment 1 Philip Withnall 2016-05-19 09:10:01 UTC
Created attachment 123901 [details] [review]
data: Set GIO_USE_VFS=local in the .service file environment

There is no need for polkit to ever use GVFS to load files from
non-local sources, so it’s best to avoid loading GVFS code, and to just
rely on the local implementation in GIO instead. This reduces the attack
surface of polkit.
Comment 2 Simon McVittie 2016-05-19 11:41:00 UTC
I'd merge it. (Am I considered to be a reviewer?)
Comment 3 Miloslav Trmac 2016-05-19 17:29:08 UTC
Thanks for the patch

Looking at https://developer.gnome.org/gio/stable/running-gio-apps.html :
> The following environment variables are only useful for debugging GIO itself or modules that it loads. They should not be set in a production environment.

Is that not so?

It does kind of seem that this should be possible to do in C code in principle (by manipulating extension point priorities or something?). Or at least having the GIO_USE_VFS=local environment variable a committed API of VFS would be more reassuring.

(I am not too worried about just loading the extension points if they are not used later at run time; if an attacker can write to /usr/lib64/gio/modules/ , they have more or less won already.)
Comment 4 Colin Walters 2016-06-02 16:17:25 UTC
Comment on attachment 123901 [details] [review]
data: Set GIO_USE_VFS=local in the .service file environment

Review of attachment 123901 [details] [review]:
-----------------------------------------------------------------

I'd prefer to do this in the C code, that way it happens if run by hand under gdb etc.

For reference that's what NetworkManager, ostree, and probably a bunch of other projects do.
Comment 5 Colin Walters 2016-06-02 16:26:24 UTC
(In reply to Miloslav Trmac from comment #3)
> Thanks for the patch
> 
> Looking at https://developer.gnome.org/gio/stable/running-gio-apps.html :
> > The following environment variables are only useful for debugging GIO itself or modules that it loads. They should not be set in a production environment.
> 
> Is that not so?

It's not so.

https://bugzilla.gnome.org/show_bug.cgi?id=767172
Comment 6 Philip Withnall 2016-06-06 16:30:55 UTC
Created attachment 124364 [details] [review]
data: Set GIO_USE_VFS=local in the environment

Updated to use setenv() at the start of each main() function.

---

There is no need for polkit to ever use GVFS to load files from
non-local sources, so it’s best to avoid loading GVFS code, and to just
rely on the local implementation in GIO instead. This reduces the attack
surface of polkit.

Implemented for the daemon, pkaction, pkcheck, pkexec and pkttyagent,
because none of them need remote file access.
Comment 7 Colin Walters 2016-06-06 16:32:00 UTC
Comment on attachment 124364 [details] [review]
data: Set GIO_USE_VFS=local in the environment

Review of attachment 124364 [details] [review]:
-----------------------------------------------------------------

LGTM, thanks!
Comment 8 Colin Walters 2016-06-06 16:34:47 UTC
There was some apparently corrupted UTF-8 in your patch instead of "'"...not sure which program in the chain was responsible.

Anyways fixed that, and pushed.  Thanks!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.