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

[Rob Crittenden]

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