libdnf/SOURCES/0049-PGP-Set-a-default-creation-SELinux-labels-on-GnuPG-d.patch

From 8752006f5f9c11bca3f04c99b463fd167caf0ddd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
Date: Wed, 4 Oct 2023 16:38:12 +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

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
Signed-off-by: Petr Písař <ppisar@redhat.com>
---
 CMakeLists.txt        |   7 +++
 libdnf.spec           |  11 +++-
 libdnf/CMakeLists.txt |   4 ++
 libdnf/repo/Repo.cpp  | 121 +++++++++++++++++++++++++++++++++++++-----
 4 files changed, 129 insertions(+), 14 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d895b2bf..e5829e6a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -34,6 +34,7 @@ option(WITH_MAN "Enables hawkey man page generation" ON)
 option(WITH_ZCHUNK "Build with zchunk support" ON)
 option(ENABLE_RHSM_SUPPORT "Build with Red Hat Subscription Manager support?" OFF)
 option(ENABLE_SOLV_URPMREORDER "Build with support for URPM-like solution reordering?" OFF)
+option(ENABLE_SELINUX "Restore SELinux labels on GnuPG directories" ON)
 
 
 # build options - debugging
@@ -83,6 +84,12 @@ if(ENABLE_RHSM_SUPPORT)
     include_directories(${RHSM_INCLUDE_DIRS})
 endif()
 
+if(ENABLE_SELINUX)
+    pkg_check_modules(SELINUX REQUIRED libselinux)
+    include_directories(${SELINUX_INCLUDE_DIRS})
+    add_definitions(-DENABLE_SELINUX=1)
+endif()
+
 
 # glibc: check if fnmatch.h has FNM_CASEFOLD symbol
 include(CheckSymbolExists)
diff --git a/libdnf.spec b/libdnf.spec
index aa51dd28..df482f54 100644
--- a/libdnf.spec
+++ b/libdnf.spec
@@ -42,6 +42,8 @@
 %bcond_with rhsm
 %endif
 
+%bcond_without selinux
+
 %if 0%{?rhel}
 %bcond_with zchunk
 %else
@@ -84,6 +86,9 @@ BuildRequires:  pkgconfig(sqlite3)
 BuildRequires:  pkgconfig(json-c)
 BuildRequires:  pkgconfig(cppunit)
 BuildRequires:  pkgconfig(libcrypto)
+%if %{with selinux}
+BuildRequires:  pkgconfig(libselinux)
+%endif
 BuildRequires:  pkgconfig(modulemd-2.0) >= %{libmodulemd_version}
 BuildRequires:  pkgconfig(smartcols)
 BuildRequires:  gettext
@@ -205,7 +210,8 @@ pushd build-py2
     %define __builddir build-py2
   %endif
   %cmake -DPYTHON_DESIRED:FILEPATH=%{__python2} -DWITH_MAN=OFF ../ %{!?with_zchunk:-DWITH_ZCHUNK=OFF} %{!?with_valgrind:-DDISABLE_VALGRIND=1} %{_cmake_opts} -DLIBDNF_MAJOR_VERSION=%{libdnf_major_version} -DLIBDNF_MINOR_VERSION=%{libdnf_minor_version} -DLIBDNF_MICRO_VERSION=%{libdnf_micro_version} \
-    -DWITH_SANITIZERS=%{?with_sanitizers:ON}%{!?with_sanitizers:OFF}
+    -DWITH_SANITIZERS=%{?with_sanitizers:ON}%{!?with_sanitizers:OFF} \
+    -DENABLE_SELINUX=%{?with_selinux:ON}%{!?with_selinux:OFF}
   %make_build
 popd
 %endif
@@ -219,7 +225,8 @@ pushd build-py3
     %define __builddir build-py3
   %endif
   %cmake -DPYTHON_DESIRED:FILEPATH=%{__python3} -DWITH_GIR=0 -DWITH_MAN=0 -Dgtkdoc=0 ../ %{!?with_zchunk:-DWITH_ZCHUNK=OFF} %{!?with_valgrind:-DDISABLE_VALGRIND=1} %{_cmake_opts} -DLIBDNF_MAJOR_VERSION=%{libdnf_major_version} -DLIBDNF_MINOR_VERSION=%{libdnf_minor_version} -DLIBDNF_MICRO_VERSION=%{libdnf_micro_version} \
