From 69e3a61e62d7b0c214e49c88a20422da4dfca238 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>

Date: Thu, 12 Oct 2023 15:55:43 +0200

Subject: [PATCH] PGP: Set a default creation SELinux labels on GnuPG

 directories

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

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

Signed-off-by: Petr Písař <ppisar@redhat.com>

---

 CMakeLists.txt         |  8 ++++++

 librepo.spec           |  9 +++++-

 librepo/CMakeLists.txt |  4 +++

 librepo/gpg.c          | 64 ++++++++++++++++++++++++++++++++++++++++++

 4 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt

index b4007e3..1a107bc 100644

--- a/CMakeLists.txt

+++ b/CMakeLists.txt

@@ -5,6 +5,7 @@ OPTION (ENABLE_TESTS "Build test?" ON)

 OPTION (ENABLE_DOCS "Build docs?" ON)

 OPTION (WITH_ZCHUNK "Build with zchunk support" ON)

 OPTION (ENABLE_PYTHON "Build Python bindings" ON)

+OPTION (ENABLE_SELINUX "Restore SELinux labels on GnuPG directories" ON)

 INCLUDE (${CMAKE_SOURCE_DIR}/VERSION.cmake)

 SET (VERSION "${LIBREPO_MAJOR}.${LIBREPO_MINOR}.${LIBREPO_PATCH}")

@@ -33,6 +34,9 @@ PKG_SEARCH_MODULE(LIBCRYPTO REQUIRED libcrypto openssl)

 PKG_CHECK_MODULES(LIBXML2 libxml-2.0 REQUIRED)

 FIND_PACKAGE(CURL 7.52.0 REQUIRED)

 FIND_PACKAGE(Gpgme REQUIRED)

+IF (ENABLE_SELINUX)

+    PKG_CHECK_MODULES(SELINUX REQUIRED libselinux)

+ENDIF(ENABLE_SELINUX)

 IF (WITH_ZCHUNK)

@@ -63,6 +67,10 @@ ENDIF (NOT CURL_FOUND)

 INCLUDE_DIRECTORIES(${LIBXML2_INCLUDE_DIRS})

 INCLUDE_DIRECTORIES(${CURL_INCLUDE_DIR})

 #INCLUDE_DIRECTORIES(${CHECK_INCLUDE_DIR})

+IF (ENABLE_SELINUX)

+    INCLUDE_DIRECTORIES(${SELINUX_INCLUDE_DIRS})

+    ADD_DEFINITIONS(-DENABLE_SELINUX=1)

+ENDIF (ENABLE_SELINUX)

 include (GNUInstallDirs)

 # Python stuff

diff --git a/librepo.spec b/librepo.spec

index e8c88d8..0a5c867 100644

--- a/librepo.spec

+++ b/librepo.spec

@@ -8,6 +8,8 @@

 %bcond_without zchunk

 %endif

+%bcond_without selinux

+

 %global dnf_conflict 2.8.8

 Name:           librepo

@@ -29,6 +31,9 @@ BuildRequires:  libattr-devel

 BuildRequires:  libcurl-devel >= %{libcurl_version}

 BuildRequires:  pkgconfig(libxml-2.0)

 BuildRequires:  pkgconfig(libcrypto)

+%if %{with selinux}

+BuildRequires:  pkgconfig(libselinux)

+%endif

 BuildRequires:  pkgconfig(openssl)

 %if %{with zchunk}

 BuildRequires:  pkgconfig(zck) >= 0.9.11

@@ -66,7 +71,9 @@ Python 3 bindings for the librepo library.

 %autosetup -p1

 %build

-%cmake %{!?with_zchunk:-DWITH_ZCHUNK=OFF}

+%cmake \

+    %{!?with_zchunk:-DWITH_ZCHUNK=OFF} \

+    -DENABLE_SELINUX=%{?with_selinux:ON}%{!?with_selinux:OFF}

 %cmake_build

 %check

diff --git a/librepo/CMakeLists.txt b/librepo/CMakeLists.txt

index 4f00a5e..e759692 100644

--- a/librepo/CMakeLists.txt

+++ b/librepo/CMakeLists.txt

@@ -53,6 +53,10 @@ TARGET_LINK_LIBRARIES(librepo

                         ${GPGME_VANILLA_LIBRARIES}

                         ${GLIB2_LIBRARIES}

                      )

