s3:winbindd: Simplfy sequence number caching
authorAndreas Schneider <asn@samba.org>
Fri, 23 Apr 2021 12:16:02 +0000 (14:16 +0200)
committerAndreas Schneider <asn@cryptomilk.org>
Thu, 29 Apr 2021 15:01:29 +0000 (15:01 +0000)
The sequence number is used to detect if the cache is still valid. It
expires when the `winbind cache time` is over. After that time we want
to fetch new information from a DC to make sure we are up to date.

If a DC goes down and we recreate the connection, we want to expire the
caches sooner. So we reset the sequence number and the next call should
refill the caches.

Using the current time as the sequence number is more reliable, as the
sequence number of two DCs could in theory be equal. All we have to do
is to make sure we reset it after we reconnect to a DC.

Previously the sequence number check was based on the AD database change
sequence number. Now this is based on a current time value which gets
reset after a successful (re)connect.

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Andreas Schneider <asn@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
source3/winbindd/winbindd_cache.c
source3/winbindd/winbindd_cm.c

index 201bb34b98a8da548f85d31e3086a0bd31aaf8b1..4a366ee6ff89075f65829f43b65d74c6ef72e72c 100644 (file)
@@ -534,13 +534,6 @@ static void refresh_sequence_number(struct winbindd_domain *domain)
 
        get_cache( domain );
 
-#if 0  /* JERRY -- disable as the default cache time is now 5 minutes */
-       /* trying to reconnect is expensive, don't do it too often */
-       if (domain->sequence_number == DOM_SEQUENCE_NONE) {
-               cache_time *= 8;
-       }
-#endif
-
        time_diff = t - domain->last_seq_check;
 
        /* see if we have to refetch the domain sequence number */
@@ -561,30 +554,9 @@ static void refresh_sequence_number(struct winbindd_domain *domain)
                goto done;
        }
 
-       /* important! make sure that we know if this is a native 
-          mode domain or not.  And that we can contact it. */
-
-       if ( winbindd_can_contact_domain( domain ) ) {          
-               status = domain->backend->sequence_number(domain, 
-                                                         &domain->sequence_number);
-       } else {
-               /* just use the current time */
-               status = NT_STATUS_OK;
-               domain->sequence_number = time(NULL);
-       }
-
-
-       /* the above call could have set our domain->backend to NULL when
-        * coming from offline to online mode, make sure to reinitialize the
-        * backend - Guenther */
-       get_cache( domain );
-
-       if (!NT_STATUS_IS_OK(status)) {
-               DEBUG(10,("refresh_sequence_number: failed with %s\n", nt_errstr(status)));
-               domain->sequence_number = DOM_SEQUENCE_NONE;
-       }
-
-       domain->last_status = status;
+       /* just use the current time */
+       domain->last_status = NT_STATUS_OK;
+       domain->sequence_number = time(NULL);
        domain->last_seq_check = time(NULL);
 
        /* save the new sequence number in the cache */
@@ -1488,11 +1460,6 @@ do_cached:
 
 do_query:
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        /* Put the query_user_list() in a retry loop.  There appears to be
         * some bug either with Windows 2000 or Samba's handling of large
         * rpc replies.  This manifests itself as sudden disconnection
@@ -1624,11 +1591,6 @@ do_query:
        *num_entries = 0;
        *info = NULL;
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("enum_dom_groups: [Cached] - doing backend query for list for domain %s\n",
                domain->name ));
 
@@ -1729,11 +1691,6 @@ do_query:
        *num_entries = 0;
        *info = NULL;
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("enum_local_groups: [Cached] - doing backend query for list for domain %s\n",
                domain->name ));
 
@@ -1845,17 +1802,6 @@ NTSTATUS wb_cache_name_to_sid(struct winbindd_domain *domain,
 
        ZERO_STRUCTP(sid);
 
-       /* If the seq number check indicated that there is a problem
-        * with this DC, then return that status... except for
-        * access_denied.  This is special because the dc may be in
-        * "restrict anonymous = 1" mode, in which case it will deny
-        * most unauthenticated operations, but *will* allow the LSA
-        * name-to-sid that we try as a fallback. */
-
-       if (!(NT_STATUS_IS_OK(domain->last_status)
-             || NT_STATUS_EQUAL(domain->last_status, NT_STATUS_ACCESS_DENIED)))
-               return domain->last_status;
-
        DEBUG(10,("name_to_sid: [Cached] - doing backend query for name for domain %s\n",
                domain->name ));
 
