From 9a576f7303fd1b7faae3a419f6d54720f234585b Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 22 Jun 2018 10:04:38 +0200 Subject: [PATCH] Fix replication races in Dogtag admin code DogtagInstance.setup_admin and related methods have multiple LDAP replication race conditions. The bugs can cause parallel ipa-replica-install to fail. The code from __add_admin_to_group() has been changed to use MOD_ADD ather than search + MOD_REPLACE. The MOD_REPLACE approach can lead to data loss, when more than one writer changes a group. setup_admin() now waits until both admin user and group membership have been replicated to the master peer. The method also adds a new ACI to allow querying group member in the replication check. Fixes: https://pagure.io/freeipa/issue/7593 Signed-off-by: Christian Heimes Reviewed-By: Fraser Tweedale --- ipaserver/install/cainstance.py | 1 + ipaserver/install/dogtaginstance.py | 101 ++++++++++++++++++++++------ ipaserver/install/krainstance.py | 1 + 3 files changed, 82 insertions(+), 21 deletions(-) diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index 3a3558a5ded6de3e578574d08a5cdb59338e99c4..0c4d9bf9ad8ae11ac88523857845e16eb62651b9 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -377,6 +377,7 @@ class CAInstance(DogtagInstance): # Setup Database self.step("creating certificate server db", self.__create_ds_db) self.step("setting up initial replication", self.__setup_replication) + self.step("creating ACIs for admin", self.add_ipaca_aci) self.step("creating installation admin user", self.setup_admin) self.step("configuring certificate server instance", self.__spawn_instance) diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py index 9470e1a13608a8a84aab8a36c269a708e3f3e9f4..960b8cc7ce495bf5ca359f72b46aa0d43ccec5c3 100644 --- a/ipaserver/install/dogtaginstance.py +++ b/ipaserver/install/dogtaginstance.py @@ -18,6 +18,8 @@ # import base64 +import time + import ldap import os import shutil @@ -84,6 +86,16 @@ class DogtagInstance(service.Service): tracking_reqs = None server_cert_name = None + ipaca_groups = DN(('ou', 'groups'), ('o', 'ipaca')) + ipaca_people = DN(('ou', 'people'), ('o', 'ipaca')) + groups_aci = ( + b'(targetfilter="(objectClass=groupOfUniqueNames)")' + b'(targetattr="cn || description || objectclass || uniquemember")' + b'(version 3.0; acl "Allow users from o=ipaca to read groups"; ' + b'allow (read, search, compare) ' + b'userdn="ldap:///uid=*,ou=people,o=ipaca";)' + ) + def __init__(self, realm, subsystem, service_desc, host_name=None, nss_db=paths.PKI_TOMCAT_ALIAS_DIR, service_prefix=None): """Initializer""" @@ -101,10 +113,11 @@ class DogtagInstance(service.Service): self.pkcs12_info = None self.clone = False - self.basedn = DN(('o', 'ipa%s' % subsystem.lower())) + self.basedn = DN(('o', 'ipaca')) self.admin_user = "admin" - self.admin_dn = DN(('uid', self.admin_user), - ('ou', 'people'), ('o', 'ipaca')) + self.admin_dn = DN( + ('uid', self.admin_user), self.ipaca_people + ) self.admin_groups = None self.tmp_agent_db = None self.subsystem = subsystem @@ -385,27 +398,33 @@ class DogtagInstance(service.Service): raise RuntimeError("%s configuration failed." % self.subsystem) - def __add_admin_to_group(self, group): - dn = DN(('cn', group), ('ou', 'groups'), ('o', 'ipaca')) - entry = api.Backend.ldap2.get_entry(dn) - members = entry.get('uniqueMember', []) - members.append(self.admin_dn) - mod = [(ldap.MOD_REPLACE, 'uniqueMember', members)] + def add_ipaca_aci(self): + """Add ACI to allow ipaca users to read their own group information + + Dogtag users aren't allowed to enumerate their own groups. The + setup_admin() method needs the permission to wait, until all group + information has been replicated. + """ + dn = self.ipaca_groups + mod = [(ldap.MOD_ADD, 'aci', [self.groups_aci])] try: api.Backend.ldap2.modify_s(dn, mod) except ldap.TYPE_OR_VALUE_EXISTS: - # already there - pass + self.log.debug( + "%s already has ACI to read group information", dn + ) + else: + self.log.debug("Added ACI to read groups to %s", dn) def setup_admin(self): self.admin_user = "admin-%s" % self.fqdn self.admin_password = ipautil.ipa_generate_password() - self.admin_dn = DN(('uid', self.admin_user), - ('ou', 'people'), ('o', 'ipaca')) - + self.admin_dn = DN( + ('uid', self.admin_user), self.ipaca_people + ) # remove user if left-over exists try: - entry = api.Backend.ldap2.delete_entry(self.admin_dn) + api.Backend.ldap2.delete_entry(self.admin_dn) except errors.NotFound: pass @@ -424,18 +443,58 @@ class DogtagInstance(service.Service): ) api.Backend.ldap2.add_entry(entry) + wait_groups = [] for group in self.admin_groups: - self.__add_admin_to_group(group) + group_dn = DN(('cn', group), self.ipaca_groups) + mod = [(ldap.MOD_ADD, 'uniqueMember', [self.admin_dn])] + try: + api.Backend.ldap2.modify_s(group_dn, mod) + except ldap.TYPE_OR_VALUE_EXISTS: + # already there + return None + else: + wait_groups.append(group_dn) # Now wait until the other server gets replicated this data ldap_uri = ipaldap.get_ldap_uri(self.master_host) - master_conn = ipaldap.LDAPClient(ldap_uri) - master_conn.gssapi_bind() - replication.wait_for_entry(master_conn, entry.dn) - del master_conn + master_conn = ipaldap.LDAPClient( + ldap_uri, start_tls=True, cacert=paths.IPA_CA_CRT + ) + self.log.debug( + "Waiting for %s to appear on %s", self.admin_dn, master_conn + ) + deadline = time.time() + api.env.replication_wait_timeout + while time.time() < deadline: + time.sleep(1) + try: + master_conn.simple_bind(self.admin_dn, self.admin_password) + except ldap.INVALID_CREDENTIALS: + pass + else: + self.log.debug("Successfully logged in as %s", self.admin_dn) + break + else: + self.log.error( + "Unable to log in as %s on %s", self.admin_dn, master_conn + ) + raise errors.NotFound( + reason="{} did not replicate to {}".format( + self.admin_dn, master_conn + ) + ) + + # wait for group membership + for group_dn in wait_groups: + replication.wait_for_entry( + master_conn, + group_dn, + timeout=api.env.replication_wait_timeout, + attr='uniqueMember', + attrvalue=self.admin_dn + ) def __remove_admin_from_group(self, group): - dn = DN(('cn', group), ('ou', 'groups'), ('o', 'ipaca')) + dn = DN(('cn', group), self.ipaca_groups) mod = [(ldap.MOD_DELETE, 'uniqueMember', self.admin_dn)] try: api.Backend.ldap2.modify_s(dn, mod) diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py index 990bb87ca2f0029d2450cbef47958399f534f2a6..915d3c3c6e038eeb6a8f94f1ed7f7008c0ef4ead 100644 --- a/ipaserver/install/krainstance.py +++ b/ipaserver/install/krainstance.py @@ -111,6 +111,7 @@ class KRAInstance(DogtagInstance): "A Dogtag CA must be installed first") if promote: + self.step("creating ACIs for admin", self.add_ipaca_aci) self.step("creating installation admin user", self.setup_admin) self.step("configuring KRA instance", self.__spawn_instance) if not self.clone: -- 2.17.1