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

[Rob Crittenden]

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