[Freeipa-devel] [PATCH] 1078 own ca_serialno

[Simo Sorce]

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.

-- 
Simo Sorce * Red Hat, Inc * New York