[PATCH] finddcs_ldap: make sure we process all the addresses

[Stefan (metze) Metzmacher]

Hi Alexander,

> this bug was found by Red Hat's QE few years ago but we never were able
> to find out why things failed apart from a vague notion of 'it is IPv6
> related'. In last few weeks its head raised again and I think I finally
> found the reason.
> 
> When we do CLDAP-based resolution of the AD DCs, one can specify exact
> server to probe or rely on SRV records from the DNS. If SRV records
> return few DCs, we are supposed to try them one by one, until we get
> successful repsonse. We start by running (essentially) getaddrinfo() and
> then creating a UDP socket to send the CLDAP request. We then switch to
> the next AD DC to try if returned result from the current request is not
> what we wanted.
> 
> However, if CLDAP request wasn't sent in the first place, the whole
> tevent request is cancelled. As result, no attempt is made to look at
> the other AD DCs.
> 
> This may happen, for example, if one of AD DCs is running on IPv6
> address and the machine which is initiating CLDAP ping has no IPv6
> connectivity. In this case we would get 'network unreachable' or a
> similar error and would fail to communicate. A similar behavior could be
> due to a firewall configuration, for example.
> 
> The solution is to make sure we are switching to the next address in
> case of failure. I've discussed this with Simo, Guenther, and David
> Holder, and the general consensus is to do just that for network-related
> issues but I also think we should be skipping to the next address even
> in the case of memory failures.
> 
> Attached patch should fix the issue.
> -- / Alexander Bokovoy
> 
> 
> 0001-s4-libcli-finddcs_cldap-continue-processing-CLDAP-un.patch
> 
> 
> From f5e10e64f7426fa063c495cc3310b0a1b01179a3 Mon Sep 17 00:00:00 2001
> From: Alexander Bokovoy <ab at samba.org>
> Date: Wed, 20 May 2015 11:17:38 +0300
> Subject: [PATCH] s4: libcli/finddcs_cldap: continue processing CLDAP until all
>  addresses are used
> 
> This is a subtle bug that causes CLDAP pings to fail if SRV records
> discovered cannot be resolved or connection to them cannot be
> established. The code that fires up CLDAP ping will silently cancel
> the whole tevent request without going to the next server in the queue.
> 
> This may happen, for example, when connection to IPv6 addresses couldn't
> be established, or when IPv4 address is not online or blocked by
> firewall.
> 
> Signed-off-by: Alexander Bokovoy <ab at samba.org>
> ---
>  source4/libcli/finddcs_cldap.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/source4/libcli/finddcs_cldap.c b/source4/libcli/finddcs_cldap.c
> index ce0e1c7..02513f6 100644
> --- a/source4/libcli/finddcs_cldap.c
> +++ b/source4/libcli/finddcs_cldap.c
> @@ -264,17 +264,23 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
>  		status = map_nt_error_from_unix_common(errno);
>  	}
>  	if (tevent_req_nterror(state->req, status)) {
> +		state->srv_address_index++;
> +		finddcs_cldap_next_server(state);
>  		return;
>  	}

I guess this should be:

 	if (!NT_STATUS_IS_OK(status)) {
+		state->srv_address_index++;
+		finddcs_cldap_next_server(state);
  		return;
  	}

Otherwise tevent_req_nterror() or tevent_req_nomem() already finish the
request
and notify the callers callback which calls finddcs_cldap_recv() which
destroys
'state'.

We may want to record 'status' in the state so that
finddcs_cldap_next_server() can report it if there're no more addresses
to try.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150520/cbca8a55/attachment.pgp>