|
|
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 |
|