From e7e942ceb1f8402d00f5f14a9e065d3fc434b711 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Aug 2018 13:55:51 +0200 Subject: [PATCH 01/29] SELINUX: Always add SELinux user to the semanage database if it doesn't exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we tried to optimize too much and only set the SELinux user to Linux user mapping in case the SELinux user was different from the system default. But this doesn't work for the case where the Linux user has a non-standard home directory, because then SELinux would not have any idea that this user's home directory should be labeled as a home directory. This patch relaxes the optimization in the sense that on the first login, the SELinux context is saved regardless of whether it is the same as the default or different. Resolves: https://pagure.io/SSSD/sssd/issue/3819 Reviewed-by: Michal Židek (cherry picked from commit 945865ae16120ffade267227ca48cefd58822fd2) --- src/providers/ipa/selinux_child.c | 10 ++++++++-- src/util/sss_semanage.c | 30 ++++++++++++++++++++++++++++++ src/util/util.h | 1 + src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 5 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/providers/ipa/selinux_child.c b/src/providers/ipa/selinux_child.c index d061417a5a..925591ec90 100644 --- a/src/providers/ipa/selinux_child.c +++ b/src/providers/ipa/selinux_child.c @@ -176,13 +176,16 @@ static bool seuser_needs_update(const char *username, ret = sss_get_seuser(username, &db_seuser, &db_mls_range); DEBUG(SSSDBG_TRACE_INTERNAL, - "getseuserbyname: ret: %d seuser: %s mls: %s\n", + "sss_get_seuser: ret: %d seuser: %s mls: %s\n", ret, db_seuser ? db_seuser : "unknown", db_mls_range ? db_mls_range : "unknown"); if (ret == EOK && db_seuser && db_mls_range && strcmp(db_seuser, seuser) == 0 && strcmp(db_mls_range, mls_range) == 0) { - needs_update = false; + ret = sss_seuser_exists(username); + if (ret == EOK) { + needs_update = false; + } } /* OR */ if (ret == ERR_SELINUX_NOT_MANAGED) { @@ -191,6 +194,9 @@ static bool seuser_needs_update(const char *username, free(db_seuser); free(db_mls_range); + DEBUG(SSSDBG_TRACE_FUNC, + "The SELinux user does %sneed an update\n", + needs_update ? "" : "not "); return needs_update; } diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c index bcce57b603..aea03852ac 100644 --- a/src/util/sss_semanage.c +++ b/src/util/sss_semanage.c @@ -248,6 +248,36 @@ static int sss_semanage_user_mod(semanage_handle_t *handle, return ret; } +int sss_seuser_exists(const char *linuxuser) +{ + int ret; + int exists; + semanage_seuser_key_t *sm_key = NULL; + semanage_handle_t *sm_handle = NULL; + + ret = sss_semanage_init(&sm_handle); + if (ret != EOK) { + return ret; + } + + ret = semanage_seuser_key_create(sm_handle, linuxuser, &sm_key); + if (ret < 0) { + sss_semanage_close(sm_handle); + return EIO; + } + + ret = semanage_seuser_exists(sm_handle, sm_key, &exists); + semanage_seuser_key_free(sm_key); + sss_semanage_close(sm_handle); + if (ret < 0) { + return EIO; + } + + DEBUG(SSSDBG_TRACE_FUNC, "seuser exists: %s\n", exists ? "yes" : "no"); + + return exists ? EOK : ERR_SELINUX_USER_NOT_FOUND; +} + int sss_get_seuser(const char *linuxuser, char **selinuxuser, char **level) diff --git a/src/util/util.h b/src/util/util.h index bc89ecbc25..c78615b9c7 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -660,6 +660,7 @@ int sss_del_seuser(const char *login_name); int sss_get_seuser(const char *linuxuser, char **selinuxuser, char **level); +int sss_seuser_exists(const char *linuxuser); /* convert time from generalized form to unix time */ errno_t sss_utc_to_time_t(const char *str, const char *format, time_t *unix_time); diff --git a/src/util/util_errors.c b/src/util/util_errors.c index e2bb2a0147..e5c5bd1401 100644 --- a/src/util/util_errors.c +++ b/src/util/util_errors.c @@ -75,6 +75,7 @@ struct err_string error_to_str[] = { { "LDAP search returned a referral" }, /* ERR_REFERRAL */ { "Error setting SELinux user context" }, /* ERR_SELINUX_CONTEXT */ { "SELinux is not managed by libsemanage" }, /* ERR_SELINUX_NOT_MANAGED */ + { "SELinux user does not exist" }, /* ERR_SELINUX_USER_NOT_FOUND */ { "Username format not allowed by re_expression" }, /* ERR_REGEX_NOMATCH */ { "Time specification not supported" }, /* ERR_TIMESPEC_NOT_SUPPORTED */ { "Invalid SSSD configuration detected" }, /* ERR_INVALID_CONFIG */ diff --git a/src/util/util_errors.h b/src/util/util_errors.h index 49501727d4..a4760a1db7 100644 --- a/src/util/util_errors.h +++ b/src/util/util_errors.h @@ -97,6 +97,7 @@ enum sssd_errors { ERR_REFERRAL, ERR_SELINUX_CONTEXT, ERR_SELINUX_NOT_MANAGED, + ERR_SELINUX_USER_NOT_FOUND, ERR_REGEX_NOMATCH, ERR_TIMESPEC_NOT_SUPPORTED, ERR_INVALID_CONFIG, From 3dc885344e6c90a0db7a759b52f56c1b4a4f6d6d Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 10 Sep 2018 15:35:45 +0200 Subject: [PATCH 02/29] intg: flush the SSSD caches to sync with files To make sure that SSSD has synced with the latest data added to the passwd file sss_cache is called in two places where the current sync scheme was not reliable. This was mainly observed when running the integration tests on Debian. Reviewed-by: Jakub Hrozek (cherry picked from commit 1e2398870e8aa512ead3012d46cbe6252429467a) --- src/tests/intg/test_files_provider.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index 9f30d2bb48..ead1cc4c34 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -644,6 +644,10 @@ def test_enum_users(setup_pw_with_canary, files_domain_only): user = user_generator(i) setup_pw_with_canary.useradd(**user) + # syncing with the help of the canary is not reliable after adding + # multiple users because the canary might still be in some caches so that + # the data is not refreshed properly. + subprocess.call(["sss_cache", "-E"]) sssd_getpwnam_sync(CANARY["name"]) user_list = call_sssd_enumeration() # +1 because the canary is added @@ -1043,6 +1047,10 @@ def test_getgrnam_add_remove_ghosts(setup_pw_with_canary, # Add this user and verify it's been added as a member pwd_ops.useradd(**USER2) + # The negative cache might still have user2 from the previous request, + # flushing the caches might help to prevent a failed lookup after adding + # the user. + subprocess.call(["sss_cache", "-E"]) res, groups = sssd_id_sync('user2') assert res == sssd_id.NssReturnCode.SUCCESS assert len(groups) == 2 From 2d9286102f23ea9d13213f1176ba669b9315a75f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Thu, 6 Sep 2018 13:38:56 +0200 Subject: [PATCH 03/29] sudo: respect case sensitivity in sudo responder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the domain is not case sensitive and the case of the original user or group name differs from the name in the rule we failed to find the rule. Now we filter the rule only with lower cased values in such domain. Steps to reproduce: 1. Add user/group with upper case, e.g. USER-1 2. Add sudo rule with lower cased name, e.g. sudoUser: user-1 3. Login to system with lower case, e.g. user-1 4. Run sudo -l Without the patch, rule is not found. Resolves: https://pagure.io/SSSD/sssd/issue/3820 Reviewed-by: Michal Židek (cherry picked from commit d7f0b58e2896ed2ef9ed5a390815c1e4df6caaee) --- src/db/sysdb_sudo.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/db/sysdb_sudo.c b/src/db/sysdb_sudo.c index 3ad462d8fd..19ed97b866 100644 --- a/src/db/sysdb_sudo.c +++ b/src/db/sysdb_sudo.c @@ -418,7 +418,17 @@ sysdb_get_sudo_user_info(TALLOC_CTX *mem_ctx, ret = EINVAL; goto done; } - DEBUG(SSSDBG_TRACE_FUNC, "original name: %s\n", orig_name); + + DEBUG(SSSDBG_TRACE_FUNC, "Original name: %s\n", orig_name); + + orig_name = sss_get_cased_name(tmp_ctx, orig_name, domain->case_sensitive); + if (orig_name == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory!\n"); + ret = ENOMEM; + goto done; + } + + DEBUG(SSSDBG_TRACE_FUNC, "Cased name: %s\n", orig_name); if (_uid != NULL) { uid = ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_UIDNUM, 0); @@ -450,8 +460,9 @@ sysdb_get_sudo_user_info(TALLOC_CTX *mem_ctx, continue; } - sysdb_groupnames[num_groups] = talloc_strdup(sysdb_groupnames, - groupname); + sysdb_groupnames[num_groups] = \ + sss_get_cased_name(sysdb_groupnames, groupname, + domain->case_sensitive); if (sysdb_groupnames[num_groups] == NULL) { DEBUG(SSSDBG_MINOR_FAILURE, "Cannot strdup %s\n", groupname); continue; From bca1935761a8ac21a45abcda9f82feec259dc65c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 2 Oct 2018 10:14:10 +0200 Subject: [PATCH 04/29] pep8: Ignore W504 and W605 to silence warnings on Debian MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code: pkcs11_txt.write("library=libsoftokn3.so\nname=soft\n" + "parameters=configdir='sql:" + config.ABS_BUILDDIR + "/../test_CA/p11_nssdb' " + "dbSlotDescription='SSSD Test Slot' " + "dbTokenDescription='SSSD Test Token' " + "secmod='secmod.db' flags=readOnly)\n\n") pkcs11_txt.close() Was producing warnings such as: ./src/tests/intg/test_pam_responder.py:143:22: W504 line break after binary operator Even though it looks OK visually and conforms to pep8's written form. Additionaly, this regular expression compilation: Template = re.compile( ' *\r?\n?', re.MULTILINE | re.DOTALL ) Was producing a warning such as: ./src/sbus/codegen/sbus_Template.py:156:29: W605 invalid escape sequence '\S' Since the \S literal is part of a regular expression, let's suppress this warning as well. Reviewed-by: Michal Židek (cherry picked from commit ec7665973936897ab6be58308e655f08d91bec5c) --- contrib/ci/run | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/ci/run b/contrib/ci/run index b481419f9d..bf29f87531 100755 --- a/contrib/ci/run +++ b/contrib/ci/run @@ -41,7 +41,7 @@ declare -r COVERAGE_MIN_FUNCS=0 # Those values are a sum up of the default warnings in all our # supported distros in our CI. -# debian_testing: E121,E123,E126,E226,E24,E704,W503 +# debian_testing: E121,E123,E126,E226,E24,E704,W503,W504,W605 # fedora22: # fedora23: # fedora24: E121,E123,E126,E226,E24,E704 @@ -51,7 +51,7 @@ declare -r COVERAGE_MIN_FUNCS=0 # fedora_rawhide: E121,E123,E126,E226,E24,E704 # rhel6: # rhel7: -declare PEP8_IGNORE="--ignore=E121,E123,E126,E226,E24,E704,W503" +declare PEP8_IGNORE="--ignore=E121,E123,E126,E226,E24,E704,W503,W504,W605" declare BASE_PFX="" declare DEPS=true declare BASE_DIR=`pwd` From 720a423a0119f23606c6029c3f48be98841ca910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 30 Oct 2018 13:21:28 +0100 Subject: [PATCH 05/29] nss: use enumeration context as talloc parent for cache req result Otherwise we end up with memory leak since the result is never freed. We need to convert nctx->*ent structures into talloc pointer so we can use enum_ctx as parent. Resolves: https://pagure.io/SSSD/sssd/issue/3870 Reviewed-by: Jakub Hrozek (cherry picked from commit 406b731ddfbeb62623640cc37a7adc76af0a4b22) --- src/responder/nss/nss_cmd.c | 12 ++++++------ src/responder/nss/nss_enum.c | 2 +- src/responder/nss/nss_private.h | 6 +++--- src/responder/nss/nsssrv.c | 21 +++++++++++++++++++++ 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/responder/nss/nss_cmd.c b/src/responder/nss/nss_cmd.c index 9ee6ca805e..25e663ed5e 100644 --- a/src/responder/nss/nss_cmd.c +++ b/src/responder/nss/nss_cmd.c @@ -942,7 +942,7 @@ static errno_t nss_cmd_setpwent(struct cli_ctx *cli_ctx) nss_ctx = talloc_get_type(cli_ctx->rctx->pvt_ctx, struct nss_ctx); - return nss_setent(cli_ctx, CACHE_REQ_ENUM_USERS, &nss_ctx->pwent); + return nss_setent(cli_ctx, CACHE_REQ_ENUM_USERS, nss_ctx->pwent); } static errno_t nss_cmd_getpwent(struct cli_ctx *cli_ctx) @@ -955,7 +955,7 @@ static errno_t nss_cmd_getpwent(struct cli_ctx *cli_ctx) return nss_getent(cli_ctx, CACHE_REQ_ENUM_USERS, &state_ctx->pwent, nss_protocol_fill_pwent, - &nss_ctx->pwent); + nss_ctx->pwent); } static errno_t nss_cmd_endpwent(struct cli_ctx *cli_ctx) @@ -998,7 +998,7 @@ static errno_t nss_cmd_setgrent(struct cli_ctx *cli_ctx) nss_ctx = talloc_get_type(cli_ctx->rctx->pvt_ctx, struct nss_ctx); - return nss_setent(cli_ctx, CACHE_REQ_ENUM_GROUPS, &nss_ctx->grent); + return nss_setent(cli_ctx, CACHE_REQ_ENUM_GROUPS, nss_ctx->grent); } static errno_t nss_cmd_getgrent(struct cli_ctx *cli_ctx) @@ -1011,7 +1011,7 @@ static errno_t nss_cmd_getgrent(struct cli_ctx *cli_ctx) return nss_getent(cli_ctx, CACHE_REQ_ENUM_GROUPS, &state_ctx->grent, nss_protocol_fill_grent, - &nss_ctx->grent); + nss_ctx->grent); } static errno_t nss_cmd_endgrent(struct cli_ctx *cli_ctx) @@ -1093,7 +1093,7 @@ static errno_t nss_cmd_setservent(struct cli_ctx *cli_ctx) nss_ctx = talloc_get_type(cli_ctx->rctx->pvt_ctx, struct nss_ctx); - return nss_setent(cli_ctx, CACHE_REQ_ENUM_SVC, &nss_ctx->svcent); + return nss_setent(cli_ctx, CACHE_REQ_ENUM_SVC, nss_ctx->svcent); } static errno_t nss_cmd_getservent(struct cli_ctx *cli_ctx) @@ -1106,7 +1106,7 @@ static errno_t nss_cmd_getservent(struct cli_ctx *cli_ctx) return nss_getent(cli_ctx, CACHE_REQ_ENUM_SVC, &state_ctx->svcent, nss_protocol_fill_svcent, - &nss_ctx->svcent); + nss_ctx->svcent); } static errno_t nss_cmd_endservent(struct cli_ctx *cli_ctx) diff --git a/src/responder/nss/nss_enum.c b/src/responder/nss/nss_enum.c index a45b65233c..9588943c97 100644 --- a/src/responder/nss/nss_enum.c +++ b/src/responder/nss/nss_enum.c @@ -138,7 +138,7 @@ static void nss_setent_internal_done(struct tevent_req *subreq) switch (ret) { case EOK: talloc_zfree(state->enum_ctx->result); - state->enum_ctx->result = talloc_steal(state->nss_ctx, result); + state->enum_ctx->result = talloc_steal(state->enum_ctx, result); if (state->type == CACHE_REQ_NETGROUP_BY_NAME) { /* We need to expand the netgroup into triples and members. */ diff --git a/src/responder/nss/nss_private.h b/src/responder/nss/nss_private.h index aa8d8e9cde..cd0d355170 100644 --- a/src/responder/nss/nss_private.h +++ b/src/responder/nss/nss_private.h @@ -78,9 +78,9 @@ struct nss_ctx { const char **extra_attributes; /* Enumeration. */ - struct nss_enum_ctx pwent; - struct nss_enum_ctx grent; - struct nss_enum_ctx svcent; + struct nss_enum_ctx *pwent; + struct nss_enum_ctx *grent; + struct nss_enum_ctx *svcent; hash_table_t *netgrent; /* Memory cache. */ diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index 004e6c1a12..d6c5a08a9a 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -378,6 +378,27 @@ int nss_process_init(TALLOC_CTX *mem_ctx, goto fail; } + nctx->pwent = talloc_zero(nctx, struct nss_enum_ctx); + if (nctx->pwent == NULL) { + DEBUG(SSSDBG_FATAL_FAILURE, "Unable to initialize pwent context!\n"); + ret = ENOMEM; + goto fail; + } + + nctx->grent = talloc_zero(nctx, struct nss_enum_ctx); + if (nctx->grent == NULL) { + DEBUG(SSSDBG_FATAL_FAILURE, "Unable to initialize grent context!\n"); + ret = ENOMEM; + goto fail; + } + + nctx->svcent = talloc_zero(nctx, struct nss_enum_ctx); + if (nctx->svcent == NULL) { + DEBUG(SSSDBG_FATAL_FAILURE, "Unable to initialize svcent context!\n"); + ret = ENOMEM; + goto fail; + } + nctx->netgrent = sss_ptr_hash_create(nctx, NULL, NULL); if (nctx->netgrent == NULL) { DEBUG(SSSDBG_FATAL_FAILURE, "Unable to initialize netgroups table!\n"); From 876f1cb87d1649d0681bf6475ab589287f15babb Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 22 Nov 2018 12:51:14 +0100 Subject: [PATCH 06/29] LDAP: minor refactoring in auth_send() to conform to our coding style Related: https://pagure.io/SSSD/sssd/issue/3451 A tevent _send() function should only return NULL on ENOMEM, otherwise it should mark the request as failed but return the req pointer. This was not much of an issue, before, but the next patch will add another function call to the auth_send call which would make error handling awkward. Reviewed-by: Sumit Bose (cherry picked from commit 09091b4b60456a989ecc8c3b6f76661a14c108ba) --- src/providers/ldap/ldap_auth.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c index d40bc94148..c409353d91 100644 --- a/src/providers/ldap/ldap_auth.c +++ b/src/providers/ldap/ldap_auth.c @@ -636,6 +636,7 @@ static struct tevent_req *auth_send(TALLOC_CTX *memctx, { struct tevent_req *req; struct auth_state *state; + errno_t ret; req = tevent_req_create(memctx, &state, struct auth_state); if (!req) return NULL; @@ -645,11 +646,11 @@ static struct tevent_req *auth_send(TALLOC_CTX *memctx, if (sss_authtok_get_type(authtok) == SSS_AUTHTOK_TYPE_SC_PIN || sss_authtok_get_type(authtok) == SSS_AUTHTOK_TYPE_SC_KEYPAD) { /* Tell frontend that we do not support Smartcard authentication */ - tevent_req_error(req, ERR_SC_AUTH_NOT_SUPPORTED); + ret = ERR_SC_AUTH_NOT_SUPPORTED; } else { - tevent_req_error(req, ERR_AUTH_FAILED); + ret = ERR_AUTH_FAILED; } - return tevent_req_post(req, ev); + goto fail; } state->ev = ev; @@ -663,13 +664,17 @@ static struct tevent_req *auth_send(TALLOC_CTX *memctx, state->sdap_service = ctx->service; } - if (!auth_connect_send(req)) goto fail; + if (auth_connect_send(req) == NULL) { + ret = ENOMEM; + goto fail; + } return req; fail: - talloc_zfree(req); - return NULL; + tevent_req_error(req, ret); + tevent_req_post(req, ev); + return req; } static struct tevent_req *auth_connect_send(struct tevent_req *req) From 7eb18ab68762d1b1ddbcbdc32dbcbd0df183d4f1 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 22 Nov 2018 12:17:51 +0100 Subject: [PATCH 07/29] LDAP: Only authenticate the auth connection if we need to look up user information Related: https://pagure.io/SSSD/sssd/issue/3451 Commit add72860c7a7a2c418f4d8b6790b5caeaf7dfb7b initially addressed #3451 by using the full sdap_cli_connect() request during LDAP authentication. This was a good idea as it addressed the case where the authentication connection must also look up some user information (typically with id_provider=proxy where you don't know the DN to bind as during authentication), but this approach also broke the use-case of id_provider=ldap and auth_provider=ldap with ldap_sasl_auth=gssapi. This is because (for reason I don't know) AD doesn't like if you use both GSSAPI and startTLS on the same connection. But the code would force TLS during the authentication as a general measure to not transmit passwords in the clear, but then, the connection would also see that ldap_sasl_auth=gssapi is set and also bind with GSSAPI. This patch checks if the user DN is already known and if yes, then doesn't authenticate the connection as the connection will then only be used for the user simple bind. Reviewed-by: Sumit Bose (cherry picked from commit 57fc60c9dc77698cf824813c36eb0f90d767b315) --- src/providers/ldap/ldap_auth.c | 53 +++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c index c409353d91..b4d045a651 100644 --- a/src/providers/ldap/ldap_auth.c +++ b/src/providers/ldap/ldap_auth.c @@ -664,6 +664,18 @@ static struct tevent_req *auth_send(TALLOC_CTX *memctx, state->sdap_service = ctx->service; } + ret = get_user_dn(state, state->ctx->be->domain, + state->ctx->opts, state->username, &state->dn, + &state->pw_expire_type, &state->pw_expire_data); + if (ret == EAGAIN) { + DEBUG(SSSDBG_TRACE_FUNC, + "Need to look up the DN of %s later\n", state->username); + } else if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot get user DN [%d]: %s\n", ret, sss_strerror(ret)); + goto fail; + } + if (auth_connect_send(req) == NULL) { ret = ENOMEM; goto fail; @@ -683,6 +695,8 @@ static struct tevent_req *auth_connect_send(struct tevent_req *req) struct auth_state *state = tevent_req_data(req, struct auth_state); bool use_tls; + bool skip_conn_auth = false; + const char *sasl_mech; /* Check for undocumented debugging feature to disable TLS * for authentication. This should never be used in production @@ -695,10 +709,33 @@ static struct tevent_req *auth_connect_send(struct tevent_req *req) "for debugging purposes only."); } + if (state->dn != NULL) { + /* In case the user's DN is known, the connection will only be used + * to bind as the user to perform the authentication. In that case, + * we don't need to authenticate the connection, because we're not + * looking up any information using the connection. This might be + * needed e.g. in case both ID and AUTH providers are set to LDAP + * and the server is AD, because otherwise the connection would + * both do a startTLS and later bind using GSSAPI which doesn't work + * well with AD. + */ + skip_conn_auth = true; + } + + if (skip_conn_auth == false) { + sasl_mech = dp_opt_get_string(state->ctx->opts->basic, + SDAP_SASL_MECH); + if (sasl_mech && strcasecmp(sasl_mech, "GSSAPI") == 0) { + /* Don't force TLS on if we're told to use GSSAPI */ + use_tls = false; + } + } + subreq = sdap_cli_connect_send(state, state->ev, state->ctx->opts, state->ctx->be, state->sdap_service, false, - use_tls ? CON_TLS_ON : CON_TLS_OFF, false); + use_tls ? CON_TLS_ON : CON_TLS_OFF, + skip_conn_auth); if (subreq == NULL) { tevent_req_error(req, ENOMEM); @@ -739,15 +776,7 @@ static void auth_connect_done(struct tevent_req *subreq) return; } - ret = get_user_dn(state, state->ctx->be->domain, - state->ctx->opts, state->username, &state->dn, - &state->pw_expire_type, &state->pw_expire_data); - if (ret == EOK) { - /* All required user data was pre-cached during an identity lookup. - * We can proceed with the bind */ - auth_do_bind(req); - return; - } else if (ret == EAGAIN) { + if (state->dn == NULL) { /* The cached user entry was missing the bind DN. Need to look * it up based on user name in order to perform the bind */ subreq = get_user_dn_send(req, state->ev, state->ctx->be->domain, @@ -760,7 +789,9 @@ static void auth_connect_done(struct tevent_req *subreq) return; } - tevent_req_error(req, ret); + /* All required user data was pre-cached during an identity lookup. + * We can proceed with the bind */ + auth_do_bind(req); return; } From 1a7c6ab6efce3720d27def426aad49ee99eb339d Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 26 Nov 2018 12:38:40 +0100 Subject: [PATCH 08/29] LDAP: Log the encryption used during LDAP authentication Reviewed-by: Jakub Hrozek (cherry picked from commit 6f113c7ddeaa5c82558e10118b499d22bf7a2b14) --- src/providers/ldap/ldap_auth.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c index b4d045a651..4666dbfbb1 100644 --- a/src/providers/ldap/ldap_auth.c +++ b/src/providers/ldap/ldap_auth.c @@ -747,6 +747,31 @@ static struct tevent_req *auth_connect_send(struct tevent_req *req) return subreq; } +static void check_encryption(LDAP *ldap) +{ + ber_len_t sasl_ssf = 0; + int tls_inplace = 0; + int ret; + + ret = ldap_get_option(ldap, LDAP_OPT_X_SASL_SSF, &sasl_ssf); + if (ret != LDAP_SUCCESS) { + DEBUG(SSSDBG_TRACE_LIBS, "ldap_get_option failed to get sasl ssf, " + "assuming SASL is not used.\n"); + } + + tls_inplace = ldap_tls_inplace(ldap); + + DEBUG(SSSDBG_TRACE_ALL, + "Encryption used: SASL SSF [%lu] tls_inplace [%s].\n", sasl_ssf, + tls_inplace == 1 ? "TLS inplace" : "TLS NOT inplace"); + + if (sasl_ssf <= 1 && tls_inplace != 1) { + DEBUG(SSSDBG_CRIT_FAILURE, + "No encryption detected on LDAP connection.\n"); + sss_log(SSS_LOG_CRIT, "No encryption detected on LDAP connection.\n"); + } +} + static void auth_connect_done(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data(subreq, @@ -776,6 +801,8 @@ static void auth_connect_done(struct tevent_req *subreq) return; } + check_encryption(state->sh->ldap); + if (state->dn == NULL) { /* The cached user entry was missing the bind DN. Need to look * it up based on user name in order to perform the bind */ From 4d3841ca379afc01184453ba45ab3e75ffec60da Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 7 Nov 2018 23:06:10 +0000 Subject: [PATCH 09/29] UTIL: Fix compilation with curl 7.62.0 The macro CURLE_SSL_CACERT is deprecated in upstream curl since commit 3f3b26d6feb0667714902e836af608094235fca2. commit 3f3b26d6feb0667714902e836af608094235fca2 Author: Han Han Date: Wed Aug 22 11:13:32 2018 -0700 ssl: deprecate CURLE_SSL_CACERT in favour of a unified error code Long live CURLE_PEER_FAILED_VERIFICATION sh$ git tag --contains 3f3b26d6feb0667714902e836af608094235fca2 curl-7_62_0 It was not removed. It is just an alias to CURLE_PEER_FAILED_VERIFICATION which causes compile time failures in switch/case. ./src/util/tev_curl.c: In function 'curl_code2errno': ./src/util/tev_curl.c:113:5: error: duplicate case value case CURLE_PEER_FAILED_VERIFICATION: ^~~~ ./src/util/tev_curl.c: 100:5: note: previously used here case CURLE_SSL_CACERT: ^~~~ Merges: https://pagure.io/SSSD/sssd/pull-request/3878 Resolves: https://pagure.io/SSSD/sssd/issue/3875 Reviewed-by: Sumit Bose (cherry picked from commit 1ee12b05570fcfb8e4190c9ec704c5563138344d) --- src/util/tev_curl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/tev_curl.c b/src/util/tev_curl.c index 6a7a580d54..d70a42972f 100644 --- a/src/util/tev_curl.c +++ b/src/util/tev_curl.c @@ -97,7 +97,9 @@ static errno_t curl_code2errno(CURLcode crv) return ETIMEDOUT; case CURLE_SSL_ISSUER_ERROR: case CURLE_SSL_CACERT_BADFILE: +#if LIBCURL_VERSION_NUM < 0x073e00 case CURLE_SSL_CACERT: +#endif case CURLE_SSL_CERTPROBLEM: return ERR_INVALID_CERT; From e80e869a92c4689888f6f5d967acfe9f085ce27a Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Sat, 10 Nov 2018 20:34:28 +0000 Subject: [PATCH 10/29] test_pac_responder: Skip test if pac responder is not installed Merges: https://pagure.io/SSSD/sssd/pull-request/3881 Reviewed-by: Sumit Bose (cherry picked from commit 4f824eca24e185e6463167b7bcc20d1398c60414) --- src/tests/intg/test_pac_responder.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tests/intg/test_pac_responder.py b/src/tests/intg/test_pac_responder.py index adc31965b5..a93c6ca6b4 100644 --- a/src/tests/intg/test_pac_responder.py +++ b/src/tests/intg/test_pac_responder.py @@ -27,6 +27,12 @@ from util import unindent +def have_sssd_responder(responder_name): + responder_path = os.path.join(config.LIBEXEC_PATH, "sssd", responder_name) + + return os.access(responder_path, os.X_OK) + + def stop_sssd(): with open(config.PIDFILE_PATH, "r") as pid_file: pid = int(pid_file.read()) @@ -99,6 +105,8 @@ def timeout_handler(signum, frame): raise Exception("Timeout") +@pytest.mark.skipif(not have_sssd_responder("sssd_pac"), + reason="No PAC responder, skipping") def test_multithreaded_pac_client(local_domain_only, sssd_pac_test_client): """ Test for ticket From de7f877304cbc82cb13ff8a932560a439b690b02 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Sat, 10 Nov 2018 20:34:37 +0000 Subject: [PATCH 11/29] INTG: Show extra test summary info with pytest It will show reasons why tests were skipped. e.g. ====================== test session starts ======================== platform linux -- Python 3.7.1, pytest-3.9.3, py-1.5.4, pluggy-0.7.1 -- /usr/bin/python3 cachedir: .pytest_cache rootdir: /dev/shm/sssd/src/tests/intg, inifile: collected 286 items / 285 deselected test_pac_responder.py::test_multithreaded_pac_client SKIPPED [100%] ==================== short test summary info ====================== SKIP [1] test_pac_responder.py:108: No PAC responder, skipping Merges: https://pagure.io/SSSD/sssd/pull-request/3881 Reviewed-by: Sumit Bose (cherry picked from commit fdbe67a88d1894471031b0aabc74bd1b29eef294) --- src/tests/intg/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index 65da9ca170..91dc86a4f3 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -140,5 +140,5 @@ intgcheck-installed: config.py passwd group DBUS_SESSION_BUS_ADDRESS="unix:path=$$DBUS_SOCK_DIR/fake_socket" \ DBUS_SYSTEM_BUS_ADDRESS="unix:path=$$DBUS_SOCK_DIR/system_bus_socket" \ DBUS_SYSTEM_BUS_DEFAULT_ADDRESS="$$DBUS_SYSTEM_BUS_ADDRESS" \ - fakeroot $(PYTHON2) $(PYTEST) -v --tb=native $(INTGCHECK_PYTEST_ARGS) . + fakeroot $(PYTHON2) $(PYTEST) -v -r a --tb=native $(INTGCHECK_PYTEST_ARGS) . rm -f $(DESTDIR)$(logpath)/* From 517fe071081619050df6d0d2983057f9007ce66b Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Wed, 14 Nov 2018 22:33:26 +0000 Subject: [PATCH 12/29] CI: Modify suppression file for c-ares-1.15.0 Valgrind does not generate full stack trace for errors. It is just limited amount of frames. Therefore we cannot see main function with the new c-ares. The suppression file generated with c-ares-1.14.0 { Memcheck:Leak match-leak-kinds: possible fun:malloc fun:strdup fun:ares_init_options fun:recreate_ares_channel fun:resolv_init fun:be_res_init fun:be_res_init fun:be_init_failover fun:test_ipa_server_create_trusts_setup obj:/usr/lib64/libcmocka.so.0.5.1 fun:_cmocka_run_group_tests fun:main } The suppression file generated with c-ares-1.15.0 { Memcheck:Leak match-leak-kinds: possible fun:malloc fun:strdup obj:/usr/lib64/libcares.so.2.3.0 obj:/usr/lib64/libcares.so.2.3.0 fun:ares_init_options fun:recreate_ares_channel fun:resolv_init fun:be_res_init fun:be_res_init fun:be_init_failover fun:test_ipa_server_create_trusts_setup obj:/usr/lib64/libcmocka.so.0.5.1 fun:_cmocka_run_group_tests } Merges: https://pagure.io/SSSD/sssd/pull-request/3884 Reviewed-by: Sumit Bose (cherry picked from commit f02714d6f0f5d9cdd504d5f0527849a6d6b88fab) --- contrib/ci/sssd.supp | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/ci/sssd.supp b/contrib/ci/sssd.supp index 38254ca8d6..d27e006a0f 100644 --- a/contrib/ci/sssd.supp +++ b/contrib/ci/sssd.supp @@ -169,7 +169,6 @@ fun:test_ipa_server_create_trusts_setup ... fun:_cmocka_run_group_tests - fun:main } # Leaks in bash if p11_child returns and error because due to libtool the From 9e858795648909d17ba8bd72ba64fd6e8817a149 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 22 Nov 2018 11:33:20 +0100 Subject: [PATCH 13/29] BUILD: Accept krb5 1.17 for building the PAC plugin Reviewed-by: Jakub Hrozek (cherry picked from commit 53e6fdfd881f051898e85448832eafdd2ea09454) --- src/external/pac_responder.m4 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/external/pac_responder.m4 b/src/external/pac_responder.m4 index e0685f0ce3..dc986a1b8a 100644 --- a/src/external/pac_responder.m4 +++ b/src/external/pac_responder.m4 @@ -18,7 +18,8 @@ then Kerberos\ 5\ release\ 1.13* | \ Kerberos\ 5\ release\ 1.14* | \ Kerberos\ 5\ release\ 1.15* | \ - Kerberos\ 5\ release\ 1.16*) + Kerberos\ 5\ release\ 1.16* | \ + Kerberos\ 5\ release\ 1.17*) krb5_version_ok=yes AC_MSG_RESULT([yes]) ;; From d1c930809fc4ea9b7abdecd4b17cd3da735beef7 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 22 Nov 2018 11:36:57 +0100 Subject: [PATCH 14/29] tests: fix mocking krb5_creds in test_copy_ccache To just test some ccache related functionality without talking to an actual KDC to get the tickets some needed libkrb5 structs were mocked based on tests from the MIT Kerberos source code. One struct member (is_skey) was so far not regarded by libkrb5 for out test case. But a recent fix for http://krbdev.mit.edu/rt/Ticket/Display.html?id=8718 changed this and we have to change the mocking. Reviewed-by: Jakub Hrozek (cherry picked from commit 08bba3a6e3e4e21f2e20b71cca463d50420aa9ee) --- src/tests/cmocka/test_copy_ccache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/cmocka/test_copy_ccache.c b/src/tests/cmocka/test_copy_ccache.c index 84225b6bff..7c76c00e8f 100644 --- a/src/tests/cmocka/test_copy_ccache.c +++ b/src/tests/cmocka/test_copy_ccache.c @@ -88,7 +88,7 @@ static int setup_ccache(void **state) test_creds.times.starttime = 2222; test_creds.times.endtime = 3333; test_creds.times.renew_till = 4444; - test_creds.is_skey = 1; + test_creds.is_skey = 0; test_creds.ticket_flags = 5555; test_creds.addresses = addrs; From 19e6c50df215e40ddd46cf9dc7583706c0a3e02b Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 22 Nov 2018 12:12:00 +0100 Subject: [PATCH 15/29] tests: increase p11_child_timeout With recent version of valgrind some tests failed during a CI run with a timeout. To avoid this the related p11_child_timeout is increased for the affected tests. Reviewed-by: Jakub Hrozek (cherry picked from commit 1617f3e3dc90788ef50bbc7948c3870d08cc9c2d) --- src/tests/cmocka/test_cert_utils.c | 2 +- src/tests/cmocka/test_ssh_srv.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/cmocka/test_cert_utils.c b/src/tests/cmocka/test_cert_utils.c index 26fffb870c..ec5858b6e2 100644 --- a/src/tests/cmocka/test_cert_utils.c +++ b/src/tests/cmocka/test_cert_utils.c @@ -50,7 +50,7 @@ /* When run under valgrind with --trace-children=yes we have to increase the * timeout not because p11_child needs much more time under valgrind but * because of the way valgrind handles the children. */ -#define P11_CHILD_TIMEOUT 40 +#define P11_CHILD_TIMEOUT 80 /* TODO: create a certificate for this test */ const uint8_t test_cert_der[] = { diff --git a/src/tests/cmocka/test_ssh_srv.c b/src/tests/cmocka/test_ssh_srv.c index 93217a1979..d611bdcfff 100644 --- a/src/tests/cmocka/test_ssh_srv.c +++ b/src/tests/cmocka/test_ssh_srv.c @@ -223,7 +223,7 @@ static int ssh_test_setup(void **state) * the timeout not because p11_child needs much more time under valgrind * but because of the way valgrind handles the children. */ struct sss_test_conf_param ssh_params[] = { - { "p11_child_timeout", "40" }, + { "p11_child_timeout", "80" }, { NULL, NULL }, /* Sentinel */ }; From d33ec64423087261fcc14acb5922793fadb83342 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 4 Dec 2018 13:08:11 +0100 Subject: [PATCH 16/29] Revert "IPA: use forest name when looking up the Global Catalog" This reverts commit 149174acae677d1e72a0da431bf0850d55f2ccb4. Reviewed-by: Jakub Hrozek (cherry picked from commit 9096fc01cca8fcaeb19c36a27f3a9fa09d60772a) --- src/providers/ipa/ipa_subdomains_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/providers/ipa/ipa_subdomains_server.c b/src/providers/ipa/ipa_subdomains_server.c index e5ea4bd023..43a3053cb2 100644 --- a/src/providers/ipa/ipa_subdomains_server.c +++ b/src/providers/ipa/ipa_subdomains_server.c @@ -266,7 +266,7 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx, DEBUG(SSSDBG_TRACE_ALL, "No extra attrs set.\n"); } - gc_service_name = talloc_asprintf(ad_options, "sd_gc_%s", subdom->forest); + gc_service_name = talloc_asprintf(ad_options, "sd_gc_%s", subdom->name); if (gc_service_name == NULL) { talloc_free(ad_options); return ENOMEM; From 74568bdde833f752187cb1a38b39715556c91279 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 4 Dec 2018 13:06:23 +0100 Subject: [PATCH 17/29] ipa: use only the global catalog service of the forest root While creating the domains and sub-domains each domain gets a global catalog services assigned but only one should be used because the global catalog is by definition responsible for the whole forest so it does not make sense to use a global catalog service for each domain and in the worst case connect to the same GC multiple times. In the AD provider this is simple because the GC service of the configured domain AD_GC_SERVICE_NAME ("AD_GC") can be used. In the IPA case all domains from the trusted forest are on the level of sub-domains so we have to pick one. Since the forest root is linked from all domain of the same forest it will be the most straight forward choice. Related to https://pagure.io/SSSD/sssd/issue/3902 Reviewed-by: Jakub Hrozek (cherry picked from commit 62d671b874a66101c0f4bff39fc6d7f49cb8fca6) --- src/providers/ipa/ipa_subdomains_id.c | 50 +++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c index a16eed284e..48cf74460f 100644 --- a/src/providers/ipa/ipa_subdomains_id.c +++ b/src/providers/ipa/ipa_subdomains_id.c @@ -713,6 +713,52 @@ int ipa_get_subdom_acct_recv(struct tevent_req *req, int *dp_error_out) return EOK; } +static struct ad_id_ctx *ipa_get_ad_id_ctx(struct ipa_id_ctx *ipa_ctx, + struct sss_domain_info *dom); + +static struct sdap_id_conn_ctx ** +ipa_ad_gc_conn_list(TALLOC_CTX *mem_ctx, struct ipa_id_ctx *ipa_ctx, + struct ad_id_ctx *ad_ctx, struct sss_domain_info *dom) +{ + struct ad_id_ctx *forest_root_ad_id_ctx; + struct sdap_id_conn_ctx **clist; + int cindex = 0; + + /* While creating the domains and sub-domains each domain gets a global + * catalog services assigned but only one should be used because the + * global catalog is by definition responsible for the whole forest so it + * does not make sense to use a global catalog service for each domain and + * in the worst case connect to the same GC multiple times. + * + * In the AD provider this is simple because the GC service of the + * configured domain AD_GC_SERVICE_NAME ("AD_GC") can be used. In the IPA + * case all domains from the trusted forest are on the level of + * sub-domains so we have to pick one. Since the forest root is linked + * from all domain of the same forest it will be the most straight forward + * choice. */ + forest_root_ad_id_ctx = ipa_get_ad_id_ctx(ipa_ctx, dom->forest_root); + if (forest_root_ad_id_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Missing ad_id_ctx for forest root.\n"); + return NULL; + } + + clist = talloc_zero_array(mem_ctx, struct sdap_id_conn_ctx *, 3); + if (clist == NULL) return NULL; + + /* Always try GC first */ + if (dp_opt_get_bool(forest_root_ad_id_ctx->ad_options->basic, + AD_ENABLE_GC)) { + clist[cindex] = forest_root_ad_id_ctx->gc_ctx; + clist[cindex]->ignore_mark_offline = true; + clist[cindex]->no_mpg_user_fallback = true; + cindex++; + } + + clist[cindex] = ad_get_dom_ldap_conn(ad_ctx, dom); + + return clist; +} + /* IPA lookup for server mode. Directly to AD. */ struct ipa_get_ad_acct_state { int dp_error; @@ -731,8 +777,6 @@ static errno_t ipa_get_ad_apply_override_step(struct tevent_req *req); static errno_t ipa_get_ad_ipa_membership_step(struct tevent_req *req); static void ipa_id_get_groups_overrides_done(struct tevent_req *subreq); static void ipa_get_ad_acct_done(struct tevent_req *subreq); -static struct ad_id_ctx *ipa_get_ad_id_ctx(struct ipa_id_ctx *ipa_ctx, - struct sss_domain_info *dom); static struct tevent_req * ipa_get_ad_acct_send(TALLOC_CTX *mem_ctx, @@ -785,7 +829,7 @@ ipa_get_ad_acct_send(TALLOC_CTX *mem_ctx, case BE_REQ_INITGROUPS: case BE_REQ_BY_SECID: case BE_REQ_GROUP: - clist = ad_gc_conn_list(req, ad_id_ctx, state->obj_dom); + clist = ipa_ad_gc_conn_list(req, ipa_ctx, ad_id_ctx, state->obj_dom); break; default: clist = ad_ldap_conn_list(req, ad_id_ctx, state->obj_dom); From 28792523a01a7d21bcc8931794164f253e691a68 Mon Sep 17 00:00:00 2001 From: Tomas Halman Date: Mon, 3 Dec 2018 14:11:31 +0100 Subject: [PATCH 18/29] nss: sssd returns '/' for emtpy home directories For empty home directory in passwd file sssd returns "/". Sssd should respect system behaviour and return the same as nsswitch "files" module - return empty string. Resolves: https://pagure.io/SSSD/sssd/issue/3901 Reviewed-by: Simo Sorce Reviewed-by: Jakub Hrozek (cherry picked from commit 90f32399b4100ce39cf665649fde82d215e5eb49) --- src/confdb/confdb.c | 9 +++++++++ src/man/include/ad_modified_defaults.xml | 19 +++++++++++++++++++ src/responder/nss/nss_protocol_pwent.c | 2 +- src/tests/intg/test_files_provider.py | 2 +- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index a3eb9c66d9..17bb4f8274 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -1301,6 +1301,15 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, ret = ENOMEM; goto done; } + } else { + if (strcasecmp(domain->provider, "ad") == 0) { + /* ad provider default */ + domain->fallback_homedir = talloc_strdup(domain, "/home/%d/%u"); + if (!domain->fallback_homedir) { + ret = ENOMEM; + goto done; + } + } } tmp = ldb_msg_find_attr_as_string(res->msgs[0], diff --git a/src/man/include/ad_modified_defaults.xml b/src/man/include/ad_modified_defaults.xml index 818a2bf787..425b7e8ee0 100644 --- a/src/man/include/ad_modified_defaults.xml +++ b/src/man/include/ad_modified_defaults.xml @@ -76,4 +76,23 @@ + + NSS configuration + + + + fallback_homedir = /home/%d/%u + + + The AD provider automatically sets + "fallback_homedir = /home/%d/%u" to provide personal + home directories for users without the homeDirectory + attribute. If your AD Domain is properly + populated with Posix attributes, and you want to avoid + this fallback behavior, you can explicitly + set "fallback_homedir = %o". + + + + diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c index af9e74fc86..86fa4ec465 100644 --- a/src/responder/nss/nss_protocol_pwent.c +++ b/src/responder/nss/nss_protocol_pwent.c @@ -118,7 +118,7 @@ nss_get_homedir(TALLOC_CTX *mem_ctx, homedir = nss_get_homedir_override(mem_ctx, msg, nss_ctx, domain, &hd_ctx); if (homedir == NULL) { - return "/"; + return ""; } return homedir; diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index ead1cc4c34..4761f1bd15 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -678,7 +678,7 @@ def test_user_no_dir(setup_pw_with_canary, files_domain_only): Test that resolving a user without a homedir defined works and returns a fallback value """ - check_user(incomplete_user_setup(setup_pw_with_canary, 'dir', '/')) + check_user(incomplete_user_setup(setup_pw_with_canary, 'dir', '')) def test_user_no_gecos(setup_pw_with_canary, files_domain_only): From 8e6c52f6b043cb4f7b4065ab6a9012a5c1acbf8f Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 25 Jan 2019 12:13:45 +0100 Subject: [PATCH 19/29] sss_cache: Do not fail for missing domains The conf.db needn't exist(sssd has never been started) and in such situation sss_cache failed when trying to invalidate all entries. There is nothing to invalidate and therefore we are already in state which we want to achieve with calling sss_cache. No reason to fail. Resolves: https://pagure.io/SSSD/sssd/issue/3919 Merges: https://pagure.io/SSSD/sssd/pull-request/3926 Reviewed-by: Jakub Hrozek (cherry picked from commit 88c0c3fcd1d97bd499bb28c2065ba19d629fa4f7) --- src/confdb/confdb.c | 2 +- src/tools/sss_cache.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 17bb4f8274..446d203dd8 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -1504,7 +1504,7 @@ int confdb_get_domains(struct confdb_ctx *cdb, CONFDB_MONITOR_ACTIVE_DOMAINS, &domlist); if (ret == ENOENT) { - DEBUG(SSSDBG_FATAL_FAILURE, "No domains configured, fatal error!\n"); + DEBUG(SSSDBG_MINOR_FAILURE, "No domains configured, fatal error!\n"); goto done; } if (ret != EOK ) { diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c index 8a40b38c07..eb310d39a9 100644 --- a/src/tools/sss_cache.c +++ b/src/tools/sss_cache.c @@ -148,7 +148,11 @@ int main(int argc, const char *argv[]) struct sss_domain_info *dinfo; ret = init_context(argc, argv, &tctx); - if (ret != EOK) { + if (ret == ENOENT) { + /* nothing to invalidate; no reason to fail */ + ret = EOK; + goto done; + } else if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Error initializing context for the application\n"); goto done; @@ -847,7 +851,10 @@ static errno_t init_context(int argc, const char *argv[], } ret = init_domains(ctx, values.domain); - if (ret != EOK) { + if (ret == ENOENT) { + /* Nothing to invalidate; do not log confusing messages. */ + goto fini; + } else if (ret != EOK) { if (values.domain) { ERROR("Could not open domain %1$s. If the domain is a subdomain " "(trusted domain), use fully qualified name instead of " From 0a27a4716adcb2d08e08db3ed719a156691106e7 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Thu, 24 Jan 2019 22:27:44 +0100 Subject: [PATCH 20/29] intg: Add test for sss_cache & shadow-utils use-case Related to: https://pagure.io/SSSD/sssd/issue/3919 Reviewed-by: Jakub Hrozek (cherry picked from commit 325df4acae303efeabd96d2247fb5799c728536a) --- src/tests/intg/test_sss_cache.py | 34 ++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 src/tests/intg/test_sss_cache.py diff --git a/src/tests/intg/test_sss_cache.py b/src/tests/intg/test_sss_cache.py new file mode 100644 index 0000000000..22f12f0764 --- /dev/null +++ b/src/tests/intg/test_sss_cache.py @@ -0,0 +1,34 @@ +# +# SSSD files domain tests +# +# Copyright (c) 2019 Red Hat, Inc. +# Author: Lukas Slebodnik +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import subprocess + + +def test_missing_domains(): + # Utilities in shadow-utils call sss_cache but it might fail in case + # sssd has never been started on such host. + ret = subprocess.call(["sss_cache", "-U"]) + assert ret == 0 + + ret = subprocess.call(["sss_cache", "-G"]) + assert ret == 0 + + ret = subprocess.call(["sss_cache", "-E"]) + assert ret == 0 From 498aaac23db9334ec83de72c8b6c4fbf708120b2 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 25 Jan 2019 12:17:59 +0100 Subject: [PATCH 21/29] sss_cache: Do not fail if noting was cached It might happen that we have some domains in conf.db but nothing has been cached yet. sss_cache failed in such situation, bash-4.4# sss_cache -E No cache object matched the specified search bash-4.4# echo $? 2 Because there is nothing to invalidate and so we are already in state which we want to achieve with calling sss_cache. There is no reason to fail. We will still fail for invalidating particular entry. User might have a typo in the name and should be informed about possible mistake. bash-4.4# sss_cache -u test_user No cache object matched the specified search bash-4.4# echo $? 2 Resolves: https://pagure.io/SSSD/sssd/issue/3919 Merges: https://pagure.io/SSSD/sssd/pull-request/3926 Reviewed-by: Jakub Hrozek (cherry picked from commit 71475f1ed78a65d78f75e5ca0fdc6e20cfdf2f39) --- src/tools/sss_cache.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c index eb310d39a9..b6ff874023 100644 --- a/src/tools/sss_cache.c +++ b/src/tools/sss_cache.c @@ -488,6 +488,13 @@ static bool invalidate_entries(TALLOC_CTX *ctx, if (ret == ENOENT) { DEBUG(SSSDBG_TRACE_FUNC, "'%s' %s: Not found in domain '%s'\n", type_string, name ? name : "", dinfo->name); + if (name == NULL) { + /* nothing to invalidate in that domain, no reason to fail */ + return true; + } else { + /* we failed to invalidate explicit name; inform about it */ + return false; + } } else { DEBUG(SSSDBG_CRIT_FAILURE, "Searching for %s in domain %s with filter %s failed\n", From 7983826c3c3736d71bd4447be5f178521b809d64 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 25 Jan 2019 12:43:21 +0100 Subject: [PATCH 22/29] test_sss_cache: Add test case for invalidating missing entries Related to: https://pagure.io/SSSD/sssd/issue/3919 Reviewed-by: Jakub Hrozek (cherry picked from commit 415094687e92789060626176c5ced31d4122692d) --- src/tests/intg/test_sss_cache.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/tests/intg/test_sss_cache.py b/src/tests/intg/test_sss_cache.py index 22f12f0764..ced6c35943 100644 --- a/src/tests/intg/test_sss_cache.py +++ b/src/tests/intg/test_sss_cache.py @@ -32,3 +32,30 @@ def test_missing_domains(): ret = subprocess.call(["sss_cache", "-E"]) assert ret == 0 + + +def test_nothing_cache(): + # Ensure we do not fail in case there are not any entries to invalidate + ret = subprocess.call(["sssd", "--genconf"]) + assert ret == 0 + + ret = subprocess.call(["sss_cache", "-U"]) + assert ret == 0 + + ret = subprocess.call(["sss_cache", "-G"]) + assert ret == 0 + + ret = subprocess.call(["sss_cache", "-E"]) + assert ret == 0 + + +def test_invalidate_missing_specific_entry(): + # Ensure we will fail when invalidating missing specific entry + ret = subprocess.call(["sssd", "--genconf"]) + assert ret == 0 + + ret = subprocess.call(["sss_cache", "-u", "non-existing"]) + assert ret == 2 + + ret = subprocess.call(["sss_cache", "-g", "non-existing"]) + assert ret == 2 From 088eb5451173624cdf798cc926c399baf8ed1a98 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 18 Jan 2019 20:55:38 +0100 Subject: [PATCH 23/29] pyhbac-test: Do not use assertEquals src/tests/pyhbac-test.py:163: DeprecationWarning: Please use assertEqual instead. self.assertEquals(el.__repr__(), u'') src/tests/pyhbac-test.py:169: DeprecationWarning: Please use assertEqual instead. u'') Merges: https://pagure.io/SSSD/sssd/pull-request/3927 Reviewed-by: Jakub Hrozek (cherry picked from commit 9b06c750b66ac675d3be19b15a60888153f11758) --- src/tests/pyhbac-test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/pyhbac-test.py b/src/tests/pyhbac-test.py index 2b7e0334bd..06163afc50 100755 --- a/src/tests/pyhbac-test.py +++ b/src/tests/pyhbac-test.py @@ -160,13 +160,13 @@ def _get_rule(): def testRepr(self): el = pyhbac.HbacRuleElement() - self.assertEquals(el.__repr__(), u'') + self.assertEqual(el.__repr__(), u'') el.category.add(pyhbac.HBAC_CATEGORY_ALL) el.names = ['foo'] el.groups = ['bar, baz'] - self.assertEquals(el.__repr__(), - u'') + self.assertEqual(el.__repr__(), + u'') class PyHbacRuleTest(unittest.TestCase): From b27ab9e75360fe94174dea2cb0ff96bf83e3fc65 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 18 Jan 2019 21:00:47 +0100 Subject: [PATCH 24/29] SSSDConfigTest: Do not use assertEquals src/config/SSSDConfigTest.py:88: DeprecationWarning: Please use assertEqual instead. self.assertEquals(new_options['debug_level'][0], int) src/config/SSSDConfigTest.py:91: DeprecationWarning: Please use assertEqual instead. self.assertEquals(new_options['command'][0], str) src/config/SSSDConfigTest.py:94: DeprecationWarning: Please use assertEqual instead. self.assertEquals(new_options['reconnection_retries'][0], int) + many more Merges: https://pagure.io/SSSD/sssd/pull-request/3927 Reviewed-by: Jakub Hrozek (cherry picked from commit c4db34c17a415abff585632d3f7547392e564cc2) --- src/config/SSSDConfigTest.py | 70 ++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 44acbf2baa..3e0972e7a6 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -85,36 +85,36 @@ def testServices(self): new_options = new_sssd_service.list_options(); self.assertTrue('debug_level' in new_options) - self.assertEquals(new_options['debug_level'][0], int) + self.assertEqual(new_options['debug_level'][0], int) self.assertTrue('command' in new_options) - self.assertEquals(new_options['command'][0], str) + self.assertEqual(new_options['command'][0], str) self.assertTrue('reconnection_retries' in new_options) - self.assertEquals(new_options['reconnection_retries'][0], int) + self.assertEqual(new_options['reconnection_retries'][0], int) self.assertTrue('services' in new_options) - self.assertEquals(new_options['debug_level'][0], int) + self.assertEqual(new_options['debug_level'][0], int) self.assertTrue('domains' in new_options) - self.assertEquals(new_options['domains'][0], list) - self.assertEquals(new_options['domains'][1], str) + self.assertEqual(new_options['domains'][0], list) + self.assertEqual(new_options['domains'][1], str) self.assertTrue('sbus_timeout' in new_options) - self.assertEquals(new_options['sbus_timeout'][0], int) + self.assertEqual(new_options['sbus_timeout'][0], int) self.assertTrue('re_expression' in new_options) - self.assertEquals(new_options['re_expression'][0], str) + self.assertEqual(new_options['re_expression'][0], str) self.assertTrue('full_name_format' in new_options) - self.assertEquals(new_options['full_name_format'][0], str) + self.assertEqual(new_options['full_name_format'][0], str) self.assertTrue('default_domain_suffix' in new_options) - self.assertEquals(new_options['default_domain_suffix'][0], str) + self.assertEqual(new_options['default_domain_suffix'][0], str) self.assertTrue('domain_resolution_order' in new_options) - self.assertEquals(new_options['domain_resolution_order'][0], list) - self.assertEquals(new_options['domain_resolution_order'][1], str) + self.assertEqual(new_options['domain_resolution_order'][0], list) + self.assertEqual(new_options['domain_resolution_order'][1], str) del sssdconfig @@ -1129,15 +1129,15 @@ def testRemoveProvider(self): domain.set_option('krb5_realm', 'EXAMPLE.COM') domain.set_option('ldap_uri', 'ldap://ldap.example.com') - self.assertEquals(domain.get_option('krb5_realm'), - 'EXAMPLE.COM') - self.assertEquals(domain.get_option('ldap_uri'), - 'ldap://ldap.example.com') + self.assertEqual(domain.get_option('krb5_realm'), + 'EXAMPLE.COM') + self.assertEqual(domain.get_option('ldap_uri'), + 'ldap://ldap.example.com') # Remove the LDAP provider and verify that krb5_realm remains domain.remove_provider('id') - self.assertEquals(domain.get_option('krb5_realm'), - 'EXAMPLE.COM') + self.assertEqual(domain.get_option('krb5_realm'), + 'EXAMPLE.COM') self.assertFalse('ldap_uri' in domain.options) # Put the LOCAL provider back @@ -1916,7 +1916,7 @@ def testSaveDomain(self): self.assertFalse('example.com' in sssdconfig.list_active_domains()) self.assertFalse('example.com' in sssdconfig.list_inactive_domains()) self.assertFalse(sssdconfig.has_section('domain/example.com')) - self.assertEquals(domain.oldname, None) + self.assertEqual(domain.oldname, None) # Positive test - Set the domain inactive and save it activelist = sssdconfig.list_active_domains() @@ -1928,10 +1928,10 @@ def testSaveDomain(self): self.assertFalse('example.com2' in sssdconfig.list_active_domains()) self.assertTrue('example.com2' in sssdconfig.list_inactive_domains()) - self.assertEquals(len(sssdconfig.list_active_domains()), - len(activelist)-1) - self.assertEquals(len(sssdconfig.list_inactive_domains()), - len(inactivelist)+1) + self.assertEqual(len(sssdconfig.list_active_domains()), + len(activelist)-1) + self.assertEqual(len(sssdconfig.list_inactive_domains()), + len(inactivelist)+1) # Positive test - Set the domain active and save it activelist = sssdconfig.list_active_domains() @@ -1942,10 +1942,10 @@ def testSaveDomain(self): self.assertTrue('example.com2' in sssdconfig.list_active_domains()) self.assertFalse('example.com2' in sssdconfig.list_inactive_domains()) - self.assertEquals(len(sssdconfig.list_active_domains()), - len(activelist)+1) - self.assertEquals(len(sssdconfig.list_inactive_domains()), - len(inactivelist)-1) + self.assertEqual(len(sssdconfig.list_active_domains()), + len(activelist)+1) + self.assertEqual(len(sssdconfig.list_inactive_domains()), + len(inactivelist)-1) # Positive test - Set the domain inactive and save it activelist = sssdconfig.list_active_domains() @@ -1956,10 +1956,10 @@ def testSaveDomain(self): self.assertFalse('example.com2' in sssdconfig.list_active_domains()) self.assertTrue('example.com2' in sssdconfig.list_inactive_domains()) - self.assertEquals(len(sssdconfig.list_active_domains()), - len(activelist)-1) - self.assertEquals(len(sssdconfig.list_inactive_domains()), - len(inactivelist)+1) + self.assertEqual(len(sssdconfig.list_active_domains()), + len(activelist)-1) + self.assertEqual(len(sssdconfig.list_inactive_domains()), + len(inactivelist)+1) # Positive test - Set the domain active and save it activelist = sssdconfig.list_active_domains() @@ -1970,10 +1970,10 @@ def testSaveDomain(self): self.assertTrue('example.com2' in sssdconfig.list_active_domains()) self.assertFalse('example.com2' in sssdconfig.list_inactive_domains()) - self.assertEquals(len(sssdconfig.list_active_domains()), - len(activelist)+1) - self.assertEquals(len(sssdconfig.list_inactive_domains()), - len(inactivelist)-1) + self.assertEqual(len(sssdconfig.list_active_domains()), + len(activelist)+1) + self.assertEqual(len(sssdconfig.list_inactive_domains()), + len(inactivelist)-1) # Positive test - Ensure that saved domains retain values domain.set_option('ldap_krb5_init_creds', True) From 07d7eeaec97910e4ae3bba29e2518b4a94908fe8 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 18 Jan 2019 21:29:50 +0100 Subject: [PATCH 25/29] SSSDConfig: Fix ResourceWarning unclosed file /usr/lib64/python3.7/unittest/case.py:763: ResourceWarning: unclosed file <_io.TextIOWrapper name='src/config/testconfigs/sssd-invalid.conf' mode='r' encoding='UTF-8'> context = None ResourceWarning: Enable tracemalloc to get the object allocation traceback /usr/lib64/python3.7/unittest/case.py:763: ResourceWarning: unclosed file <_io.TextIOWrapper name='src/config/testconfigs/noparse.api.conf' mode='r' encoding='UTF-8'> context = None ResourceWarning: Enable tracemalloc to get the object allocation traceback Merges: https://pagure.io/SSSD/sssd/pull-request/3927 Reviewed-by: Jakub Hrozek (cherry picked from commit 769dc244771db5aebeada2f45fbb284b8076cc42) --- src/config/SSSDConfig/__init__.py.in | 31 ++++++++++++---------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 2846ea2642..6f4cd98e36 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -501,16 +501,14 @@ class SSSDConfigSchema(SSSDChangeConf): schemaplugindir = '@datadir@/sssd/sssd.api.d' try: - #Read the primary config file - fd = open(schemafile, 'r') - self.readfp(fd) - fd.close() + # Read the primary config file + with open(schemafile, 'r') as fd: + self.readfp(fd) # Read in the provider files for file in filter(lambda f: re.search(r'^sssd-.*\.conf$', f), os.listdir(schemaplugindir)): - fd = open(schemaplugindir+ "/" + file) - self.readfp(fd) - fd.close() + with open(schemaplugindir+ "/" + file) as fd: + self.readfp(fd) except IOError: raise except SyntaxError: # can be raised with readfp @@ -1453,14 +1451,12 @@ class SSSDConfig(SSSDChangeConf): #TODO: get this from a global setting configfile = '@sysconfdir@/sssd/sssd.conf' # open will raise an IOError if it fails - fd = open(configfile, 'r') - - try: - self.readfp(fd) - except: - raise ParsingError + with open(configfile, 'r') as fd: + try: + self.readfp(fd) + except: + raise ParsingError - fd.close() self.configfile = configfile self.initialized = True @@ -1524,10 +1520,9 @@ class SSSDConfig(SSSDChangeConf): # open() will raise IOError if it fails old_umask = os.umask(0o177) - of = open(outputfile, "wb") - output = self.dump(self.opts).encode('utf-8') - of.write(output) - of.close() + with open(outputfile, "wb") as of: + output = self.dump(self.opts).encode('utf-8') + of.write(output) os.umask(old_umask) def list_active_services(self): From 3c0213fe55a7be01f0fbca77b7306ef1c1a46310 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 18 Jan 2019 21:41:30 +0100 Subject: [PATCH 26/29] SSSDConfigTest: Remove usage of failUnless src/config/SSSDConfigTest.py:1855: DeprecationWarning: Please use assertTrue instead. self.failUnless(domain.get_name() in sssdconfig.list_domains()) src/config/SSSDConfigTest.py:1856: DeprecationWarning: Please use assertTrue instead. self.failUnless(domain.get_name() in sssdconfig.list_inactive_domains()) src/config/SSSDConfigTest.py:1585: DeprecationWarning: Please use assertTrue instead. self.failUnless(service.get_name() in sssdconfig.list_services()) Merges: https://pagure.io/SSSD/sssd/pull-request/3927 Reviewed-by: Jakub Hrozek (cherry picked from commit 21bba050994aa59ab275a99dfa711b36d2900ebb) --- src/config/SSSDConfigTest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 3e0972e7a6..8262e822a2 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -1584,7 +1584,7 @@ def testNewService(self): # First need to remove the existing service sssdconfig.delete_service('sssd') service = sssdconfig.new_service('sssd') - self.failUnless(service.get_name() in sssdconfig.list_services()) + self.assertTrue(service.get_name() in sssdconfig.list_services()) # TODO: check that the values of this new service # are set to the defaults from the schema @@ -1854,8 +1854,8 @@ def testNewDomain(self): # Positive Test domain = sssdconfig.new_domain('example.com') self.assertTrue(isinstance(domain, SSSDConfig.SSSDDomain)) - self.failUnless(domain.get_name() in sssdconfig.list_domains()) - self.failUnless(domain.get_name() in sssdconfig.list_inactive_domains()) + self.assertTrue(domain.get_name() in sssdconfig.list_domains()) + self.assertTrue(domain.get_name() in sssdconfig.list_inactive_domains()) # TODO: check that the values of this new domain # are set to the defaults from the schema From 8f0a2acdc6c607748ba30254bb764e3b462c193e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Sat, 26 Jan 2019 13:38:59 +0100 Subject: [PATCH 27/29] BUILD: Fix condition for building sssd-kcm man page Merges: https://pagure.io/SSSD/sssd/pull-request/3928 Reviewed-by: Jakub Hrozek (cherry picked from commit 39b3b0e4a90d0624040319a177ebcb81568fc1b9) --- src/man/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/man/Makefile.am b/src/man/Makefile.am index 06e5253f4c..580cd311bd 100644 --- a/src/man/Makefile.am +++ b/src/man/Makefile.am @@ -27,7 +27,7 @@ endif if BUILD_SECRETS SEC_CONDS = ;with_secrets endif -if BUILD_SECRETS +if BUILD_KCM KCM_CONDS = ;with_kcm endif if BUILD_SYSTEMTAP From 9e6a224896474af27af6f85ec56741c5128ee4a0 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Sat, 26 Jan 2019 13:44:39 +0100 Subject: [PATCH 28/29] NSS: Do not use deprecated header files In file included from src/util/cert/nss/cert.c:26: /usr/include/nss3/key.h:9:9: note: #pragma message: key.h is deprecated. Please include keyhi.h instead. #pragma message("key.h is deprecated. Please include keyhi.h instead.") ^~~~~~~ Merges: https://pagure.io/SSSD/sssd/pull-request/3930 Reviewed-by: Jakub Hrozek (cherry picked from commit afd23bd7fbb3dad0e80d9e77e94ca2abf67d19b0) --- src/util/cert/nss/cert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c index a8efef818a..783323c6df 100644 --- a/src/util/cert/nss/cert.c +++ b/src/util/cert/nss/cert.c @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include #include From 955f3a45f93b22fa7764ed2240059abd6a5569b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Fri, 29 Mar 2019 11:10:18 +0100 Subject: [PATCH 29/29] ci: enable sssd-ci for 1-16 branch Fedora 28 is the latest version containing 1.16 so I think it is fine to not run the test against Fedora 29+. Besides this change this patch contains files from master without change. --- Jenkinsfile | 208 +++++++++++++++++++++++++++++++ contrib/test-suite/README.md | 38 ++++++ contrib/test-suite/run-client.sh | 37 ++++++ contrib/test-suite/run.sh | 100 +++++++++++++++ 4 files changed, 383 insertions(+) create mode 100644 Jenkinsfile create mode 100644 contrib/test-suite/README.md create mode 100755 contrib/test-suite/run-client.sh create mode 100755 contrib/test-suite/run.sh diff --git a/Jenkinsfile b/Jenkinsfile new file mode 100644 index 0000000000..3cad2570e1 --- /dev/null +++ b/Jenkinsfile @@ -0,0 +1,208 @@ +/** + * Remember that the build failed because one of the untrusted files were + * modified. + */ +untrusted = false + +/** + * SSSD CI. + * + * This class hold SSSD CI settings and defines several helper methods + * that helps reducing code duplication. Unfortunately, it does not + * seem to be possible to run those methods directly from the pipeline + * as CI.MethodName() as it produces 'Expected a symbol' error therefore + * functions outside this class scope must be defined as well. These functions + * can be then called directly from the pipeline. + */ +class CI { + /** + * Absolute path to directory that holds the workspace on Jenkins slave. + */ + public static String BaseDir = '/home/fedora' + + /** + * Github status context name that is visible in pull request statuses. + */ + public static String GHContext = 'sssd-ci' + + /** + * URL that will be opened when user clicks on 'details' on 'sssd-ci' status. + */ + public static String GHUrl = 'https://pagure.io/SSSD/sssd' + + /** + * URL that will be opened when user clicks on 'details' on specific + * build status (e.g. sssd-ci/fedora28). + */ + public static String AWS = 'https://s3.eu-central-1.amazonaws.com/sssd-ci' + + /** + * Path to SSSD Test Suite on Jenkins slave. + */ + public static String SuiteDir = this.BaseDir + '/sssd-test-suite' + + /** + * Workaround for https://issues.jenkins-ci.org/browse/JENKINS-39203 + * + * At this moment if one stage in parallel block fails, failure branch in + * post block is run in all stages even though they might have been successful. + * + * We remember result of test stages in this variable so we can correctly + * report a success or error even if one of the stages that are run in + * parallel failed. + */ + public static def Results = [:] + + /** + * Mark build as successfull. + */ + public static def BuildSuccessful(build) { + this.Results[build] = "success" + } + + /** + * Return true if the build was successful. + */ + public static def IsBuildSuccessful(build) { + return this.Results[build] == "success" + } + + /** + * Send commit status to Github for sssd-ci context. + */ + public static def Notify(ctx, status, message) { + ctx.githubNotify status: status, + context: this.GHContext, + description: message, + targetUrl: this.GHUrl + } + + /** + * Send commit status to Github for specific build (e.g. sssd-ci/fedora28). + */ + public static def NotifyBuild(ctx, status, message) { + ctx.githubNotify status: status, + context: String.format('%s/%s', this.GHContext, ctx.env.TEST_SYSTEM), + description: message, + targetUrl: String.format( + '%s/%s/%s/%s/index.html', + this.AWS, + ctx.env.BRANCH_NAME, + ctx.env.BUILD_ID, + ctx.env.TEST_SYSTEM + ) + } + + /** + * Run tests. TEST_SYSTEM environment variable must be defined. + */ + public static def RunTests(ctx) { + this.NotifyBuild(ctx, 'PENDING', 'Build is in progress.') + + ctx.sh String.format( + './sssd/contrib/test-suite/run.sh %s %s %s %s', + "${ctx.env.WORKSPACE}/sssd", + "${this.SuiteDir}", + "${ctx.env.WORKSPACE}/artifacts/${ctx.env.TEST_SYSTEM}", + "${this.BaseDir}/configs/${ctx.env.TEST_SYSTEM}.json" + ) + + this.BuildSuccessful(ctx.env.TEST_SYSTEM) + } + + /** + * Archive artifacts and notify Github about build result. + */ + public static def WhenCompleted(ctx) { + ctx.archiveArtifacts artifacts: "artifacts/**", allowEmptyArchive: true + ctx.sh String.format( + "${this.BaseDir}/scripts/archive.sh %s %s %s", + ctx.env.TEST_SYSTEM, + "${ctx.env.WORKSPACE}/artifacts/${ctx.env.TEST_SYSTEM}", + "${ctx.env.BRANCH_NAME}/${ctx.env.BUILD_ID}" + ) + ctx.sh "rm -fr ${ctx.env.WORKSPACE}/artifacts/${ctx.env.TEST_SYSTEM}" + + if (this.IsBuildSuccessful(ctx.env.TEST_SYSTEM)) { + this.NotifyBuild(ctx, 'SUCCESS', 'Success.') + return + } + + this.NotifyBuild(ctx, 'FAILURE', 'Build failed.') + } + + /** + * Notify Github that the build was aborted. + */ + public static def WhenAborted(ctx) { + this.NotifyBuild(ctx, 'ERROR', 'Aborted.') + } +} + +/** + * CI class methods cannot be called directly from the pipeline as it + * yield 'Expected a symbol' error for some reason. This is a workaround + * for this issue. + */ +def CI_RunTests() { CI.RunTests(this) } +def CI_Post() { CI.WhenCompleted(this) } +def CI_Aborted() { CI.WhenAborted(this) } +def CI_Notify(status, message) { CI.Notify(this, status, message) } + +pipeline { + agent none + options { + timeout(time: 10, unit: 'HOURS') + checkoutToSubdirectory('sssd') + } + stages { + stage('Prepare') { + steps { + CI_Notify('PENDING', 'Running tests.') + } + } + stage('Read trusted files') { + steps { + readTrusted './contrib/test-suite/run.sh' + readTrusted './contrib/test-suite/run-client.sh' + } + post { + failure { + script { + untrusted = true + } + } + } + } + stage('Run Tests') { + parallel { + stage('Test on Fedora 28') { + agent {label "sssd-ci"} + environment { TEST_SYSTEM = "fedora28" } + steps { CI_RunTests() } + post { + always { CI_Post() } + aborted { CI_Aborted() } + } + } + } + } + } + post { + failure { + script { + if (untrusted) { + CI_Notify('ERROR', 'Untrusted files were modified.') + } else { + CI_Notify('FAILURE', 'Some tests failed.') + } + } + } + aborted { + CI_Notify('ERROR', 'Builds were aborted.') + } + success { + CI_Notify('SUCCESS', 'All tests succeeded.') + } + } +} diff --git a/contrib/test-suite/README.md b/contrib/test-suite/README.md new file mode 100644 index 0000000000..576d12d286 --- /dev/null +++ b/contrib/test-suite/README.md @@ -0,0 +1,38 @@ +# Run SSSD Test Suite + +Script `run.sh` will run all available SSSD test on a set of virtual machines created by vagrant. These virtual machines are part of separate project located at `https://github.com/SSSD/sssd-test-suite`. + +## Automated Testing + +These test are run automatically when you submit a Pull Request to SSSD project. Status report together with logs will be available in the Pull Request when testing is finished. + +## Steps to run the tests manually + +1. Checkout `https://github.com/SSSD/sssd-test-suite` +2. Configure and setup SSSD test suite per instructions located at project readme. +3. Make sssd-test-suite use already provisioned boxes (either manually created or maintained by SSSD team at https://app.vagrantup.com/sssd-vagrant). +4. Run `run.sh`, please note that this script will call `vagrant destroy` and it will thus destroy your existing guests. + +``` +run.sh SSSD-SOURCE-DIR TEST-SUITE-DIR ARTIFACTS-DIR CONFIG-FILE + SSSD-SOURCE-DIR Path to SSSD source directory. + TEST-SUITE-DIR Path to sssd-test-suite_dir directory. + ARTIFACTS-DIR Path to directory where artifacts should be stored. + CONFIG-FILE Path to sssd-test-suite_dir configuration file to use. +``` + +At this moment only `client` guest is required. We need to expand our test cases to test agains FreeIPA and Active Directory. + +## SSSD CI Architecture + +Jenkins master polls github for new branches and pull requests. When it discovers new pull request or branch or changes to existing pull request or branch it will allocate a jenkins agent and executes pipeline defined in `./Jenkinsfile` (in SSSD source) on this agent. + +The pipeline executes `./contrib/test-suite/run.sh` and archives logs when testing is finished. Script `./contrib/test-suite/run.sh` prepares sssd-test-suite, starts the vagrant machines and copy SSSD source code to the client machine. Then it calls `./contrib/test-suite/run-client.sh` on the client machine which runs continuous integration tests. + +### Extending current tests +To extend current testing capabilities, modify `./contrib/test-suite/run.sh` and `./contrib/test-suite/run-client.sh` to new requirements. These files can be modified by anyone but are considered untrusted from contributor that is not an administrator of SSSD repository. This means that if a public contributor submits a pull request that changes those files, Jenkins will refuse to run tests. + +### Adding additional distribution to test on +You need to modify `./Jenkinsfile`. Simply copy, paste and amend existing Fedora 28 stage. This file is also considered untrusted so only administrators can modify it within a pull request. + +You also need to extend `sssd-test-suite` and prepare vagrant boxes for this distro. diff --git a/contrib/test-suite/run-client.sh b/contrib/test-suite/run-client.sh new file mode 100755 index 0000000000..3b9eb606fa --- /dev/null +++ b/contrib/test-suite/run-client.sh @@ -0,0 +1,37 @@ +#!/bin/bash +# +# DO NOT RUN THIS MANUALLY +# + +sssd_source="/shared/sssd" +artifacts_dir="/shared/artifacts" + +archive-artifacts() { + echo "Archiving artifacts..." + + cp -f $sssd_source/ci-*.log $artifacts_dir + cp -f $sssd_source/ci-build-debug/ci-*.log $artifacts_dir + cp -f $sssd_source/ci-build-debug/test-suite.log $artifacts_dir +} + +success-or-die() { + ret=$1 + msg=$2 + if [ $ret -eq 0 ]; then + return 0 + fi + + echo $msg + archive-artifacts + + exit $ret +} + +cd $sssd_source + +echo "[1/1] Running Continuous Integration Tests" +./contrib/ci/run --moderate --no-deps +success-or-die $? "CI Failed!" + +archive-artifacts +exit 0 diff --git a/contrib/test-suite/run.sh b/contrib/test-suite/run.sh new file mode 100755 index 0000000000..bb4c06f59e --- /dev/null +++ b/contrib/test-suite/run.sh @@ -0,0 +1,100 @@ +#!/bin/bash + +print-usage() { + cat < /dev/null + + run-vagrant box update $1 + success-or-die $? "Unable to update guest: $1" + + run-vagrant up $1 + success-or-die $? "Unable to start guest: $1" +} + +stop-guest() { + run-vagrant halt $1 + success-or-die $? "Unable to halt guest: $1" +} + +echo "[1/5] Creating $artifacts_dir" +mkdir -p "$artifacts_dir" +success-or-die $? "Unable to create directory: $artifacts_dir" + +echo "[2/5] Updating sssd-test-suite" +git -C "$suite_dir" pull --rebase +success-or-die $? "Unable to rebase sssd-test-suite at: $suite_dir" + +echo "[3/5] Preparing vagrant machines" +for guest in $guests; do + start-guest $guest +done + +echo "[4/5] Running tests" +run-vagrant ssh client -- "$guest_source/contrib/test-suite/run-client.sh" +success-or-die $? "SSSD Test Suite Failed: $?" + +echo "[5/5] Shutdown machines" +for guest in $guests; do + stop-guest $guest +done