s4-drsuapi: Avoid segfault when replicating as a non-admin with GUID_DRS_GET_CHANGES

Users who are not administrator do not get b_state->sam_ctx_system filled in.

We should probably use the 'sam_ctx' variable in all cases (instead of
b_state->sam_ctx*), but I'll make this change in a separate patch, so
that the bug fix remains independent from other tidy-ups.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>

diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
index 096162dfded35c6a0234d9e72b2adde59cdb2dfa..b98f14c156a0862d47afbe5d6bbf8d8b2ba10862 100644 (file)
--- a/source4/rpc_server/drsuapi/getncchanges.c
+++ b/source4/rpc_server/drsuapi/getncchanges.c
@@ -2250,7 +2250,7 @@ allowed:
                        return WERR_NOT_ENOUGH_MEMORY;
                }
 
-               ret = dsdb_find_guid_by_dn(b_state->sam_ctx_system,
+               ret = dsdb_find_guid_by_dn(b_state->sam_ctx,
                                           getnc_state->ncRoot_dn,
                                           &getnc_state->ncRoot_guid);
                if (ret != LDB_SUCCESS) {

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 215257388b2fc45d47c871c21a8b7f9d741da374..8aeba34810ea2a6125c8eaaf909aa56d10455143 100755 (executable)
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -853,6 +853,11 @@ for env in ['vampire_dc', 'promoted_dc']:
                            name="samba4.drs.getnc_exop.python(%s)" % env,
                            environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
                            extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
+    planoldpythontestsuite(env, "getnc_unpriv",
+                           extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
+                           name="samba4.drs.getnc_unpriv.python(%s)" % env,
+                           environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
+                           extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
     planoldpythontestsuite(env, "linked_attributes_drs",
                            extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
                            name="samba4.drs.linked_attributes_drs.python(%s)" % env,

diff --git a/source4/torture/drs/python/getnc_unpriv.py b/source4/torture/drs/python/getnc_unpriv.py
new file mode 100644 (file)
index 0000000..61ecefa
--- /dev/null
+++ b/source4/torture/drs/python/getnc_unpriv.py
@@ -0,0 +1,116 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+# Tests replication scenarios with different user privileges
+#
+# Copyright (C) Kamen Mazdrashki <kamenim@samba.org> 2011
+# Copyright (C) Andrew Bartlett <abartlet@samba.org> 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/>.
+#
+
+#
+# Usage:
+#  export DC1=dc1_dns_name
+#  export DC2=dc2_dns_name
+#  export SUBUNITRUN=$samba4srcdir/scripting/bin/subunitrun
+#  PYTHONPATH="$PYTHONPATH:$samba4srcdir/torture/drs/python" $SUBUNITRUN getnc_unpriv -U"$DOMAIN/$DC_USERNAME"%"$DC_PASSWORD"
+#
+
+import drs_base
+import samba.tests
+
+from samba import sd_utils
+import ldb
+from ldb import SCOPE_BASE
+
+from samba.dcerpc import drsuapi
+from samba.credentials import DONT_USE_KERBEROS
+
+class DrsReplicaSyncUnprivTestCase(drs_base.DrsBaseTestCase):
+    """Confirm the behaviour of DsGetNCChanges for unprivileged users"""
+
+    def setUp(self):
+        super(DrsReplicaSyncUnprivTestCase, self).setUp()
+        self.get_changes_user = "get-changes-user"
+        self.base_dn = self.ldb_dc1.get_default_basedn()
+        self.ou = "OU=test_getncchanges,%s" % self.base_dn
+        self.user_pass = samba.generate_random_password(12, 16)
+        self.ldb_dc1.add({
+            "dn": self.ou,
+            "objectclass": "organizationalUnit"})
+        self.ldb_dc1.newuser(self.get_changes_user, self.user_pass,
+                             userou="OU=test_getncchanges")
+        (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
+
+        self.sd_utils = sd_utils.SDUtils(self.ldb_dc1)
+        user_dn = "cn=%s,%s" % (self.get_changes_user, self.ou)
+        user_sid = self.sd_utils.get_object_sid(user_dn)
+        mod = "(A;;CR;1131f6aa-9c07-11d1-f79f-00c04fc2dcd2;;%s)" % str(user_sid)
+        self.sd_utils.dacl_add_ace(self.base_dn, mod)
+
+        # We set DONT_USE_KERBEROS to avoid a race with getting the
+        # user replicated to our selected KDC
+        self.user_creds = self.insta_creds(template=self.get_credentials(),
+                                           username=self.get_changes_user,
+                                           userpass=self.user_pass,
+                                           kerberos_state=DONT_USE_KERBEROS)
+        (self.user_drs, self.user_drs_handle) = self._ds_bind(self.dnsname_dc1,
+                                                              self.user_creds)
+
+    def tearDown(self):
+        try:
+            self.ldb_dc1.delete(self.ou, ["tree_delete:1"])
+        except ldb.LdbError as (enum, string):
+            if enum == ldb.ERR_NO_SUCH_OBJECT:
+                pass
+        super(DrsReplicaSyncUnprivTestCase, self).tearDown()
+
+    def test_do_single_repl(self):
+        """
+        Make sure that DRSU_EXOP_REPL_OBJ works as a less-privileged
+        user with the correct GET_CHANGES rights
+        """
+
+        ou1 = "OU=single_obj,%s" % self.ou
+        self.ldb_dc1.add({
+            "dn": ou1,
+            "objectclass": "organizationalUnit"
+            })
+        req8 = self._exop_req8(dest_dsa=None,
+                               invocation_id=self.ldb_dc1.get_invocation_id(),
+                               nc_dn_str=ou1,
+                               exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ,
+                               replica_flags=drsuapi.DRSUAPI_DRS_WRIT_REP)
+        (level, ctr) = self.user_drs.DsGetNCChanges(self.user_drs_handle, 8, req8)
+        self._check_ctr6(ctr, [ou1])
+
+    def test_do_full_repl(self):
+        """
+        Make sure that full replication works as a less-privileged
+        user with the correct GET_CHANGES rights
+        """
+
+        ou1 = "OU=single_obj,%s" % self.ou
+        self.ldb_dc1.add({
+            "dn": ou1,
+            "objectclass": "organizationalUnit"
+            })
+        req8 = self._exop_req8(dest_dsa=None,
+                               invocation_id=self.ldb_dc1.get_invocation_id(),
+                               nc_dn_str=ou1,
+                               exop=drsuapi.DRSUAPI_EXOP_NONE,
+                               replica_flags=drsuapi.DRSUAPI_DRS_WRIT_REP)
+        (level, ctr) = self.user_drs.DsGetNCChanges(self.user_drs_handle, 8, req8)
+        self.assertEqual(ctr.extended_ret, drsuapi.DRSUAPI_EXOP_ERR_NONE)