The branch, v3-6-test has been updated
       via  df0a827 s3:wb_lookupsids: add some paranoia checks to 
wb_lookupsids_recv()
       via  e26fb59 s3:wb_lookupsids: don't ignore 'result' and check if we got 
useable values
       via  1269dec Revert "s3-winbind: Fix paranoia checks in winbindd_samr.c."
      from  df22e63 s3:utils/net_*registry: use c99 initializers which are 
supported by old gcc 2.95 compilers (bug #8226)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test


- Log -----------------------------------------------------------------
commit df0a827e74096b295c7624278ca5f0b7b7e8d6e5
Author: Stefan Metzmacher <me...@samba.org>
Date:   Thu Jun 16 18:25:15 2011 +0200

    s3:wb_lookupsids: add some paranoia checks to wb_lookupsids_recv()
    
    This hopefully catches future bugs.
    
    metze
    
    Autobuild-User: Stefan Metzmacher <me...@samba.org>
    Autobuild-Date: Thu Jun 16 19:50:16 CEST 2011 on sn-devel-104
    (cherry picked from commit 5961852d9c0e5cf64cea988586d610af9d63d487)

commit e26fb591060b10880b25f6b0a4437f9c9052dab4
Author: Stefan Metzmacher <me...@samba.org>
Date:   Thu Jun 16 18:16:15 2011 +0200

    s3:wb_lookupsids: don't ignore 'result' and check if we got useable values
    
    The wrong fix for bug #8215 discovered this bug, as it caused
    sam_rids_to_names() to always return NT_STATUS_NONE_MAPPED.
    
    metze
    (cherry picked from commit 85809ccbe3a79f307af1fdd227f33b899d8db1b4)

commit 1269dec1b0121fcbf6dda36a385a4a510232124e
Author: Stefan Metzmacher <me...@samba.org>
Date:   Thu Jun 16 18:40:04 2011 +0200

    Revert "s3-winbind: Fix paranoia checks in winbindd_samr.c."
    
    This reverts commit 207a84d725b905c2b119d2ef0f4f4d4eb391140d.
    
    This is the wrong fix for the problem, see bug #8215.
    
    metze
    (cherry picked from commit 283f8a7fb5089a7126f07e26315fd06ab59997d8)

-----------------------------------------------------------------------

Summary of changes:
 source3/winbindd/wb_lookupsids.c |   70 +++++++++++++++++++++++++++++++++++--
 source3/winbindd/winbindd_samr.c |    4 +-
 2 files changed, 68 insertions(+), 6 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
index 05601ad..bf2ddb3 100644
--- a/source3/winbindd/wb_lookupsids.c
+++ b/source3/winbindd/wb_lookupsids.c
@@ -428,6 +428,7 @@ static void wb_lookupsids_done(struct tevent_req *subreq)
                req, struct wb_lookupsids_state);
        struct wb_lookupsids_domain *d;
        uint32_t i;
+       bool fallback = false;
 
        NTSTATUS status, result;
 
@@ -437,13 +438,31 @@ static void wb_lookupsids_done(struct tevent_req *subreq)
                return;
        }
 
+       d = &state->domains[state->domains_done];
+
+       if (NT_STATUS_IS_ERR(result)) {
+               fallback = true;
+       } else if (state->tmp_names.count != d->sids.num_sids) {
+               fallback = true;
+       }
+
+       if (fallback) {
+               for (i=0; i < d->sids.num_sids; i++) {
+                       uint32_t res_sid_index = d->sid_indexes[i];
+
+                       state->single_sids[state->num_single_sids] =
+                               res_sid_index;
+                       state->num_single_sids += 1;
+               }
+               state->domains_done += 1;
+               wb_lookupsids_next(req, state);
+               return;
+       }
+
        /*
-        * Ignore "result" here. We depend on the individual states in
-        * the translated names.
+        * Look at the individual states in the translated names.
         */
 
-       d = &state->domains[state->domains_done];
-
        for (i=0; i<state->tmp_names.count; i++) {
 
                uint32_t res_sid_index = d->sid_indexes[i];
@@ -544,6 +563,7 @@ static void wb_lookupsids_lookuprids_done(struct tevent_req 
*subreq)
        NTSTATUS status, result;
        struct wb_lookupsids_domain *d;
        uint32_t i;
+       bool fallback = false;
 
        status = dcerpc_wbint_LookupRids_recv(subreq, state, &result);
        TALLOC_FREE(subreq);
@@ -552,6 +572,30 @@ static void wb_lookupsids_lookuprids_done(struct 
tevent_req *subreq)
        }
 
        d = &state->domains[state->domains_done];
+
+       if (NT_STATUS_IS_ERR(result)) {
+               fallback = true;
+       } else if (state->rid_names.num_principals != d->sids.num_sids) {
+               fallback = true;
+       }
+
+       if (fallback) {
+               for (i=0; i < d->sids.num_sids; i++) {
+                       uint32_t res_sid_index = d->sid_indexes[i];
+
+                       state->single_sids[state->num_single_sids] =
+                               res_sid_index;
+                       state->num_single_sids += 1;
+               }
+               state->domains_done += 1;
+               wb_lookupsids_next(req, state);
+               return;
+       }
+
+       /*
+        * Look at the individual states in the translated names.
+        */
+
        sid_copy(&src_domain_sid, get_global_sam_sid());
        src_domain.name.string = get_global_sam_name();
        src_domain.sid = &src_domain_sid;
@@ -595,6 +639,24 @@ NTSTATUS wb_lookupsids_recv(struct tevent_req *req, 
TALLOC_CTX *mem_ctx,
        if (tevent_req_is_nterror(req, &status)) {
                return status;
        }
+
+       /*
+        * The returned names need to match the given sids,
+        * if not we have a bug in the code!
+        *
+        */
+       SMB_ASSERT(state->res_names->count == state->num_sids);
+
+       /*
+        * Not strictly needed, but it might make debugging in the callers
+        * easier in future, if the talloc_array_length() returns the
+        * expected result...
+        */
+       state->res_domains->domains = talloc_realloc(state->res_domains,
+                                                    
state->res_domains->domains,
+                                                    struct lsa_DomainInfo,
+                                                    state->res_domains->count);
+
        *domains = talloc_move(mem_ctx, &state->res_domains);
        *names = talloc_move(mem_ctx, &state->res_names);
        return NT_STATUS_OK;
diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index 98997b6..ee5ff08 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -762,8 +762,8 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain 
*domain,
        ZERO_STRUCT(lsa_policy);
 
        /* Paranoia check */
-       if (!sid_check_is_in_builtin(domain_sid) &&
-           !sid_check_is_in_our_domain(domain_sid) &&
+       if (!sid_check_is_builtin(domain_sid) &&
+           !sid_check_is_domain(domain_sid) &&
            !sid_check_is_unix_users(domain_sid) &&
            !sid_check_is_unix_groups(domain_sid) &&
            !sid_check_is_in_wellknown_domain(domain_sid)) {


-- 
Samba Shared Repository

Reply via email to