pgreco / rpms / ipa

Forked from forks/areguera/rpms/ipa 4 years ago
Clone
Blob Blame History Raw
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