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
>From 8ca5c7c904f935c9e7d395df9023841978a434f1 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Tue, 10 Apr 2012 21:21:08 +0200
Subject: [PATCH] Configure certmonger to execute restart scripts on renewal.

certmonger now has the ability to execute a script when it renews a
certificate. This can be used to automatically restart servers so
the certificate doesn't expire in the running server.

https://fedorahosted.org/freeipa/ticket/2050
---
 freeipa.spec.in                        |   11 +++++++++--
 install/Makefile.am                    |    1 +
 install/configure.ac                   |    1 +
 install/restart_scripts/Makefile.am    |   15 +++++++++++++++
 install/restart_scripts/README         |    2 ++
 install/restart_scripts/restart_dirsrv |   13 +++++++++++++
 install/restart_scripts/restart_httpd  |    7 +++++++
 ipapython/certmonger.py                |    9 ++++++++-
 ipaserver/install/certs.py             |   14 ++++++++++++--
 ipaserver/install/dsinstance.py        |    6 +++---
 ipaserver/install/httpinstance.py      |    4 ++--
 11 files changed, 73 insertions(+), 10 deletions(-)
 create mode 100644 install/restart_scripts/Makefile.am
 create mode 100644 install/restart_scripts/README
 create mode 100644 install/restart_scripts/restart_dirsrv
 create mode 100644 install/restart_scripts/restart_httpd

diff --git a/freeipa.spec.in b/freeipa.spec.in
index dc06d410df07519559ffea2f986b5d01fe4aab2f..f3168830c53d6d13f6a691be9146b5ab06439e4f 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -117,7 +117,7 @@ Requires(pre): systemd-units
 Requires(post): systemd-units
 %endif
 %if 0%{?fedora} >= 17
-Requires: selinux-policy >= 3.10.0-82
+Requires: selinux-policy >= 3.10.0-110
 %else
 %if 0%{?fedora} == 16
 Requires: selinux-policy >= 3.10.0-78
@@ -214,7 +214,7 @@ Requires:  xmlrpc-c
 %endif
 %endif
 Requires: sssd >= 1.8.0
-Requires: certmonger >= 0.26
+Requires: certmonger >= 0.53
 Requires: nss-tools
 Requires: bind-utils
 Requires: oddjob-mkhomedir
