[Freeipa-devel] [PATCH] 0106 Make host/service cert revocation aware of lightweight CAs

Fraser Tweedale ftweedal at redhat.com
Mon Sep 5 15:30:23 UTC 2016


On Mon, Sep 05, 2016 at 11:59:11PM +1000, Fraser Tweedale wrote:
> On Tue, Aug 30, 2016 at 10:39:16AM +0200, Jan Cholasta wrote:
> > Hi,
> > 
> > On 26.8.2016 07:42, Fraser Tweedale wrote:
> > > On Fri, Aug 26, 2016 at 03:37:17PM +1000, Fraser Tweedale wrote:
> > > > Hi all,
> > > > 
> > > > Attached patch fixes https://fedorahosted.org/freeipa/ticket/6221.
> > > > It depends on Honza's PR #20
> > > > https://github.com/freeipa/freeipa/pull/20.
> > > > 
> > > > Thanks,
> > > > Fraser
> > > > 
> > > It does help to attach the patch :)
> > 
> > I think it would be better to call cert-find once per host-del/service-del
> > with the --host/--service option specified. That way you'll get all
> > certificates for the given host/service at once.
> > 
> > Honza
> >
> I agree that is a nicer approach.
> 
> 'revoke_certs' is called from several other places besides just
> host/service_del.  If we want to land this fix Real Soon I'd suggest
> we either:
> 
> A) Define function 'revoke_certs_from_cert_find', call it from
> host/service_del, and leave 'revoke_certs' alone; or
> 
> B) Land the patch as-is and do a bigger refactor at a later time.
> 
> What do you think?
>
Updated patch for option (A) is attached.

Thanks,
Fraser
-------------- next part --------------
From dacb091292b57608af8adb97adf9a96f1cb34e54 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Fri, 26 Aug 2016 15:31:13 +1000
Subject: [PATCH] Make host/service cert revocation aware of lightweight CAs

Revocation of host/service certs on host/service deletion or other
operations is broken when cert is issued by a lightweight (sub)CA,
causing the delete operation to be aborted.  Look up the issuing CA
and pass it to 'cert_revoke' to fix the issue.

For host/service deletion, look up all certs at once and pass to an
alternative revocation function that avoids addition calls to
cert_find or cert_show.

Fixes: https://fedorahosted.org/freeipa/ticket/6221
---
 ipaserver/plugins/host.py    | 11 +++-----
 ipaserver/plugins/service.py | 66 +++++++++++++++++++++++++++++++++++++-------
 2 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 03c64c637cbba0aee1b6569f3b5dbe200953bff8..891dacd762a57c06ff032e6f262813158c94f9f2 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -42,7 +42,8 @@ from .service import (
     validate_realm, normalize_principal, validate_certificate,
     set_certificate_attrs, ticket_flags_params, update_krbticketflags,
     set_kerberos_attrs, rename_ipaallowedtoperform_from_ldap,
-    rename_ipaallowedtoperform_to_ldap, revoke_certs)
+    rename_ipaallowedtoperform_to_ldap, revoke_certs,
+    revoke_certs_from_cert_find)
 from .dns import (dns_container_exists,
         add_records_for_host_validation, add_records_for_host,
         get_reverse_zone)
@@ -843,12 +844,8 @@ class host_del(LDAPDelete):
                 )
 
         if self.api.Command.ca_is_enabled()['result']:
-            try:
-                entry_attrs = ldap.get_entry(dn, ['usercertificate'])
-            except errors.NotFound:
-                self.obj.handle_not_found(*keys)
-
-            revoke_certs(entry_attrs.get('usercertificate', []), self.log)
+            certs = self.api.Command.cert_find(host=keys)['result']
+            revoke_certs_from_cert_find(certs)
 
         return dn
 
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 04d1916fe989a8651bcc4d44f1914c460be1081c..407665fb450e7a8513b42815691d64665b6b2282 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -232,24 +232,73 @@ def revoke_certs(certs, logger=None):
                 logger.info("Problem decoding certificate: %s" % e)
 
         serial = unicode(x509.get_serial_number(cert, x509.DER))
+        issuer = unicode(x509.get_issuer(cert, x509.DER))
 
         try:
-            result = api.Command['cert_show'](unicode(serial))['result']
+            # search by serial+issuer, not full cert match
+            results = api.Command['cert_find'](
+                min_serial_number=serial,
+                max_serial_number=serial,
+                issuer=issuer
+            )['result']
+            if len(results) == 0:
+                # Dogtag doesn't know about the cert therefore
+                # we cannot revoke it.  Perhaps it was issued by
+                # a 3rd-party CA.
+                continue
+            result = results[0]
         except errors.CertificateOperationError:
             continue
-        if 'revocation_reason' in result:
+        if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
             continue
-        if x509.normalize_certificate(result['certificate']) != cert:
+        if 'cacn' not in result:
+            # cert is known to Dogtag, but CA appears to have been
+            # deleted.  We cannot revoke this cert via IPA anymore.
+            # We could go directly to Dogtag to revoke it, but the
+            # issuer's cert should have been revoked so never mind.
             continue
 
         try:
-            api.Command['cert_revoke'](unicode(serial),
-                                       revocation_reason=4)
+            api.Command['cert_revoke'](
+                serial,
+                cacn=result['cacn'],
+                revocation_reason=4,
+            )
         except errors.NotImplementedError:
             # some CA's might not implement revoke
             pass
 
 
+def revoke_certs_from_cert_find(certs):
+    """
+    revoke the certificates removed from host/service entry
+
+    ``certs``
+        Output of a 'cert_find' command.
+
+    """
+    for cert in certs:
+        if 'cacn' not in cert:
+            # cert is known to Dogtag, but CA appears to have been
+            # deleted.  We cannot revoke this cert via IPA anymore.
+            # We could go directly to Dogtag to revoke it, but the
+            # issuer's cert should have been revoked so never mind.
+            continue
+
+        if cert['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
+            # cert is already revoked
+            continue
+
+        try:
+            api.Command['cert_revoke'](
+                cert['serial_number'],
+                cacn=cert['cacn'],
+                revocation_reason=4,
+            )
+        except errors.NotImplementedError:
+            # some CA's might not implement revoke
+            pass
+
 
 def set_certificate_attrs(entry_attrs):
     """
@@ -674,11 +723,8 @@ class service_del(LDAPDelete):
         # custom services allow them to manage them.
         check_required_principal(ldap, keys[-1])
         if self.api.Command.ca_is_enabled()['result']:
-            try:
-                entry_attrs = ldap.get_entry(dn, ['usercertificate'])
-            except errors.NotFound:
-                self.obj.handle_not_found(*keys)
-            revoke_certs(entry_attrs.get('usercertificate', []), self.log)
+            certs = self.api.Command.cert_find(service=keys)['result']
+            revoke_certs_from_cert_find(certs)
 
         return dn
 
-- 
2.5.5



More information about the Freeipa-devel mailing list