SHA256
1
0
forked from pool/sssd

Add patches to kill extraneous INI permission checks

This commit is contained in:
Jan Engelhardt 2024-11-05 20:41:43 +01:00 committed by Dirk Müller
parent 0d72b3229f
commit 3024aae2eb
No known key found for this signature in database
4 changed files with 322 additions and 1 deletions

View File

@ -0,0 +1,135 @@
From 340671f16abb9c26ae97b11c4e2845337e67973e Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
Date: Wed, 23 Oct 2024 20:59:32 +0200
Subject: [PATCH] INI: relax config files checks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Only make sure:
- user is root or sssd
- group is root or sssd
- other can't access it
Don't make any assumptions wrt user/group read/write-ability.
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit 8472777ec472607ea450ddb4c4666017bd0de704)
---
src/man/sssd.conf.5.xml | 5 ++-
src/util/sss_ini.c | 68 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 3 deletions(-)
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index a074cc674..bf10acb2a 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -57,9 +57,8 @@
readable, and writeable only by 'root'.
</para>
<para condition="with_non_root_user_support">
- <filename>sssd.conf</filename> must be a regular file that is owned,
- readable, and writeable by the same user as configured to run SSSD
- service.
+ <filename>sssd.conf</filename> must be a regular file that is
+ accessible only by the user used to run SSSD service or root.
</para>
</refsect1>
diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
index e989d8caf..74cf61e0e 100644
--- a/src/util/sss_ini.c
+++ b/src/util/sss_ini.c
@@ -26,6 +26,7 @@
#include <unistd.h>
#include <string.h>
#include <errno.h>
+#include <sys/stat.h>
#include <talloc.h>
#include "config.h"
@@ -781,6 +782,71 @@ int sss_ini_open(struct sss_ini *self,
return ret;
}
+static int access_check_file(const char *filename)
+{
+ int ret;
+ struct stat st;
+ uid_t uid;
+ gid_t gid;
+
+ sss_sssd_user_uid_and_gid(&uid, &gid);
+
+ ret = stat(filename, &st);
+ if (ret != 0) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "stat(%s) failed: %s\n",
+ filename, strerror(ret));
+ return EINVAL;
+ }
+
+ if ((st.st_uid != 0) && (st.st_uid != uid)) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected user owner of '%s': %"SPRIuid"\n",
+ filename, st.st_uid);
+ return ERR_INI_INVALID_PERMISSION;
+ }
+
+ if ((st.st_gid != 0) && (st.st_gid != gid)) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected group owner of '%s': %"SPRIgid"\n",
+ filename, st.st_gid);
+ return ERR_INI_INVALID_PERMISSION;
+ }
+
+ if ((st.st_mode & (S_IROTH|S_IWOTH|S_IXOTH)) != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected access to '%s' by other users\n",
+ filename);
+ return ERR_INI_INVALID_PERMISSION;
+ }
+
+ return EOK;
+}
+
+static int access_check_ini(struct sss_ini *self)
+{
+ int ret;
+ const char *path;
+ uint32_t i;
+ const char **snippet;
+ struct ref_array *used_snippets;
+
+ if (self->main_config_exists) {
+ path = ini_config_get_filename(self->file);
+ ret = access_check_file(path);
+ if (ret != EOK) {
+ return ret;
+ }
+ }
+
+ used_snippets = sss_ini_get_ra_success_list(self);
+ for (i = 0; (snippet = ref_array_get(used_snippets, i, NULL)) != NULL; ++i) {
+ ret = access_check_file(*snippet);
+ if (ret != EOK) {
+ return ret;
+ }
+ }
+
+ return EOK;
+}
+
int sss_ini_read_sssd_conf(struct sss_ini *self,
const char *config_file,
const char *config_dir)
@@ -833,5 +899,7 @@ int sss_ini_read_sssd_conf(struct sss_ini *self,
return ERR_INI_EMPTY_CONFIG;
}
+ ret = access_check_ini(self);
+
return ret;
}
--
2.47.0

View File

