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

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

3 files changed, 50 insertions, 17 deletions

diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index e0e05da0c..44b8cfb1c 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 5ba858019..62a97dd12 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 72f55e14b..b9e5182a2 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);
 }