URL: https://github.com/freeipa/freeipa/pull/6024 Author: rcritten Title: #6024: [Backport][ipa-4-9] Don't store entries with a usercertificate in the LDAP cache Action: opened
PR body: """ This PR was opened automatically because PR #6015 was pushed to master and backport to ipa-4-9 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/6024/head:pr6024 git checkout pr6024
From 5699e022cc8adbd92f5dd33dd9da22c329835df7 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Thu, 9 Sep 2021 15:26:55 -0400 Subject: [PATCH 1/2] Don't store entries with a usercertificate in the LDAP cache usercertificate often has a subclass and both the plain and subclassed (binary) values are queried. I'm concerned that they are used more or less interchangably in places so not caching these entries is the safest path forward for now until we can dedicate the time to find all usages, determine their safety and/or perhaps handle this gracefully within the cache now. What we see in this bug is that usercertificate;binary holds the first certificate value but a user-mod is done with setattr usercertificate=<new_cert>. Since there is no usercertificate value (remember, it's usercertificate;binary) a replace is done and 389-ds wipes the existing value as we've asked it to. I'm not comfortable with simply treating them the same because in LDAP they are not. https://pagure.io/freeipa/issue/8986 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipapython/ipaldap.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index f94b784d680..ced8f1bd66d 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -1821,9 +1821,17 @@ def add_cache_entry(self, dn, attrs_list=None, get_all=False, entry=None, exception=None): # idnsname - caching prevents delete when mod value to None # cospriority - in a Class of Service object, uncacheable - # TODO - usercertificate was banned at one point and I don't remember - # why... - BANNED_ATTRS = {'idnsname', 'cospriority'} + # usercertificate* - caching subtypes is tricky, trade less + # complexity for performance + # + # TODO: teach the cache about subtypes + + BANNED_ATTRS = { + 'idnsname', + 'cospriority', + 'usercertificate', + 'usercertificate;binary' + } if not self._enable_cache: return From dc0ac3d7913f0a50f83cd35187603db7509a3275 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Fri, 10 Sep 2021 09:01:48 -0400 Subject: [PATCH 2/2] ipatests: Test that a user can be issued multiple certificates Prevent regressions in the LDAP cache layer that caused newly issued certificates to overwrite existing ones. https://pagure.io/freeipa/issue/8986 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipatests/test_integration/test_cert.py | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/ipatests/test_integration/test_cert.py b/ipatests/test_integration/test_cert.py index 7d51b76ee34..b4e85eadcf4 100644 --- a/ipatests/test_integration/test_cert.py +++ b/ipatests/test_integration/test_cert.py @@ -16,6 +16,7 @@ import time from ipaplatform.paths import paths +from ipapython.dn import DN from cryptography import x509 from cryptography.x509.oid import ExtensionOID from cryptography.hazmat.backends import default_backend @@ -183,6 +184,34 @@ def test_getcert_list_profile(self): ) assert "profile: caServerCert" in result.stdout_text + def test_multiple_user_certificates(self): + """Test that a user may be issued multiple certificates""" + ldap = self.master.ldap_connect() + + user = 'user1' + + tasks.kinit_admin(self.master) + tasks.user_add(self.master, user) + + for id in (0,1): + csr_file = f'{id}.csr' + key_file = f'{id}.key' + cert_file = f'{id}.crt' + openssl_cmd = [ + 'openssl', 'req', '-newkey', 'rsa:2048', '-keyout', key_file, + '-nodes', '-out', csr_file, '-subj', '/CN=' + user] + self.master.run_command(openssl_cmd) + + cmd_args = ['ipa', 'cert-request', '--principal', user, + '--certificate-out', cert_file, csr_file] + self.master.run_command(cmd_args) + + # easier to count by pulling the LDAP entry + entry = ldap.get_entry(DN(('uid', user), ('cn', 'users'), + ('cn', 'accounts'), self.master.domain.basedn)) + + assert len(entry.get('usercertificate')) == 2 + @pytest.fixture def test_subca_certs(self): """
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
