Attachment 14424 Details for Bug 13566 – Backport for v4.9

Backport for v4.9

patch.txt (text/plain), 13.92 KB, created by Tim Beale on 2018-08-16 00:39:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Beale

Created: 2018-08-16 00:39:31 UTC

Size: 13.92 KB

patch

obsolete

>From d51b1bc9129fae1c182e8c84e760582f9770671c Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 12 Jul 2018 16:13:27 +1200
>Subject: [PATCH 1/5] netcmd: Improve domain backup targetdir checks
>
>+ Added check that specified targetdir is actually a directory (if it
>exists)
>+ Deleted a redundant 'Creating targetdir' check that would never be hit
>+ Move code into a separate function so we can reuse it for offline
>backups (which take a different set of parameters, but still have a
>targetdir)
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>(cherry picked from commit 4f532cc177cd1e95d8ccf8e69f50b315354df34c)
>
>Backported to v4.9 for:
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13566
>---
> python/samba/netcmd/domain_backup.py | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index cfd9796..f7b8edb 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -139,6 +139,17 @@ def add_backup_marker(samdb, marker, value):
>     samdb.modify(m)
> 
> 
>+def check_targetdir(logger, targetdir):
>+    if targetdir is None:
>+        raise CommandError('Target directory required')
>+
>+    if not os.path.exists(targetdir):
>+        logger.info('Creating targetdir %s...' % targetdir)
>+        os.makedirs(targetdir)
>+    elif not os.path.isdir(targetdir):
>+        raise CommandError("%s is not a directory" % targetdir)
>+
>+
> def check_online_backup_args(logger, credopts, server, targetdir):
>     # Make sure we have all the required args.
>     u_p = {'user': credopts.creds.get_username(),
>@@ -147,12 +158,8 @@ def check_online_backup_args(logger, credopts, server, targetdir):
>         raise CommandError("Creds required.")
>     if server is None:
>         raise CommandError('Server required')
>-    if targetdir is None:
>-        raise CommandError('Target directory required')
> 
>-    if not os.path.exists(targetdir):
>-        logger.info('Creating targetdir %s...' % targetdir)
>-        os.makedirs(targetdir)
>+    check_targetdir(logger, targetdir)
> 
> 
> # For '--no-secrets' backups, this sets the Administrator user's password to a
>@@ -211,10 +218,6 @@ class cmd_domain_backup_online(samba.netcmd.Command):
>         lp = sambaopts.get_loadparm()
>         creds = credopts.get_credentials(lp)
> 
>-        if not os.path.exists(targetdir):
>-            logger.info('Creating targetdir %s...' % targetdir)
>-            os.makedirs(targetdir)
>-
>         tmpdir = tempfile.mkdtemp(dir=targetdir)
> 
>         # Run a clone join on the remote
>-- 
>2.7.4
>
>
>From 5fde4146d5f68f413f6d24c39c5618dc936b5e5a Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 9 Aug 2018 15:30:55 +1200
>Subject: [PATCH 2/5] netcmd: domain backup didn't support prompting for
> password
>
>The online/rename backups only worked if you specified both the username
>and password in the actual command itself. If you just entered the
>username (expecting to be prompted for the password later), then the
>command was rejected.
>
>The problem was the order the code was doing things in. We were checking
>credopts.creds.get_password() *before* we'd called
>credopts.get_credentials(lp), whereas it should be the other way
>around.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13566
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 14077b6682d7dc1b16e1ccb42ef61e9f4c0a1715)
>---
> python/samba/netcmd/domain_backup.py | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index f7b8edb..b16763a 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -150,10 +150,10 @@ def check_targetdir(logger, targetdir):
>         raise CommandError("%s is not a directory" % targetdir)
> 
> 
>-def check_online_backup_args(logger, credopts, server, targetdir):
>+def check_online_backup_args(logger, creds, server, targetdir):
>     # Make sure we have all the required args.
>-    u_p = {'user': credopts.creds.get_username(),
>-           'pass': credopts.creds.get_password()}
>+    u_p = {'user': creds.get_username(),
>+           'pass': creds.get_password()}
>     if None in u_p.values():
>         raise CommandError("Creds required.")
>     if server is None:
>@@ -212,12 +212,12 @@ class cmd_domain_backup_online(samba.netcmd.Command):
>         logger = self.get_logger()
>         logger.setLevel(logging.DEBUG)
> 
>-        # Make sure we have all the required args.
>-        check_online_backup_args(logger, credopts, server, targetdir)
>-
>         lp = sambaopts.get_loadparm()
>         creds = credopts.get_credentials(lp)
> 
>+        # Make sure we have all the required args.
>+        check_online_backup_args(logger, creds, server, targetdir)
>+
>         tmpdir = tempfile.mkdtemp(dir=targetdir)
> 
>         # Run a clone join on the remote
>@@ -680,8 +680,11 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
>         logger = self.get_logger()
>         logger.setLevel(logging.INFO)
> 
>+        lp = sambaopts.get_loadparm()
>+        creds = credopts.get_credentials(lp)
>+
>         # Make sure we have all the required args.
>-        check_online_backup_args(logger, credopts, server, targetdir)
>+        check_online_backup_args(logger, creds, server, targetdir)
>         delete_old_dns = not keep_dns_realm
> 
>         new_dns_realm = new_dns_realm.lower()
>@@ -695,8 +698,6 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
>         tmpdir = tempfile.mkdtemp(dir=targetdir)
> 
>         # setup a join-context for cloning the remote server
>-        lp = sambaopts.get_loadparm()
>-        creds = credopts.get_credentials(lp)
>         include_secrets = not no_secrets
>         ctx = DCCloneAndRenameContext(new_base_dn, new_domain_name,
>                                       new_dns_realm, logger=logger,
>-- 
>2.7.4
>
>
>From 34293881b75c1b8c4d6867b2a6ef9ae6847f46cb Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 9 Aug 2018 15:34:51 +1200
>Subject: [PATCH 3/5] netcmd: Fix kerberos option for domain backups
>
>The previous fix still didn't work if you specified --kerberos=yes (in
>which case the creds still doesn't have a password).
>
>credopts.get_credentials(lp) should be enough to ensure a user/password
>is set (it's all that the other commands seem to do).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13566
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 8fb706c34682bf6dc6033963518c7eccffc3944f)
>---
> python/samba/netcmd/domain_backup.py | 4 ----
> 1 file changed, 4 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index b16763a..b2c8302 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -152,10 +152,6 @@ def check_targetdir(logger, targetdir):
> 
> def check_online_backup_args(logger, creds, server, targetdir):
>     # Make sure we have all the required args.
>-    u_p = {'user': creds.get_username(),
>-           'pass': creds.get_password()}
>-    if None in u_p.values():
>-        raise CommandError("Creds required.")
>     if server is None:
>         raise CommandError('Server required')
> 
>-- 
>2.7.4
>
>
>From c6249cb7a65c56edde0431e3b62a97602b91644c Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 9 Aug 2018 15:35:59 +1200
>Subject: [PATCH 4/5] netcmd: Delete unnecessary function
>
>Minor code cleanup. The last 2 patches gutted this function, to the
>point where there's no longer any value in keeping it.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13566
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit d2d039515119523192676b311d5997afd34f4c90)
>---
> python/samba/netcmd/domain_backup.py | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index b2c8302..8e82cd7 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -150,14 +150,6 @@ def check_targetdir(logger, targetdir):
>         raise CommandError("%s is not a directory" % targetdir)
> 
> 
>-def check_online_backup_args(logger, creds, server, targetdir):
>-    # Make sure we have all the required args.
>-    if server is None:
>-        raise CommandError('Server required')
>-
>-    check_targetdir(logger, targetdir)
>-
>-
> # For '--no-secrets' backups, this sets the Administrator user's password to a
> # randomly-generated value. This is similar to the provision behaviour
> def set_admin_password(logger, samdb, username):
>@@ -212,7 +204,10 @@ class cmd_domain_backup_online(samba.netcmd.Command):
>         creds = credopts.get_credentials(lp)
> 
>         # Make sure we have all the required args.
>-        check_online_backup_args(logger, creds, server, targetdir)
>+        if server is None:
>+            raise CommandError('Server required')
>+
>+        check_targetdir(logger, targetdir)
> 
>         tmpdir = tempfile.mkdtemp(dir=targetdir)
> 
>@@ -680,7 +675,11 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
>         creds = credopts.get_credentials(lp)
> 
>         # Make sure we have all the required args.
>-        check_online_backup_args(logger, creds, server, targetdir)
>+        if server is None:
>+            raise CommandError('Server required')
>+
>+        check_targetdir(logger, targetdir)
>+
>         delete_old_dns = not keep_dns_realm
> 
>         new_dns_realm = new_dns_realm.lower()
>-- 
>2.7.4
>
>
>From f27f1368030c25e318f50481a2332a9d9a4b1345 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Thu, 9 Aug 2018 16:20:10 +1200
>Subject: [PATCH 5/5] netcmd: Fix --kerberos=yes and --no-secrets domain
> backups
>
>The --kerberos=yes and --no-secrets options didn't work in combination
>for domain backups. The problem was creds.get_username() might not
>necessarily match the kerberos user (such as in the selftest
>environment). If this was the case, then trying to reset the admin
>password failed (because the creds.get_username() didn't exist in
>the DB).
>
>Because the admin user always has a fixed RID, we can work out the
>administrator based on its object SID, instead of relying on the
>username in the creds.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13566
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Wed Aug 15 10:19:09 CEST 2018 on sn-devel-144
>
>(cherry picked from commit f249bea1e0538300288e7cf1dcb6037c45f92276)
>---
> python/samba/netcmd/domain_backup.py | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index 8e82cd7..5f18e81 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -32,7 +32,7 @@ from samba.auth import system_session
> from samba.join import DCJoinContext, join_clone, DCCloneAndRenameContext
> from samba.dcerpc.security import dom_sid
> from samba.netcmd import Option, CommandError
>-from samba.dcerpc import misc
>+from samba.dcerpc import misc, security
> from samba import Ldb
> from fsmo import cmd_fsmo_seize
> from samba.provision import make_smbconf
>@@ -152,16 +152,25 @@ def check_targetdir(logger, targetdir):
> 
> # For '--no-secrets' backups, this sets the Administrator user's password to a
> # randomly-generated value. This is similar to the provision behaviour
>-def set_admin_password(logger, samdb, username):
>+def set_admin_password(logger, samdb):
>     """Sets a randomly generated password for the backup DB's admin user"""
> 
>+    # match the admin user by RID
>+    domainsid = samdb.get_domain_sid()
>+    match_admin = "(objectsid={}-{})".format(domainsid,
>+                                             security.DOMAIN_RID_ADMINISTRATOR)
>+    search_expr = "(&(objectClass=user){})".format(match_admin)
>+
>+    # retrieve the admin username (just in case it's been renamed)
>+    res = samdb.search(base=samdb.domain_dn(), scope=ldb.SCOPE_SUBTREE,
>+                       expression=search_expr)
>+    username = str(res[0]['samaccountname'])
>+
>     adminpass = samba.generate_random_password(12, 32)
>     logger.info("Setting %s password in backup to: %s" % (username, adminpass))
>     logger.info("Run 'samba-tool user setpassword %s' after restoring DB" %
>                 username)
>-    samdb.setpassword("(&(objectClass=user)(sAMAccountName=%s))"
>-                      % ldb.binary_encode(username), adminpass,
>-                      force_change_at_next_login=False,
>+    samdb.setpassword(search_expr, adminpass, force_change_at_next_login=False,
>                       username=username)
> 
> 
>@@ -244,7 +253,7 @@ class cmd_domain_backup_online(samba.netcmd.Command):
> 
>         # ensure the admin user always has a password set (same as provision)
>         if no_secrets:
>-            set_admin_password(logger, samdb, creds.get_username())
>+            set_admin_password(logger, samdb)
> 
>         # Add everything in the tmpdir to the backup tar file
>         backup_file = backup_filepath(targetdir, realm, time_str)
>@@ -756,7 +765,7 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
> 
>         # ensure the admin user always has a password set (same as provision)
>         if no_secrets:
>-            set_admin_password(logger, samdb, creds.get_username())
>+            set_admin_password(logger, samdb)
> 
>         # Add everything in the tmpdir to the backup tar file
>         backup_file = backup_filepath(targetdir, new_dns_realm, time_str)
>-- 
>2.7.4
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 13566: 14424