[PATCH] Fix credentials for domain backup commands (Bug 13566)
Tim Beale
timbeale at catalyst.net.nz
Fri Aug 10 02:09:00 UTC 2018
Updated patch. I reworked the top patch - there's a better solution
where we can reset the admin's password based on RID (thanks for the tip
Andrew).
New CI link: https://gitlab.com/catalyst-samba/samba/pipelines/27595972
On 10/08/18 09:53, Tim Beale via samba-technical wrote:
> Hi,
>
> These patches fix some problems I noticed with the new domain backup
> commands. Basically, the commands would only ever work if you specified
> the administrator's password on the command line. Using credential
> options like --kerberos=yes didn't work.
>
> Clean CI run here:
> https://gitlab.com/catalyst-samba/samba/pipelines/27488840
>
> Thanks,
> Tim
>
-------------- next part --------------
From f07cc37a7c2f89c5391011d11bdb7f2771e514c5 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 9 Aug 2018 15:30:55 +1200
Subject: [PATCH 1/4] 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 at catalyst.net.nz>
---
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 05146c0..c251bc6 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -156,10 +156,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:
@@ -218,12 +218,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
@@ -686,8 +686,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()
@@ -701,8 +704,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 7fbf3c070d523c6146fe1f045be724f41baa0192 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 9 Aug 2018 15:34:51 +1200
Subject: [PATCH 2/4] 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 at catalyst.net.nz>
---
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 c251bc6..cb00e46 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -158,10 +158,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 e5721754952d97ca78e25f2da4bf21dee2f2d558 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 9 Aug 2018 15:35:59 +1200
Subject: [PATCH 3/4] 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 at catalyst.net.nz>
---
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 cb00e46..cf2327e 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -156,14 +156,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):
@@ -218,7 +210,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)
@@ -686,7 +681,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 4771d00c6fa4e8f7b43147f05d89a78f9e9bd93b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 9 Aug 2018 16:20:10 +1200
Subject: [PATCH 4/4] 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 at catalyst.net.nz>
---
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 cf2327e..ae04ec1 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -33,7 +33,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
@@ -158,16 +158,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)
@@ -250,7 +259,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)
@@ -762,7 +771,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
More information about the samba-technical
mailing list