From 6da17dbdae743322e1fe25d2c4064157c9336499 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 7 Aug 2020 15:57:15 +0200 Subject: [PATCH 1/2] s3:rpc_client: reverse rpccli_{is_connected,set_timeout}() and rpccli_bh_{is_connected,set_timeout}() rpccli->transport should never be used directly, everything should go via the binding handle. Internal pipes don't have a transport, so p->transport is always NULL. rpccli_is_connected() checks this and this causes all SAMR and LSA requests for the local domain to be processed a second time by the triggered retry logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14457 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme --- source3/rpc_client/cli_pipe.c | 46 ++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/source3/rpc_client/cli_pipe.c b/source3/rpc_client/cli_pipe.c index 8227ef0b0bd..074d01828ad 100644 --- a/source3/rpc_client/cli_pipe.c +++ b/source3/rpc_client/cli_pipe.c @@ -2158,22 +2158,16 @@ NTSTATUS rpc_pipe_bind(struct rpc_pipe_client *cli, unsigned int rpccli_set_timeout(struct rpc_pipe_client *rpc_cli, unsigned int timeout) { - unsigned int old; - - if (rpc_cli->transport == NULL) { - return RPCCLI_DEFAULT_TIMEOUT; - } - - if (rpc_cli->transport->set_timeout == NULL) { + if (rpc_cli == NULL) { return RPCCLI_DEFAULT_TIMEOUT; } - old = rpc_cli->transport->set_timeout(rpc_cli->transport->priv, timeout); - if (old == 0) { + if (rpc_cli->binding_handle == NULL) { return RPCCLI_DEFAULT_TIMEOUT; } - return old; + return dcerpc_binding_handle_set_timeout(rpc_cli->binding_handle, + timeout); } bool rpccli_is_connected(struct rpc_pipe_client *rpc_cli) @@ -2182,11 +2176,11 @@ bool rpccli_is_connected(struct rpc_pipe_client *rpc_cli) return false; } - if (rpc_cli->transport == NULL) { + if (rpc_cli->binding_handle == NULL) { return false; } - return rpc_cli->transport->is_connected(rpc_cli->transport->priv); + return dcerpc_binding_handle_is_connected(rpc_cli->binding_handle); } struct rpccli_bh_state { @@ -2197,8 +2191,17 @@ static bool rpccli_bh_is_connected(struct dcerpc_binding_handle *h) { struct rpccli_bh_state *hs = dcerpc_binding_handle_data(h, struct rpccli_bh_state); + struct rpc_cli_transport *transport = hs->rpc_cli->transport; + + if (transport == NULL) { + return false; + } + + if (transport->is_connected == NULL) { + return false; + } - return rpccli_is_connected(hs->rpc_cli); + return transport->is_connected(transport->priv); } static uint32_t rpccli_bh_set_timeout(struct dcerpc_binding_handle *h, @@ -2206,8 +2209,23 @@ static uint32_t rpccli_bh_set_timeout(struct dcerpc_binding_handle *h, { struct rpccli_bh_state *hs = dcerpc_binding_handle_data(h, struct rpccli_bh_state); + struct rpc_cli_transport *transport = hs->rpc_cli->transport; + unsigned int old; - return rpccli_set_timeout(hs->rpc_cli, timeout); + if (transport == NULL) { + return RPCCLI_DEFAULT_TIMEOUT; + } + + if (transport->set_timeout == NULL) { + return RPCCLI_DEFAULT_TIMEOUT; + } + + old = transport->set_timeout(transport->priv, timeout); + if (old == 0) { + return RPCCLI_DEFAULT_TIMEOUT; + } + + return old; } static void rpccli_bh_auth_info(struct dcerpc_binding_handle *h, -- GitLab From 2707ca1cca096e53f8e7991dcd12b7789edea985 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 7 Aug 2020 12:07:28 +0200 Subject: [PATCH 2/2] winbind: directly use dcerpc_binding_handle_is_connected() in reset_connection_on_error() SAMR code In the end we should avoid rpccli_is_connected(), rpccli_set_timeout() and the whole rpc_pipe_client concept. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14457 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher --- source3/winbindd/winbindd_samr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c index 396e2c97709..5681a760bd1 100644 --- a/source3/winbindd/winbindd_samr.c +++ b/source3/winbindd/winbindd_samr.c @@ -185,6 +185,7 @@ static bool reset_connection_on_error(struct winbindd_domain *domain, NTSTATUS status) { struct winbind_internal_pipes *internal_pipes = NULL; + struct dcerpc_binding_handle *b = p->binding_handle; internal_pipes = talloc_get_type_abort( domain->private_data, struct winbind_internal_pipes); @@ -197,7 +198,7 @@ static bool reset_connection_on_error(struct winbindd_domain *domain, return true; } - if (!rpccli_is_connected(p)) { + if (!dcerpc_binding_handle_is_connected(b)) { TALLOC_FREE(internal_pipes); domain->private_data = NULL; return true; -- GitLab