repl: Set GET_ALL_GROUP_MEMBERSHIP flag in the drepl server
[nivanova/samba-autobuild/.git] / python / samba / kcc / __init__.py
index 90225a4b54fed800bf245cdc2f28f8b5e1e6a498..f775a11b264d30ea8cb42a30f0eb600179e9c6bc 100644 (file)
@@ -107,7 +107,6 @@ class KCC(object):
         """
         self.part_table = {}    # partition objects
         self.site_table = {}
-        self.transport_table = {}
         self.ip_transport = None
         self.sitelink_table = {}
         self.dsa_by_dnstr = {}
@@ -142,7 +141,7 @@ class KCC(object):
         self.debug = debug
         self.dot_file_dir = dot_file_dir
 
-    def load_all_transports(self):
+    def load_ip_transport(self):
         """Loads the inter-site transport objects for Sites
 
         :return: None
@@ -163,10 +162,15 @@ class KCC(object):
             transport = Transport(dnstr)
 
             transport.load_transport(self.samdb)
-            self.transport_table.setdefault(str(transport.guid),
-                                            transport)
             if transport.name == 'IP':
                 self.ip_transport = transport
+            elif transport.name == 'SMTP':
+                logger.debug("Samba KCC is ignoring the obsolete "
+                             "SMTP transport.")
+
+            else:
+                logger.warning("Samba KCC does not support the transport "
+                               "called %r." % (transport.name,))
 
         if self.ip_transport is None:
             raise KCCError("there doesn't seem to be an IP transport")
@@ -258,14 +262,15 @@ class KCC(object):
         :return: None
         :raise: KCCError if DSA can't be found
         """
-        dn = ldb.Dn(self.samdb, "<GUID=%s>" % self.samdb.get_ntds_GUID())
+        dn_query = "<GUID=%s>" % self.samdb.get_ntds_GUID()
+        dn = ldb.Dn(self.samdb, dn_query)
         try:
             res = self.samdb.search(base=dn, scope=ldb.SCOPE_BASE,
                                     attrs=["objectGUID"])
         except ldb.LdbError, (enum, estr):
-            logger.warning("Search for %s failed: %s.  This typically happens"
-                           " in --importldif mode due to lack of module"
-                           " support.", dn, estr)
+            DEBUG_FN("Search for dn '%s' [from %s] failed: %s. "
+                     "This typically happens in --importldif mode due "
+                     "to lack of module support." % (dn, dn_query, estr))
             try:
                 # We work around the failure above by looking at the
                 # dsServiceName that was put in the fake rootdse by
@@ -302,7 +307,7 @@ class KCC(object):
                                     " it must be RODC.\n"
                                     "Let's add it, because my_dsa is special!"
                                     "\n(likewise for self.dsa_by_guid)" %
-                                    self.my_dsas_dnstr)
+                                    self.my_dsa_dnstr)
 
             self.dsa_by_dnstr[self.my_dsa_dnstr] = self.my_dsa
             self.dsa_by_guid[str(self.my_dsa.dsa_guid)] = self.my_dsa
@@ -429,19 +434,17 @@ class KCC(object):
         # that became active during this run.
         pass
 
-    def remove_unneeded_ntdsconn(self, all_connected):
-        """Remove unneeded NTDS Connections once topology is calculated
+    def _ensure_connections_are_loaded(self, connections):
+        """Load or fake-load NTDSConnections lacking GUIDs
 
-        Based on MS-ADTS 6.2.2.4 Removing Unnecessary Connections
+        New connections don't have GUIDs and created times which are
+        needed for sorting. If we're in read-only mode, we make fake
+        GUIDs, otherwise we ask SamDB to do it for us.
 
-        :param all_connected: indicates whether all sites are connected
+        :param connections: an iterable of NTDSConnection objects.
         :return: None
         """
-        mydsa = self.my_dsa
-
-        # New connections won't have GUIDs which are needed for
-        # sorting. Add them.
-        for cn_conn in mydsa.connect_table.values():
+        for cn_conn in connections:
             if cn_conn.guid is None:
                 if self.readonly:
                     cn_conn.guid = misc.GUID(str(uuid.uuid4()))
@@ -449,136 +452,158 @@ class KCC(object):
                 else:
                     cn_conn.load_connection(self.samdb)
 
-        for cn_conn in mydsa.connect_table.values():
+    def _mark_broken_ntdsconn(self):
+        """Find NTDS Connections that lack a remote
+
+        I'm not sure how they appear. Let's be rid of them by marking
+        them with the to_be_deleted attribute.
 
+        :return: None
+        """
+        for cn_conn in self.my_dsa.connect_table.values():
             s_dnstr = cn_conn.get_from_dnstr()
             if s_dnstr is None:
+                DEBUG_FN("%s has phantom connection %s" % (self.my_dsa,
+                                                           cn_conn))
                 cn_conn.to_be_deleted = True
-                continue
 
-            #XXX should an RODC be regarded as same site
-            same_site = s_dnstr in self.my_site.dsa_table
+    def _mark_unneeded_local_ntdsconn(self):
+        """Find unneeded intrasite NTDS Connections for removal
 
-            # Given an nTDSConnection object cn, if the DC with the
-            # nTDSDSA object dc that is the parent object of cn and
-            # the DC with the nTDSDA object referenced by cn!fromServer
-            # are in the same site, the KCC on dc deletes cn if all of
-            # the following are true:
-            #
-            # Bit NTDSCONN_OPT_IS_GENERATED is clear in cn!options.
-            #
-            # No site settings object s exists for the local DC's site, or
-            # bit NTDSSETTINGS_OPT_IS_TOPL_CLEANUP_DISABLED is clear in
-            # s!options.
-            #
-            # Another nTDSConnection object cn2 exists such that cn and
-            # cn2 have the same parent object, cn!fromServer = cn2!fromServer,
-            # and either
-            #
-            #     cn!whenCreated < cn2!whenCreated
-            #
-            #     cn!whenCreated = cn2!whenCreated and
-            #     cn!objectGUID < cn2!objectGUID
-            #
-            # Bit NTDSCONN_OPT_RODC_TOPOLOGY is clear in cn!options
-            if same_site:
-                if not cn_conn.is_generated():
-                    continue
+        Based on MS-ADTS 6.2.2.4 Removing Unnecessary Connections.
+        Every DC removes its own unnecessary intrasite connections.
+        This function tags them with the to_be_deleted attribute.
 
-                if self.my_site.is_cleanup_ntdsconn_disabled():
-                    continue
+        :return: None
+        """
+        # XXX should an RODC be regarded as same site? It isn't part
+        # of the intrasite ring.
 
