URL: https://github.com/freeipa/freeipa/pull/998
Author: stlaz
 Title: #998: Unify storing certificates in LDAP
Action: opened

PR body:
"""
Recent certificate refactoring left the system in a state where
the certificates are somewhere converted to DER format, somewhere
directly sent to ipaldap as IPACertificate objects. The latter
is the desirable way, make sure it's the one commonly used.

https://pagure.io/freeipa/issue/4985
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/998/head:pr998
git checkout pr998
From 97411e8e1b898e2ff11f2f4828ffbdb886f9eaad Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Wed, 23 Aug 2017 15:23:43 +0200
Subject: [PATCH] Unify storing certificates in LDAP

Recent certificate refactoring left the system in a state where
the certificates are somewhere converted to DER format, somewhere
directly sent to ipaldap as IPACertificate objects. The latter
is the desirable way, make sure it's the one commonly used.

https://pagure.io/freeipa/issue/4985
---
 ipalib/install/certstore.py               | 16 +++++++---------
 ipaserver/install/cainstance.py           |  5 +----
 ipaserver/install/ipa_cacert_manage.py    |  5 ++---
 ipaserver/install/krainstance.py          |  3 +--
 ipaserver/install/plugins/upload_cacrt.py |  7 +++----
 ipaserver/install/service.py              |  3 +--
 ipaserver/plugins/host.py                 |  9 ++++-----
 ipaserver/plugins/service.py              |  3 +--
 8 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/ipalib/install/certstore.py b/ipalib/install/certstore.py
index b26b8a7d22..481918b996 100644
--- a/ipalib/install/certstore.py
+++ b/ipalib/install/certstore.py
@@ -68,7 +68,7 @@ def init_ca_entry(entry, cert, nickname, trusted, ext_key_usage):
     entry['ipaCertSubject'] = [subject]
     entry['ipaCertIssuerSerial'] = [issuer_serial]
     entry['ipaPublicKey'] = [public_key]
-    entry['cACertificate;binary'] = [cert.public_bytes(x509.Encoding.DER)]
+    entry['cACertificate;binary'] = [cert]
 
     if trusted is not None:
         entry['ipaKeyTrust'] = ['trusted' if trusted else 'distrusted']
@@ -84,16 +84,15 @@ def update_compat_ca(ldap, base_dn, cert):
     Update the CA certificate in cn=CAcert,cn=ipa,cn=etc,SUFFIX.
     """
     dn = DN(('cn', 'CAcert'), ('cn', 'ipa'), ('cn', 'etc'), base_dn)
-    dercert = cert.public_bytes(x509.Encoding.DER)
     try:
         entry = ldap.get_entry(dn, attrs_list=['cACertificate;binary'])
-        entry.single_value['cACertificate;binary'] = dercert
+        entry.single_value['cACertificate;binary'] = cert
         ldap.update_entry(entry)
     except errors.NotFound:
         entry = ldap.make_entry(dn)
         entry['objectClass'] = ['nsContainer', 'pkiCA']
         entry.single_value['cn'] = 'CAcert'
-        entry.single_value['cACertificate;binary'] = dercert
+        entry.single_value['cACertificate;binary'] = cert
         ldap.add_entry(entry)
     except errors.EmptyModlist:
         pass
@@ -129,7 +128,7 @@ def clean_old_config(ldap, base_dn, dn, config_ipa, config_compat):
             pass
 
 
