URL: https://github.com/freeipa/freeipa/pull/677 Author: HonzaCholasta Title: #677: cert: defer cert-find result post-processing Action: opened
PR body: """ Rather than post-processing the results of each internal search, post-process the combined result. This avoids expensive per-certificate searches on certificates which won't even be included in the combined result when cert-find is executed with the --all option. https://pagure.io/freeipa/issue/6808 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/677/head:pr677 git checkout pr677
From 6e6e8438402f3117031df0a7d81bb9e62bbae5a8 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Thu, 30 Mar 2017 08:33:30 +0000 Subject: [PATCH] cert: defer cert-find result post-processing Rather than post-processing the results of each internal search, post-process the combined result. This avoids expensive per-certificate searches on certificates which won't even be included in the combined result when cert-find is executed with the --all option. https://pagure.io/freeipa/issue/6808 --- ipaserver/plugins/cert.py | 88 +++++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 1a6d045..f4eb0f0 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -1296,18 +1296,7 @@ def _get_cert_key(self, cert): return (DN(cert_obj.issuer), cert_obj.serial_number) - def _get_cert_obj(self, cert, all, raw, pkey_only): - obj = {'certificate': base64.b64encode(cert).decode('ascii')} - - full = not pkey_only and all - if not raw: - self.obj._parse(obj, full) - if not full: - del obj['certificate'] - - return obj - - def _cert_search(self, all, raw, pkey_only, **options): + def _cert_search(self, **options): result = collections.OrderedDict() try: @@ -1320,11 +1309,11 @@ def _cert_search(self, all, raw, pkey_only, **options): except ValueError: return result, True, True - result[key] = self._get_cert_obj(cert, all, raw, pkey_only) + result[key] = {'certificate': base64.b64encode(cert).decode('ascii')} return result, False, True - def _ca_search(self, all, raw, pkey_only, exactly, **options): + def _ca_search(self, raw, pkey_only, exactly, **options): ra_options = {} for name in ('revocation_reason', 'issuer', @@ -1357,7 +1346,6 @@ def _ca_search(self, all, raw, pkey_only, exactly, **options): return result, False, complete ca_objs = self.api.Command.ca_find( - all=all, timelimit=0, sizelimit=0, )['result'] @@ -1373,28 +1361,13 @@ def _ca_search(self, all, raw, pkey_only, exactly, **options): except KeyError: continue - if pkey_only: - obj = {'serial_number': serial_number} - else: - obj = ra_obj - if all: - obj.update(ra.get_certificate(str(serial_number))) - - if not raw: - obj['issuer'] = issuer - obj['subject'] = DN(ra_obj['subject']) - obj['revoked'] = ( - ra_obj['status'] in (u'REVOKED', u'REVOKED_EXPIRED')) - if all: - obj['certificate'] = ( - obj['certificate'].replace('\r\n', '')) - self.obj._parse(obj) + obj = ra_obj - if 'certificate_chain' in ca_obj: - cert = x509.load_certificate(obj['certificate']) - cert_der = cert.public_bytes(serialization.Encoding.DER) - obj['certificate_chain'] = ( - [cert_der] + ca_obj['certificate_chain']) + if not pkey_only and not raw: + obj['issuer'] = issuer + obj['subject'] = DN(ra_obj['subject']) + obj['revoked'] = ( + ra_obj['status'] in (u'REVOKED', u'REVOKED_EXPIRED')) obj['cacn'] = ca_obj['cn'][0] @@ -1402,7 +1375,7 @@ def _ca_search(self, all, raw, pkey_only, exactly, **options): return result, False, complete - def _ldap_search(self, all, raw, pkey_only, no_members, **options): + def _ldap_search(self, all, pkey_only, no_members, **options): ldap = self.api.Backend.ldap2 filters = [] @@ -1469,7 +1442,10 @@ def _ldap_search(self, all, raw, pkey_only, no_members, **options): try: obj = result[key] except KeyError: - obj = self._get_cert_obj(cert, all, raw, pkey_only) + obj = {} + if not pkey_only and all: + obj['certificate'] = ( + base64.b64encode(cert).decode('ascii')) result[key] = obj if not pkey_only and (all or not no_members): @@ -1477,10 +1453,6 @@ def _ldap_search(self, all, raw, pkey_only, no_members, **options): if entry.dn not in owners: owners.append(entry.dn) - if not raw: - for obj in six.itervalues(result): - self.obj._fill_owners(obj) - return result, truncated, complete def execute(self, criteria=None, all=False, raw=False, pkey_only=False, @@ -1537,6 +1509,38 @@ def execute(self, criteria=None, all=False, raw=False, pkey_only=False, truncated = truncated or sub_truncated complete = complete or sub_complete + ca_objs = {} + ra = self.api.Backend.ra + for key, obj in six.iteritems(result): + serial_number = key[1] + cacn = obj.get('cacn') + + if pkey_only: + obj = {'serial_number': serial_number} + if cacn is not None: + obj['cacn'] = cacn + elif all and cacn is not None: + try: + ca_obj = ca_objs[cacn] + except KeyError: + ca_obj = ca_objs[cacn] = ( + self.api.Command.ca_show(cacn, all=True)['result']) + + obj.update(ra.get_certificate(str(serial_number))) + if not raw: + obj['certificate'] = obj['certificate'].replace('\r\n', '') + + if 'certificate_chain' in ca_obj: + cert = x509.load_certificate(obj['certificate']) + cert_der = cert.public_bytes(serialization.Encoding.DER) + obj['certificate_chain'] = ( + [cert_der] + ca_obj['certificate_chain']) + + if not raw: + if 'certificate' in obj: + self.obj._parse(obj, all) + self.obj._fill_owners(obj) + result = list(six.itervalues(result)) if sizelimit > 0 and len(result) > sizelimit: if not truncated:
-- 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
