From e76c7d0fddbe800d8efc048cf53d3676610b5960 Mon Sep 17 00:00:00 2001 From: Bob Campbell 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 Reviewed-by: Andrew Bartlett Reviewed-by: Garming Sam (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 . +# + +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 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 Reviewed-by: Andrew Bartlett Reviewed-by: Garming Sam (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 2004-2005 Copyright (C) Matthieu Patou 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 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 Reviewed-by: Andrew Bartlett Reviewed-by: Garming Sam Autobuild-User(master): Andrew Bartlett 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