s3: rpc_server: Store new association groups in the id tree
authorSamuel Cabrero <scabrero@samba.org>
Fri, 26 Jun 2020 15:20:32 +0000 (17:20 +0200)
committerAndrew Bartlett <abartlet@samba.org>
Wed, 7 Apr 2021 09:18:30 +0000 (09:18 +0000)
Right now a new association group is created for each connection
assigning the legacy 0x53F0 id, but it is not stored anywhere. When a
second client request to join an association group by its id it is not
found and a new one is created with the same ID.

In practise, it means the association groups are not working even in the
same server process.

This commit stores the created association group in the idtree, but to
make use of it assigns a random id instead of the historical 0x53F0.

The test assoc_group_ok2 was wrongly passing before this change because
the same id 0x53F0 was assigned to all association groups.

Signed-off-by: Samuel Cabrero <scabrero@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
selftest/knownfail
selftest/knownfail.d/s3-rpc-assoc-groups [deleted file]
source3/rpc_server/rpc_server.c

index f4035a6dec2b6282f3dac1765044856ea0de3c28..0be542c5c1eb3f1160115d6d972ec29e17c0cf6f 100644 (file)
 ^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_no_auth_presentation_ctx_invalid4
 ^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_spnego_change_auth_type2
 ^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_spnego_change_transfer
-# Association groups not implemented yet in s3 server implementation
+# Association groups between processes not implemented yet in s3 server implementation
+^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_assoc_group_ok2\(ad_member\)
 ^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_assoc_group_fail1\(ad_member\)
 ^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_assoc_group_fail2\(ad_member\)
 ^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_assoc_group_fail3\(ad_member\)
-^samba.tests.dcerpc.raw_protocol.*.TestDCERPC_BIND.test_assoc_group_diff1\(ad_member\)
 ^samba4.rpc.echo.*on.*with.object.echo.doublepointer.*nt4_dc
 ^samba4.rpc.echo.*on.*with.object.echo.surrounding.*nt4_dc
 ^samba4.rpc.echo.*on.*with.object.echo.enum.*nt4_dc
diff --git a/selftest/knownfail.d/s3-rpc-assoc-groups b/selftest/knownfail.d/s3-rpc-assoc-groups
deleted file mode 100644 (file)
index 29b1b34..0000000
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.rpc.bind.assoc_group_handles_external\(nt4_dc\)
index 35cc1de90819ed3e3dd3d61abcafce512123d23e..b96bd90daf0b6a3b523ef246aa1a93fff6bf3670 100644 (file)
@@ -612,8 +612,19 @@ void dcesrv_log_successful_authz(
        TALLOC_FREE(frame);
 }
 
-static NTSTATUS dcesrv_assoc_group_new(struct dcesrv_call_state *call,
-                                      uint32_t assoc_group_id)
+static int dcesrv_assoc_group_destructor(struct dcesrv_assoc_group *assoc_group)
+{
+       int ret;
+       ret = idr_remove(assoc_group->dce_ctx->assoc_groups_idr,
+                        assoc_group->id);
+       if (ret != 0) {
+               DBG_ERR("Failed to remove assoc_group 0x%08x\n",
+                       assoc_group->id);
+       }
+       return 0;
+}
+
+static NTSTATUS dcesrv_assoc_group_new(struct dcesrv_call_state *call)
 {
        struct dcesrv_connection *conn = call->conn;
        struct dcesrv_context *dce_ctx = conn->dce_ctx;
@@ -621,18 +632,30 @@ static NTSTATUS dcesrv_assoc_group_new(struct dcesrv_call_state *call,
        enum dcerpc_transport_t transport =
                dcerpc_binding_get_transport(endpoint->ep_description);
        struct dcesrv_assoc_group *assoc_group = NULL;
+       int id;
 
        assoc_group = talloc_zero(conn, struct dcesrv_assoc_group);
        if (assoc_group == NULL) {
                return NT_STATUS_NO_MEMORY;
        }
 
+       id = idr_get_new_random(dce_ctx->assoc_groups_idr,
+                               assoc_group,
+                               UINT16_MAX);
+       if (id == -1) {
+               TALLOC_FREE(assoc_group);
+               DBG_ERR("Out of association groups!\n");
+               return NT_STATUS_RPC_OUT_OF_RESOURCES;
+       }
+
        assoc_group->transport = transport;
-       assoc_group->id = assoc_group_id;
+       assoc_group->id = id;
        assoc_group->dce_ctx = dce_ctx;
 
        call->conn->assoc_group = assoc_group;
 
+       talloc_set_destructor(assoc_group, dcesrv_assoc_group_destructor);
+
        return NT_STATUS_OK;
 }
 
@@ -658,7 +681,7 @@ static NTSTATUS dcesrv_assoc_group_reference(struct dcesrv_call_state *call,
                DBG_NOTICE("Failed to find assoc_group 0x%08x in this "
                           "server process, creating a new one\n",
                           assoc_group_id);
-               return dcesrv_assoc_group_new(call, assoc_group_id);
+               return dcesrv_assoc_group_new(call);
        }
        assoc_group = talloc_get_type_abort(id_ptr, struct dcesrv_assoc_group);
 
@@ -691,7 +714,7 @@ NTSTATUS dcesrv_assoc_group_find(
        }
 
        /* If not requested by client create a new association group */
-       return dcesrv_assoc_group_new(call, 0x53F0);
+       return dcesrv_assoc_group_new(call);
 }
 
 void dcesrv_transport_terminate_connection(struct dcesrv_connection *dce_conn,