CVE-2015-5370: s4:rpc_server: the assoc_group is relative to the connection (association)

All presentation contexts of a connection use the same association group.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11344

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Günther Deschner <gd@samba.org>

4 files changed, 29 insertions, 67 deletions

diff --git a/source4/rpc_server/dcerpc_server.c b/source4/rpc_server/dcerpc_server.c
index ad3b02fe845..46d16b417d0 100644
--- a/source4/rpc_server/dcerpc_server.c
+++ b/source4/rpc_server/dcerpc_server.c
@@ -42,9 +42,6 @@
 #include "lib/util/samba_modules.h"
 #include "librpc/gen_ndr/ndr_dcerpc.h"
 
-/* this is only used when the client asks for an unknown interface */
-#define DUMMY_ASSOC_GROUP 0x0FFFFFFF
-
 extern const struct dcesrv_interface dcesrv_mgmt_interface;
 
 
@@ -74,7 +71,7 @@ static struct dcesrv_assoc_group *dcesrv_assoc_group_reference(TALLOC_CTX *mem_c
 
 	assoc_group = dcesrv_assoc_group_find(dce_ctx, id);
 	if (assoc_group == NULL) {
-		DEBUG(0,(__location__ ": Failed to find assoc_group 0x%08x\n", id));
+		DEBUG(2,(__location__ ": Failed to find assoc_group 0x%08x\n", id));
 		return NULL;
 	}
 	return talloc_reference(mem_ctx, assoc_group);
@@ -714,10 +711,16 @@ static NTSTATUS dcesrv_bind(struct dcesrv_call_state *call)
 	/*
 	  if provided, check the assoc_group is valid
 	 */
-	if (call->pkt.u.bind.assoc_group_id != 0 &&
-	    lpcfg_parm_bool(call->conn->dce_ctx->lp_ctx, NULL, "dcesrv","assoc group checking", true) &&
-	    dcesrv_assoc_group_find(call->conn->dce_ctx, call->pkt.u.bind.assoc_group_id) == NULL) {
-		return dcesrv_bind_nak(call, 0);	
+	if (call->pkt.u.bind.assoc_group_id != 0) {
+		call->conn->assoc_group = dcesrv_assoc_group_reference(call->conn,
+								       call->conn->dce_ctx,
+								       call->pkt.u.bind.assoc_group_id);
+	} else {
+		call->conn->assoc_group = dcesrv_assoc_group_new(call->conn,
+								 call->conn->dce_ctx);
+	}
+	if (call->conn->assoc_group == NULL) {
+		return dcesrv_bind_nak(call, 0);
 	}
 
 	if (call->pkt.u.bind.num_contexts < 1 ||
@@ -761,17 +764,8 @@ static NTSTATUS dcesrv_bind(struct dcesrv_call_state *call)
 		context->conn = call->conn;
 		context->iface = iface;
 		context->context_id = context_id;
-		if (call->pkt.u.bind.assoc_group_id != 0) {
-			context->assoc_group = dcesrv_assoc_group_reference(context,
-									    call->conn->dce_ctx, 
-									    call->pkt.u.bind.assoc_group_id);
-		} else {
-			context->assoc_group = dcesrv_assoc_group_new(context, call->conn->dce_ctx);
-		}
-		if (context->assoc_group == NULL) {
-			talloc_free(context);
-			return dcesrv_bind_nak(call, 0);
-		}
+		/* legacy for openchange dcesrv_mapiproxy.c */
+		context->assoc_group = call->conn->assoc_group;
 		context->private_data = NULL;
 		DLIST_ADD(call->conn->contexts, context);
 		call->context = context;
@@ -829,18 +823,7 @@ static NTSTATUS dcesrv_bind(struct dcesrv_call_state *call)
 	pkt.pfc_flags = DCERPC_PFC_FLAG_FIRST | DCERPC_PFC_FLAG_LAST | extra_flags;
 	pkt.u.bind_ack.max_xmit_frag = call->conn->max_xmit_frag;
 	pkt.u.bind_ack.max_recv_frag = call->conn->max_recv_frag;
-
-	/*
-	  make it possible for iface->bind() to specify the assoc_group_id
-	  This helps the openchange mapiproxy plugin to work correctly.
-	  
-	  metze
-	*/
-	if (call->context) {
-		pkt.u.bind_ack.assoc_group_id = call->context->assoc_group->id;
-	} else {
-		pkt.u.bind_ack.assoc_group_id = DUMMY_ASSOC_GROUP;
-	}
+	pkt.u.bind_ack.assoc_group_id = call->conn->assoc_group->id;
 
 	if (iface) {
 		endpoint = dcerpc_binding_get_string_option(
@@ -1000,18 +983,8 @@ static NTSTATUS dcesrv_alter_new_context(struct dcesrv_call_state *call, uint32_
 	context->conn = call->conn;
 	context->iface = iface;
 	context->context_id = context_id;
-	if (call->pkt.u.alter.assoc_group_id != 0) {
-		context->assoc_group = dcesrv_assoc_group_reference(context,
-								    call->conn->dce_ctx, 
-								    call->pkt.u.alter.assoc_group_id);
-	} else {
-		context->assoc_group = dcesrv_assoc_group_new(context, call->conn->dce_ctx);
-	}
-	if (context->assoc_group == NULL) {
-		talloc_free(context);
-		call->context = NULL;
-		return NT_STATUS_NO_MEMORY;
-	}
+	/* legacy for openchange dcesrv_mapiproxy.c */
+	context->assoc_group = call->conn->assoc_group;
 	context->private_data = NULL;
 	DLIST_ADD(call->conn->contexts, context);
 	call->context = context;
@@ -1059,11 +1032,7 @@ static NTSTATUS dcesrv_alter_resp(struct dcesrv_call_state *call,
 	pkt.pfc_flags = DCERPC_PFC_FLAG_FIRST | DCERPC_PFC_FLAG_LAST | extra_flags;
 	pkt.u.alter_resp.max_xmit_frag = call->conn->max_xmit_frag;
 	pkt.u.alter_resp.max_recv_frag = call->conn->max_recv_frag;
-	if (result == 0) {
-		pkt.u.alter_resp.assoc_group_id = call->context->assoc_group->id;
-	} else {
-		pkt.u.alter_resp.assoc_group_id = 0;
-	}
+	pkt.u.alter_resp.assoc_group_id = call->conn->assoc_group->id;
 	pkt.u.alter_resp.num_results = 1;
 	pkt.u.alter_resp.ctx_list = talloc_zero(call, struct dcerpc_ack_ctx);
 	if (!pkt.u.alter_resp.ctx_list) {
@@ -1181,17 +1150,6 @@ static NTSTATUS dcesrv_alter(struct dcesrv_call_state *call)
 		}
 	}
 
-	if (call->pkt.u.alter.assoc_group_id != 0 &&
-	    lpcfg_parm_bool(call->conn->dce_ctx->lp_ctx, NULL, "dcesrv","assoc group checking", true) &&
-	    call->pkt.u.alter.assoc_group_id != call->context->assoc_group->id) {
-		DEBUG(0,(__location__ ": Failed attempt to use new assoc_group in alter context (0x%08x 0x%08x)\n",
-			 call->context->assoc_group->id, call->pkt.u.alter.assoc_group_id));
-		/* TODO: can they ask for a new association group? */
-		return dcesrv_alter_resp(call,
-				DCERPC_BIND_PROVIDER_REJECT,
-				DCERPC_BIND_REASON_ASYNTAX);
-	}
-
 	/* handle any authentication that is being requested */
 	if (!auth_ok) {
 		if (call->in_auth_info.auth_type !=
diff --git a/source4/rpc_server/dcerpc_server.h b/source4/rpc_server/dcerpc_server.h
index cb600cd3a81..aead405edd1 100644
--- a/source4/rpc_server/dcerpc_server.h
+++ b/source4/rpc_server/dcerpc_server.h
@@ -175,6 +175,7 @@ struct dcesrv_connection_context {
 	struct dcesrv_connection_context *next, *prev;
 	uint32_t context_id;
 
+	/* TODO: remove this legacy (for openchange) in master */
 	struct dcesrv_assoc_group *assoc_group;
 
 	/* the connection this is on */
@@ -274,6 +275,9 @@ struct dcesrv_connection {
 	bool allow_auth3;
 	bool allow_alter;
 	bool allow_request;
+
+	/* the association group the connection belongs to */
+	struct dcesrv_assoc_group *assoc_group;
 };
 
 
diff --git a/source4/rpc_server/handles.c b/source4/rpc_server/handles.c
index f99ee1d2e8f..820da49c02d 100644
--- a/source4/rpc_server/handles.c
+++ b/source4/rpc_server/handles.c
@@ -46,7 +46,7 @@ _PUBLIC_ struct dcesrv_handle *dcesrv_handle_new(struct dcesrv_connection_contex
 
 	sid = &context->conn->auth_state.session_info->security_token->sids[PRIMARY_USER_SID_INDEX];
 
-	h = talloc_zero(context->assoc_group, struct dcesrv_handle);
+	h = talloc_zero(context->conn->assoc_group, struct dcesrv_handle);
 	if (!h) {
 		return NULL;
 	}
@@ -56,12 +56,12 @@ _PUBLIC_ struct dcesrv_handle *dcesrv_handle_new(struct dcesrv_connection_contex
 		talloc_free(h);
 		return NULL;
 	}
-	h->assoc_group = context->assoc_group;
+	h->assoc_group = context->conn->assoc_group;
 	h->iface = context->iface;
 	h->wire_handle.handle_type = handle_type;
 	h->wire_handle.uuid = GUID_random();
 	
-	DLIST_ADD(context->assoc_group->handles, h);
+	DLIST_ADD(context->conn->assoc_group->handles, h);
 
 	talloc_set_destructor(h, dcesrv_handle_destructor);
 
@@ -87,7 +87,7 @@ _PUBLIC_ struct dcesrv_handle *dcesrv_handle_fetch(
 		return dcesrv_handle_new(context, handle_type);
 	}
 
-	for (h=context->assoc_group->handles; h; h=h->next) {
+	for (h=context->conn->assoc_group->handles; h; h=h->next) {
 		if (h->wire_handle.handle_type == p->handle_type &&
 		    GUID_equal(&p->uuid, &h->wire_handle.uuid)) {
 			if (handle_type != DCESRV_HANDLE_ANY &&
diff --git a/source4/rpc_server/remote/dcesrv_remote.c b/source4/rpc_server/remote/dcesrv_remote.c
index be4bd1253e0..3eb0ad4b4a1 100644
--- a/source4/rpc_server/remote/dcesrv_remote.c
+++ b/source4/rpc_server/remote/dcesrv_remote.c
@@ -117,9 +117,9 @@ static NTSTATUS remote_op_bind(struct dcesrv_call_state *dce_call, const struct
 	}
 
 	/* If we already have a remote association group ID, then use that */
-	if (dce_call->context->assoc_group->proxied_id != 0) {
+	if (dce_call->conn->assoc_group->proxied_id != 0) {
 		status = dcerpc_binding_set_assoc_group_id(b,
-			dce_call->context->assoc_group->proxied_id);
+			dce_call->conn->assoc_group->proxied_id);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0, ("dcerpc_binding_set_assoc_group_id() - %s'\n",
 				  nt_errstr(status)));
@@ -148,8 +148,8 @@ static NTSTATUS remote_op_bind(struct dcesrv_call_state *dce_call, const struct
 		return status;
 	}
 
-	if (dce_call->context->assoc_group->proxied_id == 0) {
-		dce_call->context->assoc_group->proxied_id =
+	if (dce_call->conn->assoc_group->proxied_id == 0) {
+		dce_call->conn->assoc_group->proxied_id =
 			dcerpc_binding_get_assoc_group_id(priv->c_pipe->binding);
 	}