[Freeipa-devel] [PATCH] 0106 Make host/service cert revocation aware of lightweight CAs
Fraser Tweedale
ftweedal at redhat.com
Tue Sep 6 17:36:15 UTC 2016
On Tue, Sep 06, 2016 at 10:19:14AM +0200, Jan Cholasta wrote:
> On 5.9.2016 17:30, Fraser Tweedale wrote:
> > 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 attached; comments inline.
> C) Use cert-find-based revoke_certs() everywhere; use the --certificate
> option of cert-find in the other places to get information about specific
> certificates.
>
As discussed on IRC, I have implemented this option. The caveat is
that for host/service-mod, we incur call to cert_find for each
removed certificate.
> > >
> > Updated patch for option (A) is attached.
>
> 1) Instead of
>
> if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
>
> use:
>
> if result['revoked']:
>
Done.
>
> 2)
>
> + 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
>
> Or, it could be a cert issued by a 3rd party CA.
>
I updated to comment to include this.
>
> 3) host-mod/service-mod do not revoke certs:
>
> $ ipa cert-request test.csr --principal host/test.example.com
> Serial number: 13
>
> $ ipa cert-show 13
> Revoked: False
> Owner host: test.example.com
>
> $ ipa host-mod test.example.com --certificate=
>
> $ ipa cert-show 13
> Revoked: False
>
Nice find. This was a pre-existing bug: nothing gets revoked when
all certs are removed. Here is the fix:
- if certs and self.api.Command.ca_is_enabled()['result']:
+ ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+ if 'usercertificate' in options and ca_is_enabled:
... revocation code
Finally, host/service-remove-cert does not revoke the cert because
of (I think) a bug in cert-find. If the cert does not exist on a
host/service the cert-find cannot find it with --certificate option.
Because host/service-remove-cert uses a post_callback to revoke the
cert, cert-find doesn't find it thus no revocation occurs.
Honza could you check whether this is indeed a bug/limitation of
cert-find or is it the smog in Saigon affecting me?
Thanks,
Fraser
-------------- next part --------------
From 9c829d7ec8ff67dcf814c468c406772bf311c9f8 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.
Fixes: https://fedorahosted.org/freeipa/ticket/6221
---
ipaserver/plugins/host.py | 23 +++++++++--------
ipaserver/plugins/service.py | 59 ++++++++++++++++++++++----------------------
2 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 03c64c637cbba0aee1b6569f3b5dbe200953bff8..7f63e94849b4a6f2ce871ec77b188c54d640ba94 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -843,12 +843,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(certs)
return dn
@@ -902,7 +898,8 @@ class host_mod(LDAPUpdate):
certs_der = [x509.normalize_certificate(c) for c in certs]
# revoke removed certificates
- if certs and self.api.Command.ca_is_enabled()['result']:
+ ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+ if 'usercertificate' in options and ca_is_enabled:
try:
entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
except errors.NotFound:
@@ -910,7 +907,9 @@ class host_mod(LDAPUpdate):
old_certs = entry_attrs_old.get('usercertificate', [])
old_certs_der = [x509.normalize_certificate(c) for c in old_certs]
removed_certs_der = set(old_certs_der) - set(certs_der)
- revoke_certs(removed_certs_der, self.log)
+ for der in removed_certs_der:
+ rm_certs = api.Command.cert_find(certificate=der)['result']
+ revoke_certs(rm_certs)
if certs:
entry_attrs['usercertificate'] = certs_der
@@ -1196,10 +1195,10 @@ class host_disable(LDAPQuery):
except errors.NotFound:
self.obj.handle_not_found(*keys)
if self.api.Command.ca_is_enabled()['result']:
- certs = entry_attrs.get('usercertificate', [])
+ certs = self.api.Command.cert_find(host=keys)['result']
if certs:
- revoke_certs(certs, self.log)
+ revoke_certs(certs)
# Remove the usercertificate altogether
entry_attrs['usercertificate'] = None
ldap.update_entry(entry_attrs)
@@ -1341,8 +1340,8 @@ class host_remove_cert(LDAPRemoveAttributeViaOption):
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
assert isinstance(dn, DN)
- if 'usercertificate' in options:
- revoke_certs(options['usercertificate'], self.log)
+ for cert in options.get('usercertificate', []):
+ revoke_certs(api.Command.cert_find(certificate=cert)['result'])
return dn
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 04d1916fe989a8651bcc4d44f1914c460be1081c..c0590732470ac1200d4dd4ea1f089e4384a509b3 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -220,37 +220,38 @@ def validate_certificate(ugettext, cert):
x509.validate_certificate(cert, datatype=x509.DER)
-def revoke_certs(certs, logger=None):
+def revoke_certs(certs):
"""
revoke the certificates removed from host/service entry
+
+ :param certs: Output of a 'cert_find' command.
+
"""
for cert in certs:
- try:
- cert = x509.normalize_certificate(cert)
- except errors.CertificateFormatError as e:
- if logger is not None:
- logger.info("Problem decoding certificate: %s" % e)
-
- serial = unicode(x509.get_serial_number(cert, x509.DER))
-
- try:
- result = api.Command['cert_show'](unicode(serial))['result']
- except errors.CertificateOperationError:
- continue
- if 'revocation_reason' in result:
+ if 'cacn' not in cert:
+ # Cert is known to IPA, but has no associated CA.
+ # If it was issued by 3rd-party CA, we can't revoke it.
+ # If it was issued by a Dogtag lightweight CA that was
+ # subsequently deleted, we can't revoke it via IPA.
+ # We could go directly to Dogtag to revoke it, but the
+ # issuer's cert should have been revoked so never mind.
continue
- if x509.normalize_certificate(result['certificate']) != cert:
+
+ if cert['revoked']:
+ # cert is already revoked
continue
try:
- api.Command['cert_revoke'](unicode(serial),
- revocation_reason=4)
+ 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):
"""
Set individual attributes from some values from a certificate.
@@ -674,11 +675,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(certs)
return dn
@@ -703,7 +701,8 @@ class service_mod(LDAPUpdate):
certs = entry_attrs.get('usercertificate') or []
certs_der = [x509.normalize_certificate(c) for c in certs]
# revoke removed certificates
- if certs and self.api.Command.ca_is_enabled()['result']:
+ ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+ if 'usercertificate' in options and ca_is_enabled:
try:
entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
except errors.NotFound:
@@ -711,7 +710,9 @@ class service_mod(LDAPUpdate):
old_certs = entry_attrs_old.get('usercertificate', [])
old_certs_der = [x509.normalize_certificate(c) for c in old_certs]
removed_certs_der = set(old_certs_der) - set(certs_der)
- revoke_certs(removed_certs_der, self.log)
+ for der in removed_certs_der:
+ rm_certs = api.Command.cert_find(certificate=der)['result']
+ revoke_certs(rm_certs)
if certs:
entry_attrs['usercertificate'] = certs_der
@@ -950,10 +951,10 @@ class service_disable(LDAPQuery):
done_work = False
if self.api.Command.ca_is_enabled()['result']:
- certs = entry_attrs.get('usercertificate', [])
+ certs = self.api.Command.cert_find(service=keys)['result']
if len(certs) > 0:
- revoke_certs(certs, self.log)
+ revoke_certs(certs)
# Remove the usercertificate altogether
entry_attrs['usercertificate'] = None
ldap.update_entry(entry_attrs)
@@ -989,8 +990,8 @@ class service_remove_cert(LDAPRemoveAttributeViaOption):
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
assert isinstance(dn, DN)
- if 'usercertificate' in options:
- revoke_certs(options['usercertificate'], self.log)
+ for cert in options.get('usercertificate', []):
+ revoke_certs(api.Command.cert_find(certificate=cert)['result'])
return dn
--
2.5.5
More information about the Freeipa-devel
mailing list