GNOME Bugzilla – Bug 755472
Use secure_getenv
Last modified: 2018-02-08 12:38:08 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.
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
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).
Ah right, now I remember that. Thanks.
I also checked for SUID users (again), and did not find any.
Created attachment 311970 [details] [review] 0001-libgirepository-Refuse-to-run-in-setuid-applications.patch Florian, want to review this?
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.
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.
https://git.gnome.org/browse/gobject-introspection/commit/?id=98bb6c91b710a95efe4cfeb303daeec3381b9c98
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) $ ```
In the dbus commit above, changing it to use getauxval(AT_SECURE) instead of doing a uid != euid check would break gnome-keyring.
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.
For dbus, see https://bugzilla.gnome.org/show_bug.cgi?id=755472#c1
I see, dbus still appears to have issues with fscaps and SELinux transitions, then. Thanks.
Downstream regression discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1285991
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.
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.
See comment 15. It turned out libgirepository needs hardening against untrusted environment variables, and using secure_getenv is appropriate.
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.
-- 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.