PGP: Set a default creation SELinux labels on GnuPG directories #1632
Conversation
|
The Copr Build (pull_request_target) failure is irrelevant. It builds rhel-8.10 branch in Fedora 38 where GCC errors on missing #include <cstdint>. (Reported as #1633.) |
|
Just a clarification: This pull request has not corresponding code in dnf-4-master branch because the GPGME backend was removed in commit 1649796. |
ppisar
force-pushed
the
rhel-8-10-getfscreatecon
branch
from
61edad4
to
b494db4
Compare
ppisar
changed the title
|
I only corrected a typo in the commit subject. |
ppisar
force-pushed
the
rhel-8-10-getfscreatecon
branch
from
b494db4
to
00b2d5e
Compare
|
I resolved jan-kolarik's requests and pushed patch. |
libdnf 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). libdnf 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. The same issue was with temporary GnuPG home directories created by libdnf under /tmp. This patch fixes both the isseus by restoring a SELinux label of those directories to the label defined in a default SELinux file context database. Obviously the database cannot have a record for a nonspecific /tmp/tmpdir.XXXXXX (a mkdtemp() template) directory names. Therefore I changed their names to more specific /tmp/libdnf.XXXXXX. Once a SELinux policy updates the database, directories under /tmp will get a correct label. There is yet another problem with accessing /var/cache/dnf/*/pubring, but that seems to be pure SELinux policy problem. This patch adds a new -DENABLE_SELINUX=OFF CMake option to disable the new dependency on libselinux. A default behavior is to support SELinux. Implementation details: I used selabel_lookup() + setfscreatecon() + mkdtemp() + setfscreatecon() sequence instead of mkdtemp() + selinux_restorecon() sequence because the later polutes stderr if a SELinux policy does not define the default context. One could supress stderr messages with selinux_set_callback(), but its effect cannot be restored. I also kept the sequence in one function and reused it for creating /run/user/$PID directories because the code is simpler than spliting the function into three parts. https://issues.redhat.com/browse/RHEL-6421
ppisar
force-pushed
the
rhel-8-10-getfscreatecon
branch
from
00b2d5e
to
b7888e4
Compare
|
Thanks for the review. I believe I resolved all the discovered issues. |
| char tmpdir[] = "/tmp/tmpdir.XXXXXX"; | ||
| mkdtemp(tmpdir); | ||
| char tmpdir[] = "/tmp/libdnf.XXXXXX"; | ||
| create_temporary_directory(tmpdir); |
There was a problem hiding this comment.
Although not directly connected with this PR, while reviewing the RHEL 9 version of these changes, I noticed that failures in the create_temporary_directory calls consistently result in throwing a RepoError there. In contrast, in the RHEL 8 case, these errors are sometimes ignored, like here, leading to different behavior in both systems. Not sure if it's intentional, but it appears that this related PR for the RHEL 8 branch is missing.
@kontura Do you have an idea why is that?
But anyway, this PR is not changing this behavior, so I am in favor of merging.
There was a problem hiding this comment.
If I understand correctly you are asking why the PR is missing. I think it is simply because we didn't do a rebase in RHEL 8 since it was merged and there was no request for it to be backported.
There is this bug for it but that is for RHEL 9.
jan-kolarik
merged commit 8752006
into
rpm-software-management:rhel-8.10
libdnf 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). libdnf 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.
The same issue was with temporary GnuPG home directories created by libdnf under /tmp.
This patch fixes both the isseus by restoring a SELinux label of those directories to the label defined in a default SELinux file context database.
Obviously the database cannot have a record for a nonspecific /tmp/tmpdir.XXXXXX (a mkdtemp() template) directory names. Therefore I changed their names to more specific /tmp/libdnf.XXXXXX. Once a SELinux policy updates the database, directories under /tmp will get a correct label.
There is yet another problem with accessing /var/cache/dnf/*/pubring, but that seems to be pure SELinux policy problem.
This patch adds a new -DENABLE_SELINUX=OFF CMake option to disable the new dependency on libselinux. A default behavior is to support SELinux.
Implementation details:
I used selabel_lookup() + setfscreatecon() + mkdtemp()
I also kept the sequence in one function and reused it for creating /run/user/$PID directories because the code is simpler than spliting the function into three parts.
https://issues.redhat.com/browse/RHEL-6421