-    -DWITH_SANITIZERS=%{?with_sanitizers:ON}%{!?with_sanitizers:OFF}
+    -DWITH_SANITIZERS=%{?with_sanitizers:ON}%{!?with_sanitizers:OFF} \
+    -DENABLE_SELINUX=%{?with_selinux:ON}%{!?with_selinux:OFF}
   %make_build
 popd
 %endif
diff --git a/libdnf/CMakeLists.txt b/libdnf/CMakeLists.txt
index 998a6f94..10b15230 100644
--- a/libdnf/CMakeLists.txt
+++ b/libdnf/CMakeLists.txt
@@ -89,6 +89,10 @@ if(ENABLE_RHSM_SUPPORT)
     target_link_libraries(libdnf ${RHSM_LIBRARIES})
 endif()
 
+if(ENABLE_SELINUX)
+    target_link_libraries(libdnf ${SELINUX_LIBRARIES})
+endif()
+
 set(DNF_SO_VERSION 2)
 set_target_properties(libdnf PROPERTIES OUTPUT_NAME "dnf")
 set_target_properties(libdnf PROPERTIES SOVERSION ${DNF_SO_VERSION})
diff --git a/libdnf/repo/Repo.cpp b/libdnf/repo/Repo.cpp
index d61a24a5..68b82ccc 100644
--- a/libdnf/repo/Repo.cpp
+++ b/libdnf/repo/Repo.cpp
@@ -51,6 +51,11 @@
 
 #include <gpgme.h>
 
+#if ENABLE_SELINUX
+#include <selinux/selinux.h>
+#include <selinux/label.h>
+#endif
+
 #include <solv/chksum.h>
 #include <solv/repo.h>
 #include <solv/util.h>
@@ -649,6 +654,78 @@ std::unique_ptr<LrHandle> Repo::Impl::lrHandleInitRemote(const char *destdir)
     return h;
 }
 
