rpms/ipa
7
0
Fork 0
Code Issues Pull Requests Releases Activity
ff8b7c7405
ipa/SOURCES/0002-Use-the-python-cryptography-parser-directly-in-cert-find_rhbz#2164349.patch

243 lines
8.8 KiB
Diff
Raw Blame History

From 9fe30f21c987bdccf80ef5f6d645fdc59b393bdb Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Jun 16 2023 19:09:52 +0000
Subject: Revert "Use the OpenSSL certificate parser in cert-find"
This reverts commit 191880bc9f77c3e8a3cecc82e6eea33ab5ad03e4.
The problem isn't with python-cryptography, it is with the
IPACertificate class which does way more work on a certificate
than is necessary in cert-find.
Related: https://pagure.io/freeipa/issue/9331
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
---
diff --git a/freeipa.spec.in b/freeipa.spec.in
index f3380b4..2b18963 100755
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -390,7 +390,6 @@ BuildRequires: python3-pylint
BuildRequires: python3-pytest-multihost
BuildRequires: python3-pytest-sourceorder
BuildRequires: python3-qrcode-core >= 5.0.0
-BuildRequires: python3-pyOpenSSL
BuildRequires: python3-samba
BuildRequires: python3-six
BuildRequires: python3-sss
@@ -862,7 +861,6 @@ Requires: python3-netifaces >= 0.10.4
Requires: python3-pyasn1 >= 0.3.2-2
Requires: python3-pyasn1-modules >= 0.3.2-2
Requires: python3-pyusb
-Requires: python3-pyOpenSSL
Requires: python3-qrcode-core >= 5.0.0
Requires: python3-requests
Requires: python3-six
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index cec3d93..88c6b62 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -30,7 +30,6 @@ import cryptography.x509
from cryptography.hazmat.primitives import hashes, serialization
from dns import resolver, reversename
import six
-import sys
from ipalib import Command, Str, Int, Flag, StrEnum
from ipalib import api
@@ -1623,19 +1622,7 @@ class cert_find(Search, CertMethod):
)
def _get_cert_key(self, cert):
- # for cert-find with a certificate value
- if isinstance(cert, x509.IPACertificate):
- return (DN(cert.issuer), cert.serial_number)
-
- issuer = []
- for oid, value in cert.get_issuer().get_components():
- issuer.append(
- '{}={}'.format(oid.decode('utf-8'), value.decode('utf-8'))
- )
- issuer = ','.join(issuer)
- # Use this to flip from OpenSSL reverse to X500 ordering
- issuer = DN(issuer).x500_text()
- return (DN(issuer), cert.get_serial_number())
+ return (DN(cert.issuer), cert.serial_number)
def _cert_search(self, pkey_only, **options):
result = collections.OrderedDict()
@@ -1755,11 +1742,6 @@ class cert_find(Search, CertMethod):
return result, False, complete
def _ldap_search(self, all, pkey_only, no_members, **options):
- # defer import of the OpenSSL module to not affect the requests
- # module which will use pyopenssl if this is available.
- if sys.modules.get('OpenSSL.SSL', False) is None:
- del sys.modules["OpenSSL.SSL"]
- import OpenSSL.crypto
ldap = self.api.Backend.ldap2
filters = []
@@ -1818,14 +1800,12 @@ class cert_find(Search, CertMethod):
ca_enabled = getattr(context, 'ca_enabled')
for entry in entries:
for attr in ('usercertificate', 'usercertificate;binary'):
- for der in entry.raw.get(attr, []):
- cert = OpenSSL.crypto.load_certificate(
- OpenSSL.crypto.FILETYPE_ASN1, der)
+ for cert in entry.get(attr, []):
cert_key = self._get_cert_key(cert)
try:
obj = result[cert_key]
except KeyError:
- obj = {'serial_number': cert.get_serial_number()}
+ obj = {'serial_number': cert.serial_number}
if not pkey_only and (all or not ca_enabled):
# Retrieving certificate details is now deferred
# until after all certificates are collected.
From 3b1dbcdba2994bf57908f530913998e9ab888e4c Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Jun 16 2023 19:09:52 +0000
Subject: Revert "cert_find: fix call with --all"
This reverts commit 1f30cc65276a532e7288217f216b72a2b0628c8f.
The problem isn't with python-cryptography, it is with the
IPACertificate class which does way more work on a certificate
than is necessary in cert-find.
Related: https://pagure.io/freeipa/issue/9331
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
---
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 88c6b62..ba37525 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -1812,7 +1812,6 @@ class cert_find(Search, CertMethod):
# For the case of CA-less we need to keep
# the certificate because getting it again later
# would require unnecessary LDAP searches.
- cert = cert.to_cryptography()
obj['certificate'] = (
base64.b64encode(
cert.public_bytes(x509.Encoding.DER))
From d00fd3398c32beb2c3e72f4878c87f9d2c0e833d Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Jun 16 2023 19:09:52 +0000
Subject: Use the python-cryptography parser directly in cert-find
cert-find is a rather complex beast because it not only
looks for certificates in the optional CA but within the
IPA LDAP database as well. It has a process to deduplicate
the certificates since any PKI issued certificates will
also be associated with an IPA record.
In order to obtain the data to deduplicate the certificates
the cert from LDAP must be parser for issuer and serial number.
ipaldap has automation to determine the datatype of an
attribute and will use the ipalib.x509 IPACertificate class to
decode a certificate automatically if you access
entry['usercertificate'].
The downside is that this is comparatively slow. Here is the
parse time in microseconds:
cryptography 0.0081
OpenSSL.crypto 0.2271
ipalib.x509 2.6814
Since only issuer and subject are required there is no need to
make the expensive IPACertificate call.
The IPACertificate parsing time is fine if you're parsing one
certificate but if the LDAP search returns a lot of certificates,
say in the thousands, then those microseconds add up quickly.
In testing it took ~17 seconds to parse 5k certificates (excluding
transmission overhead, etc).
cert-find when there are a lot of certificates has been
historically slow. It isn't related to the CA which returns
large sets (well, 5k anyway) in a second or two. It was the
LDAP comparision adding tens of seconds to the runtime.
When searching with the default sizelimit of 100 the time is
~10s without this patch. With it the time is 1.5s.
CLI times from before and after searching for all certs:
original:
-------------------------------
Number of entries returned 5038
-------------------------------
real 0m15.507s
user 0m0.828s
sys 0m0.241s
using cryptography:
real 0m4.037s
user 0m0.816s
sys 0m0.193s
Fixes: https://pagure.io/freeipa/issue/9331
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
---
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index ba37525..619be83 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -1800,7 +1800,8 @@ class cert_find(Search, CertMethod):
ca_enabled = getattr(context, 'ca_enabled')
for entry in entries:
for attr in ('usercertificate', 'usercertificate;binary'):
- for cert in entry.get(attr, []):
+ for der in entry.raw.get(attr, []):
+ cert = cryptography.x509.load_der_x509_certificate(der)
cert_key = self._get_cert_key(cert)
try:
obj = result[cert_key]
diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index 433cebc..583c67f 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -254,6 +254,16 @@ class test_cert(BaseCert):
result = _emails_are_valid(email_addrs, [])
assert not result
+ def test_00012_cert_find_all(self):
+ """
+ Test that cert-find --all returns successfully.
+
+ We don't know how many we'll get but there should be at least 10
+ by default.
+ """
+ res = api.Command['cert_find'](all=True)
+ assert 'count' in res and res['count'] >= 10
+
def test_99999_cleanup(self):
"""
Clean up cert test data
@@ -283,7 +293,7 @@ class test_cert_find(XMLRPC_test):
short = api.env.host.split('.', maxsplit=1)[0]
- def test_0001_find_all(self):
+ def test_0001_find_all_certs(self):
"""
Search for all certificates.
Reference in New Issue View Git Blame Copy Permalink