PGP: Set a default creation SELinux labels on GnuPG directories #287
Conversation
ppisar
force-pushed
the
getfscreatecon
branch
from
d5a1b84
to
ee46196
Compare
|
"DNF5 Integration Tests (--tags dnf5 --command dnf5)" failures are unrelated: |
|
The only DNF4 Integration Tests failure is: This seems related to a stricter RPM Sequoia than to my SELinux patch. |
|
I rerun "DNF4 Integration Tests" and the "No binding signature" error did no reproduce. Maybe it's a time race and a 2-second old key is too young to be accepted by Sequoia. When I created key locally in Fedora 40, the key had a valid self-signature and RPM accepted it. |
ppisar
force-pushed
the
getfscreatecon
branch
from
ee46196
to
223f6f4
Compare
ppisar
changed the title
|
LGTM, the changes are very well-documented. The only minor suggestion, I'd consider swapping the operands in the introduced if statements, f.e., using |
|
I used a reverse order to prevent from accidental assignments ("variable = NULL" instead of "variable == NULL"). But I can rewrite it if you like it like more legible and and less defensive notation. |
This is another way how to fix mismatching SELinux context on /run/user directories without moving the directories to /run/gnupg/user. librepo used to precreate the directory in /run/user to make sure a GnuPG agent executed by GPGME library places its socket there. The directories there are normally created and removed by systemd (logind PAM session). librepo created them for a case when a package manager is invoked out of systemd session, before the super user logs in. E.g. by a timer job to cache repository metadata. A problem was when this out-of-session process was a SELinux-confined process creating files with its own SELinux label different from a DNF program. Then the directory was created with a SELinux label different from the one expected by systemd and when logging out a corresponding user, the mismatching label clashed with systemd. This patch fixes the issue by choosing a SELinux label of those directories to the label defined in a default SELinux file context database. This patch adds a new -DENABLE_SELINUX=OFF CMake option to disable the new dependency on libselinux. A default behavior is to support SELinux only if GPGME backend is selected with -DUSE_GPGME=ON. https://issues.redhat.com/browse/RHEL-10720
ppisar
force-pushed
the
getfscreatecon
branch
from
223f6f4
to
81602bb
Compare
|
I reversed the NULL conditions and corrected a typo in a comment. |
|
This code should be calling |
|
That would introduce a race between the check and a later use. The code is written in a way that any SELinux-related failure is nonfatal and the code continues. |
|
On Tue, Jun 4, 2024, at 4:19 AM, Petr Pisar wrote:
That would introduce a race between the check and a later use.
`is_selinux_enabled` is system wide state and is not supported to change without reboot. So there is no race.
|
|
I see, I mistook it with enforcing. A similar fix will be needing in libdnf where we have similar code. By the way, it's funny to say that SELinux cannot be switched without a reboot, yet you can trick a user space to the opposite by overmounting /sys/fs/selinux :) |
Sure, and actually we do that in bootc in https://github.com/containers/bootc/blob/7cdb8de90fc767f35ad2dddd557964e4095245d5/lib/src/install.rs#L854 to "escape" our container. But we also then re-exec the current process at least. Anyways though, bigger picture it'd be nice to figure out how to not have both libdnf and librepo carry a copy of this code; the simplest and correct implementation looks much like what systemd has in Maybe librpm could expose a helper for it? I guess going forward since gpgme is dropped it doesn't matter and we can just eventually drop the labeling code here entirely. |
This is another way how to fix mismatching SELinux context on /run/user directories without moving the directories to /run/gnupg/user.
librepo used to precreate the directory in /run/user to make sure a GnuPG agent executed by GPGME library places its socket there.
The directories there are normally created and removed by systemd (logind PAM session). librepo created them for a case when a package manager is invoked out of systemd session, before the super user logs in. E.g. by a timer job to cache repository metadata.
A problem was when this out-of-session process was a SELinux-confined process creating files with its own SELinux label different from a DNF program. Then the directory was created with a SELinux label different from the one expected by systemd and when logging out a corresponding user, the mismatching label clashed with systemd.
This patch fixes the issue by choosing a SELinux label of those directories to the label defined in a default SELinux file context database.
This patch adds a new -DENABLE_SELINUX=OFF CMake option to disable the new dependency on libselinux. A default behavior is to support SELinux only if GPGME backend is selected with -DUSE_GPGME=ON.
https://issues.redhat.com/browse/RHEL-10720