[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Sun Sep 5 03:20:01 UTC 2021
The branch, master has been updated
via ae57d22e45b s4-lsa: Cache sam.ldb handle in lsa_LookupSids3/LookupNames4
via b40761b42e8 selftest: Add a test for LookupSids3 and LookupNames4 in python
via 8affe4a1e62 dsdb: Be careful to avoid use of the expensive talloc_is_parent()
via 75a5ed66731 selftest: Only run samba_tool_drs_showrepl test once
via e8b4599e093 selftest: Split up targets for samba_tool_drs from samba_tool_drs_showrepl
from 02b18730336 Fix Python docstrings
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit ae57d22e45b33537e9fca5969e9b68abd1ad633f
Author: Andrew Bartlett <abartlet at samba.org>
Date: Wed Aug 25 12:03:08 2021 +1200
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
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Sun Sep 5 03:19:26 UTC 2021 on sn-devel-184
commit b40761b42e889369599c5eb355028ba377c43b49
Author: Andrew Bartlett <abartlet at samba.org>
Date: Wed Aug 25 09:54:04 2021 +0000
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 at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 8affe4a1e625104de4ca024fdc3e9cd96498aff3
Author: Andrew Bartlett <abartlet at samba.org>
Date: Wed Aug 25 09:41:11 2021 +1200
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 at 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 at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
Autobuild-User(master): Garming Sam <garming at 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.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14806
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 75a5ed66731e947fa16af81aab7649d1fddec45f
Author: Andrew Bartlett <abartlet at samba.org>
Date: Sat Sep 4 13:11:08 2021 +1200
selftest: Only run samba_tool_drs_showrepl test once
This test is not slow, but there is no value running it twice.
Running this test twice just increases the chances we might
loose a race as it shows and validates live replication data.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit e8b4599e0935290c5e59df9fd4f695ad8d6f361c
Author: Andrew Bartlett <abartlet at samba.org>
Date: Sat Sep 4 12:28:20 2021 +1200
selftest: Split up targets for samba_tool_drs from samba_tool_drs_showrepl
These now run in the disconnected sets schema_dc/schema_pair_dc and
ad_dc/vampire_dc/promoted_dc. By aiming at different sets ofservers
we can't cause cross-contamination in terms of which servers are
listed as outbound connections.
Also, by running the tests only once we reduce the chaces of trouble
by half.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
-----------------------------------------------------------------------
Summary of changes:
python/samba/tests/dcerpc/lsa.py | 333 ++++++++++++++++++++++++++++++++++++
source4/dsdb/schema/schema_set.c | 41 +++--
source4/rpc_server/lsa/lsa_lookup.c | 131 ++++++++++----
source4/selftest/tests.py | 34 ++--
4 files changed, 480 insertions(+), 59 deletions(-)
create mode 100644 python/samba/tests/dcerpc/lsa.py
Changeset truncated at 500 lines:
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 at 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/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index f4934917c7c..45faa0912ec 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -698,6 +698,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);
@@ -710,8 +712,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);
@@ -775,10 +778,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);
@@ -809,7 +812,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;
@@ -860,13 +862,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);
+ }
}
}
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)) {
--
Samba Shared Repository
More information about the samba-cvs
mailing list