URL: https://github.com/freeipa/freeipa/pull/1993 Author: rcritten Title: #1993: [Backport][ipa-4-6] 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/1993/head:pr1993 git checkout pr1993
From 36aaf8ccf3571f07a9fe2cf083e42ac6a258dfd8 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> Reviewed-By: Christian Heimes <chei...@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 5d703865c4..8b2f5bef7c 100644 --- a/ipaserver/install/ipa_restore.py +++ b/ipaserver/install/ipa_restore.py @@ -208,6 +208,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: @@ -668,7 +672,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() @@ -882,3 +886,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 073373be00dde172cbe636e53e44a1ac7d667631 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> Reviewed-By: Christian Heimes <chei...@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 1b6298be93..63313db715 100644 --- a/ipatests/test_integration/test_backup_and_restore.py +++ b/ipatests/test_integration/test_backup_and_restore.py @@ -471,10 +471,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([ @@ -617,3 +616,36 @@ def test_replica_install_after_restore(self): # install second replica after restore tasks.install_replica(master, replica2) check_replication(master, replica2, "testuser2") + + +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 Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/freeipa-devel@lists.fedorahosted.org/message/GTSHDTM5GMHKD5WUNECMIKCYKSTLSWGK/
