From: Stefan Metzmacher Date: Sat, 27 Jun 2015 08:31:48 +0000 (+0200) Subject: CVE-2015-5370: s4:librpc/rpc: avoid using dcecli_security->auth_info and use per... X-Git-Tag: samba-4.2.10~83 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f7bb07a825db7739bbe5f549811ef86514b5697;p=thirdparty%2Fsamba.git CVE-2015-5370: s4:librpc/rpc: avoid using dcecli_security->auth_info and use per request values We now avoid reusing the same auth_info structure for incoming and outgoing values. We need to make sure that the remote server doesn't overwrite our own values. This will trigger some failures with our currently broken server, which will be fixed in the next commits. The broken server requires an dcerpc_auth structure with no credentials in order to do an alter_context request that just creates a presentation context without doing authentication. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11344 Signed-off-by: Stefan Metzmacher Reviewed-by: Günther Deschner --- diff --git a/selftest/knownfail b/selftest/knownfail index f8a52cf91e3..803439df7fe 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -1,3 +1,7 @@ +# These are temporary failures until the next commits fix it again +# +^samba4.rpc.altercontext.*seal # tmp +^samba4.rpc.altercontext.*ncalrpc # tmp # This file contains a list of regular expressions matching the names of # tests that are expected to fail. # diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c index 9a1b87b6b42..592890a6878 100644 --- a/source4/librpc/rpc/dcerpc.c +++ b/source4/librpc/rpc/dcerpc.c @@ -144,7 +144,6 @@ static struct dcecli_connection *dcerpc_connection_init(TALLOC_CTX *mem_ctx, c->security_state.auth_type = DCERPC_AUTH_TYPE_NONE; c->security_state.auth_level = DCERPC_AUTH_LEVEL_NONE; c->security_state.auth_context_id = 0; - c->security_state.auth_info = NULL; c->security_state.session_key = dcerpc_generic_session_key; c->security_state.generic_state = NULL; c->flags = 0; @@ -1224,7 +1223,7 @@ struct tevent_req *dcerpc_bind_send(TALLOC_CTX *mem_ctx, /* construct the NDR form of the packet */ status = ncacn_push_auth(&blob, state, &pkt, - p->conn->security_state.auth_info); + p->conn->security_state.tmp_auth_info.out); if (tevent_req_nterror(req, status)) { return tevent_req_post(req, ev); } @@ -1297,6 +1296,7 @@ static void dcerpc_bind_recv_handler(struct rpc_request *subreq, tevent_req_data(req, struct dcerpc_bind_state); struct dcecli_connection *conn = state->p->conn; + struct dcecli_security *sec = &conn->security_state; struct dcerpc_binding *b = NULL; NTSTATUS status; uint32_t flags; @@ -1370,11 +1370,13 @@ static void dcerpc_bind_recv_handler(struct rpc_request *subreq, } /* the bind_ack might contain a reply set of credentials */ - if (conn->security_state.auth_info && pkt->u.bind_ack.auth_info.length) { + if (pkt->auth_length != 0 && sec->tmp_auth_info.in != NULL) { uint32_t auth_length; - status = dcerpc_pull_auth_trailer(pkt, conn, &pkt->u.bind_ack.auth_info, - conn->security_state.auth_info, &auth_length, true); + status = dcerpc_pull_auth_trailer(pkt, sec->tmp_auth_info.mem, + &pkt->u.bind_ack.auth_info, + sec->tmp_auth_info.in, + &auth_length, true); if (tevent_req_nterror(req, status)) { return; } @@ -1424,9 +1426,8 @@ NTSTATUS dcerpc_auth3(struct dcerpc_pipe *p, } /* construct the NDR form of the packet */ - status = ncacn_push_auth(&blob, mem_ctx, - &pkt, - p->conn->security_state.auth_info); + status = ncacn_push_auth(&blob, mem_ctx, &pkt, + p->conn->security_state.tmp_auth_info.out); if (!NT_STATUS_IS_OK(status)) { return status; } @@ -2232,7 +2233,7 @@ struct tevent_req *dcerpc_alter_context_send(TALLOC_CTX *mem_ctx, /* construct the NDR form of the packet */ status = ncacn_push_auth(&blob, state, &pkt, - p->conn->security_state.auth_info); + p->conn->security_state.tmp_auth_info.out); if (tevent_req_nterror(req, status)) { return tevent_req_post(req, ev); } @@ -2305,6 +2306,7 @@ static void dcerpc_alter_context_recv_handler(struct rpc_request *subreq, tevent_req_data(req, struct dcerpc_alter_context_state); struct dcecli_connection *conn = state->p->conn; + struct dcecli_security *sec = &conn->security_state; NTSTATUS status; /* @@ -2363,12 +2365,13 @@ static void dcerpc_alter_context_recv_handler(struct rpc_request *subreq, } /* the alter_resp might contain a reply set of credentials */ - if (conn->security_state.auth_info && - pkt->u.alter_resp.auth_info.length) { + if (pkt->auth_length != 0 && sec->tmp_auth_info.in != NULL) { uint32_t auth_length; - status = dcerpc_pull_auth_trailer(pkt, conn, &pkt->u.alter_resp.auth_info, - conn->security_state.auth_info, &auth_length, true); + status = dcerpc_pull_auth_trailer(pkt, sec->tmp_auth_info.mem, + &pkt->u.alter_resp.auth_info, + sec->tmp_auth_info.in, + &auth_length, true); if (tevent_req_nterror(req, status)) { return; } diff --git a/source4/librpc/rpc/dcerpc.h b/source4/librpc/rpc/dcerpc.h index 541afd4b060..1b0eb7d9ba1 100644 --- a/source4/librpc/rpc/dcerpc.h +++ b/source4/librpc/rpc/dcerpc.h @@ -49,7 +49,11 @@ struct dcecli_security { enum dcerpc_AuthType auth_type; enum dcerpc_AuthLevel auth_level; uint32_t auth_context_id; - struct dcerpc_auth *auth_info; + struct { + struct dcerpc_auth *out; + struct dcerpc_auth *in; + TALLOC_CTX *mem; + } tmp_auth_info; struct gensec_security *generic_state; /* get the session key */ diff --git a/source4/librpc/rpc/dcerpc_auth.c b/source4/librpc/rpc/dcerpc_auth.c index 443c7587e72..15a843b4ef5 100644 --- a/source4/librpc/rpc/dcerpc_auth.c +++ b/source4/librpc/rpc/dcerpc_auth.c @@ -124,7 +124,8 @@ _PUBLIC_ NTSTATUS dcerpc_bind_auth_none(struct dcerpc_pipe *p, struct bind_auth_state { struct dcerpc_pipe *pipe; - DATA_BLOB credentials; + struct dcerpc_auth out_auth_info; + struct dcerpc_auth in_auth_info; bool more_processing; /* Is there anything more to do after the * first bind itself received? */ }; @@ -141,6 +142,12 @@ static void bind_auth_next_step(struct composite_context *c) state = talloc_get_type(c->private_data, struct bind_auth_state); sec = &state->pipe->conn->security_state; + state->out_auth_info = (struct dcerpc_auth) { + .auth_type = sec->auth_type, + .auth_level = sec->auth_level, + .auth_context_id = sec->auth_context_id, + }; + /* The status value here, from GENSEC is vital to the security * of the system. Even if the other end accepts, if GENSEC * claims 'MORE_PROCESSING_REQUIRED' then you must keep @@ -156,16 +163,14 @@ static void bind_auth_next_step(struct composite_context *c) c->status = gensec_update_ev(sec->generic_state, state, state->pipe->conn->event_ctx, - sec->auth_info->credentials, - &state->credentials); + state->in_auth_info.credentials, + &state->out_auth_info.credentials); if (state->pipe->timed_out) { composite_error(c, NT_STATUS_IO_TIMEOUT); return; } state->pipe->inhibit_timeout_processing = false; - data_blob_free(&sec->auth_info->credentials); - if (NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { more_processing = true; c->status = NT_STATUS_OK; @@ -173,18 +178,21 @@ static void bind_auth_next_step(struct composite_context *c) if (!composite_is_ok(c)) return; - if (state->credentials.length == 0) { + if (state->out_auth_info.credentials.length == 0) { composite_done(c); return; } - sec->auth_info->credentials = state->credentials; + state->in_auth_info = (struct dcerpc_auth) { + .auth_type = DCERPC_AUTH_TYPE_NONE, + }; + sec->tmp_auth_info.in = &state->in_auth_info; + sec->tmp_auth_info.mem = state; + sec->tmp_auth_info.out = &state->out_auth_info; if (!more_processing) { /* NO reply expected, so just send it */ c->status = dcerpc_auth3(state->pipe, state); - data_blob_free(&state->credentials); - sec->auth_info->credentials = data_blob(NULL, 0); if (!composite_is_ok(c)) return; composite_done(c); @@ -197,8 +205,6 @@ static void bind_auth_next_step(struct composite_context *c) state->pipe, &state->pipe->syntax, &state->pipe->transfer_syntax); - data_blob_free(&state->credentials); - sec->auth_info->credentials = data_blob(NULL, 0); if (composite_nomem(subreq, c)) return; tevent_req_set_callback(subreq, bind_auth_recv_alter, c); } @@ -209,6 +215,11 @@ static void bind_auth_recv_alter(struct tevent_req *subreq) struct composite_context *c = tevent_req_callback_data(subreq, struct composite_context); + struct bind_auth_state *state = talloc_get_type(c->private_data, + struct bind_auth_state); + struct dcecli_security *sec = &state->pipe->conn->security_state; + + ZERO_STRUCT(sec->tmp_auth_info); c->status = dcerpc_alter_context_recv(subreq); TALLOC_FREE(subreq); @@ -225,14 +236,15 @@ static void bind_auth_recv_bindreply(struct tevent_req *subreq) struct composite_context); struct bind_auth_state *state = talloc_get_type(c->private_data, struct bind_auth_state); + struct dcecli_security *sec = &state->pipe->conn->security_state; + + ZERO_STRUCT(sec->tmp_auth_info); c->status = dcerpc_bind_recv(subreq); TALLOC_FREE(subreq); if (!composite_is_ok(c)) return; if (state->pipe->conn->flags & DCERPC_HEADER_SIGNING) { - struct dcecli_security *sec = &state->pipe->conn->security_state; - gensec_want_feature(sec->generic_state, GENSEC_FEATURE_SIGN_PKT_HEADER); } @@ -362,15 +374,11 @@ struct composite_context *dcerpc_bind_auth_send(TALLOC_CTX *mem_ctx, */ sec->auth_context_id = 1; - sec->auth_info = talloc(p, struct dcerpc_auth); - if (composite_nomem(sec->auth_info, c)) return c; - - sec->auth_info->auth_type = sec->auth_type; - sec->auth_info->auth_level = sec->auth_level, - sec->auth_info->auth_pad_length = 0; - sec->auth_info->auth_reserved = 0; - sec->auth_info->auth_context_id = sec->auth_context_id; - sec->auth_info->credentials = data_blob(NULL, 0); + state->out_auth_info = (struct dcerpc_auth) { + .auth_type = sec->auth_type, + .auth_level = sec->auth_level, + .auth_context_id = sec->auth_context_id, + }; /* The status value here, from GENSEC is vital to the security * of the system. Even if the other end accepts, if GENSEC @@ -386,8 +394,8 @@ struct composite_context *dcerpc_bind_auth_send(TALLOC_CTX *mem_ctx, state->pipe->timed_out = false; c->status = gensec_update_ev(sec->generic_state, state, p->conn->event_ctx, - sec->auth_info->credentials, - &state->credentials); + data_blob_null, + &state->out_auth_info.credentials); if (state->pipe->timed_out) { composite_error(c, NT_STATUS_IO_TIMEOUT); return c; @@ -403,25 +411,28 @@ struct composite_context *dcerpc_bind_auth_send(TALLOC_CTX *mem_ctx, state->more_processing = NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED); - if (state->credentials.length == 0) { + if (state->out_auth_info.credentials.length == 0) { composite_done(c); return c; } - sec->auth_info->credentials = state->credentials; - if (gensec_have_feature(sec->generic_state, GENSEC_FEATURE_SIGN_PKT_HEADER)) { - if (auth_level >= DCERPC_AUTH_LEVEL_INTEGRITY) { + if (sec->auth_level >= DCERPC_AUTH_LEVEL_INTEGRITY) { state->pipe->conn->flags |= DCERPC_PROPOSE_HEADER_SIGNING; } } + state->in_auth_info = (struct dcerpc_auth) { + .auth_type = DCERPC_AUTH_TYPE_NONE, + }; + sec->tmp_auth_info.in = &state->in_auth_info; + sec->tmp_auth_info.mem = state; + sec->tmp_auth_info.out = &state->out_auth_info; + /* The first request always is a dcerpc_bind. The subsequent ones * depend on gensec results */ subreq = dcerpc_bind_send(state, p->conn->event_ctx, p, &syntax, &transfer_syntax); - data_blob_free(&state->credentials); - sec->auth_info->credentials = data_blob(NULL, 0); if (composite_nomem(subreq, c)) return c; tevent_req_set_callback(subreq, bind_auth_recv_bindreply, c);