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

2015-07-17 Thread Martin Basti

On 17/07/15 13:57, Petr Vobornik wrote:

On 07/17/2015 01:46 PM, Petr Vobornik wrote:

On 07/17/2015 01:44 PM, Alexander Bokovoy wrote:

On Fri, 17 Jul 2015, Martin Basti wrote:

From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 15 Jul 2015 16:20:59 +0200
Subject: [PATCH] sysrestore: copy files instead of moving them to 
avoind

SELinux issues


ACK.



Pushed to:
master: 9f701283534745bf93b41a1886183e9ef1d06566
ipa-4-2: 92a73e8b2a5f26744b036a36de4b9956e8883f61


Does it really fix the whole ticket?

There is also in freeipa.spec.in %post client (i.e. upgrade):

cat /etc/krb5.conf >> /etc/krb5.conf.ipanew
mv /etc/krb5.conf.ipanew /etc/krb5.conf
/sbin/restorecon /etc/krb5.conf

+ some others.

Between the mv and restorecon, SSSD tries to access the file and 
raises AVC.


In this case we can freely use mv -z since target platforms are Fedora 
and newest RHEL.

I didn't inspect specfile, I will take a look.

Thank you for catch.

--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-07-17 Thread Petr Vobornik

On 07/17/2015 01:46 PM, Petr Vobornik wrote:

On 07/17/2015 01:44 PM, Alexander Bokovoy wrote:

On Fri, 17 Jul 2015, Martin Basti wrote:

From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 15 Jul 2015 16:20:59 +0200
Subject: [PATCH] sysrestore: copy files instead of moving them to avoind
SELinux issues


ACK.



Pushed to:
master: 9f701283534745bf93b41a1886183e9ef1d06566
ipa-4-2: 92a73e8b2a5f26744b036a36de4b9956e8883f61


Does it really fix the whole ticket?

There is also in freeipa.spec.in %post client (i.e. upgrade):

cat /etc/krb5.conf >> /etc/krb5.conf.ipanew
mv /etc/krb5.conf.ipanew /etc/krb5.conf
/sbin/restorecon /etc/krb5.conf

+ some others.

Between the mv and restorecon, SSSD tries to access the file and raises AVC.

In this case we can freely use mv -z since target platforms are Fedora 
and newest RHEL.

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-07-17 Thread Petr Vobornik

On 07/17/2015 01:44 PM, Alexander Bokovoy wrote:

On Fri, 17 Jul 2015, Martin Basti wrote:

From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 2001
From: Martin Basti 
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 | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index
c058ff7c04d4604ba96c2a4ece68d476b5b6491f..99d78b1b40c82a3350a7c5ba5ad9bf1f77ba887b
100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -186,7 +186,9 @@ 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))

@@ -217,7 +219,9 @@ 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))

--
2.4.3



ACK.



Pushed to:
master: 9f701283534745bf93b41a1886183e9ef1d06566
ipa-4-2: 92a73e8b2a5f26744b036a36de4b9956e8883f61
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-07-17 Thread Alexander Bokovoy

On Fri, 17 Jul 2015, Martin Basti wrote:

From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 2001
From: Martin Basti 
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 | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index 
c058ff7c04d4604ba96c2a4ece68d476b5b6491f..99d78b1b40c82a3350a7c5ba5ad9bf1f77ba887b
 100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -186,7 +186,9 @@ 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))

@@ -217,7 +219,9 @@ 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))

--
2.4.3



ACK.

--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-07-17 Thread Martin Basti

On 17/07/15 13:25, Alexander Bokovoy wrote:

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 
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 
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': 

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

2015-07-17 Thread 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 
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 
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

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

2015-07-17 Thread Martin Basti

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 
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.

--
Martin Basti

From d480d244266a84fb6c2c6b50011b1aba809e2aef Mon Sep 17 00:00:00 2001
From: Martin Basti 
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-ce

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

2015-07-17 Thread Alexander Bokovoy

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 
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.
--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-07-15 Thread Martin Basti

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 
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?


Martin^2

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-07-15 Thread Alexander Bokovoy

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 
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.

--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code