rpm-software-management / librepo

A library providing C and Python (libcURL like) API for downloading packages and linux repository metadata in rpm-md format
http://rpm-software-management.github.io/librepo/
GNU Lesser General Public License v2.1
74 stars 90 forks source link

PGP: Set a default creation SELinux labels on GnuPG directories #287

Closed ppisar closed 9 months ago

ppisar commented 9 months ago

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 commented 9 months ago

"DNF5 Integration Tests (--tags dnf5 --command dnf5)" failures are unrelated:

2023-10-16T07:40:39.4373808Z   Scenario Outline: "update" is an alias for "upgrade" -- @1.35   # dnf/command-aliases.feature:54
2023-10-16T07:40:39.4375032Z     When I execute dnf with args "update -h"                      # dnf/steps/cmd.py:107
2023-10-16T07:40:39.4376015Z     Then the exit code is 0                                       # common/output.py:60
2023-10-16T07:40:39.4376989Z     And stdout contains "Usage:\n.*upgrade"                       # dnf/steps/cmd.py:238
2023-10-16T07:40:39.4377956Z       Assertion Failed: Stdout doesn't contain: Usage:\n.*upgrade
[...]
2023-10-16T07:40:39.4431306Z Failing scenarios:
2023-10-16T07:40:39.4432181Z   dnf/command-aliases.feature:20  "updateinfo" is an alias for "advisory" -- @1.1 
2023-10-16T07:40:39.4433388Z   dnf/command-aliases.feature:22  "check-update" is an alias for "check-upgrade" -- @1.3 
2023-10-16T07:40:39.4434526Z   dnf/command-aliases.feature:26  "dg" is an alias for "downgrade" -- @1.7 
2023-10-16T07:40:39.4435567Z   dnf/command-aliases.feature:31  "grp" is an alias for "group" -- @1.12 
2023-10-16T07:40:39.4436600Z   dnf/command-aliases.feature:32  "help" is an alias for "help" -- @1.13 
2023-10-16T07:40:39.4437622Z   dnf/command-aliases.feature:35  "in" is an alias for "install" -- @1.16 
2023-10-16T07:40:39.4438633Z   dnf/command-aliases.feature:39  "ls" is an alias for "list" -- @1.20 
2023-10-16T07:40:39.4439661Z   dnf/command-aliases.feature:43  "rei" is an alias for "reinstall" -- @1.24 
2023-10-16T07:40:39.4440706Z   dnf/command-aliases.feature:46  "rm" is an alias for "remove" -- @1.27 
2023-10-16T07:40:39.4441762Z   dnf/command-aliases.feature:47  "repoinfo" is an alias for "repo info" -- @1.28 
2023-10-16T07:40:39.4442899Z   dnf/command-aliases.feature:48  "repolist" is an alias for "repo list" -- @1.29 
2023-10-16T07:40:39.4444119Z   dnf/command-aliases.feature:50  "rq" is an alias for "repoquery" -- @1.31 
2023-10-16T07:40:39.4445161Z   dnf/command-aliases.feature:53  "up" is an alias for "upgrade" -- @1.34 
2023-10-16T07:40:39.4446223Z   dnf/command-aliases.feature:54  "update" is an alias for "upgrade" -- @1.35 
ppisar commented 9 months ago

The only DNF4 Integration Tests failure is:

