[PATCH] async dcerpc pipe connect code in samba4
Stefan (metze) Metzmacher
metze at samba.org
Wed Mar 8 10:37:58 GMT 2006
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Rafal Szczesniak schrieb:
> Hi all,
>
> Attached patch contains async rpc pipe connect code. Since
> the code grew quite big, I'm not just commiting it. I'd rather
> post the change here first - I might have missed something,
> so another pair of eyes could be of use. Apologies for the code
> doesn't have too many comments. The original didn't either, but
> I will make it up soon, once the fresh meat is in.
>
> The code doesn't break the build and passes all tests (with a few
> exceptions - just like the current tree) on my debian box.
>
> Please, don't be too harsh in criticism... ;-)
I think I found now bug:-) only some missings of composite_momem,
but this checks weren't there before too.
I only have some ideas to make the code more readable and shorter:
- - make use of composite_nomem() and composite_is_ok()
- - remove goto done; and use return c; more often. to make clear nothing is following 50 lines later...
- - use TALLOC_CTX *mem_ctx, struct dcerpc_pipe_context **p2)
in the _recv hook for the second pipe too
when you have a new patch, I'll run a short make test and then you can commit:-)
metze
> + switch (s->binding->transport) {
> + case NCACN_NP:
> + if (pc.binding->flags & DCERPC_SMB2) {
> + /* new varient of SMB a.k.a. SMB2 */
> + ncacn_np_smb2_req = dcerpc_pipe_connect_ncacn_np_smb2_send(c, &pc);
> + if (ncacn_np_smb2_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + return;
> + }
we have a short form for this:
if (composite_nomem(ncacn_np_smb2_req, c)) return;
please use this in mostly all cases...
> + composite_continue(c, ncacn_np_smb2_req, continue_pipe_connect_ncacn_np_smb2, c);
you may could add a 'return;' here to make clear that nothing will happen later in this function
> + } else {
> + /* good old ordinary SMB */
> + ncacn_np_smb_req = dcerpc_pipe_connect_ncacn_np_smb_send(c, &pc);
> + if (ncacn_np_smb_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + return;
> + }
> + composite_continue(c, ncacn_np_smb_req, continue_pipe_connect_ncacn_np_smb, c);
you may could add a 'return;' here to make clear that nothing will happen later in this function
> + }
> + break;
you may could add a 'return;' insstead of break; here to make clear that nothing will happen later
in this function
there're also a few other places...
> + case NCACN_IP_TCP:
> + ncacn_ip_tcp_req = dcerpc_pipe_connect_ncacn_ip_tcp_send(c, &pc);
> + if (ncacn_ip_tcp_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + return;
> + }
> + composite_continue(c, ncacn_ip_tcp_req, continue_pipe_connect_ncacn_ip_tcp, c);
> + break;
> +void continue_pipe_connect_ncacn_np_smb2(struct composite_context *ctx)
> +{
> + struct composite_context *c = talloc_get_type(ctx->async.private_data,
> + struct composite_context);
> + struct pipe_connect_state *s = talloc_get_type(c->private_data,
> + struct pipe_connect_state);
> +
> + c->status = dcerpc_pipe_connect_ncacn_np_smb2_recv(ctx);
> + if (!NT_STATUS_IS_OK(c->status)) {
> + composite_error(c, c->status);
> + return;
> + }
we also have a short form for this:
if (!composite_is_ok(c)) return;
please use this in all places...
> + continue_pipe_connect(c, s);
> +}
> +
> +
> +struct composite_context* dcerpc_pipe_connect_b_send(TALLOC_CTX *parent_ctx,
> + struct dcerpc_pipe **pp,
> + struct dcerpc_binding *binding,
> + const struct dcerpc_interface_table *table,
> + struct cli_credentials *credentials,
> + struct event_context *ev)
> +{
> + struct composite_context *c;
> + struct pipe_connect_state *s;
> +
> + struct composite_context *binding_req;
> +
> + /* composite context allocation and setup */
> + c = talloc_zero(parent_ctx, struct composite_context);
> + if (c == NULL) return NULL;
> +
> + s = talloc_zero(c, struct pipe_connect_state);
> + if (s == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + goto done;
> + }
if (composite_nomem(s, c)) return;
> + c->state = COMPOSITE_STATE_IN_PROGRESS;
> + c->private_data = s;
> +
> + switch (s->binding->transport) {
> + case NCACN_NP:
> + case NCACN_IP_TCP:
> + case NCALRPC:
> + if (!s->binding->endpoint) {
> + binding_req = dcerpc_epm_map_binding_send(c, s->binding, s->table,
> + s->pipe->conn->event_ctx);
> + composite_continue(c, binding_req, continue_map_binding, c);
> + goto done;
a 'return c;' would make it a bit more readable and is also just one line:-)
> + }
> + break;
> +
> + default:
> + break;
> + }
> +
> + continue_connect(c, s);
> +done:
> + return c;
> +}
> + status = dcerpc_parse_binding(c, binding, &b);
> + if (!NT_STATUS_IS_OK(status)) {
> + DEBUG(0, ("Failed to parse dcerpc binding '%s'\n", binding));
> + composite_error(c, status);
> + goto done;
> + }
maybe use c->status = ...
if (!composite_is_ok(c)) return c;
here
> +
> + DEBUG(3, ("Using binding %s\n", dcerpc_binding_string(c, b)));
> +
> + pipe_conn_req = dcerpc_pipe_connect_b_send(c, &s->pipe, b, table,
> + credentials, ev);
> + c->event_ctx = s->pipe->conn->event_ctx;
> +struct sec_conn_state {
> + struct dcerpc_pipe *pipe;
> + struct dcerpc_pipe **pipe2;
maybe use just *pipe2 here
> + struct dcerpc_binding *binding;
> + struct smbcli_tree *tree;
> +struct composite_context* dcerpc_secondary_connection_send(TALLOC_CTX *mem_ctx,
> + struct dcerpc_pipe *p,
> + struct dcerpc_pipe **p2,
and maybe move **p2 to the _recv function...
like for the functions which create the first pipe
> +void continue_open_smb(struct composite_context *ctx)
> +{
> + struct composite_context *c = talloc_get_type(ctx->async.private_data,
> + struct composite_context);
> + struct sec_conn_state *s = talloc_get_type(c->private_data,
> + struct sec_conn_state);
> +
> + c->status = dcerpc_pipe_open_smb_recv(ctx);
> + if (!NT_STATUS_IS_OK(c->status)) {
> + composite_error(c, c->status);
> +
> + talloc_free(*s->pipe2);
is this explicit talloc_free needed here? maybe you can do a talloc_steal after it's allocated
above...
(same in some other places below)
> + return;
> + }
> +
> + continue_pipe_open(c);
> +}
> +
> +
> +NTSTATUS dcerpc_secondary_connection_recv(struct composite_context *c)
maybe add TALLOC_CTX *mem_ctx and struct dcerpc_pipe **p2
> +{
> + NTSTATUS status = composite_wait(c);
struct sec_conn_state *s;
s = talloc_get_type(c->private_data, struct sec_conn_state);
if (NT_STATUS_IS_OK(status)) {
*p2 = talloc_steal(mem_ctx, s->pipe2);
}
> + talloc_free(c);
> + return status;
> Index: librpc/rpc/dcerpc_schannel.c
> ===================================================================
> --- librpc/rpc/dcerpc_schannel.c (revision 13996)
> +++ librpc/rpc/dcerpc_schannel.c (working copy)
> @@ -23,142 +23,375 @@
>
> #include "includes.h"
> #include "auth/auth.h"
> +#include "libcli/composite/composite.h"
> #include "libcli/auth/proto.h"
>
> -/*
> - get a schannel key using a netlogon challenge on a secondary pipe
> -*/
> -static NTSTATUS dcerpc_schannel_key(TALLOC_CTX *tmp_ctx,
> - struct dcerpc_pipe *p,
> - struct cli_credentials *credentials)
> -{
> - NTSTATUS status;
> - struct dcerpc_binding *b;
> - struct dcerpc_pipe *p2;
> +
> +struct schannel_key_state {
> + struct dcerpc_pipe *pipe;
> + struct dcerpc_pipe *pipe2;
> + struct dcerpc_binding *binding;
> + struct cli_credentials *credentials;
> + struct creds_CredentialState *creds;
> + uint32_t negotiate_flags;
> + struct netr_Credential credentials1;
> + struct netr_Credential credentials2;
> + struct netr_Credential credentials3;
> struct netr_ServerReqChallenge r;
> struct netr_ServerAuthenticate2 a;
> - struct netr_Credential credentials1, credentials2, credentials3;
> const struct samr_Password *mach_pwd;
> - uint32_t negotiate_flags;
> - struct creds_CredentialState *creds;
> - creds = talloc(tmp_ctx, struct creds_CredentialState);
> - if (!creds) {
> - return NT_STATUS_NO_MEMORY;
> +};
> +
> +
> +void continue_secondary_connection(struct composite_context *ctx);
> +void continue_bind_auth_none(struct composite_context *ctx);
> +void continue_srv_challenge(struct rpc_request *req);
> +void continue_srv_auth2(struct rpc_request *req);
> +
> +
> +void continue_epm_map_binding(struct composite_context *ctx)
> +{
> + struct composite_context *c;
> + struct schannel_key_state *s;
> + struct composite_context *sec_conn_req;
> +
> + c = talloc_get_type(ctx->async.private_data, struct composite_context);
> + s = talloc_get_type(c->private_data, struct schannel_key_state);
> +
> + c->status = dcerpc_epm_map_binding_recv(ctx);
> + if (!NT_STATUS_IS_OK(c->status)) {
> + DEBUG(0,("Failed to map DCERPC/TCP NCACN_NP pipe for '%s' - %s\n",
> + DCERPC_NETLOGON_UUID, nt_errstr(c->status)));
> + composite_error(c, c->status);
> + return;
> }
>
> - if (p->conn->flags & DCERPC_SCHANNEL_128) {
> - negotiate_flags = NETLOGON_NEG_AUTH2_ADS_FLAGS;
> - } else {
> - negotiate_flags = NETLOGON_NEG_AUTH2_FLAGS;
> + sec_conn_req = dcerpc_secondary_connection_send(c, s->pipe, &s->pipe2,
> + s->binding);
> + if (sec_conn_req == NULL) {
> + composite_error(c, c->status);
> + return;
> }
>
> - /*
> - step 1 - establish a netlogon connection, with no authentication
> - */
> + composite_continue(c, sec_conn_req, continue_secondary_connection, c);
> +}
>
> - b = talloc(tmp_ctx, struct dcerpc_binding);
> - if (!b) {
> - return NT_STATUS_NO_MEMORY;
> +
> +void continue_secondary_connection(struct composite_context *ctx)
> +{
> + struct composite_context *c;
> + struct schannel_key_state *s;
> + struct composite_context *auth_none_req;
> +
> + c = talloc_get_type(ctx->async.private_data, struct composite_context);
> + s = talloc_get_type(c->private_data, struct schannel_key_state);
> +
> + c->status = dcerpc_secondary_connection_recv(ctx);
> + if (!NT_STATUS_IS_OK(c->status)) {
> + composite_error(c, c->status);
> + return;
> }
> - *b = *p->binding;
>
> - /* Make binding string for netlogon, not the other pipe */
> - status = dcerpc_epm_map_binding(tmp_ctx, b,
> - &dcerpc_table_netlogon,
> - p->conn->event_ctx);
> - if (!NT_STATUS_IS_OK(status)) {
> - DEBUG(0,("Failed to map DCERPC/TCP NCACN_NP pipe for '%s' - %s\n",
> - DCERPC_NETLOGON_UUID, nt_errstr(status)));
> - return status;
> + auth_none_req = dcerpc_bind_auth_none_send(c, s->pipe2, &dcerpc_table_netlogon);
> + if (auth_none_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + return;
> }
>
> - status = dcerpc_secondary_connection(p, &p2, b);
> - if (!NT_STATUS_IS_OK(status)) {
> - return status;
> + composite_continue(c, auth_none_req, continue_bind_auth_none, c);
> +}
> +
> +
> +void continue_bind_auth_none(struct composite_context *ctx)
> +{
> + struct composite_context *c;
> + struct schannel_key_state *s;
> + struct rpc_request *srv_challenge_req;
> +
> + c = talloc_get_type(ctx->async.private_data, struct composite_context);
> + s = talloc_get_type(c->private_data, struct schannel_key_state);
> +
> + c->status = dcerpc_bind_auth_none_recv(ctx);
> + if (!NT_STATUS_IS_OK(c->status)) {
> + composite_error(c, c->status);
> +
> + talloc_free(s->pipe2);
> + return;
> }
>
> - status = dcerpc_bind_auth_none(p2, &dcerpc_table_netlogon);
> - if (!NT_STATUS_IS_OK(status)) {
> - talloc_free(p2);
> - return status;
> - }
> + s->r.in.server_name = talloc_asprintf(c, "\\\\%s", dcerpc_server_name(s->pipe));
if (composite_nomem(s->r.in.server_name, c)) return;
> + s->r.in.computer_name = cli_credentials_get_workstation(s->credentials);
maybe here too, but check if NULL isn't a valid return value
> + s->r.in.credentials = &s->credentials1;
> + s->r.out.credentials = &s->credentials2;
> /*
> - step 3 - authenticate on the netlogon pipe
> + authenticate on the netlogon pipe
> */
> - mach_pwd = cli_credentials_get_nt_hash(credentials, tmp_ctx);
>
> - creds_client_init(creds, &credentials1, &credentials2,
> - mach_pwd, &credentials3,
> - negotiate_flags);
> + s->mach_pwd = cli_credentials_get_nt_hash(s->credentials, c);
maybe: if (composite_nomem...
> - a.in.server_name = r.in.server_name;
> - a.in.account_name = cli_credentials_get_username(credentials);
> - a.in.secure_channel_type =
> - cli_credentials_get_secure_channel_type(credentials);
> - a.in.computer_name = cli_credentials_get_workstation(credentials);
> - a.in.negotiate_flags = &negotiate_flags;
> - a.out.negotiate_flags = &negotiate_flags;
> - a.in.credentials = &credentials3;
> - a.out.credentials = &credentials3;
> + creds_client_init(s->creds, &s->credentials1, &s->credentials2,
> + s->mach_pwd, &s->credentials3, s->negotiate_flags);
>
> - status = dcerpc_netr_ServerAuthenticate2(p2, tmp_ctx, &a);
> - if (!NT_STATUS_IS_OK(status)) {
> - return status;
> + s->a.in.server_name = s->r.in.server_name;
> + s->a.in.account_name = cli_credentials_get_username(s->credentials);
maybe : if (composite_nomem...
> + s->a.in.secure_channel_type =
> + cli_credentials_get_secure_channel_type(s->credentials);
> + s->a.in.computer_name = cli_credentials_get_workstation(s->credentials);
maybe : if (composite_nomem...
> + s->a.in.negotiate_flags = &s->negotiate_flags;
> + s->a.in.credentials = &s->credentials3;
> + s->a.out.negotiate_flags = &s->negotiate_flags;
> + s->a.out.credentials = &s->credentials3;
> + if (table) {
> + struct dcerpc_binding *default_binding;
> +
> + /* Find one of the default pipes for this interface */
> + for (i = 0; i < table->endpoints->count; i++) {
> + status = dcerpc_parse_binding(mem_ctx, table->endpoints->names[i], &default_binding);
> +
> + if (NT_STATUS_IS_OK(status)) {
> + if (default_binding->transport == binding->transport && default_binding->endpoint) {
> + binding->endpoint = talloc_reference(binding, default_binding->endpoint);
if (composite_nomem(...) return c;
> + talloc_free(default_binding);
> +
> + composite_done(c);
> + goto done;
> +
> + } else {
> + talloc_free(default_binding);
> + }
> + }
> + }
> + }
> +
> + epmapper_binding = talloc_zero(c, struct dcerpc_binding);
> + if (!epmapper_binding) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + goto done;
> + }
> +
> + epmapper_binding->transport = binding->transport;
> + epmapper_binding->host = talloc_reference(epmapper_binding, binding->host);
if (composite_nomem(...) return c;
> + epmapper_binding->options = NULL;
> + epmapper_binding->flags = 0;
> + epmapper_binding->endpoint = NULL;
> +
> + pipe_connect_req = dcerpc_pipe_connect_b_send(c, &s->pipe, epmapper_binding, &dcerpc_table_epmapper,
> + anon_creds, ev);
> + if (pipe_connect_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + goto done;
> + }
> +
> + composite_continue(c, pipe_connect_req, continue_epm_recv_binding, c);
> +
> +done:
> + return c;
> +}
> + if (!cli_credentials_is_anonymous(s->credentials) &&
> + (binding->flags & DCERPC_SCHANNEL) &&
> + !cli_credentials_get_netlogon_creds(s->credentials)) {
> +
> + /* If we don't already have netlogon credentials for
> + * the schannel bind, then we have to get these
> + * first */
> + auth_schannel_req = dcerpc_bind_auth_schannel_send(c, s->pipe, s->table,
> + s->credentials,
> + dcerpc_auth_level(s->pipe->conn));
> + if (auth_schannel_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + goto done;
> + }
> +
> + composite_continue(c, auth_schannel_req, continue_auth_schannel, c);
return c;
> + } else if (!cli_credentials_is_anonymous(s->credentials) &&
> + if (auth_req == NULL) {
> + composite_error(c, NT_STATUS_NO_MEMORY);
> + goto done;
> + }
> +
> + composite_continue(c, auth_req, continue_auth, c);
return c;
+ }
+ auth_none_req = dcerpc_bind_auth_none_send(c, s->pipe, s->table);
+ if (auth_none_req == NULL) {
+ composite_error(c, NT_STATUS_NO_MEMORY);
+ goto done;
+ }
+
+ composite_continue(c, auth_none_req, continue_auth_none, c);
+ return c;
+}
- --
metze
Stefan Metzmacher <metze at samba.org> www.samba.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3-nr1 (Windows XP)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFEDrO3m70gjA5TCD8RAqNMAJoC9V1iofkLSSB3l3bJeT5xnTYy6ACghxPE
5ga+o3x/d5lOeH+5WOi+9H0=
=vH55
-----END PGP SIGNATURE-----
More information about the samba-technical
mailing list