Attachment 17535 Details for Bug 14821 – Patch for Samba 4.12

[patch] Patch for Samba 4.12

v4-12-cpu-usage.patch (text/plain), 25.89 KB, created by Andrew Bartlett on 2022-09-29 00:38:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2022-09-29 00:38:16 UTC

Size: 25.89 KB

patch

obsolete

>From b898f6a6bb145e246765fbfce685b73a68117656 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 25 Aug 2021 09:41:11 +1200
>Subject: [PATCH 1/3] dsdb: Be careful to avoid use of the expensive
> talloc_is_parent()
>
>The wrong talloc API was selected while addressing a memory leak.
>
>commit ee2fe56ba0ef6626b634376e8dc2185aa89f8c99
>Author: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date:   Tue Nov 27 11:07:44 2018 +1300
>
>    drepl: memory leak fix
>
>    Fixes a memory leak where schema reference attached to ldb
>    instance is lost before it can be freed.
>
>    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14042
>
>    Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>
>    Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>    Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>
>    Autobuild-User(master): Garming Sam <garming@samba.org>
>    Autobuild-Date(master): Wed Jul 17 06:17:10 UTC 2019 on sn-devel-184
>
>By using talloc_get_parent() walking the entire talloc tree is
>avoided.
>
>RN: Address a signifcant performance regression in database access in the AD DC since Samba 4.12
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14806
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 8affe4a1e625104de4ca024fdc3e9cd96498aff3)
>---
> source4/dsdb/schema/schema_set.c | 41 ++++++++++++++++++++++----------
> 1 file changed, 29 insertions(+), 12 deletions(-)
>
>diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
>index 2ee2596570e..962f7295e06 100644
>--- a/source4/dsdb/schema/schema_set.c
>+++ b/source4/dsdb/schema/schema_set.c
>@@ -696,6 +696,8 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
> {
> 	int ret;
> 	void *ptr;
>+	void *schema_parent = NULL;
>+	bool is_already_parent;
> 	struct dsdb_schema *old_schema;
> 	old_schema = ldb_get_opaque(ldb, "dsdb_schema");
> 	ret = ldb_set_opaque(ldb, "dsdb_schema", schema);
>@@ -708,8 +710,9 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
> 	talloc_unlink(ldb, old_schema);
> 
> 	/* Reference schema on ldb if it wasn't done already */
>-	ret = talloc_is_parent(ldb, schema);
>-	if (ret == 0) {
>+	schema_parent = talloc_parent(schema);
>+	is_already_parent = (schema_parent == ldb);
>+	if (!is_already_parent) {
> 		ptr = talloc_reference(ldb, schema);
> 		if (ptr == NULL) {
> 			return ldb_oom(ldb);
>@@ -773,10 +776,10 @@ int dsdb_set_global_schema(struct ldb_context *ldb)
> 	/* Don't write indices and attributes, it's expensive */
> 	ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, SCHEMA_MEMORY_ONLY);
> 	if (ret == LDB_SUCCESS) {
>-		/* If ldb doesn't have a reference to the schema, make one,
>-		 * just in case the original copy is replaced */
>-		ret = talloc_is_parent(ldb, global_schema);
>-		if (ret == 0) {
>+		void *schema_parent = talloc_parent(global_schema);
>+		bool is_already_parent =
>+			(schema_parent == ldb);
>+		if (!is_already_parent) {
> 			ptr = talloc_reference(ldb, global_schema);
> 			if (ptr == NULL) {
> 				return ldb_oom(ldb);
>@@ -807,7 +810,6 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
> 	dsdb_schema_refresh_fn refresh_fn;
> 	struct ldb_module *loaded_from_module;
> 	bool use_global_schema;
>-	int ret;
> 	TALLOC_CTX *tmp_ctx = talloc_new(reference_ctx);
> 	if (tmp_ctx == NULL) {
> 		return NULL;
>@@ -858,13 +860,28 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
> 	/* This removes the extra reference above */
> 	talloc_free(tmp_ctx);
> 
>-	/* If ref ctx exists and doesn't already reference schema, then add
>-	 * a reference.  Otherwise, just return schema.*/
>-	ret = talloc_is_parent(reference_ctx, schema_out);
>-	if ((ret == 1) || (!reference_ctx)) {
>+	/*
>+	 * If ref ctx exists and doesn't already reference schema, then add
>+	 * a reference.  Otherwise, just return schema.
>+	 *
>+	 * We must use talloc_parent(), which is not quite free (there
>+	 * is no direct parent pointer in talloc, only one on the
>+	 * first child within a linked list), but is much cheaper than
>+	 * talloc_is_parent() which walks the whole tree up to the top
>+	 * looking for a potential grand-grand(etc)-parent.
>+	 */
>+	if (reference_ctx == NULL) {
> 		return schema_out;
> 	} else {
>-		return talloc_reference(reference_ctx, schema_out);
>+		void *schema_parent = talloc_parent(schema_out);
>+		bool is_already_parent =
>+			schema_parent == reference_ctx;
>+		if (is_already_parent) {
>+			return schema_out;
>+		} else {
>+			return talloc_reference(reference_ctx,
>+						schema_out);
>+		}
> 	}
> }
> 
>-- 
>2.30.2
>
>
>From 44860fc48f1455800cb3b32ee532070266a383ca Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 25 Aug 2021 09:54:04 +0000
>Subject: [PATCH 2/3] selftest: Add a test for LookupSids3 and LookupNames4 in
> python
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14807
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit b40761b42e889369599c5eb355028ba377c43b49)
>---
> python/samba/tests/dcerpc/lsa.py | 333 +++++++++++++++++++++++++++++++
> source4/selftest/tests.py        |   1 +
> 2 files changed, 334 insertions(+)
> create mode 100644 python/samba/tests/dcerpc/lsa.py
>
>diff --git a/python/samba/tests/dcerpc/lsa.py b/python/samba/tests/dcerpc/lsa.py
>new file mode 100644
>index 00000000000..4377c42e9b8
>--- /dev/null
>+++ b/python/samba/tests/dcerpc/lsa.py
>@@ -0,0 +1,333 @@
>+# -*- coding: utf-8 -*-
>+#
>+# Unix SMB/CIFS implementation.
>+# Copyright Â© Andrew Bartlett <abartlet@samba.org> 2021
>+# Copyright (C) Catalyst IT Ltd. 2017
>+#
>+# This program is free software; you can redistribute it and/or modify
>+# it under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# This program is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>+#
>+
>+"""Tests for samba.dcerpc.sam."""
>+
>+from samba.dcerpc import samr, security, lsa
>+from samba.credentials import Credentials
>+from samba.tests import TestCase
>+from samba.dcerpc.security import dom_sid
>+from samba import NTSTATUSError
>+from samba.ntstatus import NT_STATUS_ACCESS_DENIED
>+import samba.tests
>+
>+class LsaTests(TestCase):
>+
>+    def setUp(self):
>+        self.lp = self.get_loadparm()
>+        self.server = samba.tests.env_get_var_value('SERVER')
>+
>+    def test_lsa_LookupSids3_multiple(self):
>+        machine_creds = Credentials()
>+        machine_creds.guess(self.lp)
>+        machine_creds.set_machine_account()
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[schannel,seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        sids = lsa.SidArray()
>+        sid = lsa.SidPtr()
>+        # Need a set
>+        x = dom_sid("S-1-5-7")
>+        sid.sid = x
>+        sids.sids = [sid]
>+        sids.num_sids = 1
>+        names = lsa.TransNameArray2()
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+
>+        # We want to run LookupSids3 multiple times on the same
>+        # connection as we have code to re-use the sam.ldb and we need
>+        # to check things work for the second request.
>+        (domains, names, count) = c.LookupSids3(sids, names, level, count, lookup_options, client_revision)
>+        self.assertEqual(count, 1)
>+        self.assertEqual(names.count, 1)
>+        self.assertEqual(names.names[0].name.string,
>+                         "ANONYMOUS LOGON")
>+        (domains2, names2, count2) = c.LookupSids3(sids, names, level, count, lookup_options, client_revision)
>+        self.assertEqual(count2, 1)
>+        self.assertEqual(names2.count, 1)
>+        self.assertEqual(names2.names[0].name.string,
>+                         "ANONYMOUS LOGON")
>+
>+        # Just looking for any exceptions in the last couple of loops
>+        c.LookupSids3(sids, names, level, count, lookup_options, client_revision)
>+        c.LookupSids3(sids, names, level, count, lookup_options, client_revision)
>+
>+    def test_lsa_LookupSids3_multiple_conns(self):
>+        machine_creds = Credentials()
>+        machine_creds.guess(self.lp)
>+        machine_creds.set_machine_account()
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[schannel,seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        sids = lsa.SidArray()
>+        sid = lsa.SidPtr()
>+        # Need a set
>+        x = dom_sid("S-1-5-7")
>+        sid.sid = x
>+        sids.sids = [sid]
>+        sids.num_sids = 1
>+        names = lsa.TransNameArray2()
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+
>+        # We want to run LookupSids3, and then again on a new
>+        # connection to show that we don't have an issue with the DB
>+        # being tied to the wrong connection.
>+        (domains, names, count) = c.LookupSids3(sids,
>+                                                names,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+        self.assertEqual(count, 1)
>+        self.assertEqual(names.count, 1)
>+        self.assertEqual(names.names[0].name.string,
>+                         "ANONYMOUS LOGON")
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[schannel,seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        (domains, names, count) = c.LookupSids3(sids,
>+                                                names,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+        self.assertEqual(count, 1)
>+        self.assertEqual(names.count, 1)
>+        self.assertEqual(names.names[0].name.string,
>+                         "ANONYMOUS LOGON")
>+
>+
>+    def test_lsa_LookupNames4_LookupSids3_multiple(self):
>+        """
>+        Test by going back and forward between real DB lookups
>+        name->sid->name to ensure the sam.ldb handle is fine once
>+        shared
>+        """
>+
>+        machine_creds = Credentials()
>+        machine_creds.guess(self.lp)
>+        machine_creds.set_machine_account()
>+
>+        c_normal = lsa.lsarpc(
>+            "ncacn_np:%s[seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        username, domain = c_normal.GetUserName(None, None, None)
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[schannel,seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        sids  = lsa.TransSidArray3()
>+        names = [username]
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+        (domains, sids, count) = c.LookupNames4(names,
>+                                                sids,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+
>+        # Another lookup on the same connection, will re-used the
>+        # server-side implicit state handle on the connection
>+        (domains, sids, count) = c.LookupNames4(names,
>+                                                sids,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+
>+        self.assertEqual(count, 1)
>+        self.assertEqual(sids.count, 1)
>+
>+        # Now look the SIDs back up
>+        names = lsa.TransNameArray2()
>+        sid = lsa.SidPtr()
>+        sid.sid = sids.sids[0].sid
>+        lookup_sids = lsa.SidArray()
>+        lookup_sids.sids = [sid]
>+        lookup_sids.num_sids = 1
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 1
>+        lookup_options = 0
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+
>+        (domains, names, count) = c.LookupSids3(lookup_sids,
>+                                                names,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+        self.assertEqual(count, 1)
>+        self.assertEqual(names.count, 1)
>+        self.assertEqual(names.names[0].name.string,
>+                         username.string)
>+
>+        # And once more just to be sure, just checking for a fault
>+        sids  = lsa.TransSidArray3()
>+        names = [username]
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+        (domains, sids, count) = c.LookupNames4(names,
>+                                                sids,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+
>+
>+    def test_lsa_LookupNames4_multiple_conns(self):
>+        """
>+        Test by going back and forward between real DB lookups
>+        name->sid->name to ensure the sam.ldb handle is fine once
>+        shared
>+        """
>+
>+        machine_creds = Credentials()
>+        machine_creds.guess(self.lp)
>+        machine_creds.set_machine_account()
>+
>+        c_normal = lsa.lsarpc(
>+            "ncacn_np:%s[seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        username, domain = c_normal.GetUserName(None, None, None)
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[schannel,seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        sids  = lsa.TransSidArray3()
>+        names = [username]
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+        (domains, sids, count) = c.LookupNames4(names,
>+                                                sids,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[schannel,seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        sids  = lsa.TransSidArray3()
>+        names = [username]
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+        (domains, sids, count) = c.LookupNames4(names,
>+                                                sids,
>+                                                level,
>+                                                count,
>+                                                lookup_options,
>+                                                client_revision)
>+
>+    def test_lsa_LookupNames4_without_schannel(self):
>+
>+        machine_creds = Credentials()
>+        machine_creds.guess(self.lp)
>+        machine_creds.set_machine_account()
>+
>+        c_normal = lsa.lsarpc(
>+            "ncacn_np:%s[seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        username, domain = c_normal.GetUserName(None, None, None)
>+
>+        sids  = lsa.TransSidArray3()
>+        names = [username]
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+
>+        with self.assertRaises(NTSTATUSError) as e:
>+            c_normal.LookupNames4(names,
>+                                  sids,
>+                                  level,
>+                                  count,
>+                                  lookup_options,
>+                                  client_revision)
>+        if (e.exception.args[0] != NT_STATUS_ACCESS_DENIED):
>+            raise AssertionError("LookupNames4 without schannel must fail with ACCESS_DENIED")
>+
>+    def test_lsa_LookupSids3_without_schannel(self):
>+        machine_creds = Credentials()
>+        machine_creds.guess(self.lp)
>+        machine_creds.set_machine_account()
>+
>+        c = lsa.lsarpc(
>+            "ncacn_ip_tcp:%s[seal]" % self.server,
>+            self.lp,
>+            machine_creds)
>+
>+        sids = lsa.SidArray()
>+        sid = lsa.SidPtr()
>+        # Need a set
>+        x = dom_sid("S-1-5-7")
>+        sid.sid = x
>+        sids.sids = [sid]
>+        sids.num_sids = 1
>+        names = lsa.TransNameArray2()
>+        level = lsa.LSA_LOOKUP_NAMES_ALL
>+        count = 0
>+        lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES
>+        client_revision = lsa.LSA_CLIENT_REVISION_2
>+
>+        with self.assertRaises(NTSTATUSError) as e:
>+            c.LookupSids3(sids,
>+                          names,
>+                          level,
>+                          count,
>+                          lookup_options,
>+                          client_revision)
>+        if (e.exception.args[0] != NT_STATUS_ACCESS_DENIED):
>+            raise AssertionError("LookupSids3 without schannel must fail with ACCESS_DENIED")
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index 09c1ca71e23..7cb7257d3b8 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -674,6 +674,7 @@ planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.sam")
> planpythontestsuite("ad_dc_default:local", "samba.tests.dsdb")
> planpythontestsuite("none", "samba.tests.dsdb_lock")
> planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.bare")
>+planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.lsa")
> planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.unix")
> planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dcerpc.srvsvc")
> planpythontestsuite("ad_dc_default:local", "samba.tests.samba_tool.timecmd")
>-- 
>2.30.2
>
>
>From c58673f53cd56849a0e572915d38e33cd88ee4d7 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 25 Aug 2021 12:03:08 +1200
>Subject: [PATCH 3/3] s4-lsa: Cache sam.ldb handle in
> lsa_LookupSids3/LookupNames4
>
>Since 5c0345ea9bb34695dcd7be6c913748323bebe937 this
>would not have been implicitly cached via the ldb_wrap
>cache, due to the recording of the remote IP address
>(which is a good thing).
>
>This creates a more explicit and direct correct
>cache on the connection.
>
>The common code, including the SCHANNEL check is
>placed into a helper function.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14807
>
>RN: Fix performance regression in lsa_LookupSids3/LookupNames4 since Samba 4.9 by using an explicit database handle cache
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Sun Sep  5 03:19:26 UTC 2021 on sn-devel-184
>
>(cherry picked from commit ae57d22e45b33537e9fca5969e9b68abd1ad633f)
>---
> source4/rpc_server/lsa/lsa_lookup.c | 131 ++++++++++++++++++++--------
> 1 file changed, 94 insertions(+), 37 deletions(-)
>
>diff --git a/source4/rpc_server/lsa/lsa_lookup.c b/source4/rpc_server/lsa/lsa_lookup.c
>index d41997d4b3d..61cb8a10a23 100644
>--- a/source4/rpc_server/lsa/lsa_lookup.c
>+++ b/source4/rpc_server/lsa/lsa_lookup.c
>@@ -663,25 +663,24 @@ NTSTATUS dcesrv_lsa_LookupSids2(struct dcesrv_call_state *dce_call,
> 	return status;
> }
> 
>+/* A random hexidecimal number (honest!) */
>+#define LSA_SERVER_IMPLICIT_POLICY_STATE_MAGIC 0xc0c99e00
> 
> /*
>-  lsa_LookupSids3
>-
>-  Identical to LookupSids2, but doesn't take a policy handle
>-  
>+  Ensure we're operating on an schannel connection,
>+  and use a lsa_policy_state cache on the connection.
> */
>-NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call,
>-				TALLOC_CTX *mem_ctx,
>-				struct lsa_LookupSids3 *r)
>+static NTSTATUS schannel_call_setup(struct dcesrv_call_state *dce_call,
>+				    struct lsa_policy_state **_policy_state)
> {
>+	struct lsa_policy_state *policy_state = NULL;
> 	enum dcerpc_transport_t transport =
> 		dcerpc_binding_get_transport(dce_call->conn->endpoint->ep_description);
> 	enum dcerpc_AuthType auth_type = DCERPC_AUTH_TYPE_NONE;
>-	struct dcesrv_lsa_LookupSids_base_state *state = NULL;
>-	NTSTATUS status;
>-
> 	if (transport != NCACN_IP_TCP) {
>-		DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED);
>+		/* We can't call DCESRV_FAULT() in the sub-function */
>+		dce_call->fault_code = DCERPC_FAULT_ACCESS_DENIED;
>+		return NT_STATUS_ACCESS_DENIED;
> 	}
> 
> 	/*
>@@ -693,8 +692,59 @@ NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call,
> 	 */
> 	dcesrv_call_auth_info(dce_call, &auth_type, NULL);
> 	if (auth_type != DCERPC_AUTH_TYPE_SCHANNEL) {
>-		DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED);
>+		/* We can't call DCESRV_FAULT() in the sub-function */
>+		dce_call->fault_code = DCERPC_FAULT_ACCESS_DENIED;
>+		return NT_STATUS_ACCESS_DENIED;
>+	}
>+
>+	/*
>+	 * We don't have a policy handle on this call, so we want to
>+	 * make a policy state and cache it for the life of the
>+	 * connection, to avoid re-opening the DB each call.
>+	 */
>+	policy_state
>+		= dcesrv_iface_state_find_conn(dce_call,
>+					       LSA_SERVER_IMPLICIT_POLICY_STATE_MAGIC,
>+					       struct lsa_policy_state);
>+
>+	if (policy_state == NULL) {
>+		NTSTATUS status
>+			= dcesrv_lsa_get_policy_state(dce_call,
>+						      dce_call /* mem_ctx */,
>+						      0, /* we skip access checks */
>+						      &policy_state);
>+		if (!NT_STATUS_IS_OK(status)) {
>+			return status;
>+		}
>+
>+		/*
>+		 * This will talloc_steal() policy_state onto the
>+		 * connection, which has longer lifetime than the
>+		 * immidiate caller requires
>+		 */
>+		status = dcesrv_iface_state_store_conn(dce_call,
>+						       LSA_SERVER_IMPLICIT_POLICY_STATE_MAGIC,
>+						       policy_state);
>+		if (!NT_STATUS_IS_OK(status)) {
>+			return status;
>+		}
> 	}
>+	*_policy_state = policy_state;
>+	return NT_STATUS_OK;
>+}
>+
>+/*
>+  lsa_LookupSids3
>+
>+  Identical to LookupSids2, but doesn't take a policy handle
>+
>+*/
>+NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call,
>+				TALLOC_CTX *mem_ctx,
>+				struct lsa_LookupSids3 *r)
>+{
>+	struct dcesrv_lsa_LookupSids_base_state *state = NULL;
>+	NTSTATUS status;
> 
> 	*r->out.domains = NULL;
> 	r->out.names->count = 0;
>@@ -706,16 +756,27 @@ NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call,
> 		return NT_STATUS_NO_MEMORY;
> 	}
> 
>-	state->dce_call = dce_call;
>-	state->mem_ctx = mem_ctx;
>-
>-	status = dcesrv_lsa_get_policy_state(state->dce_call, mem_ctx,
>-					     0, /* we skip access checks */
>-					     &state->policy_state);
>+	/*
>+	 * We don't have a policy handle on this call, so we want to
>+	 * make a policy state and cache it for the life of the
>+	 * connection, to avoid re-opening the DB each call.
>+	 *
>+	 * This also enforces that this is only available over
>+	 * ncacn_ip_tcp and with SCHANNEL.
>+	 *
>+	 * schannel_call_setup may also set the fault state.
>+	 *
>+	 * state->policy_state is shared between all calls on this
>+	 * connection and is moved with talloc_steal() under the
>+	 * connection, not dce_call or state.
>+	 */
>+	status = schannel_call_setup(dce_call, &state->policy_state);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		return status;
> 	}
> 
>+	state->dce_call = dce_call;
>+	state->mem_ctx = mem_ctx;
> 	state->r.in.sids = r->in.sids;
> 	state->r.in.level = r->in.level;
> 	state->r.in.lookup_options = r->in.lookup_options;
>@@ -1296,25 +1357,9 @@ NTSTATUS dcesrv_lsa_LookupNames3(struct dcesrv_call_state *dce_call,
> NTSTATUS dcesrv_lsa_LookupNames4(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
> 				 struct lsa_LookupNames4 *r)
> {
>-	enum dcerpc_transport_t transport =
>-		dcerpc_binding_get_transport(dce_call->conn->endpoint->ep_description);
>-	enum dcerpc_AuthType auth_type = DCERPC_AUTH_TYPE_NONE;
> 	struct dcesrv_lsa_LookupNames_base_state *state = NULL;
> 	NTSTATUS status;
> 
>-	if (transport != NCACN_IP_TCP) {
>-		DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED);
>-	}
>-
>-	/*
>-	 * We don't have policy handles on this call. So this must be restricted
>-	 * to crypto connections only.
>-	 */
>-	dcesrv_call_auth_info(dce_call, &auth_type, NULL);
>-	if (auth_type != DCERPC_AUTH_TYPE_SCHANNEL) {
>-		DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED);
>-	}
>-
> 	*r->out.domains = NULL;
> 	r->out.sids->count = 0;
> 	r->out.sids->sids = NULL;
>@@ -1328,9 +1373,21 @@ NTSTATUS dcesrv_lsa_LookupNames4(struct dcesrv_call_state *dce_call, TALLOC_CTX
> 	state->dce_call = dce_call;
> 	state->mem_ctx = mem_ctx;
> 
>-	status = dcesrv_lsa_get_policy_state(state->dce_call, state,
>-					     0, /* we skip access checks */
>-					     &state->policy_state);
>+	/*
>+	 * We don't have a policy handle on this call, so we want to
>+	 * make a policy state and cache it for the life of the
>+	 * connection, to avoid re-opening the DB each call.
>+	 *
>+	 * This also enforces that this is only available over
>+	 * ncacn_ip_tcp and with SCHANNEL.
>+	 *
>+	 * schannel_call_setup may also set the fault state.
>+	 *
>+	 * state->policy_state is shared between all calls on this
>+	 * connection and is moved with talloc_steal() under the
>+	 * connection, not dce_call or state.
>+	 */
>+	status = schannel_call_setup(dce_call, &state->policy_state);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		return status;
> 	}
>-- 
>2.30.2
>

Actions: View

Attachments on bug 14821: 16774 | 17535