URL: https://github.com/SSSD/sssd/pull/5552 Author: sumit-bose Title: #5552: files: split update into batches Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5552/head:pr5552 git checkout pr5552
From fcb5aef25385ba99238ce1806a7195b338d7c497 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Thu, 25 Mar 2021 12:08:34 +0100 Subject: [PATCH 1/4] files: split update into batches If the files managed by the files provider contain many users or groups processing them might take a considerable amount of time. To keep the backend responsive this patch splits the update into multiple steps running one after the other but returning to the main loop in between. This avoids issues during startup because the watchdog timer state is reset properly. Additionally SBUS messages are process and as a result the domain can be marked inconsistent in the frontends properly. Resolves: https://github.com/SSSD/sssd/issues/5557 :fixes: Update large files in the files provider in batches to avoid timeouts --- src/providers/files/files_ops.c | 397 +++++++++++++++++++++++++------- 1 file changed, 317 insertions(+), 80 deletions(-) diff --git a/src/providers/files/files_ops.c b/src/providers/files/files_ops.c index 54d2b41646..ee6edd749b 100644 --- a/src/providers/files/files_ops.c +++ b/src/providers/files/files_ops.c @@ -449,26 +449,15 @@ static errno_t refresh_override_attrs(struct files_id_ctx *id_ctx, } static errno_t sf_enum_groups(struct files_id_ctx *id_ctx, - const char *group_file); + struct group **groups, size_t start, size_t size); -errno_t sf_enum_users(struct files_id_ctx *id_ctx, - const char *passwd_file) +static errno_t sf_enum_users(struct files_id_ctx *id_ctx, struct passwd **users, + size_t start, size_t size) { errno_t ret; - TALLOC_CTX *tmp_ctx = NULL; - struct passwd **users = NULL; + size_t i; - tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) { - return ENOMEM; - } - - ret = enum_files_users(tmp_ctx, passwd_file, &users); - if (ret != EOK) { - goto done; - } - - for (size_t i = 0; users[i]; i++) { + for (i = start; users[i] != NULL && i < (start + size); i++) { ret = save_file_user(id_ctx, users[i]); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, @@ -478,16 +467,19 @@ errno_t sf_enum_users(struct files_id_ctx *id_ctx, } } - ret = refresh_override_attrs(id_ctx, SYSDB_MEMBER_USER); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, - "Failed to refresh override attributes, " - "override values might not be available.\n"); + if (users[i] == NULL) { + ret = refresh_override_attrs(id_ctx, SYSDB_MEMBER_USER); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Failed to refresh override attributes, " + "override values might not be available.\n"); + } + + ret = EOK; + } else { + ret = EAGAIN; } - ret = EOK; -done: - talloc_free(tmp_ctx); return ret; } @@ -665,29 +657,25 @@ static errno_t save_file_group(struct files_id_ctx *id_ctx, } static errno_t sf_enum_groups(struct files_id_ctx *id_ctx, - const char *group_file) + struct group **groups, size_t start, size_t size) { errno_t ret; TALLOC_CTX *tmp_ctx = NULL; - struct group **groups = NULL; const char **cached_users = NULL; + size_t i; tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { return ENOMEM; } - ret = enum_files_groups(tmp_ctx, group_file, &groups); - if (ret != EOK) { - goto done; - } - cached_users = get_cached_user_names(tmp_ctx, id_ctx->domain); if (cached_users == NULL) { + ret = ENOMEM; goto done; } - for (size_t i = 0; groups[i]; i++) { + for (i = start; groups[i] != NULL && i < (start + size); i++) { ret = save_file_group(id_ctx, groups[i], cached_users); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, @@ -696,93 +684,266 @@ static errno_t sf_enum_groups(struct files_id_ctx *id_ctx, } } - ret = refresh_override_attrs(id_ctx, SYSDB_MEMBER_GROUP); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, - "Failed to refresh override attributes, " - "override values might not be available.\n"); + if (groups[i] == NULL) { + ret = refresh_override_attrs(id_ctx, SYSDB_MEMBER_GROUP); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Failed to refresh override attributes, " + "override values might not be available.\n"); + } + + ret = EOK; + } else { + ret = EAGAIN; } - ret = EOK; done: talloc_free(tmp_ctx); return ret; } -static errno_t sf_enum_files(struct files_id_ctx *id_ctx, - uint8_t flags) +enum update_steps { + DELETE_USERS, + READ_USERS, + SAVE_USERS, + DELETE_GROUPS, + READ_GROUPS, + SAVE_GROUPS, + UPDATE_FINISH, + UPDATE_DONE, +}; + +struct sf_enum_files_state { + struct files_id_ctx *id_ctx; + uint8_t flags; + struct tevent_timer *te; + enum update_steps current_step; + size_t step; + bool in_transaction; + size_t batch_size; + size_t obj_idx; + size_t file_idx; + struct passwd **users; + struct group **groups; + uint32_t delay; + uint32_t initial_delay; +}; + +static void sf_enum_files_first_step(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval tv, + void *data); +static struct tevent_req *sf_enum_files_send(struct files_id_ctx *id_ctx, + uint8_t flags) { + struct tevent_req *req; + struct sf_enum_files_state *state; + struct timeval tv; errno_t ret; - errno_t tret; - bool in_transaction = false; + + req = tevent_req_create(id_ctx, &state, struct sf_enum_files_state); + if (req == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create tevent request!\n"); + return NULL; + } + + state->id_ctx = id_ctx; + state->flags = flags; + state->step = 0; + state->batch_size = 1000; + state->obj_idx = 0; + state->file_idx = 0; + state->initial_delay = 100; + state->delay = 100; + + if (state->flags & SF_UPDATE_PASSWD) { + state->current_step = DELETE_USERS; + } else if (state->flags & SF_UPDATE_GROUP) { + state->current_step = DELETE_GROUPS; + } + + tv = tevent_timeval_current_ofs(0, state->initial_delay); + state->te = tevent_add_timer(id_ctx->be->ev, state, tv, + sf_enum_files_first_step, req); + if (state->te == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Unable to schedule files update.\n"); + ret = EFAULT; + goto done; + } ret = sysdb_transaction_start(id_ctx->domain->sysdb); if (ret != EOK) { goto done; } - in_transaction = true; + state->in_transaction = true; + + return req; + +done: + tevent_req_error(req, ret); + tevent_req_post(req, id_ctx->be->ev); + return req; +} - if (flags & SF_UPDATE_PASSWD) { +static void sf_enum_files_first_step(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval tv, + void *data) +{ + errno_t ret; + errno_t tret; + struct sf_enum_files_state *state; + struct tevent_req *req; + struct files_id_ctx *id_ctx; + const char *filename = NULL; + + req = talloc_get_type(data, struct tevent_req); + state = tevent_req_data(req, struct sf_enum_files_state); + + state->te = NULL; + id_ctx = state->id_ctx; + + switch (state->current_step) { + case DELETE_USERS: + DEBUG(SSSDBG_TRACE_ALL, "Step DELETE_USERS.\n"); ret = delete_all_users(id_ctx->domain); if (ret != EOK) { goto done; } - + state->file_idx = 0; + state->current_step = READ_USERS; + break; + case READ_USERS: + DEBUG(SSSDBG_TRACE_ALL, "Step READ_USERS.\n"); + talloc_zfree(state->users); + state->obj_idx = 0; /* All users were deleted, therefore we need to enumerate each file again */ - for (size_t i = 0; id_ctx->passwd_files[i] != NULL; i++) { - ret = sf_enum_users(id_ctx, id_ctx->passwd_files[i]); - if (ret == ENOENT) { + if (id_ctx->passwd_files[state->file_idx] != NULL) { + filename = id_ctx->passwd_files[state->file_idx++]; + ret = enum_files_users(state, filename, &state->users); + if (ret == EOK) { + state->current_step = SAVE_USERS; + } else if (ret == ENOENT) { DEBUG(SSSDBG_MINOR_FAILURE, "The file %s does not exist (yet), skipping\n", - id_ctx->passwd_files[i]); - continue; + filename); } else if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, - "Cannot enumerate users from %s, aborting\n", - id_ctx->passwd_files[i]); + "Cannot enumerate groups from %s, aborting\n", + filename); goto done; } + } else { + if (state->flags & SF_UPDATE_GROUP) { + state->current_step = DELETE_GROUPS; + } else { + state->current_step = UPDATE_FINISH; + } } - } - - if (flags & SF_UPDATE_GROUP) { + break; + case SAVE_USERS: + DEBUG(SSSDBG_TRACE_ALL, "Step SAVE_USERS.\n"); + if (state->users != NULL) { + ret = sf_enum_users(id_ctx, state->users, + state->obj_idx, state->batch_size); + if (ret == EOK) { + state->current_step = READ_USERS; + } else if (ret == EAGAIN) { + state->obj_idx += state->batch_size; + } else { + DEBUG(SSSDBG_OP_FAILURE, "Saving users failed.\n"); + goto done; + } + } + break; + case DELETE_GROUPS: + DEBUG(SSSDBG_TRACE_ALL, "Step DELETE_GROUPS.\n"); ret = delete_all_groups(id_ctx->domain); if (ret != EOK) { goto done; } - + state->file_idx = 0; + state->current_step = READ_GROUPS; + break; + case READ_GROUPS: + DEBUG(SSSDBG_TRACE_ALL, "Step READ_GROUPS.\n"); + talloc_zfree(state->groups); + state->obj_idx = 0; /* All groups were deleted, therefore we need to enumerate each file again */ - for (size_t i = 0; id_ctx->group_files[i] != NULL; i++) { - ret = sf_enum_groups(id_ctx, id_ctx->group_files[i]); - if (ret == ENOENT) { + if (id_ctx->group_files[state->file_idx] != NULL) { + filename = id_ctx->group_files[state->file_idx++]; + ret = enum_files_groups(state, filename, &state->groups); + if (ret == EOK) { + state->current_step = SAVE_GROUPS; + } else if (ret == ENOENT) { DEBUG(SSSDBG_MINOR_FAILURE, "The file %s does not exist (yet), skipping\n", - id_ctx->group_files[i]); - continue; + filename); } else if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot enumerate groups from %s, aborting\n", - id_ctx->group_files[i]); + filename); goto done; } + } else { + state->current_step = UPDATE_FINISH; + } + break; + case SAVE_GROUPS: + DEBUG(SSSDBG_TRACE_ALL, "Step SAVE_GROUPS.\n"); + if (state->groups != NULL) { + ret = sf_enum_groups(id_ctx, state->groups, + state->obj_idx, state->batch_size); + if (ret == EOK) { + state->current_step = READ_GROUPS; + } else if (ret == EAGAIN) { + state->obj_idx += state->batch_size; + } else { + DEBUG(SSSDBG_OP_FAILURE, "Saving groups failed.\n"); + goto done; + } + } + break; + case UPDATE_FINISH: + DEBUG(SSSDBG_TRACE_ALL, "Step UPDATE_FINISH.\n"); + ret = dp_add_sr_attribute(id_ctx->be); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to add session recording attribute, ignored.\n"); } - } - ret = dp_add_sr_attribute(id_ctx->be); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Failed to add session recording attribute, ignored.\n"); - } + ret = sysdb_transaction_commit(id_ctx->domain->sysdb); + if (ret != EOK) { + goto done; + } + state->in_transaction = false; - ret = sysdb_transaction_commit(id_ctx->domain->sysdb); - if (ret != EOK) { + state->current_step = UPDATE_DONE; + + ret = EOK; + break; + default: + DEBUG(SSSDBG_CRIT_FAILURE, "Undefined update step [%zu].\n", + state->current_step); + ret = EINVAL; goto done; } - in_transaction = false; - ret = EOK; + if (state->current_step != UPDATE_DONE) { + tv = tevent_timeval_current_ofs(0, state->delay); + state->te = tevent_add_timer(id_ctx->be->ev, state, tv, + sf_enum_files_first_step, req); + if (state->te == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Unable to schedule files update.\n"); + ret = EFAULT; + goto done; + } + + return; + } + done: - if (in_transaction) { + if (state->in_transaction) { tret = sysdb_transaction_cancel(id_ctx->domain->sysdb); if (tret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -790,7 +951,20 @@ static errno_t sf_enum_files(struct files_id_ctx *id_ctx, } } - return ret; + if (ret == EOK) { + tevent_req_done(req); + } else { + tevent_req_error(req, ret); + } + + return; +} + +static errno_t sf_enum_files_recv(struct tevent_req *req) +{ + TEVENT_REQ_RETURN_ON_ERROR(req); + + return EOK; } static void sf_cb_done(struct files_id_ctx *id_ctx) @@ -803,9 +977,11 @@ static void sf_cb_done(struct files_id_ctx *id_ctx) } } +static void sf_passwd_cb_done(struct tevent_req *req); static int sf_passwd_cb(const char *filename, uint32_t flags, void *pvt) { struct files_id_ctx *id_ctx; + struct tevent_req *req; errno_t ret; id_ctx = talloc_get_type(pvt, struct files_id_ctx); @@ -826,7 +1002,30 @@ static int sf_passwd_cb(const char *filename, uint32_t flags, void *pvt) * only then edits passwd and adds the user. The reverse is not needed, * because member/memberof links are established when groups are saved. */ - ret = sf_enum_files(id_ctx, SF_UPDATE_BOTH); + req = sf_enum_files_send(id_ctx, SF_UPDATE_BOTH); + if (req == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to start files update.\n"); + ret = ENOMEM; + id_ctx->updating_passwd = false; + sf_cb_done(id_ctx); + files_account_info_finished(id_ctx, BE_REQ_USER, ret); + return ret; + } + + tevent_req_set_callback(req, sf_passwd_cb_done, id_ctx); + + return EOK; +} + +static void sf_passwd_cb_done(struct tevent_req *req) +{ + struct files_id_ctx *id_ctx; + errno_t ret; + + id_ctx = tevent_req_callback_data(req, struct files_id_ctx); + + ret = sf_enum_files_recv(req); + talloc_zfree(req); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Could not update files: [%d]: %s\n", @@ -839,13 +1038,14 @@ static int sf_passwd_cb(const char *filename, uint32_t flags, void *pvt) id_ctx->updating_passwd = false; sf_cb_done(id_ctx); files_account_info_finished(id_ctx, BE_REQ_USER, ret); - return ret; } +static void sf_group_cb_done(struct tevent_req *req); static int sf_group_cb(const char *filename, uint32_t flags, void *pvt) { struct files_id_ctx *id_ctx; errno_t ret; + struct tevent_req *req; id_ctx = talloc_get_type(pvt, struct files_id_ctx); if (id_ctx == NULL) { @@ -861,7 +1061,30 @@ static int sf_group_cb(const char *filename, uint32_t flags, void *pvt) dp_sbus_reset_groups_memcache(id_ctx->be->provider); dp_sbus_reset_initgr_memcache(id_ctx->be->provider); - ret = sf_enum_files(id_ctx, SF_UPDATE_GROUP); + req = sf_enum_files_send(id_ctx, SF_UPDATE_GROUP); + if (req == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to start files update.\n"); + ret = ENOMEM; + id_ctx->updating_groups = false; + sf_cb_done(id_ctx); + files_account_info_finished(id_ctx, BE_REQ_GROUP, ret); + return ret; + } + + tevent_req_set_callback(req, sf_group_cb_done, id_ctx); + + return EOK; +} + +static void sf_group_cb_done(struct tevent_req *req) +{ + struct files_id_ctx *id_ctx; + errno_t ret; + + id_ctx = tevent_req_callback_data(req, struct files_id_ctx); + + ret = sf_enum_files_recv(req); + talloc_zfree(req); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Could not update files: [%d]: %s\n", @@ -874,19 +1097,33 @@ static int sf_group_cb(const char *filename, uint32_t flags, void *pvt) id_ctx->updating_groups = false; sf_cb_done(id_ctx); files_account_info_finished(id_ctx, BE_REQ_GROUP, ret); - return ret; } +static void startup_enum_files_done(struct tevent_req *req); static void startup_enum_files(struct tevent_context *ev, struct tevent_immediate *imm, void *pvt) { struct files_id_ctx *id_ctx = talloc_get_type(pvt, struct files_id_ctx); - errno_t ret; + struct tevent_req *req; talloc_zfree(imm); - ret = sf_enum_files(id_ctx, SF_UPDATE_BOTH); + req = sf_enum_files_send(id_ctx, SF_UPDATE_BOTH); + if (req == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Could not update files after startup.\n"); + return; + } + + tevent_req_set_callback(req, startup_enum_files_done, NULL); +} + +static void startup_enum_files_done(struct tevent_req *req) +{ + errno_t ret; + + ret = sf_enum_files_recv(req); + talloc_zfree(req); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Could not update files after startup: [%d]: %s\n", From 586976a7b1094a4741c3acb5dc3d38968af95eb4 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Thu, 6 May 2021 17:52:26 +0200 Subject: [PATCH 2/4] files: add new option fallback_to_nss To not block callers when SSSD's files is doing a refresh of /etc/passwd or /etc/group allow to fall back to the next nss module which is typically libnss_files. Resolves: https://github.com/SSSD/sssd/issues/5557 --- src/confdb/confdb.c | 12 ++++++++++++ src/confdb/confdb.h | 2 ++ src/man/sssd-files.5.xml | 26 ++++++++++++++++++++++++++ src/responder/common/responder_dp.c | 6 ++++++ src/tests/intg/test_files_provider.py | 7 +++++++ src/tests/intg/test_pam_responder.py | 7 +++++++ src/util/domain_info_utils.c | 5 +++++ src/util/util.h | 1 + 8 files changed, 66 insertions(+) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index ca2892e793..2c3372876e 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -1622,6 +1622,18 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, domain->view_name = NULL; domain->state = DOM_ACTIVE; + + domain->fallback_to_nss = false; + if (is_files_provider(domain)) { + ret = get_entry_as_bool(res->msgs[0], &domain->fallback_to_nss, + CONFDB_DOMAIN_FALLBACK_TO_NSS, true); + if(ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Invalid value for %s\n", CONFDB_DOMAIN_FALLBACK_TO_NSS); + goto done; + } + } + domain->not_found_counter = 0; *_domain = domain; diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 155b8a9f04..61a60c8bed 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -253,6 +253,7 @@ #define CONFDB_DOMAIN_TYPE_POSIX "posix" #define CONFDB_DOMAIN_TYPE_APP "application" #define CONFDB_DOMAIN_INHERIT_FROM "inherit_from" +#define CONFDB_DOMAIN_FALLBACK_TO_NSS "fallback_to_nss" /* Local Provider */ #define CONFDB_LOCAL_DEFAULT_SHELL "default_shell" @@ -424,6 +425,7 @@ struct sss_domain_info { struct sss_domain_info *next; enum sss_domain_state state; + bool fallback_to_nss; char **sd_inherit; /* Do not use the forest pointer directly in new code, but rather the diff --git a/src/man/sssd-files.5.xml b/src/man/sssd-files.5.xml index 34b1079650..6207c917da 100644 --- a/src/man/sssd-files.5.xml +++ b/src/man/sssd-files.5.xml @@ -122,6 +122,32 @@ </listitem> </varlistentry> + <varlistentry> + <term>fallback_to_nss (boolean)</term> + <listitem> + <para> + While updating the internal data SSSD will return an + error and let the client continue with the next NSS + module. This helps to avoid delays when using the + default system files + <filename>/etc/passwd</filename> and + <filename>/etc/group</filename> and the NSS + configuration has 'sss' before 'files' for the + 'passwd' and 'group' maps. + </para> + <para> + If the files provider is configured to monitor other + files it makes sense to set this option to 'False' + to avoid inconsistent behavior because in general + there would be no other NSS module which can be used + as a fallback. + </para> + <para> + Default: True + </para> + </listitem> + </varlistentry> + </variablelist> </para> </refsect1> diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c index 597d058534..8076e1e43e 100644 --- a/src/responder/common/responder_dp.c +++ b/src/responder/common/responder_dp.c @@ -41,6 +41,12 @@ sss_dp_account_files_params(struct sss_domain_info *dom, return EOK; } + if (sss_domain_fallback_to_nss(dom)) { + DEBUG(SSSDBG_TRACE_INTERNAL, + "Domain files is not consistent, falling back to nss.\n"); + return ENOENT; + } + DEBUG(SSSDBG_TRACE_INTERNAL, "Domain files is not consistent, issuing update\n"); } diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index 3ea326c27e..b8fa9e6031 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -151,6 +151,7 @@ def files_domain_only(request): [domain/files] id_provider = files + fallback_to_nss = False """).format(**locals()) create_conf_fixture(request, conf) create_sssd_fixture(request) @@ -180,6 +181,7 @@ def files_multiple_sources(request): [domain/files] id_provider = files + fallback_to_nss = False passwd_files = {passwd_list} group_files = {group_list} debug_level = 10 @@ -214,6 +216,7 @@ def files_multiple_sources_nocreate(request): [domain/files] id_provider = files + fallback_to_nss = False passwd_files = {passwd_list} group_files = {group_list} debug_level = 10 @@ -269,6 +272,7 @@ def no_files_domain(request): [domain/disabled.files] id_provider = files + fallback_to_nss = False """).format(**locals()) create_conf_fixture(request, conf) create_sssd_fixture(request) @@ -307,6 +311,7 @@ def domain_resolution_order(request): [domain/files] id_provider = files + fallback_to_nss = False """).format(**locals()) create_conf_fixture(request, conf) create_sssd_fixture(request) @@ -323,6 +328,7 @@ def default_domain_suffix(request): [domain/files] id_provider = files + fallback_to_nss = False """).format(**locals()) create_conf_fixture(request, conf) create_sssd_fixture(request) @@ -338,6 +344,7 @@ def override_homedir_and_shell(request): [domain/files] id_provider = files + fallback_to_nss = False override_homedir = /test/bar override_shell = /bin/bar diff --git a/src/tests/intg/test_pam_responder.py b/src/tests/intg/test_pam_responder.py index f0bcf42fb9..072be68e9b 100644 --- a/src/tests/intg/test_pam_responder.py +++ b/src/tests/intg/test_pam_responder.py @@ -141,6 +141,7 @@ def format_pam_cert_auth_conf(config): [domain/auth_only] debug_level = 10 id_provider = files + fallback_to_nss = False [certmap/auth_only/user1] matchrule = <SUBJECT>.*CN=SSSD test cert 0001.* @@ -172,6 +173,7 @@ def format_pam_cert_auth_conf_name_format(config): full_name_format = %2$s\\%1$s debug_level = 10 id_provider = files + fallback_to_nss = False [certmap/auth_only/user1] matchrule = <SUBJECT>.*CN=SSSD test cert 0001.* @@ -195,6 +197,7 @@ def format_pam_krb5_auth(config, kdc_instance): [domain/krb5_auth] debug_level = 10 id_provider = files + fallback_to_nss = False auth_provider = krb5 krb5_realm = PAMKRB5TEST @@ -219,6 +222,7 @@ def format_pam_krb5_auth_domains(config, kdc_instance): [domain/wrong.dom1] debug_level = 10 id_provider = files + fallback_to_nss = False auth_provider = krb5 krb5_realm = WRONG1REALM @@ -227,6 +231,7 @@ def format_pam_krb5_auth_domains(config, kdc_instance): [domain/wrong.dom2] debug_level = 10 id_provider = files + fallback_to_nss = False auth_provider = krb5 krb5_realm = WRONG2REALM @@ -235,6 +240,7 @@ def format_pam_krb5_auth_domains(config, kdc_instance): [domain/wrong.dom3] debug_level = 10 id_provider = files + fallback_to_nss = False auth_provider = krb5 krb5_realm = WRONG3REALM @@ -243,6 +249,7 @@ def format_pam_krb5_auth_domains(config, kdc_instance): [domain/krb5_auth] debug_level = 10 id_provider = files + fallback_to_nss = False auth_provider = krb5 krb5_realm = PAMKRB5TEST diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 7ceeb42428..c2e510ecf1 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -901,6 +901,11 @@ void sss_domain_set_state(struct sss_domain_info *dom, "Domain %s is %s\n", dom->name, domain_state_str(dom)); } +bool sss_domain_fallback_to_nss(struct sss_domain_info *dom) +{ + return dom->fallback_to_nss; +} + bool sss_domain_is_forest_root(struct sss_domain_info *dom) { return (dom->forest_root == dom); diff --git a/src/util/util.h b/src/util/util.h index 7553585119..a5baf0b136 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -588,6 +588,7 @@ struct sss_domain_info *find_domain_by_sid(struct sss_domain_info *domain, enum sss_domain_state sss_domain_get_state(struct sss_domain_info *dom); void sss_domain_set_state(struct sss_domain_info *dom, enum sss_domain_state state); +bool sss_domain_fallback_to_nss(struct sss_domain_info *dom); bool sss_domain_is_forest_root(struct sss_domain_info *dom); const char *sss_domain_type_str(struct sss_domain_info *dom); From a5dc61198eed3cbd73597f571df21817040513ef Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Fri, 23 Apr 2021 18:22:09 +0200 Subject: [PATCH 3/4] files: delay refresh and not run in parallel To avoid constant refreshes if /etc/passwd or /etc/group are modified multiple times in a short interval the refresh is only started after 1s of inactivity. Additionally the request makes sure that only one instance is run. Resolves: https://github.com/SSSD/sssd/issues/5557 --- src/providers/files/files_id.c | 25 +-- src/providers/files/files_ops.c | 281 +++++++++++++++++++++++++--- src/providers/files/files_private.h | 9 +- 3 files changed, 268 insertions(+), 47 deletions(-) diff --git a/src/providers/files/files_id.c b/src/providers/files/files_id.c index 626c631018..a959d7736a 100644 --- a/src/providers/files/files_id.c +++ b/src/providers/files/files_id.c @@ -57,7 +57,7 @@ files_account_info_handler_send(TALLOC_CTX *mem_ctx, goto immediate; } update_req = &id_ctx->users_req; - needs_update = id_ctx->updating_passwd ? true : false; + needs_update = (id_ctx->refresh_ctx != NULL); break; case BE_REQ_GROUP: if (data->filter_type != BE_FILTER_ENUM) { @@ -67,7 +67,7 @@ files_account_info_handler_send(TALLOC_CTX *mem_ctx, goto immediate; } update_req = &id_ctx->groups_req; - needs_update = id_ctx->updating_groups ? true : false; + needs_update = (id_ctx->refresh_ctx != NULL); break; case BE_REQ_INITGROUPS: if (data->filter_type != BE_FILTER_NAME) { @@ -83,9 +83,7 @@ files_account_info_handler_send(TALLOC_CTX *mem_ctx, goto immediate; } update_req = &id_ctx->initgroups_req; - needs_update = id_ctx->updating_groups || id_ctx->updating_passwd \ - ? true \ - : false; + needs_update = (id_ctx->refresh_ctx != NULL); break; case BE_REQ_BY_CERT: if (data->filter_type != BE_FILTER_CERT) { @@ -164,24 +162,9 @@ void files_account_info_finished(struct files_id_ctx *id_ctx, int req_type, errno_t ret) { - switch (req_type) { - case BE_REQ_USER: finish_update_req(&id_ctx->users_req, ret); - if (id_ctx->updating_groups == false) { - finish_update_req(&id_ctx->initgroups_req, ret); - } - break; - case BE_REQ_GROUP: finish_update_req(&id_ctx->groups_req, ret); - if (id_ctx->updating_passwd == false) { - finish_update_req(&id_ctx->initgroups_req, ret); - } - break; - default: - DEBUG(SSSDBG_CRIT_FAILURE, - "Unexpected req_type %d\n", req_type); - return; - } + finish_update_req(&id_ctx->initgroups_req, ret); } errno_t files_account_info_handler_recv(TALLOC_CTX *mem_ctx, diff --git a/src/providers/files/files_ops.c b/src/providers/files/files_ops.c index ee6edd749b..2ffb695577 100644 --- a/src/providers/files/files_ops.c +++ b/src/providers/files/files_ops.c @@ -39,6 +39,7 @@ #define SF_UPDATE_PASSWD 1<<0 #define SF_UPDATE_GROUP 1<<1 #define SF_UPDATE_BOTH (SF_UPDATE_PASSWD | SF_UPDATE_GROUP) +#define SF_UPDATE_IMMEDIATE 1<<2 struct files_ctx { struct files_ops_ctx *ops; @@ -703,9 +704,11 @@ static errno_t sf_enum_groups(struct files_id_ctx *id_ctx, } enum update_steps { + WAIT_TO_START_USERS, DELETE_USERS, READ_USERS, SAVE_USERS, + WAIT_TO_START_GROUPS, DELETE_GROUPS, READ_GROUPS, SAVE_GROUPS, @@ -713,8 +716,119 @@ enum update_steps { UPDATE_DONE, }; +struct files_refresh_ctx { + struct timeval start_passwd_refresh; + enum refresh_task_status updating_passwd; + bool passwd_start_again; + struct timeval start_group_refresh; + enum refresh_task_status updating_groups; + bool group_start_again; +}; + +static errno_t check_state(struct files_refresh_ctx *refresh_ctx, uint8_t flags) +{ + errno_t ret; + struct timeval tv; + struct timeval delay = { 1, 0 }; + const struct timeval tv_zero = {0 , 0}; + + ret = gettimeofday(&tv, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "gettimeofday failed [%d][%s], keeping old value.\n", + ret, sss_strerror(ret)); + } + + if ((flags & SF_UPDATE_PASSWD) && (flags & SF_UPDATE_GROUP)) { + if (flags & SF_UPDATE_IMMEDIATE) { + refresh_ctx->start_passwd_refresh = tv_zero; + } else { + if (ret == EOK) { + timeradd(&tv, &delay, + &refresh_ctx->start_passwd_refresh); + } + } + + switch (refresh_ctx->updating_passwd) { + case REFRESH_NOT_RUNNIG: + break; + case REFRESH_WAITING_TO_START: + DEBUG(SSSDBG_TRACE_FUNC, + "Refresh is already waiting to start, nothing to do.\n"); + return EAGAIN; + case REFRESH_ACTIVE: + DEBUG(SSSDBG_TRACE_FUNC, + "Refresh currently active, queing another refresh.\n"); + refresh_ctx->passwd_start_again = true; + return EAGAIN; + default: + DEBUG(SSSDBG_OP_FAILURE, "Unknown refresh state [%d].\n", + refresh_ctx->updating_passwd); + return EINVAL; + } + + /* Groups are updated after passwd, in case a new passwd update + * arrives we have to run the passwd steps again. */ + switch (refresh_ctx->updating_groups) { + case REFRESH_NOT_RUNNIG: + break; + case REFRESH_WAITING_TO_START: + refresh_ctx->passwd_start_again = true; + return EAGAIN; + case REFRESH_ACTIVE: + refresh_ctx->passwd_start_again = true; + refresh_ctx->group_start_again = true; + return EAGAIN; + default: + DEBUG(SSSDBG_OP_FAILURE, "Unknown refresh state [%d].\n", + refresh_ctx->updating_groups); + return EINVAL; + } + + refresh_ctx->passwd_start_again = false; + refresh_ctx->updating_passwd = REFRESH_WAITING_TO_START; + refresh_ctx->updating_groups = REFRESH_WAITING_TO_START; + return EOK; + } else if (flags & SF_UPDATE_GROUP) { + if (flags & SF_UPDATE_IMMEDIATE) { + refresh_ctx->start_group_refresh = tv_zero; + } else { + if (ret == EOK) { + timeradd(&tv, &delay, + &refresh_ctx->start_group_refresh); + } + } + + switch (refresh_ctx->updating_groups) { + case REFRESH_NOT_RUNNIG: + break; + case REFRESH_WAITING_TO_START: + DEBUG(SSSDBG_TRACE_FUNC, + "Refresh is already waiting to start, nothing to do.\n"); + return EAGAIN; + case REFRESH_ACTIVE: + DEBUG(SSSDBG_TRACE_FUNC, + "Refresh currently active, queing another refresh.\n"); + refresh_ctx->group_start_again = true; + return EAGAIN; + default: + DEBUG(SSSDBG_OP_FAILURE, "Unknown refresh state [%d].\n", + refresh_ctx->updating_passwd); + return EINVAL; + } + + refresh_ctx->group_start_again = false; + refresh_ctx->updating_groups = REFRESH_WAITING_TO_START; + return EOK; + } + + DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected refresh flags [%"PRIu8"].\n", flags); + return EINVAL; +} + struct sf_enum_files_state { struct files_id_ctx *id_ctx; + struct files_refresh_ctx *refresh_ctx; uint8_t flags; struct tevent_timer *te; enum update_steps current_step; @@ -729,6 +843,15 @@ struct sf_enum_files_state { uint32_t initial_delay; }; +static int clear_refresh_ctx(void *ptr) +{ + struct sf_enum_files_state *state = (struct sf_enum_files_state *) ptr; + + state->id_ctx->refresh_ctx = NULL; + + return 0; +} + static void sf_enum_files_first_step(struct tevent_context *ev, struct tevent_timer *te, struct timeval tv, @@ -740,6 +863,24 @@ static struct tevent_req *sf_enum_files_send(struct files_id_ctx *id_ctx, struct sf_enum_files_state *state; struct timeval tv; errno_t ret; + struct files_refresh_ctx *refresh_ctx = NULL; + + if (id_ctx->refresh_ctx != NULL) { + refresh_ctx = id_ctx->refresh_ctx; + } else { + refresh_ctx = talloc_zero(id_ctx, struct files_refresh_ctx); + if (refresh_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to allocate refresh context.\n"); + return NULL; + } + refresh_ctx->updating_passwd = REFRESH_NOT_RUNNIG; + refresh_ctx->updating_groups = REFRESH_NOT_RUNNIG; + } + + ret = check_state(refresh_ctx, flags); + if (ret != EOK) { + return NULL; + } req = tevent_req_create(id_ctx, &state, struct sf_enum_files_state); if (req == NULL) { @@ -747,6 +888,17 @@ static struct tevent_req *sf_enum_files_send(struct files_id_ctx *id_ctx, return NULL; } + if (id_ctx->refresh_ctx == NULL) { + id_ctx->refresh_ctx = talloc_steal(state, refresh_ctx); + talloc_set_destructor((TALLOC_CTX *) state, clear_refresh_ctx); + } else { + DEBUG(SSSDBG_CRIT_FAILURE, "The files refresh task should run only " + "once, but a second was detected. Error in internal procession " + "logic.\n"); + ret = EFAULT; + goto done; + } + state->id_ctx = id_ctx; state->flags = flags; state->step = 0; @@ -757,9 +909,9 @@ static struct tevent_req *sf_enum_files_send(struct files_id_ctx *id_ctx, state->delay = 100; if (state->flags & SF_UPDATE_PASSWD) { - state->current_step = DELETE_USERS; + state->current_step = WAIT_TO_START_USERS; } else if (state->flags & SF_UPDATE_GROUP) { - state->current_step = DELETE_GROUPS; + state->current_step = WAIT_TO_START_GROUPS; } tv = tevent_timeval_current_ofs(0, state->initial_delay); @@ -771,12 +923,6 @@ static struct tevent_req *sf_enum_files_send(struct files_id_ctx *id_ctx, goto done; } - ret = sysdb_transaction_start(id_ctx->domain->sysdb); - if (ret != EOK) { - goto done; - } - state->in_transaction = true; - return req; done: @@ -796,15 +942,48 @@ static void sf_enum_files_first_step(struct tevent_context *ev, struct tevent_req *req; struct files_id_ctx *id_ctx; const char *filename = NULL; + struct timeval now; + struct timeval diff; + uint32_t delay; req = talloc_get_type(data, struct tevent_req); state = tevent_req_data(req, struct sf_enum_files_state); state->te = NULL; id_ctx = state->id_ctx; + delay = state->delay; switch (state->current_step) { + case WAIT_TO_START_USERS: + DEBUG(SSSDBG_TRACE_ALL, "Step WAIT_TO_START_USERS.\n"); + ret = gettimeofday(&now, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "gettimeofday failed [%d][%s], starting user refresh now.\n", + ret, sss_strerror(ret)); + state->current_step = DELETE_USERS; + delay = 0; + } else { + timersub(&state->id_ctx->refresh_ctx->start_passwd_refresh, &now, + &diff); + if (diff.tv_sec < 0) { + state->current_step = DELETE_USERS; + delay = 0; + } else { + delay = diff.tv_sec*1000000 + diff.tv_usec; + } + } + break; case DELETE_USERS: + if (!state->in_transaction) { + ret = sysdb_transaction_start(id_ctx->domain->sysdb); + if (ret != EOK) { + goto done; + } + state->in_transaction = true; + } + + id_ctx->refresh_ctx->updating_passwd = REFRESH_ACTIVE; DEBUG(SSSDBG_TRACE_ALL, "Step DELETE_USERS.\n"); ret = delete_all_users(id_ctx->domain); if (ret != EOK) { @@ -829,15 +1008,26 @@ static void sf_enum_files_first_step(struct tevent_context *ev, filename); } else if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, - "Cannot enumerate groups from %s, aborting\n", + "Cannot enumerate users from %s, aborting\n", filename); goto done; } } else { + id_ctx->refresh_ctx->updating_passwd = REFRESH_NOT_RUNNIG; if (state->flags & SF_UPDATE_GROUP) { - state->current_step = DELETE_GROUPS; + state->current_step = WAIT_TO_START_GROUPS; } else { - state->current_step = UPDATE_FINISH; + if (state->id_ctx->refresh_ctx->passwd_start_again) { + state->id_ctx->refresh_ctx->passwd_start_again = false; + id_ctx->refresh_ctx->updating_passwd = REFRESH_WAITING_TO_START; + state->current_step = WAIT_TO_START_USERS; + } else if (state->id_ctx->refresh_ctx->group_start_again) { + state->id_ctx->refresh_ctx->group_start_again = false; + id_ctx->refresh_ctx->updating_groups = REFRESH_WAITING_TO_START; + state->current_step = WAIT_TO_START_GROUPS; + } else { + state->current_step = UPDATE_FINISH; + } } } break; @@ -847,6 +1037,7 @@ static void sf_enum_files_first_step(struct tevent_context *ev, ret = sf_enum_users(id_ctx, state->users, state->obj_idx, state->batch_size); if (ret == EOK) { + /* check next file */ state->current_step = READ_USERS; } else if (ret == EAGAIN) { state->obj_idx += state->batch_size; @@ -856,7 +1047,35 @@ static void sf_enum_files_first_step(struct tevent_context *ev, } } break; + case WAIT_TO_START_GROUPS: + DEBUG(SSSDBG_TRACE_ALL, "Step WAIT_TO_START_GROUPS.\n"); + ret = gettimeofday(&now, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "gettimeofday failed [%d][%s], starting user refresh now.\n", + ret, sss_strerror(ret)); + state->current_step = DELETE_GROUPS; + delay = 0; + } else { + timersub(&state->id_ctx->refresh_ctx->start_passwd_refresh, &now, + &diff); + if (diff.tv_sec < 0) { + state->current_step = DELETE_GROUPS; + delay = 0; + } else { + delay = diff.tv_sec*1000000 + diff.tv_usec; + } + } + break; case DELETE_GROUPS: + if (!state->in_transaction) { + ret = sysdb_transaction_start(id_ctx->domain->sysdb); + if (ret != EOK) { + goto done; + } + state->in_transaction = true; + } + id_ctx->refresh_ctx->updating_groups = REFRESH_ACTIVE; DEBUG(SSSDBG_TRACE_ALL, "Step DELETE_GROUPS.\n"); ret = delete_all_groups(id_ctx->domain); if (ret != EOK) { @@ -886,7 +1105,18 @@ static void sf_enum_files_first_step(struct tevent_context *ev, goto done; } } else { - state->current_step = UPDATE_FINISH; + id_ctx->refresh_ctx->updating_groups = REFRESH_NOT_RUNNIG; + if (state->id_ctx->refresh_ctx->passwd_start_again) { + state->id_ctx->refresh_ctx->passwd_start_again = false; + id_ctx->refresh_ctx->updating_passwd = REFRESH_WAITING_TO_START; + state->current_step = WAIT_TO_START_USERS; + } else if (state->id_ctx->refresh_ctx->group_start_again) { + state->id_ctx->refresh_ctx->group_start_again = false; + id_ctx->refresh_ctx->updating_groups = REFRESH_WAITING_TO_START; + state->current_step = WAIT_TO_START_GROUPS; + } else { + state->current_step = UPDATE_FINISH; + } } break; case SAVE_GROUPS: @@ -930,7 +1160,7 @@ static void sf_enum_files_first_step(struct tevent_context *ev, } if (state->current_step != UPDATE_DONE) { - tv = tevent_timeval_current_ofs(0, state->delay); + tv = tevent_timeval_current_ofs(0, delay); state->te = tevent_add_timer(id_ctx->be->ev, state, tv, sf_enum_files_first_step, req); if (state->te == NULL) { @@ -942,6 +1172,7 @@ static void sf_enum_files_first_step(struct tevent_context *ev, return; } + ret = EOK; done: if (state->in_transaction) { tret = sysdb_transaction_cancel(id_ctx->domain->sysdb); @@ -951,6 +1182,8 @@ static void sf_enum_files_first_step(struct tevent_context *ev, } } + id_ctx->refresh_ctx->updating_passwd = REFRESH_NOT_RUNNIG; + id_ctx->refresh_ctx->updating_groups = REFRESH_NOT_RUNNIG; if (ret == EOK) { tevent_req_done(req); } else { @@ -970,8 +1203,7 @@ static errno_t sf_enum_files_recv(struct tevent_req *req) static void sf_cb_done(struct files_id_ctx *id_ctx) { /* Only activate a domain when both callbacks are done */ - if (id_ctx->updating_passwd == false - && id_ctx->updating_groups == false) { + if (id_ctx->refresh_ctx == NULL) { dp_sbus_domain_active(id_ctx->be->provider, id_ctx->domain); } @@ -990,8 +1222,6 @@ static int sf_passwd_cb(const char *filename, uint32_t flags, void *pvt) } DEBUG(SSSDBG_TRACE_FUNC, "passwd notification\n"); - - id_ctx->updating_passwd = true; dp_sbus_domain_inconsistent(id_ctx->be->provider, id_ctx->domain); dp_sbus_reset_users_ncache(id_ctx->be->provider, id_ctx->domain); @@ -1004,9 +1234,12 @@ static int sf_passwd_cb(const char *filename, uint32_t flags, void *pvt) */ req = sf_enum_files_send(id_ctx, SF_UPDATE_BOTH); if (req == NULL) { + if (id_ctx->refresh_ctx != NULL) { + /* Update is currently active, nothing to do */ + return EOK; + } DEBUG(SSSDBG_OP_FAILURE, "Failed to start files update.\n"); ret = ENOMEM; - id_ctx->updating_passwd = false; sf_cb_done(id_ctx); files_account_info_finished(id_ctx, BE_REQ_USER, ret); return ret; @@ -1035,9 +1268,9 @@ static void sf_passwd_cb_done(struct tevent_req *req) ret = EOK; done: - id_ctx->updating_passwd = false; sf_cb_done(id_ctx); files_account_info_finished(id_ctx, BE_REQ_USER, ret); + files_account_info_finished(id_ctx, BE_REQ_GROUP, ret); } static void sf_group_cb_done(struct tevent_req *req); @@ -1053,8 +1286,6 @@ static int sf_group_cb(const char *filename, uint32_t flags, void *pvt) } DEBUG(SSSDBG_TRACE_FUNC, "group notification\n"); - - id_ctx->updating_groups = true; dp_sbus_domain_inconsistent(id_ctx->be->provider, id_ctx->domain); dp_sbus_reset_groups_ncache(id_ctx->be->provider, id_ctx->domain); @@ -1063,9 +1294,12 @@ static int sf_group_cb(const char *filename, uint32_t flags, void *pvt) req = sf_enum_files_send(id_ctx, SF_UPDATE_GROUP); if (req == NULL) { + if (id_ctx->refresh_ctx != NULL) { + /* Update is currently active, nothing to do */ + return EOK; + } DEBUG(SSSDBG_OP_FAILURE, "Failed to start files update.\n"); ret = ENOMEM; - id_ctx->updating_groups = false; sf_cb_done(id_ctx); files_account_info_finished(id_ctx, BE_REQ_GROUP, ret); return ret; @@ -1094,7 +1328,6 @@ static void sf_group_cb_done(struct tevent_req *req) ret = EOK; done: - id_ctx->updating_groups = false; sf_cb_done(id_ctx); files_account_info_finished(id_ctx, BE_REQ_GROUP, ret); } @@ -1109,7 +1342,7 @@ static void startup_enum_files(struct tevent_context *ev, talloc_zfree(imm); - req = sf_enum_files_send(id_ctx, SF_UPDATE_BOTH); + req = sf_enum_files_send(id_ctx, SF_UPDATE_BOTH|SF_UPDATE_IMMEDIATE); if (req == NULL) { DEBUG(SSSDBG_OP_FAILURE, "Could not update files after startup.\n"); return; diff --git a/src/providers/files/files_private.h b/src/providers/files/files_private.h index fd17819308..fff5a3a7d1 100644 --- a/src/providers/files/files_private.h +++ b/src/providers/files/files_private.h @@ -34,6 +34,12 @@ #include "providers/data_provider/dp.h" +enum refresh_task_status { + REFRESH_NOT_RUNNIG = 0, + REFRESH_WAITING_TO_START, + REFRESH_ACTIVE +}; + struct files_id_ctx { struct be_ctx *be; struct sss_domain_info *domain; @@ -43,8 +49,7 @@ struct files_id_ctx { const char **passwd_files; const char **group_files; - bool updating_passwd; - bool updating_groups; + struct files_refresh_ctx *refresh_ctx; struct tevent_req *users_req; struct tevent_req *groups_req; From 979880ef8e17d7e27604ae45c8bbd557e44f3248 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Tue, 15 Jun 2021 09:01:57 +0200 Subject: [PATCH 4/4] files: queue certmap requests if a refresh is running To make sure current and valid data is used when a certificate should be matched to a users from the files provider the request has to wait until a running refresh is finished. Resolves: https://github.com/SSSD/sssd/issues/5557 --- src/providers/files/files_id.c | 40 +++++++++++++++++++++++++++++ src/providers/files/files_ops.c | 36 ++++++++++++++++++++++++++ src/providers/files/files_private.h | 4 +++ 3 files changed, 80 insertions(+) diff --git a/src/providers/files/files_id.c b/src/providers/files/files_id.c index a959d7736a..210347832d 100644 --- a/src/providers/files/files_id.c +++ b/src/providers/files/files_id.c @@ -26,8 +26,32 @@ struct files_account_info_handler_state { struct dp_reply_std reply; struct files_id_ctx *id_ctx; + struct dp_id_data *data; }; +void handle_certmap(struct tevent_req *req) +{ + struct files_account_info_handler_state *state; + int ret; + + state = tevent_req_data(req, struct files_account_info_handler_state); + + ret = files_map_cert_to_user(state->id_ctx, state->data); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "files_map_cert_to_user failed\n"); + } + + dp_reply_std_set(&state->reply, DP_ERR_DECIDE, ret, NULL); + + if (ret == EOK) { + tevent_req_done(req); + } else { + tevent_req_error(req, ret); + } + + return; +} + struct tevent_req * files_account_info_handler_send(TALLOC_CTX *mem_ctx, struct files_id_ctx *id_ctx, @@ -93,12 +117,28 @@ files_account_info_handler_send(TALLOC_CTX *mem_ctx, ret = EINVAL; goto immediate; } + if (id_ctx->sss_certmap_ctx == NULL) { DEBUG(SSSDBG_TRACE_ALL, "Certificate mapping not configured.\n"); ret = EOK; goto immediate; } + /* Refresh is running, we have to wait until it is done */ + if (id_ctx->refresh_ctx != NULL) { + state->data = data; + + ret = sf_add_certmap_req(id_ctx->refresh_ctx, req); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Failed to add request certmap request list.\n"); + goto immediate; + } + + return req; + } + + /* No refresh is running, we have reply immediately */ ret = files_map_cert_to_user(id_ctx, data); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "files_map_cert_to_user failed\n"); diff --git a/src/providers/files/files_ops.c b/src/providers/files/files_ops.c index 2ffb695577..03841cb7a5 100644 --- a/src/providers/files/files_ops.c +++ b/src/providers/files/files_ops.c @@ -716,6 +716,12 @@ enum update_steps { UPDATE_DONE, }; +struct certmap_req_list { + struct tevent_req *req; + struct certmap_req_list *prev; + struct certmap_req_list *next; +}; + struct files_refresh_ctx { struct timeval start_passwd_refresh; enum refresh_task_status updating_passwd; @@ -723,8 +729,27 @@ struct files_refresh_ctx { struct timeval start_group_refresh; enum refresh_task_status updating_groups; bool group_start_again; + + struct certmap_req_list *certmap_req_list; }; +errno_t sf_add_certmap_req(struct files_refresh_ctx *refresh_ctx, + struct tevent_req *req) +{ + struct certmap_req_list *certmap_req_item; + + certmap_req_item = talloc_zero(refresh_ctx, struct certmap_req_list); + if (certmap_req_item == NULL) { + DEBUG(SSSDBG_OP_FAILURE, + "Failed to allow memory for certmap request list.\n"); + return ENOMEM; + } + certmap_req_item->req = req; + DLIST_ADD(refresh_ctx->certmap_req_list, certmap_req_item); + + return EOK; +} + static errno_t check_state(struct files_refresh_ctx *refresh_ctx, uint8_t flags) { errno_t ret; @@ -875,6 +900,7 @@ static struct tevent_req *sf_enum_files_send(struct files_id_ctx *id_ctx, } refresh_ctx->updating_passwd = REFRESH_NOT_RUNNIG; refresh_ctx->updating_groups = REFRESH_NOT_RUNNIG; + refresh_ctx->certmap_req_list = NULL; } ret = check_state(refresh_ctx, flags); @@ -945,6 +971,8 @@ static void sf_enum_files_first_step(struct tevent_context *ev, struct timeval now; struct timeval diff; uint32_t delay; + struct certmap_req_list *certmap_req_item; + struct certmap_req_list *certmap_req_tmp; req = talloc_get_type(data, struct tevent_req); state = tevent_req_data(req, struct sf_enum_files_state); @@ -1182,6 +1210,14 @@ static void sf_enum_files_first_step(struct tevent_context *ev, } } + DLIST_FOR_EACH_SAFE(certmap_req_item, certmap_req_tmp, + id_ctx->refresh_ctx->certmap_req_list) { + handle_certmap(certmap_req_item->req); + DLIST_REMOVE(certmap_req_item, + id_ctx->refresh_ctx->certmap_req_list); + talloc_free(certmap_req_item); + } + id_ctx->refresh_ctx->updating_passwd = REFRESH_NOT_RUNNIG; id_ctx->refresh_ctx->updating_groups = REFRESH_NOT_RUNNIG; if (ret == EOK) { diff --git a/src/providers/files/files_private.h b/src/providers/files/files_private.h index fff5a3a7d1..4134a05b2e 100644 --- a/src/providers/files/files_private.h +++ b/src/providers/files/files_private.h @@ -63,7 +63,11 @@ struct files_ctx *sf_init(TALLOC_CTX *mem_ctx, const char **group_files, struct files_id_ctx *id_ctx); +errno_t sf_add_certmap_req(struct files_refresh_ctx *refresh_ctx, + struct tevent_req *req); /* files_id.c */ +void handle_certmap(struct tevent_req *req); + struct tevent_req * files_account_info_handler_send(TALLOC_CTX *mem_ctx, struct files_id_ctx *id_ctx,
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
