Caching user, group and domain sam handles was a stupid idea.

Now we just keep a record of the open pipes.

4 files changed, 177 insertions, 103 deletions

diff --git a/source/nsswitch/winbindd_cache.c b/source/nsswitch/winbindd_cache.c
index 00e1905d764..02639f25fb3 100644
--- a/source/nsswitch/winbindd_cache.c
+++ b/source/nsswitch/winbindd_cache.c
@@ -56,20 +56,40 @@ static uint32 domain_sequence_number(struct winbindd_domain *domain)
         uint16 switch_value = 2;
         NTSTATUS result;
         uint32 seqnum = DOM_SEQUENCE_NONE;
+        POLICY_HND dom_pol;
+        BOOL got_dom_pol = False;
+        uint32 des_access = SEC_RIGHTS_MAXIMUM_ALLOWED;
 
         if (!(mem_ctx = talloc_init()))
                 return DOM_SEQUENCE_NONE;
 
-        if (!(hnd = cm_get_sam_dom_handle(domain->name, &domain->sid)))
+        /* Get sam handle */
+
+        if (!(hnd = cm_get_sam_handle(domain->name)))
+                goto done;
+
+        /* Get domain handle */
+
+        result = cli_samr_open_domain(hnd->cli, mem_ctx, &hnd->pol, 
+                                      des_access, &domain->sid, &dom_pol);
+
+        if (!NT_STATUS_IS_OK(result))
                 goto done;
 
