From 6c849600b416ddd8a4b9131228a3db3483f5f458 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 14:22:52 +0200 Subject: [PATCH 01/42] MONITOR: remove useless trailing '\' Most probably it was copy-paste from macro definition. --- src/monitor/monitor.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index b0fb661b1d7..399392607c3 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1998,21 +1998,19 @@ int main(int argc, const char *argv[]) SSSD_MAIN_OPTS SSSD_LOGGER_OPTS SSSD_CONFIG_OPTS(opt_config_file) - {"daemon", 'D', POPT_ARG_NONE, &opt_daemon, 0, \ - _("Become a daemon (default)"), NULL }, \ - {"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, \ - _("Run interactive (not a daemon)"), NULL}, \ + {"daemon", 'D', POPT_ARG_NONE, &opt_daemon, 0, + _("Become a daemon (default)"), NULL }, + {"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, + _("Run interactive (not a daemon)"), NULL}, {"disable-netlink", '\0', POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN, - &opt_netlinkoff, 0, \ - _("Disable netlink interface"), NULL}, \ - {"genconf", 'g', POPT_ARG_NONE, &opt_genconf, 0, \ - _("Refresh the configuration database, then exit"), \ - NULL}, \ - {"genconf-section", 's', POPT_ARG_STRING, &opt_genconf_section, 0, \ - _("Similar to --genconf, but only refreshes the given section"), \ - NULL}, \ - {"version", '\0', POPT_ARG_NONE, &opt_version, 0, \ - _("Print version number and exit"), NULL }, \ + &opt_netlinkoff, 0, + _("Disable netlink interface"), NULL}, + {"genconf", 'g', POPT_ARG_NONE, &opt_genconf, 0, + _("Refresh the configuration database, then exit"), NULL}, + {"genconf-section", 's', POPT_ARG_STRING, &opt_genconf_section, 0, + _("Similar to --genconf, but only refreshes the given section"), NULL}, + {"version", '\0', POPT_ARG_NONE, &opt_version, 0, + _("Print version number and exit"), NULL }, POPT_TABLEEND }; From c4eb784def78ace375d76abba81eef54360bea84 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 14:24:37 +0200 Subject: [PATCH 02/42] MONITOR: remove 'opt_netlinkoff' removal notice It was given in 632fc5d8991d167eea20769c823163551c3f1d8c several years ago. --- src/monitor/monitor.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 399392607c3..121b1cc5483 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1981,7 +1981,6 @@ int main(int argc, const char *argv[]) int opt_interactive = 0; int opt_genconf = 0; int opt_version = 0; - int opt_netlinkoff = 0; char *opt_config_file = NULL; const char *opt_logger = NULL; char *config_file = NULL; @@ -2002,9 +2001,6 @@ int main(int argc, const char *argv[]) _("Become a daemon (default)"), NULL }, {"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, _("Run interactive (not a daemon)"), NULL}, - {"disable-netlink", '\0', POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN, - &opt_netlinkoff, 0, - _("Disable netlink interface"), NULL}, {"genconf", 'g', POPT_ARG_NONE, &opt_genconf, 0, _("Refresh the configuration database, then exit"), NULL}, {"genconf-section", 's', POPT_ARG_STRING, &opt_genconf_section, 0, @@ -2099,16 +2095,6 @@ int main(int argc, const char *argv[]) } else { config_file = talloc_strdup(tmp_ctx, SSSD_CONFIG_FILE); } - - if (opt_netlinkoff) { - DEBUG(SSSDBG_MINOR_FAILURE, - "Option --disable-netlink has been removed and " - "replaced as a monitor option in sssd.conf\n"); - sss_log(SSS_LOG_ALERT, - "--disable-netlink has been deprecated, tunable option " - "disable_netlink available as replacement(man sssd.conf)"); - } - if (!config_file) { return 6; } From f9a83c6977e8f15fa01fc8e4f05d0601c9766301 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 14:57:04 +0200 Subject: [PATCH 03/42] MONITOR: replace fprintf() with ERROR() --- src/monitor/monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 121b1cc5483..efac7917689 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -2017,8 +2017,8 @@ int main(int argc, const char *argv[]) while((opt = poptGetNextOpt(pc)) != -1) { switch(opt) { default: - fprintf(stderr, "\nInvalid option %s: %s\n\n", - poptBadOption(pc, 0), poptStrerror(opt)); + ERROR("\nInvalid option %s: %s\n\n", + poptBadOption(pc, 0), poptStrerror(opt)); poptPrintUsage(pc, stderr, 0); return 1; } From 7322478a20df538c49e3bb629633c68c95667d8b Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 15:07:45 +0200 Subject: [PATCH 04/42] MNITOR: cosmetics Keep all checks of command line options together and slightly reorder for a (hopefully) better readability. Error exit codes updated to: - 1 - bad command line options or config - 2 - no mem - 5 - all kinds of other issues --- src/monitor/monitor.c | 88 ++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index efac7917689..2543293a9e7 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1985,13 +1985,18 @@ int main(int argc, const char *argv[]) const char *opt_logger = NULL; char *config_file = NULL; char *opt_genconf_section = NULL; - int flags = 0; + int flags = FLAGS_NO_WATCHDOG; struct main_context *main_ctx; TALLOC_CTX *tmp_ctx; struct mt_ctx *monitor; int ret; uid_t uid; + tmp_ctx = talloc_new(NULL); + if (!tmp_ctx) { + return 2; + } + struct poptOption long_options[] = { POPT_AUTOHELP SSSD_MAIN_OPTS @@ -2029,13 +2034,6 @@ int main(int argc, const char *argv[]) return EXIT_SUCCESS; } - if (opt_genconf_section) { - /* --genconf-section implies genconf, just restricted to a single - * section - */ - opt_genconf = 1; - } - /* If the level or timestamps was passed at the command-line, we want * to save it and pass it to the children later. */ @@ -2043,52 +2041,39 @@ int main(int argc, const char *argv[]) cmdline_debug_timestamps = debug_timestamps; cmdline_debug_microseconds = debug_microseconds; - if (opt_daemon && opt_interactive) { - ERROR("Option -i|--interactive is not allowed together with -D|--daemon\n"); - poptPrintUsage(pc, stderr, 0); - return 1; + if (opt_genconf_section) { + /* --genconf-section implies genconf, just limited to a single section */ + opt_genconf = 1; } - if (opt_genconf && (opt_daemon || opt_interactive)) { ERROR("Option -g is incompatible with -D or -i\n"); poptPrintUsage(pc, stderr, 0); return 1; } - - if (!opt_daemon && !opt_interactive && !opt_genconf) { - opt_daemon = 1; + if (opt_genconf) { + flags |= FLAGS_GEN_CONF; + if (!opt_logger) { + opt_logger = sss_logger_str[STDERR_LOGGER]; + } } - poptFreeContext(pc); - - uid = getuid(); - if (uid != 0) { - ERROR("Running under %"PRIu64", must be root\n", (uint64_t) uid); - sss_log(SSS_LOG_ALERT, "sssd must be run as root"); - return 8; + if (opt_daemon && opt_interactive) { + ERROR("Option -i|--interactive is not allowed together with -D|--daemon\n"); + poptPrintUsage(pc, stderr, 0); + return 1; } - tmp_ctx = talloc_new(NULL); - if (!tmp_ctx) { - return 7; + if (!opt_daemon && !opt_interactive && !opt_genconf) { + opt_daemon = 1; } - - if (opt_daemon) flags |= FLAGS_DAEMON; - if (opt_interactive) { + if (opt_daemon) { + flags |= FLAGS_DAEMON; + } else if (opt_interactive) { flags |= FLAGS_INTERACTIVE; if (!opt_logger) { opt_logger = sss_logger_str[STDERR_LOGGER]; } } - if (opt_genconf) { - flags |= FLAGS_GEN_CONF; - if (!opt_logger) { - opt_logger = sss_logger_str[STDERR_LOGGER]; - } - } - - /* default value of 'debug_prg_name' will be used */ - DEBUG_INIT(debug_level, opt_logger); if (opt_config_file) { config_file = talloc_strdup(tmp_ctx, opt_config_file); @@ -2096,11 +2081,20 @@ int main(int argc, const char *argv[]) config_file = talloc_strdup(tmp_ctx, SSSD_CONFIG_FILE); } if (!config_file) { - return 6; + return 2; } - /* the monitor should not run a watchdog on itself */ - flags |= FLAGS_NO_WATCHDOG; + poptFreeContext(pc); + + uid = getuid(); + if (uid != 0) { + ERROR("Running under %"PRIu64", must be root\n", (uint64_t) uid); + sss_log(SSS_LOG_ALERT, "sssd must be run as root"); + return 1; + } + + /* default value of 'debug_prg_name' will be used */ + DEBUG_INIT(debug_level, opt_logger); #ifdef USE_KEYRING /* Do this before all the forks, it sets the session key ring so all @@ -2136,7 +2130,7 @@ int main(int argc, const char *argv[]) DEBUG(SSSDBG_FATAL_FAILURE, "pidfile exists at %s\n", SSSD_PIDFILE); ERROR("SSSD is already running\n"); - return 2; + return 5; } } @@ -2194,7 +2188,7 @@ int main(int argc, const char *argv[]) ret, sss_strerror(ret)); break; } - return 4; + return 5; } /* at this point we are done generating the config file, we may exit @@ -2204,11 +2198,11 @@ int main(int argc, const char *argv[]) /* set up things like debug, signals, daemonization, etc. */ monitor->conf_path = CONFDB_MONITOR_CONF_ENTRY; ret = close(STDIN_FILENO); - if (ret != EOK) return 6; + if (ret != EOK) return 5; ret = server_setup(SSSD_MONITOR_NAME, false, flags, CONFDB_FILE, monitor->conf_path, &main_ctx, false); - if (ret != EOK) return 2; + if (ret != EOK) return 5; /* Use confd initialized in server_setup. ldb_tdb module (1.4.0) check PID * of process which initialized db for locking purposes. @@ -2221,7 +2215,7 @@ int main(int argc, const char *argv[]) ret = confdb_get_domains(monitor->cdb, &monitor->domains); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "No domains configured.\n"); - return 4; + return 1; } monitor->is_daemon = !opt_interactive; @@ -2230,8 +2224,8 @@ int main(int argc, const char *argv[]) talloc_steal(main_ctx, monitor); ret = monitor_process_init(monitor, config_file); + if (ret != EOK) return 5; - if (ret != EOK) return 3; talloc_free(tmp_ctx); /* loop on main */ From 75e9d18d8cc2c9be55d159b869ce20897a9852e3 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 15:10:16 +0200 Subject: [PATCH 05/42] MONITOR: get rid of unsed FLAGS_GEN_CONF definition --- src/monitor/monitor.c | 1 - src/util/util.h | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 2543293a9e7..7f071970200 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -2051,7 +2051,6 @@ int main(int argc, const char *argv[]) return 1; } if (opt_genconf) { - flags |= FLAGS_GEN_CONF; if (!opt_logger) { opt_logger = sss_logger_str[STDERR_LOGGER]; } diff --git a/src/util/util.h b/src/util/util.h index 960f301fd6d..cbb8b908781 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -109,7 +109,7 @@ extern int socket_activated; #define FLAGS_DAEMON 0x0001 #define FLAGS_INTERACTIVE 0x0002 #define FLAGS_PID_FILE 0x0004 -#define FLAGS_GEN_CONF 0x0008 +/* 0x0008 was used by FLAGS_GEN_CONF that was removed; can be reused */ #define FLAGS_NO_WATCHDOG 0x0010 enum sssd_exit_status { From 2cf8658fbb58ad822e06de2d1487877777c49bc5 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 31 Aug 2023 17:15:13 +0200 Subject: [PATCH 06/42] SPEC: make most folders group accessible This will allow to avoid the need for CAP_DAC_OVERRIDE with single addition of supplementary group. --- contrib/sssd.spec.in | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 791473d342b..5d5d9141d70 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -786,15 +786,15 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %dir %{sssdstatedir} %dir %{_localstatedir}/cache/krb5rcache -%attr(700,%{sssd_user},%{sssd_user}) %dir %{dbpath} +%attr(770,%{sssd_user},%{sssd_user}) %dir %{dbpath} %attr(775,%{sssd_user},%{sssd_user}) %dir %{mcpath} %attr(700,root,root) %dir %{secdbpath} -%attr(751,%{sssd_user},%{sssd_user}) %dir %{deskprofilepath} -%attr(755,%{sssd_user},%{sssd_user}) %dir %{pipepath} -%attr(750,%{sssd_user},root) %dir %{pipepath}/private -%attr(755,%{sssd_user},%{sssd_user}) %dir %{pubconfpath} -%attr(750,%{sssd_user},%{sssd_user}) %dir %{gpocachepath} -%attr(750,%{sssd_user},%{sssd_user}) %dir %{_var}/log/%{name} +%attr(771,%{sssd_user},%{sssd_user}) %dir %{deskprofilepath} +%attr(775,%{sssd_user},%{sssd_user}) %dir %{pipepath} +%attr(770,%{sssd_user},root) %dir %{pipepath}/private +%attr(775,%{sssd_user},%{sssd_user}) %dir %{pubconfpath} +%attr(770,%{sssd_user},%{sssd_user}) %dir %{gpocachepath} +%attr(770,%{sssd_user},%{sssd_user}) %dir %{_var}/log/%{name} %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 @@ -846,7 +846,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %files krb5-common %license COPYING -%attr(755,%{sssd_user},%{sssd_user}) %dir %{pubconfpath}/krb5.include.d +%attr(775,%{sssd_user},%{sssd_user}) %dir %{pubconfpath}/krb5.include.d %attr(%{child_attrs},root,%{sssd_user}) %{_libexecdir}/%{servicename}/ldap_child %attr(%{child_attrs},root,%{sssd_user}) %{_libexecdir}/%{servicename}/krb5_child @@ -864,7 +864,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %files ipa -f sssd_ipa.lang %license COPYING -%attr(700,%{sssd_user},%{sssd_user}) %dir %{keytabdir} +%attr(770,%{sssd_user},%{sssd_user}) %dir %{keytabdir} %{_libdir}/%{name}/libsss_ipa.so %attr(%{child_attrs},root,%{sssd_user}) %{_libexecdir}/%{servicename}/selinux_child %{_mandir}/man5/sssd-ipa.5* From cc92721602bcbc889e6666bd3097d9a8de7f644a Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 4 Jan 2024 13:40:41 +0100 Subject: [PATCH 07/42] SPEC: make '%{pipepath}/private' sssd:sssd owned Since db1a919ff5760119df3083f535e66d0e4470cad8 the only socket in '/private' is an internal SBUS socket. --- contrib/sssd.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 5d5d9141d70..1cb602d87a1 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -791,7 +791,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %attr(700,root,root) %dir %{secdbpath} %attr(771,%{sssd_user},%{sssd_user}) %dir %{deskprofilepath} %attr(775,%{sssd_user},%{sssd_user}) %dir %{pipepath} -%attr(770,%{sssd_user},root) %dir %{pipepath}/private +%attr(770,%{sssd_user},%{sssd_user}) %dir %{pipepath}/private %attr(775,%{sssd_user},%{sssd_user}) %dir %{pubconfpath} %attr(770,%{sssd_user},%{sssd_user}) %dir %{gpocachepath} %attr(770,%{sssd_user},%{sssd_user}) %dir %{_var}/log/%{name} From 6b5d7337ada58bc101ab8e07a7fc959213e652b4 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 31 Aug 2023 17:15:13 +0200 Subject: [PATCH 08/42] Make all SSSD processes a member of sssd supplementary group. Previously it was done only for 'sssd_nss' to allow it to write to sssd:sssd owned mem-cache file while running under 'root'. Let's use this approach for all other files to avoid using CAP_DAC_OVERRIDE in run time (in following patches). Primarily rely on systemd to set group, but try to set it manually if (required and) missing at runtime. --- Makefile.am | 11 ++- src/monitor/monitor.c | 11 +++ src/monitor/monitor_bootstrap.c | 122 ++++++++++++++++++++++++ src/responder/nss/nss_private.h | 2 - src/responder/nss/nsssrv.c | 104 +++----------------- src/responder/nss/nsssrv_mmap_cache.c | 2 + src/sysv/systemd/sssd-autofs.service.in | 1 + src/sysv/systemd/sssd-ifp.service.in | 1 + src/sysv/systemd/sssd-kcm.service.in | 1 + src/sysv/systemd/sssd-nss.service.in | 1 + src/sysv/systemd/sssd-pac.service.in | 1 + src/sysv/systemd/sssd-pam.service.in | 1 + src/sysv/systemd/sssd-ssh.service.in | 1 + src/sysv/systemd/sssd-sudo.service.in | 1 + src/sysv/systemd/sssd.service.in | 1 + 15 files changed, 163 insertions(+), 98 deletions(-) create mode 100644 src/monitor/monitor_bootstrap.c diff --git a/Makefile.am b/Makefile.am index f2a6b836787..5d44a5a1922 100644 --- a/Makefile.am +++ b/Makefile.am @@ -105,12 +105,15 @@ if SSSD_NON_ROOT_USER additional_caps = CAP_DAC_OVERRIDE nss_service_user_group = User=$(SSSD_USER)\nGroup=$(SSSD_USER) nss_socket_user_group = SocketUser=$(SSSD_USER)\nSocketGroup=$(SSSD_USER) -endif +supplementary_groups = \# If service configured to be run under "root", uncomment "SupplementaryGroups"\n\#SupplementaryGroups=$(SSSD_USER) +else +supplementary_groups = \# Note: SSSD package was built without support of running as non-privileged user +endif # SSSD_NON_ROOT_USER else ifp_dbus_exec_comment = \# "sss_signal" is used to force SSSD monitor to trigger "sssd_ifp" reconnection to dbus ifp_dbus_exec_cmd = $(sssdlibexecdir)/sss_signal ifp_systemdservice = -endif +endif # HAVE_SYSTEMD_UNIT secdbpath = @secdbpath@ @@ -1512,6 +1515,7 @@ endif #################### sssd_SOURCES = \ src/monitor/monitor.c \ + src/monitor/monitor_bootstrap.c \ src/monitor/monitor_netlink.c \ src/confdb/confdb_setup.c \ src/util/nscd.c \ @@ -5289,7 +5293,8 @@ edit_cmd = $(SED) \ -e 's|@condconfigexists[@]|$(condconfigexists)|g' \ -e 's|@additional_caps[@]|$(additional_caps)|g' \ -e 's|@nss_service_user_group[@]|$(nss_service_user_group)|g' \ - -e 's|@nss_socket_user_group[@]|$(nss_socket_user_group)|g' + -e 's|@nss_socket_user_group[@]|$(nss_socket_user_group)|g' \ + -e 's|@supplementary_groups[@]|$(supplementary_groups)|g' replace_script = \ @rm -f $@ $@.tmp; \ diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 7f071970200..74c665cffbd 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -19,6 +19,7 @@ along with this program. If not, see . */ +#include "config.h" #include "util/util.h" #include "util/child_common.h" #include @@ -1973,6 +1974,8 @@ static void monitor_restart_service(struct mt_svc *svc) } } +int bootstrap_monitor_process(void); + int main(int argc, const char *argv[]) { int opt; @@ -2085,6 +2088,7 @@ int main(int argc, const char *argv[]) poptFreeContext(pc); + /* TODO: revisit */ uid = getuid(); if (uid != 0) { ERROR("Running under %"PRIu64", must be root\n", (uint64_t) uid); @@ -2092,6 +2096,13 @@ int main(int argc, const char *argv[]) return 1; } + ret = bootstrap_monitor_process(); + if (ret != 0) { + ERROR("Failed to boostrap SSSD 'monitor' process: %s", sss_strerror(ret)); + sss_log(SSS_LOG_ALERT, "Failed to boostrap SSSD 'monitor' process."); + return 5; + } + /* default value of 'debug_prg_name' will be used */ DEBUG_INIT(debug_level, opt_logger); diff --git a/src/monitor/monitor_bootstrap.c b/src/monitor/monitor_bootstrap.c new file mode 100644 index 00000000000..2ee33f079c8 --- /dev/null +++ b/src/monitor/monitor_bootstrap.c @@ -0,0 +1,122 @@ +/* + SSSD + + Service monitor bootstrap + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "config.h" + +#include +#include +#include +#include + +#include "util/util.h" + + +/* Attention! + * When those routines are being executed, internal logger isn't yet initialized. + */ + + +#ifdef SSSD_NON_ROOT_USER +/* returns: -1 on error, 0 - group not set, 1 - group set */ +static int check_supplementary_group(gid_t gid) +{ + int size; + gid_t *supp_gids = NULL; + + if (getegid() == gid) { + return 1; + } + + size = getgroups(0, NULL); + if (size == -1) { + return -1; + } + + if (size > 0) { + supp_gids = talloc_zero_array(NULL, gid_t, size); + if (supp_gids == NULL) { + return -1; + } + + size = getgroups(size, supp_gids); + if (size == -1) { + talloc_free(supp_gids); + return -1; + } + + for (int i = 0; i < size; i++) { + if (supp_gids[i] == gid) { + talloc_free(supp_gids); + return 1; + } + } + + talloc_free(supp_gids); + } + + return 0; +} +#endif /* SSSD_NON_ROOT_USER */ + +int bootstrap_monitor_process(void) +{ + +#ifdef SSSD_NON_ROOT_USER + /* In case SSSD is built with non-root user support, + * a number of files are sssd:sssd owned. + * Make sure all processes are added to sssd supplementary + * group to avoid the need for CAP_DAC_OVERRIDE. + * + * TODO: read 'sssd.conf::user' first and in case it is set + * to 'sssd' become_user(sssd) instead. + */ + int ret; + gid_t sssd_gid = 0; + if ((getuid() == 0) || (geteuid() == 0)) { + sss_sssd_user_uid_and_gid(NULL, &sssd_gid); + ret = check_supplementary_group(sssd_gid); + if (ret == -1) { + sss_log(SSS_LOG_ALERT, "Can't check own supplementary groups."); + return 1; + } + /* Expected outcome is 'ret == 1' since supplementary group should be set + by systemd service description. */ + if (ret == 0) { + /* Probably non-systemd based system or service file was edited, + let's try to set group manually. */ + sss_log(SSS_LOG_WARNING, + "SSSD is built with support of 'run under non-root user' " + "feature but started under 'root'. Trying to add process " + "to SSSD supplementary group."); + ret = setgroups(1, &sssd_gid); + if (ret != 0) { + sss_log(SSS_LOG_CRIT, + "Failed to add process to the "SSSD_USER" supplementary group. " + "Either run under '"SSSD_USER"' or make sure that run-under-root " + "main SSSD process has CAP_SETGID."); + return 1; + } + } + + /* TODO: drop CAP_SET_GID capability */ + } +#endif /* SSSD_NON_ROOT_USER */ + + return 0; +} diff --git a/src/responder/nss/nss_private.h b/src/responder/nss/nss_private.h index e2f5a3e5a58..71e33ff1b80 100644 --- a/src/responder/nss/nss_private.h +++ b/src/responder/nss/nss_private.h @@ -93,8 +93,6 @@ struct sss_nss_ctx { struct sss_mc_ctx *grp_mc_ctx; struct sss_mc_ctx *initgr_mc_ctx; struct sss_mc_ctx *sid_mc_ctx; - uid_t mc_uid; - gid_t mc_gid; }; struct sss_cmd_table *get_sss_nss_cmds(void); diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index 45ce78fcaad..0bc53191233 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -92,7 +92,7 @@ sss_nss_clear_memcache(TALLOC_CTX *mem_ctx, } DEBUG(SSSDBG_TRACE_FUNC, "Clearing memory caches.\n"); - ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid, + ret = sss_mmap_cache_reinit(nctx, -1, -1, -1, /* keep current size */ (time_t) memcache_timeout, &nctx->pwd_mc_ctx); @@ -102,7 +102,7 @@ sss_nss_clear_memcache(TALLOC_CTX *mem_ctx, goto done; } - ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid, + ret = sss_mmap_cache_reinit(nctx, -1, -1, -1, /* keep current size */ (time_t) memcache_timeout, &nctx->grp_mc_ctx); @@ -112,7 +112,7 @@ sss_nss_clear_memcache(TALLOC_CTX *mem_ctx, goto done; } - ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid, + ret = sss_mmap_cache_reinit(nctx, -1, -1, -1, /* keep current size */ (time_t)memcache_timeout, &nctx->initgr_mc_ctx); @@ -287,6 +287,10 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) int mc_size_group; int mc_size_initgroups; int mc_size_sid; + uid_t uid; + gid_t gid; + + sss_sssd_user_uid_and_gid(&uid, &gid); /* Remove the CLEAR_MC_FLAG file if exists. */ ret = unlink(SSS_NSS_MCACHE_DIR"/"CLEAR_MC_FLAG); @@ -361,7 +365,7 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) /* Initialize the fast in-memory caches if they were not disabled */ ret = sss_mmap_cache_init(nctx, "passwd", - nctx->mc_uid, nctx->mc_gid, + uid, gid, SSS_MC_PASSWD, mc_size_passwd * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -373,7 +377,7 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) } ret = sss_mmap_cache_init(nctx, "group", - nctx->mc_uid, nctx->mc_gid, + uid, gid, SSS_MC_GROUP, mc_size_group * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -385,7 +389,7 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) } ret = sss_mmap_cache_init(nctx, "initgroups", - nctx->mc_uid, nctx->mc_gid, + uid, gid, SSS_MC_INITGROUPS, mc_size_initgroups * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -397,7 +401,7 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) } ret = sss_mmap_cache_init(nctx, "sid", - nctx->mc_uid, nctx->mc_gid, + uid, gid, SSS_MC_SID, mc_size_sid * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -441,79 +445,6 @@ sss_nss_register_service_iface(struct sss_nss_ctx *nss_ctx, return ret; } -static int sssd_supplementary_group(struct sss_nss_ctx *nss_ctx) -{ - errno_t ret; - int size; - gid_t *supp_gids = NULL; - - /* - * We explicitly read the IDs of the SSSD user even though the server - * receives --uid and --gid by parameters to account for the case where - * the SSSD is compiled --with-sssd-user=sssd but the default of the - * user option is root (this is what RHEL does) - */ - ret = sss_user_by_name_or_uid(SSSD_USER, - &nss_ctx->mc_uid, - &nss_ctx->mc_gid); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, "Cannot get info on "SSSD_USER); - return ret; - } - - if (getgid() == nss_ctx->mc_gid) { - DEBUG(SSSDBG_TRACE_FUNC, "Already running as the sssd group\n"); - return EOK; - } - - size = getgroups(0, NULL); - if (size == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, "Getgroups failed! (%d, %s)\n", - ret, sss_strerror(ret)); - return ret; - } - - if (size > 0) { - supp_gids = talloc_zero_array(NULL, gid_t, size); - if (supp_gids == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "Allocation failed!\n"); - ret = ENOMEM; - goto done; - } - - size = getgroups(size, supp_gids); - if (size == -1) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, "Getgroups failed! (%d, %s)\n", - ret, sss_strerror(ret)); - goto done; - } - - for (int i = 0; i < size; i++) { - if (supp_gids[i] == nss_ctx->mc_gid) { - DEBUG(SSSDBG_TRACE_FUNC, - "Already assigned to the SSSD supplementary group\n"); - ret = EOK; - goto done; - } - } - } - - ret = setgroups(1, &nss_ctx->mc_gid); - if (ret != EOK) { - ret = errno; - DEBUG(SSSDBG_OP_FAILURE, - "Cannot setgroups [%d]: %s\n", ret, sss_strerror(ret)); - goto done; - } - - ret = EOK; -done: - talloc_free(supp_gids); - return ret; -} - int sss_nss_process_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct confdb_ctx *cdb) @@ -605,19 +536,6 @@ int sss_nss_process_init(TALLOC_CTX *mem_ctx, goto fail; } - /* - * Adding the NSS process to the SSSD supplementary group avoids - * dac_override AVC messages from SELinux in case sssd_nss runs - * as root and tries to write to memcache owned by sssd:sssd - */ - ret = sssd_supplementary_group(nctx); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, - "Cannot add process to the sssd supplementary group [%d]: %s\n", - ret, sss_strerror(ret)); - goto fail; - } - ret = setup_memcaches(nctx); if (ret != EOK) { goto fail; diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index 06f3d7a694a..7d4f23c05e4 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -1291,6 +1291,7 @@ static errno_t sss_mc_create_file(struct sss_mc_ctx *mc_ctx) return ret; } +#ifdef SSSD_NON_ROOT_USER /* Make sure that the memory cache files are chowned to sssd.sssd even * if the nss responder runs as root. This is because the specfile * has the ownership recorded as sssd.sssd @@ -1304,6 +1305,7 @@ static errno_t sss_mc_create_file(struct sss_mc_ctx *mc_ctx) return ret; } } +#endif /* SSSD_NON_ROOT_USER */ ret = sss_br_lock_file(mc_ctx->fd, 0, 1, retries, t); if (ret != EOK) { diff --git a/src/sysv/systemd/sssd-autofs.service.in b/src/sysv/systemd/sssd-autofs.service.in index e917ed78e27..ce597602b24 100644 --- a/src/sysv/systemd/sssd-autofs.service.in +++ b/src/sysv/systemd/sssd-autofs.service.in @@ -16,3 +16,4 @@ ExecStart=@libexecdir@/sssd/sssd_autofs ${DEBUG_LOGGER} --socket-activated Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-ifp.service.in b/src/sysv/systemd/sssd-ifp.service.in index 4aed90bb96b..2e307f3b001 100644 --- a/src/sysv/systemd/sssd-ifp.service.in +++ b/src/sysv/systemd/sssd-ifp.service.in @@ -18,3 +18,4 @@ CapabilityBoundingSet= @additional_caps@ Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-kcm.service.in b/src/sysv/systemd/sssd-kcm.service.in index cb1e7a3a154..1c6b914ab82 100644 --- a/src/sysv/systemd/sssd-kcm.service.in +++ b/src/sysv/systemd/sssd-kcm.service.in @@ -14,3 +14,4 @@ ExecStart=@libexecdir@/sssd/sssd_kcm ${DEBUG_LOGGER} # ('User=' and 'Group=' defaults to 'root' for system services) # 'CapabilityBoundingSet' is used to limit privileges set: CapabilityBoundingSet= @additional_caps@ CAP_SETGID CAP_SETUID +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-nss.service.in b/src/sysv/systemd/sssd-nss.service.in index 0bf2bb2de13..c0e5fa4ec31 100644 --- a/src/sysv/systemd/sssd-nss.service.in +++ b/src/sysv/systemd/sssd-nss.service.in @@ -20,3 +20,4 @@ Restart=on-failure # In case SSSD was built with support of running under non-root user, there is a special # handling in 'libnss_sss' and it is allowed to use build time configured user in 'User='/'Group=' @nss_service_user_group@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-pac.service.in b/src/sysv/systemd/sssd-pac.service.in index db435897abb..b1bc2907303 100644 --- a/src/sysv/systemd/sssd-pac.service.in +++ b/src/sysv/systemd/sssd-pac.service.in @@ -16,3 +16,4 @@ ExecStart=@libexecdir@/sssd/sssd_pac ${DEBUG_LOGGER} --socket-activated Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-pam.service.in b/src/sysv/systemd/sssd-pam.service.in index ffb9665d2d8..19d4bd3c733 100644 --- a/src/sysv/systemd/sssd-pam.service.in +++ b/src/sysv/systemd/sssd-pam.service.in @@ -16,3 +16,4 @@ ExecStart=@libexecdir@/sssd/sssd_pam ${DEBUG_LOGGER} --socket-activated Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-ssh.service.in b/src/sysv/systemd/sssd-ssh.service.in index 5b992e08376..42324b922c2 100644 --- a/src/sysv/systemd/sssd-ssh.service.in +++ b/src/sysv/systemd/sssd-ssh.service.in @@ -16,3 +16,4 @@ ExecStart=@libexecdir@/sssd/sssd_ssh ${DEBUG_LOGGER} --socket-activated Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd-sudo.service.in b/src/sysv/systemd/sssd-sudo.service.in index 0092442fbc3..0adb9c0a9d3 100644 --- a/src/sysv/systemd/sssd-sudo.service.in +++ b/src/sysv/systemd/sssd-sudo.service.in @@ -16,3 +16,4 @@ ExecStart=@libexecdir@/sssd/sssd_sudo ${DEBUG_LOGGER} --socket-activated Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ +@supplementary_groups@ diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in index 4d9596a8374..b988d43b6d9 100644 --- a/src/sysv/systemd/sssd.service.in +++ b/src/sysv/systemd/sssd.service.in @@ -19,6 +19,7 @@ PIDFile=@pidpath@/sssd.pid # 'CapabilityBoundingSet' is used to limit privileges set: CapabilityBoundingSet= @additional_caps@ CAP_CHOWN CAP_KILL CAP_SETGID CAP_SETUID Restart=on-abnormal +@supplementary_groups@ [Install] WantedBy=multi-user.target From f27c099dd568ca2dfd05684dc6ff7f21172cd33c Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 30 Jan 2024 21:13:42 +0100 Subject: [PATCH 09/42] NSS: don't `fchown()` mem-cache files Since ec77ec4e8b2f7ce80848f8840d7b9fa8403e297a mem-cache files aren't tracked as a part of a package anymore so there is no need to keep SSSD_USER ownership of those files. --- src/responder/nss/nsssrv.c | 14 +++------- src/responder/nss/nsssrv_mmap_cache.c | 39 ++------------------------- src/responder/nss/nsssrv_mmap_cache.h | 2 -- 3 files changed, 5 insertions(+), 50 deletions(-) diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index 0bc53191233..4d91f89f867 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -92,7 +92,7 @@ sss_nss_clear_memcache(TALLOC_CTX *mem_ctx, } DEBUG(SSSDBG_TRACE_FUNC, "Clearing memory caches.\n"); - ret = sss_mmap_cache_reinit(nctx, -1, -1, + ret = sss_mmap_cache_reinit(nctx, -1, /* keep current size */ (time_t) memcache_timeout, &nctx->pwd_mc_ctx); @@ -102,7 +102,7 @@ sss_nss_clear_memcache(TALLOC_CTX *mem_ctx, goto done; } - ret = sss_mmap_cache_reinit(nctx, -1, -1, + ret = sss_mmap_cache_reinit(nctx, -1, /* keep current size */ (time_t) memcache_timeout, &nctx->grp_mc_ctx); @@ -112,7 +112,7 @@ sss_nss_clear_memcache(TALLOC_CTX *mem_ctx, goto done; } - ret = sss_mmap_cache_reinit(nctx, -1, -1, + ret = sss_mmap_cache_reinit(nctx, -1, /* keep current size */ (time_t)memcache_timeout, &nctx->initgr_mc_ctx); @@ -287,10 +287,6 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) int mc_size_group; int mc_size_initgroups; int mc_size_sid; - uid_t uid; - gid_t gid; - - sss_sssd_user_uid_and_gid(&uid, &gid); /* Remove the CLEAR_MC_FLAG file if exists. */ ret = unlink(SSS_NSS_MCACHE_DIR"/"CLEAR_MC_FLAG); @@ -365,7 +361,6 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) /* Initialize the fast in-memory caches if they were not disabled */ ret = sss_mmap_cache_init(nctx, "passwd", - uid, gid, SSS_MC_PASSWD, mc_size_passwd * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -377,7 +372,6 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) } ret = sss_mmap_cache_init(nctx, "group", - uid, gid, SSS_MC_GROUP, mc_size_group * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -389,7 +383,6 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) } ret = sss_mmap_cache_init(nctx, "initgroups", - uid, gid, SSS_MC_INITGROUPS, mc_size_initgroups * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, @@ -401,7 +394,6 @@ static int setup_memcaches(struct sss_nss_ctx *nctx) } ret = sss_mmap_cache_init(nctx, "sid", - uid, gid, SSS_MC_SID, mc_size_sid * SSS_MC_CACHE_SLOTS_PER_MB, (time_t)memcache_timeout, diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index 7d4f23c05e4..d1ba9302605 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -52,9 +52,6 @@ struct sss_mc_ctx { char *file; /* mmap cache file name */ int fd; /* file descriptor */ - uid_t uid; /* User ID of owner */ - gid_t gid; /* Group ID of owner */ - uint32_t seed; /* pseudo-random seed to avoid collision attacks */ time_t valid_time_slot; /* maximum time the entry is valid in seconds */ @@ -650,9 +647,7 @@ static errno_t sss_mc_get_record(struct sss_mc_ctx **_mcc, if (ret == EFAULT) { DEBUG(SSSDBG_CRIT_FAILURE, "Fatal internal mmap cache error, invalidating cache!\n"); - (void)sss_mmap_cache_reinit(talloc_parent(mcc), - -1, -1, -1, -1, - _mcc); + (void)sss_mmap_cache_reinit(talloc_parent(mcc), -1, -1, _mcc); } return ret; } @@ -773,7 +768,7 @@ static errno_t sss_mmap_cache_validate_or_reinit(struct sss_mc_ctx **_mcc) done: if (reinit) { - return sss_mmap_cache_reinit(talloc_parent(mcc), -1, -1, -1, -1, _mcc); + return sss_mmap_cache_reinit(talloc_parent(mcc), -1, -1, _mcc); } return ret; @@ -1291,22 +1286,6 @@ static errno_t sss_mc_create_file(struct sss_mc_ctx *mc_ctx) return ret; } -#ifdef SSSD_NON_ROOT_USER - /* Make sure that the memory cache files are chowned to sssd.sssd even - * if the nss responder runs as root. This is because the specfile - * has the ownership recorded as sssd.sssd - */ - if ((getuid() == 0) || (geteuid() == 0)) { - ret = fchown(mc_ctx->fd, mc_ctx->uid, mc_ctx->gid); - if (ret != 0) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, "Failed to chown mmap file %s: %d(%s)\n", - mc_ctx->file, ret, strerror(ret)); - return ret; - } - } -#endif /* SSSD_NON_ROOT_USER */ - ret = sss_br_lock_file(mc_ctx->fd, 0, 1, retries, t); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, @@ -1389,7 +1368,6 @@ static int mc_ctx_destructor(struct sss_mc_ctx *mc_ctx) #define POSIX_FALLOCATE_ATTEMPTS 3 errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, - uid_t uid, gid_t gid, enum sss_mc_type type, size_t n_elem, time_t timeout, struct sss_mc_ctx **mcc) { @@ -1437,9 +1415,6 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, goto done; } - mc_ctx->uid = uid; - mc_ctx->gid = gid; - mc_ctx->type = type; mc_ctx->valid_time_slot = timeout; @@ -1533,7 +1508,6 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, } errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, - uid_t uid, gid_t gid, size_t n_elem, time_t timeout, struct sss_mc_ctx **mc_ctx) { @@ -1571,14 +1545,6 @@ errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, timeout = (*mc_ctx)->valid_time_slot; } - if (uid == (uid_t)-1) { - uid = (*mc_ctx)->uid; - } - - if (gid == (gid_t)-1) { - gid = (*mc_ctx)->gid; - } - talloc_free(*mc_ctx); /* make sure we do not leave a potentially freed pointer around */ @@ -1586,7 +1552,6 @@ errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, ret = sss_mmap_cache_init(mem_ctx, name, - uid, gid, type, n_elem, timeout, diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h index 28ee5adb643..ed00ad1ba35 100644 --- a/src/responder/nss/nsssrv_mmap_cache.h +++ b/src/responder/nss/nsssrv_mmap_cache.h @@ -33,7 +33,6 @@ enum sss_mc_type { }; errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, - uid_t uid, gid_t gid, enum sss_mc_type type, size_t n_elem, time_t valid_time, struct sss_mc_ctx **mcc); @@ -77,7 +76,6 @@ errno_t sss_mmap_cache_initgr_invalidate(struct sss_mc_ctx **_mcc, const struct sized_string *name); errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, - uid_t uid, gid_t gid, size_t n_elem, time_t timeout, struct sss_mc_ctx **mc_ctx); From 98b2b64555de628fcfeaed9f10ffd2308150f84f Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 2 Sep 2023 14:51:52 +0200 Subject: [PATCH 10/42] UTILS: add capabilities management helpers --- Makefile.am | 4 + configure.ac | 7 ++ contrib/ci/deps.sh | 2 + contrib/sssd.spec.in | 1 + src/util/capabilities.c | 208 ++++++++++++++++++++++++++++++++++++++++ src/util/server.c | 13 +++ src/util/util.h | 3 + 7 files changed, 238 insertions(+) create mode 100644 src/util/capabilities.c diff --git a/Makefile.am b/Makefile.am index 5d44a5a1922..f277bf6cfd1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1290,6 +1290,7 @@ libsss_util_la_SOURCES = \ src/util/well_known_sids.c \ src/util/string_utils.c \ src/util/become_user.c \ + src/util/capabilities.c \ src/util/util_watchdog.c \ src/util/sss_ptr_hash.c \ src/util/files.c \ @@ -1306,6 +1307,7 @@ libsss_util_la_CFLAGS = \ libsss_util_la_LIBADD = \ $(LIBADD_TIMER) \ $(SSSD_LIBS) \ + $(CAP_LIBS) \ $(SYSTEMD_LOGIN_LIBS) \ $(UNICODE_LIBS) \ $(PCRE_LIBS) \ @@ -4727,6 +4729,7 @@ krb5_child_LDADD = \ $(CLIENT_LIBS) \ $(SYSTEMD_LOGIN_LIBS) \ $(JANSSON_LIBS) \ + $(CAP_LIBS) \ $(NULL) ldap_child_SOURCES = \ @@ -4752,6 +4755,7 @@ ldap_child_LDADD = \ libsss_debug.la \ $(TALLOC_LIBS) \ $(POPT_LIBS) \ + $(CAP_LIBS) \ $(DHASH_LIBS) \ $(KRB5_LIBS) diff --git a/configure.ac b/configure.ac index 36302fbfb35..b41b468dcd3 100644 --- a/configure.ac +++ b/configure.ac @@ -513,6 +513,13 @@ AS_IF([test x$have_check = x], [ AC_CHECK_HEADERS([check.h],,AC_MSG_ERROR([Could not find CHECK headers])) ]) +PKG_CHECK_MODULES([CAP], [libcap], [have_libcap=1], [have_libcap=]) +AS_IF([test x$have_libcap = x], [ + AC_MSG_ERROR([libcap is missing]) +], [ + AC_CHECK_HEADERS([sys/capability.h],,AC_MSG_ERROR([Could not find sys/capability.h headers])) +]) + AC_PATH_PROG([DOXYGEN], [doxygen], [false]) AM_CONDITIONAL([HAVE_DOXYGEN], [test x$DOXYGEN != xfalse ]) diff --git a/contrib/ci/deps.sh b/contrib/ci/deps.sh index f6f50185866..426a743c823 100644 --- a/contrib/ci/deps.sh +++ b/contrib/ci/deps.sh @@ -46,6 +46,7 @@ if [[ "$DISTRO_BRANCH" == -redhat-* ]]; then krb5-server krb5-workstation libunistring-devel + libcap-devel ) if [[ "$DISTRO_BRANCH" == -redhat-redhatenterprise*-8.*- || @@ -180,6 +181,7 @@ if [[ "$DISTRO_BRANCH" == -debian-* ]]; then libp11-kit-dev bc libunistring-dev + libcap-dev ) DEPS_INTGCHECK_SATISFIED=true diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 1cb602d87a1..265501968e8 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -112,6 +112,7 @@ BuildRequires: gettext-devel # required for p11_child smartcard tests BuildRequires: gnutls-utils BuildRequires: jansson-devel +BuildRequires: libcap-devel BuildRequires: libcurl-devel BuildRequires: libjose-devel BuildRequires: keyutils-libs-devel diff --git a/src/util/capabilities.c b/src/util/capabilities.c new file mode 100644 index 00000000000..ca5f09bee29 --- /dev/null +++ b/src/util/capabilities.c @@ -0,0 +1,208 @@ +/* + SSSD + + Capabilities management helpers + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "config.h" +#include +#include +#include + +#include "util/util.h" + + +typedef struct _cap_description +{ + cap_value_t val; + const char *name; +} cap_description; + +#define _CAP_DESCR(cap) {cap, #cap} + +static cap_description _all_caps[] = +{ + _CAP_DESCR(CAP_AUDIT_CONTROL), + _CAP_DESCR(CAP_AUDIT_READ), + _CAP_DESCR(CAP_AUDIT_WRITE), + _CAP_DESCR(CAP_BLOCK_SUSPEND), + _CAP_DESCR(CAP_BPF), + _CAP_DESCR(CAP_CHECKPOINT_RESTORE), + _CAP_DESCR(CAP_CHOWN), + _CAP_DESCR(CAP_DAC_OVERRIDE), + _CAP_DESCR(CAP_DAC_READ_SEARCH), + _CAP_DESCR(CAP_FOWNER), + _CAP_DESCR(CAP_FSETID), + _CAP_DESCR(CAP_IPC_LOCK), + _CAP_DESCR(CAP_IPC_OWNER), + _CAP_DESCR(CAP_KILL), + _CAP_DESCR(CAP_LEASE), + _CAP_DESCR(CAP_LINUX_IMMUTABLE), + _CAP_DESCR(CAP_MAC_ADMIN), + _CAP_DESCR(CAP_MAC_OVERRIDE), + _CAP_DESCR(CAP_MKNOD), + _CAP_DESCR(CAP_NET_ADMIN), + _CAP_DESCR(CAP_NET_BIND_SERVICE), + _CAP_DESCR(CAP_NET_BROADCAST), + _CAP_DESCR(CAP_NET_RAW), + _CAP_DESCR(CAP_PERFMON), + _CAP_DESCR(CAP_SETGID), + _CAP_DESCR(CAP_SETFCAP), + _CAP_DESCR(CAP_SETPCAP), + _CAP_DESCR(CAP_SETUID), + _CAP_DESCR(CAP_SYS_ADMIN), + _CAP_DESCR(CAP_SYS_BOOT), + _CAP_DESCR(CAP_SYS_CHROOT), + _CAP_DESCR(CAP_SYS_MODULE), + _CAP_DESCR(CAP_SYS_NICE), + _CAP_DESCR(CAP_SYS_PACCT), + _CAP_DESCR(CAP_SYS_PTRACE), + _CAP_DESCR(CAP_SYS_RAWIO), + _CAP_DESCR(CAP_SYS_RESOURCE), + _CAP_DESCR(CAP_SYS_TIME), + _CAP_DESCR(CAP_SYS_TTY_CONFIG), + _CAP_DESCR(CAP_SYSLOG), + _CAP_DESCR(CAP_WAKE_ALARM) +}; + +static inline const char *cap_flag_to_str(cap_flag_value_t flag) +{ + if (flag == CAP_SET) { + return "*1*"; + } + return " 0 "; +} + +errno_t sss_log_caps_to_str(bool only_non_zero, char **_str) +{ + int ret; + char *str = NULL; + size_t i; + cap_t caps; + cap_flag_value_t effective, permitted, bounding; + + caps = cap_get_proc(); + if (caps == NULL) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, "cap_get_proc() failed: %d ('%s')\n", + ret, strerror(ret)); + return ret; + } + + for (i = 0; i < sizeof(_all_caps)/sizeof(cap_description); ++i) { + if (!CAP_IS_SUPPORTED(_all_caps[i].val)) { + continue; + } + ret = cap_get_flag(caps, _all_caps[i].val, CAP_EFFECTIVE, &effective); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, + "cap_get_flag(CAP_EFFECTIVE) failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } + ret = cap_get_flag(caps, _all_caps[i].val, CAP_PERMITTED, &permitted); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, + "cap_get_flag(CAP_PERMITTED) failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } + ret = cap_get_bound(_all_caps[i].val); + if (ret == 1) { + bounding = CAP_SET; + } else if (ret == 0) { + bounding = CAP_CLEAR; + } else { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, "cap_get_bound failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } + + if (only_non_zero && (effective == CAP_CLEAR) && + (permitted == CAP_CLEAR) && (bounding == CAP_CLEAR)) { + continue; + } + + str = talloc_asprintf_append(str, + " %25s: effective = %s, permitted = %s, bounding = %s\n", + _all_caps[i].name, cap_flag_to_str(effective), + cap_flag_to_str(permitted), cap_flag_to_str(bounding)); + if (str == NULL) { + ret = ENOMEM; + goto done; + } + } + + ret = 0; + +done: + if (ret == 0) { + *_str = str; + } else { + talloc_free(str); + } + + if (cap_free(caps) == -1) { + DEBUG(SSSDBG_TRACE_FUNC, "cap_free() failed\n"); + } + + return ret; +} + +errno_t sss_drop_cap(cap_value_t cap) +{ + int ret; + + cap_t caps = cap_get_proc(); + if (caps == NULL) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, "cap_get_proc() failed: %d ('%s')\n", + ret, strerror(ret)); + return ret; + } + if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap, CAP_CLEAR) == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, + "cap_set_flag(CAP_EFFECTIVE) failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } + if (cap_set_flag(caps, CAP_PERMITTED, 1, &cap, CAP_CLEAR) == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, + "cap_set_flag(CAP_PERMITTED) failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } + if (cap_set_proc(caps) == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, "cap_set_proc() failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } + + ret = 0; + +done: + if (cap_free(caps) == -1) { + DEBUG(SSSDBG_TRACE_FUNC, "cap_free() failed\n"); + } + + return ret; +} diff --git a/src/util/server.c b/src/util/server.c index d2c4a5ab42b..c2d6944db88 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -770,6 +770,19 @@ int server_setup(const char *name, bool is_responder, void server_loop(struct main_context *main_ctx) { + char *caps; + int ret; + + ret = sss_log_caps_to_str(true, &caps); + if (ret != 0) { + DEBUG(SSSDBG_IMPORTANT_INFO, "Failed to log current capabilities\n"); + } else { + DEBUG(SSSDBG_IMPORTANT_INFO, + "Entering main loop with following capabilities:\n%s", + caps ? caps : " (nothing)\n"); + talloc_free(caps); + } + /* wait for events - this is where the server sits for most of its life */ tevent_loop_wait(main_ctx->event_ctx); diff --git a/src/util/util.h b/src/util/util.h index cbb8b908781..0a04f281552 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -752,6 +753,8 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, int num_gids, gid_t *gids, struct sss_creds **saved_creds); errno_t restore_creds(struct sss_creds *saved_creds); +errno_t sss_log_caps_to_str(bool only_non_zero, char **_str); +errno_t sss_drop_cap(cap_value_t cap); /* from sss_semanage.c */ /* Please note that libsemange relies on files and directories created with From 8e052162164d5e4ad62061c27eba355e4ca77483 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 13 Sep 2023 18:59:46 +0200 Subject: [PATCH 11/42] Get rid of `--genconf` and `--genconf-section` monitor options. The only usage was 'sssd-kcm.service', but it was wrong since 'sssd_kcm' should be usable without other SSSD packages being installed (see #6926) --- Makefile.am | 1 - src/confdb/confdb_setup.c | 49 ++++--- src/confdb/confdb_setup.h | 15 -- src/man/sssd.8.xml | 27 ---- src/monitor/monitor.c | 127 ++++++----------- src/tests/multihost/basic/test_config.py | 114 --------------- src/tests/system/tests/test_config.py | 172 ----------------------- src/util/sss_ini.c | 46 ------ 8 files changed, 69 insertions(+), 482 deletions(-) delete mode 100644 src/tests/multihost/basic/test_config.py delete mode 100644 src/tests/system/tests/test_config.py diff --git a/Makefile.am b/Makefile.am index f277bf6cfd1..2fe094434b8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5705,7 +5705,6 @@ dist_noinst_DATA += \ src/tests/multihost/conftest.py \ src/tests/multihost/basic/mhc.yaml \ src/tests/multihost/basic/test_basic.py \ - src/tests/multihost/basic/test_config.py \ src/tests/multihost/basic/test_files.py \ src/tests/multihost/basic/test_ifp.py \ src/tests/multihost/basic/test_kcm.py \ diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index 5b459262ea2..ca75a0dcbd8 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -28,6 +28,22 @@ #include "confdb_setup.h" #include "util/sss_ini.h" +#define CONFDB_BASE_LDIF \ + "dn: @ATTRIBUTES\n" \ + "cn: CASE_INSENSITIVE\n" \ + "dc: CASE_INSENSITIVE\n" \ + "dn: CASE_INSENSITIVE\n" \ + "name: CASE_INSENSITIVE\n" \ + "objectclass: CASE_INSENSITIVE\n" \ + "\n" \ + "dn: @INDEXLIST\n" \ + "@IDXATTR: cn\n" \ + "\n" \ + "dn: @MODULES\n" \ + "@LIST: server_sort\n" \ + "\n" + + static int confdb_purge(struct confdb_ctx *cdb) { int ret; @@ -116,9 +132,7 @@ static int confdb_ldif_from_ini_file(TALLOC_CTX *mem_ctx, return EOK; } -static int confdb_write_ldif(struct confdb_ctx *cdb, - const char *config_ldif, - bool replace_whole_db) +static int confdb_write_ldif(struct confdb_ctx *cdb, const char *config_ldif) { int ret; struct ldb_ldif *ldif; @@ -133,21 +147,11 @@ static int confdb_write_ldif(struct confdb_ctx *cdb, } } else { ret = ldb_add(cdb->ldb, ldif->msg); - if (ret != LDB_SUCCESS && replace_whole_db == false) { - /* This section already existed, remove and re-add it. We - * really want to replace the whole thing instead of messing - * around with changetypes and flags on individual elements - */ - ret = ldb_delete(cdb->ldb, ldif->msg->dn); - if (ret == LDB_SUCCESS) { - ret = ldb_add(cdb->ldb, ldif->msg); - } - } } if (ret != LDB_SUCCESS) { DEBUG(SSSDBG_FATAL_FAILURE, - "Failed to initialize DB (%d,[%s]), aborting!\n", + "Failed to update DB (%d,[%s]), aborting!\n", ret, ldb_errstring(cdb->ldb)); return EIO; } @@ -215,19 +219,14 @@ static int confdb_init_db(const char *config_file, } in_transaction = true; - /* Purge existing database, if we are reinitializing the confdb completely */ - if (only_section == NULL) { - ret = confdb_purge(cdb); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Could not purge existing configuration\n"); - goto done; - } + ret = confdb_purge(cdb); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Could not purge existing configuration\n"); + goto done; } - ret = confdb_write_ldif(cdb, - config_ldif, - only_section == NULL ? true : false); + ret = confdb_write_ldif(cdb, config_ldif); if (ret != EOK) { goto done; } diff --git a/src/confdb/confdb_setup.h b/src/confdb/confdb_setup.h index d816c7ea0a7..c2186f753d2 100644 --- a/src/confdb/confdb_setup.h +++ b/src/confdb/confdb_setup.h @@ -27,21 +27,6 @@ #include "util/util_errors.h" -#define CONFDB_BASE_LDIF \ - "dn: @ATTRIBUTES\n" \ - "cn: CASE_INSENSITIVE\n" \ - "dc: CASE_INSENSITIVE\n" \ - "dn: CASE_INSENSITIVE\n" \ - "name: CASE_INSENSITIVE\n" \ - "objectclass: CASE_INSENSITIVE\n" \ - "\n" \ - "dn: @INDEXLIST\n" \ - "@IDXATTR: cn\n" \ - "\n" \ - "dn: @MODULES\n" \ - "@LIST: server_sort\n" \ - "\n" - struct confdb_ctx; errno_t confdb_setup(TALLOC_CTX *mem_ctx, diff --git a/src/man/sssd.8.xml b/src/man/sssd.8.xml index 7b992f7da50..8647277be2a 100644 --- a/src/man/sssd.8.xml +++ b/src/man/sssd.8.xml @@ -145,33 +145,6 @@ - - - , - - - - Do not start the SSSD, but refresh the configuration - database from the contents of - /etc/sssd/sssd.conf and exit. - - - - - - , - - - - Similar to --genconf, but only refresh - a single section from the configuration file. This - option is useful mainly to be called from systemd - unit files to allow socket-activated responders - to refresh their configuration without requiring - the administrator to restart the whole SSSD. - - - diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 74c665cffbd..2d08ae201ff 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1457,7 +1457,6 @@ static int monitor_ctx_destructor(void *mem) errno_t load_configuration(TALLOC_CTX *mem_ctx, const char *config_file, const char *config_dir, - const char *only_section, struct mt_ctx **monitor) { errno_t ret; @@ -1481,21 +1480,15 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx, goto done; } - ret = confdb_setup(ctx, cdb_file, config_file, config_dir, only_section, - false, &ctx->cdb); + + ret = confdb_setup(ctx, cdb_file, config_file, config_dir, NULL, false, + &ctx->cdb); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Unable to setup ConfDB [%d]: %s\n", ret, sss_strerror(ret)); goto done; } - /* return EOK for genconf-section to exit 0 when no - * sssd configuration exists (KCM use case) */ - if (only_section != NULL) { - *monitor = NULL; - goto done; - } - /* Validate the configuration in the database */ /* Read in the monitor's configuration */ ret = get_monitor_config(ctx); @@ -1521,7 +1514,7 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx, done: talloc_free(cdb_file); - if (ret != EOK || only_section != NULL) { + if (ret != EOK) { talloc_free(ctx); } return ret; @@ -1982,12 +1975,10 @@ int main(int argc, const char *argv[]) poptContext pc; int opt_daemon = 0; int opt_interactive = 0; - int opt_genconf = 0; int opt_version = 0; char *opt_config_file = NULL; const char *opt_logger = NULL; char *config_file = NULL; - char *opt_genconf_section = NULL; int flags = FLAGS_NO_WATCHDOG; struct main_context *main_ctx; TALLOC_CTX *tmp_ctx; @@ -2009,10 +2000,6 @@ int main(int argc, const char *argv[]) _("Become a daemon (default)"), NULL }, {"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, _("Run interactive (not a daemon)"), NULL}, - {"genconf", 'g', POPT_ARG_NONE, &opt_genconf, 0, - _("Refresh the configuration database, then exit"), NULL}, - {"genconf-section", 's', POPT_ARG_STRING, &opt_genconf_section, 0, - _("Similar to --genconf, but only refreshes the given section"), NULL}, {"version", '\0', POPT_ARG_NONE, &opt_version, 0, _("Print version number and exit"), NULL }, POPT_TABLEEND @@ -2044,28 +2031,13 @@ int main(int argc, const char *argv[]) cmdline_debug_timestamps = debug_timestamps; cmdline_debug_microseconds = debug_microseconds; - if (opt_genconf_section) { - /* --genconf-section implies genconf, just limited to a single section */ - opt_genconf = 1; - } - if (opt_genconf && (opt_daemon || opt_interactive)) { - ERROR("Option -g is incompatible with -D or -i\n"); - poptPrintUsage(pc, stderr, 0); - return 1; - } - if (opt_genconf) { - if (!opt_logger) { - opt_logger = sss_logger_str[STDERR_LOGGER]; - } - } - if (opt_daemon && opt_interactive) { ERROR("Option -i|--interactive is not allowed together with -D|--daemon\n"); poptPrintUsage(pc, stderr, 0); return 1; } - if (!opt_daemon && !opt_interactive && !opt_genconf) { + if (!opt_daemon && !opt_interactive) { opt_daemon = 1; } if (opt_daemon) { @@ -2129,58 +2101,53 @@ int main(int argc, const char *argv[]) } #endif - /* Check if the SSSD is already running and for nscd conflicts unless we're - * only interested in re-reading the configuration - */ - if (opt_genconf == 0) { - ret = check_file(SSSD_PIDFILE, 0, 0, S_IFREG|0600, 0, NULL, false); - if (ret == EOK) { - ret = check_pidfile(SSSD_PIDFILE); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "pidfile exists at %s\n", SSSD_PIDFILE); - ERROR("SSSD is already running\n"); - return 5; - } + /* Check if the SSSD is already running and for nscd conflicts */ + ret = check_file(SSSD_PIDFILE, 0, 0, S_IFREG|0600, 0, NULL, false); + if (ret == EOK) { + ret = check_pidfile(SSSD_PIDFILE); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "pidfile exists at %s\n", SSSD_PIDFILE); + ERROR("SSSD is already running\n"); + return 5; } + } - /* Warn if nscd seems to be running */ - ret = check_file(NSCD_SOCKET_PATH, - -1, -1, S_IFSOCK, S_IFMT, NULL, false); - if (ret == EOK) { - ret = sss_nscd_parse_conf(NSCD_CONF_PATH); - - switch (ret) { - case ENOENT: - sss_log(SSS_LOG_NOTICE, - "NSCD socket was detected. NSCD caching capabilities " - "may conflict with SSSD for users and groups. It is " - "recommended not to run NSCD in parallel with SSSD, " - "unless NSCD is configured not to cache the passwd, " - "group, netgroup and services nsswitch maps."); - break; - - case EEXIST: - sss_log(SSS_LOG_NOTICE, - "NSCD socket was detected and seems to be configured " - "to cache some of the databases controlled by " - "SSSD [passwd,group,netgroup,services]. It is " - "recommended not to run NSCD in parallel with SSSD, " - "unless NSCD is configured not to cache these."); - break; - - case EOK: - DEBUG(SSSDBG_TRACE_FUNC, "NSCD socket was detected and it " - "seems to be configured not to interfere with " - "SSSD's caching capabilities\n"); - } - } + /* Warn if nscd seems to be running */ + ret = check_file(NSCD_SOCKET_PATH, + -1, -1, S_IFSOCK, S_IFMT, NULL, false); + if (ret == EOK) { + ret = sss_nscd_parse_conf(NSCD_CONF_PATH); + + switch (ret) { + case ENOENT: + sss_log(SSS_LOG_NOTICE, + "NSCD socket was detected. NSCD caching capabilities " + "may conflict with SSSD for users and groups. It is " + "recommended not to run NSCD in parallel with SSSD, " + "unless NSCD is configured not to cache the passwd, " + "group, netgroup and services nsswitch maps."); + break; + case EEXIST: + sss_log(SSS_LOG_NOTICE, + "NSCD socket was detected and seems to be configured " + "to cache some of the databases controlled by " + "SSSD [passwd,group,netgroup,services]. It is " + "recommended not to run NSCD in parallel with SSSD, " + "unless NSCD is configured not to cache these."); + break; + + case EOK: + DEBUG(SSSDBG_TRACE_FUNC, "NSCD socket was detected and it " + "seems to be configured not to interfere with " + "SSSD's caching capabilities\n"); + } } /* Parse config file, fail if cannot be done */ ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, - opt_genconf_section, &monitor); + &monitor); if (ret != EOK) { switch (ret) { case EPERM: @@ -2201,10 +2168,6 @@ int main(int argc, const char *argv[]) return 5; } - /* at this point we are done generating the config file, we may exit - * if that's all we were asked to do */ - if (opt_genconf) return 0; - /* set up things like debug, signals, daemonization, etc. */ monitor->conf_path = CONFDB_MONITOR_CONF_ENTRY; ret = close(STDIN_FILENO); diff --git a/src/tests/multihost/basic/test_config.py b/src/tests/multihost/basic/test_config.py deleted file mode 100644 index 8d4847b9e5f..00000000000 --- a/src/tests/multihost/basic/test_config.py +++ /dev/null @@ -1,114 +0,0 @@ -""" SSSD Configuration-related Test Cases - -:requirement: IDM-SSSD-REQ: Configuration merging -:casecomponent: sssd -:subsystemteam: sst_idm_sssd -:upstream: yes -:status: approved -""" - -import pytest -from utils_config import remove_section, set_param - - -class TestSSSDConfig(object): - """ - Test cases around SSSD config management - """ - def _assert_config_value(self, multihost, section, key, value): - # This would really be much, much nicer to implement using python-ldb - # but at the moment, the multihost tests rely on a virtual environment - # where everything is pip-installed..and python-ldb is not present in - # pip - confdb_dn = 'cn=%s,cn=config' % (section) - ldb_cmd = 'ldbsearch -H /var/lib/sss/db/config.ldb -b %s' % (confdb_dn) - cmd = multihost.master[0].run_command(ldb_cmd) - check_str = '%s: %s' % (key, value) - assert check_str in cmd.stdout_text - - @pytest.mark.converted('test_config.py', 'test_config__change_config_while_sssd_running') - def test_sssd_genconf_sssd_running(self, multihost): - """ - :title: config: sssd --genconf is able to re-generate - the configuration even while SSSD is running - :id: 078721e9-536b-4fd8-a36d-bd94673228fc - """ - multihost.master[0].service_sssd('restart') - - self._assert_config_value(multihost, 'pam', 'debug_level', '9') - - set_param(multihost, 'pam', 'debug_level', '1') - multihost.master[0].run_command('/usr/sbin/sssd --genconf') - self._assert_config_value(multihost, 'pam', 'debug_level', '1') - - set_param(multihost, 'pam', 'debug_level', '9') - - @pytest.mark.converted('test_config.py', 'test_config__genconf_particular_section') - def test_sssd_genconf_section_only(self, multihost): - """ - :title: config: sssd --genconf-section only - refreshes those sections given on the command line - :id: 011bf2ad-4a2a-4350-adfa-7826349e262f - """ - multihost.master[0].service_sssd('restart') - - self._assert_config_value(multihost, 'pam', 'debug_level', '9') - self._assert_config_value(multihost, 'nss', 'debug_level', '9') - - set_param(multihost, 'pam', 'debug_level', '1') - set_param(multihost, 'nss', 'debug_level', '1') - multihost.master[0].run_command( - '/usr/sbin/sssd --genconf-section=pam') - - # We only told genconf to touch the pam section.. - self._assert_config_value(multihost, 'pam', 'debug_level', '1') - # ..so the NSS section shouldn't be updated at all - self._assert_config_value(multihost, 'nss', 'debug_level', '9') - - set_param(multihost, 'nss', 'debug_level', '9') - set_param(multihost, 'pam', 'debug_level', '9') - - @pytest.mark.converted('test_config.py', 'test_config__add_remove_section') - def test_sssd_genconf_add_remove_section(self, multihost): - """ - :title: config: sssd --genconf-section can not only modify - existing configuration sections, but also add a new section - :id: 8df66b51-aadc-456e-8f27-a1a787e61769 - """ - # Establish a baseline - multihost.master[0].service_sssd('restart') - self._assert_config_value(multihost, 'pam', 'debug_level', '9') - self._assert_config_value(multihost, 'nss', 'debug_level', '9') - - set_param(multihost, 'foo', 'bar', 'baz') - - multihost.master[0].run_command( - '/usr/sbin/sssd --genconf-section=foo') - - ldb_cmd = 'ldbsearch -H /var/lib/sss/db/config.ldb -b cn=foo,cn=config' - cmd = multihost.master[0].run_command(ldb_cmd) - assert 'bar: baz' in cmd.stdout_text - - remove_section(multihost, 'foo') - multihost.master[0].run_command( - '/usr/sbin/sssd --genconf-section=foo') - - ldb_cmd = 'ldbsearch -H /var/lib/sss/db/config.ldb -b cn=foo,cn=config' - cmd = multihost.master[0].run_command(ldb_cmd) - assert 'foo' not in cmd.stdout_text - # Also make sure the existing sections were intact - self._assert_config_value(multihost, 'pam', 'debug_level', '9') - self._assert_config_value(multihost, 'nss', 'debug_level', '9') - - @pytest.mark.converted('test_config.py', 'test_config__genconf_no_such_section') - def test_sssd_genconf_no_such_section(self, multihost): - """ - :title: config: Referencing a non-existant section must not fail - :id: 4e160dcc-9789-4f3f-b8d4-c67d27ef4a1c - :description: Referencing a non-existant section must not fail, - because we want to call this command from the systemd unit files - and by default the sections don't have to be present - """ - multihost.master[0].service_sssd('restart') - multihost.master[0].run_command( - '/usr/sbin/sssd --genconf-section=xyz') diff --git a/src/tests/system/tests/test_config.py b/src/tests/system/tests/test_config.py deleted file mode 100644 index b4a522a05ee..00000000000 --- a/src/tests/system/tests/test_config.py +++ /dev/null @@ -1,172 +0,0 @@ -""" -SSSD Configuration-related Test Cases - -:requirement: IDM-SSSD-REQ: Configuration merging -""" - -from __future__ import annotations - -import pytest -from sssd_test_framework.roles.client import Client -from sssd_test_framework.topology import KnownTopologyGroup - - -@pytest.mark.importance("critical") -@pytest.mark.topology(KnownTopologyGroup.AnyProvider) -def test_config__change_config_while_sssd_running(client: Client): - """ - :title: Re-generate config while SSSD is running - :setup: - 1. In pam domain set "debug_level" to 9 - 2. Start SSSD - :steps: - 1. Check that "debug_level" in pam domain is 9 - 2. Change "debug_level" in pam to 1 - 3. Apply config changes - 4. Call "sssd --genconf" - 5. Check that "debug_level" in pam is 1 - :expectedresults: - 1. "debug_level" is set to 9 - 2. "debug_level" is changed successfully - 3. Changes are apllied successfully - 4. "sssd --genconf" is called successfully - 5. "debug_level" is set to 1 - :customerscenario: False - """ - client.sssd.pam["debug_level"] = "9" - client.sssd.start() - - result = client.ldb.search("/var/lib/sss/db/config.ldb", "cn=pam,cn=config") - assert result["cn=pam,cn=config"]["debug_level"] == ["9"] - - client.sssd.pam["debug_level"] = "1" - client.sssd.config_apply() - client.sssd.genconf() - - result = client.ldb.search("/var/lib/sss/db/config.ldb", "cn=pam,cn=config") - assert result["cn=pam,cn=config"]["debug_level"] == ["1"] - - -@pytest.mark.importance("critical") -@pytest.mark.config -@pytest.mark.topology(KnownTopologyGroup.AnyProvider) -def test_config__genconf_particular_section(client: Client): - """ - :title: Re-generate only particular section in config while SSSD is running - :setup: - 1. In pam domain set "debug_level" to 9 - 2. In nss domain set "debug_level" to 9 - 3. Start SSSD - :steps: - 1. Check that "debug_level" in pam domain is 9 - 2. Check that "debug_level" in nss domain is 9 - 3. Change "debug_level" in pam and in nss to 1 - 4. Apply config changes - 5. Call "sssd --genconf-section==pam" - 6. Check that "debug_level" in pam is 1 - 7. Check that "debug_level" in nss remained 9 - :expectedresults: - 1. "debug_level" is set to 9 - 2. "debug_level" is set to 9 - 3. "debug_level" is changed successfully - 4. Changes are apllied successfully - 5. "sssd --genconf-section==pam" is called successfully - 6. "debug_level" in pam is 1 - 7. "debug_level" in nss remains 9 - :customerscenario: False - """ - client.sssd.pam["debug_level"] = "9" - client.sssd.nss["debug_level"] = "9" - client.sssd.start() - - result = client.ldb.search("/var/lib/sss/db/config.ldb") - assert result["cn=pam,cn=config"]["debug_level"] == ["9"] - assert result["cn=nss,cn=config"]["debug_level"] == ["9"] - - client.sssd.pam["debug_level"] = "1" - client.sssd.nss["debug_level"] = "1" - client.sssd.config_apply() - - client.sssd.genconf("pam") - - result = client.ldb.search("/var/lib/sss/db/config.ldb") - assert result["cn=pam,cn=config"]["debug_level"] == ["1"] - assert result["cn=nss,cn=config"]["debug_level"] == ["9"] - - -@pytest.mark.importance("critical") -@pytest.mark.topology(KnownTopologyGroup.AnyProvider) -def test_config__add_remove_section(client: Client): - """ - :title: Add and remove new section to config file - with --genconf-section while SSSD is running - :setup: - 1. In pam domain set "debug_level" to 9 - 2. In nss domain set "debug_level" to 9 - 3. Start SSSD - :steps: - 1. Check that "debug_level" in pam and nss is 9 - 2. Add new section to config with key, value pair set - 3. Apply config changes - 4. Call "sssd --genconf-section==$newSection" - 5. Check that the new section is properly set - 6. Remove new section - 7. Call "sssd --genconf-section==$newSection" - 8. Check that the new section was deleted - 9. Check that "debug_level" in pam and nss is 9 - :expectedresults: - 1. "debug_level" is set to 9 in both domains - 2. Added successfully - 3. New configuration was written - 4. Changes are applied successfully - 5. "sssd --genconf-section==$newSection" is called successfully - 6. New section is removed successfully - 7. "sssd --genconf-section==$newSection" is called successfully - 8. New section was deleted correctly - 9. "debug_level" in pam and nss remained 9 - :customerscenario: False - """ - client.sssd.pam["debug_level"] = "9" - client.sssd.nss["debug_level"] = "9" - client.sssd.start() - - result = client.ldb.search("/var/lib/sss/db/config.ldb") - assert result["cn=pam,cn=config"]["debug_level"] == ["9"] - assert result["cn=nss,cn=config"]["debug_level"] == ["9"] - - client.sssd.config["new_section"] = {"key": "value"} - client.sssd.config_apply(check_config=False) - client.sssd.genconf("new_section") - - result = client.ldb.search("/var/lib/sss/db/config.ldb", "cn=new_section,cn=config") - assert result["cn=new_section,cn=config"]["key"] == ["value"] - - del client.sssd.config["new_section"] - - client.sssd.config_apply() - client.sssd.genconf("new_section") - - result = client.ldb.search("/var/lib/sss/db/config.ldb") - assert result["cn=pam,cn=config"]["debug_level"] == ["9"] - assert result["cn=nss,cn=config"]["debug_level"] == ["9"] - with pytest.raises(KeyError): - assert result["cn=new_section,cn=config"]["key"] != ["value"] - - -@pytest.mark.importance("critical") -@pytest.mark.topology(KnownTopologyGroup.AnyProvider) -def test_config__genconf_no_such_section(client: Client): - """ - :title: genconf-section with nonexisting section did not fail - :setup: - 1. Start SSSD - :steps: - 1. Call 'sssd --genconf-section=$nonexistingSection' - :expectedresults: - 1. Call did not fail - :customerscenario: False - """ - client.sssd.start() - result = client.sssd.genconf("nonexistingSection") - assert result.rc == 0 - assert not result.stderr diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 6290da8ce9b..450d8150d7a 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -424,14 +424,6 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, size_t ldif_len = 0; size_t attr_len; struct value_obj *obj = NULL; - bool section_handled = true; - - if (only_section != NULL) { - /* If the section is specified, we must handle it, either by adding - * its contents or by deleting the section if it doesn't exist - */ - section_handled = false; - } tmp_ctx = talloc_new(mem_ctx); if (!tmp_ctx) { @@ -460,11 +452,6 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, if (strcasecmp(only_section, sections[i])) { DEBUG(SSSDBG_TRACE_FUNC, "Skipping section %s\n", sections[i]); continue; - } else { - /* Mark the requested section as handled so that we don't - * try to re-add it later - */ - section_handled = true; } } @@ -554,39 +541,6 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, talloc_free(dn); } - - if (only_section != NULL && section_handled == false) { - /* If only a single section was supposed to be - * handled, but it wasn't found in the INI file, - * create an LDIF that would remove the section - */ - ret = parse_section(tmp_ctx, only_section, &sec_dn, NULL); - if (ret != EOK) { - goto error; - } - - dn = talloc_asprintf(tmp_ctx, - "dn: %s,cn=config\n" - "changetype: delete\n\n", - sec_dn); - if (dn == NULL) { - ret = ENOMEM; - goto error; - } - dn_size = strlen(dn); - - tmp_ldif = talloc_realloc(mem_ctx, ldif, char, - ldif_len+dn_size+1); - if (!tmp_ldif) { - ret = ENOMEM; - goto error; - } - - ldif = tmp_ldif; - memcpy(ldif+ldif_len, dn, dn_size); - ldif_len += dn_size; - } - if (ldif == NULL) { ret = ERR_INI_EMPTY_CONFIG; goto error; From b0777106df52f9c060c5c1cbbabaa7b07d618505 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 12 Oct 2023 19:36:32 +0200 Subject: [PATCH 12/42] SSS_INI: const correctness --- src/util/sss_ini.c | 2 +- src/util/sss_ini.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 450d8150d7a..4b8397f41db 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -404,7 +404,7 @@ const char *sss_ini_get_string_config_value(struct sss_ini *self, /* Create LDIF */ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, - struct sss_ini *self, + const struct sss_ini *self, const char *only_section, const char **config_ldif) { diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h index 3f05701ab76..97d2df3a1a3 100644 --- a/src/util/sss_ini.h +++ b/src/util/sss_ini.h @@ -143,7 +143,7 @@ const char *sss_ini_get_string_config_value(struct sss_ini *self, * @brief Create LDIF */ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, - struct sss_ini *self, + const struct sss_ini *self, const char *only_section, const char **config_ldif); From 456e8620f5368c64fea74f46e527ab8b5e5cfa6f Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 14 Sep 2023 13:54:53 +0200 Subject: [PATCH 13/42] CONFDB: split confdb_setup() into 2 steps It will be used by 'monitor' to first read 'sssd.conf' then switch uid/gid before writing 'config.ldb' This is required in case sssd.service::User and sssd.conf::user do not match. --- src/confdb/confdb_setup.c | 126 ++++++++++++++++++++------------------ 1 file changed, 65 insertions(+), 61 deletions(-) diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index ca75a0dcbd8..a0123b62e1b 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -44,6 +44,33 @@ "\n" +errno_t confdb_read_ini(TALLOC_CTX *mem_ctx, + const char *config_file, + const char *config_dir, + bool allow_missing_file, + struct sss_ini **_ini) +{ + int ret; + + *_ini = sss_ini_new(mem_ctx); + if (*_ini == NULL) { + return ENOMEM; + } + + ret = sss_ini_read_sssd_conf(*_ini, config_file, config_dir); + if (ret != EOK) { + if ((ret == ERR_INI_EMPTY_CONFIG) && allow_missing_file) { + return EOK; + } + talloc_zfree(*_ini); + return ret; + } + + sss_ini_call_validators(*_ini, SSSDDATADIR"/cfg_rules.ini"); + + return EOK; +} + static int confdb_purge(struct confdb_ctx *cdb) { int ret; @@ -100,38 +127,6 @@ static int confdb_create_base(struct confdb_ctx *cdb) return EOK; } -static int confdb_ldif_from_ini_file(TALLOC_CTX *mem_ctx, - const char *config_file, - const char *config_dir, - const char *only_section, - struct sss_ini *init_data, - const char **_ldif) -{ - errno_t ret; - - ret = sss_ini_read_sssd_conf(init_data, - config_file, - config_dir); - if (ret != EOK) { - return ret; - } - - ret = sss_ini_call_validators(init_data, - SSSDDATADIR"/cfg_rules.ini"); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "Failed to call validators\n"); - /* This is not fatal, continue */ - } - - ret = sss_confdb_create_ldif(mem_ctx, init_data, only_section, _ldif); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "Could not create LDIF for confdb\n"); - return ret; - } - - return EOK; -} - static int confdb_write_ldif(struct confdb_ctx *cdb, const char *config_ldif) { int ret; @@ -161,18 +156,16 @@ static int confdb_write_ldif(struct confdb_ctx *cdb, const char *config_ldif) return EOK; } -static int confdb_init_db(const char *config_file, - const char *config_dir, - const char *only_section, - struct confdb_ctx *cdb, - bool allow_missing_file) +static int confdb_populate(const struct sss_ini *ini, + const char *only_section, + struct confdb_ctx *cdb, + bool allow_missing_content) { TALLOC_CTX *tmp_ctx; int ret; int sret = EOK; bool in_transaction = false; const char *config_ldif; - struct sss_ini *init_data; tmp_ctx = talloc_new(cdb); if (tmp_ctx == NULL) { @@ -180,21 +173,9 @@ static int confdb_init_db(const char *config_file, return ENOMEM; } - init_data = sss_ini_new(tmp_ctx); - if (!init_data) { - DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory.\n"); - ret = ENOMEM; - goto done; - } - - ret = confdb_ldif_from_ini_file(tmp_ctx, - config_file, - config_dir, - only_section, - init_data, - &config_ldif); + ret = sss_confdb_create_ldif(tmp_ctx, ini, only_section, &config_ldif); if (ret != EOK) { - if (ret == ERR_INI_EMPTY_CONFIG && allow_missing_file) { + if ((ret == ERR_INI_EMPTY_CONFIG) && allow_missing_content) { DEBUG(SSSDBG_TRACE_FUNC, "Empty configuration. Using the defaults.\n"); ret = EOK; goto done; @@ -252,13 +233,12 @@ static int confdb_init_db(const char *config_file, return ret; } -errno_t confdb_setup(TALLOC_CTX *mem_ctx, - const char *cdb_file, - const char *config_file, - const char *config_dir, - const char *only_section, - bool allow_missing_file, - struct confdb_ctx **_cdb) +errno_t confdb_write_ini(TALLOC_CTX *mem_ctx, + const struct sss_ini *ini, + const char *cdb_file, + const char *only_section, + bool allow_missing_content, + struct confdb_ctx **_cdb) { TALLOC_CTX *tmp_ctx; struct stat statbuf; @@ -302,8 +282,7 @@ errno_t confdb_setup(TALLOC_CTX *mem_ctx, } /* Initialize the CDB from the configuration file */ - ret = confdb_init_db(config_file, config_dir, only_section, cdb, - allow_missing_file); + ret = confdb_populate(ini, only_section, cdb, allow_missing_content); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "ConfDB initialization has failed " "[%d]: %s\n", ret, sss_strerror(ret)); @@ -318,3 +297,28 @@ errno_t confdb_setup(TALLOC_CTX *mem_ctx, talloc_free(tmp_ctx); return ret; } + +errno_t confdb_setup(TALLOC_CTX *mem_ctx, + const char *cdb_file, + const char *config_file, + const char *config_dir, + const char *only_section, + bool allow_missing_file, + struct confdb_ctx **_cdb) +{ + int ret; + struct sss_ini *ini; + + ret = confdb_read_ini(mem_ctx, config_file, config_dir, allow_missing_file, + &ini); + if (ret != EOK) { + return ret; + } + + ret = confdb_write_ini(mem_ctx, ini, cdb_file, only_section, allow_missing_file, + _cdb); + + talloc_free(ini); + + return ret; +} From da32bdac186bb085dd3f98a290fa3f35bb11b027 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 11:23:20 +0200 Subject: [PATCH 14/42] CONFDB: always delete old ldb-file --- src/confdb/confdb_setup.c | 73 ++++++--------------------------------- 1 file changed, 11 insertions(+), 62 deletions(-) diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index a0123b62e1b..47450c462c8 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -71,41 +71,6 @@ errno_t confdb_read_ini(TALLOC_CTX *mem_ctx, return EOK; } -static int confdb_purge(struct confdb_ctx *cdb) -{ - int ret; - unsigned int i; - TALLOC_CTX *tmp_ctx; - struct ldb_result *res; - struct ldb_dn *dn; - const char *attrs[] = { "dn", NULL }; - - tmp_ctx = talloc_new(NULL); - - dn = ldb_dn_new(tmp_ctx, cdb->ldb, "cn=config"); - - /* Get the list of all DNs */ - ret = ldb_search(cdb->ldb, tmp_ctx, &res, dn, - LDB_SCOPE_SUBTREE, attrs, NULL); - if (ret != LDB_SUCCESS) { - ret = sss_ldb_error_to_errno(ret); - goto done; - } - - for(i=0; icount; i++) { - /* Delete this DN */ - ret = ldb_delete(cdb->ldb, res->msgs[i]->dn); - if (ret != LDB_SUCCESS) { - ret = sss_ldb_error_to_errno(ret); - goto done; - } - } - -done: - talloc_free(tmp_ctx); - return ret; -} - static int confdb_create_base(struct confdb_ctx *cdb) { int ret; @@ -200,13 +165,6 @@ static int confdb_populate(const struct sss_ini *ini, } in_transaction = true; - ret = confdb_purge(cdb); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Could not purge existing configuration\n"); - goto done; - } - ret = confdb_write_ldif(cdb, config_ldif); if (ret != EOK) { goto done; @@ -241,9 +199,7 @@ errno_t confdb_write_ini(TALLOC_CTX *mem_ctx, struct confdb_ctx **_cdb) { TALLOC_CTX *tmp_ctx; - struct stat statbuf; struct confdb_ctx *cdb; - bool missing_cdb = false; errno_t ret; tmp_ctx = talloc_new(NULL); @@ -252,16 +208,11 @@ errno_t confdb_write_ini(TALLOC_CTX *mem_ctx, return ENOMEM; } - ret = stat(cdb_file, &statbuf); - if (ret == -1) { - if (errno == ENOENT) { - missing_cdb = true; - } else { - ret = errno; - goto done; - } - } else if (statbuf.st_size == 0) { - missing_cdb = true; + ret = unlink(cdb_file); + if ((ret == -1) && (errno != ENOENT)) { + ret = errno; + DEBUG(SSSDBG_FATAL_FAILURE, "Can't delete old '%s'\n", cdb_file); + goto done; } ret = confdb_init(tmp_ctx, &cdb, cdb_file); @@ -271,14 +222,12 @@ errno_t confdb_write_ini(TALLOC_CTX *mem_ctx, goto done; } - if (missing_cdb) { - /* Load special entries */ - ret = confdb_create_base(cdb); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Unable to load special entries into confdb\n"); - goto done; - } + /* Load special entries */ + ret = confdb_create_base(cdb); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Unable to load special entries into confdb\n"); + goto done; } /* Initialize the CDB from the configuration file */ From 1d453dc9a419e976277e0f9020b78dd0a7484163 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 12:42:48 +0200 Subject: [PATCH 15/42] MONITOR: no need to read domain list twice It's already read in `get_monitor_config()` --- src/monitor/monitor.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 2d08ae201ff..b56850f00ce 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -2185,12 +2185,6 @@ int main(int argc, const char *argv[]) talloc_zfree(monitor->cdb); monitor->cdb = main_ctx->confdb_ctx; - ret = confdb_get_domains(monitor->cdb, &monitor->domains); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "No domains configured.\n"); - return 1; - } - monitor->is_daemon = !opt_interactive; monitor->parent_pid = main_ctx->parent_pid; monitor->ev = main_ctx->event_ctx; From a94e7fe4242cbe251305cc2fa55c550037bf3af1 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 13:25:27 +0200 Subject: [PATCH 16/42] MONITOR: remove unused mt_ctx::conf_path --- src/monitor/monitor.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index b56850f00ce..0598b72b19d 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -120,7 +120,6 @@ struct mt_ctx { bool check_children; bool services_started; struct netlink_ctx *nlctx; - const char *conf_path; struct sss_sigchild_ctx *sigchld_ctx; bool pid_file_created; bool is_daemon; @@ -2169,12 +2168,11 @@ int main(int argc, const char *argv[]) } /* set up things like debug, signals, daemonization, etc. */ - monitor->conf_path = CONFDB_MONITOR_CONF_ENTRY; ret = close(STDIN_FILENO); if (ret != EOK) return 5; ret = server_setup(SSSD_MONITOR_NAME, false, flags, CONFDB_FILE, - monitor->conf_path, &main_ctx, false); + CONFDB_MONITOR_CONF_ENTRY, &main_ctx, false); if (ret != EOK) return 5; /* Use confd initialized in server_setup. ldb_tdb module (1.4.0) check PID From 12c6e31971c447d53355757a403a7873f9c64dc7 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:00:16 +0200 Subject: [PATCH 17/42] MONITOR: move keyring setup code to a function --- src/monitor/monitor.c | 30 +++--------------------------- src/monitor/monitor_bootstrap.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 0598b72b19d..2e497cc79aa 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -42,10 +42,6 @@ #include "monitor/monitor.h" #include "sss_iface/sss_iface_async.h" -#ifdef USE_KEYRING -#include -#endif - #ifdef HAVE_SYSTEMD #include #endif @@ -1967,6 +1963,7 @@ static void monitor_restart_service(struct mt_svc *svc) } int bootstrap_monitor_process(void); +void setup_keyring(void); int main(int argc, const char *argv[]) { @@ -2077,30 +2074,9 @@ int main(int argc, const char *argv[]) /* default value of 'debug_prg_name' will be used */ DEBUG_INIT(debug_level, opt_logger); -#ifdef USE_KEYRING - /* Do this before all the forks, it sets the session key ring so all - * keys are private to the daemon and cannot be read by any other process - * tree */ - - /* make a new session */ - ret = keyctl_join_session_keyring(NULL); - if (ret == -1) { - sss_log(SSS_LOG_ALERT, - "Could not create private keyring session. " - "If you store password there they may be easily accessible " - "to the root user. (%d, %s)", errno, strerror(errno)); - } - - ret = keyctl_setperm(KEY_SPEC_SESSION_KEYRING, KEY_POS_ALL); - if (ret == -1) { - sss_log(SSS_LOG_ALERT, - "Could not set permissions on private keyring. " - "If you store password there they may be easily accessible " - "to the root user. (%d, %s)", errno, strerror(errno)); - } -#endif + setup_keyring(); - /* Check if the SSSD is already running and for nscd conflicts */ + /* Check if the SSSD is already running */ ret = check_file(SSSD_PIDFILE, 0, 0, S_IFREG|0600, 0, NULL, false); if (ret == EOK) { ret = check_pidfile(SSSD_PIDFILE); diff --git a/src/monitor/monitor_bootstrap.c b/src/monitor/monitor_bootstrap.c index 2ee33f079c8..d85483aa9d3 100644 --- a/src/monitor/monitor_bootstrap.c +++ b/src/monitor/monitor_bootstrap.c @@ -23,6 +23,9 @@ #include #include #include +#ifdef USE_KEYRING +#include +#endif #include "util/util.h" @@ -120,3 +123,31 @@ int bootstrap_monitor_process(void) return 0; } + +void setup_keyring(void) +{ +#ifdef USE_KEYRING + int ret; + + /* Do this before all the forks, it sets the session key ring so all + * keys are private to the daemon and cannot be read by any other process + * tree */ + + /* make a new session */ + ret = keyctl_join_session_keyring(NULL); + if (ret == -1) { + sss_log(SSS_LOG_ALERT, + "Could not create private keyring session. " + "If you store password there they may be easily accessible " + "to the root user. (%d, %s)", errno, strerror(errno)); + } + + ret = keyctl_setperm(KEY_SPEC_SESSION_KEYRING, KEY_POS_ALL); + if (ret == -1) { + sss_log(SSS_LOG_ALERT, + "Could not set permissions on private keyring. " + "If you store password there they may be easily accessible " + "to the root user. (%d, %s)", errno, strerror(errno)); + } +#endif +} From 0b1fcd8eefdb37afbcf9a9cc4a2b7e6fe76e893b Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:16:48 +0200 Subject: [PATCH 18/42] MONITOR: move nscd check code to a function --- src/monitor/monitor.c | 67 +++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 2e497cc79aa..a1555b4b8cd 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1962,6 +1962,41 @@ static void monitor_restart_service(struct mt_svc *svc) } } +static void check_nscd(void) +{ + int ret; + ret = check_file(NSCD_SOCKET_PATH, + -1, -1, S_IFSOCK, S_IFMT, NULL, false); + if (ret == EOK) { + ret = sss_nscd_parse_conf(NSCD_CONF_PATH); + + switch (ret) { + case ENOENT: + sss_log(SSS_LOG_NOTICE, + "NSCD socket was detected. NSCD caching capabilities " + "may conflict with SSSD for users and groups. It is " + "recommended not to run NSCD in parallel with SSSD, " + "unless NSCD is configured not to cache the passwd, " + "group, netgroup and services nsswitch maps."); + break; + + case EEXIST: + sss_log(SSS_LOG_NOTICE, + "NSCD socket was detected and seems to be configured " + "to cache some of the databases controlled by " + "SSSD [passwd,group,netgroup,services]. It is " + "recommended not to run NSCD in parallel with SSSD, " + "unless NSCD is configured not to cache these."); + break; + + case EOK: + DEBUG(SSSDBG_TRACE_FUNC, "NSCD socket was detected and it " + "seems to be configured not to interfere with " + "SSSD's caching capabilities\n"); + } + } +} + int bootstrap_monitor_process(void); void setup_keyring(void); @@ -2088,37 +2123,7 @@ int main(int argc, const char *argv[]) } } - /* Warn if nscd seems to be running */ - ret = check_file(NSCD_SOCKET_PATH, - -1, -1, S_IFSOCK, S_IFMT, NULL, false); - if (ret == EOK) { - ret = sss_nscd_parse_conf(NSCD_CONF_PATH); - - switch (ret) { - case ENOENT: - sss_log(SSS_LOG_NOTICE, - "NSCD socket was detected. NSCD caching capabilities " - "may conflict with SSSD for users and groups. It is " - "recommended not to run NSCD in parallel with SSSD, " - "unless NSCD is configured not to cache the passwd, " - "group, netgroup and services nsswitch maps."); - break; - - case EEXIST: - sss_log(SSS_LOG_NOTICE, - "NSCD socket was detected and seems to be configured " - "to cache some of the databases controlled by " - "SSSD [passwd,group,netgroup,services]. It is " - "recommended not to run NSCD in parallel with SSSD, " - "unless NSCD is configured not to cache these."); - break; - - case EOK: - DEBUG(SSSDBG_TRACE_FUNC, "NSCD socket was detected and it " - "seems to be configured not to interfere with " - "SSSD's caching capabilities\n"); - } - } + check_nscd(); /* Parse config file, fail if cannot be done */ ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, From fc76b705aa7b6229f4cfb4c5a3407f5f45fd8d86 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 18 Oct 2023 20:50:47 +0200 Subject: [PATCH 19/42] SSS_INI: remove 'const' specifier from getter `sss_ini_get_string_config_value()` is a wrapper around `ini_get_string_config_value()`, whose docs says ``` Returned value needs to be freed after use. ``` But an attempt to free 'const char *' results in discarded-qualifiers warning. --- src/tools/sssd_check_socket_activated_responders.c | 3 ++- src/util/sss_ini.c | 4 ++-- src/util/sss_ini.h | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/tools/sssd_check_socket_activated_responders.c b/src/tools/sssd_check_socket_activated_responders.c index f8ef1feab55..dddc02ee24e 100644 --- a/src/tools/sssd_check_socket_activated_responders.c +++ b/src/tools/sssd_check_socket_activated_responders.c @@ -30,7 +30,7 @@ static errno_t check_socket_activated_responder(const char *responder) { errno_t ret; - const char *services; + char *services = NULL; const char *str; TALLOC_CTX *tmp_ctx; struct sss_ini *init_data; @@ -90,6 +90,7 @@ static errno_t check_socket_activated_responder(const char *responder) ret = EOK; done: + free(services); talloc_free(tmp_ctx); return ret; diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 4b8397f41db..7936c6a8efc 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -395,8 +395,8 @@ int sss_ini_get_int_config_value(struct sss_ini *self, /* Get string value */ -const char *sss_ini_get_string_config_value(struct sss_ini *self, - int *error) +char *sss_ini_get_string_config_value(struct sss_ini *self, + int *error) { return ini_get_string_config_value(self->obj, error); } diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h index 97d2df3a1a3..280ea837de2 100644 --- a/src/util/sss_ini.h +++ b/src/util/sss_ini.h @@ -136,8 +136,8 @@ int sss_ini_get_int_config_value(struct sss_ini *self, /** * @brief Get string value */ -const char *sss_ini_get_string_config_value(struct sss_ini *self, - int *error); +char *sss_ini_get_string_config_value(struct sss_ini *self, + int *error); /** * @brief Create LDIF From 512b93759db7381018aee44aa1c530075edfc472 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 21 Oct 2023 22:54:11 +0200 Subject: [PATCH 20/42] DEBUG: a couple of message changes Following changes were done: - perform_checks(): log actual owner - sss_confdb_create_ldif(): use SSSDBG_TRACE_LDB --- src/util/check_file.c | 10 ++++++---- src/util/sss_ini.c | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/util/check_file.c b/src/util/check_file.c index 2203a41c328..cb0e44dc4a5 100644 --- a/src/util/check_file.c +++ b/src/util/check_file.c @@ -90,14 +90,16 @@ static errno_t perform_checks(const char *filename, } if (uid != (uid_t)(-1) && stat_buf->st_uid != uid) { - DEBUG(SSSDBG_TRACE_LIBS, "File '%s' must be owned by uid [%d].\n", - filename, uid); + DEBUG(SSSDBG_TRACE_LIBS, + "File '%s' is owned by uid [%"SPRIuid"], expected [%"SPRIuid"].\n", + filename, stat_buf->st_uid, uid); return EINVAL; } if (gid != (gid_t)(-1) && stat_buf->st_gid != gid) { - DEBUG(SSSDBG_TRACE_LIBS, "File '%s' must be owned by gid [%d].\n", - filename, gid); + DEBUG(SSSDBG_TRACE_LIBS, + "File '%s' is owned by gid [%"SPRIgid"], expected [%"SPRIgid"].\n", + filename, stat_buf->st_gid, gid); return EINVAL; } diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 7936c6a8efc..3134544d7c0 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -441,7 +441,7 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, for (i = 0; i < section_count; i++) { const char *rdn = NULL; - DEBUG(SSSDBG_TRACE_FUNC, + DEBUG(SSSDBG_TRACE_LDB, "Processing config section [%s]\n", sections[i]); ret = parse_section(tmp_ctx, sections[i], &sec_dn, &rdn); if (ret != EOK) { @@ -450,7 +450,7 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, if (only_section != NULL) { if (strcasecmp(only_section, sections[i])) { - DEBUG(SSSDBG_TRACE_FUNC, "Skipping section %s\n", sections[i]); + DEBUG(SSSDBG_TRACE_LDB, "Skipping section %s\n", sections[i]); continue; } } @@ -475,7 +475,7 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, } for (j = 0; j < attr_count; j++) { - DEBUG(SSSDBG_TRACE_FUNC, + DEBUG(SSSDBG_TRACE_LDB, "Processing attribute [%s]\n", attrs[j]); ret = sss_ini_get_config_obj(sections[i], attrs[j], self->sssd_config, @@ -493,7 +493,7 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, ldif_attr = talloc_asprintf(tmp_ctx, "%s: %s\n", attrs[j], value); - DEBUG(SSSDBG_TRACE_ALL, "%s\n", ldif_attr); + DEBUG(SSSDBG_TRACE_LDB, "%s\n", ldif_attr); attr_len = strlen(ldif_attr); @@ -523,7 +523,7 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, dn[dn_size-1] = '\n'; dn[dn_size] = '\0'; - DEBUG(SSSDBG_TRACE_ALL, "Section dn\n%s\n", dn); + DEBUG(SSSDBG_TRACE_LDB, "Section dn\n%s\n", dn); tmp_ldif = talloc_realloc(mem_ctx, ldif, char, ldif_len+dn_size+1); From b600246e160d45e743dfd5cf39f62e8a5b5ddac0 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 15 Jan 2024 16:32:18 +0100 Subject: [PATCH 21/42] TOOLS: remove the upgrade-cache command 552390afcc81af96ca201fa6c25ddefbbecbeb4e mentioned ``` might be useful e.g. in RPM %post scripts. ``` but it didn't happen. SSSD performs cache upgrade at startup automatically, explicit command doesn't have any use. On the other hand, it can spoil cache files ownership if users used to run 'sssctl' and SSSD do not match. :relnote: sssct `cache-upgrade` command was removed. SSSD performs automatic upgrade at startup when needed. --- src/tools/sssctl/sssctl.c | 1 - src/tools/sssctl/sssctl.h | 4 ---- src/tools/sssctl/sssctl_data.c | 34 ---------------------------------- 3 files changed, 39 deletions(-) diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c index c494b4e4201..6fbfc95631b 100644 --- a/src/tools/sssctl/sssctl.c +++ b/src/tools/sssctl/sssctl.c @@ -327,7 +327,6 @@ int main(int argc, const char **argv) SSS_TOOL_COMMAND("client-data-backup", "Backup local data", 0, sssctl_client_data_backup), SSS_TOOL_COMMAND("client-data-restore", "Restore local data from backup", 0, sssctl_client_data_restore), SSS_TOOL_COMMAND("cache-remove", "Backup local data and remove cached content", 0, sssctl_cache_remove), - SSS_TOOL_COMMAND("cache-upgrade", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade), SSS_TOOL_COMMAND("cache-expire", "Invalidate cached objects", 0, sssctl_cache_expire), SSS_TOOL_COMMAND("cache-index", "Manage cache indexes", 0, sssctl_cache_index), SSS_TOOL_DELIMITER("Log files tools:"), diff --git a/src/tools/sssctl/sssctl.h b/src/tools/sssctl/sssctl.h index 3a53a890476..2b35609bdc7 100644 --- a/src/tools/sssctl/sssctl.h +++ b/src/tools/sssctl/sssctl.h @@ -81,10 +81,6 @@ errno_t sssctl_cache_remove(struct sss_cmdline *cmdline, struct sss_tool_ctx *tool_ctx, void *pvt); -errno_t sssctl_cache_upgrade(struct sss_cmdline *cmdline, - struct sss_tool_ctx *tool_ctx, - void *pvt); - errno_t sssctl_cache_expire(struct sss_cmdline *cmdline, struct sss_tool_ctx *tool_ctx, void *pvt); diff --git a/src/tools/sssctl/sssctl_data.c b/src/tools/sssctl/sssctl_data.c index 82f80c61e58..ef7ae574bf2 100644 --- a/src/tools/sssctl/sssctl_data.c +++ b/src/tools/sssctl/sssctl_data.c @@ -259,40 +259,6 @@ errno_t sssctl_cache_remove(struct sss_cmdline *cmdline, return EOK; } -errno_t sssctl_cache_upgrade(struct sss_cmdline *cmdline, - struct sss_tool_ctx *tool_ctx, - void *pvt) -{ - struct sysdb_upgrade_ctx db_up_ctx; - errno_t ret; - - ret = sss_tool_popt(cmdline, NULL, SSS_TOOL_OPT_OPTIONAL, NULL, NULL); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse command arguments\n"); - return ret; - } - - if (sss_daemon_running()) { - return ERR_SSSD_RUNNING; - } - - ret = confdb_get_domains(tool_ctx->confdb, &tool_ctx->domains); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "No domains configured.\n"); - return ret; - } - - db_up_ctx.cdb = tool_ctx->confdb; - ret = sysdb_init_ext(tool_ctx, tool_ctx->domains, &db_up_ctx, - true, 0, 0); - if (ret != EOK) { - SYSDB_VERSION_ERROR_DAEMON(ret); - return ret; - } - - return EOK; -} - errno_t sssctl_cache_expire(struct sss_cmdline *cmdline, struct sss_tool_ctx *tool_ctx, void *pvt) From afda89e167566918a3e3d41585706df525f3ef4b Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 30 Jan 2024 21:36:17 +0100 Subject: [PATCH 22/42] SYSTEMD: remove unused CAP_KILL There are some known issues like #5536 but those have to be solved differently. Having 'CAP_KILL' in sssd.service doesn't help anyway (and currently isn't used anyhow). --- src/sysv/systemd/sssd.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in index b988d43b6d9..5c9942ed200 100644 --- a/src/sysv/systemd/sssd.service.in +++ b/src/sysv/systemd/sssd.service.in @@ -17,7 +17,7 @@ PIDFile=@pidpath@/sssd.pid # Currently main SSSD process ('sssd') always runs under 'root' # ('User=' and 'Group=' defaults to 'root' for system services) # 'CapabilityBoundingSet' is used to limit privileges set: -CapabilityBoundingSet= @additional_caps@ CAP_CHOWN CAP_KILL CAP_SETGID CAP_SETUID +CapabilityBoundingSet= @additional_caps@ CAP_CHOWN CAP_SETGID CAP_SETUID Restart=on-abnormal @supplementary_groups@ From 7e36f59a09ef305ac402a0f8865a6278b212e652 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 31 Jan 2024 10:48:12 +0100 Subject: [PATCH 23/42] SYSTEMD: responders do not need any capabilities --- src/sysv/systemd/sssd-autofs.service.in | 2 ++ src/sysv/systemd/sssd-ifp.service.in | 6 ++---- src/sysv/systemd/sssd-nss.service.in | 2 ++ src/sysv/systemd/sssd-pac.service.in | 2 ++ src/sysv/systemd/sssd-pam.service.in | 2 ++ src/sysv/systemd/sssd-ssh.service.in | 2 ++ src/sysv/systemd/sssd-sudo.service.in | 2 ++ 7 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/sysv/systemd/sssd-autofs.service.in b/src/sysv/systemd/sssd-autofs.service.in index ce597602b24..1c4cb02bbc7 100644 --- a/src/sysv/systemd/sssd-autofs.service.in +++ b/src/sysv/systemd/sssd-autofs.service.in @@ -13,6 +13,8 @@ Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_autofs.log ExecStart=@libexecdir@/sssd/sssd_autofs ${DEBUG_LOGGER} --socket-activated +# No capabilities: +CapabilityBoundingSet= Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ diff --git a/src/sysv/systemd/sssd-ifp.service.in b/src/sysv/systemd/sssd-ifp.service.in index 2e307f3b001..39e9d23d1ab 100644 --- a/src/sysv/systemd/sssd-ifp.service.in +++ b/src/sysv/systemd/sssd-ifp.service.in @@ -11,10 +11,8 @@ Type=dbus BusName=org.freedesktop.sssd.infopipe ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_ifp.log ExecStart=@libexecdir@/sssd/sssd_ifp ${DEBUG_LOGGER} --socket-activated -# 'CapabilityBoundingSet' is used to limit privileges set only in case -# SSSD IFP service is configured to run under 'root' (if service -# is configured to run under non-privileged user this is a "no-op"): -CapabilityBoundingSet= @additional_caps@ +# No capabilities: +CapabilityBoundingSet= Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ diff --git a/src/sysv/systemd/sssd-nss.service.in b/src/sysv/systemd/sssd-nss.service.in index c0e5fa4ec31..bea93d192a5 100644 --- a/src/sysv/systemd/sssd-nss.service.in +++ b/src/sysv/systemd/sssd-nss.service.in @@ -12,6 +12,8 @@ Also=sssd-nss.socket Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ ExecStart=@libexecdir@/sssd/sssd_nss ${DEBUG_LOGGER} --socket-activated +# No capabilities: +CapabilityBoundingSet= Restart=on-failure # 'sssd_nss' is special in that it might be used for resolution of 'User='/'Group='/etc, # and this may cause the service to hang (loop). diff --git a/src/sysv/systemd/sssd-pac.service.in b/src/sysv/systemd/sssd-pac.service.in index b1bc2907303..57359a98f6c 100644 --- a/src/sysv/systemd/sssd-pac.service.in +++ b/src/sysv/systemd/sssd-pac.service.in @@ -13,6 +13,8 @@ Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_pac.log ExecStart=@libexecdir@/sssd/sssd_pac ${DEBUG_LOGGER} --socket-activated +# No capabilities: +CapabilityBoundingSet= Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ diff --git a/src/sysv/systemd/sssd-pam.service.in b/src/sysv/systemd/sssd-pam.service.in index 19d4bd3c733..2fececccac1 100644 --- a/src/sysv/systemd/sssd-pam.service.in +++ b/src/sysv/systemd/sssd-pam.service.in @@ -13,6 +13,8 @@ Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_pam.log @logpath@/p11_child.log ExecStart=@libexecdir@/sssd/sssd_pam ${DEBUG_LOGGER} --socket-activated +# No capabilities: +CapabilityBoundingSet= Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ diff --git a/src/sysv/systemd/sssd-ssh.service.in b/src/sysv/systemd/sssd-ssh.service.in index 42324b922c2..e0ca06e7143 100644 --- a/src/sysv/systemd/sssd-ssh.service.in +++ b/src/sysv/systemd/sssd-ssh.service.in @@ -13,6 +13,8 @@ Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_ssh.log @logpath@/p11_child.log ExecStart=@libexecdir@/sssd/sssd_ssh ${DEBUG_LOGGER} --socket-activated +# No capabilities: +CapabilityBoundingSet= Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ diff --git a/src/sysv/systemd/sssd-sudo.service.in b/src/sysv/systemd/sssd-sudo.service.in index 0adb9c0a9d3..fbf195031d0 100644 --- a/src/sysv/systemd/sssd-sudo.service.in +++ b/src/sysv/systemd/sssd-sudo.service.in @@ -13,6 +13,8 @@ Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_sudo.log ExecStart=@libexecdir@/sssd/sssd_sudo ${DEBUG_LOGGER} --socket-activated +# No capabilities: +CapabilityBoundingSet= Restart=on-failure User=@SSSD_USER@ Group=@SSSD_USER@ From f9a9f79e7c3c6d26d2f4829a825584041a5f8c56 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:24:56 +0200 Subject: [PATCH 24/42] MONITOR: startup logic was changed Startup logic was changed as follows: (1) read sssd.conf (should be readable by user that is used to start monitor) (2) switch user to sssd.conf::user (if configured), drop all capabilities (3) write config.ldb This ensures all SSSD components can read config.ldb without capabilities even if (deprecated) sssd.conf::user is used. --- src/confdb/confdb.c | 14 +- src/confdb/confdb_setup.c | 2 - src/confdb/confdb_setup.h | 14 + src/db/sysdb.h | 4 +- src/db/sysdb_init.c | 45 +-- src/man/sssd.conf.5.xml | 35 +- src/monitor/monitor.c | 309 +++++++++--------- src/monitor/monitor_bootstrap.c | 78 +++-- src/sbus/connection/sbus_connection_connect.c | 5 +- src/sbus/sbus.h | 8 - src/sbus/sbus_private.h | 2 - src/sbus/server/sbus_server.c | 45 +-- src/sbus/server/sbus_server_interface.c | 2 +- .../system/tests/test_default_debug_level.py | 16 +- src/util/capabilities.c | 34 +- src/util/server.c | 9 +- src/util/sss_ini.c | 36 +- src/util/usertools.c | 44 +-- src/util/util.h | 3 +- 19 files changed, 295 insertions(+), 410 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index bb12f7c9ebb..b4d133a2c80 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -649,8 +649,6 @@ int confdb_init(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb; int ret = EOK; mode_t old_umask; - uid_t sssd_uid; - gid_t sssd_gid; cdb = talloc_zero(mem_ctx, struct confdb_ctx); if (!cdb) @@ -683,19 +681,9 @@ int confdb_init(TALLOC_CTX *mem_ctx, } old_umask = umask(SSS_DFL_UMASK); - /* file may exists and could be owned by root from previous version */ - sss_sssd_user_uid_and_gid(&sssd_uid, &sssd_gid); - ret = chown(confdb_location, sssd_uid, sssd_gid); - if (ret != EOK && errno != ENOENT) { - DEBUG(SSSDBG_MINOR_FAILURE, "Unable to chown config database [%s]: %s\n", - confdb_location, sss_strerror(errno)); - } - sss_set_sssd_user_eid(); - ret = ldb_connect(cdb->ldb, confdb_location, 0, NULL); - - sss_restore_sssd_user_eid(); umask(old_umask); + if (ret != LDB_SUCCESS) { DEBUG(SSSDBG_FATAL_FAILURE, "Unable to open config database [%s]\n", confdb_location); diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index 47450c462c8..8ae5f33dd3f 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -66,8 +66,6 @@ errno_t confdb_read_ini(TALLOC_CTX *mem_ctx, return ret; } - sss_ini_call_validators(*_ini, SSSDDATADIR"/cfg_rules.ini"); - return EOK; } diff --git a/src/confdb/confdb_setup.h b/src/confdb/confdb_setup.h index c2186f753d2..f924d03d96f 100644 --- a/src/confdb/confdb_setup.h +++ b/src/confdb/confdb_setup.h @@ -26,6 +26,7 @@ #include #include "util/util_errors.h" +#include "util/sss_ini.h" struct confdb_ctx; @@ -37,4 +38,17 @@ errno_t confdb_setup(TALLOC_CTX *mem_ctx, bool allow_missing_file, struct confdb_ctx **_cdb); +errno_t confdb_read_ini(TALLOC_CTX *mem_ctx, + const char *config_file, + const char *config_dir, + bool allow_missing_config, + struct sss_ini **_ini); + +errno_t confdb_write_ini(TALLOC_CTX *mem_ctx, + const struct sss_ini *ini, + const char *cdb_file, + const char *only_section, + bool allow_missing_content, + struct confdb_ctx **_cdb); + #endif /* CONFDB_SETUP_H_ */ diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 55c6437f2de..1afd720cc59 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -803,9 +803,7 @@ struct sysdb_upgrade_ctx { int sysdb_init_ext(TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, - struct sysdb_upgrade_ctx *upgrade_ctx, - bool chown_dbfile, - uid_t uid, gid_t gid); + struct sysdb_upgrade_ctx *upgrade_ctx); /* used to initialize only one domain database. * Do NOT use if sysdb_init has already been called */ diff --git a/src/db/sysdb_init.c b/src/db/sysdb_init.c index 38a9cd64a9e..85db5f9e153 100644 --- a/src/db/sysdb_init.c +++ b/src/db/sysdb_init.c @@ -122,34 +122,6 @@ static errno_t sysdb_ldb_reconnect(TALLOC_CTX *mem_ctx, return ret; } -static errno_t sysdb_chown_db_files(struct sysdb_ctx *sysdb, - uid_t uid, gid_t gid) -{ - errno_t ret; - - ret = chown(sysdb->ldb_file, uid, gid); - if (ret != 0) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "Cannot set sysdb ownership of %s to %"SPRIuid":%"SPRIgid"\n", - sysdb->ldb_file, uid, gid); - return ret; - } - - if (sysdb->ldb_ts_file != NULL) { - ret = chown(sysdb->ldb_ts_file, uid, gid); - if (ret != 0) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, - "Cannot set sysdb ownership of %s to %"SPRIuid":%"SPRIgid"\n", - sysdb->ldb_ts_file, uid, gid); - return ret; - } - } - - return EOK; -} - int sysdb_get_db_file(TALLOC_CTX *mem_ctx, const char *provider, const char *name, @@ -1025,15 +997,12 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx, int sysdb_init(TALLOC_CTX *mem_ctx, struct sss_domain_info *domains) { - return sysdb_init_ext(mem_ctx, domains, NULL, false, 0, 0); + return sysdb_init_ext(mem_ctx, domains, NULL); } int sysdb_init_ext(TALLOC_CTX *mem_ctx, struct sss_domain_info *domains, - struct sysdb_upgrade_ctx *upgrade_ctx, - bool chown_dbfile, - uid_t uid, - gid_t gid) + struct sysdb_upgrade_ctx *upgrade_ctx) { struct sss_domain_info *dom; struct sysdb_ctx *sysdb; @@ -1080,16 +1049,6 @@ int sysdb_init_ext(TALLOC_CTX *mem_ctx, goto done; } - if (chown_dbfile) { - ret = sysdb_chown_db_files(sysdb, uid, gid); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Cannot chown databases for %s: [%d]: %s\n", - dom->name, ret, sss_strerror(ret)); - goto done; - } - } - dom->sysdb = talloc_move(dom, &sysdb); } diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index a5fa131a7da..fbd7c7b28a7 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -58,9 +58,8 @@ 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'). + readable, and writeable by the same user as configured to run SSSD + service. @@ -404,24 +403,26 @@ user (string) - The user to drop the privileges to where - appropriate to avoid running as the - root user. - Currently the only supported value is '&sssd_user_name;'. + A legacy (deprecated) method to configure the user + to drop the privileges to where appropriate to avoid + running as the root user. + The only supported value is '&sssd_user_name;'. - - This option does not work when running socket-activated - services, as the user set up to run the processes is - set up during compilation time. + + This option is ignored if main SSSD process is started + under non-root user initially (preferred method). + - The way to override the systemd unit files is by creating - the appropriate files in /etc/systemd/system/. + + This option doesn't apply to socket activated + services, as in this case the user to run the processes + is configured in systemd service files. - Keep in mind that any change in the socket user, group or - permissions may result in a non-usable SSSD. The same may - occur in case of changes of the user running the NSS - responder. + Keep in mind that using different service users for + different SSSD components in general isn't supported: + everything should be configured to run either under + '&sssd_user_name;' or 'root'. diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index a1555b4b8cd..732a287023c 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -36,6 +36,7 @@ #include #include +#include "util/sss_ini.h" #include "confdb/confdb.h" #include "confdb/confdb_setup.h" #include "db/sysdb.h" @@ -66,11 +67,6 @@ */ #define KRB5_RCACHE_DIR_DISABLE "__LIBKRB5_DEFAULTS__" -/* Warning messages */ -#define CONF_FILE_PERM_ERROR_MSG "Cannot read config file %s. Please check "\ - "that the file is accessible only by the "\ - "owner and owned by root.root.\n" - int cmdline_debug_level; int cmdline_debug_timestamps; int cmdline_debug_microseconds; @@ -815,29 +811,48 @@ static char *check_services(char **services) return NULL; } -static int get_service_user(struct mt_ctx *ctx) +static int get_service_user(struct sss_ini *config, struct mt_ctx *ctx) { errno_t ret = EOK; ctx->uid = 0; ctx->gid = 0; +/* If SSSD wasn't built '--with-sssd-user=sssd' then 'sssd.conf::user' + * option isn't supported completely (no man page entry). + */ #ifdef SSSD_NON_ROOT_USER char *user_str = NULL; - ret = confdb_get_string(ctx->cdb, ctx, CONFDB_MONITOR_CONF_ENTRY, - CONFDB_MONITOR_USER_RUNAS, - "root", &user_str); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "Failed to get the user to run as\n"); + ret = sss_ini_get_cfgobj(config, "sssd", CONFDB_MONITOR_USER_RUNAS); + if (ret != 0) { + ERROR("Config operation failed\n"); return ret; } + if (sss_ini_check_config_obj(config) == EOK) { + user_str = sss_ini_get_string_config_value(config, NULL); + } + + if (geteuid() != 0) { + if (user_str != NULL) { + sss_log(SSS_LOG_ALERT, "'"CONFDB_MONITOR_USER_RUNAS"' config option is " + "ignored when SSSD is run under non-root user initially."); + ERROR("'"CONFDB_MONITOR_USER_RUNAS"' config option is " + "ignored when SSSD is run under non-root user initially.\n"); + free(user_str); + } + ctx->uid = geteuid(); + ctx->gid = getegid(); + return EOK; + } - if (strcmp(user_str, SSSD_USER) == 0) { + if (user_str == NULL) { + /* defaults to 'root' */ + } else if (strcmp(user_str, SSSD_USER) == 0) { sss_sssd_user_uid_and_gid(&ctx->uid, &ctx->gid); + /* Deprecation warning is given in `bootstrap_monitor_process()` */ } else if (strcmp(user_str, "root") != 0) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Unsupported value '%s' of config option '%s'! Only 'root' or '" + ERROR("Unsupported value '%s' of config option '%s'! Only 'root' or '" SSSD_USER"' are supported.\n", user_str, CONFDB_MONITOR_USER_RUNAS); sss_log(SSS_LOG_CRIT, "Unsupported value of config option '%s'!", @@ -845,12 +860,37 @@ static int get_service_user(struct mt_ctx *ctx) ret = ERR_INVALID_CONFIG; } - talloc_free(user_str); + free(user_str); #endif return ret; } +static void get_debug_level(struct sss_ini *config) +{ + int ret; + + if (debug_level == SSSDBG_INVALID) { + debug_level = SSSDBG_DEFAULT; + } + + ret = sss_ini_get_cfgobj(config, "sssd", CONFDB_SERVICE_DEBUG_LEVEL); + if (ret != 0) { + return; + } + if (sss_ini_check_config_obj(config) != EOK) { + ret = sss_ini_get_cfgobj(config, "sssd", CONFDB_SERVICE_DEBUG_LEVEL_ALIAS); + if (ret != 0) { + return; + } + if (sss_ini_check_config_obj(config) != EOK) { + return; + } + } + + debug_level = sss_ini_get_int_config_value(config, 1, debug_level, NULL); +} + static int get_monitor_config(struct mt_ctx *ctx) { int ret; @@ -897,12 +937,6 @@ static int get_monitor_config(struct mt_ctx *ctx) } } - ret = get_service_user(ctx); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "Failed to get the unprivileged user\n"); - return ret; - } - ret = confdb_expand_app_domains(ctx->cdb); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to expand application domains\n"); @@ -1441,84 +1475,9 @@ static int monitor_ctx_destructor(void *mem) return 0; } -/* - * This function should not be static otherwise gcc does some special kind of - * optimisations which should not happen according to code: chown (unlink) - * failed (return -1) but errno was zero. - * As a result of this * warning is printed ‘monitor’ may be used - * uninitialized in this function. Instead of checking errno for 0 - * it's better to disable optimisation (in-lining) of this function. - */ -errno_t load_configuration(TALLOC_CTX *mem_ctx, - const char *config_file, - const char *config_dir, - struct mt_ctx **monitor) -{ - errno_t ret; - struct mt_ctx *ctx; - char *cdb_file = NULL; - uid_t sssd_uid; - gid_t sssd_gid; - - ctx = talloc_zero(mem_ctx, struct mt_ctx); - if(!ctx) { - return ENOMEM; - } - - ctx->pid_file_created = false; - talloc_set_destructor((TALLOC_CTX *)ctx, monitor_ctx_destructor); - - cdb_file = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE); - if (cdb_file == NULL) { - DEBUG(SSSDBG_FATAL_FAILURE,"Out of memory, aborting!\n"); - ret = ENOMEM; - goto done; - } - - - ret = confdb_setup(ctx, cdb_file, config_file, config_dir, NULL, false, - &ctx->cdb); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "Unable to setup ConfDB [%d]: %s\n", - ret, sss_strerror(ret)); - goto done; - } - - /* Validate the configuration in the database */ - /* Read in the monitor's configuration */ - ret = get_monitor_config(ctx); - if (ret != EOK) { - goto done; - } - - /* Allow configuration database to be accessible - * when SSSD runs as nonroot */ - sss_sssd_user_uid_and_gid(&sssd_uid, &sssd_gid); - ret = chown(cdb_file, sssd_uid, sssd_gid); - if (ret != 0) { - ret = errno; - DEBUG(SSSDBG_FATAL_FAILURE, - "chown failed for [%s]: [%d][%s].\n", - cdb_file, ret, sss_strerror(ret)); - goto done; - } - - *monitor = ctx; - - ret = EOK; - -done: - talloc_free(cdb_file); - if (ret != EOK) { - talloc_free(ctx); - } - return ret; -} - static void monitor_sbus_connected(struct tevent_req *req); -static int monitor_process_init(struct mt_ctx *ctx, - const char *config_file) +static int monitor_process_init(struct mt_ctx *ctx) { TALLOC_CTX *tmp_ctx; struct tevent_signal *tes; @@ -1535,6 +1494,8 @@ static int monitor_process_init(struct mt_ctx *ctx, KRB5_RCACHE_DIR, &rcachedir); if (ret != EOK) { + DEBUG(SSSDBG_TRACE_FUNC, + "confdb_get_string("CONFDB_MONITOR_KRB5_RCACHEDIR") failed\n"); return ret; } @@ -1604,9 +1565,10 @@ static int monitor_process_init(struct mt_ctx *ctx, } db_up_ctx.cdb = ctx->cdb; - ret = sysdb_init_ext(tmp_ctx, ctx->domains, &db_up_ctx, - true, ctx->uid, ctx->gid); + ret = sysdb_init_ext(tmp_ctx, ctx->domains, &db_up_ctx); if (ret != EOK) { + DEBUG(SSSDBG_TRACE_FUNC, + "sysdb_init_ext() failed: '%s'\n", sss_strerror(ret)); SYSDB_VERSION_ERROR_DAEMON(ret); goto done; } @@ -1614,9 +1576,9 @@ static int monitor_process_init(struct mt_ctx *ctx, req = sbus_server_create_and_connect_send(ctx, ctx->ev, SSS_BUS_MONITOR, NULL, SSS_BUS_ADDRESS, - false, 100, ctx->uid, ctx->gid, - NULL, NULL); + false, 100, NULL, NULL); if (req == NULL) { + DEBUG(SSSDBG_TRACE_FUNC, "sbus_server_create_and_connect_send() failed\n"); ret = ENOMEM; goto done; } @@ -1829,12 +1791,6 @@ static void service_startup_handler(struct tevent_context *ev, } /* child */ - ret = become_user(mt_svc->mt_ctx->uid, mt_svc->mt_ctx->gid); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "become_user() failed: '%s'\n", - sss_strerror(ret)); - _exit(1); - } if (mt_svc->type != MT_SVC_PROVIDER) { /* providers are excluded becase they will need to execute * child processes that elevate privs @@ -1997,7 +1953,7 @@ static void check_nscd(void) } } -int bootstrap_monitor_process(void); +int bootstrap_monitor_process(uid_t target_uid, gid_t target_gid); void setup_keyring(void); int main(int argc, const char *argv[]) @@ -2015,12 +1971,21 @@ int main(int argc, const char *argv[]) TALLOC_CTX *tmp_ctx; struct mt_ctx *monitor; int ret; - uid_t uid; + uid_t uid, euid; + gid_t gid, egid; + char *initial_caps; + struct sss_ini *config; + char *cdb_file = NULL; tmp_ctx = talloc_new(NULL); - if (!tmp_ctx) { + if (tmp_ctx == NULL) { + return 2; + } + monitor = talloc_zero(tmp_ctx, struct mt_ctx); + if (monitor == NULL) { return 2; } + talloc_set_destructor((TALLOC_CTX *)monitor, monitor_ctx_destructor); struct poptOption long_options[] = { POPT_AUTOHELP @@ -2085,29 +2050,79 @@ int main(int argc, const char *argv[]) } else { config_file = talloc_strdup(tmp_ctx, SSSD_CONFIG_FILE); } - if (!config_file) { + if (config_file == NULL) { + return 2; + } + + cdb_file = talloc_asprintf(tmp_ctx, "%s/%s", DB_PATH, CONFDB_FILE); + if (cdb_file == NULL) { return 2; } poptFreeContext(pc); - /* TODO: revisit */ uid = getuid(); - if (uid != 0) { - ERROR("Running under %"PRIu64", must be root\n", (uint64_t) uid); - sss_log(SSS_LOG_ALERT, "sssd must be run as root"); + euid = geteuid(); + gid = getgid(); + egid = getegid(); + ret = sss_log_caps_to_str(true, &initial_caps); + if (ret != 0) { + ERROR("Failed to get initial capabilities\n"); + return 3; + } + +#ifndef SSSD_NON_ROOT_USER + /* Non-root service user support isn't built. */ + /* SSSD should run under root */ + if ((euid != 0) || (egid != 0)) { + sss_log(SSS_LOG_ALERT, "Non-root service user support isn't built. " + "Can't run under non-root"); + ERROR("Non-root service user support isn't built. " + "Can't run under %"SPRIuid":%"SPRIgid"\n", euid, egid); return 1; } + /* Everything is root:root owned. No caps required. */ + if (initial_caps != NULL) { + sss_log(SSS_LOG_ALERT, + "Those capabilities aren't needed and can be removed:\n %s", + initial_caps); + } +#endif /* !SSSD_NON_ROOT_USER */ - ret = bootstrap_monitor_process(); + /* read/parse configuration (but don't save yet) */ + ret = confdb_read_ini(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, false, + &config); + if (ret != EOK) { + ERROR("Can't read config: '%s'\n", sss_strerror(ret)); + sss_log(SSS_LOG_ALERT, + "Failed to read configuration: '%s'", sss_strerror(ret)); + return 3; + } + + ret = get_service_user(config, monitor); + if (ret != EOK) { + return 4; + } + + ret = bootstrap_monitor_process(monitor->uid, monitor->gid); if (ret != 0) { ERROR("Failed to boostrap SSSD 'monitor' process: %s", sss_strerror(ret)); sss_log(SSS_LOG_ALERT, "Failed to boostrap SSSD 'monitor' process."); return 5; } - /* default value of 'debug_prg_name' will be used */ - DEBUG_INIT(debug_level, opt_logger); + get_debug_level(config); + DEBUG_INIT(debug_level, opt_logger); /* use default value of 'debug_prg_name' */ + + DEBUG(SSSDBG_IMPORTANT_INFO, + "Started under uid=%"SPRIuid" (euid=%"SPRIuid") : " + "gid=%"SPRIgid" (egid=%"SPRIgid") with SECBIT_KEEP_CAPS = %d" + " and following capabilities:\n%s", + uid, euid, gid, egid, prctl(PR_GET_KEEPCAPS, 0, 0, 0, 0), + initial_caps ? initial_caps : " (nothing)\n"); + talloc_free(initial_caps); + + sss_ini_call_validators(config, SSSDDATADIR"/cfg_rules.ini"); setup_keyring(); @@ -2117,60 +2132,48 @@ int main(int argc, const char *argv[]) ret = check_pidfile(SSSD_PIDFILE); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, - "pidfile exists at %s\n", SSSD_PIDFILE); - ERROR("SSSD is already running\n"); - return 5; + "SSSD is already running: pidfile exists at '"SSSD_PIDFILE"'\n"); + return 6; } } check_nscd(); - /* Parse config file, fail if cannot be done */ - ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, - &monitor); - if (ret != EOK) { - switch (ret) { - case EPERM: - case EACCES: - DEBUG(SSSDBG_FATAL_FAILURE, - CONF_FILE_PERM_ERROR_MSG, config_file); - sss_log(SSS_LOG_CRIT, CONF_FILE_PERM_ERROR_MSG, config_file); - break; - default: - DEBUG(SSSDBG_FATAL_FAILURE, - "SSSD couldn't load the configuration database [%d]: %s\n", - ret, sss_strerror(ret)); - sss_log(SSS_LOG_CRIT, - "SSSD couldn't load the configuration database [%d]: %s\n", - ret, sss_strerror(ret)); - break; - } - return 5; - } - /* set up things like debug, signals, daemonization, etc. */ ret = close(STDIN_FILENO); - if (ret != EOK) return 5; + if (ret != EOK) return 7; - ret = server_setup(SSSD_MONITOR_NAME, false, flags, CONFDB_FILE, - CONFDB_MONITOR_CONF_ENTRY, &main_ctx, false); - if (ret != EOK) return 5; + ret = confdb_write_ini(tmp_ctx, config, cdb_file, false, false, &monitor->cdb); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to write config DB: '%s'\n", sss_strerror(ret)); + return 8; + } - /* Use confd initialized in server_setup. ldb_tdb module (1.4.0) check PID + /* Use confdb initialized in server_setup. ldb_tdb module (1.4.0) check PID * of process which initialized db for locking purposes. * Failed to unlock db: ../ldb_tdb/ldb_tdb.c:147: * Reusing ldb opened by pid 28889 in process 28893 */ talloc_zfree(monitor->cdb); - monitor->cdb = main_ctx->confdb_ctx; + ret = server_setup(SSSD_MONITOR_NAME, false, flags, CONFDB_FILE, + CONFDB_MONITOR_CONF_ENTRY, &main_ctx, false); + if (ret != EOK) return 9; + + monitor->cdb = main_ctx->confdb_ctx; + get_monitor_config(monitor); monitor->is_daemon = !opt_interactive; monitor->parent_pid = main_ctx->parent_pid; monitor->ev = main_ctx->event_ctx; talloc_steal(main_ctx, monitor); - ret = monitor_process_init(monitor, config_file); - if (ret != EOK) return 5; + ret = monitor_process_init(monitor); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "monitor_process_init() failed: '%s'\n", sss_strerror(ret)); + return 10; + } talloc_free(tmp_ctx); @@ -2178,7 +2181,7 @@ int main(int argc, const char *argv[]) server_loop(main_ctx); ret = monitor_cleanup(); - if (ret != EOK) return 5; + if (ret != EOK) return 12; return 0; } diff --git a/src/monitor/monitor_bootstrap.c b/src/monitor/monitor_bootstrap.c index d85483aa9d3..6bbee881e9c 100644 --- a/src/monitor/monitor_bootstrap.c +++ b/src/monitor/monitor_bootstrap.c @@ -77,50 +77,62 @@ static int check_supplementary_group(gid_t gid) } #endif /* SSSD_NON_ROOT_USER */ -int bootstrap_monitor_process(void) +int bootstrap_monitor_process(uid_t target_uid, gid_t target_gid) { - #ifdef SSSD_NON_ROOT_USER - /* In case SSSD is built with non-root user support, - * a number of files are sssd:sssd owned. - * Make sure all processes are added to sssd supplementary - * group to avoid the need for CAP_DAC_OVERRIDE. - * - * TODO: read 'sssd.conf::user' first and in case it is set - * to 'sssd' become_user(sssd) instead. - */ int ret; gid_t sssd_gid = 0; - if ((getuid() == 0) || (geteuid() == 0)) { - sss_sssd_user_uid_and_gid(NULL, &sssd_gid); - ret = check_supplementary_group(sssd_gid); - if (ret == -1) { - sss_log(SSS_LOG_ALERT, "Can't check own supplementary groups."); - return 1; - } - /* Expected outcome is 'ret == 1' since supplementary group should be set - by systemd service description. */ - if (ret == 0) { - /* Probably non-systemd based system or service file was edited, - let's try to set group manually. */ - sss_log(SSS_LOG_WARNING, - "SSSD is built with support of 'run under non-root user' " - "feature but started under 'root'. Trying to add process " - "to SSSD supplementary group."); - ret = setgroups(1, &sssd_gid); + + if (geteuid() == 0) { + if (target_uid != 0) { + /* Started under root but non-root 'sssd.conf::user' configured - + * deprecated method. + */ + sss_log(SSS_LOG_WARNING, "'sssd.conf::"CONFDB_MONITOR_USER_RUNAS"' " + "option is deprecated. Run under '"SSSD_USER"' initially instead."); + ret = become_user(target_uid, target_gid); /* drops all caps */ if (ret != 0) { - sss_log(SSS_LOG_CRIT, - "Failed to add process to the "SSSD_USER" supplementary group. " - "Either run under '"SSSD_USER"' or make sure that run-under-root " - "main SSSD process has CAP_SETGID."); + sss_log(SSS_LOG_ALERT, "Failed to change uid:gid"); + return 1; + } + } else { + /* In case SSSD is built with non-root user support, but + * runs under 'root', a number of files are still sssd:sssd owned. + * Make sure all processes are added to 'sssd' supplementary + * group to avoid the need for CAP_DAC_OVERRIDE. + */ + sss_sssd_user_uid_and_gid(NULL, &sssd_gid); + ret = check_supplementary_group(sssd_gid); + if (ret == -1) { + sss_log(SSS_LOG_ALERT, "Can't check own supplementary groups."); return 1; } + /* Expected outcome is 'ret == 1' since supplementary group should be set + by systemd service description. */ + if (ret == 0) { + /* Probably non-systemd based system or service file was edited, + let's try to set group manually. */ + sss_log(SSS_LOG_NOTICE, + "SSSD is built with support of 'run under non-root user' " + "feature but started under 'root'. Trying to add process " + "to SSSD supplementary group."); + ret = setgroups(1, &sssd_gid); + if (ret != 0) { + sss_log(SSS_LOG_CRIT, + "Failed to add process to the "SSSD_USER" supplementary group. " + "Either run under '"SSSD_USER"' or make sure that run-under-root " + "main SSSD process has CAP_SETGID."); + return 1; + } + } } - - /* TODO: drop CAP_SET_GID capability */ + } else { + /* SSSD started under non 'root' initially - nothing to do */ } #endif /* SSSD_NON_ROOT_USER */ + sss_drop_all_caps(); + return 0; } diff --git a/src/sbus/connection/sbus_connection_connect.c b/src/sbus/connection/sbus_connection_connect.c index de6865039a3..c8bf2652958 100644 --- a/src/sbus/connection/sbus_connection_connect.c +++ b/src/sbus/connection/sbus_connection_connect.c @@ -348,8 +348,6 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx, const char *address, bool use_symlink, uint32_t max_connections, - uid_t uid, - gid_t gid, sbus_server_on_connection_cb on_conn_cb, sbus_server_on_connection_data on_conn_data) { @@ -365,8 +363,7 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx, } state->server = sbus_server_create(state, ev, address, use_symlink, - max_connections, uid, gid, - on_conn_cb, on_conn_data); + max_connections, on_conn_cb, on_conn_data); if (state->server == NULL) { ret = ENOMEM; goto done; diff --git a/src/sbus/sbus.h b/src/sbus/sbus.h index 77fd506cf75..c5f564f20ac 100644 --- a/src/sbus/sbus.h +++ b/src/sbus/sbus.h @@ -91,8 +91,6 @@ sbus_connect_private(TALLOC_CTX *mem_ctx, * @param ev Tevent context. * @param address Socket address. * @param use_symlink If a symlink to @address should be created. - * @param uid Socket owner uid. - * @param gid Socket owner gid. * @param on_conn_cb On new connection callback function. * @param on_conn_data Private data passed to the callback. * @@ -104,8 +102,6 @@ sbus_server_create(TALLOC_CTX *mem_ctx, const char *address, bool use_symlink, uint32_t max_connections, - uid_t uid, - gid_t gid, sbus_server_on_connection_cb on_conn_cb, sbus_server_on_connection_data on_conn_data); @@ -119,8 +115,6 @@ sbus_server_create(TALLOC_CTX *mem_ctx, * an event occurs on connection. * @param address Socket address. * @param use_symlink If a symlink to @address should be created. - * @param uid Socket owner uid. - * @param gid Socket owner gid. * @param on_conn_cb On new connection callback function. * @param on_conn_data Private data passed to the callback. * @@ -134,8 +128,6 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx, const char *address, bool use_symlink, uint32_t max_connections, - uid_t uid, - gid_t gid, sbus_server_on_connection_cb on_conn_cb, sbus_server_on_connection_data on_conn_data); diff --git a/src/sbus/sbus_private.h b/src/sbus/sbus_private.h index eef397b8655..a55709086bb 100644 --- a/src/sbus/sbus_private.h +++ b/src/sbus/sbus_private.h @@ -121,8 +121,6 @@ struct sbus_server { hash_table_t *names; hash_table_t *match_rules; uint32_t max_connections; - uid_t uid; - gid_t gid; struct sbus_server_on_connection *on_connection; bool disconnecting; diff --git a/src/sbus/server/sbus_server.c b/src/sbus/server/sbus_server.c index 9c9ddc89088..83b99821b5a 100644 --- a/src/sbus/server/sbus_server.c +++ b/src/sbus/server/sbus_server.c @@ -267,7 +267,7 @@ sbus_server_symlink_remove(const char *name) } static errno_t -sbus_server_check_file(const char *filename, uid_t uid, gid_t gid) +sbus_server_check_file(const char *filename) { struct stat stat_buf; errno_t ret; @@ -290,16 +290,6 @@ sbus_server_check_file(const char *filename, uid_t uid, gid_t gid) } } - if (stat_buf.st_uid != uid || stat_buf.st_gid != gid) { - ret = chown(filename, uid, gid); - if (ret != EOK) { - ret = errno; - DEBUG(SSSDBG_CRIT_FAILURE, "chown failed for [%s] [%d]: %s\n", - filename, ret, sss_strerror(ret)); - return ret; - } - } - return EOK; } @@ -308,8 +298,6 @@ sbus_server_setup_dbus(TALLOC_CTX *mem_ctx, struct tevent_context *ev, const char *address, bool use_symlink, - uid_t uid, - gid_t gid, const char **_symlink) { TALLOC_CTX *tmp_ctx; @@ -353,8 +341,8 @@ sbus_server_setup_dbus(TALLOC_CTX *mem_ctx, symlink_created = true; } - /* Check file permissions and setup proper owner. */ - ret = sbus_server_check_file(filename, uid, gid); + /* Check file permissions. */ + ret = sbus_server_check_file(filename); if (ret != EOK) { goto done; } @@ -404,22 +392,6 @@ sbus_server_filter_add(struct sbus_server *server, return true; } -static dbus_bool_t -sbus_server_check_connection_uid(DBusConnection *dbus_conn, - unsigned long uid, - void *data) -{ - struct sbus_server *sbus_server; - - sbus_server = talloc_get_type(data, struct sbus_server); - - if (uid == 0 || uid == sbus_server->uid) { - return true; - } - - return false; -} - static void sbus_server_new_connection(DBusServer *dbus_server, DBusConnection *dbus_conn, @@ -435,11 +407,6 @@ sbus_server_new_connection(DBusServer *dbus_server, DEBUG(SSSDBG_FUNC_DATA, "Adding connection %p.\n", dbus_conn); - /* Allow access from uid that is associated with this sbus server. */ - dbus_connection_set_unix_user_function(dbus_conn, - sbus_server_check_connection_uid, - sbus_server, NULL); - /* First, add a message filter that will take care of routing messages * between connections. */ bret = sbus_server_filter_add(sbus_server, dbus_conn); @@ -638,8 +605,6 @@ sbus_server_create(TALLOC_CTX *mem_ctx, const char *address, bool use_symlink, uint32_t max_connections, - uid_t uid, - gid_t gid, sbus_server_on_connection_cb on_conn_cb, sbus_server_on_connection_data on_conn_data) { @@ -658,7 +623,7 @@ sbus_server_create(TALLOC_CTX *mem_ctx, talloc_set_destructor(sbus_server, sbus_server_destructor); dbus_server = sbus_server_setup_dbus(sbus_server, ev, address, - use_symlink, uid, gid, &symlink); + use_symlink, &symlink); if (dbus_server == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "Unable to setup a D-Bus server!\n"); ret = ENOMEM; @@ -671,8 +636,6 @@ sbus_server_create(TALLOC_CTX *mem_ctx, sbus_server->max_connections = max_connections; sbus_server->name.major = 1; sbus_server->name.minor = 0; - sbus_server->uid = uid; - sbus_server->gid = gid; sbus_server->on_connection = talloc_zero(sbus_server, struct sbus_server_on_connection); diff --git a/src/sbus/server/sbus_server_interface.c b/src/sbus/server/sbus_server_interface.c index 9c0ba0abbfb..6e5715db248 100644 --- a/src/sbus/server/sbus_server_interface.c +++ b/src/sbus/server/sbus_server_interface.c @@ -292,7 +292,7 @@ sbus_server_bus_get_connection_unix_user(TALLOC_CTX *mem_ctx, dbus_bool_t dbret; if (strcmp(name, DBUS_SERVICE_DBUS) == 0) { - *_uid = server->uid; + *_uid = geteuid(); return EOK; } diff --git a/src/tests/system/tests/test_default_debug_level.py b/src/tests/system/tests/test_default_debug_level.py index 29fa9308a9f..93de621ab77 100644 --- a/src/tests/system/tests/test_default_debug_level.py +++ b/src/tests/system/tests/test_default_debug_level.py @@ -80,11 +80,9 @@ def test_default_debug_level__fatal_and_critical_failures(client: Client): 1. Start SSSD with default debug level (config file is created) 2. Restrict sssd.conf permissions :steps: - 1. Restart sssd - 2. Check logs + 1. Restart sssd and check exit code :expectedresults: - 1. SSSD failed to start - 2. Fatal failures (level 0) and Critical failures (level 1) are in log file + 1. SSSD failed to start with expected error code :customerscenario: True """ client.sssd.common.local() @@ -93,12 +91,8 @@ def test_default_debug_level__fatal_and_critical_failures(client: Client): client.fs.chmod(mode="444", path="/etc/sssd/sssd.conf") assert ( - client.sssd.restart(debug_level=None, raise_on_error=False, apply_config=False).rc != 0 - ), "SSSD started successfully, which is not expected" - - log_str = client.fs.read(client.sssd.logs.monitor) - assert "0x0010" in log_str, "Fatal failures were not found in log" - assert "0x0020" in log_str, "Critical failures were not found in log" + client.sssd.restart(debug_level=None, raise_on_error=False, apply_config=False).rc == 3 + ), "SSSD didn't fail to read config, which is not expected" @pytest.mark.ticket(bz=1893159) @@ -120,7 +114,7 @@ def test_default_debug_level__cannot_load_sssd_config(client: Client): assert ( client.sssd.start(debug_level=None, raise_on_error=False).rc != 0 ), "SSSD started successfully, which is not expected" - assert "SSSD couldn't load the configuration database" in client.fs.read(client.sssd.logs.monitor) + assert "id_provider is not set for domain [non_existing_domain]" in client.fs.read(client.sssd.logs.monitor) @pytest.mark.ticket(bz=1893159) diff --git a/src/util/capabilities.c b/src/util/capabilities.c index ca5f09bee29..c627518f831 100644 --- a/src/util/capabilities.c +++ b/src/util/capabilities.c @@ -92,7 +92,7 @@ errno_t sss_log_caps_to_str(bool only_non_zero, char **_str) char *str = NULL; size_t i; cap_t caps; - cap_flag_value_t effective, permitted, bounding; + cap_flag_value_t effective, permitted, inheritable, bounding; caps = cap_get_proc(); if (caps == NULL) { @@ -122,6 +122,14 @@ errno_t sss_log_caps_to_str(bool only_non_zero, char **_str) ret, strerror(ret)); goto done; } + ret = cap_get_flag(caps, _all_caps[i].val, CAP_INHERITABLE, &inheritable); + if (ret == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, + "cap_get_flag(CAP_INHERITABLE) failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } ret = cap_get_bound(_all_caps[i].val); if (ret == 1) { bounding = CAP_SET; @@ -135,14 +143,16 @@ errno_t sss_log_caps_to_str(bool only_non_zero, char **_str) } if (only_non_zero && (effective == CAP_CLEAR) && - (permitted == CAP_CLEAR) && (bounding == CAP_CLEAR)) { + (permitted == CAP_CLEAR) && (inheritable == CAP_CLEAR)) { + /* 'bounding' doesn't matter */ continue; } str = talloc_asprintf_append(str, - " %25s: effective = %s, permitted = %s, bounding = %s\n", + " %25s: effective = %s, permitted = %s, inheritable = %s, bounding = %s\n", _all_caps[i].name, cap_flag_to_str(effective), - cap_flag_to_str(permitted), cap_flag_to_str(bounding)); + cap_flag_to_str(permitted), cap_flag_to_str(inheritable), + cap_flag_to_str(bounding)); if (str == NULL) { ret = ENOMEM; goto done; @@ -190,6 +200,13 @@ errno_t sss_drop_cap(cap_value_t cap) ret, strerror(ret)); goto done; } + if (cap_set_flag(caps, CAP_INHERITABLE, 1, &cap, CAP_CLEAR) == -1) { + ret = errno; + DEBUG(SSSDBG_TRACE_FUNC, + "cap_set_flag(CAP_INHERITABLE) failed: %d ('%s')\n", + ret, strerror(ret)); + goto done; + } if (cap_set_proc(caps) == -1) { ret = errno; DEBUG(SSSDBG_TRACE_FUNC, "cap_set_proc() failed: %d ('%s')\n", @@ -206,3 +223,12 @@ errno_t sss_drop_cap(cap_value_t cap) return ret; } + +void sss_drop_all_caps(void) +{ + size_t i; + + for (i = 0; i < sizeof(_all_caps)/sizeof(cap_description); ++i) { + sss_drop_cap(_all_caps[i].val); + } +} diff --git a/src/util/server.c b/src/util/server.c index c2d6944db88..ac59f918ca6 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -778,8 +778,15 @@ void server_loop(struct main_context *main_ctx) DEBUG(SSSDBG_IMPORTANT_INFO, "Failed to log current capabilities\n"); } else { DEBUG(SSSDBG_IMPORTANT_INFO, - "Entering main loop with following capabilities:\n%s", + "Entering main loop under uid=%"SPRIuid" (euid=%"SPRIuid") : " + "gid=%"SPRIgid" (egid=%"SPRIgid") with SECBIT_KEEP_CAPS = %d" + " and following capabilities:\n%s", + getuid(), geteuid(), getgid(), getegid(), + prctl(PR_GET_KEEPCAPS, 0, 0, 0, 0), caps ? caps : " (nothing)\n"); + if (caps != NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Non empty capabilities set!\n"); + } talloc_free(caps); } diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 3134544d7c0..37ae4c7bd8c 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -149,37 +149,20 @@ 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; } - /* 'SSSD_USER:SSSD_USER' 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 */ + geteuid(), + getegid(), 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; } @@ -284,8 +267,6 @@ static int sss_ini_add_snippets(struct sss_ini *self, char *msg = NULL; struct ini_cfgobj *modified_sssd_config = NULL; struct access_check snip_check; - uid_t uid = 0; - gid_t gid = 0; if (self == NULL || self->sssd_config == NULL || config_dir == NULL) { return EINVAL; @@ -295,17 +276,8 @@ static int sss_ini_add_snippets(struct sss_ini *self, snip_check.flags = INI_ACCESS_CHECK_MODE | INI_ACCESS_CHECK_UID | INI_ACCESS_CHECK_GID; - if (getuid() == 0) { - /* SSSD is configured to run under root, let's allow 'root:root' - owned snippets to avoid breaking existing setups */ - snip_check.uid = 0; /* owned by root */ - snip_check.gid = 0; /* owned by root */ - } else { - /* Otherwise let's make sure snippets are 'sssd:sssd' owned. */ - sss_sssd_user_uid_and_gid(&uid, &gid); - snip_check.uid = uid; /* owned by SSSD_USER */ - snip_check.gid = gid; /* owned by SSSD_USER */ - } + snip_check.uid = geteuid(); + snip_check.gid = getegid(); snip_check.mode = S_IRUSR; /* r**------ */ snip_check.mask = ALLPERMS & ~(S_IWUSR | S_IXUSR); diff --git a/src/util/usertools.c b/src/util/usertools.c index 8084760a03e..0d11a95719d 100644 --- a/src/util/usertools.c +++ b/src/util/usertools.c @@ -832,8 +832,9 @@ int sss_output_fqname(TALLOC_CTX *mem_ctx, void sss_sssd_user_uid_and_gid(uid_t *_uid, gid_t *_gid) { - uid_t sssd_uid; - gid_t sssd_gid; + uid_t sssd_uid = 0; + gid_t sssd_gid = 0; +#ifdef SSSD_NON_ROOT_USER errno_t ret; ret = sss_user_by_name_or_uid(SSSD_USER, &sssd_uid, &sssd_gid); @@ -842,6 +843,7 @@ void sss_sssd_user_uid_and_gid(uid_t *_uid, gid_t *_gid) sssd_uid = 0; sssd_gid = 0; } +#endif /* SSSD_NON_ROOT_USER */ if (_uid != NULL) { *_uid = sssd_uid; @@ -851,41 +853,3 @@ void sss_sssd_user_uid_and_gid(uid_t *_uid, gid_t *_gid) *_gid = sssd_gid; } } - -void sss_set_sssd_user_eid(void) -{ - uid_t uid; - gid_t gid; - - - if (geteuid() == 0) { - sss_sssd_user_uid_and_gid(&uid, &gid); - - if (setegid(gid) != EOK) { - DEBUG(SSSDBG_IMPORTANT_INFO, - "Failed to set egid to %"SPRIgid": %s\n", - gid, sss_strerror(errno)); - } - if (seteuid(uid) != EOK) { - DEBUG(SSSDBG_IMPORTANT_INFO, - "Failed to set euid to %"SPRIuid": %s\n", - uid, sss_strerror(errno)); - } - } -} - -void sss_restore_sssd_user_eid(void) -{ - if (getuid() == 0) { - if (seteuid(getuid()) != EOK) { - DEBUG(SSSDBG_IMPORTANT_INFO, - "Failed to restore euid: %s\n", - sss_strerror(errno)); - } - if (setegid(getgid()) != EOK) { - DEBUG(SSSDBG_IMPORTANT_INFO, - "Failed to restore egid: %s\n", - sss_strerror(errno)); - } - } -} diff --git a/src/util/util.h b/src/util/util.h index 0a04f281552..aad112b5cca 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -390,8 +390,6 @@ const char * const * get_known_services(void); errno_t sss_user_by_name_or_uid(const char *input, uid_t *_uid, gid_t *_gid); void sss_sssd_user_uid_and_gid(uid_t *_uid, gid_t *_gid); -void sss_set_sssd_user_eid(void); -void sss_restore_sssd_user_eid(void); int split_on_separator(TALLOC_CTX *mem_ctx, const char *str, const char sep, bool trim, bool skip_empty, @@ -755,6 +753,7 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, errno_t restore_creds(struct sss_creds *saved_creds); errno_t sss_log_caps_to_str(bool only_non_zero, char **_str); errno_t sss_drop_cap(cap_value_t cap); +void sss_drop_all_caps(void); /* from sss_semanage.c */ /* Please note that libsemange relies on files and directories created with From 183d739e585a503f60b96daab111518a5541a5ed Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 1 Feb 2024 11:44:57 +0100 Subject: [PATCH 25/42] KRB5_/LDAP_CHILD: print capabilities at startup --- Makefile.am | 2 ++ src/providers/krb5/krb5_child.c | 15 ++++++++++++++- src/providers/ldap/ldap_child.c | 15 ++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 2fe094434b8..e572f94c0e6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4701,6 +4701,7 @@ krb5_child_SOURCES = \ src/util/authtok.c \ src/util/authtok-utils.c \ src/util/util.c \ + src/util/capabilities.c \ src/util/util_ext.c \ src/util/signal.c \ src/util/sss_chain_id.c \ @@ -4743,6 +4744,7 @@ ldap_child_SOURCES = \ src/util/authtok-utils.c \ src/util/util.c \ src/util/util_ext.c \ + src/util/capabilities.c \ src/util/signal.c \ src/util/become_user.c \ src/util/util_errors.c \ diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index 9de39f6bf93..b5d7ffa0635 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -4063,6 +4063,7 @@ int main(int argc, const char *argv[]) struct cli_opts cli_opts = { 0 }; int sss_creds_password = 0; long dummy_long = 0; + char *caps = NULL; struct poptOption long_options[] = { POPT_AUTOHELP @@ -4159,7 +4160,19 @@ int main(int argc, const char *argv[]) DEBUG_INIT(debug_level, opt_logger); - DEBUG(SSSDBG_TRACE_FUNC, "krb5_child started.\n"); + DEBUG(SSSDBG_CONF_SETTINGS, + "Starting under uid=%"SPRIuid" (euid=%"SPRIuid") : " + "gid=%"SPRIgid" (egid=%"SPRIgid")\n", + getuid(), geteuid(), getgid(), getegid()); + + ret = sss_log_caps_to_str(true, &caps); + if (ret == 0) { + DEBUG(SSSDBG_CONF_SETTINGS, "With following capabilities:\n%s", + caps ? caps : " (nothing)\n"); + talloc_free(caps); + } else { + DEBUG(SSSDBG_MINOR_FAILURE, "Failed to get current capabilities\n"); + } kr = talloc_zero(NULL, struct krb5_req); if (kr == NULL) { diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index 31faceaa3fc..470f52e7cfa 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -999,6 +999,7 @@ int main(int argc, const char *argv[]) struct input_buffer *ibuf = NULL; struct response *resp = NULL; ssize_t written; + char *caps = NULL; struct poptOption long_options[] = { POPT_AUTOHELP @@ -1050,7 +1051,19 @@ int main(int argc, const char *argv[]) BlockSignals(false, SIGTERM); CatchSignal(SIGTERM, sig_term_handler); - DEBUG(SSSDBG_TRACE_FUNC, "ldap_child started.\n"); + DEBUG(SSSDBG_CONF_SETTINGS, + "Starting under uid=%"SPRIuid" (euid=%"SPRIuid") : " + "gid=%"SPRIgid" (egid=%"SPRIgid")\n", + getuid(), geteuid(), getgid(), getegid()); + + ret = sss_log_caps_to_str(true, &caps); + if (ret == 0) { + DEBUG(SSSDBG_CONF_SETTINGS, "With following capabilities:\n%s", + caps ? caps : " (nothing)\n"); + talloc_free(caps); + } else { + DEBUG(SSSDBG_MINOR_FAILURE, "Failed to get current capabilities\n"); + } main_ctx = talloc_new(NULL); if (main_ctx == NULL) { From 6d079f183731b30e95e3d8044d1551896c69238a Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 31 Jan 2024 19:59:13 +0100 Subject: [PATCH 26/42] sssd.service: run under SSSD_USER by default :relnote: *IMPORTANT note for downstream maintainers!* This release features significant improvements of "running with less privileges (under unprivileged service user)" feature. There is still a ./configure option '--with-sssd-user=' available that allows downstream package maintainers to choose if support of non-root service user should be built. In case such support is built, a preferred way to configure service user is simply by starting SSSD under this user; for example, using 'User=/Group=' options of systemd sssd.service file. Upstream defaults are to build "--with-sssd-user=sssd" and to install systemd service with "User=/Group=sssd'. In this case, only several helper processes - 'ldap_child', 'krb5_child' and 'selinux_child' - are executed with elevated capabilities (that are now granted using fine grained file capabilities instead of SUID bit). All other SSSD components run without any capabilities. In this scenario it's still possible to re-configure SSSD to run under 'root' (if needed for some reason): besides changing "User/Group=" options, some other tweaks of systemd service files are required. Those tweaks are described in the comments in service files. If SSSD is built "--with-sssd-user=sssd" but configured to run under "root", it's still possible to use a legacy sssd.conf::user option to change a service user at runtime. This requires granting CAP_SET_UID/ CAP_SET_GID capabilities to sssd.service (again, read comments in the service file). User will be changed and all capabilities dropped immediately at startup. There should be no reason to prefer sssd.conf::user option over sssd.service::User option, barring very exotics setups where it's impossible to configure initial service user. Take a note, that this release deprecates sssd.conf::user option and its support might be removed in future releases. Further, doesn't matter if SSSD is built "--with-sssd-user=sssd" or "--with-sssd-user=root", when it's configured to run under "root" (in both cases) it still runs without capabilities, the same way as when it's configured to run under "sssd" user. The only difference is from DAC perspective. Important: owner of /etc/sssd/sssd.conf file (and snippets) should match user configured to start SSSD service. Upstream spec file changes ownership of existing sssd.conf to 'sssd' during package installation for seamless upgrades. Additionally, this release fixes a large number of issues with "socket activation of responder" feature, making it operable out-of-the-box when the package is built "--with-sssd-user=sssd". Please take a note, that user configured to run main sssd.service and socket activated responders (if used) should match (i.e. if sssd.service is re-configured from upstream defaults to 'root' then responders services also should be re-configured). Downstream package maintainers are advised to carefully inspect changes in contrib/sssd.spec.in, src/sysv/systemd/* and ./configure options that this release brings! --- Makefile.am | 18 +++++++++++------- src/sysv/systemd/sssd-kcm.service.in | 2 +- src/sysv/systemd/sssd.service.in | 7 +++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/Makefile.am b/Makefile.am index e572f94c0e6..b811ca2fdb2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -90,25 +90,29 @@ sssdkcmdatadir = $(datadir)/sssd-kcm deskprofilepath = $(sss_statedir)/deskprofile if HAVE_SYSTEMD_UNIT + ifp_dbus_exec_comment = \# If system is configured to use systemd ifp service ("SystemdService=") then "Exec=" and "User=" options are not used ifp_dbus_exec_cmd = $(sssdlibexecdir)/sssd_ifp --socket-activated ifp_systemdservice = SystemdService=sssd-ifp.service # SSSD requires a configuration file (either /etc/sssd/sssd.conf, # or some snippet under /etc/sssd/sssd.conf.d/) to be present. condconfigexists = ConditionPathExists=\|/etc/sssd/sssd.conf\nConditionDirectoryNotEmpty=\|/etc/sssd/conf.d/ -# If sssd is configured with --with-sssd-user= where !='root' -# but is actually run under the root we need CAP_DAC_OVERRIDE to access -# files owned by : -# If sssd is really run under non-root account that doesn't have this cap -# originally then it's addition to CapabilityBoundingSet doesn't matter. + if SSSD_NON_ROOT_USER -additional_caps = CAP_DAC_OVERRIDE +# If non-root service user is supported, monitor might need SET-ID to switch user (deprecated 'sssd.conf::user' option) +# but this is non default configuration, so 'AmbientCapabilities=' are commented out. +# Bounding set needs to list capabilities required by ldap/krb5/selinux_childs, otherwise they can't gain it. +capabilities = CapabilityBoundingSet= CAP_CHOWN CAP_DAC_OVERRIDE CAP_SETGID CAP_SETUID\n\# Uncomment if support of deprecated "sssd.conf::user" option is required:\n\#AmbientCapabilities= CAP_SETGID CAP_SETUID nss_service_user_group = User=$(SSSD_USER)\nGroup=$(SSSD_USER) nss_socket_user_group = SocketUser=$(SSSD_USER)\nSocketGroup=$(SSSD_USER) supplementary_groups = \# If service configured to be run under "root", uncomment "SupplementaryGroups"\n\#SupplementaryGroups=$(SSSD_USER) else +# If non-root service user isn't supported, monitor/sssd_be/responders don't need any effective capabilities +# but bounding set needs to list capabilities required by ldap/krb5/selinux_childs, otherwise they can't gain it. +capabilities = CapabilityBoundingSet= CAP_CHOWN CAP_DAC_OVERRIDE CAP_SETGID CAP_SETUID supplementary_groups = \# Note: SSSD package was built without support of running as non-privileged user endif # SSSD_NON_ROOT_USER + else ifp_dbus_exec_comment = \# "sss_signal" is used to force SSSD monitor to trigger "sssd_ifp" reconnection to dbus ifp_dbus_exec_cmd = $(sssdlibexecdir)/sss_signal @@ -5297,7 +5301,7 @@ edit_cmd = $(SED) \ -e 's|@prefix[@]|$(prefix)|g' \ -e 's|@SSSD_USER[@]|$(SSSD_USER)|g' \ -e 's|@condconfigexists[@]|$(condconfigexists)|g' \ - -e 's|@additional_caps[@]|$(additional_caps)|g' \ + -e 's|@capabilities[@]|$(capabilities)|g' \ -e 's|@nss_service_user_group[@]|$(nss_service_user_group)|g' \ -e 's|@nss_socket_user_group[@]|$(nss_socket_user_group)|g' \ -e 's|@supplementary_groups[@]|$(supplementary_groups)|g' diff --git a/src/sysv/systemd/sssd-kcm.service.in b/src/sysv/systemd/sssd-kcm.service.in index 1c6b914ab82..c76a10412c5 100644 --- a/src/sysv/systemd/sssd-kcm.service.in +++ b/src/sysv/systemd/sssd-kcm.service.in @@ -13,5 +13,5 @@ ExecStart=@libexecdir@/sssd/sssd_kcm ${DEBUG_LOGGER} # Currently SSSD KCM server ('sssd_kcm') always runs under 'root' # ('User=' and 'Group=' defaults to 'root' for system services) # 'CapabilityBoundingSet' is used to limit privileges set: -CapabilityBoundingSet= @additional_caps@ CAP_SETGID CAP_SETUID +CapabilityBoundingSet= CAP_SETGID CAP_SETUID @supplementary_groups@ diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in index 5c9942ed200..02d9c14b7f1 100644 --- a/src/sysv/systemd/sssd.service.in +++ b/src/sysv/systemd/sssd.service.in @@ -14,11 +14,10 @@ ExecStart=@sbindir@/sssd -i ${DEBUG_LOGGER} Type=notify NotifyAccess=main PIDFile=@pidpath@/sssd.pid -# Currently main SSSD process ('sssd') always runs under 'root' -# ('User=' and 'Group=' defaults to 'root' for system services) -# 'CapabilityBoundingSet' is used to limit privileges set: -CapabilityBoundingSet= @additional_caps@ CAP_CHOWN CAP_SETGID CAP_SETUID Restart=on-abnormal +@capabilities@ +User=@SSSD_USER@ +Group=@SSSD_USER@ @supplementary_groups@ [Install] From a51898d6aa5b59a2d7b5e630f2fcaac6e635a251 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Fri, 2 Feb 2024 14:50:50 +0100 Subject: [PATCH 27/42] SPEC: make sure cache files are accessible Since now SSSD starts and runs under %{sssd_user} by default, make sure cache files left from previous version are %{sssd_user}:%{sssd_user} owned. --- contrib/sssd.spec.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 265501968e8..b0a4ef31a4f 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -202,6 +202,7 @@ Requires: (libsss_autofs%{?_isa} = %{version}-%{release} if autofs) Requires: (sssd-nfs-idmap = %{version}-%{release} if libnfsidmap) Requires: libsss_idmap = %{version}-%{release} Requires: libsss_certmap = %{version}-%{release} +Requires(post): coreutils Requires(postun): coreutils %if 0%{?rhel} Requires(pre): shadow-utils @@ -1051,6 +1052,11 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us %systemd_post sssd-pam.socket %systemd_post sssd-ssh.socket %systemd_post sssd-sudo.socket +%__rm -f %{mcpath}/passwd +%__rm -f %{mcpath}/group +%__rm -f %{mcpath}/initgroups +%__rm -f %{mcpath}/sid +%__chown %{sssd_user}:%{sssd_user} %{dbpath}/* %preun common %systemd_preun sssd.service From 170d6ab26334a9f047e6e340eec3a4a4fb7b6af6 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 3 Feb 2024 11:46:56 +0100 Subject: [PATCH 28/42] SPEC: make sure config files are accesible Since now SSSD starts and runs under %{sssd_user} by default, make sure config files left from previous version are %{sssd_user}:%{sssd_user} owned. --- contrib/sssd.spec.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index b0a4ef31a4f..e0e69e5e422 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -1057,6 +1057,8 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us %__rm -f %{mcpath}/initgroups %__rm -f %{mcpath}/sid %__chown %{sssd_user}:%{sssd_user} %{dbpath}/* +%__chown %{sssd_user}:%{sssd_user} %{_sysconfdir}/sssd/sssd.conf +%__chown -R %{sssd_user}:%{sssd_user} %{_sysconfdir}/sssd/conf.d %preun common %systemd_preun sssd.service From 6487d5aed7cda55843133fedafc31a53eedc4ba1 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Fri, 2 Feb 2024 14:32:35 +0100 Subject: [PATCH 29/42] SYSTEMD: KCM capabilities 'sssd_kcm' doesn't need CAP CHOWN/SET-ID itself but needs to have it in bounding set so that 'krb5_child' run by 'sssd_kcm' can get those capabilities. CAP_DAC_OVERRIDE is used to access sssd.conf and log folder. The latter can be dropped once (if) 'sssd_kcm' is changed to run under 'sssd' user by default. An approach to use 'SupplementaryGroups=' isn't practical here because config files aren't readable by group and changing this in existing setups might be cumbersome. It should be easier to make 'sssd_kcm' to run under 'sssd' user. --- src/sysv/systemd/sssd-kcm.service.in | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sysv/systemd/sssd-kcm.service.in b/src/sysv/systemd/sssd-kcm.service.in index c76a10412c5..d33413e8877 100644 --- a/src/sysv/systemd/sssd-kcm.service.in +++ b/src/sysv/systemd/sssd-kcm.service.in @@ -12,6 +12,4 @@ Environment=DEBUG_LOGGER=--logger=files ExecStart=@libexecdir@/sssd/sssd_kcm ${DEBUG_LOGGER} # Currently SSSD KCM server ('sssd_kcm') always runs under 'root' # ('User=' and 'Group=' defaults to 'root' for system services) -# 'CapabilityBoundingSet' is used to limit privileges set: -CapabilityBoundingSet= CAP_SETGID CAP_SETUID -@supplementary_groups@ +CapabilityBoundingSet= CAP_DAC_OVERRIDE CAP_CHOWN CAP_SETGID CAP_SETUID From d59d16b11d828ecef7da201a7ecaa8eca2779a36 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 5 Feb 2024 14:53:58 +0100 Subject: [PATCH 30/42] SSS_INI: only check file ownership from 'sssd' User used to run 'sssctl', 'sssd_kcm', etc (typically root) might not match user configured to run SSSD service. --- src/tests/system/tests/test_sssctl.py | 53 --------------------------- src/util/sss_ini.c | 36 +++++++++++++++--- 2 files changed, 31 insertions(+), 58 deletions(-) diff --git a/src/tests/system/tests/test_sssctl.py b/src/tests/system/tests/test_sssctl.py index c575c941d8a..8bb5258f99f 100644 --- a/src/tests/system/tests/test_sssctl.py +++ b/src/tests/system/tests/test_sssctl.py @@ -546,32 +546,6 @@ def test_sssctl__verify_permission(client: Client): assert "File ownership and permissions check failed" in result.stdout, "Wrong error message on stdout" -@pytest.mark.tools -@pytest.mark.topology(KnownTopology.Client) -def test_sssctl__verify_ownership(client: Client): - """ - :title: Verify the ownership of default configuration file - :setup: - 1. Start SSSD, so default config is autimatically created - 2. Change ownership of default config file - :steps: - 1. Call sssctl config-check - 2. Check error message - :expectedresults: - 1. config-check detects an error - 2. Error message is properly set - :customerscenario: False - """ - client.local.user("user1").add() - client.local.group("group1").add() - client.sssd.common.local() - client.sssd.start() - client.fs.chown("/etc/sssd/sssd.conf", "user1", "group1") - result = client.sssctl.config_check() - assert result.rc != 0, "Config-check did not detect misconfigured config" - assert "File ownership and permissions check failed" in result.stdout, "Wrong error message on stdout" - - @pytest.mark.tools @pytest.mark.topology(KnownTopology.Client) def test_sssctl__verify_missing_closing_bracket(client: Client): @@ -837,33 +811,6 @@ def test_sssctl__non_default_config_location_permission(client: Client): assert "File ownership and permissions check failed" in result.stdout, "Wrong error message on stdout" -@pytest.mark.tools -@pytest.mark.ticket(bz=1723273) -@pytest.mark.topology(KnownTopology.Client) -def test_sssctl__non_default_config_location_ownership(client: Client): - """ - :title: sssctl config-check complains about proper ownership when config is non default - :setup: - 1. Copy sssd.conf file to different directory and set it wrong ownership - :steps: - 1. Call sssctl config-check on that different directory - 2. Check error message - :expectedresults: - 1. config-check failed - 2. Error message is properly set - :customerscenario: True - """ - client.local.user("user1").add() - client.sssd.common.local() - client.sssd.config_apply() - client.fs.mkdir("/tmp/test/") - client.fs.copy("/etc/sssd/sssd.conf", "/tmp/test/sssd.conf", user="user1") - - result = client.sssctl.config_check(config="/tmp/test/sssd.conf") - assert result.rc != 0, "Config-check successfully finished" - assert "File ownership and permissions check failed" in result.stdout, "Wrong error message on stdout" - - @pytest.mark.tools @pytest.mark.ticket(bz=1723273) @pytest.mark.topology(KnownTopology.Client) diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 37ae4c7bd8c..7f9824d8890 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -23,6 +23,8 @@ */ #include +#include +#include #include #include @@ -147,18 +149,39 @@ static int sss_ini_config_file_from_mem(struct sss_ini *self, /* 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, - INI_ACCESS_CHECK_MODE | - INI_ACCESS_CHECK_UID | - INI_ACCESS_CHECK_GID, + flags, geteuid(), getegid(), S_IRUSR, /* r**------ */ @@ -274,8 +297,11 @@ static int sss_ini_add_snippets(struct sss_ini *self, sss_ini_free_ra_messages(self); - snip_check.flags = INI_ACCESS_CHECK_MODE | INI_ACCESS_CHECK_UID - | INI_ACCESS_CHECK_GID; + 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**------ */ From ddbdd072e17738d198121cec0fe4ec032de8d2d5 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 5 Feb 2024 13:36:44 +0100 Subject: [PATCH 31/42] SYSTEMD: remove "PIDFile=" See https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html#PIDFile= ``` Note that PID files should be avoided in modern projects. Use Type=notify, Type=notify-reload or Type=simple where possible, which does not require use of PID files to determine the main process of a service and avoids needless forking. ``` SSSD uses "Type=notify" --- Makefile.am | 1 - src/sysv/systemd/sssd.service.in | 1 - 2 files changed, 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index b811ca2fdb2..0143ccfa8e3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5294,7 +5294,6 @@ edit_cmd = $(SED) \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ -e 's|@runstatedir[@]|$(runstatedir)|g' \ - -e 's|@pidpath[@]|$(pidpath)|g' \ -e 's|@logpath[@]|$(logpath)|g' \ -e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@pipepath[@]|$(pipepath)|g' \ diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in index 02d9c14b7f1..aa3b6871c70 100644 --- a/src/sysv/systemd/sssd.service.in +++ b/src/sysv/systemd/sssd.service.in @@ -13,7 +13,6 @@ EnvironmentFile=-@environment_file@ ExecStart=@sbindir@/sssd -i ${DEBUG_LOGGER} Type=notify NotifyAccess=main -PIDFile=@pidpath@/sssd.pid Restart=on-abnormal @capabilities@ User=@SSSD_USER@ From fb9bf1008f01e304dccde035c88e4950cb6158e6 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 6 Feb 2024 11:41:57 +0100 Subject: [PATCH 32/42] CONF: store pid file in /run/sssd instead of /var/run. SSSD run under non-privileged user can't write to /var/run directly. --- Makefile.am | 1 + contrib/sssd.spec.in | 4 ++-- src/conf_macros.m4 | 6 +++--- src/sysv/sssd.in | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index 0143ccfa8e3..0d866e6edb0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4085,6 +4085,7 @@ intgcheck-prepare: $(abs_top_srcdir)/configure \ --prefix="$$prefix" \ --with-ldb-lib-dir="$$prefix"/lib/ldb \ + --with-pid-path="$$prefix"/run/sssd \ --enable-intgcheck-reqs \ --without-semanage \ --with-files-provider \ diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index e0e69e5e422..6ba72a0c26e 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -563,7 +563,6 @@ autoreconf -ivf --with-initscript=systemd \ --with-krb5-rcache-dir=%{_localstatedir}/cache/krb5rcache \ --with-mcache-path=%{mcpath} \ - --with-pid-path=%{_rundir} \ --with-pipe-path=%{pipepath} \ --with-pubconf-path=%{pubconfpath} \ --with-sssd-user=%{sssd_user} \ @@ -786,7 +785,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %{_sbindir}/sss_cache %{_libexecdir}/%{servicename}/sss_signal -%dir %{sssdstatedir} +%attr(775,%{sssd_user},%{sssd_user}) %dir %{sssdstatedir} %dir %{_localstatedir}/cache/krb5rcache %attr(770,%{sssd_user},%{sssd_user}) %dir %{dbpath} %attr(775,%{sssd_user},%{sssd_user}) %dir %{mcpath} @@ -806,6 +805,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %dir %{_sysconfdir}/rwtab.d %config(noreplace) %{_sysconfdir}/rwtab.d/sssd %dir %{_datadir}/sssd +%attr(775,%{sssd_user},%{sssd_user}) %dir %{_rundir}/sssd %config(noreplace) %{_sysconfdir}/pam.d/sssd-shadowutils %dir %{_libdir}/%{name}/conf %{_libdir}/%{name}/conf/sssd.conf diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index e37dcd3fee3..c1340238ed4 100644 --- a/src/conf_macros.m4 +++ b/src/conf_macros.m4 @@ -35,12 +35,12 @@ AC_DEFUN([WITH_PLUGIN_PATH], AC_DEFUN([WITH_PID_PATH], [ AC_ARG_WITH([pid-path], [AC_HELP_STRING([--with-pid-path=PATH], - [Where to store pid files for the SSSD [/var/run]] + [Where to store pid files for the SSSD [/run/sssd/]] ) ] ) - config_pidpath="\"VARDIR\"/run" - pidpath="${localstatedir}/run" + config_pidpath="/run/sssd" + pidpath="/run/sssd" if test x"$with_pid_path" != x; then config_pidpath=$with_pid_path pidpath=$with_pid_path diff --git a/src/sysv/sssd.in b/src/sysv/sssd.in index 385785e02b6..68485bfb881 100644 --- a/src/sysv/sssd.in +++ b/src/sysv/sssd.in @@ -38,7 +38,7 @@ fi SSSD=@sbindir@/sssd LOCK_FILE=@localstatedir@/lock/subsys/sssd -PID_FILE=@localstatedir@/run/sssd.pid +PID_FILE=/run/sssd/sssd.pid TIMEOUT=15 From 76172d8357d59f967fd94829c7a9002277db4bd7 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 20 Feb 2024 10:55:40 +0100 Subject: [PATCH 33/42] UTILS: make pidfile readable by everyone --- src/util/server.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/server.c b/src/util/server.c index ac59f918ca6..fcc159f86b4 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -211,13 +211,16 @@ int pidfile(const char *file) int ret, err; size_t size; ssize_t written; + mode_t old_umask; ret = check_pidfile(file); if (ret != EOK) { return ret; } + old_umask = umask(0133); fd = open(file, O_CREAT | O_WRONLY | O_EXCL, 0644); + umask(old_umask); err = errno; if (fd == -1) { return err; From 1a0ca14162e91a20564fb8ec660538db701e8022 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 6 Feb 2024 15:38:26 +0100 Subject: [PATCH 34/42] SPEC: replace SUID bit with more fine-grained capabilities This will also allow to use "SecureBits=noroot" in sssd.service --- contrib/sssd.spec.in | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 6ba72a0c26e..923df42cb29 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -14,12 +14,8 @@ %global use_sysusers 0 %endif -# Set setuid bit on child helpers if we support non-root user. -%if "%{sssd_user}" == "root" -%global child_attrs 0750 -%else -%global child_attrs 4750 -%endif +# Capabilities of privileged child helpers (required even if SSSD runs under root) +%global child_capabilities cap_chown,cap_dac_override,cap_setuid,cap_setgid=ep %if 0%{?fedora} >= 35 || 0%{?rhel} >= 9 %global build_subid 1 @@ -849,8 +845,8 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %files krb5-common %license COPYING %attr(775,%{sssd_user},%{sssd_user}) %dir %{pubconfpath}/krb5.include.d -%attr(%{child_attrs},root,%{sssd_user}) %{_libexecdir}/%{servicename}/ldap_child -%attr(%{child_attrs},root,%{sssd_user}) %{_libexecdir}/%{servicename}/krb5_child +%attr(0750,root,%{sssd_user}) %caps(%{child_capabilities}) %{_libexecdir}/%{servicename}/ldap_child +%attr(0750,root,%{sssd_user}) %caps(%{child_capabilities}) %{_libexecdir}/%{servicename}/krb5_child %files krb5 -f sssd_krb5.lang %license COPYING @@ -868,7 +864,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %license COPYING %attr(770,%{sssd_user},%{sssd_user}) %dir %{keytabdir} %{_libdir}/%{name}/libsss_ipa.so -%attr(%{child_attrs},root,%{sssd_user}) %{_libexecdir}/%{servicename}/selinux_child +%attr(0750,root,%{sssd_user}) %caps(%{child_capabilities}) %{_libexecdir}/%{servicename}/selinux_child %{_mandir}/man5/sssd-ipa.5* %files ad -f sssd_ad.lang From 758ed759ec86073ab81bc8d0246a9faac2dbf573 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 6 Feb 2024 19:56:40 +0100 Subject: [PATCH 35/42] SYSTEMD: set "SecureBits=noroot noroot-locked" in sssd.service to avoid processes gaining all capabilities from bounding set during execv() with uid=0/gid=0 (so that, for example, 'sssd_be' runs without capabilities even if "User=root") --- src/sysv/systemd/sssd.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in index aa3b6871c70..f982ef263f6 100644 --- a/src/sysv/systemd/sssd.service.in +++ b/src/sysv/systemd/sssd.service.in @@ -15,6 +15,7 @@ Type=notify NotifyAccess=main Restart=on-abnormal @capabilities@ +SecureBits=noroot noroot-locked User=@SSSD_USER@ Group=@SSSD_USER@ @supplementary_groups@ From d0b529dd5783eab867425c54b5cce6ce4f49d0c7 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 7 Feb 2024 21:15:11 +0100 Subject: [PATCH 36/42] SPEC: make conf folder g+rx so that SSSD built --with-sssd-user=sssd but run under 'root' can get to sssd.conf without capabilities (using "SupplementaryGroups=sssd") sssd.conf still needs to be chown'ed to 'root:root' manually in this case. --- contrib/sssd.spec.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 923df42cb29..2745b3704e7 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -792,9 +792,9 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %attr(775,%{sssd_user},%{sssd_user}) %dir %{pubconfpath} %attr(770,%{sssd_user},%{sssd_user}) %dir %{gpocachepath} %attr(770,%{sssd_user},%{sssd_user}) %dir %{_var}/log/%{name} -%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 +%attr(750,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd +%attr(750,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/conf.d +%attr(750,%{sssd_user},%{sssd_user}) %dir %{_sysconfdir}/sssd/pki %ghost %attr(0600,%{sssd_user},%{sssd_user}) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf %dir %{_sysconfdir}/logrotate.d %config(noreplace) %{_sysconfdir}/logrotate.d/sssd From 8cda9b7f9178677341745fed6da42f70e07e6ed2 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 12 Feb 2024 19:43:25 +0100 Subject: [PATCH 37/42] TESTS: system: skip 'passkey' tests if SSSD runs under non-root For a real device this is handled by udev rule that makes device readable by SSSD. This rule doesn't work with mocked device. --- src/tests/system/tests/test_passkey.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/tests/system/tests/test_passkey.py b/src/tests/system/tests/test_passkey.py index 3ce26a68a38..9f0e9c448a2 100644 --- a/src/tests/system/tests/test_passkey.py +++ b/src/tests/system/tests/test_passkey.py @@ -19,6 +19,13 @@ from sssd_test_framework.topology import KnownTopology, KnownTopologyGroup +def passkey_requires_root(client: Client) -> tuple[bool, str] | bool: + if client.svc.get_property("sssd", "User") != "root": + return False, "Passkey tests don't work if SSSD runs under non-root" + + return True + + @pytest.mark.importance("high") @pytest.mark.topology(KnownTopology.Client) @pytest.mark.builtwith(client="passkey") @@ -82,6 +89,7 @@ def test_passkey__register__ipa(ipa: IPA, moduledatadir: str, testdatadir: str): @pytest.mark.importance("critical") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__su(client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str): """ :title: Check su authentication of user with LDAP, IPA, AD and Samba @@ -116,6 +124,7 @@ def test_passkey__su(client: Client, provider: GenericProvider, moduledatadir: s @pytest.mark.importance("high") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__su_fail_pin(client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str): """ :title: Check su authentication deny of user with LDAP, IPA, AD and Samba with incorrect pin @@ -150,6 +159,7 @@ def test_passkey__su_fail_pin(client: Client, provider: GenericProvider, moduled @pytest.mark.importance("critical") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__su_fail_mapping(client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str): """ :title: Check su authentication deny of user with LDAP, IPA, AD and Samba with incorrect mapping @@ -186,6 +196,7 @@ def test_passkey__su_fail_mapping(client: Client, provider: GenericProvider, mod @pytest.mark.importance("high") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__su_srv_not_resolvable( client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str ): @@ -246,6 +257,7 @@ def test_passkey__su_srv_not_resolvable( @pytest.mark.importance("high") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__offline_su(client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str): """ :title: Check offline su authentication of a user with LDAP, IPA, AD and Samba @@ -339,6 +351,7 @@ def test_passkey__user_fetch_from_cache( @pytest.mark.importance("high") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__su_multi_keys_for_same_user( client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str ): @@ -378,6 +391,7 @@ def test_passkey__su_multi_keys_for_same_user( @pytest.mark.importance("high") @pytest.mark.topology(KnownTopologyGroup.AnyProvider) @pytest.mark.builtwith(client="passkey", provider="passkey") +@pytest.mark.require.with_args(passkey_requires_root) def test_passkey__su_same_key_for_multi_user( client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str ): From f3c67be97509a85db3c849a132ea78d4e3877eb8 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Fri, 1 Sep 2023 19:39:40 +0200 Subject: [PATCH 38/42] SPEC: build Fedora >= 41 package with sssd user support --- contrib/fedora/bashrc_sssd | 1 + contrib/sssd.spec.in | 14 ++++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/contrib/fedora/bashrc_sssd b/contrib/fedora/bashrc_sssd index 8fb652497a6..06d585c2495 100644 --- a/contrib/fedora/bashrc_sssd +++ b/contrib/fedora/bashrc_sssd @@ -51,6 +51,7 @@ fedconfig() --with-test-dir=/dev/shm \ --cache-file=/tmp/fedconfig.cache \ --with-passkey \ + --with-sssd-user=sssd \ ${SSSD_NO_MANPAGES-} \ "$@" } diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 2745b3704e7..042e66260c9 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -1,9 +1,11 @@ # SSSD SPEC file for Fedora 34+ and RHEL-9+ # define SSSD user -%if 0%{?rhel} +%if 0%{?fedora} >= 41 || 0%{?rhel} +%global use_sssd_user 1 %global sssd_user sssd %else +%global use_sssd_user 0 %global sssd_user root %endif @@ -200,7 +202,7 @@ Requires: libsss_idmap = %{version}-%{release} Requires: libsss_certmap = %{version}-%{release} Requires(post): coreutils Requires(postun): coreutils -%if 0%{?rhel} +%if %{use_sssd_user} Requires(pre): shadow-utils %endif %{?systemd_requires} @@ -449,7 +451,7 @@ Requires: sssd-common = %{version}-%{release} Provides the D-Bus responder of the SSSD, called the InfoPipe, that allows the information from the SSSD to be transmitted over the system bus. -%if 0%{?rhel} +%if %{use_sssd_user} %package polkit-rules Summary: Rules for polkit integration for SSSD Group: Applications/System @@ -567,7 +569,7 @@ autoreconf -ivf %if %{build_subid} --with-subid \ %endif -%if 0%{?fedora} +%if ! %{use_sssd_user} --disable-polkit-rules-path \ %endif %if %{build_passkey} @@ -831,7 +833,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %endif -%if 0%{?rhel} +%if %{use_sssd_user} %files polkit-rules %{_datadir}/polkit-1/rules.d/* %endif @@ -1030,7 +1032,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %config(noreplace) %{_sysconfdir}/krb5.conf.d/sssd_enable_passkey %endif -%if 0%{?rhel} +%if %{use_sssd_user} %pre common %if %{use_sysusers} %sysusers_create_compat contrib/sssd.sysusers From f4fdedbc8fcbf8389d1ea65eff298b8296d23b9e Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 20 Feb 2024 20:50:28 +0100 Subject: [PATCH 39/42] SSSDConfig: chown() sssd.conf to SSSD service user --- src/config/SSSDConfig/__init__.py.in | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 225b92fdc95..006e20903e8 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -6,7 +6,10 @@ Created on Sep 18, 2009 import os import re +import shutil +import subprocess import sys +from contextlib import suppress from .sssdoptions import SSSDOptions from .ipachangeconf import SSSDChangeConf @@ -1063,6 +1066,14 @@ class SSSDConfig(SSSDChangeConf): output = self.dump(self.opts) of.write(output) os.umask(old_umask) + service_user = "" + ret = subprocess.run(["systemctl", "show", "sssd", "--value", "--property", "User"], capture_output=True, text=True) + if ret.returncode == 0: + service_user = ret.stdout.strip() + if service_user == "": + service_user = "root" + with suppress(PermissionError): + shutil.chown(outputfile, service_user, service_user) def list_active_services(self): """ From 291a4e738fcf86d8060148f97a59d8654bb6870e Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 7 Mar 2024 17:03:27 +0100 Subject: [PATCH 40/42] MONITOR: free 'tmp_ctx' in case of failure too --- src/monitor/monitor.c | 63 +++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 732a287023c..78c3d0ae00f 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1979,11 +1979,13 @@ int main(int argc, const char *argv[]) tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { - return 2; + ret = 2; + goto out; } monitor = talloc_zero(tmp_ctx, struct mt_ctx); if (monitor == NULL) { - return 2; + ret = 2; + goto out; } talloc_set_destructor((TALLOC_CTX *)monitor, monitor_ctx_destructor); @@ -2011,13 +2013,15 @@ int main(int argc, const char *argv[]) ERROR("\nInvalid option %s: %s\n\n", poptBadOption(pc, 0), poptStrerror(opt)); poptPrintUsage(pc, stderr, 0); - return 1; + ret = 1; + goto out; } } if (opt_version) { puts(VERSION""PRERELEASE_VERSION); - return EXIT_SUCCESS; + ret = EXIT_SUCCESS; + goto out; } /* If the level or timestamps was passed at the command-line, we want @@ -2030,7 +2034,8 @@ int main(int argc, const char *argv[]) if (opt_daemon && opt_interactive) { ERROR("Option -i|--interactive is not allowed together with -D|--daemon\n"); poptPrintUsage(pc, stderr, 0); - return 1; + ret = 1; + goto out; } if (!opt_daemon && !opt_interactive) { @@ -2051,12 +2056,14 @@ int main(int argc, const char *argv[]) config_file = talloc_strdup(tmp_ctx, SSSD_CONFIG_FILE); } if (config_file == NULL) { - return 2; + ret = 2; + goto out; } cdb_file = talloc_asprintf(tmp_ctx, "%s/%s", DB_PATH, CONFDB_FILE); if (cdb_file == NULL) { - return 2; + ret = 2; + goto out; } poptFreeContext(pc); @@ -2068,7 +2075,8 @@ int main(int argc, const char *argv[]) ret = sss_log_caps_to_str(true, &initial_caps); if (ret != 0) { ERROR("Failed to get initial capabilities\n"); - return 3; + ret = 3; + goto out; } #ifndef SSSD_NON_ROOT_USER @@ -2079,7 +2087,8 @@ int main(int argc, const char *argv[]) "Can't run under non-root"); ERROR("Non-root service user support isn't built. " "Can't run under %"SPRIuid":%"SPRIgid"\n", euid, egid); - return 1; + ret = 1; + goto out; } /* Everything is root:root owned. No caps required. */ if (initial_caps != NULL) { @@ -2096,19 +2105,22 @@ int main(int argc, const char *argv[]) ERROR("Can't read config: '%s'\n", sss_strerror(ret)); sss_log(SSS_LOG_ALERT, "Failed to read configuration: '%s'", sss_strerror(ret)); - return 3; + ret = 3; + goto out; } ret = get_service_user(config, monitor); if (ret != EOK) { - return 4; + ret = 4; /* Error message already logged */ + goto out; } ret = bootstrap_monitor_process(monitor->uid, monitor->gid); if (ret != 0) { ERROR("Failed to boostrap SSSD 'monitor' process: %s", sss_strerror(ret)); sss_log(SSS_LOG_ALERT, "Failed to boostrap SSSD 'monitor' process."); - return 5; + ret = 5; + goto out; } get_debug_level(config); @@ -2133,7 +2145,8 @@ int main(int argc, const char *argv[]) if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "SSSD is already running: pidfile exists at '"SSSD_PIDFILE"'\n"); - return 6; + ret = 6; + goto out; } } @@ -2141,13 +2154,18 @@ int main(int argc, const char *argv[]) /* set up things like debug, signals, daemonization, etc. */ ret = close(STDIN_FILENO); - if (ret != EOK) return 7; + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Failed to close stdin [%d]\n", errno); + ret = 7; + goto out; + } ret = confdb_write_ini(tmp_ctx, config, cdb_file, false, false, &monitor->cdb); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to write config DB: '%s'\n", sss_strerror(ret)); - return 8; + ret = 8; + goto out; } /* Use confdb initialized in server_setup. ldb_tdb module (1.4.0) check PID @@ -2159,7 +2177,10 @@ int main(int argc, const char *argv[]) ret = server_setup(SSSD_MONITOR_NAME, false, flags, CONFDB_FILE, CONFDB_MONITOR_CONF_ENTRY, &main_ctx, false); - if (ret != EOK) return 9; + if (ret != EOK) { + ret = 9; /* Error message should be already logged */ + goto out; + } monitor->cdb = main_ctx->confdb_ctx; get_monitor_config(monitor); @@ -2172,7 +2193,8 @@ int main(int argc, const char *argv[]) if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "monitor_process_init() failed: '%s'\n", sss_strerror(ret)); - return 10; + ret = 10; + goto out; } talloc_free(tmp_ctx); @@ -2184,4 +2206,11 @@ int main(int argc, const char *argv[]) if (ret != EOK) return 12; return 0; + +out: + talloc_free(tmp_ctx); + if (ret == 2) { + ERROR("Out of memory\n"); + } + return ret; } From 3ff5b7bb63d8d4ca1feb9f30ebb3bf9e9cfa9fd1 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 11 Mar 2024 18:41:27 +0100 Subject: [PATCH 41/42] MAN: 'monitor' exit codes description --- src/man/sssd.8.xml | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/man/sssd.8.xml b/src/man/sssd.8.xml index 8647277be2a..867d97274dd 100644 --- a/src/man/sssd.8.xml +++ b/src/man/sssd.8.xml @@ -207,6 +207,53 @@ + + EXIT STATUS + + + 0 + + + SSSD was shutdown gracefully. + + + + + 1 + + + Bad configuration or command line option. + + + + + 2 + + + Memory allocation error. + + + + + 6 + + + SSSD is already running. + + + + + Other codes + + + Other codes denote different errors, most probably about missing + required access rights. See SSSD and system logs for details. + + + + + + NOTES From 0981e0c5d9836d9ede367780e112b35f1755f2b1 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 13 Mar 2024 14:14:06 +0100 Subject: [PATCH 42/42] SPEC/SYSTEMD: try harder making sure logs ownership matches service user --- contrib/sssd.spec.in | 1 + src/sysv/systemd/sssd-nss.service.in | 1 + 2 files changed, 2 insertions(+) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 042e66260c9..d137c092cdb 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -1057,6 +1057,7 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us %__chown %{sssd_user}:%{sssd_user} %{dbpath}/* %__chown %{sssd_user}:%{sssd_user} %{_sysconfdir}/sssd/sssd.conf %__chown -R %{sssd_user}:%{sssd_user} %{_sysconfdir}/sssd/conf.d +%__chown %{sssd_user}:%{sssd_user} %{_var}/log/%{name}/*.log %preun common %systemd_preun sssd.service diff --git a/src/sysv/systemd/sssd-nss.service.in b/src/sysv/systemd/sssd-nss.service.in index bea93d192a5..3da897c4d65 100644 --- a/src/sysv/systemd/sssd-nss.service.in +++ b/src/sysv/systemd/sssd-nss.service.in @@ -11,6 +11,7 @@ Also=sssd-nss.socket [Service] Environment=DEBUG_LOGGER=--logger=files EnvironmentFile=-@environment_file@ +ExecStartPre=+-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_nss.log ExecStart=@libexecdir@/sssd/sssd_nss ${DEBUG_LOGGER} --socket-activated # No capabilities: CapabilityBoundingSet=