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

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