Attachment 13424 Details for Bug 12842 – patch for 4.7 cherry-picked from master

[patch] patch for 4.7 cherry-picked from master

4.7-cracknames.patch (text/plain), 15.71 KB, created by Andrew Bartlett on 2017-07-24 10:04:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2017-07-24 10:04:05 UTC

Size: 15.71 KB

patch

obsolete

>From e76c7d0fddbe800d8efc048cf53d3676610b5960 Mon Sep 17 00:00:00 2001
>From: Bob Campbell <bobcampbell@catalyst.net.nz>
>Date: Wed, 5 Jul 2017 11:08:45 +1200
>Subject: [PATCH 1/3] python/tests: add python test for cracknames
>
>This fails due the bug, which causes the related test in
>drsuapi_cracknames.c to flap. It also fails due to us not yet supporting
>DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL or
>DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12842
>
>Signed-off-by: Bob Campbell <bobcampbell@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit 4779afe0d2dd14371b68e80f47d11942456bb365)
>---
> selftest/knownfail                       |   1 +
> source4/selftest/tests.py                |   5 +
> source4/torture/drs/python/cracknames.py | 166 +++++++++++++++++++++++++++++++
> 3 files changed, 172 insertions(+)
> create mode 100644 source4/torture/drs/python/cracknames.py
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 1cba331..2d3e0f2 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -299,6 +299,7 @@
> #ntvfs server blocks copychunk with execute access on read handle
> ^samba4.smb2.ioctl.copy_chunk_bad_access
> ^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaPrefixMapTestCase.test_regular_prefix_map_ex_attid.*
>+^samba4.drs.cracknames.python.*cracknames.DrsCracknamesTestCase.test_Cracknames.*
> # We don't support NDR64 yet, so we generate the wrong FAULT code
> ^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
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index 15037a2..44c0b08 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -839,6 +839,11 @@ for env in ['ad_dc_ntvfs']:
>     planoldpythontestsuite(env, "samba.tests.join",
>                            name="samba.tests.join.python(%s)" % env,
>                            extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
>+    planoldpythontestsuite(env, "cracknames",
>+                           extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
>+                           name="samba4.drs.cracknames.python(%s)" % env,
>+                           environ={'DC1': "$DC_SERVER", 'DC2': '$DC_SERVER'},
>+                           extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
> 
> planoldpythontestsuite("chgdcpass:local", "samba.tests.blackbox.samba_dnsupdate",
>                        environ={'DNS_SERVER_IP': '$SERVER_IP'})
>diff --git a/source4/torture/drs/python/cracknames.py b/source4/torture/drs/python/cracknames.py
>new file mode 100644
>index 0000000..d8c8ae5
>--- /dev/null
>+++ b/source4/torture/drs/python/cracknames.py
>@@ -0,0 +1,166 @@
>+#!/usr/bin/env python
>+# -*- coding: utf-8 -*-
>+#
>+# Copyright (C) Catalyst .Net 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/>.
>+#
>+
>+import samba.tests
>+import ldb
>+import drs_base
>+
>+from samba.dcerpc import drsuapi
>+
>+class DrsCracknamesTestCase(drs_base.DrsBaseTestCase):
>+    def setUp(self):
>+        super(DrsCracknamesTestCase, self).setUp()
>+        (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
>+
>+        self.ou = "ou=Cracknames_ou,%s" % self.ldb_dc1.get_default_basedn()
>+        self.username = "Cracknames_user"
>+        self.user = "cn=%s,%s" % (self.username, self.ou)
>+
>+        self.ldb_dc1.add({
>+            "dn": self.ou,
>+            "objectclass": "organizationalUnit"})
>+
>+        self.user_record = {
>+            "dn": self.user,
>+            "objectclass": "user",
>+            "sAMAccountName" : self.username,
>+            "userPrincipalName" : "test@test.com",
>+            "servicePrincipalName" : "test/%s" % self.ldb_dc1.get_default_basedn(),
>+            "displayName" : "test"}
>+
>+        self.ldb_dc1.add(self.user_record)
>+        self.ldb_dc1.delete(self.user_record["dn"])
>+        self.ldb_dc1.add(self.user_record)
>+
>+        # The formats specified in MS-DRSR 4.1.4.13; DS_NAME_FORMAT
>+        # We don't support any of the ones specified in 4.1.4.1.2.
>+        self.formats = {
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_FQDN_1779,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_NT4_ACCOUNT,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_DISPLAY,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_CANONICAL,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_CANONICAL_EX,
>+            drsuapi.DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL,
>+            # We currently don't support this
>+            #drsuapi.DRSUAPI_DS_NAME_FORMAT_SID_OR_SID_HISTORY,
>+            # This format is not supported by Windows (or us)
>+            #drsuapi.DRSUAPI_DS_NAME_FORMAT_DNS_DOMAIN,
>+        }
>+
>+    def tearDown(self):
>+        self.ldb_dc1.delete(self.user)
>+        self.ldb_dc1.delete(self.ou)
>+        super(DrsCracknamesTestCase, self).tearDown()
>+
>+    def test_Cracknames(self):
>+        """
>+        Verifies that we can cracknames any of the standard formats
>+        (DS_NAME_FORMAT) to a GUID, and that we can cracknames a
>+        GUID to any of the standard formats.
>+
>+        GUID was chosen just so that we don't have to do an n^2 loop.
>+        """
>+        (result, ctr) = self._do_cracknames(self.user,
>+                                            drsuapi.DRSUAPI_DS_NAME_FORMAT_FQDN_1779,
>+                                            drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID)
>+
>+        self.assertEquals(ctr.count, 1)
>+        self.assertEquals(ctr.array[0].status,
>+                          drsuapi.DRSUAPI_DS_NAME_STATUS_OK)
>+
>+        user_guid = ctr.array[0].result_name
>+
>+        for name_format in self.formats:
>+            (result, ctr) = self._do_cracknames(user_guid,
>+                                                drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID,
>+                                                name_format)
>+
>+            self.assertEquals(ctr.count, 1)
>+            self.assertEquals(ctr.array[0].status,
>+                              drsuapi.DRSUAPI_DS_NAME_STATUS_OK,
>+                              "Expected 0, got %s, desired format is %s"
>+                              % (ctr.array[0].status, name_format))
>+
>+            (result, ctr) = self._do_cracknames(ctr.array[0].result_name,
>+                                                name_format,
>+                                                drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID)
>+
>+            self.assertEquals(ctr.count, 1)
>+            self.assertEquals(ctr.array[0].status,
>+                              drsuapi.DRSUAPI_DS_NAME_STATUS_OK,
>+                              "Expected 0, got %s, offered format is %s"
>+                              % (ctr.array[0].status, name_format))
>+
>+    def test_MultiValuedAttribute(self):
>+        """
>+        Verifies that, if we try and cracknames with the desired output
>+        being a multi-valued attribute, it returns
>+        DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE.
>+        """
>+        username = "Cracknames_user_MVA"
>+        user = "cn=%s,%s" % (username, self.ou)
>+
>+        user_record = {
>+            "dn": user,
>+            "objectclass": "user",
>+            "sAMAccountName" : username,
>+            "userPrincipalName" : "test2@test.com",
>+            "servicePrincipalName" : ["test2/%s" % self.ldb_dc1.get_default_basedn(),
>+                                      "test3/%s" % self.ldb_dc1.get_default_basedn()],
>+            "displayName" : "test2"}
>+
>+        self.ldb_dc1.add(user_record)
>+
>+        (result, ctr) = self._do_cracknames(user,
>+                                            drsuapi.DRSUAPI_DS_NAME_FORMAT_FQDN_1779,
>+                                            drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID)
>+
>+        self.assertEquals(ctr.count, 1)
>+        self.assertEquals(ctr.array[0].status,
>+                          drsuapi.DRSUAPI_DS_NAME_STATUS_OK)
>+
>+        user_guid = ctr.array[0].result_name
>+
>+        (result, ctr) = self._do_cracknames(user_guid,
>+                                            drsuapi.DRSUAPI_DS_NAME_FORMAT_GUID,
>+                                            drsuapi.DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL)
>+
>+        self.assertEquals(ctr.count, 1)
>+        self.assertEquals(ctr.array[0].status,
>+                          drsuapi.DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE)
>+
>+        self.ldb_dc1.delete(user)
>+
>+    def _do_cracknames(self, name, format_offered, format_desired):
>+        req = drsuapi.DsNameRequest1()
>+        names = drsuapi.DsNameString()
>+        names.str = name
>+
>+        req.codepage = 1252 # German, but it doesn't really matter here
>+        req.language = 1033
>+        req.format_flags = 0
>+        req.format_offered = format_offered
>+        req.format_desired = format_desired
>+        req.count = 1
>+        req.names = [names]
>+
>+        (result, ctr) = self.drs.DsCrackNames(self.drs_handle, 1, req)
>+        return (result, ctr)
>-- 
>2.9.4
>
>
>From 2506bb08673e86560fc061a8594bb04bdf180ccd Mon Sep 17 00:00:00 2001
>From: Bob Campbell <bobcampbell@catalyst.net.nz>
>Date: Wed, 5 Jul 2017 11:15:04 +1200
>Subject: [PATCH 2/3] samdb/cracknames: do not show recycled when a guid is
> desired
>
>Previously, when a GUID was desired to
>cracknames, it would include recycled objects as well. This would
>sometimes result in two objects being returned from a query which is
>supposed to return a unique GUID. For example, if a deleted user had
>the same sAMAccountName as a non-deleted user and cracknames was used to
>find the GUID of this account, it would return two GUIDs, and so would
>fail with DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12842
>
>Signed-off-by: Bob Campbell <bobcampbell@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit c186e02b40c921d33e23c8b2f7c5f1abb235a438)
>---
> source4/dsdb/samdb/cracknames.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c
>index 14d6a53a..bb25b00 100644
>--- a/source4/dsdb/samdb/cracknames.c
>+++ b/source4/dsdb/samdb/cracknames.c
>@@ -7,6 +7,7 @@
>    Copyright (C) Stefan Metzmacher 2004
>    Copyright (C) Andrew Bartlett <abartlet@samba.org> 2004-2005
>    Copyright (C) Matthieu Patou <mat@matws.net> 2012
>+   Copyright (C) Catalyst .Net 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
>@@ -980,7 +981,7 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_
> 		} else {
> 			real_search_dn = ldb_get_default_basedn(sam_ctx);
> 		}
>-		if (format_desired == DRSUAPI_DS_NAME_FORMAT_GUID){
>+		if (format_offered == DRSUAPI_DS_NAME_FORMAT_GUID){
> 			 dsdb_flags |= DSDB_SEARCH_SHOW_RECYCLED;
> 		}
> 		/* search with the 'phantom root' flag */
>-- 
>2.9.4
>
>
>From 97084d847fa247c52a579f8f0f47fb3ba1cfb207 Mon Sep 17 00:00:00 2001
>From: Bob Campbell <bobcampbell@catalyst.net.nz>
>Date: Wed, 5 Jul 2017 16:08:11 +1200
>Subject: [PATCH 3/3] samdb/cracknames: support user and service principal as
> desired format
>
>This adds support for DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL and
>DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL as desired formats.
>
>This also causes the test in cracknames.py to no longer fail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12842
>
>Signed-off-by: Bob Campbell <bobcampbell@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Mon Jul 24 11:10:26 CEST 2017 on sn-devel-144
>
>(cherry picked from commit eb2e77970e41c1cb62c041877565e939c78ff52d)
>---
> selftest/knownfail              |  1 -
> source4/dsdb/samdb/cracknames.c | 35 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 2d3e0f2..1cba331 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -299,7 +299,6 @@
> #ntvfs server blocks copychunk with execute access on read handle
> ^samba4.smb2.ioctl.copy_chunk_bad_access
> ^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaPrefixMapTestCase.test_regular_prefix_map_ex_attid.*
>-^samba4.drs.cracknames.python.*cracknames.DrsCracknamesTestCase.test_Cracknames.*
> # We don't support NDR64 yet, so we generate the wrong FAULT code
> ^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
>diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c
>index bb25b00..d43f510 100644
>--- a/source4/dsdb/samdb/cracknames.c
>+++ b/source4/dsdb/samdb/cracknames.c
>@@ -881,6 +881,12 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_
> 	const char * const _domain_attrs_guid[] = { "ncName", "dnsRoot", NULL};
> 	const char * const _result_attrs_guid[] = { "objectGUID", NULL};
> 
>+	const char * const _domain_attrs_upn[] = { "ncName", "dnsRoot", NULL};
>+	const char * const _result_attrs_upn[] = { "userPrincipalName", NULL};
>+
>+	const char * const _domain_attrs_spn[] = { "ncName", "dnsRoot", NULL};
>+	const char * const _result_attrs_spn[] = { "servicePrincipalName", NULL};
>+
> 	const char * const _domain_attrs_display[] = { "ncName", "dnsRoot", NULL};
> 	const char * const _result_attrs_display[] = { "displayName", "samAccountName", NULL};
> 
>@@ -910,6 +916,14 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_
> 		domain_attrs = _domain_attrs_display;
> 		result_attrs = _result_attrs_display;
> 		break;
>+	case DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL:
>+		domain_attrs = _domain_attrs_upn;
>+		result_attrs = _result_attrs_upn;
>+		break;
>+	case DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL:
>+		domain_attrs = _domain_attrs_spn;
>+		result_attrs = _result_attrs_spn;
>+		break;
> 	default:
> 		domain_attrs = _domain_attrs_none;
> 		result_attrs = _result_attrs_none;
>@@ -1239,7 +1253,17 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_
> 		return WERR_OK;
> 	}
> 	case DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL: {
>-		info1->status = DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE;
>+		if (result->elements[0].num_values > 1) {
>+			info1->status = DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE;
>+			return WERR_OK;
>+		}
>+
>+		info1->result_name = ldb_msg_find_attr_as_string(result, "servicePrincipalName", NULL);
>+		if (!info1->result_name) {
>+			info1->status = DRSUAPI_DS_NAME_STATUS_NO_MAPPING;
>+		} else {
>+			info1->status = DRSUAPI_DS_NAME_STATUS_OK;
>+		}
> 		return WERR_OK;
> 	}
> 	case DRSUAPI_DS_NAME_FORMAT_DNS_DOMAIN:	
>@@ -1248,6 +1272,15 @@ static WERROR DsCrackNameOneFilter(struct ldb_context *sam_ctx, TALLOC_CTX *mem_
> 		info1->status = DRSUAPI_DS_NAME_STATUS_RESOLVE_ERROR;
> 		return WERR_OK;
> 	}
>+	case DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL: {
>+		info1->result_name = ldb_msg_find_attr_as_string(result, "userPrincipalName", NULL);
>+		if (!info1->result_name) {
>+			info1->status = DRSUAPI_DS_NAME_STATUS_NO_MAPPING;
>+		} else {
>+			info1->status = DRSUAPI_DS_NAME_STATUS_OK;
>+		}
>+		return WERR_OK;
>+	}
> 	default:
> 		info1->status = DRSUAPI_DS_NAME_STATUS_NO_MAPPING;
> 		return WERR_OK;
>-- 
>2.9.4
>

Flags:

abartlet: review? (garming)
dbagnall: review+

Actions: View

Attachments on bug 12842: 13424