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
From dacb091292b57608af8adb97adf9a96f1cb34e54 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@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
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code