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

Tue Apr 10 22:17:39 UTC 2012

Martin Kosek wrote:
> On Fri, 2012-04-06 at 10:22 +0200, Martin Kosek wrote:
>> On Thu, 2012-04-05 at 16:47 -0400, Rob Crittenden wrote:
>>> 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
>>
>> Thanks, this is better, dirsrv restart is now fixed. I still have few
>> issues:
>>
>> 1) Wrong command for httpd certs (should be reload_httpd):
>>
>> -            db.track_server_cert(nickname, self.principal,
>> db.passwd_fname)
>> +            db.track_server_cert(nickname, self.principal,
>> db.passwd_fname, 'restart_httpd')
>>
>> -            db.track_server_cert("Server-Cert", self.principal,
>> db.passwd_fname)
>> +            db.track_server_cert("Server-Cert", self.principal,
>> db.passwd_fname, 'restart_httpd')
>>
>>
>> 2) What about current certmonger monitored certs? We can use the command
>> Nalin suggested to modify existing monitored certs to set up a restart
>> command:
>>
>> # ipa-getcert list
>> ...
>> Request ID '20120404083924':
>> 	status: MONITORING
>> 	stuck: no
>> 	key pair storage:
>> type=NSSDB,location='/etc/httpd/alias',nickname='Server-Cert',token='NSS
>> Certificate DB',pinfile='/etc/httpd/alias/pwdfile.txt'
>> 	certificate:
>> type=NSSDB,location='/etc/httpd/alias',nickname='Server-Cert',token='NSS
>> Certificate DB'
>> 	CA: IPA
>> 	issuer: CN=Certificate Authority,O=IDM.LAB.BOS.REDHAT.COM
>> 	subject: CN=vm-079.idm.lab.bos.redhat.com,O=IDM.LAB.BOS.REDHAT.COM
>> 	expires: 2014-04-05 08:39:24 UTC
>> 	eku: id-kp-serverAuth,id-kp-clientAuth
>> 	command:
>> 	track: yes
>> 	auto-renew: yes
>>
>> # ipa-getcert start-tracking -i 20120404083924 -C /usr/lib64/ipa/certmonger/reload_httpd
>> Request "20120404083924" modified.
>>
>> # ipa-getcert list
>> Request ID '20120404083924':
>> 	status: MONITORING
>> 	stuck: no
>> 	key pair storage:
>> type=NSSDB,location='/etc/httpd/alias',nickname='Server-Cert',token='NSS
>> Certificate DB',pinfile='/etc/httpd/alias/pwdfile.txt'
>> 	certificate:
>> type=NSSDB,location='/etc/httpd/alias',nickname='Server-Cert',token='NSS
>> Certificate DB'
>> 	CA: IPA
>> 	issuer: CN=Certificate Authority,O=IDM.LAB.BOS.REDHAT.COM
>> 	subject: CN=vm-079.idm.lab.bos.redhat.com,O=IDM.LAB.BOS.REDHAT.COM
>> 	expires: 2014-04-05 08:39:24 UTC
>> 	eku: id-kp-serverAuth,id-kp-clientAuth
>> 	command: /usr/lib64/ipa/certmonger/reload_httpd
>> 	track: yes
>> 	auto-renew: yes
>>
>> 3) The new dirsrv service restart command does not work either with
>> systemd:
>> # service dirsrv restart IDM-LAB-BOS-REDHAT-COM
>>
>> I think the approach here would be to take advantage of our system
>> service management abstraction that ipactl uses. It will then work well
>> with SysV, systemd or any other future configurations or ports to other
>> Linux distributions. Example of the script which restarts the required
>> instance:
>>
>> # cat /tmp/restart_dirsrv
>> #!/usr/bin/python
>>
>> import sys
>> from ipapython import services as ipaservices
>>
>> try:
>>      instance = sys.argv[1]
>> except IndexError:
>>      instance = ""
>>
>> dirsrv = ipaservices.knownservices.dirsrv
>> dirsrv.restart(instance)
>>
>> # /tmp/restart_dirsrv IDM-LAB-BOS-REDHAT-COM
>>
>> Martin
>>
>
> I prepared a rebased and fixed patch which works on both SystemV and
> systemd systems, please check attached file.
>
> The patch does not include updating existing certs, it would need more
> time to implement a safe update that it is not available this late in
> the game. We may want to create a ticket to add a documentation about
> how to update existing certs + RFE for add a support of updating
> existing certs later.
>
> Martin

This works fine. We'll address existing certs in the future.

ACK, pushed to master and ipa-2-2

rob