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

Reply via email to