From adfdd766d7561bc9f4941ffd1d370a656a72c8a5 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 18 Jul 2023 16:21:42 +0200 Subject: [PATCH 1/3] SPEC: make permissions of config folders consistent It doesn't make sense to allow 'go+x' for sub-folders under '/etc/sssd' since this folder itself doesn't have those permissions. --- contrib/sssd.spec.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 831c5dc9eff..9c4e0be7a97 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -774,8 +774,8 @@ done %attr(755,%{sssd_user},%{sssd_user}) %dir %{gpocachepath} %attr(750,%{sssd_user},%{sssd_user}) %dir %{_var}/log/%{name} %attr(700,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd -%attr(711,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/conf.d -%attr(711,root,root) %dir %{_sysconfdir}/sssd/pki +%attr(700,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/conf.d +%attr(700,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/pki %ghost %attr(0600,root,root) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf %dir %{_sysconfdir}/logrotate.d %config(noreplace) %{_sysconfdir}/logrotate.d/sssd From 7f81e61b6317df63d0509ae166da9c5b05191bbc Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 20 Jul 2023 15:16:54 +0200 Subject: [PATCH 2/3] TOOLS: get rid of strings duplications --- .../alltests/test_config_validation.py | 3 +-- src/tools/sssctl/sssctl_config.c | 21 ++----------------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/tests/multihost/alltests/test_config_validation.py b/src/tests/multihost/alltests/test_config_validation.py index 95f2ea99bb8..f30ee27a00e 100644 --- a/src/tests/multihost/alltests/test_config_validation.py +++ b/src/tests/multihost/alltests/test_config_validation.py @@ -555,8 +555,7 @@ def test_0028_bz1723273(self, multihost, backupsssdconf): result = sssctl_check.stdout_text.strip() rm_dir = 'rm -rf /tmp/test' multihost.client[0].run_command(rm_dir, raiseonerr=False) - assert 'File ownership and permissions check failed. Expected ' \ - 'root:root and 0600' in result and \ + assert 'File ownership and permissions check failed' in result and \ sssctl_check.returncode == 1 @pytest.mark.tier1 diff --git a/src/tools/sssctl/sssctl_config.c b/src/tools/sssctl/sssctl_config.c index f00915125fd..8f6e139875f 100644 --- a/src/tools/sssctl/sssctl_config.c +++ b/src/tools/sssctl/sssctl_config.c @@ -114,8 +114,8 @@ errno_t sssctl_config_check(struct sss_cmdline *cmdline, config_path, config_snippet_path); - if (ret == ERR_INI_OPEN_FAILED) { - PRINT("Failed to open %s\n", config_path); + if (ret != EOK) { + PRINT("Failed to read '%s': %s\n", config_path, sss_strerror(ret)); goto done; } @@ -123,23 +123,6 @@ errno_t sssctl_config_check(struct sss_cmdline *cmdline, PRINT("File %1$s does not exist.\n", config_path); } - if (ret == ERR_INI_INVALID_PERMISSION) { - PRINT("File ownership and permissions check failed. " - "Expected root:root and 0600.\n"); - goto done; - } - - if (ret == ERR_INI_PARSE_FAILED) { - PRINT("Failed to load configuration from %s.\n", - config_path); - goto done; - } - - if (ret == ERR_INI_ADD_SNIPPETS_FAILED) { - PRINT("Error while reading configuration directory.\n"); - goto done; - } - /* Used snippet files */ ra_success = sss_ini_get_ra_success_list(init_data); num_ra_success = ref_array_len(ra_success); From 0764f0796e13f4eba88f52f45931cd3a1dead299 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 18 Jul 2023 17:10:23 +0200 Subject: [PATCH 3/3] SPEC: make ownership of sssd.conf consistent with config folders. :packaging: sssd.conf should be owned by user specified with '--with-sssd-user=' at build time. If SSSD runs under 'root' then 'root' ownership of this file will be also allowed in runtime. --- contrib/sssd.spec.in | 2 +- src/man/Makefile.am | 2 ++ src/man/sssd.conf.5.xml | 15 ++++++++++----- src/util/sss_ini.c | 37 +++++++++++++++++++++++++++++-------- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 9c4e0be7a97..7c0fdadc616 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -776,7 +776,7 @@ done %attr(700,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd %attr(700,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/conf.d %attr(700,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/pki -%ghost %attr(0600,root,root) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf +%ghost %attr(0600,%{sssd_user},%{sssd_user}) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf %dir %{_sysconfdir}/logrotate.d %config(noreplace) %{_sysconfdir}/logrotate.d/sssd %dir %{_sysconfdir}/rwtab.d diff --git a/src/man/Makefile.am b/src/man/Makefile.am index 77b08e84ce2..38666e9c104 100644 --- a/src/man/Makefile.am +++ b/src/man/Makefile.am @@ -62,6 +62,8 @@ ENUM_CONDS = ;without_ext_enumeration endif if SSSD_NON_ROOT_USER SSSD_NON_ROOT_USER_CONDS = ;with_non_root_user_support +else +SSSD_NON_ROOT_USER_CONDS = ;without_non_root_user_support endif diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 8fecf49f026..c9bcc44e301 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -52,9 +52,15 @@ is only as a label for the section. - - sssd.conf must be a regular file, owned by - root and only root may read from or write to the file. + + sssd.conf must be a regular file that is owned, + readable, and writeable only by 'root'. + + + sssd.conf must be a regular file that is owned, + readable, and writeable by '&sssd_user_name;' user (if SSSD is configured + to run under 'root' then sssd.conf also + can be owned by 'root'). @@ -92,8 +98,7 @@ The snippet files require the same owner and permissions - as sssd.conf. Which are by default - root:root and 0600. + as sssd.conf. diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 3f71b18ae9c..e5ff64b8ec5 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -149,18 +149,39 @@ static int sss_ini_config_file_from_mem(struct sss_ini *self, static int sss_ini_access_check(struct sss_ini *self) { + uid_t uid = 0; + gid_t gid = 0; + int ret; + if (!self->main_config_exists) { return EOK; } - return ini_config_access_check(self->file, - INI_ACCESS_CHECK_MODE | - INI_ACCESS_CHECK_UID | - INI_ACCESS_CHECK_GID, - 0, /* owned by root */ - 0, /* owned by root */ - S_IRUSR, /* r**------ */ - ALLPERMS & ~(S_IWUSR|S_IXUSR)); + /* 'sssd:sssd' owned config is always fine */ + sss_sssd_user_uid_and_gid(&uid, &gid); + ret = ini_config_access_check(self->file, + INI_ACCESS_CHECK_MODE | + INI_ACCESS_CHECK_UID | + INI_ACCESS_CHECK_GID, + uid, /* owned by SSSD_USER */ + gid, /* owned by SSSD_USER */ + S_IRUSR, /* r**------ */ + ALLPERMS & ~(S_IWUSR|S_IXUSR)); + if (ret != 0) { + /* if SSSD runs under 'root' then 'root:root' owned config is also fine */ + if ((getuid() == 0) && (uid != 0)) { + ret = ini_config_access_check(self->file, + INI_ACCESS_CHECK_MODE | + INI_ACCESS_CHECK_UID | + INI_ACCESS_CHECK_GID, + 0, /* owned by root */ + 0, /* owned by root */ + S_IRUSR, /* r**------ */ + ALLPERMS & ~(S_IWUSR|S_IXUSR)); + } + } + + return ret; }