@@ -538,6 +538,8 @@ fi
 %endif
 %dir %{python_sitelib}/ipaserver
 %{python_sitelib}/ipaserver/*
+%dir %{_libdir}/ipa/certmonger
+%attr(755,root,root) %{_libdir}/ipa/certmonger/*
 %dir %{_usr}/share/ipa
 %{_usr}/share/ipa/wsgi.py*
 %{_usr}/share/ipa/*.ldif
@@ -674,6 +676,11 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Tue Apr 10 2012 Rob Crittenden <rcrit...@redhat.com> - 2.2.0-21
+- Set min for selinux-policy to 3.10.0-110 on F-17 to pick up certmonger
+  policy for restarting services.
+- Set min for certmonger to 0.53 so we have the -C option to set restart
+  commands.
 
 * Thu Apr  5 2012 Rob Crittenden <rcrit...@redhat.com> - 2.2.0-20
 - Bump minimum version of slapi-nis to 0.40
diff --git a/install/Makefile.am b/install/Makefile.am
index 1b0cda90a26f6ce8687e6dce35c90fee055fbc7a..4d24d072dbda5817691ac21388edf196ee942c5d 100644
--- a/install/Makefile.am
+++ b/install/Makefile.am
@@ -13,6 +13,7 @@ SUBDIRS =			\
         tools			\
         updates			\
         po			\
+        restart_scripts		\
 	$(NULL)
 
 install-exec-local:
diff --git a/install/configure.ac b/install/configure.ac
index bb1ec5025576371d93af1a838930113f3689fae5..827ddbab411a4aa8abbdd4488e217ce67046bd6b 100644
--- a/install/configure.ac
+++ b/install/configure.ac
@@ -81,6 +81,7 @@ AC_CONFIG_FILES([
     tools/man/Makefile
     updates/Makefile
     po/Makefile
+    restart_scripts/Makefile
 ])
 
 AC_OUTPUT
diff --git a/install/restart_scripts/Makefile.am b/install/restart_scripts/Makefile.am
new file mode 100644
index 0000000000000000000000000000000000000000..abc066b305356da3ed51beafc8c8dee7829b98ff
--- /dev/null
+++ b/install/restart_scripts/Makefile.am
@@ -0,0 +1,15 @@
+NULL =
+
+appdir = $(libdir)/ipa/certmonger
+app_DATA =                              \
+	restart_dirsrv			\
+	restart_httpd			\
+	$(NULL)
+
+EXTRA_DIST =                            \
+        $(app_DATA)                     \
+        $(NULL)
+
+MAINTAINERCLEANFILES =                  \
+        *~                              \
+        Makefile.in
diff --git a/install/restart_scripts/README b/install/restart_scripts/README
new file mode 100644
index 0000000000000000000000000000000000000000..64ad8b43eb24bd7c131e044fc8590f7cd0157678
--- /dev/null
+++ b/install/restart_scripts/README
@@ -0,0 +1,2 @@
+This directory contains scripts to be used by the command (-C) option
+of certmonger to restart services when the certificates are renewed.
diff --git a/install/restart_scripts/restart_dirsrv b/install/restart_scripts/restart_dirsrv
new file mode 100644
index 0000000000000000000000000000000000000000..e243583f964b047c7af6075ece1c1de25ad56d98
--- /dev/null
+++ b/install/restart_scripts/restart_dirsrv
@@ -0,0 +1,13 @@
+#!/usr/bin/python -E
+import sys
+from ipapython import services as ipaservices
+
+try:
+    instance = sys.argv[1]
+except IndexError:
+    instance = ""
+
+try:
+    ipaservices.knownservices.dirsrv.restart(instance)
+except Exception, e:
+    print "Cannot restart dirsrv (instance: '%s'): %s" % (instance, str(e))
diff --git a/install/restart_scripts/restart_httpd b/install/restart_scripts/restart_httpd
new file mode 100644
index 0000000000000000000000000000000000000000..a53ab6e6291ca679bff0f40cb6210e1f7bc792e1
--- /dev/null
+++ b/install/restart_scripts/restart_httpd
@@ -0,0 +1,7 @@
+#!/usr/bin/python -E
+from ipapython import services as ipaservices
+
+try:
+    ipaservices.knownservices.httpd.restart()
+except Exception, e:
+    print "Cannot restart httpd: %s" % str(e)
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index fda539b34829e4a05f5614c551929facaf8c8260..22a599ae69526b82b9364b96271d44306e440315 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -189,11 +189,15 @@ def cert_exists(nickname, secdir):
     else:
         return False
 
-def start_tracking(nickname, secdir, password_file=None):
+def start_tracking(nickname, secdir, password_file=None, command=None):
     """
     Tell certmonger to track the given certificate nickname in NSS
     database in secdir protected by optional password file password_file.
 
+    command is an optional parameter which specifies a command for
+    certmonger to run when it renews a certificate. This command must
+    reside in /usr/lib/ipa/certmonger to work with SELinux.
+
     Returns the stdout, stderr and returncode from running ipa-getcert
 
     This assumes that certmonger is already running.
@@ -206,6 +210,9 @@ def start_tracking(nickname, secdir, password_file=None):
     if password_file:
         args.append("-p")
         args.append(os.path.abspath(password_file))
+    if command:
+        args.append("-C")
+        args.append(command)
 
     (stdout, stderr, returncode) = ipautil.run(args)
 
diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 13770811ecb5716b02493332006ed09004ffc1c3..d25a471ea92072781bb96095a3f5178b0762eeb4 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -18,6 +18,7 @@
 #
 
 import os, stat, subprocess, re
+import sys
 import errno
 import tempfile
 import shutil
@@ -492,16 +493,25 @@ class CertDB(object):
 
         raise RuntimeError("Unable to find serial number")
 
-    def track_server_cert(self, nickname, principal, password_file=None):
+    def track_server_cert(self, nickname, principal, password_file=None, command=None):
         """
         Tell certmonger to track the given certificate nickname.
+
+        If command is not a full path then it is prefixed with
+        /usr/lib[64]/ipa/certmonger.
         """
+        if command is not None and not os.path.isabs(command):
+            if sys.maxsize > 2**32:
+                libpath = 'lib64'
+            else:
+                libpath = 'lib'
+            command = '/usr/%s/ipa/certmonger/%s' % (libpath, command)
         cmonger = ipaservices.knownservices.certmonger
         cmonger.enable()
         ipaservices.knownservices.messagebus.start()
         cmonger.start()
         try:
-            (stdout, stderr, rc) = certmonger.start_tracking(nickname, self.secdir, password_file)
+            (stdout, stderr, rc) = certmonger.start_tracking(nickname, self.secdir, password_file, command)
         except (ipautil.CalledProcessError, RuntimeError), e:
             root_logger.error("certmonger failed starting to track certificate: %s" % str(e))
             return
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 0dc3d2560098d390c96d9e018978a2220a8aadd2..5a92bf978fbe3243ea47e7f49845b1d77a915487 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -513,21 +513,21 @@ class DsInstance(service.Service):
             # We only handle one server cert
             nickname = server_certs[0][0]
             self.dercert = dsdb.get_cert_from_db(nickname, pem=False)
-            dsdb.track_server_cert(nickname, self.principal, dsdb.passwd_fname)
+            dsdb.track_server_cert(nickname, self.principal, dsdb.passwd_fname, 'restart_dirsrv %s' % self.serverid )
         else:
             nickname = "Server-Cert"
             cadb = certs.CertDB(self.realm_name, host_name=self.fqdn, subject_base=self.subject_base)
             if self.self_signed_ca:
                 dsdb.create_from_cacert(cadb.cacert_fname, passwd=None)
                 self.dercert = dsdb.create_server_cert("Server-Cert", self.fqdn, cadb)
-                dsdb.track_server_cert("Server-Cert", self.principal, dsdb.passwd_fname)
+                dsdb.track_server_cert("Server-Cert", self.principal, dsdb.passwd_fname, 'restart_dirsrv %s' % self.serverid)
                 dsdb.create_pin_file()
             else:
                 # FIXME, need to set this nickname in the RA plugin
                 cadb.export_ca_cert('ipaCert', False)
                 dsdb.create_from_cacert(cadb.cacert_fname, passwd=None)
                 self.dercert = dsdb.create_server_cert("Server-Cert", self.fqdn, cadb)
-                dsdb.track_server_cert("Server-Cert", self.principal, dsdb.passwd_fname)
+                dsdb.track_server_cert("Server-Cert", self.principal, dsdb.passwd_fname, 'restart_dirsrv %s' % self.serverid)
                 dsdb.create_pin_file()
 
         conn = ipaldap.IPAdmin("127.0.0.1")
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 0a09c26f2d16af62b66bc5b9c24851f2cfd46158..e1bbc30a1b70a138e0f92c74d3956272a3beba92 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -210,7 +210,7 @@ class HTTPInstance(service.Service):
             # We only handle one server cert
             nickname = server_certs[0][0]
             self.dercert = db.get_cert_from_db(nickname, pem=False)
-            db.track_server_cert(nickname, self.principal, db.passwd_fname)
+            db.track_server_cert(nickname, self.principal, db.passwd_fname, 'restart_httpd')
 
             self.__set_mod_nss_nickname(nickname)
         else:
@@ -219,7 +219,7 @@ class HTTPInstance(service.Service):
 
             db.create_password_conf()
             self.dercert = db.create_server_cert("Server-Cert", self.fqdn, ca_db)
-            db.track_server_cert("Server-Cert", self.principal, db.passwd_fname)
+            db.track_server_cert("Server-Cert", self.principal, db.passwd_fname, 'restart_httpd')
             db.create_signing_cert("Signing-Cert", "Object Signing Cert", ca_db)
 
         # Fix the database permissions
-- 
1.7.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to