[PATCH] finddcs_ldap: make sure we process all the addresses
Alexander Bokovoy
ab at samba.org
Wed May 20 06:40:10 MDT 2015
On Wed, May 20, 2015 at 10:43:13AM +0200, Stefan (metze) Metzmacher wrote:
> 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.
New patch is attached. I've stored the status into the state and now
return either not found or the status depending of whether we processed
whole list of possible candidates properly or with an error.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 982a6aee336c9ee5c8c3dad2c6438458ddd8ff04 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 | 39 +++++++++++++++++++++++++++++----------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/source4/libcli/finddcs_cldap.c b/source4/libcli/finddcs_cldap.c
index ce0e1c7..940737f 100644
--- a/source4/libcli/finddcs_cldap.c
+++ b/source4/libcli/finddcs_cldap.c
@@ -42,6 +42,7 @@ struct finddcs_cldap_state {
uint32_t srv_address_index;
struct cldap_socket *cldap;
struct cldap_netlogon *netlogon;
+ NTSTATUS status;
};
static void finddcs_cldap_srv_resolved(struct composite_context *ctx);
@@ -245,10 +246,13 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
struct tevent_req *subreq;
struct tsocket_address *dest;
int ret;
- NTSTATUS status;
if (state->srv_addresses[state->srv_address_index] == NULL) {
- tevent_req_nterror(state->req, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+ if (NT_STATUS_IS_OK(state->status)) {
+ tevent_req_nterror(state->req, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+ } else {
+ tevent_req_nterror(state->req, state->status);
+ }
DEBUG(2,("finddcs: No matching CLDAP server found\n"));
return;
}
@@ -259,22 +263,29 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
389,
&dest);
if (ret == 0) {
- status = NT_STATUS_OK;
+ state->status = NT_STATUS_OK;
} else {
- status = map_nt_error_from_unix_common(errno);
+ state->status = map_nt_error_from_unix_common(errno);
}
- if (tevent_req_nterror(state->req, status)) {
+ if (!NT_STATUS_IS_OK(state->status)) {
+ state->srv_address_index++;
+ finddcs_cldap_next_server(state);
return;
}
- status = cldap_socket_init(state, NULL, dest, &state->cldap);
- if (tevent_req_nterror(state->req, status)) {
+ state->status = cldap_socket_init(state, NULL, dest, &state->cldap);
+ if (!NT_STATUS_IS_OK(state->status)) {
+ state->srv_address_index++;
+ finddcs_cldap_next_server(state);
return;
}
TALLOC_FREE(state->netlogon);
state->netlogon = talloc_zero(state, struct cldap_netlogon);
- if (tevent_req_nomem(state->netlogon, state->req)) {
+ if (state->netlogon == NULL) {
+ state->status = NT_STATUS_NO_MEMORY;
+ state->srv_address_index++;
+ finddcs_cldap_next_server(state);
return;
}
@@ -283,7 +294,10 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
}
if (state->domain_sid) {
state->netlogon->in.domain_sid = dom_sid_string(state, state->domain_sid);
- if (tevent_req_nomem(state->netlogon->in.domain_sid, state->req)) {
+ if (state->netlogon->in.domain_sid == NULL) {
+ state->status = NT_STATUS_NO_MEMORY;
+ state->srv_address_index++;
+ finddcs_cldap_next_server(state);
return;
}
}
@@ -299,7 +313,10 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
subreq = cldap_netlogon_send(state, state->ev,
state->cldap, state->netlogon);
- if (tevent_req_nomem(subreq, state->req)) {
+ if (subreq == NULL) {
+ state->status = NT_STATUS_NO_MEMORY;
+ state->srv_address_index++;
+ finddcs_cldap_next_server(state);
return;
}
@@ -321,6 +338,7 @@ static void finddcs_cldap_netlogon_replied(struct tevent_req *subreq)
TALLOC_FREE(subreq);
TALLOC_FREE(state->cldap);
if (!NT_STATUS_IS_OK(status)) {
+ state->status = status;
state->srv_address_index++;
finddcs_cldap_next_server(state);
return;
@@ -364,6 +382,7 @@ static void finddcs_cldap_name_resolved(struct composite_context *ctx)
state->srv_address_index = 0;
+ state->status = NT_STATUS_OK;
finddcs_cldap_next_server(state);
}
--
2.1.0
More information about the samba-technical
mailing list