KCC: fix is_bridgehead_failed() according to documentation

Throughout the KCC specification `detectFailedDCs` is documented along
the lines of "true to detect failed DCs", and it gets passed down to
this function. And what do we see here? It is used as a default value
when a stale link is not detected. That is entirely different. So who is
right -- the comments or the pseudo-code?

This commit follows the comments. It works!

   Documentation 1,  Pseudo-code 0

See [MS-ADTS] — v20140502, section 6.2.2.3.4.4, page 569.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/scripting/bin/samba_kcc b/source4/scripting/bin/samba_kcc
index c4a81e1dc25ab206d6d1c090ac01636f301a0ed2..cd584029880958355205d949ea8d15b2b273f8dc 100755 (executable)
--- a/source4/scripting/bin/samba_kcc
+++ b/source4/scripting/bin/samba_kcc
@@ -1255,15 +1255,50 @@ class KCC(object):
     def is_bridgehead_failed(self, dsa, detect_failed):
         """Determine whether a given DC is known to be in a failed state
         ::returns: True if and only if the DC should be considered failed
+
+        Here we DEPART from the pseudo code spec which appears to be
+        wrong. It says, in full:
+
+    /***** BridgeheadDCFailed *****/
+    /* Determine whether a given DC is known to be in a failed state.
+     * IN: objectGUID - objectGUID of the DC's nTDSDSA object.
+     * IN: detectFailedDCs - TRUE if and only failed DC detection is
+     *     enabled.
+     * RETURNS: TRUE if and only if the DC should be considered to be in a
+     *          failed state.
+     */
+    BridgeheadDCFailed(IN GUID objectGUID, IN bool detectFailedDCs) : bool
+    {
+        IF bit NTDSSETTINGS_OPT_IS_TOPL_DETECT_STALE_DISABLED is set in
+        the options attribute of the site settings object for the local
+        DC's site
+            RETURN FALSE
+        ELSEIF a tuple z exists in the kCCFailedLinks or
+        kCCFailedConnections variables such that z.UUIDDsa =
+        objectGUID, z.FailureCount > 1, and the current time -
+        z.TimeFirstFailure > 2 hours
+            RETURN TRUE
+        ELSE
+            RETURN detectFailedDCs
+        ENDIF
+    }
+
+        where you will see detectFailedDCs is not behaving as
+        advertised -- it is acting as a default return code in the
+        event that a failure is not detected, not a switch turning
+        detection on or off. Elsewhere the documentation seems to
+        concur with the comment rather than the code.
         """
+        if not detect_failed:
+            return False
+
         # NTDSSETTINGS_OPT_IS_TOPL_DETECT_STALE_DISABLED = 0x00000008
         # When DETECT_STALE_DISABLED, we can never know of if it's in a failed state
         if self.my_site.site_options & 0x00000008:
             return False
-        elif self.is_stale_link_connection(dsa):
-            return True
 
-        return detect_failed # TODO WHY?
+        return self.is_stale_link_connection(dsa)
+
 
     def create_connection(self, part, rbh, rsite, transport,
                           lbh, lsite, link_opt, link_sched,