[Freeipa-devel] [PATCH 0027] Add checks for SELinux in install scripts

[Tomas Babej]

On Mon 01 Apr 2013 10:01:14 PM CEST, Rob Crittenden wrote:
> Tomas Babej wrote:
>> On Tue 19 Feb 2013 08:37:26 PM CET, Rob Crittenden wrote:
>>> Tomas Babej wrote:
>>>> On 02/04/2013 04:21 PM, Rob Crittenden wrote:
>>>>> Tomas Babej wrote:
>>>>>> On 01/30/2013 05:12 PM, Tomas Babej wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> The checks make sure that SELinux is:
>>>>>>>   - installed and enabled (on server install)
>>>>>>>   - installed and enabled OR not installed (on client install)
>>>>>>>
>>>>>>> Please note that client installs with SELinux not installed are
>>>>>>> allowed since freeipa-client package has no dependency on SELinux.
>>>>>>> (any objections to this approach?)
>>>>>>>
>>>>>>> The (unsupported) option --allow-no-selinux has been added. It can
>>>>>>> used to bypass the checks.
>>>>>>>
>>>>>>> Parts of platform-dependant code were refactored to use newly added
>>>>>>> is_selinux_enabled() function.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/3359
>>>>>>>
>>>>>>> Tomas
>>>>>>
>>>>>> I forgot to edit the man pages. Thanks Rob!
>>>>>>
>>>>>> Updated patch attached.
>>>>>>
>>>>>> Tomas
>>>>>
>>>>> After a bit of off-line discussion I don't think we're quite ready
>>>>> yet
>>>>> to require SELinux by default on client installations (even with a
>>>>> flag to work around it). The feeling is this would be disruptive to
>>>>> existing automation.
>>>>>
>>>>> Can you still do the check but not enforce it, simply display a big
>>>>> warning if SELinux is disabled?
>>>>>
>>>>> rob
>>>>>
>>>>
>>>> Sure, here is the updated patch.
>>>>
>>>> I edited the commit message, RFE description and man pages
>>>> according to
>>>> the new behaviour.
>>>>
>>>> Tomas
>>>
>>> The patch looks good, I'm just wondering about one thing. The default
>>> value for is_selinux_enabled() is True in ipapython/services.py.in.
>>>
>>> So this means that any non-Red Hat/non-Fedora system, by default, is
>>> going to assume that SELinux is enabled.
>>>
>>> My hesitation has to when we call check_selinux_status(). It may
>>> incorrectly error out. I suspect that the user would have to work
>>> around this using --allow-selinux-disabled but this wouldn't make a
>>> lot of sense since they actually do have SELinux disabled.
>>
>> Yes, you're right. And the error message would not even be helpful since
>> it would tell the user to install policycoreutils package. This would be
>> the
>> case both with server and client installs when selinux would not be
>> installed
>> at all.
>>
>>> What do you think?
>>>
>>> rob
>>
>> Well we have 2 options as I see it:
>>
>> 1.) We can either return None as default, and add checks to
>> check_selinux_status, restore_context and install scripts that would
>> ensure that we behave properly when is_selinux_enabled() is not
>> implemented.
>>
>> 2.) We can remove the default value, since it would cause forementioned
>> crash and add comment that this function needs to be implemented
>> properly in every platform file.
>>
>> I'm probably for option 2, there's no need to clutter the code with
>> checks
>> that compensate for improper platform file implementations.
>>
>> Tomas
>
> I agree with you on option 2.
>
> rob

I updated the patch accordingly.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0027-4-Add-checks-for-SELinux-in-install-scripts.patch
Type: text/x-patch
Size: 13793 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130402/511d78d6/attachment.bin>