-def add_ca_cert(ldap, base_dn, dercert, nickname, trusted=None,
+def add_ca_cert(ldap, base_dn, cert, nickname, trusted=None,
                 ext_key_usage=None, config_ipa=False, config_compat=False):
     """
     Add new entry for a CA certificate to the certificate store.
@@ -139,7 +138,7 @@ def add_ca_cert(ldap, base_dn, dercert, nickname, trusted=None,
     dn = DN(('cn', nickname), container_dn)
     entry = ldap.make_entry(dn)
 
-    init_ca_entry(entry, dercert, nickname, trusted, ext_key_usage)
+    init_ca_entry(entry, cert, nickname, trusted, ext_key_usage)
 
     if config_ipa:
         entry.setdefault('ipaConfigString', []).append('ipaCA')
@@ -147,7 +146,7 @@ def add_ca_cert(ldap, base_dn, dercert, nickname, trusted=None,
         entry.setdefault('ipaConfigString', []).append('compatCA')
 
     if config_compat:
-        update_compat_ca(ldap, base_dn, dercert)
+        update_compat_ca(ldap, base_dn, cert)
 
     ldap.add_entry(entry)
     clean_old_config(ldap, base_dn, dn, config_ipa, config_compat)
@@ -182,8 +181,7 @@ def update_ca_cert(ldap, base_dn, cert, trusted=None, ext_key_usage=None,
         if entry.single_value['ipaPublicKey'] != public_key:
             raise ValueError("subject public key info mismatch")
         entry['ipaCertIssuerSerial'].append(issuer_serial)
-        entry['cACertificate;binary'].append(
-            cert.public_bytes(x509.Encoding.DER))
+        entry['cACertificate;binary'].append(cert)
 
     # Update key trust
     if trusted is not None:
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 7f43249995..52840a7299 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -730,9 +730,6 @@ def __create_ca_agent(self):
         the appropriate groups for accessing CA services.
         """
 
-        # get RA certificate
-        cert_data = self.ra_cert.public_bytes(serialization.Encoding.DER)
-
         # connect to CA database
         conn = ldap2.ldap2(api)
         conn.connect(autobind=True)
@@ -748,7 +745,7 @@ def __create_ca_agent(self):
             cn=["ipara"],
             usertype=["agentType"],
             userstate=["1"],
-            userCertificate=[cert_data],
+            userCertificate=[self.ra_cert],
             description=['2;%s;%s;%s' % (
                 self.ra_cert.serial_number,
                 DN(self.ca_subject),
diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py
index 86243d342b..cd8e0c97d8 100644
--- a/ipaserver/install/ipa_cacert_manage.py
+++ b/ipaserver/install/ipa_cacert_manage.py
@@ -275,17 +275,16 @@ def renew_external_step_2(self, ca, old_cert):
         dn = DN(('cn', self.cert_nickname), ('cn', 'ca_renewal'),
                 ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn)
 
-        new_cert_der = new_cert.public_bytes(x509.Encoding.DER)
         try:
             entry = conn.get_entry(dn, ['usercertificate'])
-            entry['usercertificate'] = [new_cert_der]
+            entry['usercertificate'] = [new_cert]
             conn.update_entry(entry)
         except errors.NotFound:
             entry = conn.make_entry(
                 dn,
                 objectclass=['top', 'pkiuser', 'nscontainer'],
                 cn=[self.cert_nickname],
-                usercertificate=[new_cert_der])
+                usercertificate=[new_cert])
             conn.add_entry(entry)
         except errors.EmptyModlist:
             pass
diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py
index 79ad853b9f..c1af7c0bd6 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -306,7 +306,6 @@ def __create_kra_agent(self):
 
         # get RA agent certificate
         cert = x509.load_certificate_from_file(paths.RA_AGENT_PEM)
-        cert_data = cert.public_bytes(x509.Encoding.DER)
 
         # connect to KRA database
         conn = ldap2.ldap2(api)
@@ -322,7 +321,7 @@ def __create_kra_agent(self):
             sn=["IPA KRA User"],
             cn=["IPA KRA User"],
             usertype=["undefined"],
-            userCertificate=[cert_data],
+            userCertificate=[cert],
             description=['2;%s;%s;%s' % (
                 cert.serial_number,
                 DN(self.subject),
diff --git a/ipaserver/install/plugins/upload_cacrt.py b/ipaserver/install/plugins/upload_cacrt.py
index 25faa6e77c..a71ba602c5 100644
--- a/ipaserver/install/plugins/upload_cacrt.py
+++ b/ipaserver/install/plugins/upload_cacrt.py
@@ -22,7 +22,7 @@
 from ipalib.install import certstore
 from ipaplatform.paths import paths
 from ipaserver.install import certs
-from ipalib import Registry, errors, x509
+from ipalib import Registry, errors
 from ipalib import Updater
 from ipapython import certdb
 from ipapython.dn import DN
@@ -90,7 +90,6 @@ def execute(self, **options):
                         pass
 
         if ca_cert:
-            dercert = ca_cert.public_bytes(x509.Encoding.DER)
             dn = DN(('cn', 'CACert'), ('cn', 'ipa'), ('cn','etc'),
                     self.api.env.basedn)
             try:
@@ -99,11 +98,11 @@ def execute(self, **options):
                 entry = ldap.make_entry(dn)
                 entry['objectclass'] = ['nsContainer', 'pkiCA']
                 entry.single_value['cn'] = 'CAcert'
-                entry.single_value['cACertificate;binary'] = dercert
+                entry.single_value['cACertificate;binary'] = ca_cert
                 ldap.add_entry(entry)
             else:
                 if b'' in entry['cACertificate;binary']:
-                    entry.single_value['cACertificate;binary'] = dercert
+                    entry.single_value['cACertificate;binary'] = ca_cert
                     ldap.update_entry(entry)
 
         return False, []
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index d2c3bbd5b2..1aadee15eb 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -370,8 +370,7 @@ def add_cert_to_service(self):
         dn = DN(('krbprincipalname', self.principal), ('cn', 'services'),
                 ('cn', 'accounts'), self.suffix)
         entry = api.Backend.ldap2.get_entry(dn)
-        entry.setdefault('userCertificate', []).append(
-            self.cert.public_bytes(x509.Encoding.DER))
+        entry.setdefault('userCertificate', []).append(self.cert)
         try:
             api.Backend.ldap2.update_entry(entry)
         except Exception as e:
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 88e2a327be..2e357c5db0 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -902,9 +902,9 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             except errors.NotFound:
                 self.obj.handle_not_found(*keys)
             old_certs = entry_attrs_old.get('usercertificate', [])
-            removed_certs_der = set(old_certs) - set(certs)
-            for der in removed_certs_der:
-                rm_certs = api.Command.cert_find(certificate=der)['result']
+            removed_certs = set(old_certs) - set(certs)
+            for cert in removed_certs:
+                rm_certs = api.Command.cert_find(certificate=cert)['result']
                 revoke_certs(rm_certs)
 
         if certs:
@@ -1340,8 +1340,7 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
 
         for cert in options.get('usercertificate', []):
-            revoke_certs(api.Command.cert_find(
-                certificate=cert.public_bytes(x509_Encoding.DER))['result'])
+            revoke_certs(api.Command.cert_find(certificate=cert)['result'])
 
         return dn
 
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index ff8f6be437..9ff361fbbc 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -983,8 +983,7 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
 
         for cert in options.get('usercertificate', []):
-            revoke_certs(api.Command.cert_find(
-                certificate=cert.public_bytes(x509.Encoding.DER))['result'])
+            revoke_certs(api.Command.cert_find(certificate=cert)['result'])
 
         return dn
 
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to