From 33f562bba3729bc596e07dc2805d78b80de55784 Mon Sep 17 00:00:00 2001

From: Christian Heimes <cheimes@redhat.com>

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 <cheimes@redhat.com>

Reviewed-By: Thierry Bordaz <tbordaz@redhat.com>

Reviewed-By: Thierry Bordaz <tbordaz@redhat.com>

---

 ipapython/ipaldap.py             |   5 ++

 ipaserver/install/replication.py | 125 ++++++++++++++++++-------------

 ipaserver/plugins/ldap2.py       |   3 +-

 3 files changed, 81 insertions(+), 52 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py

index 1b0aaddd63d92776448d1a7ae6c1bd26a02d5dcc..e2ff0626986aa20f3a2bc42721fba9fad3156498 100644

--- a/ipapython/ipaldap.py

+++ b/ipapython/ipaldap.py

@@ -965,7 +965,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 c017764468674830670a817b3d815c5e2d78728e..ffda9a182f840317d96f1b3b914b38233022fb5b 100644

--- a/ipaserver/install/replication.py

+++ b/ipaserver/install/replication.py

@@ -34,7 +34,7 @@ from ipalib import api, errors

 from ipalib.cli import textui

 from ipapython.ipa_log_manager import root_logger

 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 ipaplatform.paths import paths

@@ -457,7 +457,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

@@ -472,11 +472,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)

@@ -484,49 +479,64 @@ class ReplicationManager(object):

                 root_logger.debug(

                     "nsds5replicabinddngroup attribute not supported on "

                     "remote master.")

+            except (ldap.ALREADY_EXISTS, ldap.CONSTRAINT_VIOLATION):

+                root_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)

+        root_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:

+                root_logger.debug(

+                    "Lost race against another replica, updating"

+                )

+                # fetch entry that have been added by another replica

+                entry = conn.get_entry(dn)

+            else:

+                root_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:

+            root_logger.debug("No update to %s necessary", entry.dn)

+        else:

+            root_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(

@@ -686,7 +696,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'))

@@ -1644,7 +1657,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):

         """

@@ -1654,23 +1670,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):

@@ -1690,7 +1707,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'),

@@ -1702,7 +1722,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 3b1e4da57a8e16e3d9b27eea24025de2caa53216..c0a0ed251f98cd06ccd13c873f38809d04d1b5d5 100644

--- a/ipaserver/plugins/ldap2.py

+++ b/ipaserver/plugins/ldap2.py

@@ -422,7 +422,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