@@ -1984,17 +1930,6 @@ NTSTATUS wb_cache_sid_to_name(struct winbindd_domain *domain,
        *name = NULL;
        *domain_name = NULL;
 
-       /* If the seq number check indicated that there is a problem
-        * with this DC, then return that status... except for
-        * access_denied.  This is special because the dc may be in
-        * "restrict anonymous = 1" mode, in which case it will deny
-        * most unauthenticated operations, but *will* allow the LSA
-        * sid-to-name that we try as a fallback. */
-
-       if (!(NT_STATUS_IS_OK(domain->last_status)
-             || NT_STATUS_EQUAL(domain->last_status, NT_STATUS_ACCESS_DENIED)))
-               return domain->last_status;
-
        DEBUG(10,("sid_to_name: [Cached] - doing backend query for name for domain %s\n",
                domain->name ));
 
@@ -2276,21 +2211,6 @@ static NTSTATUS wcache_query_user(struct winbindd_domain *domain,
                return NT_STATUS_NOT_FOUND;
        }
 
-       /*
-        * If we have an access denied cache entry and a cached info3
-        * in the samlogon cache then do a query.  This will force the
-        * rpc back end to return the info3 data.
-        */
-
-       if (NT_STATUS_EQUAL(domain->last_status, NT_STATUS_ACCESS_DENIED) &&
-           netsamlogon_cache_have(user_sid)) {
-               DEBUG(10, ("query_user: cached access denied and have cached "
-                          "info3\n"));
-               domain->last_status = NT_STATUS_OK;
-               centry_free(centry);
-               return NT_STATUS_NOT_FOUND;
-       }
-
        /* if status is not ok then this is a negative hit
           and the rest of the data doesn't matter */
        status = centry->status;
@@ -2374,19 +2294,6 @@ static NTSTATUS wcache_lookup_usergroups(struct winbindd_domain *domain,
                return NT_STATUS_NOT_FOUND;
        }
 
-       /* If we have an access denied cache entry and a cached info3 in the
-           samlogon cache then do a query.  This will force the rpc back end
-           to return the info3 data. */
-
-       if (NT_STATUS_EQUAL(domain->last_status, NT_STATUS_ACCESS_DENIED)
-           && netsamlogon_cache_have(user_sid)) {
-               DEBUG(10, ("lookup_usergroups: cached access denied and have "
-                          "cached info3\n"));
-               domain->last_status = NT_STATUS_OK;
-               centry_free(centry);
-               return NT_STATUS_NOT_FOUND;
-       }
-
        num_sids = centry_uint32(centry);
        sids = talloc_array(mem_ctx, struct dom_sid, num_sids);
        if (sids == NULL) {
@@ -2433,11 +2340,6 @@ NTSTATUS wb_cache_lookup_usergroups(struct winbindd_domain *domain,
        (*num_groups) = 0;
        (*user_gids) = NULL;
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("lookup_usergroups: [Cached] - doing backend query for info for domain %s\n",
                domain->name ));
 
@@ -2589,9 +2491,6 @@ NTSTATUS wb_cache_lookup_useraliases(struct winbindd_domain *domain,
        (*num_aliases) = 0;
        (*alias_rids) = NULL;
 
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("lookup_usergroups: [Cached] - doing backend query for info "
                  "for domain %s\n", domain->name ));
 
@@ -2726,11 +2625,6 @@ NTSTATUS wb_cache_lookup_groupmem(struct winbindd_domain *domain,
        (*names) = NULL;
        (*name_types) = NULL;
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("lookup_groupmem: [Cached] - doing backend query for info for domain %s\n",
                domain->name ));
 
@@ -2861,11 +2755,6 @@ do_fetch_cache:
        return NT_STATUS_OK;
 
 do_query:
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("trusted_domains: [Cached] - doing backend query for info for domain %s\n",
                domain->name ));
 
@@ -2932,11 +2821,6 @@ do_fetch_cache:
 do_query:
        ZERO_STRUCTP(policy);
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("lockout_policy: [Cached] - doing backend query for info for domain %s\n",
                domain->name ));
 
@@ -3004,11 +2888,6 @@ do_fetch_cache:
 do_query:
        ZERO_STRUCTP(policy);
 
-       /* Return status value returned by seq number check */
-
-       if (!NT_STATUS_IS_OK(domain->last_status))
-               return domain->last_status;
-
        DEBUG(10,("password_policy: [Cached] - doing backend query for info for domain %s\n",
                domain->name ));
 
index df785a0ba623bdfb2288449920da94d732b0e883..79e840780049836ca076fcdd83ff4ca2e703faa0 100644 (file)
@@ -2070,6 +2070,10 @@ void invalidate_cm_connection(struct winbindd_domain *domain)
        NTSTATUS result;
        struct winbindd_cm_conn *conn = &domain->conn;
 
+       domain->sequence_number = DOM_SEQUENCE_NONE;
+       domain->last_seq_check = 0;
+       domain->last_status = NT_STATUS_SERVER_DISABLED;
+
        /* We're closing down a possibly dead
           connection. Don't have impossibly long (10s) timeouts. */