@ -0,0 +1,182 @@
From 1d19b8ad9415e0a12ed3aaf039d4d0956ef4dbad Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
Date: Wed, 23 Oct 2024 19:53:09 +0200
Subject: [PATCH] INI: stop using 'libini_config' for access check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
---
src/util/sss_ini.c | 100 +----------------------------------------------------
src/util/sss_ini.h | 12 ------
2 files changed, 3 insertions(+), 109 deletions(-)
Index: sssd-2.10.0/src/util/sss_ini.c
===================================================================
--- sssd-2.10.0.orig/src/util/sss_ini.c
+++ sssd-2.10.0/src/util/sss_ini.c
@@ -147,81 +147,6 @@ static int sss_ini_config_file_from_mem(
&self->file);
}
-/* Check configuration file permissions */
-
-static bool is_running_sssd(void)
-{
- static char exe[1024];
- int ret;
- const char *s = NULL;
-
- ret = readlink("/proc/self/exe", exe, sizeof(exe) - 1);
- if ((ret > 0) && (ret < 1024)) {
- exe[ret] = 0;
- s = strstr(exe, debug_prg_name);
- if ((s != NULL) && (strlen(s) == strlen(debug_prg_name))) {
- return true;
- }
- }
-
- return false;
-}
-
-static int sss_ini_access_check(struct sss_ini *self)
-{
- int ret;
- uint32_t flags = INI_ACCESS_CHECK_MODE;
-
- if (!self->main_config_exists) {
- return EOK;
- }
-
- if (is_running_sssd()) {
- flags |= INI_ACCESS_CHECK_UID | INI_ACCESS_CHECK_GID;
- }
-
- ret = ini_config_access_check(self->file,
- flags,
- geteuid(),
- getegid(),
- S_IRUSR, /* r**------ */
- ALLPERMS & ~(S_IWUSR|S_IXUSR));
-
- return ret;
-}
-
-
-
-/* Get cstat */
-
-int sss_ini_get_stat(struct sss_ini *self)
-{
- self->cstat = ini_config_get_stat(self->file);
-
- if (!self->cstat) return EIO;
-
- return EOK;
-}
-
-
-
-/* Get mtime */
-
-int sss_ini_get_mtime(struct sss_ini *self,
- size_t timestr_len,
- char *timestr)
-{
- return snprintf(timestr, timestr_len, "%llu",
- (long long unsigned)self->cstat->st_mtime);
-}
-
-/* Get file_exists */
-
-bool sss_ini_exists(struct sss_ini *self)
-{
- return self->main_config_exists;
-}
-
/* Print ini_config errors */
static void sss_ini_config_print_errors(char **error_list)
@@ -289,7 +214,6 @@ static int sss_ini_add_snippets(struct s
uint32_t i = 0;
char *msg = NULL;
struct ini_cfgobj *modified_sssd_config = NULL;
- struct access_check snip_check;
if (self == NULL || self->sssd_config == NULL || config_dir == NULL) {
return EINVAL;
@@ -297,21 +221,11 @@ static int sss_ini_add_snippets(struct s
sss_ini_free_ra_messages(self);
- snip_check.flags = INI_ACCESS_CHECK_MODE;
-
- if (is_running_sssd()) {
- snip_check.flags |= INI_ACCESS_CHECK_UID | INI_ACCESS_CHECK_GID;
- }
- snip_check.uid = geteuid();
- snip_check.gid = getegid();
- snip_check.mode = S_IRUSR; /* r**------ */
- snip_check.mask = ALLPERMS & ~(S_IWUSR | S_IXUSR);
-
ret = ini_config_augment(self->sssd_config,
config_dir,
patterns,
sections,
- &snip_check,
+ NULL,
INI_STOP_ON_ANY,
INI_MV1S_OVERWRITE,
INI_PARSE_NOWRAP,
@@ -894,15 +808,7 @@ int sss_ini_read_sssd_conf(struct sss_in
return ERR_INI_OPEN_FAILED;
}
- if (sss_ini_exists(self)) {
- ret = sss_ini_access_check(self);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "Permission check on config file %s failed: %d\n",
- config_file, ret);
- return ERR_INI_INVALID_PERMISSION;
- }
- } else {
+ if (!self->main_config_exists) {
DEBUG(SSSDBG_CONF_SETTINGS,
"File %s does not exist.\n", config_file);
}
@@ -923,7 +829,7 @@ int sss_ini_read_sssd_conf(struct sss_in
return ERR_INI_ADD_SNIPPETS_FAILED;
}
- if (!sss_ini_exists(self) &&
+ if ((!self->main_config_exists) &&
(ref_array_len(sss_ini_get_ra_success_list(self)) == 0)) {
return ERR_INI_EMPTY_CONFIG;
}
Index: sssd-2.10.0/src/util/sss_ini.h
===================================================================
--- sssd-2.10.0.orig/src/util/sss_ini.h
+++ sssd-2.10.0/src/util/sss_ini.h
@@ -81,18 +81,6 @@ int sss_ini_open(struct sss_ini *self,
const char *fallback_cfg);
/**
- * @brief Check whether sss_ini_open() reported that ini file is
- * not present
- *
- * @param[in] self pointer to sss_ini structure
- *
- * @return
- * - true we are using ini file
- * - false file was not found
- */
-bool sss_ini_exists(struct sss_ini *self);
-
-/**
* @brief get Cstat structure of the ini file
*/
int sss_ini_get_stat(struct sss_ini *self);

View File

@ -16,6 +16,8 @@ Tue Oct 15 12:59:51 UTC 2024 - Jan Engelhardt <jengelh@inai.de>
false to true for improved security.
* https://github.com/SSSD/sssd/releases/tag/2.10.0
- Add 0001-sssd-always-print-path-when-config-object-is-rejecte.patch,
0001-INI-stop-using-libini_config-for-access-check.patch,
0001-INI-relax-config-files-checks.patch,
0001-Configuration-make-sure-etc-sssd-and-everything.patch
- Fix socket activation of responders

View File

@ -28,7 +28,9 @@ Source: https://github.com/SSSD/sssd/releases/download/%version/%name-%v
Source2: https://github.com/SSSD/sssd/releases/download/%version/%name-%version.tar.gz.asc
Source3: baselibs.conf
Source5: %name.keyring
Patch5: 0001-sssd-always-print-path-when-config-object-is-rejecte.patch
Patch3: 0001-sssd-always-print-path-when-config-object-is-rejecte.patch
Patch4: 0001-INI-stop-using-libini_config-for-access-check.patch
Patch5: 0001-INI-relax-config-files-checks.patch
Patch6: 0001-Configuration-make-sure-etc-sssd-and-everything.patch
Patch11: krb-noversion.diff
Patch12: harden_sssd-ifp.service.patch