[PATCH] cracknames: add python test & fix issues (bug #12842)
Bob Campbell
bobcampbell at catalyst.net.nz
Mon Jul 10 01:31:17 UTC 2017
Hi again,
Turns out that I actually broke the existing cracknames test with this;
oops. Attached is a patch which passes. The change is using a different
error code when it can't find the expected format.
Thanks,
Bob
On 06/07/17 13:23, Bob Campbell wrote:
> Hi all,
>
> The attached patch #2 fixes the flapping test in rpc.cracknames, and the
> first patch tests this fix as well as the implementation of user and
> service principals.
>
> Before this patch, we could potentially return two results from a
> cracknames trying to map from an account name to a GUID, in the scenario
> where a deleted user had the same account name as a non-deleted user
> (this would happen with any applicable attribute). This was just due to
> a check being the wrong way around; we should search in the recycled
> partition when we're mapping _from_ a GUID, since there can only be one
> object mapped from a GUID and it's possible that we're trying to find a
> recycled object in this scenario.
>
> The third patch adds support for getting user principals and service
> principals from cracknames. While poking around in it, I found that
> these two weren't implemented and that it would be trivial to do so.
> Note that, since service principal is a multi-valued attribute, we check
> if the result would be unique before returning. Windows also gives
> DS_NAME_STATUS_NOT_UNIQUE if there is more than one result, but also
> returns null whereas we appear to return the first result. In addition
> to this, we don't support SID_OR_SID_HISTORY_NAME or DNS_DOMAIN_NAME. We
> also don't support any of the formats in MS-DRSR 4.1.4.1.2. This could
> be a potential future task, but seems like it would be non-trivial since
> some of these aren't just for converting from one format to another.
>
> Please review & push if applicable.
>
> Thanks,
> Bob
>
-------------- next part --------------
From d8a41771ceacb76f4d4b40d11bd64cc5bbdeb78d Mon Sep 17 00:00:00 2001
From: Bob Campbell <bobcampbell at catalyst.net.nz>
Date: Wed, 5 Jul 2017 11:08:45 +1200
Subject: [PATCH 1/3] python/tests: add python test for cracknames
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12842
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.
Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
---
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 4e0642f..dbef003 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -835,6 +835,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 at 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 at 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.7.4
From dad60f52009521b3fa3a884cc25b43fbe86505a1 Mon Sep 17 00:00:00 2001
From: Bob Campbell <bobcampbell at 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
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12842
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.
Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
---
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 at samba.org> 2004-2005
Copyright (C) Matthieu Patou <mat at 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.7.4
From 5a15070284c019c9f60202370614387781173624 Mon Sep 17 00:00:00 2001
From: Bob Campbell <bobcampbell at 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.
Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
---
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.7.4
More information about the samba-technical
mailing list