CVE-2015-5370: s4:librpc/rpc: avoid using dcecli_security->auth_info and use per...

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 <metze@samba.org>
Reviewed-by: Günther Deschner <gd@samba.org>

diff --git a/selftest/knownfail b/selftest/knownfail
index e17bceedfaa3113c3ea2a54730321d711682110b..013c3cac2f160e1988c52b8e53b338a14337a99f 100644 (file)
--- 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 5113f63a22a45c7c348b71b4caeff71fad61b718..908fed20113decbe1571471b073626266cd790f9 100644 (file)
--- 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;
@@ -1226,7 +1225,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);
        }
@@ -1299,6 +1298,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;
@@ -1372,11 +1372,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;
                }
@@ -1426,9 +1428,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;
        }
@@ -2234,7 +2235,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);
        }
@@ -2307,6 +2308,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;
 
        /*
@@ -2365,12 +2367,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 c7299fec5e6be8da0fa2ddc17cd6530a602a784f..39d28a6e5d4bff6072ae663172906bda4a7cd021 100644 (file)
--- 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 443c7587e72500d991e2af138908aba71683ad8c..15a843b4ef5a9c86159643b1a90096d6218e3422 100644 (file)
--- 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);