-                # Loop thru connections looking for a duplicate that
-                # fulfills the previous criteria
-                lesser = False
-                packed_guid = ndr_pack(cn_conn.guid)
-                for cn2_conn in mydsa.connect_table.values():
-                    if cn2_conn is cn_conn:
-                        continue
+        if self.my_site.is_cleanup_ntdsconn_disabled():
+            DEBUG_FN("not doing ntdsconn cleanup for site %s, "
+                     "because it is disabled" % self.my_site)
+            return
 
-                    s2_dnstr = cn2_conn.get_from_dnstr()
+        mydsa = self.my_dsa
 
-                    # If the NTDS Connections has a different
-                    # fromServer field then no match
-                    if s2_dnstr != s_dnstr:
-                        continue
+        try:
+            self._ensure_connections_are_loaded(mydsa.connect_table.values())
+        except KCCError:
+            # RODC never actually added any connections to begin with
+            if mydsa.is_ro():
+                return
 
-                    #XXX GUID comparison
-                    lesser = (cn_conn.whenCreated < cn2_conn.whenCreated or
-                              (cn_conn.whenCreated == cn2_conn.whenCreated and
-                               packed_guid < ndr_pack(cn2_conn.guid)))
+        local_connections = []
 
-                    if lesser:
-                        break
+        for cn_conn in mydsa.connect_table.values():
+            s_dnstr = cn_conn.get_from_dnstr()
+            if s_dnstr in self.my_site.dsa_table:
+                removable = not (cn_conn.is_generated() or
+                                 cn_conn.is_rodc_topology())
+                packed_guid = ndr_pack(cn_conn.guid)
+                local_connections.append((cn_conn, s_dnstr,
+                                          packed_guid, removable))
+
+        for a, b in itertools.permutations(local_connections, 2):
+            cn_conn, s_dnstr, packed_guid, removable = a
+            cn_conn2, s_dnstr2, packed_guid2, removable2 = b
+            if (removable and
+                s_dnstr == s_dnstr2 and
+                cn_conn.whenCreated < cn_conn2.whenCreated or
+                (cn_conn.whenCreated == cn_conn2.whenCreated and
+                 packed_guid < packed_guid2)):
+                cn_conn.to_be_deleted = True
 
-                if lesser and not cn_conn.is_rodc_topology():
-                    cn_conn.to_be_deleted = True
+    def _mark_unneeded_intersite_ntdsconn(self):
+        """find unneeded intersite NTDS Connections for removal
 
-            # Given an nTDSConnection object cn, if the DC with the nTDSDSA
-            # object dc that is the parent object of cn and the DC with
-            # the nTDSDSA object referenced by cn!fromServer are in
-            # different sites, a KCC acting as an ISTG in dc's site
-            # deletes cn if all of the following are true:
-            #
-            #     Bit NTDSCONN_OPT_IS_GENERATED is clear in cn!options.
-            #
-            #     cn!fromServer references an nTDSDSA object for a DC
-            #     in a site other than the local DC's site.
-            #
-            #     The keepConnections sequence returned by
-            #     CreateIntersiteConnections() does not contain
-            #     cn!objectGUID, or cn is "superseded by" (see below)
-            #     another nTDSConnection cn2 and keepConnections
-            #     contains cn2!objectGUID.
-            #
-            #     The return value of CreateIntersiteConnections()
-            #     was true.
-            #
-            #     Bit NTDSCONN_OPT_RODC_TOPOLOGY is clear in
-            #     cn!options
-            #
-            else:  # different site
+        Based on MS-ADTS 6.2.2.4 Removing Unnecessary Connections. The
+        intersite topology generator removes links for all DCs in its
+        site. Here we just tag them with the to_be_deleted attribute.
 
-                if not mydsa.is_istg():
-                    continue
+        :return: None
+        """
+        # TODO Figure out how best to handle the RODC case
+        # The RODC is ITSG, but shouldn't act on anyone's behalf.
+        if self.my_dsa.is_ro():
+            return
 
-                if not cn_conn.is_generated():
+        # Find the intersite connections
+        local_dsas = self.my_site.dsa_table
+        connections_and_dsas = []
+        for dsa in local_dsas.values():
+            for cn in dsa.connect_table.values():
+                if cn.to_be_deleted:
                     continue
-
-                # TODO
-                # We are directly using this connection in intersite or
-                # we are using a connection which can supersede this one.
-                #
-                # MS-ADTS 6.2.2.4 - Removing Unnecessary Connections does not
-                # appear to be correct.
-                #
-                # 1. cn!fromServer and cn!parent appear inconsistent with
-                #    no cn2
-                # 2. The repsFrom do not imply each other
-                #
-                if cn_conn in self.kept_connections:  # and not_superceded:
+                s_dnstr = cn.get_from_dnstr()
+                if s_dnstr is None:
                     continue
+                if s_dnstr not in local_dsas:
+                    from_dsa = self.get_dsa(s_dnstr)
+                    # Samba ONLY: ISTG removes connections to dead DCs
+                    if from_dsa is None and '\\0ADEL' in s_dnstr:
+                        logger.info("DSA appears deleted, removing connection %s" % s_dnstr)
+                        cn.to_be_deleted = True
+                        continue
+                    connections_and_dsas.append((cn, dsa, from_dsa))
 
-                # This is the result of create_intersite_connections
-                if not all_connected:
-                    continue
+        self._ensure_connections_are_loaded(x[0] for x in connections_and_dsas)
+        for cn, to_dsa, from_dsa in connections_and_dsas:
+            if not cn.is_generated() or cn.is_rodc_topology():
+                continue
 
