[PATCH] cracknames: add python test & fix issues (bug #12842)
Bob Campbell
bobcampbell at catalyst.net.nz
Thu Jul 6 01:23:22 UTC 2017
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)
--
1.9.1
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 */
--
1.9.1
From 410fb7931dc61b329aa987d8c714cd99e1b1bfc4 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..68bb193 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_NOT_FOUND;
+ } 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_NOT_FOUND;
+ } else {
+ info1->status = DRSUAPI_DS_NAME_STATUS_OK;
+ }
+ return WERR_OK;
+ }
default:
info1->status = DRSUAPI_DS_NAME_STATUS_NO_MAPPING;
return WERR_OK;
--
1.9.1
More information about the samba-technical
mailing list