[SCM] Samba Shared Repository - branch master updated

Michael Adam obnox at samba.org
Mon Jul 29 06:43:02 MDT 2013


The branch, master has been updated
       via  8f8e843 s3:winbind: add a warning DEBUG message when skipping a sid from the mapped GID list
       via  482212e s3:winbind: change getgroups to only do one sids2xids call instead of many
       via  6e41745 s3:winbind: fix the getgroups implementation to include the user sid's GID in case of ID_TYPE_BOTH
       via  f62219e s3:winbind: fix gid counting and error handling in the getgroups implementation
      from  45f5ea0 dns: Update TODO list

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 8f8e843267636b5fea076014980031afc2c0a7b4
Author: Michael Adam <obnox at samba.org>
Date:   Fri Jul 26 12:26:30 2013 +0200

    s3:winbind: add a warning DEBUG message when skipping a sid from the mapped GID list
    
    This presents a potential security problem when ACLs contain DENY ACEs.
    
    Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
    
    Signed-off-by: Michael Adam <obnox at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Michael Adam <obnox at samba.org>
    Autobuild-Date(master): Mon Jul 29 14:42:27 CEST 2013 on sn-devel-104

commit 482212e3d348e4247759cbca9507db74f61f9703
Author: Michael Adam <obnox at samba.org>
Date:   Fri Jul 26 12:25:27 2013 +0200

    s3:winbind: change getgroups to only do one sids2xids call instead of many
    
    Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
    
    Signed-off-by: Michael Adam <obnox at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

commit 6e41745173989dff1b4e2f03e174e9d1020857d5
Author: Michael Adam <obnox at samba.org>
Date:   Fri Jul 26 11:32:34 2013 +0200

    s3:winbind: fix the getgroups implementation to include the user sid's GID in case of ID_TYPE_BOTH
    
    This is important for acl checks on the unix level where only a group ace
    has been added to the ACL for the user sid, e.g. when accessing Files with
    nfs or local unix processes.
    
    Signed-off-by: Michael Adam <obnox at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit f62219e71af69ec8b331500b75fd5fd77d51a636
Author: Michael Adam <obnox at samba.org>
Date:   Fri Jul 26 11:31:41 2013 +0200

    s3:winbind: fix gid counting and error handling in the getgroups implementation
    
    Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
    
    Signed-off-by: Michael Adam <obnox at samba.org>
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

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

Summary of changes:
 source3/winbindd/winbindd_getgroups.c |  102 +++++++++++++++++++++++----------
 1 files changed, 71 insertions(+), 31 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/winbindd/winbindd_getgroups.c b/source3/winbindd/winbindd_getgroups.c
index 1774901..b899beb 100644
--- a/source3/winbindd/winbindd_getgroups.c
+++ b/source3/winbindd/winbindd_getgroups.c
@@ -29,7 +29,6 @@ struct winbindd_getgroups_state {
 	enum lsa_SidType type;
 	int num_sids;
 	struct dom_sid *sids;
-	int next_sid;
 	int num_gids;
 	gid_t *gids;
 };
@@ -124,18 +123,13 @@ static void winbindd_getgroups_gettoken_done(struct tevent_req *subreq)
 
 	/*
 	 * Convert the group SIDs to gids. state->sids[0] contains the user
-	 * sid, so start at index 1.
+	 * sid. If the idmap backend uses ID_TYPE_BOTH, we might need the
+	 * the id of the user sid in the list of group sids, so map the
+	 * complete token.
 	 */
 
-	state->gids = talloc_array(state, gid_t, state->num_sids-1);
-	if (tevent_req_nomem(state->gids, req)) {
-		return;
-	}
-	state->num_gids = 0;
-	state->next_sid = 1;
-
 	subreq = wb_sids2xids_send(state, state->ev,
-				   &state->sids[state->next_sid], 1);
+				   state->sids, state->num_sids);
 	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
@@ -149,38 +143,84 @@ static void winbindd_getgroups_sid2gid_done(struct tevent_req *subreq)
 	struct winbindd_getgroups_state *state = tevent_req_data(
 		req, struct winbindd_getgroups_state);
 	NTSTATUS status;
-	struct unixid xid;
+	struct unixid *xids;
+	int i;
 
-	xid.type = ID_TYPE_NOT_SPECIFIED;
-	xid.id = UINT32_MAX;
+	xids = talloc_array(state, struct unixid, state->num_sids);
+	if (tevent_req_nomem(xids, req)) {
+		return;
+	}
+	for (i=0; i < state->num_sids; i++) {
+		xids[i].type = ID_TYPE_NOT_SPECIFIED;
+		xids[i].id = UINT32_MAX;
+	}
 
-	status = wb_sids2xids_recv(subreq, &xid);
+	status = wb_sids2xids_recv(subreq, xids);
 	TALLOC_FREE(subreq);
-	if (xid.type == ID_TYPE_GID || xid.type == ID_TYPE_BOTH) {
-		state->gids[state->num_gids] = (gid_t)xid.id;
-	} else {
-		state->gids[state->num_gids] = (uid_t)-1;
+	if (NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED) ||
+	    NT_STATUS_EQUAL(status, STATUS_SOME_UNMAPPED))
+	{
+		status = NT_STATUS_OK;
 	}
-
-	/*
-	 * In case of failure, just continue with the next gid
-	 */
-	if (NT_STATUS_IS_OK(status)) {
-		state->num_gids += 1;
+	if (tevent_req_nterror(req, status)) {
+		return;
 	}
-	state->next_sid += 1;
 
-	if (state->next_sid >= state->num_sids) {
-		tevent_req_done(req);
+	state->gids = talloc_array(state, gid_t, state->num_sids);
+	if (tevent_req_nomem(state->gids, req)) {
 		return;
 	}
+	state->num_gids = 0;
 
-	subreq = wb_sids2xids_send(state, state->ev,
-				   &state->sids[state->next_sid], 1);
-	if (tevent_req_nomem(subreq, req)) {
+	for (i=0; i < state->num_sids; i++) {
+		bool include_gid = false;
+		const char *debug_missing = NULL;
+
+		switch (xids[i].type) {
+		case ID_TYPE_NOT_SPECIFIED:
+			debug_missing = "not specified";
+			break;
+		case ID_TYPE_UID:
+			if (i != 0) {
+				debug_missing = "uid";
+			}
+			break;
+		case ID_TYPE_GID:
+		case ID_TYPE_BOTH:
+			include_gid = true;
+			break;
+		}
+
+		if (!include_gid) {
+			if (debug_missing == NULL) {
+				continue;
+			}
+
+			DEBUG(10, ("WARNING: skipping unix id (%u) for sid %s "
+				   "from group list because the idmap type "
+				   "is %s. "
+				   "This might be a security problem when ACLs "
+				   "contain DENY ACEs!\n",
+				   (unsigned)xids[i].id,
+				   sid_string_tos(&state->sids[i]),
+				   debug_missing));
+			continue;
+		}
+
+		state->gids[state->num_gids] = (gid_t)xids[i].id;
+		state->num_gids += 1;
+	}
+
+	/*
+	 * This should not fail, as it does not do any reallocation,
+	 * just updating the talloc size.
+	 */
+	state->gids = talloc_realloc(state, state->gids, gid_t, state->num_gids);
+	if (tevent_req_nomem(state->gids, req)) {
 		return;
 	}
-	tevent_req_set_callback(subreq, winbindd_getgroups_sid2gid_done, req);
+
+	tevent_req_done(req);
 }
 
 NTSTATUS winbindd_getgroups_recv(struct tevent_req *req,


-- 
Samba Shared Repository


More information about the samba-cvs mailing list