+/*
+ * @brief Create a temporary directory.
+ *
+ * Creates a temporary directory with 0700 mode attempting to set a proper
+ * SELinux file context. Encountered errors are logged at debug level to
+ * a global logger.
+ *
+ * @param name_template As an input value it is a template according to
+ * mkdtemp(3). As an output value it will contain the created directory name.
+ *
+ * @return 0 if the directory was created, -1 if it wasn't. SELinux failures
+ * are not considered an error.
+ */
+static int create_temporary_directory(char *name_template) {
+    auto logger(Log::getLogger());
+    int retval = 0;
+#if ENABLE_SELINUX
+    char *old_default_context = NULL;
+    char *new_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 libdnf executes GnuPG which expects a default file context. */
+    if (0 == getfscreatecon(&old_default_context)) {
+        old_default_context_was_retrieved = 1;
+    } else {
+        logger->debug(tfm::format("Failed to retrieve a default SELinux context"));
+    }
+
+    labeling_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+    if (NULL == labeling_handle) {
+        logger->debug(tfm::format("Failed to open a SELinux labeling handle: %s",
+                    strerror(errno)));
+    } else {
+        if (selabel_lookup(labeling_handle, &new_default_context, name_template, 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 incpomplete. */
+            logger->debug(tfm::format("Failed to look up a default SELinux label for \"%s\"",
+                        name_template));
+        } else {
+            if (setfscreatecon(new_default_context)) {
+                logger->debug(tfm::format("Failed to set default SELinux context to \"%s\"",
+                            new_default_context));
+            }
+            freecon(new_default_context);
+        }
+        selabel_close(labeling_handle);
+    }
+#endif
+
+    /* mkdtemp() assures 0700 mode. */
+    if (NULL == mkdtemp(name_template)) {
+        logger->debug(tfm::format("Failed to create a directory \"%s\": %s",
+                                  name_template, strerror(errno)));
+        retval = -1;
+    }
+
+#if ENABLE_SELINUX
+    if (old_default_context_was_retrieved) {
+        if (setfscreatecon(old_default_context)) {
+            logger->debug(tfm::format("Failed to restore a default SELinux context"));
+        }
+    }
+    freecon(old_default_context);
+#endif
+
+    return retval;
+}
+
 static void gpgImportKey(gpgme_ctx_t context, int keyFd)
 {
     auto logger(Log::getLogger());
@@ -703,8 +780,8 @@ static std::vector<Key> rawkey2infos(int fd) {
     std::unique_ptr<std::remove_pointer<gpgme_ctx_t>::type> context(ctx);
 
     // set GPG home dir
-    char tmpdir[] = "/tmp/tmpdir.XXXXXX";
-    mkdtemp(tmpdir);
+    char tmpdir[] = "/tmp/libdnf.XXXXXX";
+    create_temporary_directory(tmpdir);
     Finalizer tmpDirRemover([&tmpdir](){
         dnf_remove_recursive(tmpdir, NULL);
     });
@@ -853,6 +930,13 @@ std::vector<Key> Repo::Impl::retrieve(const std::string & url)
  * would cause a race condition with calling gpgme_release(), see [2], [3],
  * [4].
  *
+ * Current solution precreating /run/user/$UID showed problematic when this
+ * library was used out of a systemd-logind session from a programm with an
+ * unexpected SELinux context. Then /run/user/$UID, normally maintained by
+ * systemd, was assigned a SELinux label unexpected by systemd causing errors
+ * on a user logout [5]. We remedy it by restoring the label according to
+ * a file context policy.
+ *
  * 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.
@@ -861,14 +945,27 @@ std::vector<Key> Repo::Impl::retrieve(const std::string & url)
  * [2] https://bugzilla.redhat.com/show_bug.cgi?id=1769831
  * [3] https://github.com/rpm-software-management/microdnf/issues/50
  * [4] https://bugzilla.redhat.com/show_bug.cgi?id=1781601
+ * [5] https://issues.redhat.com/browse/RHEL-6421
  */
 static void ensure_socket_dir_exists() {
     auto logger(Log::getLogger());
+    char tmpdir[] = "/run/user/libdnf.XXXXXX";
     std::string dirname = "/run/user/" + std::to_string(getuid());
-    int res = mkdir(dirname.c_str(), 0700);
-    if (res != 0 && errno != EEXIST) {
-        logger->debug(tfm::format("Failed to create directory \"%s\": %d - %s",
-                                  dirname, errno, strerror(errno)));
+
+    /* create_temporary_directory() assures 0700 mode and tries its best to
+     * correct a SELinux label. */
+    if (create_temporary_directory(tmpdir)) {
+        return;
+    }
+
+    /* Set the desired name. */
+    if (rename(tmpdir, dirname.c_str())) {
+        if (errno != EEXIST && errno != ENOTEMPTY && errno != EBUSY) {
+            logger->debug(tfm::format("Failed to rename \"%s\" directory to \"%s\": %s",
+                                      tmpdir, dirname, strerror(errno)));
+        }
+        rmdir(tmpdir);
+        return;
     }
 }
 
@@ -1151,8 +1248,8 @@ void Repo::Impl::addCountmeFlag(LrHandle *handle) {
 bool Repo::Impl::isMetalinkInSync()
 {
     auto logger(Log::getLogger());
-    char tmpdir[] = "/tmp/tmpdir.XXXXXX";
-    mkdtemp(tmpdir);
+    char tmpdir[] = "/tmp/libdnf.XXXXXX";
+    create_temporary_directory(tmpdir);
     Finalizer tmpDirRemover([&tmpdir](){
         dnf_remove_recursive(tmpdir, NULL);
     });
@@ -1221,8 +1318,8 @@ bool Repo::Impl::isRepomdInSync()
 {
     auto logger(Log::getLogger());
     LrYumRepo *yum_repo;
-    char tmpdir[] = "/tmp/tmpdir.XXXXXX";
-    mkdtemp(tmpdir);
+    char tmpdir[] = "/tmp/libdnf.XXXXXX";
+    create_temporary_directory(tmpdir);
     Finalizer tmpDirRemover([&tmpdir](){
         dnf_remove_recursive(tmpdir, NULL);
     });
@@ -1260,8 +1357,8 @@ void Repo::Impl::fetch(const std::string & destdir, std::unique_ptr<LrHandle> &&
         throw RepoError(tfm::format(_("Cannot create repo destination directory \"%s\": %s"),
                                       destdir, errTxt));
     }
-    auto tmpdir = destdir + "/tmpdir.XXXXXX";
-    if (!mkdtemp(&tmpdir.front())) {
+    auto tmpdir = destdir + "/libdnf.XXXXXX";
+    if (create_temporary_directory(&tmpdir.front())) {
         const char * errTxt = strerror(errno);
         throw RepoError(tfm::format(_("Cannot create repo temporary directory \"%s\": %s"),
                                       tmpdir.c_str(), errTxt));
-- 
2.41.0