The Samba-Bugzilla – Attachment 13424 Details for
Bug 12842
Cracknames can fail when desired format is GUID due to deleted accounts
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[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:
patch for 4.7 cherry-picked from master
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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
abartlet
:
review?
(
garming
)
dbagnall
:
review+
Actions:
View
Attachments on
bug 12842
: 13424