[PATCH] Dependent changes for rename tool
Tim Beale
timbeale at catalyst.net.nz
Mon Jul 2 22:41:09 UTC 2018
Looking for one more reviewer.
Patches attached. I don't think they've really changed, although I've
pulled in Andrew's dbchecker patches, i.e. from:
https://lists.samba.org/archive/samba-technical/2018-June/128797.html
git branch:
https://gitlab.com/catalyst-samba/samba/commits/tim-backup-for-master
CI link:
https://gitlab.com/catalyst-samba/samba/pipelines/24994338
On 29/06/18 11:02, Tim Beale via samba-technical wrote:
> Thanks Douglas. That's a good point - the change in logic wasn't
> entirely obvious. I've split it out into a separate patch now. Updated
> patch-set attached.
>
> New CI link:
> https://gitlab.com/catalyst-samba/samba/pipelines/24794958
>
>
-------------- next part --------------
From 5da6833c828b351d904f2bdfc81d6fc52ae5c7fe Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 29 Jun 2018 14:53:19 +1200
Subject: [PATCH 1/7] dbcheck: Use symbolic control name for
DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS
While we do not wish to encourage use of this control, manually typed OIDs are
even more trouble, so pass out via pydsdb.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/dbchecker.py | 2 +-
source4/dsdb/pydsdb.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 9d72fc6..00194af 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -765,7 +765,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
m = ldb.Message()
m.dn = obj.dn
m['value'] = ldb.MessageElement(forward_vals, ldb.FLAG_MOD_REPLACE, forward_attr)
- if self.do_modify(m, ["local_oid:1.3.6.1.4.1.7165.4.3.19.2:1"],
+ if self.do_modify(m, ["local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS],
"Failed to fix duplicate links in attribute '%s'" % forward_attr):
self.report("Fixed duplicate links in attribute '%s'" % (forward_attr))
duplicate_cache_key = "%s:%s" % (str(obj.dn), forward_attr)
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index d9177bb..d10c7ec 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1581,6 +1581,7 @@ MODULE_INIT_FUNC(dsdb)
ADD_DSDB_STRING(DSDB_SYNTAX_OR_NAME);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
+ ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
--
2.7.4
From ed2418e7c2eaeb0c8ece7ffe414cd2bdc4d8cbca Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 25 May 2018 14:05:27 +1200
Subject: [PATCH 2/7] dbchecker: Fixing up incorrect DNs wasn't working
dbcheck would fail to fix up attributes where the extended DN's GUID is
correct, but the DN itself is incorrect. The code failed attempting to
remove the old/incorrect DN, e.g.
NOTE: old (due to rename or delete) DN string component for
objectCategory in object CN=alice,CN=Users,DC=samba,DC=example,DC=com -
<GUID=7bfdf9d8-62f9-420c-8a71-e3d3e931c91e>;
CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=bad,DC=com
Change DN to <GUID=7bfdf9d8-62f9-420c-8a71-e3d3e931c91e>;
CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=example,DC=com?
[y/N/all/none] y
Failed to fix old DN string on attribute objectCategory : (16,
"attribute 'objectCategory': no matching attribute value while deleting
attribute on 'CN=alice,CN=Users,DC=samba,DC=example,DC=com'")
The problem was the LDB message specified the value to delete with its
full DN, including the GUID. The LDB code then helpfully corrected this
value on the way through, so that the DN got updated to reflect the
correct DN (i.e. 'DC=example,DC=com') of the object matching that GUID,
rather than the incorrect DN (i.e. 'DC=bad,DC=com') that we were trying
to remove. Because the requested value and the existing DB value didn't
match, the operation failed.
We can avoid this problem by passing down just the DN (not the extended
DN) of the value we want to delete. Without the GUID portion of the DN,
the LDB code will no longer try to correct it on the way through, and
the dbcheck operation will succeed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13495
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Pair-programmed-with: Andrew Bartlett <abartlet at samba.org>
---
python/samba/dbchecker.py | 19 +++++--
source4/dsdb/pydsdb.c | 1 +
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 64 +++++++++++++++++++++
source4/dsdb/samdb/samdb.h | 3 +
...ected-after-dbcheck-oneway-link-corruption.ldif | 19 +++++++
...-dbcheck-link-output-oneway-link-corruption.txt | 5 ++
testprogs/blackbox/dbcheck-links.sh | 65 ++++++++++++++++++++++
7 files changed, 171 insertions(+), 5 deletions(-)
create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 00194af..c64fd4c 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -656,7 +656,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
m.dn = dn
m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
m['new_value'] = ldb.MessageElement(str(dsdb_dn), ldb.FLAG_MOD_ADD, attrname)
- if self.do_modify(m, ["show_recycled:1"],
+ if self.do_modify(m, ["show_recycled:1",
+ "local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME],
"Failed to fix old DN string on attribute %s" % (attrname)):
self.report("Fixed old DN string on attribute %s" % (attrname))
@@ -1298,14 +1299,22 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
res[0].dn, "SID")
continue
+ # Only for non-links, not even forward-only links
+ # (otherwise this breaks repl_meta_data):
+ #
# Now we have checked the GUID and SID, offer to fix old
- # DN strings as a non-error (for forward links with no
+ # DN strings as a non-error (DNs, not links so no
# backlink). Samba does not maintain this string
# otherwise, so we don't increment error_count.
if reverse_link_name is None:
- if str(res[0].dn) != str(dsdb_dn.dn):
- self.err_dn_string_component_old(obj.dn, attrname, val, dsdb_dn,
- res[0].dn)
+ if linkID == 0 and str(res[0].dn) != str(dsdb_dn.dn):
+ # Pass in the old/bad DN without the <GUID=...> part,
+ # otherwise the LDB code will correct it on the way through
+ # (Note: we still want to preserve the DSDB DN prefix in the
+ # case of binary DNs)
+ bad_dn = dsdb_dn.prefix + dsdb_dn.dn.get_linearized()
+ self.err_dn_string_component_old(obj.dn, attrname, bad_dn,
+ dsdb_dn, res[0].dn)
continue
# check the reverse_link is correct if there should be one
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index d10c7ec..a25f341 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1582,6 +1582,7 @@ MODULE_INIT_FUNC(dsdb)
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
+ ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 7395f52..c4a41a2 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -3333,6 +3333,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
const struct ldb_message_element *guid_el = NULL;
struct ldb_control *sd_propagation_control;
struct ldb_control *fix_links_control = NULL;
+ struct ldb_control *fix_dn_name_control = NULL;
struct replmd_private *replmd_private =
talloc_get_type(ldb_module_get_private(module), struct replmd_private);
@@ -3391,6 +3392,69 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
return ldb_next_request(module, req);
}
+ fix_dn_name_control = ldb_request_get_control(req,
+ DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
+ if (fix_dn_name_control != NULL) {
+ struct dsdb_schema *schema = NULL;
+ const struct dsdb_attribute *sa = NULL;
+
+ if (req->op.mod.message->num_elements != 2) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[0].flags != LDB_FLAG_MOD_DELETE) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[1].flags != LDB_FLAG_MOD_ADD) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[0].num_values != 1) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[1].num_values != 1) {
+ return ldb_module_operr(module);
+ }
+
+ schema = dsdb_get_schema(ldb, req);
+ if (schema == NULL) {
+ return ldb_module_operr(module);
+ }
+
+ if (ldb_attr_cmp(req->op.mod.message->elements[0].name,
+ req->op.mod.message->elements[1].name) != 0) {
+ return ldb_module_operr(module);
+ }
+
+ sa = dsdb_attribute_by_lDAPDisplayName(schema,
+ req->op.mod.message->elements[0].name);
+ if (sa == NULL) {
+ return ldb_module_operr(module);
+ }
+
+ if (sa->dn_format == DSDB_INVALID_DN) {
+ return ldb_module_operr(module);
+ }
+
+ if (sa->linkID != 0) {
+ return ldb_module_operr(module);
+ }
+
+ /*
+ * If we are run from dbcheck and we are not updating
+ * a link (as these would need to be sorted and so
+ * can't go via such a simple update, then do not
+ * trigger replicated updates and a new USN from this
+ * change, it wasn't a real change, just a new
+ * (correct) string DN
+ */
+
+ fix_dn_name_control->critical = false;
+ return ldb_next_request(module, req);
+ }
+
ldb_debug(ldb, LDB_DEBUG_TRACE, "replmd_modify\n");
guid_el = ldb_msg_find_element(req->op.mod.message, "objectGUID");
diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
index 28d1b5c..805a1f6 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -132,6 +132,9 @@ struct dsdb_control_password_change {
/* passed by dbcheck to fix duplicate linked attributes (bug #13095) */
#define DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS "1.3.6.1.4.1.7165.4.3.19.2"
+/* passed by dbcheck to fix the DN strong of a one-way-link (bug #13495) */
+#define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME "1.3.6.1.4.1.7165.4.3.19.3"
+
/* passed when importing plain text password on upgrades */
#define DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID "1.3.6.1.4.1.7165.4.3.20"
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
new file mode 100644
index 0000000..b2142df
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
@@ -0,0 +1,19 @@
+# record 1
+dn: OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# record 2
+dn: OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp
+seeAlso: OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# Referral
+ref: ldap:///CN=Configuration,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# Referral
+ref: ldap:///DC=DomainDnsZones,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# Referral
+ref: ldap:///DC=ForestDnsZones,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# returned 5 records
+# 2 entries
+# 3 referrals
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
new file mode 100644
index 0000000..69d1e51
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
@@ -0,0 +1,5 @@
+Checking 228 objects
+NOTE: old (due to rename or delete) DN string component for seeAlso in object OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp - OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
+Change DN to <GUID=20600e7c-92bb-492e-9552-f3ed7f8a2cad>;OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
+Fixed old DN string on attribute seeAlso
+Checked 228 objects (0 errors)
diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh
index 778edf0..13811dd 100755
--- a/testprogs/blackbox/dbcheck-links.sh
+++ b/testprogs/blackbox/dbcheck-links.sh
@@ -205,6 +205,67 @@ check_expected_after_dbcheck_forward_link_corruption() {
fi
}
+oneway_link_corruption() {
+ #
+ # Step1: add OU "dangling-ou"
+ #
+ ldif=$PREFIX_ABS/${RELEASE}/oneway_link_corruption.ldif
+ cat > $ldif <<EOF
+dn: OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
+changetype: add
+objectclass: organizationalUnit
+objectGUID: 20600e7c-92bb-492e-9552-f3ed7f8a2cad
+EOF
+
+ out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
+ if [ "$?" != "0" ]; then
+ echo "ldbmodify returned:\n$out"
+ return 1
+ fi
+
+ #
+ # Step2: add msExchConfigurationContainer "dangling-msexch"
+ #
+ ldif=$PREFIX_ABS/${RELEASE}/oneway_link_corruption2.ldif
+ cat > $ldif <<EOF
+dn: OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp
+changetype: add
+objectclass: organizationalUnit
+seeAlso: OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
+EOF
+
+ out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif)
+ if [ "$?" != "0" ]; then
+ echo "ldbmodify returned:\n$out"
+ return 1
+ fi
+
+ #
+ # Step3: rename dangling-ou to dangling-ou2
+ #
+ # Because this is a one-way link we don't fix it at runtime
+ #
+ out=$(TZ=UTC $ldbrename -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp)
+ if [ "$?" != "0" ]; then
+ echo "ldbmodify returned:\n$out"
+ return 1
+ fi
+}
+
+dbcheck_oneway_link_corruption() {
+ dbcheck "-oneway-link-corruption" "0" ""
+ return $?
+}
+
+check_expected_after_dbcheck_oneway_link_corruption() {
+ tmpldif=$PREFIX_ABS/$RELEASE/expected-after-dbcheck-oneway-link-corruption.ldif.tmp
+ TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(|(ou=dangling-ou)(ou=dangling-ou2)(ou=dangling-from))' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --sorted seeAlso > $tmpldif
+ diff $tmpldif $release_dir/expected-after-dbcheck-oneway-link-corruption.ldif
+ if [ "$?" != "0" ]; then
+ return 1
+ fi
+}
+
dbcheck_dangling_multi_valued() {
$PYTHON $BINDIR/samba-tool dbcheck -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --fix --yes
@@ -276,6 +337,10 @@ if [ -d $release_dir ]; then
testit "dbcheck_forward_link_corruption" dbcheck_forward_link_corruption
testit "check_expected_after_dbcheck_forward_link_corruption" check_expected_after_dbcheck_forward_link_corruption
testit "forward_link_corruption_clean" dbcheck_clean
+ testit "oneway_link_corruption" oneway_link_corruption
+ testit "dbcheck_oneway_link_corruption" dbcheck_oneway_link_corruption
+ testit "check_expected_after_dbcheck_oneway_link_corruption" check_expected_after_dbcheck_oneway_link_corruption
+ testit "oneway_link_corruption_clean" dbcheck_clean
testit "dangling_one_way_link" dangling_one_way_link
testit "dbcheck_one_way" dbcheck_one_way
testit "dbcheck_clean2" dbcheck_clean
--
2.7.4
From c9928e44c6d626e44ba3fbdf828977499da0e35d Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 11 Jun 2018 09:14:06 +1200
Subject: [PATCH 3/7] provision: Small refactor to host-IP logic
Split out the code that determines the host-IP of the new server into
separate functions. This will allow us to re-use the same logic in the
backup/restore case.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/provision/__init__.py | 54 ++++++++++++++++++++++----------------
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index e571894..8bdb95c 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -2042,6 +2042,35 @@ def directory_create_or_exists(path, mode=0o755):
else:
raise ProvisioningError("Failed to create directory %s: %s" % (path, e.strerror))
+def determine_host_ip(logger, lp, hostip=None):
+ if hostip is None:
+ logger.info("Looking up IPv4 addresses")
+ hostips = interface_ips_v4(lp)
+ if len(hostips) > 0:
+ hostip = hostips[0]
+ if len(hostips) > 1:
+ logger.warning("More than one IPv4 address found. Using %s",
+ hostip)
+ if hostip == "127.0.0.1":
+ hostip = None
+ if hostip is None:
+ logger.warning("No IPv4 address will be assigned")
+
+ return hostip
+
+def determine_host_ip6(logger, lp, hostip6=None):
+ if hostip6 is None:
+ logger.info("Looking up IPv6 addresses")
+ hostips = interface_ips_v6(lp)
+ if hostips:
+ hostip6 = hostips[0]
+ if len(hostips) > 1:
+ logger.warning("More than one IPv6 address found. Using %s", hostip6)
+ if hostip6 is None:
+ logger.warning("No IPv6 address will be assigned")
+
+ return hostip6
+
def provision(logger, session_info, smbconf=None,
targetdir=None, samdb_fill=FILL_FULL, realm=None, rootdn=None,
domaindn=None, schemadn=None, configdn=None, serverdn=None,
@@ -2149,29 +2178,8 @@ def provision(logger, session_info, smbconf=None,
paths.root_uid = root_uid;
paths.root_gid = root_gid
- if hostip is None:
- logger.info("Looking up IPv4 addresses")
- hostips = interface_ips_v4(lp)
- if len(hostips) > 0:
- hostip = hostips[0]
- if len(hostips) > 1:
- logger.warning("More than one IPv4 address found. Using %s",
- hostip)
- if hostip == "127.0.0.1":
- hostip = None
- if hostip is None:
- logger.warning("No IPv4 address will be assigned")
-
- if hostip6 is None:
- logger.info("Looking up IPv6 addresses")
- hostips = interface_ips_v6(lp)
- if hostips:
- hostip6 = hostips[0]
- if len(hostips) > 1:
- logger.warning("More than one IPv6 address found. Using %s", hostip6)
- if hostip6 is None:
- logger.warning("No IPv6 address will be assigned")
-
+ hostip = determine_host_ip(logger, lp, hostip)
+ hostip6 = determine_host_ip6(logger, lp, hostip6)
names.hostip = hostip
names.hostip6 = hostip6
names.domainguid = domainguid
--
2.7.4
From 0ccbb46aa31ed9f91b562fd60e90fe0540bc4f12 Mon Sep 17 00:00:00 2001
From: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Date: Tue, 1 May 2018 11:10:11 +1200
Subject: [PATCH 4/7] join: Pipe through dns_backend option for clones
Allow join_clone() calls to specify a dns_backend parameter for the new
cloned DB.
Signed-off-by: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/join.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/python/samba/join.py b/python/samba/join.py
index 30ecce7..03981a1 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -1495,10 +1495,11 @@ def join_DC(logger=None, server=None, creds=None, lp=None, site=None, netbios_na
logger.info("Joined domain %s (SID %s) as a DC" % (ctx.domain_name, ctx.domsid))
def join_clone(logger=None, server=None, creds=None, lp=None,
- targetdir=None, domain=None, include_secrets=False):
+ targetdir=None, domain=None, include_secrets=False,
+ dns_backend="NONE"):
"""Join as a DC."""
ctx = dc_join(logger, server, creds, lp, site=None, netbios_name=None, targetdir=targetdir, domain=domain,
- machinepass=None, use_ntvfs=False, dns_backend="NONE", promote_existing=False, clone_only=True)
+ machinepass=None, use_ntvfs=False, dns_backend=dns_backend, promote_existing=False, clone_only=True)
lp.set("workgroup", ctx.domain_name)
logger.info("workgroup is %s" % ctx.domain_name)
--
2.7.4
From 9977f0d9924c3e2ba6a4644f63f72317ba9f2b32 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 25 Jun 2018 17:21:00 +1200
Subject: [PATCH 5/7] join: Rename dc_join() so it looks like an object
dc_join() is creating an object, but it currently looks like it's
just a function call. Rename it to look more object-like.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/join.py | 29 +++++++++++++++++------------
python/samba/tests/join.py | 13 +++++++------
source4/dsdb/tests/python/acl.py | 13 +++++++------
source4/torture/drs/python/repl_rodc.py | 20 +++++++++++++-------
4 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/python/samba/join.py b/python/samba/join.py
index 03981a1..9635a6b 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -50,7 +50,7 @@ class DCJoinException(Exception):
super(DCJoinException, self).__init__("Can't join, error: %s" % msg)
-class dc_join(object):
+class DCJoinContext(object):
"""Perform a DC join."""
def __init__(ctx, logger=None, server=None, creds=None, lp=None, site=None,
@@ -1418,9 +1418,10 @@ def join_RODC(logger=None, server=None, creds=None, lp=None, site=None, netbios_
backend_store=None):
"""Join as a RODC."""
- ctx = dc_join(logger, server, creds, lp, site, netbios_name, targetdir, domain,
- machinepass, use_ntvfs, dns_backend, promote_existing,
- plaintext_secrets, backend_store=backend_store)
+ ctx = DCJoinContext(logger, server, creds, lp, site, netbios_name,
+ targetdir, domain, machinepass, use_ntvfs, dns_backend,
+ promote_existing, plaintext_secrets,
+ backend_store=backend_store)
lp.set("workgroup", ctx.domain_name)
logger.info("workgroup is %s" % ctx.domain_name)
@@ -1470,9 +1471,10 @@ def join_DC(logger=None, server=None, creds=None, lp=None, site=None, netbios_na
promote_existing=False, plaintext_secrets=False,
backend_store=None):
"""Join as a DC."""
- ctx = dc_join(logger, server, creds, lp, site, netbios_name, targetdir, domain,
- machinepass, use_ntvfs, dns_backend, promote_existing,
- plaintext_secrets, backend_store=backend_store)
+ ctx = DCJoinContext(logger, server, creds, lp, site, netbios_name,
+ targetdir, domain, machinepass, use_ntvfs, dns_backend,
+ promote_existing, plaintext_secrets,
+ backend_store=backend_store)
lp.set("workgroup", ctx.domain_name)
logger.info("workgroup is %s" % ctx.domain_name)
@@ -1498,8 +1500,10 @@ def join_clone(logger=None, server=None, creds=None, lp=None,
targetdir=None, domain=None, include_secrets=False,
dns_backend="NONE"):
"""Join as a DC."""
- ctx = dc_join(logger, server, creds, lp, site=None, netbios_name=None, targetdir=targetdir, domain=domain,
- machinepass=None, use_ntvfs=False, dns_backend=dns_backend, promote_existing=False, clone_only=True)
+ ctx = DCJoinContext(logger, server, creds, lp, site=None, netbios_name=None,
+ targetdir=targetdir, domain=domain, machinepass=None,
+ use_ntvfs=False, dns_backend=dns_backend,
+ promote_existing=False, clone_only=True)
lp.set("workgroup", ctx.domain_name)
logger.info("workgroup is %s" % ctx.domain_name)
@@ -1522,9 +1526,10 @@ def join_subdomain(logger=None, server=None, creds=None, lp=None, site=None,
dns_backend=None, plaintext_secrets=False,
backend_store=None):
"""Join as a DC."""
- ctx = dc_join(logger, server, creds, lp, site, netbios_name, targetdir, parent_domain,
- machinepass, use_ntvfs, dns_backend, plaintext_secrets,
- backend_store=backend_store)
+ ctx = DCJoinContext(logger, server, creds, lp, site, netbios_name,
+ targetdir, parent_domain, machinepass, use_ntvfs,
+ dns_backend, plaintext_secrets,
+ backend_store=backend_store)
ctx.subdomain = True
if adminpass is None:
ctx.adminpass = samba.generate_random_password(12, 32)
diff --git a/python/samba/tests/join.py b/python/samba/tests/join.py
index 17de3ab..31b8921 100644
--- a/python/samba/tests/join.py
+++ b/python/samba/tests/join.py
@@ -21,7 +21,7 @@ import sys
import shutil
import os
from samba.tests.dns_base import DNSTKeyTest
-from samba.join import dc_join
+from samba.join import DCJoinContext
from samba.dcerpc import drsuapi, misc, dns
from samba.credentials import Credentials
@@ -42,11 +42,12 @@ class JoinTestCase(DNSTKeyTest):
self.netbios_name = "jointest1"
logger = get_logger()
- self.join_ctx = dc_join(server=self.server, creds=self.creds, lp=self.get_loadparm(),
- netbios_name=self.netbios_name,
- targetdir=self.tempdir,
- domain=None, logger=logger,
- dns_backend="SAMBA_INTERNAL")
+ self.join_ctx = DCJoinContext(server=self.server, creds=self.creds,
+ lp=self.get_loadparm(),
+ netbios_name=self.netbios_name,
+ targetdir=self.tempdir,
+ domain=None, logger=logger,
+ dns_backend="SAMBA_INTERNAL")
self.join_ctx.userAccountControl = (samba.dsdb.UF_SERVER_TRUST_ACCOUNT |
samba.dsdb.UF_TRUSTED_FOR_DELEGATION)
diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py
index 29fcf55..c698bf1 100755
--- a/source4/dsdb/tests/python/acl.py
+++ b/source4/dsdb/tests/python/acl.py
@@ -13,7 +13,7 @@ import samba
from samba.tests.subunitrun import SubunitOptions, TestProgram
import samba.getopt as options
-from samba.join import dc_join
+from samba.join import DCJoinContext
from ldb import (
SCOPE_BASE, SCOPE_SUBTREE, LdbError, ERR_NO_SUCH_OBJECT,
@@ -1841,11 +1841,12 @@ class AclSPNTests(AclTests):
self.computerdn = "CN=%s,CN=computers,%s" % (self.computername, self.base_dn)
self.dc_dn = "CN=%s,OU=Domain Controllers,%s" % (self.dcname, self.base_dn)
self.site = "Default-First-Site-Name"
- self.rodcctx = dc_join(server=host, creds=creds, lp=lp,
- site=self.site, netbios_name=self.rodcname, targetdir=None,
- domain=None)
- self.dcctx = dc_join(server=host, creds=creds, lp=lp, site=self.site,
- netbios_name=self.dcname, targetdir=None, domain=None)
+ self.rodcctx = DCJoinContext(server=host, creds=creds, lp=lp,
+ site=self.site, netbios_name=self.rodcname,
+ targetdir=None, domain=None)
+ self.dcctx = DCJoinContext(server=host, creds=creds, lp=lp,
+ site=self.site, netbios_name=self.dcname,
+ targetdir=None, domain=None)
self.ldb_admin.newuser(self.test_user, self.user_pass)
self.ldb_user1 = self.get_ldb_connection(self.test_user, self.user_pass)
self.user_sid1 = self.sd_utils.get_object_sid(self.get_user_dn(self.test_user))
diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py
index 1d84c99..a785932 100644
--- a/source4/torture/drs/python/repl_rodc.py
+++ b/source4/torture/drs/python/repl_rodc.py
@@ -33,7 +33,7 @@ import ldb
from ldb import SCOPE_BASE
from samba import WERRORError
-from samba.join import dc_join
+from samba.join import DCJoinContext
from samba.dcerpc import drsuapi, misc, drsblobs, security
from samba.drs_utils import drs_DsBind, drs_Replicate
from samba.ndr import ndr_unpack, ndr_pack
@@ -101,9 +101,12 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase):
self.computer_dn = "CN=%s,OU=Domain Controllers,%s" % (self.rodc_name, self.base_dn)
- self.rodc_ctx = dc_join(server=self.ldb_dc1.host_dns_name(), creds=self.get_credentials(), lp=self.get_loadparm(),
- site=self.site, netbios_name=self.rodc_name,
- targetdir=None, domain=None, machinepass=self.rodc_pass)
+ self.rodc_ctx = DCJoinContext(server=self.ldb_dc1.host_dns_name(),
+ creds=self.get_credentials(),
+ lp=self.get_loadparm(), site=self.site,
+ netbios_name=self.rodc_name,
+ targetdir=None, domain=None,
+ machinepass=self.rodc_pass)
self._create_rodc(self.rodc_ctx)
self.rodc_ctx.create_tmp_samdb()
self.tmp_samdb = self.rodc_ctx.tmp_samdb
@@ -459,9 +462,12 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase):
"""
# Create a new identical RODC with just the first letter missing
other_rodc_name = self.rodc_name[1:]
- other_rodc_ctx = dc_join(server=self.ldb_dc1.host_dns_name(), creds=self.get_credentials(), lp=self.get_loadparm(),
- site=self.site, netbios_name=other_rodc_name,
- targetdir=None, domain=None, machinepass=self.rodc_pass)
+ other_rodc_ctx = DCJoinContext(server=self.ldb_dc1.host_dns_name(),
+ creds=self.get_credentials(),
+ lp=self.get_loadparm(), site=self.site,
+ netbios_name=other_rodc_name,
+ targetdir=None, domain=None,
+ machinepass=self.rodc_pass)
self._create_rodc(other_rodc_ctx)
other_rodc_creds = Credentials()
--
2.7.4
From f019a12583c83f8f6fb2efd6bd0e5aaa87ca5541 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 11 Jun 2018 16:33:19 +1200
Subject: [PATCH 6/7] join: Refactor clone_only case to simplify code
Currently for DC clones, we create a regular DCJoinContext, se a
'clone_only' flag, and then make lots of special checks for this flag
throughout the code. Instead, we can use inheritance to create a
DCCloneContext sub-class, and put the specialization there.
This means we can remove all the 'clone_only' checks from the code. The
only 2 methods that really differ are do_join() and join_finalize(), and
these don't share much code at all. (To avoid duplication, I split the
first part of do_join() into a new build_nc_lists() function, but this
is a pretty trivial code move).
We still pass the clone_only flag into the __init__() as there's still
one case where we want to avoid doing work in the case of the clone.
For clarity, I'll refactor this in a subsequent patch.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/join.py | 124 +++++++++++++++++++++++++++++++--------------------
1 file changed, 75 insertions(+), 49 deletions(-)
diff --git a/python/samba/join.py b/python/samba/join.py
index 9635a6b..51a82f2 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -61,8 +61,6 @@ class DCJoinContext(object):
if site is None:
site = "Default-First-Site-Name"
- ctx.clone_only=clone_only
-
ctx.logger = logger
ctx.creds = creds
ctx.lp = lp
@@ -119,18 +117,7 @@ class DCJoinContext(object):
ctx.acct_pass = samba.generate_random_machine_password(128, 255)
ctx.dnsdomain = ctx.samdb.domain_dns_name()
- if clone_only:
- # As we don't want to create or delete these DNs, we set them to None
- ctx.server_dn = None
- ctx.ntds_dn = None
- ctx.acct_dn = None
- ctx.myname = ctx.server.split('.')[0]
- ctx.ntds_guid = None
- ctx.rid_manager_dn = None
-
- # Save this early
- ctx.remote_dc_ntds_guid = ctx.samdb.get_ntds_GUID()
- else:
+ if not clone_only:
# work out the DNs of all the objects we will be adding
ctx.myname = netbios_name
ctx.samname = "%s$" % ctx.myname
@@ -1188,12 +1175,11 @@ class DCJoinContext(object):
# DC we just replicated from then we don't need to send the updatereplicateref
# as replication between sites is time based and on the initiative of the
# requesting DC
- if not ctx.clone_only:
- ctx.logger.info("Sending DsReplicaUpdateRefs for all the replicated partitions")
- for nc in ctx.nc_list:
- ctx.send_DsReplicaUpdateRefs(nc)
+ ctx.logger.info("Sending DsReplicaUpdateRefs for all the replicated partitions")
+ for nc in ctx.nc_list:
+ ctx.send_DsReplicaUpdateRefs(nc)
- if not ctx.clone_only and ctx.RODC:
+ if ctx.RODC:
print("Setting RODC invocationId")
ctx.local_samdb.set_invocation_id(str(ctx.invocation_id))
ctx.local_samdb.set_opaque_integer("domainFunctionality",
@@ -1224,17 +1210,12 @@ class DCJoinContext(object):
m.dn = ldb.Dn(ctx.local_samdb, '@ROOTDSE')
m["isSynchronized"] = ldb.MessageElement("TRUE", ldb.FLAG_MOD_REPLACE, "isSynchronized")
- # We want to appear to be the server we just cloned
- if ctx.clone_only:
- guid = ctx.remote_dc_ntds_guid
- else:
- guid = ctx.ntds_guid
-
+ guid = ctx.ntds_guid
m["dsServiceName"] = ldb.MessageElement("<GUID=%s>" % str(guid),
ldb.FLAG_MOD_REPLACE, "dsServiceName")
ctx.local_samdb.modify(m)
- if ctx.clone_only or ctx.subdomain:
+ if ctx.subdomain:
return
secrets_ldb = Ldb(ctx.paths.secrets, session_info=system_session(), lp=ctx.lp)
@@ -1359,7 +1340,7 @@ class DCJoinContext(object):
ctx.local_samdb.add(rec)
- def do_join(ctx):
+ def build_nc_lists(ctx):
# nc_list is the list of naming context (NC) for which we will
# replicate in and send a updateRef command to the partner DC
@@ -1380,23 +1361,24 @@ class DCJoinContext(object):
ctx.full_nc_list += [ctx.domaindns_zone]
ctx.full_nc_list += [ctx.forestdns_zone]
- if not ctx.clone_only:
- if ctx.promote_existing:
- ctx.promote_possible()
- else:
- ctx.cleanup_old_join()
+ def do_join(ctx):
+ ctx.build_nc_lists()
+
+ if ctx.promote_existing:
+ ctx.promote_possible()
+ else:
+ ctx.cleanup_old_join()
try:
- if not ctx.clone_only:
- ctx.join_add_objects()
+ ctx.join_add_objects()
ctx.join_provision()
ctx.join_replicate()
- if (not ctx.clone_only and ctx.subdomain):
+ if ctx.subdomain:
ctx.join_add_objects2()
ctx.join_provision_own_domain()
ctx.join_setup_trusts()
- if not ctx.clone_only and ctx.dns_backend != "NONE":
+ if ctx.dns_backend != "NONE":
ctx.join_add_dns_records()
ctx.join_replicate_new_dns_records()
@@ -1406,8 +1388,7 @@ class DCJoinContext(object):
print("Join failed - cleaning up")
except IOError:
pass
- if not ctx.clone_only:
- ctx.cleanup_old_join()
+ ctx.cleanup_old_join()
raise
@@ -1499,11 +1480,10 @@ def join_DC(logger=None, server=None, creds=None, lp=None, site=None, netbios_na
def join_clone(logger=None, server=None, creds=None, lp=None,
targetdir=None, domain=None, include_secrets=False,
dns_backend="NONE"):
- """Join as a DC."""
- ctx = DCJoinContext(logger, server, creds, lp, site=None, netbios_name=None,
- targetdir=targetdir, domain=domain, machinepass=None,
- use_ntvfs=False, dns_backend=dns_backend,
- promote_existing=False, clone_only=True)
+ """Creates a local clone of a remote DC."""
+ ctx = DCCloneContext(logger, server, creds, lp, targetdir=targetdir,
+ domain=domain, dns_backend=dns_backend,
+ include_secrets=include_secrets)
lp.set("workgroup", ctx.domain_name)
logger.info("workgroup is %s" % ctx.domain_name)
@@ -1511,12 +1491,6 @@ def join_clone(logger=None, server=None, creds=None, lp=None,
lp.set("realm", ctx.realm)
logger.info("realm is %s" % ctx.realm)
- ctx.replica_flags |= (drsuapi.DRSUAPI_DRS_WRIT_REP |
- drsuapi.DRSUAPI_DRS_FULL_SYNC_IN_PROGRESS)
- if not include_secrets:
- ctx.replica_flags |= drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING
- ctx.domain_replica_flags = ctx.replica_flags
-
ctx.do_join()
logger.info("Cloned domain %s (SID %s)" % (ctx.domain_name, ctx.domsid))
@@ -1573,3 +1547,55 @@ def join_subdomain(logger=None, server=None, creds=None, lp=None, site=None,
ctx.do_join()
ctx.logger.info("Created domain %s (SID %s) as a DC" % (ctx.domain_name, ctx.domsid))
+
+
+class DCCloneContext(DCJoinContext):
+ """Clones a remote DC."""
+
+ def __init__(ctx, logger=None, server=None, creds=None, lp=None,
+ targetdir=None, domain=None, dns_backend=None,
+ include_secrets=False):
+ super(DCCloneContext, ctx).__init__(logger, server, creds, lp,
+ targetdir=targetdir, domain=domain,
+ dns_backend=dns_backend,
+ clone_only=True)
+
+ # As we don't want to create or delete these DNs, we set them to None
+ ctx.server_dn = None
+ ctx.ntds_dn = None
+ ctx.acct_dn = None
+ ctx.myname = ctx.server.split('.')[0]
+ ctx.ntds_guid = None
+ ctx.rid_manager_dn = None
+
+ # Save this early
+ ctx.remote_dc_ntds_guid = ctx.samdb.get_ntds_GUID()
+
+ ctx.replica_flags |= (drsuapi.DRSUAPI_DRS_WRIT_REP |
+ drsuapi.DRSUAPI_DRS_FULL_SYNC_IN_PROGRESS)
+ if not include_secrets:
+ ctx.replica_flags |= drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING
+ ctx.domain_replica_flags = ctx.replica_flags
+
+ def join_finalise(ctx):
+ ctx.logger.info("Setting isSynchronized and dsServiceName")
+ m = ldb.Message()
+ m.dn = ldb.Dn(ctx.local_samdb, '@ROOTDSE')
+ m["isSynchronized"] = ldb.MessageElement("TRUE", ldb.FLAG_MOD_REPLACE,
+ "isSynchronized")
+
+ # We want to appear to be the server we just cloned
+ guid = ctx.remote_dc_ntds_guid
+ m["dsServiceName"] = ldb.MessageElement("<GUID=%s>" % str(guid),
+ ldb.FLAG_MOD_REPLACE,
+ "dsServiceName")
+ ctx.local_samdb.modify(m)
+
+ def do_join(ctx):
+ ctx.build_nc_lists()
+
+ # When cloning a DC, we just want to provision a DC locally, then
+ # grab the remote DC's entire DB via DRS replication
+ ctx.join_provision()
+ ctx.join_replicate()
+ ctx.join_finalise()
--
2.7.4
From 1ab0261ebc13f5969447e6f3f13827234a36cc6c Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 29 Jun 2018 10:40:58 +1200
Subject: [PATCH 7/7] join: Remove unnecessary clone_only flag
For the clone-only case, we have been avoiding a block of code in the
DCJoinContext's __init__(). The main reason we do this is because the
netbios_name is None for clones, and this block of code tries to derive
a bunch of values based on the netbios_name (otherwise, a few lines into
this block, it tries to do NoneType.lower(), which Python doesn't like
very much).
This code is not particularly clone-specific - it is just never going to
work if the netbios_name is None. So we can change the conditional
check, which allows us to get rid of the clone_only flag.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/join.py | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/python/samba/join.py b/python/samba/join.py
index 51a82f2..b7ab1ed 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -56,8 +56,8 @@ class DCJoinContext(object):
def __init__(ctx, logger=None, server=None, creds=None, lp=None, site=None,
netbios_name=None, targetdir=None, domain=None,
machinepass=None, use_ntvfs=False, dns_backend=None,
- promote_existing=False, clone_only=False,
- plaintext_secrets=False, backend_store=None):
+ promote_existing=False, plaintext_secrets=False,
+ backend_store=None):
if site is None:
site = "Default-First-Site-Name"
@@ -117,7 +117,10 @@ class DCJoinContext(object):
ctx.acct_pass = samba.generate_random_machine_password(128, 255)
ctx.dnsdomain = ctx.samdb.domain_dns_name()
- if not clone_only:
+
+ # the following are all dependent on the new DC's netbios_name (which
+ # we expect to always be specified, except when cloning a DC)
+ if netbios_name:
# work out the DNs of all the objects we will be adding
ctx.myname = netbios_name
ctx.samname = "%s$" % ctx.myname
@@ -1557,8 +1560,7 @@ class DCCloneContext(DCJoinContext):
include_secrets=False):
super(DCCloneContext, ctx).__init__(logger, server, creds, lp,
targetdir=targetdir, domain=domain,
- dns_backend=dns_backend,
- clone_only=True)
+ dns_backend=dns_backend)
# As we don't want to create or delete these DNs, we set them to None
ctx.server_dn = None
--
2.7.4
More information about the samba-technical
mailing list