Skip to content

Commit

Permalink
PGP: Set a default creation SELinux labels on GnuGP directories
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ppisar committed Oct 16, 2023
1 parent c32ce10 commit 61edad4
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 14 deletions.
7 changes: 7 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions libdnf.spec
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
%bcond_with rhsm
%endif

%bcond_without selinux

%if 0%{?rhel}
%bcond_with zchunk
%else
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions libdnf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ target_link_libraries(libdnf
${OPENSSL_CRYPTO_LIBRARY}
${RPM_LIBRARIES}
${SCOLS_LIBRARIES}
${SELINUX_LIBRARIES}
${SQLite3_LIBRARIES}
${JSONC_LIBRARIES}
${LIBMODULEMD_LIBRARIES}
Expand Down
120 changes: 108 additions & 12 deletions libdnf/repo/Repo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -649,6 +654,77 @@ 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());
#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)));
return -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 0;
}

static void gpgImportKey(gpgme_ctx_t context, int keyFd)
{
auto logger(Log::getLogger());
Expand Down Expand Up @@ -703,8 +779,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);
});
Expand Down Expand Up @@ -853,6 +929,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.
Expand All @@ -861,14 +944,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-642
*/
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;
}
}

Expand Down Expand Up @@ -1151,8 +1247,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);
});
Expand Down Expand Up @@ -1221,8 +1317,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);
});
Expand Down Expand Up @@ -1260,8 +1356,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));
Expand Down

0 comments on commit 61edad4

Please sign in to comment.