-                if not cn_conn.is_rodc_topology():
-                    cn_conn.to_be_deleted = True
+            # If the connection is in the kept_connections list, we
+            # only remove it if an endpoint seems down.
+            if (cn in self.kept_connections and
+                not (self.is_bridgehead_failed(to_dsa, True) or
+                     self.is_bridgehead_failed(from_dsa, True))):
+                continue
 
-        if mydsa.is_ro() or self.readonly:
-            for connect in mydsa.connect_table.values():
+            # this one is broken and might be superseded by another.
+            # But which other? Let's just say another link to the same
+            # site can supersede.
+            from_dnstr = from_dsa.dsa_dnstr
+            for site in self.site_table.values():
+                if from_dnstr in site.rw_dsa_table:
+                    for cn2, to_dsa2, from_dsa2 in connections_and_dsas:
+                        if (cn is not cn2 and
+                            from_dsa2 in site.rw_dsa_table):
+                            cn.to_be_deleted = True
+
+    def _commit_changes(self, dsa):
+        if dsa.is_ro() or self.readonly:
+            for connect in dsa.connect_table.values():
                 if connect.to_be_deleted:
-                    DEBUG_FN("TO BE DELETED:\n%s" % connect)
+                    logger.info("TO BE DELETED:\n%s" % connect)
                 if connect.to_be_added:
-                    DEBUG_FN("TO BE ADDED:\n%s" % connect)
+                    logger.info("TO BE ADDED:\n%s" % connect)
+                if connect.to_be_modified:
+                    logger.info("TO BE MODIFIED:\n%s" % connect)
 
             # Peform deletion from our tables but perform
             # no database modification
-            mydsa.commit_connections(self.samdb, ro=True)
+            dsa.commit_connections(self.samdb, ro=True)
         else:
             # Commit any modified connections
-            mydsa.commit_connections(self.samdb)
+            dsa.commit_connections(self.samdb)
+
+    def remove_unneeded_ntdsconn(self, all_connected):
+        """Remove unneeded NTDS Connections once topology is calculated
+
+        Based on MS-ADTS 6.2.2.4 Removing Unnecessary Connections
+
+        :param all_connected: indicates whether all sites are connected
+        :return: None
+        """
+        self._mark_broken_ntdsconn()
+        self._mark_unneeded_local_ntdsconn()
+        # if we are not the istg, we're done!
+        # if we are the istg, but all_connected is False, we also do nothing.
+        if self.my_dsa.is_istg() and all_connected:
+            self._mark_unneeded_intersite_ntdsconn()
+
+        for dsa in self.my_site.dsa_table.values():
+            self._commit_changes(dsa)
 
     def modify_repsFrom(self, n_rep, t_repsFrom, s_rep, s_dsa, cn_conn):
         """Update an repsFrom object if required.
@@ -612,6 +637,12 @@ class KCC(object):
         if times != t_repsFrom.schedule:
             t_repsFrom.schedule = times
 
+        # Bit DRS_ADD_REF is set in replicaFlags unconditionally
+        # Samba ONLY:
+        if ((t_repsFrom.replica_flags &
+             drsuapi.DRSUAPI_DRS_ADD_REF) == 0x0):
+            t_repsFrom.replica_flags |= drsuapi.DRSUAPI_DRS_ADD_REF
+
         # Bit DRS_PER_SYNC is set in replicaFlags if and only
         # if nTDSConnection schedule has a value v that specifies
         # scheduled replication is to be performed at least once
@@ -642,7 +673,7 @@ class KCC(object):
              dsdb.NTDSCONN_OPT_OVERRIDE_NOTIFY_DEFAULT) != 0x0):
 
             if (cn_conn.options & dsdb.NTDSCONN_OPT_USE_NOTIFY) == 0x0:
-                # XXX WARNING
+                # WARNING
                 #
                 # it LOOKS as if this next test is a bit silly: it
                 # checks the flag then sets it if it not set; the same
@@ -767,18 +798,17 @@ class KCC(object):
 
         # See (NOTE MS-TECH INCORRECT) above
 
-        # XXX it looks like these conditionals are pointless, because
-        # the state will end up as `t_repsFrom.dns_name1 == nastr` in
-        # either case, BUT the repsFrom thing is magic and assigning
-        # to it alters some flags. So we try not to update it unless
-        # necessary.
+        # NOTE: it looks like these conditionals are pointless,
+        # because the state will end up as `t_repsFrom.dns_name1 ==
+        # nastr` in either case, BUT the repsFrom thing is magic and
+        # assigning to it alters some flags. So we try not to update
+        # it unless necessary.
         if t_repsFrom.dns_name1 != nastr:
             t_repsFrom.dns_name1 = nastr
 
         if t_repsFrom.version > 0x1 and t_repsFrom.dns_name2 != nastr:
             t_repsFrom.dns_name2 = nastr
 
-
         if t_repsFrom.is_modified():
             DEBUG_FN("modify_repsFrom(): %s" % t_repsFrom)
 
@@ -796,14 +826,12 @@ class KCC(object):
         :param cn_conn: NTDS Connection
         :return: source DSA or None
         """
-        #XXX different conditions for "implies" than MS-ADTS 6.2.2
+        # XXX different conditions for "implies" than MS-ADTS 6.2.2
+        # preamble.
 
-        # NTDS Connection must satisfy all the following criteria
-        # to imply a repsFrom tuple is needed:
-        #
-        #    cn!enabledConnection = true.
-        #    cn!options does not contain NTDSCONN_OPT_RODC_TOPOLOGY.
-        #    cn!fromServer references an nTDSDSA object.
+        # It boils down to: we want an enabled, non-FRS connections to
+        # a valid remote DSA with a non-RO replica corresponding to
+        # n_rep.
 
         if not cn_conn.is_enabled() or cn_conn.is_rodc_topology():
             return None
@@ -815,39 +843,11 @@ class KCC(object):
         if s_dsa is None:
             return None
 
