From 3893c669429ec78fd1f47f5793297b3c6bf21cc4 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 14:22:52 +0200 Subject: [PATCH 01/36] 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 9ffbc003546f6fb83d8904692f9112717029b547 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 14:24:37 +0200 Subject: [PATCH 02/36] 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 0b296252d86f204d2e1071218518222c55f05f8a Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 14:57:04 +0200 Subject: [PATCH 03/36] 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 df5a1e58fc389d1f374ee548c9056ef01f40a3a2 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 15:07:45 +0200 Subject: [PATCH 04/36] 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 8c7a1330812630d79ee2d0f8e98fbe0d177a292a Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 9 Sep 2023 15:10:16 +0200 Subject: [PATCH 05/36] MONITOR: get rid of unsed FLAGS_GEN_CONF definition --- src/monitor/monitor.c | 1 - src/util/util.h | 1 - 2 files changed, 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..1c2a42b09eb 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -109,7 +109,6 @@ extern int socket_activated; #define FLAGS_DAEMON 0x0001 #define FLAGS_INTERACTIVE 0x0002 #define FLAGS_PID_FILE 0x0004 -#define FLAGS_GEN_CONF 0x0008 #define FLAGS_NO_WATCHDOG 0x0010 enum sssd_exit_status { From c3d5361f8c6c1cbf39e6faaaf15d6384d60f7a98 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 31 Aug 2023 17:15:13 +0200 Subject: [PATCH 06/36] 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 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 5f22288fe42..16503964349 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -785,15 +785,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(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 From 6af79b79ccdcd837ccd778bea36cdd8c4c573541 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 4 Jan 2024 13:40:41 +0100 Subject: [PATCH 07/36] 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 16503964349..aebb1a9637d 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -790,7 +790,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %attr(700,root,root) %dir %{secdbpath} %attr(751,%{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 cef64eadb3c5c11eb1b389a8fcb168ae0f2edcf4 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 31 Aug 2023 17:15:13 +0200 Subject: [PATCH 08/36] 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 2ed5a866b74..46ef95a8865 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@ @@ -1511,6 +1514,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 \ @@ -5279,7 +5283,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 1453995de34e70e06a4c72b15c447a900970f6be Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 30 Jan 2024 21:13:42 +0100 Subject: [PATCH 09/36] 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 bf38e0773c110124c3a32ef5bfec6e3f5561e9b5 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 2 Sep 2023 14:51:52 +0200 Subject: [PATCH 10/36] 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 46ef95a8865..ae8893542b8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1289,6 +1289,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 \ @@ -1305,6 +1306,7 @@ libsss_util_la_CFLAGS = \ libsss_util_la_LIBADD = \ $(LIBADD_TIMER) \ $(SSSD_LIBS) \ + $(CAP_LIBS) \ $(SYSTEMD_LOGIN_LIBS) \ $(UNICODE_LIBS) \ $(PCRE_LIBS) \ @@ -4717,6 +4719,7 @@ krb5_child_LDADD = \ $(CLIENT_LIBS) \ $(SYSTEMD_LOGIN_LIBS) \ $(JANSSON_LIBS) \ + $(CAP_LIBS) \ $(NULL) ldap_child_SOURCES = \ @@ -4742,6 +4745,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 aebb1a9637d..70c4a92c1a8 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 1c2a42b09eb..bfef4bb8aba 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -751,6 +752,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 e2d4d448515b6a9ad4293d1093830bcc8ce11711 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 13 Sep 2023 18:59:46 +0200 Subject: [PATCH 11/36] 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 ae8893542b8..4447e4ab9f3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5695,7 +5695,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 a97cb6cc2e379ff02d6f154ce5b7c311fbf02dd9 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 12 Oct 2023 19:36:32 +0200 Subject: [PATCH 12/36] 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 cb55dbbf3d7f7c3d48db8552b27ad08177846284 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 14 Sep 2023 13:54:53 +0200 Subject: [PATCH 13/36] 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 | 128 ++++++++++++++++++++------------------ 1 file changed, 67 insertions(+), 61 deletions(-) diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index ca75a0dcbd8..5eb52b565bc 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -44,6 +44,35 @@ "\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 +129,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 +158,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 +175,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 +235,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 +284,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 +299,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 b3f22da4344d45b868a603bc045b9e9df5b64831 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 11:23:20 +0200 Subject: [PATCH 14/36] 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 5eb52b565bc..680fe13cae5 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -73,41 +73,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; @@ -202,13 +167,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; @@ -243,9 +201,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); @@ -254,16 +210,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); @@ -273,14 +224,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 b9baa7cecff32d0f31873bf0793fee5c324ca344 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 12:42:48 +0200 Subject: [PATCH 15/36] 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 cafd6dc92a7d0ae05d7e1ddb2e3acbcad84e424e Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 13:25:27 +0200 Subject: [PATCH 16/36] 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 5e31846a2c64d7da49645e87d92c7e8014fee584 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:00:16 +0200 Subject: [PATCH 17/36] 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 d39095b94bb6afa5b8b0f449254ff9ac60ff47fb Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:16:48 +0200 Subject: [PATCH 18/36] 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 175f14f87a250e91ae3699759fa4cea1ea1e9add Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 18 Oct 2023 20:50:47 +0200 Subject: [PATCH 19/36] 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 | 2 +- src/util/sss_ini.h | 2 +- 3 files changed, 4 insertions(+), 3 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..e4233559a9d 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -395,7 +395,7 @@ 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, +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..a41e1c96c39 100644 --- a/src/util/sss_ini.h +++ b/src/util/sss_ini.h @@ -136,7 +136,7 @@ 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, +char *sss_ini_get_string_config_value(struct sss_ini *self, int *error); /** From 8bea829ba9af8f19cf1911e17656aae22b7ebe82 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 21 Oct 2023 22:54:11 +0200 Subject: [PATCH 20/36] DEBUG: changed verbosity of: - 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 e4233559a9d..cf7a1879b9f 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 4ed00ad55c9d72210f00340e3e00fc9496995da6 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 15 Jan 2024 16:32:18 +0100 Subject: [PATCH 21/36] 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 7c1c39e2bfe45ff8ba18a402031c33a7ab9f7ba5 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 30 Jan 2024 21:36:17 +0100 Subject: [PATCH 22/36] 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 69aa5100821b354fc0a9c21e3d64d55b5accfa84 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 31 Jan 2024 10:48:12 +0100 Subject: [PATCH 23/36] 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 8a00fd5dec609ca66c372f382bbd4db6e634cf0b Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:24:56 +0200 Subject: [PATCH 24/36] MONITOR: changed startup logic to: (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 680fe13cae5..11fd6d6960e 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -68,8 +68,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 6aeb362557e..d3add9aa4d7 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 cf7a1879b9f..0de36f085c3 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 bfef4bb8aba..265488da8d2 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -389,8 +389,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, @@ -754,6 +752,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 977dd0922211595ab040d97e8029485f89b90e12 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 1 Feb 2024 11:44:57 +0100 Subject: [PATCH 25/36] 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 4447e4ab9f3..06ca1de1414 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4691,6 +4691,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 \ @@ -4733,6 +4734,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 141aeb69559..292a0fd264a 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -4022,6 +4022,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 @@ -4118,7 +4119,19 @@ int main(int argc, const char *argv[]) DEBUG_INIT(debug_level, opt_logger); - DEBUG(SSSDBG_TRACE_FUNC, "krb5_child started.\n"); + DEBUG(SSSDBG_IMPORTANT_INFO, + "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_IMPORTANT_INFO, "With following capabilities:\n%s", + caps ? caps : " (nothing)\n"); + talloc_free(caps); + } else { + DEBUG(SSSDBG_IMPORTANT_INFO, "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..eb6bb4746ab 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_IMPORTANT_INFO, + "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_IMPORTANT_INFO, "With following capabilities:\n%s", + caps ? caps : " (nothing)\n"); + talloc_free(caps); + } else { + DEBUG(SSSDBG_IMPORTANT_INFO, "Failed to get current capabilities\n"); + } main_ctx = talloc_new(NULL); if (main_ctx == NULL) { From ea7274e57166b22a2bd7b7db8ca8cc367567a3fd Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 31 Jan 2024 19:59:13 +0100 Subject: [PATCH 26/36] 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 startup. This requires granting CAP_SET_UID/ CAP_SET_GID capabilities to sssd.service (again, read comments in the service file). 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 and it's 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 06ca1de1414..ce7612b0478 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 @@ -5287,7 +5291,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 9004a41f9ed22c6e85b4cd353df695af2136fca7 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Fri, 2 Feb 2024 14:50:50 +0100 Subject: [PATCH 27/36] 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 70c4a92c1a8..bda413560ff 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 @@ -1049,6 +1050,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 e2c4b2165009ce48ce9bc8428fdabec92d8d01fc Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Sat, 3 Feb 2024 11:46:56 +0100 Subject: [PATCH 28/36] 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 bda413560ff..bba1bc52381 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -1055,6 +1055,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 fd5986a5178b3f89f553237ee0d61cbc30350928 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Fri, 2 Feb 2024 14:32:35 +0100 Subject: [PATCH 29/36] 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 7cdbdb8e6f98d367d9b8e9a01fa2370abceddfa7 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 5 Feb 2024 14:53:58 +0100 Subject: [PATCH 30/36] 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 0de36f085c3..7a9551a12ed 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 36018c107579204f7bb1fc8915bf5951feb753c0 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 5 Feb 2024 13:36:44 +0100 Subject: [PATCH 31/36] 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 ce7612b0478..d8c58c54d40 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5284,7 +5284,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 d9cd8eff4e99e615ea8f7a73153da470ef40c832 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 6 Feb 2024 11:41:57 +0100 Subject: [PATCH 32/36] CONF: store pid file in /var/lib/sss instead of /var/run. SSSD run under non-privileged user can't write to /var/run. Anyway this file is for internal use only, systemd doesn't need it. --- contrib/sssd.spec.in | 3 +-- src/conf_macros.m4 | 6 +++--- src/sysv/sssd.in | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index bba1bc52381..45012af4b95 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} \ @@ -785,7 +784,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} diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index e37dcd3fee3..59878b8bbca 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 [/var/lib/sss/]] ) ] ) - config_pidpath="\"VARDIR\"/run" - pidpath="${localstatedir}/run" + config_pidpath="\"SSS_STATEDIR\"/" + pidpath="${localstatedir}/lib/sss/" 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..10722737cf7 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=@localstatedir@/lib/sss/sssd.pid TIMEOUT=15 From 33e220d9498c8e77ad5fa51958454aa5b9f4a9f9 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 6 Feb 2024 15:38:26 +0100 Subject: [PATCH 33/36] 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 45012af4b95..331e5f54d2b 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 @@ -846,8 +842,8 @@ 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(%{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 @@ -865,7 +861,7 @@ install -D -p -m 0644 contrib/sssd.sysusers %{buildroot}%{_sysusersdir}/sssd.con %license COPYING %attr(700,%{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 7984d70c9b07909d6d8dab0375800eec2a486a0e Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 6 Feb 2024 19:56:40 +0100 Subject: [PATCH 34/36] 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 7e319818542af4fb26eb5159a0bd519fcf029365 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 7 Feb 2024 21:15:11 +0100 Subject: [PATCH 35/36] 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 331e5f54d2b..41657c67549 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -791,9 +791,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 8fd4c874cac784948a81891d30d935e2fff2f023 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 12 Feb 2024 19:43:25 +0100 Subject: [PATCH 36/36] 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 | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/tests/system/tests/test_passkey.py b/src/tests/system/tests/test_passkey.py index 3ce26a68a38..a64819b696e 100644 --- a/src/tests/system/tests/test_passkey.py +++ b/src/tests/system/tests/test_passkey.py @@ -82,6 +82,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-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 +120,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-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 +158,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-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 +198,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-root", +) def test_passkey__su_srv_not_resolvable( client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str ): @@ -246,6 +262,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-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 +359,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-root", +) def test_passkey__su_multi_keys_for_same_user( client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str ): @@ -378,6 +402,10 @@ 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( + lambda client: (client.svc.get_property("sssd", "User") == "root"), + "Currently passkey tests don't work if SSSD runs under non-root", +) def test_passkey__su_same_key_for_multi_user( client: Client, provider: GenericProvider, moduledatadir: str, testdatadir: str ):