[Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

Thu Apr 5 20:47:55 UTC 2012

Rob Crittenden wrote:
> Martin Kosek wrote:
>> On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:
>>> Rob Crittenden wrote:
>>>> Martin Kosek wrote:
>>>>> On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:
>>>>>> Rob Crittenden wrote:
>>>>>>> Martin Kosek wrote:
>>>>>>>> On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
>>>>>>>>> Certmonger will currently automatically renew server
>>>>>>>>> certificates but
>>>>>>>>> doesn't restart the services so you can still end up with expired
>>>>>>>>> certificates if you services never restart.
>>>>>>>>>
>>>>>>>>> This patch registers are restart command with certmonger so the
>>>>>>>>> IPA
>>>>>>>>> services will automatically be restarted to get the updated cert.
>>>>>>>>>
>>>>>>>>> Easy to test. Install IPA then resubmit the current server
>>>>>>>>> certs and
>>>>>>>>> watch the services restart:
>>>>>>>>>
>>>>>>>>> # ipa-getcert list
>>>>>>>>>
>>>>>>>>> Find the ID for either your dirsrv or httpd instance
>>>>>>>>>
>>>>>>>>> # ipa-getcert resubmit -i<ID>
>>>>>>>>>
>>>>>>>>> Watch /var/log/httpd/error_log or
>>>>>>>>> /var/log/dirsrv/slapd-INSTANCE/errors
>>>>>>>>> to see the service restart.
>>>>>>>>>
>>>>>>>>> rob
>>>>>>>>
>>>>>>>> What about current instances - can we/do we want to update
>>>>>>>> certmonger
>>>>>>>> tracking so that their instances are restarted as well?
>>>>>>>>
>>>>>>>> Anyway, I found few issues SELinux issues with the patch:
>>>>>>>>
>>>>>>>> 1) # rpm -Uvh freeipa-*
>>>>>>>> Preparing... ########################################### [100%]
>>>>>>>> 1:freeipa-python ########################################### [ 20%]
>>>>>>>> 2:freeipa-client ########################################### [ 40%]
>>>>>>>> 3:freeipa-admintools ########################################### [
>>>>>>>> 60%]
>>>>>>>> 4:freeipa-server ########################################### [ 80%]
>>>>>>>> /usr/bin/chcon: failed to change context of
>>>>>>>> `/usr/lib64/ipa/certmonger' to
>>>>>>>> `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
>>>>>>>> argument
>>>>>>>> /usr/bin/chcon: failed to change context of
>>>>>>>> `/usr/lib64/ipa/certmonger/restart_dirsrv' to
>>>>>>>> `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
>>>>>>>> argument
>>>>>>>> /usr/bin/chcon: failed to change context of
>>>>>>>> `/usr/lib64/ipa/certmonger/restart_httpd' to
>>>>>>>> `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
>>>>>>>> argument
>>>>>>>> warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
>>>>>>>> scriptlet failed, exit status 1
>>>>>>>> 5:freeipa-server-selinux
>>>>>>>> ###########################################
>>>>>>>> [100%]
>>>>>>>>
>>>>>>>> certmonger_unconfined_exec_t type was unknown with my selinux
>>>>>>>> policy:
>>>>>>>>
>>>>>>>> selinux-policy-3.10.0-80.fc16.noarch
>>>>>>>> selinux-policy-targeted-3.10.0-80.fc16.noarch
>>>>>>>>
>>>>>>>> If we need a higher SELinux version, we should bump the required
>>>>>>>> package
>>>>>>>> version spec file.
>>>>>>>
>>>>>>> Yeah, waiting on it to be backported.
>>>>>>>
>>>>>>>>
>>>>>>>> 2) Change of SELinux context with /usr/bin/chcon is temporary until
>>>>>>>> restorecon or system relabel occurs. I think we should make it
>>>>>>>> persistent and enforce this type in our SELinux policy and
>>>>>>>> rather call
>>>>>>>> restorecon instead of chcon
>>>>>>>
>>>>>>> That's a good idea, why didn't I think of that :-(
>>>>>>
>>>>>> Ah, now I remember, it will be handled by selinux-policy. I would
>>>>>> have
>>>>>> used restorecon here but since the policy isn't there yet this seemed
>>>>>> like a good idea.
>>>>>>
>>>>>> I'm trying to find out the status of this new policy, it may only
>>>>>> make
>>>>>> it into F-17.
>>>>>>
>>>>>> rob
>>>>>
>>>>> Ok. But if this policy does not go in F-16 and if we want this fix in
>>>>> F16 release too, I guess we would have to implement both approaches in
>>>>> our spec file:
>>>>>
>>>>> 1) When on F16, include SELinux policy for restart scripts + run
>>>>> restorecon
>>>>> 2) When on F17, do not include the SELinux policy (+ run restorecon)
>>>>>
>>>>> Martin
>>>>>
>>>>
>>>> Won't work without updated selinux-policy. Without the permission for
>>>> certmonger to execute the commands things will still fail (just in
>>>> really non-obvious and far in the future ways).
>>>>
>>>> It looks like this is fixed in F-17 selinux-policy-3.10.0-107.
>>>>
>>>> rob
>>>
>>> Updated patch which works on F-17.
>>>
>>> rob
>>
>> What about F-16? The restart scripts won't work with enabled enforcing
>> and will raise AVCs. Maybe we really need to deliver our own SELinux
>> policy allowing it on F-16.
>
> Right, I don't see this working on F-16. I don't really want to carry
> this type of policy. It goes beyond marking a few files as certmonger_t,
> it is going to let certmonger execute arbitrary scripts. This is best
> left to the SELinux team who understand the consequences better.
>
>>
>> I also found an issue with the restart scripts:
>>
>> 1) restart_dirsrv: this won't work with systemd:
>>
>> # /sbin/service dirsrv restart
>> Redirecting to /bin/systemctl restart dirsrv.service
>> Failed to issue method call: Unit dirsrv.service failed to load: No such
>> file or directory. See system logs and 'systemctl status dirsrv.service'
>> for details.
>
> Wouldn't work so hot for sysV either as we'd be restarting all
> instances. I'll take a look.
>
>> We would need to pass an instance of IPA dirsrv for this to work.
>>
>> 2) restart_httpd:
>> Is reload enough for httpd to pull a new certificate? Don't we need a
>> full restart? If reload is enough, I think the command should be named
>> reload_httpd
>
> Yes, it causes the modules to be reloaded which will reload the NSS
> database, that's all we need. I named it this way for consistency. I can
> rename it, though I doubt it would cause any confusion either way.
>
> rob

revised patch.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-998-3-certmonger.patch
Type: text/x-diff
Size: 11075 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120405/e2b25d9c/attachment.bin>