-        # To imply a repsFrom tuple is needed, each of these
-        # must be True:
-        #
-        #     An NC replica of the NC "is present" on the DC to
-        #     which the nTDSDSA object referenced by cn!fromServer
-        #     corresponds.
-        #
-        #     An NC replica of the NC "should be present" on
-        #     the local DC
         s_rep = s_dsa.get_current_replica(n_rep.nc_dnstr)
 
-        if s_rep is None or not s_rep.is_present():
-            return None
-
-        # To imply a repsFrom tuple is needed, each of these
-        # must be True:
-        #
-        #     The NC replica on the DC referenced by cn!fromServer is
-        #     a writable replica or the NC replica that "should be
-        #     present" on the local DC is a partial replica.
-        #
-        #     The NC is not a domain NC, the NC replica that
-        #     "should be present" on the local DC is a partial
-        #     replica, cn!transportType has no value, or
-        #     cn!transportType has an RDN of CN=IP.
-        #
-        implied = (not s_rep.is_ro() or n_rep.is_partial()) and \
-                  (not n_rep.is_domain() or
-                   n_rep.is_partial() or
-                   cn_conn.transport_dnstr is None or
-                   cn_conn.transport_dnstr.find("CN=IP") == 0)
-
-        if implied:
+        if (s_rep is not None and
+            s_rep.is_present() and
+            (not s_rep.is_ro() or n_rep.is_partial())):
             return s_dsa
         return None
 
@@ -865,9 +865,13 @@ class KCC(object):
         """
         count = 0
 
+        ro = False
         if current_dsa is None:
             current_dsa = self.my_dsa
 
+        if current_dsa.is_ro():
+            ro = True
+
         if current_dsa.is_translate_ntdsconn_disabled():
             DEBUG_FN("skipping translate_ntdsconn() "
                      "because disabling flag is set")
@@ -896,17 +900,34 @@ class KCC(object):
         # If we have the replica and its not needed
         # then we add it to the "to be deleted" list.
         for dnstr in current_rep_table:
-            if dnstr not in needed_rep_table:
-                delete_reps.add(dnstr)
+            # If we're on the RODC, hardcode the update flags
+            if ro:
+                c_rep = current_rep_table[dnstr]
+                c_rep.load_repsFrom(self.samdb)
+                for t_repsFrom in c_rep.rep_repsFrom:
+                    replica_flags = (drsuapi.DRSUAPI_DRS_INIT_SYNC |
+                                     drsuapi.DRSUAPI_DRS_PER_SYNC |
+                                     drsuapi.DRSUAPI_DRS_ADD_REF |
+                                     drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING |
+                                     drsuapi.DRSUAPI_DRS_NONGC_RO_REP)
+                    if t_repsFrom.replica_flags != replica_flags:
+                        t_repsFrom.replica_flags = replica_flags
+                c_rep.commit_repsFrom(self.samdb)
+            else:
+                if dnstr not in needed_rep_table:
+                    delete_reps.add(dnstr)
 
         DEBUG_FN('current %d needed %d delete %d' % (len(current_rep_table),
                  len(needed_rep_table), len(delete_reps)))
 
         if delete_reps:
+            # TODO Must delete repsFrom/repsTo for these replicas
             DEBUG('deleting these reps: %s' % delete_reps)
             for dnstr in delete_reps:
                 del current_rep_table[dnstr]
 
+        # HANDLE REPS-FROM
+        #
         # Now perform the scan of replicas we'll need
         # and compare any current repsFrom against the
         # connections
@@ -917,7 +938,7 @@ class KCC(object):
             n_rep.load_repsFrom(self.samdb)
             n_rep.load_fsmo_roles(self.samdb)
 
-            # Loop thru the existing repsFrom tupples (if any)
+            # Loop thru the existing repsFrom tuples (if any)
             # XXX This is a list and could contain duplicates
             #     (multiple load_repsFrom calls)
             for t_repsFrom in n_rep.rep_repsFrom:
@@ -976,13 +997,12 @@ class KCC(object):
                 if s_dsa is None:
                     continue
 
-                # Loop thru the existing repsFrom tupples (if any) and
+                # Loop thru the existing repsFrom tuples (if any) and
                 # if we already have a tuple for this connection then
                 # no need to proceed to add.  It will have been changed
                 # to have the correct attributes above
                 for t_repsFrom in n_rep.rep_repsFrom:
                     guidstr = str(t_repsFrom.source_dsa_obj_guid)
-                    #XXX what?
                     if s_dsa is self.get_dsa_by_guidstr(guidstr):
                         s_dsa = None
                         break
@@ -1004,7 +1024,7 @@ class KCC(object):
                 if t_repsFrom.is_modified():
                     n_rep.rep_repsFrom.append(t_repsFrom)
 
-            if self.readonly:
+            if self.readonly or ro:
                 # Display any to be deleted or modified repsFrom
                 text = n_rep.dumpstr_to_be_deleted()
                 if text:
@@ -1020,6 +1040,74 @@ class KCC(object):
                 # Commit any modified repsFrom to the NC replica
                 n_rep.commit_repsFrom(self.samdb)
 
+        # HANDLE REPS-TO:
+        #
+        # Now perform the scan of replicas we'll need
+        # and compare any current repsTo against the
+        # connections
+
+        # RODC should never push to anybody (should we check this?)
+        if ro:
+            return
+
+        for n_rep in needed_rep_table.values():
+
+            # load any repsTo and fsmo roles as we'll
+            # need them during connection translation
+            n_rep.load_repsTo(self.samdb)
+
+            # Loop thru the existing repsTo tuples (if any)
+            # XXX This is a list and could contain duplicates
+            #     (multiple load_repsTo calls)
+            for t_repsTo in n_rep.rep_repsTo:
+
+                # for each tuple t in n!repsTo, let s be the nTDSDSA
+                # object such that s!objectGUID = t.uuidDsa
+                guidstr = str(t_repsTo.source_dsa_obj_guid)
+                s_dsa = self.get_dsa_by_guidstr(guidstr)
+
+                # Source dsa is gone from config (strange)
+                # so cleanup stale repsTo for unlisted DSA
+                if s_dsa is None:
+                    logger.warning("repsTo source DSA guid (%s) not found" %
+                                   guidstr)
+                    t_repsTo.to_be_deleted = True
+                    continue
+
+                # Find the connection that this repsTo would use. If
+                # there isn't a good one (i.e. non-RODC_TOPOLOGY,
+                # meaning non-FRS), we delete the repsTo.
+                s_dnstr = s_dsa.dsa_dnstr
+                if '\\0ADEL' in s_dnstr:
+                    logger.warning("repsTo source DSA guid (%s) appears deleted" %
+                                   guidstr)
+                    t_repsTo.to_be_deleted = True
+                    continue
+
+                connections = s_dsa.get_connection_by_from_dnstr(self.my_dsa_dnstr)
+                if len(connections) > 0:
+                    # Then this repsTo is tentatively valid
+                    continue
+                else:
+                    # There is no plausible connection for this repsTo
+                    t_repsTo.to_be_deleted = True
+
+            if self.readonly:
+                # Display any to be deleted or modified repsTo
+                text = n_rep.dumpstr_reps_to()
+                if text:
+                    logger.info("REMOVING REPS-TO:\n%s" % text)
+
+                # Peform deletion from our tables but perform
+                # no database modification
+                n_rep.commit_repsTo(self.samdb, ro=True)
+            else:
+                # Commit any modified repsTo to the NC replica
+                n_rep.commit_repsTo(self.samdb)
+
+        # TODO Remove any duplicate repsTo values. This should never happen in
+        # any normal situations.
+
     def merge_failed_links(self, ping=None):
         """Merge of kCCFailedLinks and kCCFailedLinks from bridgeheads.
 
