Blame SOURCES/0013-Don-t-store-entries-with-a-usercertificate-in-the-LD_rhbz#1999893.patch

4b8dd5
From be1e3bbfc13aff9a583108376f245b81cc3666fb Mon Sep 17 00:00:00 2001
4b8dd5
From: Rob Crittenden <rcritten@redhat.com>
4b8dd5
Date: Thu, 9 Sep 2021 15:26:55 -0400
4b8dd5
Subject: [PATCH] Don't store entries with a usercertificate in the LDAP cache
4b8dd5
4b8dd5
usercertificate often has a subclass and both the plain and
4b8dd5
subclassed (binary) values are queried. I'm concerned that
4b8dd5
they are used more or less interchangably in places so not
4b8dd5
caching these entries is the safest path forward for now until
4b8dd5
we can dedicate the time to find all usages, determine their
4b8dd5
safety and/or perhaps handle this gracefully within the cache
4b8dd5
now.
4b8dd5
4b8dd5
What we see in this bug is that usercertificate;binary holds the
4b8dd5
first certificate value but a user-mod is done with
4b8dd5
setattr usercertificate=<new_cert>. Since there is no
4b8dd5
usercertificate value (remember, it's usercertificate;binary)
4b8dd5
a replace is done and 389-ds wipes the existing value as we've
4b8dd5
asked it to.
4b8dd5
4b8dd5
I'm not comfortable with simply treating them the same because
4b8dd5
in LDAP they are not.
4b8dd5
4b8dd5
https://pagure.io/freeipa/issue/8986
4b8dd5
4b8dd5
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
4b8dd5
Reviewed-By: Francois Cami <fcami@redhat.com>
4b8dd5
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
4b8dd5
---
4b8dd5
 ipapython/ipaldap.py | 14 +++++++++++---
4b8dd5
 1 file changed, 11 insertions(+), 3 deletions(-)
4b8dd5
4b8dd5
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
4b8dd5
index f94b784d6..ced8f1bd6 100644
4b8dd5
--- a/ipapython/ipaldap.py
4b8dd5
+++ b/ipapython/ipaldap.py
4b8dd5
@@ -1821,9 +1821,17 @@ class LDAPCache(LDAPClient):
4b8dd5
                         entry=None, exception=None):
4b8dd5
         # idnsname - caching prevents delete when mod value to None
4b8dd5
         # cospriority - in a Class of Service object, uncacheable
4b8dd5
-        # TODO - usercertificate was banned at one point and I don't remember
4b8dd5
-        #        why...
4b8dd5
-        BANNED_ATTRS = {'idnsname', 'cospriority'}
4b8dd5
+        # usercertificate* - caching subtypes is tricky, trade less
4b8dd5
+        #                    complexity for performance
4b8dd5
+        #
4b8dd5
+        # TODO: teach the cache about subtypes
4b8dd5
+
4b8dd5
+        BANNED_ATTRS = {
4b8dd5
+            'idnsname',
4b8dd5
+            'cospriority',
4b8dd5
+            'usercertificate',
4b8dd5
+            'usercertificate;binary'
4b8dd5
+        }
4b8dd5
         if not self._enable_cache:
4b8dd5
             return
4b8dd5
 
4b8dd5
-- 
4b8dd5
2.31.1
4b8dd5
4b8dd5
From 86588640137562b2016fdb0f91142d00bc38e54a Mon Sep 17 00:00:00 2001
4b8dd5
From: Rob Crittenden <rcritten@redhat.com>
4b8dd5
Date: Fri, 10 Sep 2021 09:01:48 -0400
4b8dd5
Subject: [PATCH] ipatests: Test that a user can be issued multiple
4b8dd5
 certificates
4b8dd5
4b8dd5
Prevent regressions in the LDAP cache layer that caused newly
4b8dd5
issued certificates to overwrite existing ones.
4b8dd5
4b8dd5
https://pagure.io/freeipa/issue/8986
4b8dd5
4b8dd5
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
4b8dd5
Reviewed-By: Francois Cami <fcami@redhat.com>
4b8dd5
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
4b8dd5
---
4b8dd5
 ipatests/test_integration/test_cert.py | 29 ++++++++++++++++++++++++++
4b8dd5
 1 file changed, 29 insertions(+)
4b8dd5
4b8dd5
diff --git a/ipatests/test_integration/test_cert.py b/ipatests/test_integration/test_cert.py
4b8dd5
index 7d51b76ee..b4e85eadc 100644
4b8dd5
--- a/ipatests/test_integration/test_cert.py
4b8dd5
+++ b/ipatests/test_integration/test_cert.py
4b8dd5
@@ -16,6 +16,7 @@ import string
4b8dd5
 import time
4b8dd5
 
4b8dd5
 from ipaplatform.paths import paths
4b8dd5
+from ipapython.dn import DN
4b8dd5
 from cryptography import x509
4b8dd5
 from cryptography.x509.oid import ExtensionOID
4b8dd5
 from cryptography.hazmat.backends import default_backend
4b8dd5
@@ -183,6 +184,34 @@ class TestInstallMasterClient(IntegrationTest):
4b8dd5
         )
4b8dd5
         assert "profile: caServerCert" in result.stdout_text
4b8dd5
 
4b8dd5
+    def test_multiple_user_certificates(self):
4b8dd5
+        """Test that a user may be issued multiple certificates"""
4b8dd5
+        ldap = self.master.ldap_connect()
4b8dd5
+
4b8dd5
+        user = 'user1'
4b8dd5
+
4b8dd5
+        tasks.kinit_admin(self.master)
4b8dd5
+        tasks.user_add(self.master, user)
4b8dd5
+
4b8dd5
+        for id in (0,1):
4b8dd5
+            csr_file = f'{id}.csr'
4b8dd5
+            key_file = f'{id}.key'
4b8dd5
+            cert_file = f'{id}.crt'
4b8dd5
+            openssl_cmd = [
4b8dd5
+                'openssl', 'req', '-newkey', 'rsa:2048', '-keyout', key_file,
4b8dd5
+                '-nodes', '-out', csr_file, '-subj', '/CN=' + user]
4b8dd5
+            self.master.run_command(openssl_cmd)
4b8dd5
+
4b8dd5
+            cmd_args = ['ipa', 'cert-request', '--principal', user,
4b8dd5
+                        '--certificate-out', cert_file, csr_file]
4b8dd5
+            self.master.run_command(cmd_args)
4b8dd5
+
4b8dd5
+        # easier to count by pulling the LDAP entry
4b8dd5
+        entry = ldap.get_entry(DN(('uid', user), ('cn', 'users'),
4b8dd5
+                               ('cn', 'accounts'), self.master.domain.basedn))
4b8dd5
+
4b8dd5
+        assert len(entry.get('usercertificate')) == 2
4b8dd5
+
4b8dd5
     @pytest.fixture
4b8dd5
     def test_subca_certs(self):
4b8dd5
         """
4b8dd5
-- 
4b8dd5
2.31.1
4b8dd5