2023-10-16T07:42:53.6137015Z   Scenario: Reposync --gpgcheck removes unsigned packages                                                       # dnf/plugins-core/reposync.feature:347
2023-10-16T07:42:53.6138116Z     Given I enable plugin "reposync"                                                                            # dnf/steps/cmd.py:197
2023-10-16T07:42:53.6139366Z     Given I use repository "reposync-gpg" with configuration                                                    # dnf/steps/repo.py:208
2023-10-16T07:42:53.6140289Z       | key      | value                                                                          |
2023-10-16T07:42:53.6140967Z       | gpgcheck | 1                                                                              |
2023-10-16T07:42:53.6141897Z       | gpgkey   | file://{context.dnf.fixturesdir}/gpgkeys/keys/reposync-gpg/reposync-gpg-public |
2023-10-16T07:42:53.6143132Z     And I successfully execute dnf with args "install dedalo-signed"                                            # dnf/steps/cmd.py:205
2023-10-16T07:42:53.6145140Z       Assertion Failed: Command has returned exit code 1: dnf -y --installroot=/tmp/dnf_ci_installroot_m0mi1rxq --releasever=29 --setopt=module_platform_id=platform:f29 --disableplu
gin='*' --enableplugin='reposync' install dedalo-signed
2023-10-16T07:42:53.6146556Z       Captured stdout:
2023-10-16T07:42:53.6146901Z       ESC[31m
2023-10-16T07:42:53.6148167Z       Last Command: ESC[31mESC[1mdnf -y --installroot=/tmp/dnf_ci_installroot_m0mi1rxq --releasever=29 --setopt=module_platform_id=platform:f29 --disableplugin='*' --enableplugin='r
eposync' install dedalo-signed
2023-10-16T07:42:53.6149399Z       ESC[36mESC[1m
2023-10-16T07:42:53.6149756Z       Last Command stdout:ESC[90m
2023-10-16T07:42:53.6150485Z       reposync-gpg test repository                    794 kB/s | 1.2 kB     00:00    
2023-10-16T07:42:53.6151268Z       Dependencies resolved.
2023-10-16T07:42:53.6151700Z       ================================================================================
2023-10-16T07:42:53.6152369Z        Package              Architecture  Version           Repository           Size
2023-10-16T07:42:53.6153014Z       ================================================================================
2023-10-16T07:42:53.6153480Z       Installing:
2023-10-16T07:42:53.6154061Z        dedalo-signed        x86_64        1.0-1.fc29        reposync-gpg        6.0 k
2023-10-16T07:42:53.6154616Z       
2023-10-16T07:42:53.6154895Z       Transaction Summary
2023-10-16T07:42:53.6155310Z       ================================================================================
2023-10-16T07:42:53.6155772Z       Install  1 Package
2023-10-16T07:42:53.6156159Z       
2023-10-16T07:42:53.6156420Z       Total size: 6.0 k
2023-10-16T07:42:53.6156746Z       Installed size: 0  
2023-10-16T07:42:53.6157099Z       Downloading Packages:
2023-10-16T07:42:53.6157733Z       reposync-gpg test repository                    602 kB/s | 616  B     00:00
2023-10-16T07:42:53.6158377Z       ESC[36mESC[1m
2023-10-16T07:42:53.6158727Z       Last Command stderr:ESC[90m
2023-10-16T07:42:53.6159125Z       Importing GPG key 0xBBA28359:
2023-10-16T07:42:53.6159570Z        Userid     : "reposync-gpg"
2023-10-16T07:42:53.6160105Z        Fingerprint: 679F 39C6 77D2 E292 97D3 7388 9AB2 41C9 BBA2 8359
2023-10-16T07:42:53.6161054Z        From       : /opt/ci/dnf-behave-tests/fixtures/gpgkeys/keys/reposync-gpg/reposync-gpg-public
2023-10-16T07:42:53.6161805Z       error: Certificate 9AB241C9BBA28359:
2023-10-16T07:42:53.6162643Z         Policy rejects 9AB241C9BBA28359: No binding signature at time 2023-10-16T07:42:51Z
2023-10-16T07:42:53.6163620Z       Key import failed (code 2). Failing package is: dedalo-signed-1.0-1.fc29.x86_64
2023-10-16T07:42:53.6164825Z        GPG Keys are configured as: file:///opt/ci/dnf-behave-tests/fixtures/gpgkeys/keys/reposync-gpg/reposync-gpg-public
2023-10-16T07:42:53.6165684Z       Error: GPG check FAILED
2023-10-16T07:42:53.6166062Z       ESC[0m
2023-10-16T07:42:53.6166223Z 

This seems related to a stricter RPM Sequoia than to my SELinux patch.

ppisar commented 9 months ago

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.

jan-kolarik commented 9 months ago

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 if (labeling_handle != NULL), to maintain consistency with the rest of the file and enhance code readability.

ppisar commented 9 months ago

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.

ppisar commented 9 months ago

I reversed the NULL conditions and corrected a typo in a comment.

cgwalters commented 1 month ago

cc https://issues.redhat.com/browse/RHEL-39796?focusedId=24849650&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-24849650

This code should be calling is_selinux_enabled() first.

ppisar commented 1 month ago

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.

cgwalters commented 1 month ago

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.

cgwalters commented 1 month ago

https://github.com/rpm-software-management/librepo/pull/313

ppisar commented 1 month ago

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 :)

cgwalters commented 1 month ago

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 mkdir_p_label https://github.com/systemd/systemd/blob/b1213ef73860c1599c6bfeae993a215cb9db7c12/src/shared/mkdir-label.c#L40 (see also https://github.com/systemd/systemd/blob/main/src/shared/selinux-util.c#L76).

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.