From 0598ed6e1c48c6600774c53065716aad759da5e7 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Tue, 10 Jul 2018 14:03:28 +0200 Subject: [PATCH] Handle races in replica config When multiple replicas are installed in parallel, two replicas may try to create the cn=replica entry at the same time. This leads to a conflict on one of the replicas. replica_config() and ensure_replication_managers() now handle conflicts. ipaldap now maps TYPE_OR_VALUE_EXISTS to DuplicateEntry(). The type or value exists exception is raised, when an attribute value or type is already set. Fixes: https://pagure.io/freeipa/issue/7566 Signed-off-by: Christian Heimes Reviewed-By: Thierry Bordaz --- ipapython/ipaldap.py | 5 ++ ipaserver/install/replication.py | 123 ++++++++++++++++++------------- ipaserver/plugins/ldap2.py | 3 +- 3 files changed, 79 insertions(+), 52 deletions(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index dcac32c31c670753b83f99eac7f6483fbad2b14d..53fdf4967868961effea7f3f64dfb3c0edfc75f3 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -1016,7 +1016,12 @@ class LDAPClient(object): except ldap.NO_SUCH_OBJECT: raise errors.NotFound(reason=arg_desc or 'no such entry') except ldap.ALREADY_EXISTS: + # entry already exists raise errors.DuplicateEntry() + except ldap.TYPE_OR_VALUE_EXISTS: + # attribute type or attribute value already exists, usually only + # occurs, when two machines try to write at the same time. + raise errors.DuplicateEntry(message=unicode(desc)) except ldap.CONSTRAINT_VIOLATION: # This error gets thrown by the uniqueness plugin _msg = 'Another entry with the same attribute value already exists' diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py index d0f92c76c108d237104a81c72567250583ac4ff1..78c4a43cc9b8d9a6740d26209f347f64b879743e 100644 --- a/ipaserver/install/replication.py +++ b/ipaserver/install/replication.py @@ -34,7 +34,7 @@ import ldap from ipalib import api, errors from ipalib.cli import textui from ipalib.text import _ -from ipapython import ipautil, ipaldap, kerberos +from ipapython import ipautil, ipaldap from ipapython.admintool import ScriptError from ipapython.dn import DN from ipapython.ipaldap import ldap_initialize @@ -461,7 +461,7 @@ class ReplicationManager(object): return DN(('cn', 'replica'), ('cn', self.db_suffix), ('cn', 'mapping tree'), ('cn', 'config')) - def set_replica_binddngroup(self, r_conn, entry): + def _set_replica_binddngroup(self, r_conn, entry): """ Set nsds5replicabinddngroup attribute on remote master's replica entry. Older masters (ipa < 3.3) may not support setting this attribute. In @@ -476,11 +476,6 @@ class ReplicationManager(object): mod.append((ldap.MOD_ADD, 'nsds5replicabinddngroup', self.repl_man_group_dn)) - if 'nsds5replicabinddngroupcheckinterval' not in entry: - mod.append( - (ldap.MOD_ADD, - 'nsds5replicabinddngroupcheckinterval', - '60')) if mod: try: r_conn.modify_s(entry.dn, mod) @@ -488,49 +483,62 @@ class ReplicationManager(object): logger.debug( "nsds5replicabinddngroup attribute not supported on " "remote master.") + except (ldap.ALREADY_EXISTS, ldap.CONSTRAINT_VIOLATION): + logger.debug("No update to %s necessary", entry.dn) def replica_config(self, conn, replica_id, replica_binddn): assert isinstance(replica_binddn, DN) dn = self.replica_dn() assert isinstance(dn, DN) + logger.debug("Add or update replica config %s", dn) try: entry = conn.get_entry(dn) except errors.NotFound: - pass - else: - binddns = entry.setdefault('nsDS5ReplicaBindDN', []) - if replica_binddn not in {DN(m) for m in binddns}: - # Add the new replication manager - binddns.append(replica_binddn) - for key, value in REPLICA_CREATION_SETTINGS.items(): - entry[key] = value + # no entry, create new one + entry = conn.make_entry( + dn, + objectclass=["top", "nsds5replica", "extensibleobject"], + cn=["replica"], + nsds5replicaroot=[str(self.db_suffix)], + nsds5replicaid=[str(replica_id)], + nsds5replicatype=[self.get_replica_type()], + nsds5flags=["1"], + nsds5replicabinddn=[replica_binddn], + nsds5replicabinddngroup=[self.repl_man_group_dn], + nsds5replicalegacyconsumer=["off"], + **REPLICA_CREATION_SETTINGS + ) try: - conn.update_entry(entry) - except errors.EmptyModlist: - pass + conn.add_entry(entry) + except errors.DuplicateEntry: + logger.debug("Lost race against another replica, updating") + # fetch entry that have been added by another replica + entry = conn.get_entry(dn) + else: + logger.debug("Added replica config %s", dn) + # added entry successfully + return entry - self.set_replica_binddngroup(conn, entry) + # either existing entry or lost race + binddns = entry.setdefault('nsDS5ReplicaBindDN', []) + if replica_binddn not in {DN(m) for m in binddns}: + # Add the new replication manager + binddns.append(replica_binddn) - # replication is already configured - return + for key, value in REPLICA_CREATION_SETTINGS.items(): + entry[key] = value - replica_type = self.get_replica_type() + try: + conn.update_entry(entry) + except errors.EmptyModlist: + logger.debug("No update to %s necessary", entry.dn) + else: + logger.debug("Update replica config %s", entry.dn) - entry = conn.make_entry( - dn, - objectclass=["top", "nsds5replica", "extensibleobject"], - cn=["replica"], - nsds5replicaroot=[str(self.db_suffix)], - nsds5replicaid=[str(replica_id)], - nsds5replicatype=[replica_type], - nsds5flags=["1"], - nsds5replicabinddn=[replica_binddn], - nsds5replicabinddngroup=[self.repl_man_group_dn], - nsds5replicalegacyconsumer=["off"], - **REPLICA_CREATION_SETTINGS - ) - conn.add_entry(entry) + self._set_replica_binddngroup(conn, entry) + + return entry def setup_changelog(self, conn): ent = conn.get_entry( @@ -690,7 +698,10 @@ class ReplicationManager(object): uid=["passsync"], userPassword=[password], ) - conn.add_entry(entry) + try: + conn.add_entry(entry) + except errors.DuplicateEntry: + pass # Add the user to the list of users allowed to bypass password policy extop_dn = DN(('cn', 'ipa_pwd_extop'), ('cn', 'plugins'), ('cn', 'config')) @@ -1658,7 +1669,10 @@ class ReplicationManager(object): objectclass=['top', 'groupofnames'], cn=['replication managers'] ) - conn.add_entry(entry) + try: + conn.add_entry(entry) + except errors.DuplicateEntry: + pass def ensure_replication_managers(self, conn, r_hostname): """ @@ -1668,23 +1682,24 @@ class ReplicationManager(object): On FreeIPA 3.x masters lacking support for nsds5ReplicaBinddnGroup attribute, add replica bind DN directly into the replica entry. """ - my_princ = kerberos.Principal((u'ldap', unicode(self.hostname)), - realm=self.realm) - remote_princ = kerberos.Principal((u'ldap', unicode(r_hostname)), - realm=self.realm) - services_dn = DN(api.env.container_service, api.env.basedn) - - mydn, remote_dn = tuple( - DN(('krbprincipalname', unicode(p)), services_dn) for p in ( - my_princ, remote_princ)) + my_dn = DN( + ('krbprincipalname', u'ldap/%s@%s' % (self.hostname, self.realm)), + api.env.container_service, + api.env.basedn + ) + remote_dn = DN( + ('krbprincipalname', u'ldap/%s@%s' % (r_hostname, self.realm)), + api.env.container_service, + api.env.basedn + ) try: conn.get_entry(self.repl_man_group_dn) except errors.NotFound: - self._add_replica_bind_dn(conn, mydn) + self._add_replica_bind_dn(conn, my_dn) self._add_replication_managers(conn) - self._add_dn_to_replication_managers(conn, mydn) + self._add_dn_to_replication_managers(conn, my_dn) self._add_dn_to_replication_managers(conn, remote_dn) def add_temp_sasl_mapping(self, conn, r_hostname): @@ -1704,7 +1719,10 @@ class ReplicationManager(object): entry = conn.get_entry(self.replica_dn()) entry['nsDS5ReplicaBindDN'].append(replica_binddn) - conn.update_entry(entry) + try: + conn.update_entry(entry) + except errors.EmptyModlist: + pass entry = conn.make_entry( DN(('cn', 'Peer Master'), ('cn', 'mapping'), ('cn', 'sasl'), @@ -1716,7 +1734,10 @@ class ReplicationManager(object): nsSaslMapFilterTemplate=['(cn=&@%s)' % self.realm], nsSaslMapPriority=['1'], ) - conn.add_entry(entry) + try: + conn.add_entry(entry) + except errors.DuplicateEntry: + pass def remove_temp_replication_user(self, conn, r_hostname): """ diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index 8745866ad11cf6248d2b9dcff560c0a5ee6d109b..5d39cdf628d7e20ce5b259d97b479ef7c8ed88d5 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -419,7 +419,8 @@ class ldap2(CrudBackend, LDAPClient): modlist = [(a, b, self.encode(c)) for a, b, c in modlist] self.conn.modify_s(str(group_dn), modlist) - except errors.DatabaseError: + except errors.DuplicateEntry: + # TYPE_OR_VALUE_EXISTS raise errors.AlreadyGroupMember() def remove_entry_from_group(self, dn, group_dn, member_attr='member'): -- 2.17.1