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

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

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0286.2-Allow-value-no-for-replica-certify-all-attr-in-abort.patch
Type: text/x-patch
Size: 2815 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150717/a3d320ee/attachment.bin>