@@ -1064,9 +1152,10 @@ class KCC(object):
         # NTDSTRANSPORT_OPT_BRIDGES_REQUIRED 0x00000002
         # No documentation for this however, ntdsapi.h appears to have:
         # NTDSSETTINGS_OPT_W2K3_BRIDGES_REQUIRED = 0x00001000
-        bridges_required = self.my_site.site_options & 0x00001002 == 0
+        bridges_required = self.my_site.site_options & 0x00001002 != 0
+        transport_guid = str(self.ip_transport.guid)
 
-        g = setup_graph(part, self.site_table, self.transport_table,
+        g = setup_graph(part, self.site_table, transport_guid,
                         self.sitelink_table, bridges_required)
 
         if self.verify or self.dot_file_dir is not None:
@@ -1075,7 +1164,8 @@ class KCC(object):
                 for a, b in itertools.combinations(edge.vertices, 2):
                     dot_edges.append((a.site.site_dnstr, b.site.site_dnstr))
             verify_properties = ()
-            verify_and_dot('site_edges', dot_edges, directed=False,
+            name = 'site_edges_%s' % part.partstr
+            verify_and_dot(name, dot_edges, directed=False,
                            label=self.my_dsa_dnstr,
                            properties=verify_properties, debug=DEBUG,
                            verify=self.verify,
@@ -1104,14 +1194,14 @@ class KCC(object):
 
         bhs = self.get_all_bridgeheads(site, part, transport,
                                        partial_ok, detect_failed)
-        if len(bhs) == 0:
-            debug.DEBUG_MAGENTA("get_bridgehead:\n\tsitedn=%s\n\tbhdn=None" %
+        if not bhs:
+            debug.DEBUG_MAGENTA("get_bridgehead FAILED:\nsitedn = %s" %
                                 site.site_dnstr)
             return None
-        else:
-            debug.DEBUG_GREEN("get_bridgehead:\n\tsitedn=%s\n\tbhdn=%s" %
-                              (site.site_dnstr, bhs[0].dsa_dnstr))
-            return bhs[0]
+
+        debug.DEBUG_GREEN("get_bridgehead:\n\tsitedn = %s\n\tbhdn = %s" %
+                          (site.site_dnstr, bhs[0].dsa_dnstr))
+        return bhs[0]
 
     def get_all_bridgeheads(self, site, part, transport,
                             partial_ok, detect_failed):
@@ -1134,7 +1224,11 @@ class KCC(object):
         """
         bhs = []
 
-        DEBUG_FN("get_all_bridgeheads: %s" % transport.name)
+        if transport.name != "IP":
+            raise KCCError("get_all_bridgeheads has run into a "
+                           "non-IP transport! %r"
+                           % (transport.name,))
+
         DEBUG_FN(site.rw_dsa_table)
         for dsa in site.rw_dsa_table.values():
 
@@ -1176,34 +1270,13 @@ class KCC(object):
                 if not dsa.is_minimum_behavior(dsdb.DS_DOMAIN_FUNCTION_2008):
                     continue
 
-            # IF t!name != "IP" and the parent object of dc has no value for
-            # the attribute specified by t!transportAddressAttribute
-            #     Skip dc
-            if transport.name != "IP":
-                # MS tech specification says we retrieve the named
-                # attribute in "transportAddressAttribute" from the parent
-                # of the DSA object
-                try:
-                    attrs = [transport.address_attr]
-
-                    res = self.samdb.search(base=pdnstr, scope=ldb.SCOPE_BASE,
-                                            attrs=attrs)
-                except ldb.LdbError, (enum, estr):
-                    continue
-
-                msg = res[0]
-                if transport.address_attr not in msg:
-                    continue
-                #XXX nastr is NEVER USED. It will be removed.
-                nastr = str(msg[transport.address_attr][0])
-
             # IF BridgeheadDCFailed(dc!objectGUID, detectFailedDCs) = TRUE
             #     Skip dc
             if self.is_bridgehead_failed(dsa, detect_failed):
                 DEBUG("bridgehead is failed")
                 continue
 
-            DEBUG_FN("get_all_bridgeheads: dsadn=%s" % dsa.dsa_dnstr)
+            DEBUG_FN("found a bridgehead: %s" % dsa.dsa_dnstr)
             bhs.append(dsa)
 
         # IF bit NTDSSETTINGS_OPT_IS_RAND_BH_SELECTION_DISABLED is set in
@@ -1299,7 +1372,7 @@ class KCC(object):
         """
         rbhs_all = self.get_all_bridgeheads(rsite, part, transport,
                                             partial_ok, False)
-        rbh_table = {x.dsa_dnstr: x for x in rbhs_all}
+        rbh_table = dict((x.dsa_dnstr, x) for x in rbhs_all)
 
         debug.DEBUG_GREY("rbhs_all: %s %s" % (len(rbhs_all),
                                               [x.dsa_dnstr for x in rbhs_all]))
@@ -1504,7 +1577,10 @@ class KCC(object):
             # cn!options = opt, cn!transportType is a reference to t,
             # cn!fromServer is a reference to rbh, and cn!schedule = sch
             DEBUG_FN("new connection, KCC dsa: %s" % self.my_dsa.dsa_dnstr)
-            cn = lbh.new_connection(opt, 0, transport,
+            system_flags = (dsdb.SYSTEM_FLAG_CONFIG_ALLOW_RENAME |
+                            dsdb.SYSTEM_FLAG_CONFIG_ALLOW_MOVE)
+
+            cn = lbh.new_connection(opt, system_flags, transport,
                                     rbh.dsa_dnstr, link_sched)
 
             # Display any added connection
@@ -1539,7 +1615,7 @@ class KCC(object):
         # here, but using vertex seems to make more sense. That is,
         # the docs want this:
         #
-        #bh = self.get_bridgehead(vertex.site, vertex.part, transport,
+        #bh = self.get_bridgehead(local_vertex.site, vertex.part, transport,
         #                         local_vertex.is_black(), detect_failed)
         #
         # TODO WHY?????
@@ -1547,30 +1623,23 @@ class KCC(object):
         vertex.accept_red_red = []
         vertex.accept_black = []
         found_failed = False
-        for t_guid, transport in self.transport_table.items():
-            if transport.name != 'IP':
-                #XXX well this is cheating a bit
-                logger.warning("WARNING: we are ignoring a transport named %r"
-                               % transport.name)
-                continue
 
-            if vertex not in graph.connected_vertices:
-                continue
+        if vertex in graph.connected_vertices:
+            t_guid = str(self.ip_transport.guid)
 
-            partial_replica_okay = vertex.is_black()
-            bh = self.get_bridgehead(vertex.site, vertex.part, transport,
-                                     partial_replica_okay, detect_failed)
+            bh = self.get_bridgehead(vertex.site, vertex.part,
+                                     self.ip_transport,
+                                     vertex.is_black(), detect_failed)
             if bh is None:
                 if vertex.site.is_rodc_site():
                     vertex.accept_red_red.append(t_guid)
                 else:
                     found_failed = True
-                continue
-
-            vertex.accept_red_red.append(t_guid)
-            vertex.accept_black.append(t_guid)
+            else:
+                vertex.accept_red_red.append(t_guid)
+                vertex.accept_black.append(t_guid)
 
-        # Add additional transport to allow another run of Dijkstra
+        # Add additional transport to ensure another run of Dijkstra
         vertex.accept_red_red.append("EDGE_TYPE_ALL")
         vertex.accept_black.append("EDGE_TYPE_ALL")
 
@@ -1617,7 +1686,7 @@ class KCC(object):
 
         for v in graph.vertices:
             v.color_vertex()
-            if self.add_transports(v, my_vertex, graph, False):
+            if self.add_transports(v, my_vertex, graph, detect_failed):
                 found_failed = True
 
         # No NC replicas for this NC in the site of the local DC,
@@ -1682,13 +1751,9 @@ class KCC(object):
                 debug.DEBUG_RED("DISASTER! lbh is None")
                 return False, True
 
-            debug.DEBUG_CYAN("SITES")
-            print lsite, rsite
-            debug.DEBUG_BLUE("vertices")
-            print e.vertices
-            debug.DEBUG_BLUE("bridgeheads")
-            print lbh, rbh
-            debug.DEBUG_BLUE("-" * 70)
+            DEBUG_FN("lsite: %s\nrsite: %s" % (lsite, rsite))
+            DEBUG_FN("vertices %s" % (e.vertices,))
+            debug.DEBUG_BLUE("bridgeheads\n%s\n%s\n%s" % (lbh, rbh, "-" * 70))
 
             sitelink = e.site_link
             if sitelink is None:
@@ -1819,7 +1884,9 @@ class KCC(object):
         DEBUG_FN("intersite(): exit all_connected=%d" % all_connected)
         return all_connected
 
-    def update_rodc_connection(self):
+    # This function currently does no actions. The reason being that we cannot
+    # perform modifies in this way on the RODC.
+    def update_rodc_connection(self, ro=True):
         """Updates the RODC NTFRS connection object.
 
         If the local DSA is not an RODC, this does nothing.
@@ -1853,20 +1920,21 @@ class KCC(object):
                 con.schedule = cn2.schedule
                 con.to_be_modified = True
 
-            self.my_dsa.commit_connections(self.samdb, ro=self.readonly)
+            self.my_dsa.commit_connections(self.samdb, ro=ro)
 
     def intrasite_max_node_edges(self, node_count):
-        """Returns the maximum number of edges directed to a node in
-        the intrasite replica graph.
+        """Find the maximum number of edges directed to an intrasite node
 
-        The KCC does not create more
-        than 50 edges directed to a single DC. To optimize replication,
-        we compute that each node should have n+2 total edges directed
-        to it such that (n) is the smallest non-negative integer
-        satisfying (node_count <= 2*(n*n) + 6*n + 7)
+        The KCC does not create more than 50 edges directed to a
+        single DC. To optimize replication, we compute that each node
+        should have n+2 total edges directed to it such that (n) is
+        the smallest non-negative integer satisfying
+        (node_count <= 2*(n*n) + 6*n + 7)
 
         (If the number of edges is m (i.e. n + 2), that is the same as
-        2 * m*m - 2 * m + 3).
+        2 * m*m - 2 * m + 3). We think in terms of n because that is
+        the number of extra connections over the double directed ring
+        that exists by default.
 
         edges  n   nodecount
           2    0    7
@@ -1885,6 +1953,9 @@ class KCC(object):
         guarantee holds at e.g. 15 nodes in degenerate cases, but
         those are quite unlikely given the extra edges are randomly
         arranged.
+
+        :param node_count: the number of nodes in the site
+        "return: The desired maximum number of connections
         """
         n = 0
         while True:
@@ -2166,7 +2237,7 @@ class KCC(object):
                     dot_edges.append((v2, v1.dsa_dnstr))
                     dot_vertices.add(v2)
 
-            verify_properties = ('connected', 'directed_double_ring_or_small')
+            verify_properties = ('connected',)
             verify_and_dot('intrasite_pre_ntdscon', dot_edges, dot_vertices,
                            label='%s__%s__%s' % (site_local.site_dnstr,
                                                  nctype_lut[nc_x.nc_type],
@@ -2176,6 +2247,22 @@ class KCC(object):
                            dot_file_dir=self.dot_file_dir,
                            directed=True)
 
+            rw_dot_vertices = set(x for x in dot_vertices
+                                  if not self.get_dsa(x).is_ro())
+            rw_dot_edges = [(a, b) for a, b in dot_edges if
+                            a in rw_dot_vertices and b in rw_dot_vertices]
+            rw_verify_properties = ('connected',
+                                    'directed_double_ring_or_small')
+            verify_and_dot('intrasite_rw_pre_ntdscon', rw_dot_edges,
+                           rw_dot_vertices,
+                           label='%s__%s__%s' % (site_local.site_dnstr,
+                                                 nctype_lut[nc_x.nc_type],
+                                                 nc_x.nc_dnstr),
+                           properties=rw_verify_properties, debug=DEBUG,
+                           verify=self.verify,
+                           dot_file_dir=self.dot_file_dir,
+                           directed=True)
+
         # For each existing nTDSConnection object implying an edge
         # from rj of R to ri such that j != i, an edge from rj to ri
         # is not already in the graph, and the total edges directed
@@ -2218,9 +2305,9 @@ class KCC(object):
 
                 while candidates and not tnode.has_sufficient_edges():
                     other = random.choice(candidates)
-                    DEBUG("trying to add candidate %s" % other.dsa_dstr)
-                    if not tnode.add_edge_from(other):
-                        debug.DEBUG_RED("could not add %s" % other.dsa_dstr)
+                    DEBUG("trying to add candidate %s" % other.dsa_dnstr)
+                    if not tnode.add_edge_from(other.dsa_dnstr):
+                        debug.DEBUG_RED("could not add %s" % other.dsa_dnstr)
                     candidates.remove(other)
             else:
                 DEBUG_FN("not adding links to %s: nodes %s, links is %s/%s" %
@@ -2234,7 +2321,7 @@ class KCC(object):
             # points to us that satisfies the KCC criteria
 
             if tnode.dsa_dnstr == dc_local.dsa_dnstr:
-                tnode.add_connections_from_edges(dc_local)
+                tnode.add_connections_from_edges(dc_local, self.ip_transport)
 
         if self.verify or do_dot_files:
             dot_edges = []
@@ -2245,7 +2332,7 @@ class KCC(object):
                     dot_edges.append((v2, v1.dsa_dnstr))
                     dot_vertices.add(v2)
 
-            verify_properties = ('connected', 'directed_double_ring_or_small')
+            verify_properties = ('connected',)
             verify_and_dot('intrasite_post_ntdscon', dot_edges, dot_vertices,
                            label='%s__%s__%s' % (site_local.site_dnstr,
                                                  nctype_lut[nc_x.nc_type],
@@ -2255,6 +2342,22 @@ class KCC(object):
                            dot_file_dir=self.dot_file_dir,
                            directed=True)
 
+            rw_dot_vertices = set(x for x in dot_vertices
+                                  if not self.get_dsa(x).is_ro())
+            rw_dot_edges = [(a, b) for a, b in dot_edges if
+                            a in rw_dot_vertices and b in rw_dot_vertices]
+            rw_verify_properties = ('connected',
+                                    'directed_double_ring_or_small')
+            verify_and_dot('intrasite_rw_post_ntdscon', rw_dot_edges,
+                           rw_dot_vertices,
+                           label='%s__%s__%s' % (site_local.site_dnstr,
+                                                 nctype_lut[nc_x.nc_type],
+                                                 nc_x.nc_dnstr),
+                           properties=rw_verify_properties, debug=DEBUG,
+                           verify=self.verify,
+                           dot_file_dir=self.dot_file_dir,
+                           directed=True)
+
     def intrasite(self):
         """Generate the intrasite KCC connections
 
@@ -2333,20 +2436,7 @@ class KCC(object):
                 self.construct_intrasite_graph(mysite, mydsa, part, True,
                                                False)  # don't detect stale
 
-        if self.readonly:
-            # Display any to be added or modified repsFrom
-            for connect in mydsa.connect_table.values():
-                if connect.to_be_deleted:
-                    logger.info("TO BE DELETED:\n%s" % connect)
-                if connect.to_be_modified:
-                    logger.info("TO BE MODIFIED:\n%s" % connect)
-                if connect.to_be_added:
-                    debug.DEBUG_GREEN("TO BE ADDED:\n%s" % connect)
-
-            mydsa.commit_connections(self.samdb, ro=True)
-        else:
-            # Commit any newly created connections to the samdb
-            mydsa.commit_connections(self.samdb)
+        self._commit_changes(mydsa)
 
     def list_dsas(self):
         """Compile a comprehensive list of DSA DNs
@@ -2364,7 +2454,7 @@ class KCC(object):
 
         self.load_all_sites()
         self.load_all_partitions()
-        self.load_all_transports()
+        self.load_ip_transport()
         self.load_all_sitelinks()
         dsas = []
         for site in self.site_table.values():
@@ -2372,16 +2462,26 @@ class KCC(object):
                          for dsa in site.dsa_table.values()])
         return dsas
 
-    def load_samdb(self, dburl, lp, creds):
+    def load_samdb(self, dburl, lp, creds, force=False):
         """Load the database using an url, loadparm, and credentials
 
+        If force is False, the samdb won't be reloaded if it already
+        exists.
+
         :param dburl: a database url.
         :param lp: a loadparm object.
         :param creds: a Credentials object.
+        :param force: a boolean indicating whether to overwrite.
+
         """
-        self.samdb = SamDB(url=dburl,
-                           session_info=system_session(),
-                           credentials=creds, lp=lp)
+        if force or self.samdb is None:
+            try:
+                self.samdb = SamDB(url=dburl,
+                                   session_info=system_session(),
+                                   credentials=creds, lp=lp)
+            except ldb.LdbError, (num, msg):
+                raise KCCError("Unable to open sam database %s : %s" %
+                               (dburl, msg))
 
     def plot_all_connections(self, basename, verify_properties=()):
         """Helper function to plot and verify NTDSConnections
@@ -2412,8 +2512,9 @@ class KCC(object):
                 dot_edges.append((con.from_dnstr, dsa.dsa_dnstr))
 
         verify_and_dot(basename, dot_edges, vertices=dot_vertices,
-                       label=self.my_dsa_dnstr, properties=verify_properties,
-                       debug=DEBUG, verify=verify, dot_file_dir=self.dot_file_dir,
+                       label=self.my_dsa_dnstr,
+                       properties=verify_properties, debug=DEBUG,
+                       verify=verify, dot_file_dir=self.dot_file_dir,
                        directed=True, edge_colors=edge_colours,
                        vertex_colors=vertex_colours)
 
@@ -2434,15 +2535,9 @@ class KCC(object):
                determine link availability (boolean, default False)
         :return: 1 on error, 0 otherwise
         """
-        # We may already have a samdb setup if we are
-        # currently importing an ldif for a test run
         if self.samdb is None:
-            try:
-                self.load_samdb(dburl, lp, creds)
-            except ldb.LdbError, (num, msg):
-                logger.error("Unable to open sam database %s : %s" %
-                             (dburl, msg))
-                return 1
+            DEBUG_FN("samdb is None; let's load it from %s" % (dburl,))
+            self.load_samdb(dburl, lp, creds, force=False)
 
         if forced_local_dsa:
             self.samdb.set_ntds_settings_dn("CN=NTDS Settings,%s" %
@@ -2455,7 +2550,7 @@ class KCC(object):
 
             self.load_all_sites()
             self.load_all_partitions()
-            self.load_all_transports()
+            self.load_ip_transport()
             self.load_all_sitelinks()
 
             if self.verify or self.dot_file_dir is not None:
@@ -2507,18 +2602,20 @@ class KCC(object):
 
             if forget_local_links:
                 for dsa in self.my_site.dsa_table.values():
-                    dsa.connect_table = {k: v for k, v in
-                                         dsa.connect_table.items()
-                                         if v.is_rodc_topology()}
+                    dsa.connect_table = dict((k, v) for k, v in
+                                             dsa.connect_table.items()
+                                             if v.is_rodc_topology() or
+                                             (v.from_dnstr not in
+                                              self.my_site.dsa_table))
                 self.plot_all_connections('dsa_forgotten_local')
 
             if forget_intersite_links:
                 for site in self.site_table.values():
                     for dsa in site.dsa_table.values():
-                        dsa.connect_table = {k: v for k, v in
-                                             dsa.connect_table.items()
-                                             if site is self.my_site and
-                                             v.is_rodc_topology()}
+                        dsa.connect_table = dict((k, v) for k, v in
+                                                 dsa.connect_table.items()
+                                                 if site is self.my_site and
+                                                 v.is_rodc_topology())
 
                 self.plot_all_connections('dsa_forgotten_all')
 
@@ -2559,7 +2656,7 @@ class KCC(object):
 
             if self.verify or self.dot_file_dir is not None:
                 self.plot_all_connections('dsa_final',
-                                          ('connected', 'forest_of_rings'))
+                                          ('connected',))
 
                 debug.DEBUG_MAGENTA("there are %d dsa guids" %
                                     len(guid_to_dnstr))
@@ -2601,9 +2698,8 @@ class KCC(object):
 
         return 0
 
-    def import_ldif(self, dburl, lp, creds, ldif_file, forced_local_dsa=None):
-        """Import all objects and attributes that are relevent
-        to the KCC algorithms from a previously exported LDIF file.
+    def import_ldif(self, dburl, lp, ldif_file, forced_local_dsa=None):
+        """Import relevant objects and attributes from an LDIF file.
 
         The point of this function is to allow a programmer/debugger to
         import an LDIF file with non-security relevent information that
@@ -2614,19 +2710,21 @@ class KCC(object):
         same between different OSes and algorithms.
 
         :param dburl: path to the temporary abbreviated db to create
+        :param lp: a loadparm object.
         :param ldif_file: path to the ldif file to import
+        :param forced_local_dsa: perform KCC from this DSA's point of view
+        :return: zero on success, 1 on error
         """
         try:
             self.samdb = ldif_import_export.ldif_to_samdb(dburl, lp, ldif_file,
                                                           forced_local_dsa)
         except ldif_import_export.LdifError, e:
-            print e
+            logger.critical(e)
             return 1
         return 0
 
     def export_ldif(self, dburl, lp, creds, ldif_file):
-        """Routine to extract all objects and attributes that are relevent
-        to the KCC algorithms from a DC database.
+        """Save KCC relevant details to an ldif file
 
         The point of this function is to allow a programmer/debugger to
         extract an LDIF file with non-security relevent information from
@@ -2637,12 +2735,15 @@ class KCC(object):
         is computationally the same between different OSes and algorithms.
 
         :param dburl: LDAP database URL to extract info from
+        :param lp: a loadparm object.
+        :param cred: a Credentials object.
         :param ldif_file: output LDIF file name to create
+        :return: zero on success, 1 on error
         """
         try:
             ldif_import_export.samdb_to_ldif_file(self.samdb, dburl, lp, creds,
                                                   ldif_file)
         except ldif_import_export.LdifError, e:
-            print e
+            logger.critical(e)
             return 1
         return 0