URL: https://github.com/freeipa/freeipa/pull/1911 Author: rcritten Title: #1911: Validate the Directory Manager password in ipa-restore (take 2) Action: opened
PR body: """ Validate the DM password before starting the actual restoration if /etc/ipa/default.conf exists AND dirsrv is running. If default.conf exists and dirsrv is not running raise an error. Add two tests to validate this behavior. https://pagure.io/freeipa/issue/7535 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1911/head:pr1911 git checkout pr1911
From 6296646e9c3426809aa695d0ffca60072d3e3280 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 1 May 2018 10:05:05 -0400 Subject: [PATCH 1/2] Validate the Directory Manager password before starting restore The password was only indirectly validated when trying to disable replication agreements for the restoration. Only validate the password if the IPA configuration is available and dirsrv is running. https://pagure.io/freeipa/issue/7136 https://pagure.io/freeipa/issue/7535 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipaserver/install/ipa_restore.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py index 08eb32fa7f..e7604a6d94 100644 --- a/ipaserver/install/ipa_restore.py +++ b/ipaserver/install/ipa_restore.py @@ -210,6 +210,10 @@ def ask_for_options(self): options = self.options super(Restore, self).ask_for_options() + # no IPA config means we are reinstalling from nothing so + # there is no need for the DM password + if not os.path.exists(paths.IPA_DEFAULT_CONF): + return # get the directory manager password self.dirman_password = options.password if not options.password: @@ -670,7 +674,7 @@ def restore_default_conf(self): ''' Restore paths.IPA_DEFAULT_CONF to temporary directory. - Primary purpose of this method is to get cofiguration for api + Primary purpose of this method is to get configuration for api finalization when restoring ipa after uninstall. ''' cwd = os.getcwd() @@ -884,3 +888,18 @@ def init_api(self, **overrides): self.instances = [installutils.realm_to_serverid(api.env.realm)] self.backends = ['userRoot', 'ipaca'] + + # no IPA config means we are reinstalling from nothing so + # there is nothing to test the DM password against. + if os.path.exists(paths.IPA_DEFAULT_CONF): + instance_name = installutils.realm_to_serverid(api.env.realm) + if not services.knownservices.dirsrv.is_running(instance_name): + raise admintool.ScriptError( + "directory server instance is not running" + ) + try: + ReplicationManager(api.env.realm, api.env.host, + self.dirman_password) + except errors.ACIError: + logger.error("Incorrect Directory Manager password provided") + raise From 8095acba2d421fb9e5acbb262682e555026bea0c Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 8 May 2018 07:44:23 -0400 Subject: [PATCH 2/2] Add tests for ipa-restore with DM password validation check ipa-restore should validate the DM password before executing the restoration. This adds two test cases: 1. Restore with a bad DM password 2. Restore with dirsrv down so password cannot be checked Related: https://pagure.io/freeipa/issue/7136 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- .../test_integration/test_backup_and_restore.py | 36 ++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/ipatests/test_integration/test_backup_and_restore.py b/ipatests/test_integration/test_backup_and_restore.py index c1199f211d..c10e42aead 100644 --- a/ipatests/test_integration/test_backup_and_restore.py +++ b/ipatests/test_integration/test_backup_and_restore.py @@ -482,10 +482,9 @@ def test_full_backup_and_restore_with_replica(self): "systemctl", "disable", "oddjobd" ]) - dirman_password = self.master.config.dirman_password self.master.run_command( ["ipa-restore", backup_path], - stdin_text=dirman_password + '\nyes' + stdin_text='yes' ) status = self.master.run_command([ @@ -547,3 +546,36 @@ def test_userroot_ldif_files_ownership(self): unexp_str = "CRITICAL: db2ldif failed:" assert cmd.returncode == 0 assert unexp_str not in cmd.stdout_text + + +class TestBackupAndRestoreDMPassword(IntegrationTest): + """Negative tests for incorrect DM password""" + topology = 'star' + + def test_restore_bad_dm_password(self): + """backup, uninstall, restore, wrong DM password (expect failure)""" + with restore_checker(self.master): + backup_path = backup(self.master) + + # No uninstall, just pure restore, the only case where + # prompting for the DM password matters. + result = self.master.run_command(['ipa-restore', backup_path], + stdin_text='badpass\nyes', + raiseonerr=False) + assert result.returncode == 1 + + def test_restore_dirsrv_not_running(self): + """backup, restore, dirsrv not running (expect failure)""" + + # Flying blind without the restore_checker so we can have + # an error thrown when dirsrv is down. + backup_path = backup(self.master) + + self.master.run_command(['ipactl', 'stop']) + + dirman_password = self.master.config.dirman_password + result = self.master.run_command( + ['ipa-restore', backup_path], + stdin_text=dirman_password + '\nyes', + raiseonerr=False) + assert result.returncode == 1
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org