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>

diff --git a/source4/rpc_server/dcerpc_server.c b/source4/rpc_server/dcerpc_server.c
index ad3b02fe845db0ca0f67c34eecc1c64c88f0fe6b..46d16b417d02e59cf40470f9462eb3e137f7ae49 100644 (file)
--- 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 cb600cd3a81d71983cc9bf4c42ed3a64c3644f2f..aead405edd1e40d510bc92b2619a64629ef841f0 100644 (file)
--- 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 f99ee1d2e8ff859b3099a93d6d404f2638dfb226..820da49c02d2d895caebed1829b3cefdeac70e24 100644 (file)
--- 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 be4bd1253e0a1ab57acee6f256a35603c30fc0ab..3eb0ad4b4a1cb5a27450491250e957d6382308cc 100644 (file)
--- 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);
        }