From 6dcec31b3fe6ed505bed2d406ee5fb68c8584557 Mon Sep 17 00:00:00 2001 From: Andre Boscatto Date: Wed, 7 Feb 2024 12:28:28 +0100 Subject: [PATCH 1/8] sssd: adding mail as case insensitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: https://github.com/SSSD/sssd/issues/7173 Reviewed-by: Iker Pedrosa Reviewed-by: Tomáš Halman (cherry picked from commit 945cebcf72ef53ea0368f19c09e710f7fff11b51) (cherry picked from commit dd0f63246aa75d5f53b44cbc185e88833e79976e) --- src/db/sysdb_init.c | 7 ++++++ src/db/sysdb_private.h | 5 +++- src/db/sysdb_upgrade.c | 56 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/db/sysdb_init.c b/src/db/sysdb_init.c index c2ea6c36996..38a9cd64a9e 100644 --- a/src/db/sysdb_init.c +++ b/src/db/sysdb_init.c @@ -603,6 +603,13 @@ static errno_t sysdb_domain_cache_upgrade(TALLOC_CTX *mem_ctx, } } + if (strcmp(version, SYSDB_VERSION_0_23) == 0) { + ret = sysdb_upgrade_23(sysdb, &version); + if (ret != EOK) { + goto done; + } + } + ret = EOK; done: sysdb->ldb = save_ldb; diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index 1f55007bc1b..63f7b560123 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@ #ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__ +#define SYSDB_VERSION_0_24 "0.24" #define SYSDB_VERSION_0_23 "0.23" #define SYSDB_VERSION_0_22 "0.22" #define SYSDB_VERSION_0_21 "0.21" @@ -47,7 +48,7 @@ #define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1" -#define SYSDB_VERSION SYSDB_VERSION_0_23 +#define SYSDB_VERSION SYSDB_VERSION_0_24 #define SYSDB_BASE_LDIF \ "dn: @ATTRIBUTES\n" \ @@ -60,6 +61,7 @@ "objectclass: CASE_INSENSITIVE\n" \ "ipHostNumber: CASE_INSENSITIVE\n" \ "ipNetworkNumber: CASE_INSENSITIVE\n" \ + "mail: CASE_INSENSITIVE\n" \ "\n" \ "dn: @INDEXLIST\n" \ "@IDXATTR: cn\n" \ @@ -191,6 +193,7 @@ int sysdb_upgrade_19(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_20(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_21(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_22(struct sysdb_ctx *sysdb, const char **ver); +int sysdb_upgrade_23(struct sysdb_ctx *sysdb, const char **ver); int sysdb_ts_upgrade_01(struct sysdb_ctx *sysdb, const char **ver); diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index 346a1cb0b39..56083e6be06 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c @@ -2718,6 +2718,62 @@ int sysdb_upgrade_22(struct sysdb_ctx *sysdb, const char **ver) return ret; } +int sysdb_upgrade_23(struct sysdb_ctx *sysdb, const char **ver) +{ + TALLOC_CTX *tmp_ctx; + int ret; + struct ldb_message *msg; + struct upgrade_ctx *ctx; + + tmp_ctx = talloc_new(NULL); + if (!tmp_ctx) { + return ENOMEM; + } + + ret = commence_upgrade(sysdb, sysdb->ldb, SYSDB_VERSION_0_24, &ctx); + if (ret) { + return ret; + } + + /* Add new indexes */ + msg = ldb_msg_new(tmp_ctx); + if (!msg) { + ret = ENOMEM; + goto done; + } + msg->dn = ldb_dn_new(tmp_ctx, sysdb->ldb, "@ATTRIBUTES"); + if (!msg->dn) { + ret = ENOMEM; + goto done; + } + + /* Case insensitive search for mail */ + ret = ldb_msg_add_empty(msg, SYSDB_USER_EMAIL, LDB_FLAG_MOD_ADD, NULL); + if (ret != LDB_SUCCESS) { + ret = ENOMEM; + goto done; + } + ret = ldb_msg_add_string(msg, SYSDB_USER_EMAIL, "CASE_INSENSITIVE"); + if (ret != LDB_SUCCESS) { + ret = ENOMEM; + goto done; + } + + ret = ldb_modify(sysdb->ldb, msg); + if (ret != LDB_SUCCESS) { + ret = sysdb_error_to_errno(ret); + goto done; + } + + /* conversion done, update version number */ + ret = update_version(ctx); + +done: + ret = finish_upgrade(ret, &ctx, ver); + talloc_free(tmp_ctx); + return ret; +} + int sysdb_ts_upgrade_01(struct sysdb_ctx *sysdb, const char **ver) { struct upgrade_ctx *ctx; From ae4387cbc1bc2cff3234e21f03d62d6bb92c71da Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 23 Jan 2024 13:47:53 +0100 Subject: [PATCH 2/8] sdap: add search_bases option to groups_by_user_send() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AD handles users and computer objects very similar and so does SSSD's GPO code when lookup up the host's group-memberships. But users and computers might be stored in different sub-tree of the AD LDAP tree and if a dedicated user search base is given with the ldap_user_search_base option in sssd.conf the host object might be in a different sub-tree. To make sure the host can still be found this patch uses the base DN of the LDAP tree when searching for hosts in the GPO code. Resolves: https://github.com/SSSD/sssd/issues/5708 Reviewed-by: Alejandro López Reviewed-by: Tomáš Halman (cherry picked from commit 29a77c6e79020d7e8cb474b4d3b394d390eba196) (cherry picked from commit a7621a5b464af7a3c8409dcbde038b35fee2c895) --- src/providers/ad/ad_gpo.c | 10 ++++++++++ src/providers/ldap/ldap_common.h | 1 + src/providers/ldap/ldap_id.c | 6 +++++- src/providers/ldap/sdap_async.h | 1 + src/providers/ldap/sdap_async_initgroups.c | 4 +++- 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 94959c36b46..b0ee3e616b4 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -2091,6 +2091,7 @@ ad_gpo_connect_done(struct tevent_req *subreq) char *server_uri; LDAPURLDesc *lud; struct sdap_domain *sdom; + struct sdap_search_base **search_bases; req = tevent_req_callback_data(subreq, struct tevent_req); state = tevent_req_data(req, struct ad_gpo_access_state); @@ -2184,9 +2185,18 @@ ad_gpo_connect_done(struct tevent_req *subreq) goto done; } + ret = common_parse_search_base(state, sdom->basedn, state->ldb_ctx, + "AD_HOSTS", NULL, &search_bases); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Failed to create dedicated search base for host lookups, " + "trying with user search base."); + } + subreq = groups_by_user_send(state, state->ev, state->access_ctx->ad_id_ctx->sdap_id_ctx, sdom, state->conn, + search_bases, state->host_fqdn, BE_FILTER_NAME, NULL, diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 7159d635641..2c984ef50c4 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -304,6 +304,7 @@ struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx, struct sdap_id_ctx *ctx, struct sdap_domain *sdom, struct sdap_id_conn_ctx *conn, + struct sdap_search_base **search_bases, const char *filter_value, int filter_type, const char *extra_value, diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index da54816bdf9..b3ea2333f18 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -1139,6 +1139,7 @@ struct groups_by_user_state { struct sdap_id_op *op; struct sysdb_ctx *sysdb; struct sss_domain_info *domain; + struct sdap_search_base **search_bases; const char *filter_value; int filter_type; @@ -1160,6 +1161,7 @@ struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx, struct sdap_id_ctx *ctx, struct sdap_domain *sdom, struct sdap_id_conn_ctx *conn, + struct sdap_search_base **search_bases, const char *filter_value, int filter_type, const char *extra_value, @@ -1192,6 +1194,7 @@ struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx, state->extra_value = extra_value; state->domain = sdom->dom; state->sysdb = sdom->dom->sysdb; + state->search_bases = search_bases; if (state->domain->type == DOM_TYPE_APPLICATION || set_non_posix) { state->non_posix = true; @@ -1254,6 +1257,7 @@ static void groups_by_user_connect_done(struct tevent_req *subreq) sdap_id_op_handle(state->op), state->ctx, state->conn, + state->search_bases, state->filter_value, state->filter_type, state->extra_value, @@ -1449,7 +1453,7 @@ sdap_handle_acct_req_send(TALLOC_CTX *mem_ctx, } subreq = groups_by_user_send(state, be_ctx->ev, id_ctx, - sdom, conn, + sdom, conn, NULL, ar->filter_value, ar->filter_type, ar->extra_value, diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index 5458d21f10c..89245f41fac 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -158,6 +158,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx, struct sdap_handle *sh, struct sdap_id_ctx *id_ctx, struct sdap_id_conn_ctx *conn, + struct sdap_search_base **search_bases, const char *name, int filter_type, const char *extra_value, diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 97be594a389..fb3d8fe2462 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -2732,6 +2732,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx, struct sdap_handle *sh, struct sdap_id_ctx *id_ctx, struct sdap_id_conn_ctx *conn, + struct sdap_search_base **search_bases, const char *filter_value, int filter_type, const char *extra_value, @@ -2764,7 +2765,8 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx, state->orig_user = NULL; state->timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT); state->user_base_iter = 0; - state->user_search_bases = sdom->user_search_bases; + state->user_search_bases = (search_bases == NULL) ? sdom->user_search_bases + : search_bases; if (!state->user_search_bases) { DEBUG(SSSDBG_CRIT_FAILURE, "Initgroups lookup request without a user search base\n"); From 74cb354b358b2cfe0fffb2652b2f0132d7b6d9b6 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Wed, 24 Jan 2024 14:21:12 +0100 Subject: [PATCH 3/8] sdap: add naming_context as new member of struct sdap_domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The naming_context could be a more reliable source than basedn for the actual base DN because basedn is set very early from the domain name given in sssd.conf. Although it is recommended to use the fully qualified DNS domain name here it is not required. As a result basedn might not reflect the actual based DN of the LDAP server. Also pure LDAP server (i.e. not AD or FreeIPA) might use different schemes to set the base DN which will not be based on the DNS domain of the LDAP server. Resolves: https://github.com/SSSD/sssd/issues/5708 Reviewed-by: Alejandro López Reviewed-by: Tomáš Halman (cherry picked from commit a153f13f296401247a862df2b99048bb1bbb8e2e) (cherry picked from commit 6a8e60df84d5d2565bec36be19c2def25a6ece1f) --- src/providers/ad/ad_gpo.c | 6 ++++-- src/providers/ldap/sdap.c | 36 +++++++++++++----------------------- src/providers/ldap/sdap.h | 11 +++++++++++ 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index b0ee3e616b4..3d1ad39c729 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -2185,8 +2185,10 @@ ad_gpo_connect_done(struct tevent_req *subreq) goto done; } - ret = common_parse_search_base(state, sdom->basedn, state->ldb_ctx, - "AD_HOSTS", NULL, &search_bases); + ret = common_parse_search_base(state, + sdom->naming_context == NULL ? sdom->basedn + : sdom->naming_context, + state->ldb_ctx, "AD_HOSTS", NULL, &search_bases); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Failed to create dedicated search base for host lookups, " diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c index f5637c5fbf5..956eba93a22 100644 --- a/src/providers/ldap/sdap.c +++ b/src/providers/ldap/sdap.c @@ -1252,19 +1252,10 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, struct sdap_domain *sdom) { int ret; - char *naming_context = NULL; - if (!sdom->search_bases - || !sdom->user_search_bases - || !sdom->group_search_bases - || !sdom->netgroup_search_bases - || !sdom->host_search_bases - || !sdom->sudo_search_bases - || !sdom->iphost_search_bases - || !sdom->ipnetwork_search_bases - || !sdom->autofs_search_bases) { - naming_context = get_naming_context(opts->basic, rootdse); - if (naming_context == NULL) { + if (!sdom->naming_context) { + sdom->naming_context = get_naming_context(sdom, rootdse); + if (sdom->naming_context == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "get_naming_context failed.\n"); /* This has to be non-fatal, since some servers offer @@ -1280,7 +1271,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1288,7 +1279,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->user_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_USER_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1296,7 +1287,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->group_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_GROUP_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1304,7 +1295,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->netgroup_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_NETGROUP_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1312,7 +1303,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->host_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_HOST_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1320,7 +1311,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->sudo_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_SUDO_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1328,7 +1319,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->service_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_SERVICE_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1336,7 +1327,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->autofs_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_AUTOFS_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1344,7 +1335,7 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->iphost_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_IPHOST_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } @@ -1352,14 +1343,13 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse, if (!sdom->ipnetwork_search_bases) { ret = sdap_set_search_base(opts, sdom, SDAP_IPNETWORK_SEARCH_BASE, - naming_context); + sdom->naming_context); if (ret != EOK) goto done; } ret = EOK; done: - talloc_free(naming_context); return ret; } diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 161bc5c269f..103d50ed417 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -454,6 +454,17 @@ struct sdap_domain { char *basedn; + /* The naming_context could be a more reliable source than basedn for the + * actual base DN because basedn is set very early from the domain name + * given in sssd.conf. Although it is recommended to use the fully + * qualified DNS domain name here it is not required. As a result basedn + * might not reflect the actual based DN of the LDAP server. Also pure + * LDAP server (i.e. not AD or FreeIPA) might use different schemes to set + * the base DN which will not be based on the DNS domain of the LDAP + * server. naming_context might be NULL even after connection to an LDAP + * server. */ + char *naming_context; + struct sdap_search_base **search_bases; struct sdap_search_base **user_search_bases; struct sdap_search_base **group_search_bases; From 251ae7906be198af3a07dd047f3b22fc559cc56c Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 18 Jan 2024 13:08:17 +0100 Subject: [PATCH 4/8] pam: fix SC auth with multiple certs and missing login name While introducing the local_auth_policy option a quite specific use-case was not covered correctly. If there are multiple matching certificates on the Smartcard, 'local_auth_policy = only' is set and GDM's Smartcard mode was used for login, i.e. there is no user name given and the user has to be derived from the certificate used for login, authentication failed. The main reason for the failure is that in this case the Smartcard interaction and the user mapping has to be done first to determine the user before local_auth_policy is evaluated. As a result when checking if the authentication can be finished the request was in an unexpected state because the indicator for local Smartcard authentication was not enabled. Resolves: https://github.com/SSSD/sssd/issues/7109 Reviewed-by: Justin Stephenson Reviewed-by: Scott Poore (cherry picked from commit 44ec3e4638b0c6f7f45a3390a28c2e8745d52bc3) (cherry picked from commit 50077c3255177fe1b01837fbe31a7f8fd47dee74) --- src/responder/pam/pamsrv.h | 10 ++++ src/responder/pam/pamsrv_cmd.c | 17 +++++-- src/tests/intg/Makefile.am | 2 + src/tests/intg/test_pam_responder.py | 74 +++++++++++++++++++++++++++- 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h index 7013a8edd66..61883618944 100644 --- a/src/responder/pam/pamsrv.h +++ b/src/responder/pam/pamsrv.h @@ -93,7 +93,17 @@ struct pam_auth_req { struct ldb_message *user_obj; struct cert_auth_info *cert_list; struct cert_auth_info *current_cert; + /* Switched to 'true' if the backend indicates that it cannot handle + * Smartcard authentication, but Smartcard authentication is + * possible and local Smartcard authentication is allowed. */ bool cert_auth_local; + /* Switched to 'true' if authentication (not pre-authentication) was + * started without a login name and the name had to be lookup up with the + * certificate used for authentication. Since reading the certificate from + * the Smartcard already involves the PIN validation in this case there + * would be no need for an additional Smartcard interaction if only local + * Smartcard authentication is possible. */ + bool initial_cert_auth_successful; bool passkey_data_exists; uint32_t client_id_num; diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index c23ea7ba417..a7c18173358 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -2200,8 +2200,8 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) ret = ENOENT; goto done; } - - if (cert_count > 1) { + /* Multiple certificates are only expected during pre-auth */ + if (cert_count > 1 && preq->pd->cmd == SSS_PAM_PREAUTH) { for (preq->current_cert = preq->cert_list; preq->current_cert != NULL; preq->current_cert = sss_cai_get_next(preq->current_cert)) { @@ -2285,7 +2285,9 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) } /* If logon_name was not given during authentication add a - * SSS_PAM_CERT_INFO message to send the name to the caller. */ + * SSS_PAM_CERT_INFO message to send the name to the caller. + * Additionally initial_cert_auth_successful is set to + * indicate that the user is already authenticated. */ if (preq->pd->cmd == SSS_PAM_AUTHENTICATE && preq->pd->logon_name == NULL) { ret = add_pam_cert_response(preq->pd, @@ -2297,6 +2299,8 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) preq->pd->pam_status = PAM_AUTHINFO_UNAVAIL; goto done; } + + preq->initial_cert_auth_successful = true; } /* cert_user will be returned to the PAM client as user name, so @@ -2851,12 +2855,15 @@ static void pam_dom_forwarder(struct pam_auth_req *preq) if (found) { if (local_policy != NULL && strcasecmp(local_policy, "only") == 0) { talloc_free(tmp_ctx); - DEBUG(SSSDBG_IMPORTANT_INFO, "Local auth only set, skipping online auth\n"); + DEBUG(SSSDBG_IMPORTANT_INFO, + "Local auth only set and matching certificate was found, " + "skipping online auth\n"); if (preq->pd->cmd == SSS_PAM_PREAUTH) { preq->pd->pam_status = PAM_SUCCESS; } else if (preq->pd->cmd == SSS_PAM_AUTHENTICATE && IS_SC_AUTHTOK(preq->pd->authtok) - && preq->cert_auth_local) { + && (preq->cert_auth_local + || preq->initial_cert_auth_successful)) { preq->pd->pam_status = PAM_SUCCESS; preq->callback = pam_reply; } diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index 3866d3ca63c..0cfd268dce7 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -199,6 +199,7 @@ clean-local: PAM_CERT_DB_PATH="$(abs_builddir)/../test_CA/SSSD_test_CA.pem" SOFTHSM2_CONF="$(abs_builddir)/../test_CA/softhsm2_one.conf" +SOFTHSM2_TWO_CONF="$(abs_builddir)/../test_CA/softhsm2_two.conf" intgcheck-installed: config.py passwd group pam_sss_service pam_sss_alt_service pam_sss_sc_required pam_sss_try_sc pam_sss_allow_missing_name pam_sss_domains sss_netgroup_thread_test pipepath="$(DESTDIR)$(pipepath)"; \ @@ -233,6 +234,7 @@ intgcheck-installed: config.py passwd group pam_sss_service pam_sss_alt_service PAM_CERT_DB_PATH=$(PAM_CERT_DB_PATH) \ ABS_SRCDIR=$(abs_srcdir) \ SOFTHSM2_CONF=$(SOFTHSM2_CONF) \ + SOFTHSM2_TWO_CONF=$(SOFTHSM2_TWO_CONF) \ KCM_RENEW=$(KCM_RENEW) \ FILES_PROVIDER=$(FILES_PROVIDER) \ DBUS_SOCK_DIR="$(DESTDIR)$(runstatedir)/dbus/" \ diff --git a/src/tests/intg/test_pam_responder.py b/src/tests/intg/test_pam_responder.py index 1fc3937e6f4..0fbf8065e49 100644 --- a/src/tests/intg/test_pam_responder.py +++ b/src/tests/intg/test_pam_responder.py @@ -168,7 +168,7 @@ def format_pam_cert_auth_conf(config, provider): {provider.p} [certmap/auth_only/user1] - matchrule = .*CN=SSSD test cert 0001.* + matchrule = .*CN=SSSD test cert 000[12].* """).format(**locals()) @@ -201,7 +201,7 @@ def format_pam_cert_auth_conf_name_format(config, provider): {provider.p} [certmap/auth_only/user1] - matchrule = .*CN=SSSD test cert 0001.* + matchrule = .*CN=SSSD test cert 000[12].* """).format(**locals()) @@ -380,6 +380,28 @@ def simple_pam_cert_auth_no_cert(request, passwd_ops_setup): return None +@pytest.fixture +def simple_pam_cert_auth_two_certs(request, passwd_ops_setup): + """Setup SSSD with pam_cert_auth=True""" + config.PAM_CERT_DB_PATH = os.environ['PAM_CERT_DB_PATH'] + + old_softhsm2_conf = os.environ['SOFTHSM2_CONF'] + softhsm2_two_conf = os.environ['SOFTHSM2_TWO_CONF'] + os.environ['SOFTHSM2_CONF'] = softhsm2_two_conf + + conf = format_pam_cert_auth_conf(config, provider_switch(request.param)) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + + os.environ['SOFTHSM2_CONF'] = old_softhsm2_conf + + passwd_ops_setup.useradd(**USER1) + passwd_ops_setup.useradd(**USER2) + sync_files_provider(USER2['name']) + + return None + + @pytest.fixture def simple_pam_cert_auth_name_format(request, passwd_ops_setup): """Setup SSSD with pam_cert_auth=True and full_name_format""" @@ -522,6 +544,54 @@ def test_sc_auth(simple_pam_cert_auth, env_for_sssctl): assert err.find("pam_authenticate for user [user1]: Success") != -1 +@pytest.mark.parametrize('simple_pam_cert_auth_two_certs', provider_list(), indirect=True) +def test_sc_auth_two(simple_pam_cert_auth_two_certs, env_for_sssctl): + + sssctl = subprocess.Popen(["sssctl", "user-checks", "user1", + "--action=auth", "--service=pam_sss_service"], + universal_newlines=True, + env=env_for_sssctl, stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + try: + out, err = sssctl.communicate(input="2\n123456") + except Exception: + sssctl.kill() + out, err = sssctl.communicate() + + sssctl.stdin.close() + sssctl.stdout.close() + + if sssctl.wait() != 0: + raise Exception("sssctl failed") + + assert err.find("pam_authenticate for user [user1]: Success") != -1 + + +@pytest.mark.parametrize('simple_pam_cert_auth_two_certs', provider_list(), indirect=True) +def test_sc_auth_two_missing_name(simple_pam_cert_auth_two_certs, env_for_sssctl): + + sssctl = subprocess.Popen(["sssctl", "user-checks", "", + "--action=auth", "--service=pam_sss_allow_missing_name"], + universal_newlines=True, + env=env_for_sssctl, stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + try: + out, err = sssctl.communicate(input="2\n123456") + except Exception: + sssctl.kill() + out, err = sssctl.communicate() + + sssctl.stdin.close() + sssctl.stdout.close() + + if sssctl.wait() != 0: + raise Exception("sssctl failed") + + assert err.find("pam_authenticate for user [user1]: Success") != -1 + + @pytest.mark.parametrize('simple_pam_cert_auth', ['proxy_password'], indirect=True) def test_sc_proxy_password_fallback(simple_pam_cert_auth, env_for_sssctl): """ From 89207365d9959799826c44fc0e913347f6c75808 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Wed, 8 Nov 2023 14:50:24 +0100 Subject: [PATCH 5/8] ad-gpo: use hash to store intermediate results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently after the evaluation of a single GPO file the intermediate results are stored in the cache and this cache entry is updated until all applicable GPO files are evaluated. Finally the data in the cache is used to make the decision of access is granted or rejected. If there are two or more access-control request running in parallel one request might overwrite the cache object with intermediate data while another request reads the cached data for the access decision and as a result will do this decision based on intermediate data. To avoid this the intermediate results are not stored in the cache anymore but in hash tables which are specific to the request. Only the final result is written to the cache to have it available for offline authentication. Reviewed-by: Alexey Tikhonov Reviewed-by: Tomáš Halman (cherry picked from commit d7db7971682da2dbf7642ac94940d6b0577ec35a) (cherry picked from commit e1bfbc2493c4194988acc3b2413df3dde0735ae3) --- src/providers/ad/ad_gpo.c | 116 +++++++++++++++++++++++++++++++++----- 1 file changed, 102 insertions(+), 14 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 3d1ad39c729..b879b0a0800 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1431,6 +1431,33 @@ ad_gpo_extract_policy_setting(TALLOC_CTX *mem_ctx, return ret; } +static errno_t +add_result_to_hash(hash_table_t *hash, const char *key, char *value) +{ + int hret; + hash_key_t k; + hash_value_t v; + + if (hash == NULL || key == NULL || value == NULL) { + return EINVAL; + } + + k.type = HASH_KEY_CONST_STRING; + k.c_str = key; + + v.type = HASH_VALUE_PTR; + v.ptr = value; + + hret = hash_enter(hash, &k, &v); + if (hret != HASH_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to add [%s][%s] to hash: [%s].\n", + key, value, hash_error_string(hret)); + return EIO; + } + + return EOK; +} + /* * This function parses the cse-specific (GP_EXT_GUID_SECURITY) filename, * and stores the allow_key and deny_key of all of the gpo_map_types present @@ -1438,6 +1465,7 @@ ad_gpo_extract_policy_setting(TALLOC_CTX *mem_ctx, */ static errno_t ad_gpo_store_policy_settings(struct sss_domain_info *domain, + hash_table_t *allow_maps, hash_table_t *deny_maps, const char *filename) { struct ini_cfgfile *file_ctx = NULL; @@ -1571,14 +1599,14 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, goto done; } else if (ret != ENOENT) { const char *value = allow_value ? allow_value : empty_val; - ret = sysdb_gpo_store_gpo_result_setting(domain, - allow_key, - value); + ret = add_result_to_hash(allow_maps, allow_key, + talloc_strdup(allow_maps, value)); if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "sysdb_gpo_store_gpo_result_setting failed for key:" - "'%s' value:'%s' [%d][%s]\n", allow_key, allow_value, - ret, sss_strerror(ret)); + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add key: [%s] " + "value: [%s] to allow maps " + "[%d][%s].\n", + allow_key, value, ret, + sss_strerror(ret)); goto done; } } @@ -1598,14 +1626,14 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, goto done; } else if (ret != ENOENT) { const char *value = deny_value ? deny_value : empty_val; - ret = sysdb_gpo_store_gpo_result_setting(domain, - deny_key, - value); + ret = add_result_to_hash(deny_maps, deny_key, + talloc_strdup(deny_maps, value)); if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "sysdb_gpo_store_gpo_result_setting failed for key:" - "'%s' value:'%s' [%d][%s]\n", deny_key, deny_value, - ret, sss_strerror(ret)); + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add key: [%s] " + "value: [%s] to deny maps " + "[%d][%s].\n", + deny_key, value, ret, + sss_strerror(ret)); goto done; } } @@ -1902,6 +1930,8 @@ struct ad_gpo_access_state { int num_cse_filtered_gpos; int cse_gpo_index; const char *ad_domain; + hash_table_t *allow_maps; + hash_table_t *deny_maps; }; static void ad_gpo_connect_done(struct tevent_req *subreq); @@ -2023,6 +2053,19 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, goto immediately; } + ret = sss_hash_create(state, 0, &state->allow_maps); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Could not create allow maps " + "hash table [%d]: %s\n", ret, sss_strerror(ret)); + goto immediately; + } + + ret = sss_hash_create(state, 0, &state->deny_maps); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Could not create deny maps " + "hash table [%d]: %s\n", ret, sss_strerror(ret)); + goto immediately; + } subreq = sdap_id_op_connect_send(state->sdap_op, state, &ret); if (subreq == NULL) { @@ -2713,6 +2756,43 @@ ad_gpo_cse_step(struct tevent_req *req) return EAGAIN; } +static errno_t +store_hash_maps_in_cache(struct sss_domain_info *domain, + hash_table_t *allow_maps, hash_table_t *deny_maps) +{ + int ret; + struct hash_iter_context_t *iter; + hash_entry_t *entry; + size_t c; + hash_table_t *hash_list[] = { allow_maps, deny_maps, NULL}; + + + for (c = 0; hash_list[c] != NULL; c++) { + iter = new_hash_iter_context(hash_list[c]); + if (iter == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to create hash iterator.\n"); + return EINVAL; + } + + while ((entry = iter->next(iter)) != NULL) { + ret = sysdb_gpo_store_gpo_result_setting(domain, + entry->key.c_str, + entry->value.ptr); + if (ret != EOK) { + free(iter); + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_gpo_store_gpo_result_setting failed for key:" + "[%s] value:[%s] [%d][%s]\n", entry->key.c_str, + (char *) entry->value.ptr, ret, sss_strerror(ret)); + return ret; + } + } + talloc_free(iter); + } + + return EOK; +} + /* * This cse-specific function (GP_EXT_GUID_SECURITY) increments the * cse_gpo_index until the policy settings for all applicable GPOs have been @@ -2754,6 +2834,7 @@ ad_gpo_cse_done(struct tevent_req *subreq) * (as part of the GPO Result object in the sysdb cache). */ ret = ad_gpo_store_policy_settings(state->host_domain, + state->allow_maps, state->deny_maps, cse_filtered_gpo->policy_filename); if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_OP_FAILURE, @@ -2767,6 +2848,13 @@ ad_gpo_cse_done(struct tevent_req *subreq) if (ret == EOK) { /* ret is EOK only after all GPO policy files have been downloaded */ + ret = store_hash_maps_in_cache(state->host_domain, + state->allow_maps, state->deny_maps); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to store evaluated GPO maps " + "[%d][%s].\n", ret, sss_strerror(ret)); + goto done; + } ret = ad_gpo_perform_hbac_processing(state, state->gpo_mode, state->gpo_map_type, From c01218630260e8e1dd4e4d48c210d6307ffd35dc Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 1 Mar 2024 10:50:07 +0100 Subject: [PATCH 6/8] ad: refresh root domain when read directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the domain object of the forest root domain cannot be found in the LDAP tree of the local AD domain SSSD tries to read the request data from an LDAP server of the forest root domain directly. After reading this data the information is stored in the cache but currently the information about the domain store in memory is not updated with the additional data. As a result e.g. the domain SID is missing in this data and only becomes available after a restart where it is read from the cache. With this patch an unconditional refresh is triggered at the end of the fallback code path. Resolves: https://github.com/SSSD/sssd/issues/7250 Reviewed-by: Dan Lavu Reviewed-by: Tomáš Halman (cherry picked from commit 0de6c33047ac7a2b5316ec5ec936d6b675671c53) (cherry picked from commit db27a51f274640e1aa2f13476c80955a3ec9e91c) --- src/providers/ad/ad_subdomains.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index a8d1892cc6e..d8f3738ce9f 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -1395,7 +1395,7 @@ struct ad_get_root_domain_state { static void ad_get_root_domain_done(struct tevent_req *subreq); static void ad_check_root_domain_done(struct tevent_req *subreq); static errno_t -ad_get_root_domain_refresh(struct ad_get_root_domain_state *state); +ad_get_root_domain_refresh(struct ad_get_root_domain_state *state, bool refresh); struct tevent_req * ad_check_domain_send(TALLOC_CTX *mem_ctx, @@ -1582,7 +1582,7 @@ static void ad_get_root_domain_done(struct tevent_req *subreq) return; } - ret = ad_get_root_domain_refresh(state); + ret = ad_get_root_domain_refresh(state, false); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "ad_get_root_domain_refresh() failed.\n"); } @@ -1682,7 +1682,7 @@ static void ad_check_root_domain_done(struct tevent_req *subreq) state->reply_count = 1; - ret = ad_get_root_domain_refresh(state); + ret = ad_get_root_domain_refresh(state, true); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "ad_get_root_domain_refresh() failed.\n"); } @@ -1697,7 +1697,7 @@ static void ad_check_root_domain_done(struct tevent_req *subreq) } static errno_t -ad_get_root_domain_refresh(struct ad_get_root_domain_state *state) +ad_get_root_domain_refresh(struct ad_get_root_domain_state *state, bool refresh) { struct sss_domain_info *root_domain; bool has_changes; @@ -1713,7 +1713,7 @@ ad_get_root_domain_refresh(struct ad_get_root_domain_state *state) goto done; } - if (has_changes) { + if (has_changes || refresh) { ret = ad_subdom_reinit(state->sd_ctx); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Could not reinitialize subdomains\n"); From f5bfcb9e055e31fb8cc5c954a7228130c8023a81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 30 Apr 2024 12:28:53 +0200 Subject: [PATCH 7/8] failover: add failover_primary_timeout option This was previously hardcoded to 31 seconds (hardcoded retry_timout + 1). This may be too short period under some circumstances. When we retry primary server we drop connection to the backup server and if the primary server is not yet available (and there are many unavailable primary servers) we may go through a long timeout cycle every half minute. This patch makes the value configurable. :config: Added `failover_primary_timout` configuration option. This can be used to configure how often SSSD tries to reconnect to a primary server after a successful connection to a backup server. This was previously hardcoded to 31 seconds which is kept as the default value. Resolves: https://github.com/SSSD/sssd/issues/7375 Reviewed-by: Alexey Tikhonov Reviewed-by: Iker Pedrosa (cherry picked from commit e9738e36937e78f80bb2772c48cffbddf39bd5fe) (cherry picked from commit 14f32f681a25aac185d72bc6d22a9e3b59dd265a) --- src/config/SSSDConfig/sssdoptions.py | 2 + src/config/SSSDConfigTest.py | 2 + src/config/cfg_rules.ini | 1 + src/config/etc/sssd.api.conf | 1 + src/man/sssd.conf.5.xml | 19 ++++++++ src/providers/data_provider.h | 1 + src/providers/data_provider_fo.c | 14 +++++- src/providers/fail_over.c | 10 +++++ src/providers/fail_over.h | 3 ++ src/tests/system/tests/test_failover.py | 59 +++++++++++++++++++++++++ 10 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 src/tests/system/tests/test_failover.py diff --git a/src/config/SSSDConfig/sssdoptions.py b/src/config/SSSDConfig/sssdoptions.py index 0d75e6d822d..95b39aa59b5 100644 --- a/src/config/SSSDConfig/sssdoptions.py +++ b/src/config/SSSDConfig/sssdoptions.py @@ -186,6 +186,8 @@ def __init__(self): 'dns_resolver_op_timeout': _('How long should keep trying to resolve single DNS query (seconds)'), 'dns_resolver_timeout': _('How long to wait for replies from DNS when resolving servers (seconds)'), 'dns_discovery_domain': _('The domain part of service discovery DNS query'), + 'failover_primary_timeout': _('How often SSSD tries to reconnect to the primary server after a successful ' + 'connection to the backup server.'), 'override_gid': _('Override GID value from the identity provider with this value'), 'case_sensitive': _('Treat usernames as case sensitive'), 'entry_cache_user_timeout': _('Entry cache timeout length (seconds)'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index b160be2b128..f333c35eb3c 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -579,6 +579,7 @@ def testListOptions(self): 'dns_resolver_op_timeout', 'dns_resolver_timeout', 'dns_discovery_domain', + 'failover_primary_timeout', 'dyndns_update', 'dyndns_ttl', 'dyndns_iface', @@ -939,6 +940,7 @@ def testRemoveProvider(self): 'dns_resolver_op_timeout', 'dns_resolver_timeout', 'dns_discovery_domain', + 'failover_primary_timeout', 'dyndns_update', 'dyndns_ttl', 'dyndns_iface', diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 61cbdafaade..4ad358f08b5 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -404,6 +404,7 @@ option = dns_resolver_op_timeout option = dns_resolver_timeout option = dns_resolver_use_search_list option = dns_discovery_domain +option = failover_primary_timeout option = override_gid option = case_sensitive option = override_homedir diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 5ae6aab1905..31787c23c5c 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -172,6 +172,7 @@ dns_resolver_server_timeout = int, None, false dns_resolver_op_timeout = int, None, false dns_resolver_timeout = int, None, false dns_discovery_domain = str, None, false +failover_primary_timeout = int, None, false override_gid = int, None, false case_sensitive = str, None, false override_homedir = str, None, false diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index e7a8cbd9a40..83bf59ab5de 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -3773,6 +3773,25 @@ pam_gssapi_indicators_map = sudo:pkinit, sudo-i:pkinit + + failover_primary_timeout (integer) + + + When no primary server is currently available, + SSSD fail overs to a backup server. This option + defines the amount of time (in seconds) to + wait before SSSD tries to reconnect to a primary + server again. + + + Note: The minimum value is 31. + + + Default: 31 + + + + override_gid (integer) diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h index 36a82b84d7d..def35e491a3 100644 --- a/src/providers/data_provider.h +++ b/src/providers/data_provider.h @@ -267,6 +267,7 @@ enum dp_res_opts { DP_RES_OPT_RESOLVER_SERVER_TIMEOUT, DP_RES_OPT_RESOLVER_USE_SEARCH_LIST, DP_RES_OPT_DNS_DOMAIN, + DP_RES_OPT_FAILOVER_PRIMARY_TIMEOUT, DP_RES_OPTS /* attrs counter */ }; diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c index b0aed54e97b..c23f92e3556 100644 --- a/src/providers/data_provider_fo.c +++ b/src/providers/data_provider_fo.c @@ -48,10 +48,20 @@ static int be_fo_get_options(struct be_ctx *ctx, DP_RES_OPT_RESOLVER_TIMEOUT); opts->use_search_list = dp_opt_get_bool(ctx->be_res->opts, DP_RES_OPT_RESOLVER_USE_SEARCH_LIST); + opts->primary_timeout = dp_opt_get_int(ctx->be_res->opts, + DP_RES_OPT_FAILOVER_PRIMARY_TIMEOUT); + opts->retry_timeout = 30; opts->srv_retry_neg_timeout = 15; opts->family_order = ctx->be_res->family_order; + if (opts->primary_timeout <= opts->retry_timeout) { + opts->primary_timeout = opts->retry_timeout + 1; + DEBUG(SSSDBG_CONF_SETTINGS, + "Warning: failover_primary_timeout is too low, using %lu " + "seconds instead\n", opts->primary_timeout); + } + return EOK; } @@ -551,7 +561,7 @@ static void be_resolve_server_done(struct tevent_req *subreq) struct tevent_req); struct be_resolve_server_state *state = tevent_req_data(req, struct be_resolve_server_state); - time_t timeout = fo_get_service_retry_timeout(state->svc->fo_service) + 1; + time_t timeout = fo_get_primary_retry_timeout(state->svc->fo_service); int ret; ret = be_resolve_server_process(subreq, state, &new_subreq); @@ -564,7 +574,6 @@ static void be_resolve_server_done(struct tevent_req *subreq) } if (!fo_is_server_primary(state->srv)) { - /* FIXME: make the timeout configurable */ ret = be_primary_server_timeout_activate(state->ctx, state->ev, state->ctx, state->svc, timeout); @@ -871,6 +880,7 @@ static struct dp_option dp_res_default_opts[] = { { "dns_resolver_server_timeout", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER }, { "dns_resolver_use_search_list", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "dns_discovery_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING }, + { "failover_primary_timeout", DP_OPT_NUMBER, { .number = 31 }, NULL_NUMBER }, DP_OPTION_TERMINATOR }; diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c index 7cb64244877..7f94407c538 100644 --- a/src/providers/fail_over.c +++ b/src/providers/fail_over.c @@ -158,6 +158,7 @@ fo_context_init(TALLOC_CTX *mem_ctx, struct fo_options *opts) ctx->opts->srv_retry_neg_timeout = opts->srv_retry_neg_timeout; ctx->opts->retry_timeout = opts->retry_timeout; + ctx->opts->primary_timeout = opts->primary_timeout; ctx->opts->family_order = opts->family_order; ctx->opts->service_resolv_timeout = opts->service_resolv_timeout; ctx->opts->use_search_list = opts->use_search_list; @@ -1740,6 +1741,15 @@ time_t fo_get_service_retry_timeout(struct fo_service *svc) return svc->ctx->opts->retry_timeout; } +time_t fo_get_primary_retry_timeout(struct fo_service *svc) +{ + if (svc == NULL || svc->ctx == NULL || svc->ctx->opts == NULL) { + return 0; + } + + return svc->ctx->opts->primary_timeout; +} + bool fo_get_use_search_list(struct fo_server *server) { if ( diff --git a/src/providers/fail_over.h b/src/providers/fail_over.h index 36021ad6ffb..924a09970b1 100644 --- a/src/providers/fail_over.h +++ b/src/providers/fail_over.h @@ -83,6 +83,7 @@ struct fo_server; struct fo_options { time_t srv_retry_neg_timeout; time_t retry_timeout; + time_t primary_timeout; int service_resolv_timeout; bool use_search_list; enum restrict_family family_order; @@ -211,6 +212,8 @@ int fo_is_srv_lookup(struct fo_server *s); time_t fo_get_service_retry_timeout(struct fo_service *svc); +time_t fo_get_primary_retry_timeout(struct fo_service *svc); + bool fo_get_use_search_list(struct fo_server *server); void fo_reset_services(struct fo_ctx *fo_ctx); diff --git a/src/tests/system/tests/test_failover.py b/src/tests/system/tests/test_failover.py new file mode 100644 index 00000000000..565cec9bc40 --- /dev/null +++ b/src/tests/system/tests/test_failover.py @@ -0,0 +1,59 @@ +""" +SSSD Failover tests. + +:requirement: Failover +""" + +from __future__ import annotations + +import pytest +from sssd_test_framework.roles.client import Client +from sssd_test_framework.roles.ldap import LDAP +from sssd_test_framework.topology import KnownTopology + + +@pytest.mark.parametrize("value, expected", [(None, 31), (15, 31), (60, 60)]) +@pytest.mark.importance("low") +@pytest.mark.ticket(gh=7375, jira="RHEL-17659") +@pytest.mark.topology(KnownTopology.LDAP) +def test_failover__retry_primary(client: Client, ldap: LDAP, value: int | None, expected: int): + """ + :title: Primary server reactivation timeout is respected + :setup: + 1. Create LDAP user "user-1" + 2. Set failover_primary_timeout to @value + 3. Set ldap_uri to invalid, not working server + 4. Set ldap_backup_uri to working server + 5. Start SSSD + :steps: + 1. Lookup user-1 + 2. Check that SSSD is connected to backup server + 3. Find "Primary server reactivation timeout set to @expected seconds" in domain logs + :expectedresults: + 1. SSSD failover to backup server + 2. SSSD is indeed connected to the backup server + 3. String is found + :customerscenario: True + """ + ldap.user("user-1").add() + + if value is not None: + client.sssd.domain["failover_primary_timeout"] = str(value) + + client.sssd.enable_responder("ifp") + client.sssd.domain["ldap_uri"] = "ldap://ldap.invalid" + client.sssd.domain["ldap_backup_uri"] = f"ldap://{ldap.host.hostname}" + client.sssd.start() + + # Lookup user to make sure SSSD did correctly failover to backup server + result = client.tools.id("user-1") + assert result is not None + + # Check that SSSD is indeed connected to backup server + assert client.sssd.default_domain is not None + status = client.sssctl.domain_status(client.sssd.default_domain, active=True) + assert ldap.host.hostname in status.stdout + + # Check that primary server reactivation timeout was correctly created + log = client.fs.read(client.sssd.logs.domain()) + assert f"Primary server reactivation timeout set to {expected} seconds" in log From a646f5d7a73d261f74b7b1c28d1f75ee6931e7e3 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 21 Mar 2024 15:53:44 +0100 Subject: [PATCH 8/8] INTG-TESTS: backport `sync_files_provider()` from b9c1d7d667d49080c27641fb4a800bd4c2612d43 Reviewed-by: Justin Stephenson (cherry picked from commit ea2d0aab36e033ef76e533af287e35059a754ec2) --- src/tests/intg/test_files_provider.py | 13 +++++++++++++ src/tests/intg/test_pam_responder.py | 1 + 2 files changed, 14 insertions(+) diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index fa503dddd0f..c318d733cda 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -456,6 +456,19 @@ def sssd_id_sync(name): return res, groups +def sync_files_provider(name=None): + """ + Tests with files provider can fail because files provider did not yet + finish updating its cache. Polling for presents of the canary user makes + sure that we wait until the cache is updated. + """ + if name is None: + name = CANARY["name"] + + ret = poll_canary(call_sssd_getpwnam, name) + assert ret + + # Helper functions def user_generator(seqnum): return dict(name='user%d' % seqnum, diff --git a/src/tests/intg/test_pam_responder.py b/src/tests/intg/test_pam_responder.py index 0fbf8065e49..a4b36c0f0c3 100644 --- a/src/tests/intg/test_pam_responder.py +++ b/src/tests/intg/test_pam_responder.py @@ -34,6 +34,7 @@ import pytest +from .test_files_provider import sync_files_provider from intg.util import unindent LDAP_BASE_DN = "dc=example,dc=com"