One way to fix this would be to revert: f58ffb26aeaae0642a149643672fa59ec01a3a36 1476d5348fcf387e7481d833becbd993d91f8019
These patches simplified the entryUSN fitler from (&(entryUSN >= $last)(entryUSN != $last)) to (entryUSN >= $last + 1). This was requested by Thierry to improve filter-evaluation performance so I choose to keep this filter and move the incremental logic into parsing function, appending "Z" or whatever we string remains to the new value in case of modifyTimestamp attribute.
From fb6c21f1e511592d24dbf712d457ce629426b389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Fri, 4 Mar 2016 12:10:35 +0100 Subject: [PATCH] SUDO: be able to parse modifyTimestamp correctly We were unable to parse modifyTimestamp where a non-numeric part (timezone) was involved. The format is YYYYMMDDHHmmssZ. It may also contain fraction or different timezone, everytime separated from the datetime by character. This patch gets the numberic part and then appends the string part again to get value usable in filter. Resolves: https://fedorahosted.org/sssd/ticket/2970 --- src/providers/ipa/ipa_sudo_refresh.c | 24 ++++++++--------- src/providers/ldap/sdap.h | 2 +- src/providers/ldap/sdap_sudo_refresh.c | 17 ++++++------ src/providers/ldap/sdap_sudo_shared.c | 48 ++++++++++++++++++++++++++++------ 4 files changed, 62 insertions(+), 29 deletions(-) diff --git a/src/providers/ipa/ipa_sudo_refresh.c b/src/providers/ipa/ipa_sudo_refresh.c index 7871802ef7462ce98f6ff43bc33da57ff123ff6f..e7219d3147c860a0162bfe8e5004fe6d22de7f25 100644 --- a/src/providers/ipa/ipa_sudo_refresh.c +++ b/src/providers/ipa/ipa_sudo_refresh.c @@ -153,7 +153,7 @@ ipa_sudo_smart_refresh_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; char *cmdgroups_filter; char *search_filter; - unsigned long usn; + const char *usn; errno_t ret; req = tevent_req_create(mem_ctx, &state, @@ -166,29 +166,29 @@ ipa_sudo_smart_refresh_send(TALLOC_CTX *mem_ctx, /* Download all rules from LDAP that are newer than usn */ if (srv_opts == NULL || srv_opts->max_sudo_value == 0) { DEBUG(SSSDBG_TRACE_FUNC, "USN value is unknown, assuming zero.\n"); - usn = 0; + usn = "0"; + search_filter = NULL; } else { - usn = srv_opts->max_sudo_value + 1; + usn = srv_opts->max_sudo_value; + search_filter = talloc_asprintf(state, "(%s>=%s)", + sudo_ctx->sudorule_map[IPA_AT_SUDORULE_ENTRYUSN].name, usn); + if (search_filter == NULL) { + ret = ENOMEM; + goto immediately; + } } - cmdgroups_filter = talloc_asprintf(state, "(%s>=%lu)", + cmdgroups_filter = talloc_asprintf(state, "(%s>=%s)", sudo_ctx->sudocmdgroup_map[IPA_AT_SUDOCMDGROUP_ENTRYUSN].name, usn); if (cmdgroups_filter == NULL) { ret = ENOMEM; goto immediately; } - search_filter = talloc_asprintf(state, "(%s>=%lu)", - sudo_ctx->sudorule_map[IPA_AT_SUDORULE_ENTRYUSN].name, usn); - if (search_filter == NULL) { - ret = ENOMEM; - goto immediately; - } - /* Do not remove any rules that are already in the sysdb. */ DEBUG(SSSDBG_TRACE_FUNC, "Issuing a smart refresh of sudo rules " - "(USN > %lu)\n", usn); + "(USN >= %s)\n", usn); subreq = ipa_sudo_refresh_send(state, ev, sudo_ctx, cmdgroups_filter, search_filter, NULL); diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index e0e05da0c8270a8f131870bc755da862e43783cb..44b8cfb1c971e059db5d8a12613db9c0e67d13a6 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -485,7 +485,7 @@ struct sdap_server_opts { char *max_user_value; char *max_group_value; char *max_service_value; - unsigned long max_sudo_value; + char *max_sudo_value; bool posix_checked; }; diff --git a/src/providers/ldap/sdap_sudo_refresh.c b/src/providers/ldap/sdap_sudo_refresh.c index 5ba858019e0bda91a9e0919ed2b0345d9faf085e..62a97dd12cd8b92f719494f173d028cb7c66928c 100644 --- a/src/providers/ldap/sdap_sudo_refresh.c +++ b/src/providers/ldap/sdap_sudo_refresh.c @@ -167,7 +167,7 @@ struct tevent_req *sdap_sudo_smart_refresh_send(TALLOC_CTX *mem_ctx, struct sdap_server_opts *srv_opts = id_ctx->srv_opts; struct sdap_sudo_smart_refresh_state *state = NULL; char *search_filter = NULL; - unsigned long usn; + const char *usn; int ret; req = tevent_req_create(mem_ctx, &state, struct sdap_sudo_smart_refresh_state); @@ -182,14 +182,15 @@ struct tevent_req *sdap_sudo_smart_refresh_send(TALLOC_CTX *mem_ctx, /* Download all rules from LDAP that are newer than usn */ if (srv_opts == NULL || srv_opts->max_sudo_value == 0) { DEBUG(SSSDBG_TRACE_FUNC, "USN value is unknown, assuming zero.\n"); - usn = 0; + usn = "0"; + search_filter = talloc_asprintf(state, "(objectclass=%s)", + map[SDAP_OC_SUDORULE].name); } else { - usn = srv_opts->max_sudo_value + 1; + usn = srv_opts->max_sudo_value; + search_filter = talloc_asprintf(state, "(&(objectclass=%s)(%s>=%s))", + map[SDAP_OC_SUDORULE].name, + map[SDAP_AT_SUDO_USN].name, usn); } - - search_filter = talloc_asprintf(state, "(&(objectclass=%s)(%s>=%lu))", - map[SDAP_OC_SUDORULE].name, - map[SDAP_AT_SUDO_USN].name, usn); if (search_filter == NULL) { ret = ENOMEM; goto immediately; @@ -199,7 +200,7 @@ struct tevent_req *sdap_sudo_smart_refresh_send(TALLOC_CTX *mem_ctx, * sysdb_filter = NULL; */ DEBUG(SSSDBG_TRACE_FUNC, "Issuing a smart refresh of sudo rules " - "(USN > %lu)\n", usn); + "(USN >= %s)\n", usn); subreq = sdap_sudo_refresh_send(state, sudo_ctx, search_filter, NULL); if (subreq == NULL) { diff --git a/src/providers/ldap/sdap_sudo_shared.c b/src/providers/ldap/sdap_sudo_shared.c index 72f55e14baa8f8cf896205fb20f14d5f446cfb0a..b9e5182a2f11d56b5cff0b77fd83282654eae94c 100644 --- a/src/providers/ldap/sdap_sudo_shared.c +++ b/src/providers/ldap/sdap_sudo_shared.c @@ -120,11 +120,38 @@ sdap_sudo_ptask_setup_generic(struct be_ctx *be_ctx, return EOK; } +static char * +sdap_sudo_new_usn(TALLOC_CTX *mem_ctx, + unsigned long usn, + const char *leftover) +{ + const char *str = leftover == NULL ? "" : leftover; + char *newusn; + + /* We increment USN number so that we can later use simplify filter + * (just usn >= last+1 instaed of usn >= last && usn != last). + */ + usn++; + + /* Convert back to string appending non-converted values since it + * is an indicator that modifyTimestamp is used instead of entryUSN. + * modifyTimestamp contains also timezone specification, usually Z. + * We can't really handle any errors here so we just use what we got. */ + newusn = talloc_asprintf(mem_ctx, "%lu%s", usn, str); + if (newusn == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, "Unable to change USN value (OOM)!\n"); + return NULL; + } + + return newusn; +} + void sdap_sudo_set_usn(struct sdap_server_opts *srv_opts, const char *usn) { - unsigned int usn_number; + unsigned long usn_number; + char *newusn; char *endptr = NULL; errno_t ret; @@ -140,24 +167,29 @@ sdap_sudo_set_usn(struct sdap_server_opts *srv_opts, errno = 0; usn_number = strtoul(usn, &endptr, 10); - if (endptr != NULL && *endptr != '\0') { - DEBUG(SSSDBG_MINOR_FAILURE, "Unable to convert USN %s\n", usn); - return; - } else if (errno != 0) { + if (errno != 0) { ret = errno; DEBUG(SSSDBG_MINOR_FAILURE, "Unable to convert USN %s [%d]: %s\n", usn, ret, sss_strerror(ret)); return; } - if (usn_number > srv_opts->max_sudo_value) { - srv_opts->max_sudo_value = usn_number; + newusn = sdap_sudo_new_usn(srv_opts, usn_number, endptr); + if (newusn == NULL) { + return; + } + + if (sysdb_compare_usn(newusn, srv_opts->max_sudo_value) > 0) { + talloc_zfree(srv_opts->max_sudo_value); + srv_opts->max_sudo_value = newusn; + } else { + talloc_zfree(newusn); } if (usn_number > srv_opts->last_usn) { srv_opts->last_usn = usn_number; } - DEBUG(SSSDBG_FUNC_DATA, "SUDO higher USN value: [%lu]\n", + DEBUG(SSSDBG_FUNC_DATA, "SUDO higher USN value: [%s]\n", srv_opts->max_sudo_value); } -- 2.1.0
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
