From d46e243acc6a2dd8dcb0d02052dca04134486e53 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 16 Oct 2023 14:24:56 +0200 Subject: [PATCH] MONITOR: todo --- src/confdb/confdb.c | 14 +- src/confdb/confdb_setup.h | 14 ++ src/monitor/monitor.c | 234 ++++++++++++++------------------ src/monitor/monitor_bootstrap.c | 2 +- src/util/server.c | 5 +- src/util/usertools.c | 38 ------ src/util/util.h | 2 - 7 files changed, 119 insertions(+), 190 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 34e0514af2b..c3ee7803334 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -648,8 +648,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) @@ -682,19 +680,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.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/monitor/monitor.c b/src/monitor/monitor.c index d724d4a3cfe..3d37464ebe4 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -69,11 +69,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; @@ -820,7 +815,7 @@ 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; @@ -828,21 +823,24 @@ static int get_service_user(struct mt_ctx *ctx) ctx->gid = 0; #ifdef SSSD_NON_ROOT_USER - char *user_str = NULL; + const 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, CONFDB_MONITOR_CONF_ENTRY, + CONFDB_MONITOR_USER_RUNAS); + if (ret != 0) { + ERROR("Failed to get the user to run as\n"); return ret; } + if (config->obj != NULL) { + user_str = sss_ini_get_string_config_value(config, NULL); + } - if (strcmp(user_str, SSSD_USER) == 0) { + if (user_str == NULL) { + /* revisit: not set, defaults to 'root' for a time being */ + } else if (strcmp(user_str, SSSD_USER) == 0) { sss_sssd_user_uid_and_gid(&ctx->uid, &ctx->gid); } 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'!", @@ -850,7 +848,7 @@ static int get_service_user(struct mt_ctx *ctx) ret = ERR_INVALID_CONFIG; } - talloc_free(user_str); + free(user_str); #endif return ret; @@ -902,12 +900,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"); @@ -1462,80 +1454,6 @@ 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, @@ -2019,12 +1937,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 @@ -2089,20 +2016,56 @@ 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"); - return 1; + 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"); } +#ifdef SSSD_NON_ROOT_USER + else if (initial_caps != NULL) { + /* revisit: SSSD runs under root, everything is root:root owned. No caps required. + sss_log(SSS_LOG_ALERT, + "Those capabilities aren't needed and can be removed:\n %s", + initial_caps); + */ + } +#endif /* SSSD_NON_ROOT_USER */ + + 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 5; + } + /* revisit: check ownership of sssd.conf - should be SSSD_USER, + * sss_log(SSS_LOG_ALERT, ...) otherwise; + * also revisit `sss_ini_read_sssd_conf() -> sss_ini_access_check()` */ + + sss_drop_cap(CAP_DAC_OVERRIDE); + + ret = get_service_user(config, monitor); + if (ret != EOK) { + return 5; + } + + /* TODO: provide 'user' to 'bootstrap()' */ ret = bootstrap_monitor_process(); if (ret != 0) { ERROR("Failed to boostrap SSSD 'monitor' process: %s", sss_strerror(ret)); @@ -2113,6 +2076,14 @@ int main(int argc, const char *argv[]) /* default value of 'debug_prg_name' will be used */ DEBUG_INIT(debug_level, opt_logger); + DEBUG(SSSDBG_IMPORTANT_INFO, + "Started under uid=%"SPRIuid" (euid=%"SPRIuid") : " + "gid=%"SPRIgid" (egid=%"SPRIgid")" + " with following capabilities:\n%s", + uid, euid, gid, egid, + initial_caps ? initial_caps : " (nothing)\n"); + talloc_free(initial_caps); + setup_keyring(); /* Check if the SSSD is already running */ @@ -2121,53 +2092,46 @@ 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"); + "SSSD is already running: pidfile exists at '"SSSD_PIDFILE"'\n"); return 5; } } check_nscd(); - /* Parse config file, fail if cannot be done */ - ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, - &monitor); + /* set up things like debug, signals, daemonization, etc. */ + ret = close(STDIN_FILENO); + if (ret != EOK) return 5; + + ret = confdb_write_ini(tmp_ctx, config, cdb_file, false, false, &monitor->cdb); 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; - } + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to write config DB: '%s'\n", sss_strerror(ret)); return 5; } - /* set up things like debug, signals, daemonization, etc. */ - ret = close(STDIN_FILENO); - if (ret != EOK) return 5; + /* revisit: remove */ + /* For a time being this will break 'run under sssd' systemd services + * if sssd.conf::user == root. + * But they don't work in this setup anyway, so doesn't matter. + * Will be fixed later when everything will be run by default under 'sssd' + */ + ret = chown(cdb_file, monitor->uid, monitor->gid); + if (ret != 0) { + ret = errno; + DEBUG(SSSDBG_FATAL_FAILURE, + "chown failed for '%s': '%s'.\n", cdb_file, sss_strerror(ret)); + return 5; + } + + get_monitor_config(monitor); + talloc_zfree(monitor->cdb); ret = server_setup(SSSD_MONITOR_NAME, false, flags, 0, 0, CONFDB_FILE, 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 - * 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; - monitor->is_daemon = !opt_interactive; monitor->parent_pid = main_ctx->parent_pid; monitor->ev = main_ctx->event_ctx; diff --git a/src/monitor/monitor_bootstrap.c b/src/monitor/monitor_bootstrap.c index d85483aa9d3..3b1f8b828d0 100644 --- a/src/monitor/monitor_bootstrap.c +++ b/src/monitor/monitor_bootstrap.c @@ -117,7 +117,7 @@ int bootstrap_monitor_process(void) } } - /* TODO: drop CAP_SET_GID capability */ + /* revisit: drop CAP_SET_GID capability */ } #endif /* SSSD_NON_ROOT_USER */ diff --git a/src/util/server.c b/src/util/server.c index 0535398041e..40d019fce57 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -790,7 +790,10 @@ 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 following capabilities:\n%s", + getuid(), geteuid(), getgid(), getegid(), caps ? caps : " (nothing)\n"); talloc_free(caps); } diff --git a/src/util/usertools.c b/src/util/usertools.c index 27cc390d0dc..32b210a66c9 100644 --- a/src/util/usertools.c +++ b/src/util/usertools.c @@ -870,41 +870,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 447155633d8..cdec44bf936 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -393,8 +393,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,