+IF (ENABLE_SELINUX)

+    TARGET_LINK_LIBRARIES(librepo ${SELINUX_LIBRARIES})

+ENDIF(ENABLE_SELINUX)

+

 IF (WITH_ZCHUNK)

     TARGET_LINK_LIBRARIES(librepo ${ZCHUNKLIB_LIBRARIES})

 ENDIF (WITH_ZCHUNK)

diff --git a/librepo/gpg.c b/librepo/gpg.c

index a134d44..e4b6589 100644

--- a/librepo/gpg.c

+++ b/librepo/gpg.c

@@ -28,6 +28,11 @@

 #include <gpgme.h>

 #include <unistd.h>

+#if ENABLE_SELINUX

+#include <selinux/selinux.h>

+#include <selinux/label.h>

+#endif

+

 #include "rcodes.h"

 #include "util.h"

 #include "gpg.h"

@@ -44,6 +49,14 @@

  * Previous solution was to send the agent a "KILLAGENT" message, but that

  * would cause a race condition with calling gpgme_release(), see [2], [3].

  *

+ * Current solution with precreating /run/user/$UID showed problematic when

+ * this library was used out of an systemd-logind session. Then

+ * /run/user/$UID, normally maintained by systemd, was assigned a SELinux

+ * label unexpected by systemd causing errors on a user logout [4].

+ *

+ * We remedy it by choosing the label according to a default file context

+ * policy (ENABLE_SELINUX macro).

+ *

  * Since the agent doesn't clean up its sockets properly, by creating this

  * directory we make sure they are in a place that is not causing trouble with

  * container images.

@@ -51,14 +64,65 @@

  * [1] https://bugzilla.redhat.com/show_bug.cgi?id=1650266

  * [2] https://bugzilla.redhat.com/show_bug.cgi?id=1769831

  * [3] https://github.com/rpm-software-management/microdnf/issues/50

+ * [4] https://issues.redhat.com/browse/RHEL-10720

  */

 void ensure_socket_dir_exists() {

     char dirname[32];

+#if ENABLE_SELINUX

+    char *old_default_context = NULL;

+    int old_default_context_was_retrieved = 0;

+    struct selabel_handle *labeling_handle = NULL;

+

+    /* A purpose of this piece of code is to deal with applications whose

+     * security policy overrides a file context for temporary files but don't

+     * know that librepo executes GnuPG which expects a default file context. */

+    if (0 == getfscreatecon(&old_default_context)) {

+        old_default_context_was_retrieved = 1;

+    } else {

+        g_debug("Failed to retrieve a default SELinux context");

+    }

+    labeling_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);

+    if (labeling_handle == NULL) {

+        g_debug("Failed to open a SELinux labeling handle: %s", strerror(errno));

+    }

+#endif

+

     snprintf(dirname, sizeof(dirname), "/run/user/%u", getuid());

+

+#if ENABLE_SELINUX

+    if (labeling_handle != NULL) {

+        char *new_default_context = NULL;

+        if (selabel_lookup(labeling_handle, &new_default_context, dirname, 0700)) {

+            /* Here we could hard-code "system_u:object_r:user_tmp_t:s0", but

+             * that value should be really defined in default file context

+             * SELinux policy. Only log that the policy is incomplete. */

+            g_debug("Failed to look up a default SELinux label for \"%s\"", dirname);

+        } else {

+            if (setfscreatecon(new_default_context)) {

+                g_debug("Failed to set default SELinux context to \"%s\"",

+                        new_default_context);

+            }

+            freecon(new_default_context);

+        }

+    }

+#endif

+

     int res = mkdir(dirname, 0700);

     if (res != 0 && errno != EEXIST) {

         g_debug("Failed to create \"%s\": %d - %s\n", dirname, errno, strerror(errno));

     }

+

+#if ENABLE_SELINUX

+    if (labeling_handle != NULL) {

+        selabel_close(labeling_handle);

+    }

+    if (old_default_context_was_retrieved) {

+        if (setfscreatecon(old_default_context)) {

+            g_debug("Failed to restore a default SELinux context");

+         }

+     }

+    freecon(old_default_context);

+#endif

 }

 gboolean

-- 

2.41.0