[Freeipa-devel] [PATCH] 1078 own ca_serialno

[Rob Crittenden]

Simo Sorce wrote:
> On Thu, 2012-12-13 at 10:44 -0500, Rob Crittenden wrote:
>> 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.
>
> Why would it hose fixfiles ?
> fixfiles knows not to bother with missing files afaik.
>
> There is something I guess I am missing here :/
>
> Simo.
>

Ok, I think I misunderstood your proposal to remove policy from the rpm 
then. What is it you're suggesting?

rob