[Freeipa-devel] [PATCH] 1078 own ca_serialno

[Rob Crittenden]

Simo Sorce wrote:
> On Thu, 2012-12-13 at 10:28 -0500, Rob Crittenden wrote:
>> Simo Sorce wrote:
>>> On Thu, 2012-12-13 at 15:38 +0100, Martin Kosek wrote:
>>>> On 12/13/2012 03:34 PM, Petr Viktorin wrote:
>>>>> On 12/13/2012 02:47 PM, Rob Crittenden wrote:
>>>>>> Petr Viktorin wrote:
>>>>>>> On 12/13/2012 06:01 AM, Rob Crittenden wrote:
>>>>>>>> We don't currently include the ca_serialno file in our spec file. This
>>>>>>>> can generate an SELinux warning upon fresh install because we try to set
>>>>>>>> context on a non-existent file.
>>>>>>>>
>>>>>>>> This creates an empty file on rpm install so the file can be owned by
>>>>>>>> the spec.
>>>>>>>>
>>>>>>>> I also updated the selfsign serial number code to deal with an existing
>>>>>>>> but empty file.
>>>>>>>>
>>>>>>>> rob
>>>>>>>>
>>>>>>>
>>>>>>> I couldn't reproduce the error, but I noticed you've left out the
>>>>>>> percent sign in %attr:
>>>>>>
>>>>>> It was reported against RHEL systems, so perhaps the SELinux (or rpm) in
>>>>>> Fedora suppresses this message.
>>>>>>
>>>>>>>> --- a/freeipa.spec.in
>>>>>>>> +++ b/freeipa.spec.in
>>>>>>> [...]
>>>>>>>> @@ -660,6 +662,7 @@ fi
>>>>>>>>     %attr(755,root,root) %{plugin_dir}/libipa_cldap.so
>>>>>>>>     %attr(755,root,root) %{plugin_dir}/libipa_range_check.so
>>>>>>>>     %dir %{_localstatedir}/lib/ipa
>>>>>>>> +attr(600,root,root) %config(noreplace)
>>>>>>>> %{_localstatedir}/lib/ipa/ca_serialno
>>>>>>>
>>>>>>> RPM build errors:
>>>>>>>        File must begin with "/": attr(600,root,root)
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> D'oh. I had tested this in RHEL and cut-n-pasted the fix upstream. Fixed.
>>>>>>
>>>>>> rob
>>>>>
>>>>> On Fedora this doesn't hurt, ACK.
>>>>>
>>>>
>>>> NACK.
>>>>
>>>> When FreeIPA gets uninstalled, we end up without this file again. Which would
>>>> again lead to this warning on upgrades.
>>>>
>>>> I think we should rather truncate the file on server uninstall instead of
>>>> removing it.
>>>>
>>>
>>> Why don't we simply declare it as %ghost and conditionally label it ?
>>>
>>> I do not really like to have empty files just as an artifact, sounds
>>> like the wrong solution, sorry.
>>>
>>> Simo.
>>>
>>
>> The file has to exist for SELinux to label it. If we ghost it them the
>> package will own it if it exists but the SELinux context will still fail
>> to apply.
>
> We can apply selinux context in ipa-server-install and not in the spec.
> That's when we need it anyway.
>
> Simo.
>

I don't think we should. It would hose up fixfiles. If things ever got 
out-of-sync there would be no easy way to reset the contexts to what 
they should be.

And yeah, this is a rather ugly case. I'm not super keen on carrying a 
0-length file for no reason either. I tried the ghost method first which 
is why I know it doesn't work.

rob