[Freeipa-devel] [PATCH 0286] Sysrestore: copy files instead of moving them to avoid SELinux issues

[Alexander Bokovoy]

On Fri, 17 Jul 2015, Martin Basti wrote:
>On 17/07/15 13:04, Alexander Bokovoy wrote:
>>On Wed, 15 Jul 2015, Martin Basti wrote:
>>>On 15/07/15 18:01, Alexander Bokovoy wrote:
>>>>On Wed, 15 Jul 2015, Martin Basti wrote:
>>>>>Moved files temporarily exist without a proper SElinux context 
>>>>>which causes issues when running SSSD/ntpd tries to work with 
>>>>>files.
>>>>>
>>>>>https://fedorahosted.org/freeipa/ticket/4923
>>>>>
>>>>>Patch attached.
>>>>>
>>>>>-- 
>>>>>Martin Basti
>>>>>
>>>>
>>>>>From a86424429eea3bede519284e2d986c4fad8755f8 Mon Sep 17 00:00:00 2001
>>>>>From: Martin Basti <mbasti at redhat.com>
>>>>>Date: Wed, 15 Jul 2015 16:20:59 +0200
>>>>>Subject: [PATCH] sysrestore: copy files instead of moving them 
>>>>>to avoind
>>>>>SELinux issues
>>>>>
>>>>>Copying files restores SELinux context.
>>>>>
>>>>>https://fedorahosted.org/freeipa/ticket/4923
>>>>>---
>>>>>ipapython/sysrestore.py | 12 ++++++------
>>>>>1 file changed, 6 insertions(+), 6 deletions(-)
>>>>>
>>>>>diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
>>>>>index c058ff7c04d4604ba96c2a4ece68d476b5b6491f..354897240b542c2671b662a4fdad1a089652f899 
>>>>>100644
>>>>>--- a/ipapython/sysrestore.py
>>>>>+++ b/ipapython/sysrestore.py
>>>>>@@ -186,12 +186,12 @@ class FileStore:
>>>>>       if new_path is not None:
>>>>>           path = new_path
>>>>>
>>>>>-        shutil.move(backup_path, path)
>>>>>+        shutil.copy(backup_path, path)  # SELinux needs copy
>>>>>+        os.remove(backup_path)
>>>>>+
>>>>>       os.chown(path, int(uid), int(gid))
>>>>>       os.chmod(path, int(mode))
>>>>>
>>>>>-        tasks.restore_context(path)
>>>>>-
>>>>Please keep restorecon calls because we might have a case when 
>>>>old label
>>>>was wrong in the backup.
>>>>
>>>>
>>>>>       del self.files[filename]
>>>>>       self.save()
>>>>>
>>>>>@@ -217,12 +217,12 @@ class FileStore:
>>>>>               root_logger.debug("  -> Not restoring - '%s' 
>>>>>doesn't exist", backup_path)
>>>>>               continue
>>>>>
>>>>>-            shutil.move(backup_path, path)
>>>>>+            shutil.copy(backup_path, path)  # SELinux needs copy
>>>>>+            os.remove(backup_path)
>>>>>+
>>>>>           os.chown(path, int(uid), int(gid))
>>>>>           os.chmod(path, int(mode))
>>>>>
>>>>>-            tasks.restore_context(path)
>>>>>-
>>>>Same here.
>>>>
>>>
>>>Sorry I don't get it.
>>>Label is not copied from backup_file.
>>>I changed Selinux context, then copy to original location and 
>>>context was restored when file does not exist.
>>>
>>>Do you mean case when the target file has different label than it 
>>>should have?
>>Yes, it could happen quite often.
>
>Updated patch attached.
You attached wrong patch

>
>-- 
>Martin Basti
>

>From d480d244266a84fb6c2c6b50011b1aba809e2aef Mon Sep 17 00:00:00 2001
>From: Martin Basti <mbasti at redhat.com>
>Date: Thu, 16 Jul 2015 16:26:55 +0200
>Subject: [PATCH] Allow value 'no' for replica-certify-all attr in
> abort-clean-ruv subcommand
>
>--force option set replica-certify-all to 'no' during abort-clean-ruv
>subcommand
>
>https://fedorahosted.org/freeipa/ticket/4988
>---
> install/tools/ipa-replica-manage       | 2 +-
> install/tools/man/ipa-replica-manage.1 | 2 +-
> ipaserver/install/replication.py       | 3 ++-
> 3 files changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
>index e525a02f4c60350b7a943abab4b4aedd957e984a..50a57f70ec452c0df5bf2ea55d2a136e8149aa41 100755
>--- a/install/tools/ipa-replica-manage
>+++ b/install/tools/ipa-replica-manage
>@@ -470,7 +470,7 @@ def abort_clean_ruv(realm, ruv, options):
>     print
>     thisrepl = replication.ReplicationManager(realm, options.host,
>                                               options.dirman_passwd)
>-    thisrepl.abortcleanallruv(ruv)
>+    thisrepl.abortcleanallruv(ruv, options.force)
> 
>     print "Cleanup task stopped"
> 
>diff --git a/install/tools/man/ipa-replica-manage.1 b/install/tools/man/ipa-replica-manage.1
>index 8a7c78f39eeb6c7902ed99e7bed37e32eb0e92dc..c09ed362f3143e6e38716e1b3a96e90001a64674 100644
>--- a/install/tools/man/ipa-replica-manage.1
>+++ b/install/tools/man/ipa-replica-manage.1
>@@ -49,7 +49,7 @@ Manages the replication agreements of an IPA server. The available commands are:
> \- Run the CLEANALLRUV task to remove a replication ID.
> .TP
> \fBabort\-clean\-ruv\fR [REPLICATION_ID]
>-\- Abort a running CLEANALLRUV task.
>+\- Abort a running CLEANALLRUV task. With \-\-force option the task does not wait for all the replica servers to have been sent the abort task, or be online, before completing.
> .TP
> \fBlist\-clean\-ruv\fR
> \- List all running CLEANALLRUV and abort CLEANALLRUV tasks.
>diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
>index 0f420106e093e8a7a277016857d27aaa48daa4dc..e9af88dc4356d4fd5495f4fea399ab09c75db953 100644
>--- a/ipaserver/install/replication.py
>+++ b/ipaserver/install/replication.py
>@@ -1451,7 +1451,7 @@ class ReplicationManager(object):
> 
>         wait_for_task(self.conn, dn)
> 
>-    def abortcleanallruv(self, replicaId):
>+    def abortcleanallruv(self, replicaId, force=False):
>         """
>         Create a task to abort a CLEANALLRUV operation.
>         """
>@@ -1465,6 +1465,7 @@ class ReplicationManager(object):
>                 'replica-id': [replicaId],
>                 'objectclass': ['top', 'extensibleObject'],
>                 'cn': ['abort %d' % replicaId],
>+                'replica-certify-all': ['no'] if force else ['yes'],
>             }
>         )
>         try:
>-- 
>2.4.3
>


-- 
/ Alexander Bokovoy