-        result = cli_samr_query_dom_info(hnd->cli, mem_ctx, &hnd->pol,
+        got_dom_pol = True;
+
+        /* Query domain info */
+
+        result = cli_samr_query_dom_info(hnd->cli, mem_ctx, &dom_pol,
                                          switch_value, &ctr);
 
         if (NT_STATUS_IS_OK(result))
                 seqnum = ctr.info.inf2.seq_num;
 
  done:
+        if (got_dom_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &dom_pol);
+
         talloc_destroy(mem_ctx);
 
         return seqnum;
diff --git a/source/nsswitch/winbindd_cm.c b/source/nsswitch/winbindd_cm.c
index 205ab4fb310..68a46e36834 100644
--- a/source/nsswitch/winbindd_cm.c
+++ b/source/nsswitch/winbindd_cm.c
@@ -38,8 +38,7 @@
    like that but at the moment it's simply staying as part of winbind.  I
    think the TNG architecture of forcing every user of the rpc layer to use
    the connection caching system is a bad idea.  It should be an optional
-   method of using the routines.  We actually cache policy handles - tng
-   caches connections to pipes.
+   method of using the routines.
 
    The TNG design is quite good but I disagree with some aspects of the
    implementation. -tpot
@@ -55,9 +54,6 @@
      - There needs to be a utility function in libsmb/namequery.c that does
        cm_get_dc_name() 
 
-     - When closing down sam handles we need to close down user, group and
-       domain handles.
-
      - Take care when destroying cli_structs as they can be shared between
        various sam handles.
 
@@ -65,42 +61,6 @@
 
 #include "winbindd.h"
 
-/* We store lists of connections here */
-
-enum sam_pipe_type {
-        SAM_PIPE_BASIC,         /* A basic handle */
-        SAM_PIPE_DOM,           /* A domain handle */
-        SAM_PIPE_USER,          /* A handle on a user */
-        SAM_PIPE_GROUP          /* A handle on a group */
-};
-
-/* Return a string description of a SAM pipe type */
-
-static char *pipe_type(enum sam_pipe_type pt)
-{
-        char *msg;
-
-        switch (pt) {
-        case SAM_PIPE_BASIC:
-                msg = "BASIC";
-                break;
-        case SAM_PIPE_DOM:
-                msg = "DOMAIN";
-                break;
-        case SAM_PIPE_USER:
-                msg = "USER";
-                break;
-        case SAM_PIPE_GROUP:
-                msg = "GROUP";
-                break;
-        default:
-                msg = "??";
-                break;
-        }
-
-        return msg;
-}
-
 /* Global list of connections.  Initially a DLIST but can become a hash
    table or whatever later. */
 
@@ -111,15 +71,6 @@ struct winbindd_cm_conn {
         fstring pipe_name;
         struct cli_state *cli;
         POLICY_HND pol;
-
-        /* Pipe-specific properties for this instance */
-
-        union {
-                struct {
-                        enum sam_pipe_type pipe_type;
-                        uint32 rid;
-                } samr;
-        } pipe_data;
 };
 
 struct winbindd_cm_conn *cm_conns = NULL;
@@ -419,8 +370,7 @@ CLI_POLICY_HND *cm_get_sam_handle(char *domain)
 
         for (conn = cm_conns; conn; conn = conn->next) {
                 if (strequal(conn->domain, domain) &&
-                    strequal(conn->pipe_name, PIPE_SAMR) &&
-                    conn->pipe_data.samr.pipe_type == SAM_PIPE_BASIC) {
+                    strequal(conn->pipe_name, PIPE_SAMR)) {
 
                         if (!connection_ok(conn)) {
                                 DLIST_REMOVE(cm_conns, conn);
@@ -462,6 +412,8 @@ CLI_POLICY_HND *cm_get_sam_handle(char *domain)
         return &hnd;        
 }
 
+#if 0
+
 /* Return a SAM domain policy handle on a domain */
 
 CLI_POLICY_HND *cm_get_sam_dom_handle(char *domain, DOM_SID *domain_sid)
@@ -682,7 +634,10 @@ CLI_POLICY_HND *cm_get_sam_group_handle(char *domain, DOM_SID *domain_sid,
         return &hnd;
 }
 
-/* Get a handle on a netlogon pipe */
+#endif
+
+/* Get a handle on a netlogon pipe.  This is a bit of a hack to re-use the
+   netlogon pipe as no handle is returned. */
 
 struct cli_state *cm_get_netlogon_cli(char *domain, unsigned char *trust_passwd)
 {
@@ -718,7 +673,7 @@ static void dump_conn_list(void)
 {
         struct winbindd_cm_conn *con;
 
-        DEBUG(0, ("\tDomain          Controller      Pipe             Handle type\n"));
+        DEBUG(0, ("\tDomain          Controller      Pipe\n"));
 
         for(con = cm_conns; con; con = con->next) {
                 char *msg;
@@ -728,30 +683,6 @@ static void dump_conn_list(void)
                 asprintf(&msg, "\t%-15s %-15s %-16s", con->domain, 
                          con->controller, con->pipe_name);
                 
-                /* Display sam specific info */
-                
-                if (strequal(con->pipe_name, PIPE_SAMR)) {
-                        char *msg2;
-                        
-                        asprintf(&msg2, "%s %-7s", msg, 
-                                 pipe_type(con->pipe_data.samr.pipe_type));
-
-                        free(msg);
-                        msg = msg2;
-                }
-                
-                if (strequal(con->pipe_name, PIPE_SAMR) &&
-                    (con->pipe_data.samr.pipe_type == SAM_PIPE_USER ||
-                     con->pipe_data.samr.pipe_type == SAM_PIPE_GROUP)) {
-                        char *msg2;
-
-                        asprintf(&msg2, "%s %4xh", msg, 
-                                 con->pipe_data.samr.rid);
-
-                        free(msg);
-                        msg = msg2;
-                }
-
                 DEBUG(0, ("%s\n", msg));
                 free(msg);
         }
diff --git a/source/nsswitch/winbindd_group.c b/source/nsswitch/winbindd_group.c
index 335f1d65147..3bee95b8451 100644
--- a/source/nsswitch/winbindd_group.c
+++ b/source/nsswitch/winbindd_group.c
@@ -473,20 +473,30 @@ static BOOL get_sam_group_entries(struct getent_state *ent)
 		
 	do {
 		struct acct_info *sam_grp_entries = NULL;
+                uint32 des_access = SEC_RIGHTS_MAXIMUM_ALLOWED;
                 CLI_POLICY_HND *hnd;
+                POLICY_HND dom_pol;
 
 		num_entries = 0;
 
-                if (!(hnd = cm_get_sam_dom_handle(ent->domain->name,
-                                                  &ent->domain->sid)))
+                if (!(hnd = cm_get_sam_handle(ent->domain->name)))
+                        break;
+
+                status = cli_samr_open_domain(hnd->cli, mem_ctx,
+                                              &hnd->pol, des_access,
+                                              &ent->domain->sid, &dom_pol);
+
+                if (!NT_STATUS_IS_OK(status))
                         break;
 
                 status = cli_samr_enum_dom_groups(
-                        hnd->cli, mem_ctx, &hnd->pol,
+                        hnd->cli, mem_ctx, &dom_pol,
                         &ent->grp_query_start_ndx,
                         0x8000, /* buffer size? */
                         (struct acct_info **) &sam_grp_entries, &num_entries);
 
+                cli_samr_close(hnd->cli, mem_ctx, &dom_pol);
+
 		/* Copy entries into return buffer */
 
 		if (num_entries) {
diff --git a/source/nsswitch/winbindd_util.c b/source/nsswitch/winbindd_util.c
index 1b63cd28100..6883a86d685 100644
--- a/source/nsswitch/winbindd_util.c
+++ b/source/nsswitch/winbindd_util.c
@@ -315,15 +315,49 @@ BOOL winbindd_lookup_userinfo(struct winbindd_domain *domain,
         CLI_POLICY_HND *hnd;
         uint16 info_level = 0x15;
         NTSTATUS result = NT_STATUS_UNSUCCESSFUL;
+        uint32 des_access = SEC_RIGHTS_MAXIMUM_ALLOWED;
+        POLICY_HND dom_pol, user_pol;
+        BOOL got_dom_pol = False, got_user_pol = False;
 
-        if (!(hnd = cm_get_sam_user_handle(domain->name, &domain->sid, 
-                                           user_rid)))
+        /* Get sam handle */
+
+        if (!(hnd = cm_get_sam_handle(domain->name)))
+                goto done;
+
+        /* Get domain handle */
+
+        result = cli_samr_open_domain(hnd->cli, mem_ctx, &hnd->pol,
+                                      des_access, &domain->sid, &dom_pol);
+
+        if (!NT_STATUS_IS_OK(result))
+                goto done;
+
+        got_dom_pol = True;
+
+        /* Get user handle */
+
+        result = cli_samr_open_user(hnd->cli, mem_ctx, &dom_pol,
+                                    des_access, user_rid, &user_pol);
+
+        if (!NT_STATUS_IS_OK(result))
                 goto done;
 
-        result = cli_samr_query_userinfo(hnd->cli, mem_ctx, &hnd->pol, 
+        /* Get user info */
+
+        result = cli_samr_query_userinfo(hnd->cli, mem_ctx, &user_pol, 
                                          info_level, user_info);
 
+        cli_samr_close(hnd->cli, mem_ctx, &user_pol);
+
  done:
+        /* Clean up policy handles */
+
+        if (got_user_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &user_pol);
+
+        if (got_dom_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &dom_pol);
+
         return NT_STATUS_IS_OK(result);
 }                                   
 
@@ -336,18 +370,52 @@ BOOL winbindd_lookup_usergroups(struct winbindd_domain *domain,
         TALLOC_CTX *mem_ctx;
 	CLI_POLICY_HND *hnd;
 	NTSTATUS result = NT_STATUS_UNSUCCESSFUL;
+        POLICY_HND dom_pol, user_pol;
+        uint32 des_access = SEC_RIGHTS_MAXIMUM_ALLOWED;
+        BOOL got_dom_pol = False, got_user_pol = False;
 
         if (!(mem_ctx = talloc_init()))
                 return False;
 
-        if (!(hnd = cm_get_sam_user_handle(domain->name, &domain->sid,
-                                           user_rid)))
+        /* Get sam handle */
+
+        if (!(hnd = cm_get_sam_handle(domain->name)))
+                goto done;
+
+        /* Get domain handle */
+
+        result = cli_samr_open_domain(hnd->cli, mem_ctx, &hnd->pol,
+                                      des_access, &domain->sid, &dom_pol);
+
+        if (!NT_STATUS_IS_OK(result))
+                goto done;
+
+        got_dom_pol = True;
+
+        /* Get user handle */
+
+        result = cli_samr_open_user(hnd->cli, mem_ctx, &dom_pol,
+                                    des_access, user_rid, &user_pol);
+
+        if (!NT_STATUS_IS_OK(result))
                 goto done;
 
+        got_user_pol = True;
+
+        /* Query user rids */
+
         result = cli_samr_query_usergroups(hnd->cli, mem_ctx, &hnd->pol, 
                                            num_groups, user_groups);
 
  done:
+        /* Clean up policy handles */
+
+        if (got_user_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &user_pol);
+
+        if (got_dom_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &dom_pol);
+
         talloc_destroy(mem_ctx);
 
         return NT_STATUS_IS_OK(result);
@@ -361,21 +429,43 @@ BOOL winbindd_lookup_groupmem(struct winbindd_domain *domain,
                               uint32 **rid_mem, char ***names, 
                               uint32 **name_types)
 {
-        CLI_POLICY_HND *group_hnd, *dom_hnd;
+        CLI_POLICY_HND *hnd;
         NTSTATUS result = NT_STATUS_UNSUCCESSFUL;
         uint32 i, total_names = 0;
+        POLICY_HND dom_pol, group_pol;
+        uint32 des_access = SEC_RIGHTS_MAXIMUM_ALLOWED;
+        BOOL got_dom_pol = False, got_group_pol = False;
 
-        /* Step #1: Get a list of user rids that are the members of the
-           group. */
+        /* Get sam handle */
+
+        if (!(hnd = cm_get_sam_handle(domain->name)))
+                goto done;
+
+        /* Get domain handle */
 
-        if (!(group_hnd = cm_get_sam_group_handle(domain->name, &domain->sid,
-                                                  group_rid)))
+        result = cli_samr_open_domain(hnd->cli, mem_ctx, &hnd->pol,
+                                      des_access, &domain->sid, &dom_pol);
+
+        if (!NT_STATUS_IS_OK(result))
+                goto done;
+
+        got_dom_pol = True;
+
+        /* Get group handle */
+
+        result = cli_samr_open_group(hnd->cli, mem_ctx, &dom_pol,
+                                     des_access, group_rid, &group_pol);
+
+        if (!NT_STATUS_IS_OK(result))
                 goto done;
 
-        /* Get group membership.  This is a list of rids. */
+        got_group_pol = True;
 
-        result = cli_samr_query_groupmem(group_hnd->cli, mem_ctx,
-                                         &group_hnd->pol, num_names, rid_mem,
+        /* Step #1: Get a list of user rids that are the members of the
+           group. */
+
+        result = cli_samr_query_groupmem(hnd->cli, mem_ctx,
+                                         &group_pol, num_names, rid_mem,
                                          name_types);
 
         if (!NT_STATUS_IS_OK(result))
@@ -386,9 +476,6 @@ BOOL winbindd_lookup_groupmem(struct winbindd_domain *domain,
            is a buffer overflow or something like that lurking around
            somewhere. */
 
-        if (!(dom_hnd = cm_get_sam_dom_handle(domain->name, &domain->sid)))
-                goto done;
-
 #define MAX_LOOKUP_RIDS 900
 
         *names = talloc(mem_ctx, *num_names * sizeof(char *));
@@ -402,8 +489,8 @@ BOOL winbindd_lookup_groupmem(struct winbindd_domain *domain,
 
                 /* Lookup a chunk of rids */
 
-                result = cli_samr_lookup_rids(dom_hnd->cli, mem_ctx,
-                                              &dom_hnd->pol, 1000, /* flags */
+                result = cli_samr_lookup_rids(hnd->cli, mem_ctx,
+                                              &dom_pol, 1000, /* flags */
                                               num_lookup_rids,
                                               &(*rid_mem)[i],
                                               &tmp_num_names,
@@ -427,6 +514,12 @@ BOOL winbindd_lookup_groupmem(struct winbindd_domain *domain,
         *num_names = total_names;
 
  done:
+        if (got_group_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &group_pol);
+
+        if (got_dom_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &dom_pol);
+
         return NT_STATUS_IS_OK(result);
 }
 
@@ -571,15 +664,35 @@ NTSTATUS winbindd_query_dispinfo(struct winbindd_domain *domain,
 {
         CLI_POLICY_HND *hnd;
         NTSTATUS result = NT_STATUS_UNSUCCESSFUL;
+        POLICY_HND dom_pol;
+        BOOL got_dom_pol = False;
+        uint32 des_access = SEC_RIGHTS_MAXIMUM_ALLOWED;
+
+        /* Get sam handle */
 
-        if (!(hnd = cm_get_sam_dom_handle(domain->name, &domain->sid)))
+        if (!(hnd = cm_get_sam_handle(domain->name)))
                 goto done;
 
+        /* Get domain handle */
+
+        result = cli_samr_open_domain(hnd->cli, mem_ctx, &hnd->pol,
+                                      des_access, &domain->sid, &dom_pol);
+
+        if (!NT_STATUS_IS_OK(result))
+                goto done;
+
+        got_dom_pol = True;
+
+        /* Query display info */
+
         result = cli_samr_query_dispinfo(hnd->cli, mem_ctx,
-                                         &hnd->pol, start_ndx, info_level,
+                                         &dom_pol, start_ndx, info_level,
                                          num_entries, 0xffff, ctr);
 
  done:
+        if (got_dom_pol)
+                cli_samr_close(hnd->cli, mem_ctx, &dom_pol);
+
         return result;
 }