After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755472 - Use secure_getenv
Use secure_getenv
Status: RESOLVED OBSOLETE
Product: gobject-introspection
Classification: Platform
Component: libgirepository
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-09-23 15:23 UTC by Florian Weimer
Modified: 2018-02-08 12:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-libgirepository-Refuse-to-run-in-setuid-applications.patch (1.75 KB, patch)
2015-09-23 18:14 UTC, Colin Walters
none Details | Review

Description Florian Weimer 2015-09-23 15:23:27 UTC
As a safety measure against use from SUID/SGID processes, secure_getenv should be used in girepository/girepository.c to obtain the value of the GI_TYPELIB_PATH environment variable (and not g_getenv).

Alternatively, the library could refuse to work if getauxval(AT_SECURE) is true.
Comment 1 Colin Walters 2015-09-23 15:30:37 UTC
I assume you're filing this because there does exist a setuid application using g-i - what one is it?

The last time this came up for me was in the context of libdbus and X.org being setuid.  I argued then that is the responsibility of setuid apps to audit their dependencies.

See:
http://cgit.freedesktop.org/dbus/dbus/commit/?id=a52319bc294d05445fd8aa8f4a7f759c34558b5d
Comment 2 Florian Weimer 2015-09-23 15:32:43 UTC
No, I'm just moving the remaining unfixed downstream bugs upstream.  This one corresponds to: https://bugzilla.redhat.com/show_bug.cgi?id=1026964

The general situation has not changed at all.  It's just a hardening proposal worth considering (in my humble opinion).
Comment 3 Colin Walters 2015-09-23 15:36:03 UTC
Ah right, now I remember that.  Thanks.
Comment 4 Florian Weimer 2015-09-23 15:39:59 UTC
I also checked for SUID users (again), and did not find any.
Comment 5 Colin Walters 2015-09-23 18:14:08 UTC
Created attachment 311970 [details] [review]
0001-libgirepository-Refuse-to-run-in-setuid-applications.patch

Florian, want to review this?
Comment 6 Florian Weimer 2015-09-24 09:59:17 UTC
Review of attachment 311970 [details] [review]:

Maybe use _exit instead of exit, to avoid running cleanup routines?

You could add a test case, but it's not straightforward.  It would not have to run as root if you use an SGID binary and a supplementary group.
Comment 7 Colin Walters 2015-09-24 15:14:31 UTC
Changed to use _exit.

I did test this manually (with two local users).  But any other kind of test (even the setgid one) can't run in traditional non-root rpm %check type setups as we can't know we'll have a supplementary group.  It would need to be an external container/VM test, living outside of this repo.
Comment 9 Colin Walters 2015-09-24 15:21:29 UTC
Florian, before I forget - if we continue farther down this path of modifying libraries to abort when AT_SECURE, note at least:

```
$ rpm -q gnome-keyring
gnome-keyring-3.14.0-1.el7.x86_64
$ getcap /usr/bin/gnome-keyring-daemon 
/usr/bin/gnome-keyring-daemon = cap_ipc_lock+ep
$ ldd /usr/bin/gnome-keyring-daemon 
	linux-vdso.so.1 =>  (0x00007ffd90140000)
	libgcr-base-3.so.1 => /lib64/libgcr-base-3.so.1 (0x00007faa3142a000)
	libgck-1.so.0 => /lib64/libgck-1.so.0 (0x00007faa311f3000)
	libgio-2.0.so.0 => /lib64/libgio-2.0.so.0 (0x00007faa30e7c000)
	libp11-kit.so.0 => /lib64/libp11-kit.so.0 (0x00007faa30c36000)
	libgobject-2.0.so.0 => /lib64/libgobject-2.0.so.0 (0x00007faa309e6000)
	libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x00007faa306ae000)
	libdbus-1.so.3 => /lib64/libdbus-1.so.3 (0x00007faa30466000)
	libgcrypt.so.11 => /lib64/libgcrypt.so.11 (0x00007faa301e5000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007faa2ffe0000)
	libgpg-error.so.0 => /lib64/libgpg-error.so.0 (0x00007faa2fddb000)
	libcap-ng.so.0 => /lib64/libcap-ng.so.0 (0x00007faa2fbd5000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007faa2f9b8000)
	libc.so.6 => /lib64/libc.so.6 (0x00007faa2f5f7000)
	libgmodule-2.0.so.0 => /lib64/libgmodule-2.0.so.0 (0x00007faa2f3f3000)
	libgthread-2.0.so.0 => /lib64/libgthread-2.0.so.0 (0x00007faa2f1f0000)
	libffi.so.6 => /lib64/libffi.so.6 (0x00007faa2efe8000)
	libz.so.1 => /lib64/libz.so.1 (0x00007faa2edd2000)
	libselinux.so.1 => /lib64/libselinux.so.1 (0x00007faa2ebac000)
	libresolv.so.2 => /lib64/libresolv.so.2 (0x00007faa2e992000)
	librt.so.1 => /lib64/librt.so.1 (0x00007faa2e78a000)
	/lib64/ld-linux-x86-64.so.2 (0x00007faa319da000)
	libpcre.so.1 => /lib64/libpcre.so.1 (0x00007faa2e528000)
	liblzma.so.5 => /lib64/liblzma.so.5 (0x00007faa2e303000)
$
```
Comment 10 Colin Walters 2015-09-24 15:22:08 UTC
In the dbus commit above, changing it to use getauxval(AT_SECURE) instead of doing a uid != euid check would break gnome-keyring.
Comment 11 Florian Weimer 2015-09-24 15:25:34 UTC
Which dbus commit?

I agree that applying the same thing to libraries such as glib can be tricky.

gnome-keyring could probably use kernel keyrings to avoid CAP_IPC_LOCK privilege.
Comment 12 Colin Walters 2015-09-24 16:07:35 UTC
For dbus, see https://bugzilla.gnome.org/show_bug.cgi?id=755472#c1
Comment 13 Florian Weimer 2015-09-30 11:34:32 UTC
I see, dbus still appears to have issues with fscaps and SELinux transitions, then.  Thanks.
Comment 14 Colin Walters 2015-11-27 17:29:48 UTC
Downstream regression discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1285991
Comment 15 Colin Walters 2015-12-09 21:27:28 UTC
I backed this out https://git.gnome.org/browse/gobject-introspection/commit/?id=13f7ca3a3e3f823690add83dd8bfada52da559d2

Florian, if you have any thoughts on this let me know.  Maybe there's a way to teach the dbus setuid launcher to sanitize things, but long term I think our best bet is to simply stop using setuid binaries entirely.

We're slowly getting closer with Xorg not being setuid anymore etc.
Comment 16 Florian Weimer 2015-12-09 21:41:56 UTC
AT_SECURE is also set for SELinux trust transitions.  To make those transitions safe, girepository needs to be hardened in a similar way to SUID programs, e.g., use secure_getenv where applicable, treat certain file system paths as suspect, and so on.
Comment 17 Florian Weimer 2016-04-23 16:04:44 UTC
See comment 15.  It turned out libgirepository needs hardening against untrusted environment variables, and using secure_getenv is appropriate.
Comment 18 Colin Walters 2016-05-09 14:07:36 UTC
The domain transition here was from the system bus to an app - in practice if the system bus is compromised we're in a pretty bad place anyways.

To rephrase, I don't think that downstream regression shows there's an important security issue.  Do you?

In the broader picture - I'm still worried about the potential side effects of a campaign to use secure_getenv given how often domain transitions occur.
Comment 19 GNOME Infrastructure Team 2018-02-08 12:38:08 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gobject-introspection/issues/143.