From 5d2c73d5fb081c71cb09ef9780029d2b71404683 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2016 16:11:37 +0200 Subject: [PATCH 1/5] s4:rpc_server: parse auth data only for BIND,ALTER_REQ,AUTH3 We should tell dcerpc_pull_auth_trailer() that we only want auth data. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11982 Signed-off-by: Stefan Metzmacher --- source4/rpc_server/dcesrv_auth.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c index 2b3f8b0..802876b 100644 --- a/source4/rpc_server/dcesrv_auth.c +++ b/source4/rpc_server/dcesrv_auth.c @@ -44,7 +44,6 @@ bool dcesrv_auth_bind(struct dcesrv_call_state *call) struct dcesrv_connection *dce_conn = call->conn; struct dcesrv_auth *auth = &dce_conn->auth_state; NTSTATUS status; - uint32_t auth_length; if (pkt->auth_length == 0) { auth->auth_type = DCERPC_AUTH_TYPE_NONE; @@ -55,7 +54,7 @@ bool dcesrv_auth_bind(struct dcesrv_call_state *call) status = dcerpc_pull_auth_trailer(pkt, call, &pkt->u.bind.auth_info, &call->in_auth_info, - &auth_length, false); + NULL, true); if (!NT_STATUS_IS_OK(status)) { return false; } @@ -241,7 +240,6 @@ bool dcesrv_auth_auth3(struct dcesrv_call_state *call) struct ncacn_packet *pkt = &call->pkt; struct dcesrv_connection *dce_conn = call->conn; NTSTATUS status; - uint32_t auth_length; if (pkt->auth_length == 0) { return false; @@ -257,7 +255,7 @@ bool dcesrv_auth_auth3(struct dcesrv_call_state *call) } status = dcerpc_pull_auth_trailer(pkt, call, &pkt->u.auth3.auth_info, - &call->in_auth_info, &auth_length, true); + &call->in_auth_info, NULL, true); if (!NT_STATUS_IS_OK(status)) { return false; } @@ -324,7 +322,6 @@ bool dcesrv_auth_alter(struct dcesrv_call_state *call) struct ncacn_packet *pkt = &call->pkt; struct dcesrv_connection *dce_conn = call->conn; NTSTATUS status; - uint32_t auth_length; /* on a pure interface change there is no auth blob */ if (pkt->auth_length == 0) { @@ -344,7 +341,7 @@ bool dcesrv_auth_alter(struct dcesrv_call_state *call) } status = dcerpc_pull_auth_trailer(pkt, call, &pkt->u.alter.auth_info, - &call->in_auth_info, &auth_length, true); + &call->in_auth_info, NULL, true); if (!NT_STATUS_IS_OK(status)) { return false; } -- 1.9.1 From ae2ca37bd61c6e97ea100181dafd82eaf7695849 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2016 16:16:23 +0200 Subject: [PATCH 2/5] s4:librpc/rpc: don't ask for auth_length if we ask for auth data only dcerpc_pull_auth_trailer() handles auth_length=NULL just fine. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11982 Signed-off-by: Stefan Metzmacher --- source4/librpc/rpc/dcerpc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c index 464ae95..e073f53 100644 --- a/source4/librpc/rpc/dcerpc.c +++ b/source4/librpc/rpc/dcerpc.c @@ -1414,12 +1414,10 @@ static void dcerpc_bind_recv_handler(struct rpc_request *subreq, /* the bind_ack might contain a reply set of credentials */ if (pkt->auth_length != 0 && sec->tmp_auth_info.in != NULL) { - uint32_t auth_length; - 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); + NULL, true); if (tevent_req_nterror(req, status)) { return; } @@ -2434,12 +2432,10 @@ static void dcerpc_alter_context_recv_handler(struct rpc_request *subreq, /* the alter_resp might contain a reply set of credentials */ if (pkt->auth_length != 0 && sec->tmp_auth_info.in != NULL) { - uint32_t auth_length; - 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); + NULL, true); if (tevent_req_nterror(req, status)) { return; } -- 1.9.1 From f25c84ed50cdc68ea71434f387135f388879500c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2016 16:17:45 +0200 Subject: [PATCH 3/5] librpc/rpc: let dcerpc_pull_auth_trailer() only accept auth_length!=NULL or auth_data_only=true BUG: https://bugzilla.samba.org/show_bug.cgi?id=11982 Signed-off-by: Stefan Metzmacher --- librpc/rpc/dcerpc_util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index 43e1b7f..4d82e9a5 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -99,6 +99,14 @@ NTSTATUS dcerpc_pull_auth_trailer(const struct ncacn_packet *pkt, ZERO_STRUCTP(auth); if (_auth_length != NULL) { *_auth_length = 0; + + if (auth_data_only) { + return NT_STATUS_INTERNAL_ERROR; + } + } else { + if (!auth_data_only) { + return NT_STATUS_INTERNAL_ERROR; + } } /* Paranoia checks for auth_length. The caller should check this... */ -- 1.9.1 From 2bdd8e0e8600ba8447addd766087e6c56b676540 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2016 16:25:12 +0200 Subject: [PATCH 4/5] librpc/rpc: let dcerpc_pull_auth_trailer() check that auth_pad_length fits within the whole pdu. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11982 Signed-off-by: Stefan Metzmacher --- librpc/rpc/dcerpc_util.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index 4d82e9a5..768cb3d 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -157,6 +157,33 @@ NTSTATUS dcerpc_pull_auth_trailer(const struct ncacn_packet *pkt, return ndr_map_error2ntstatus(ndr_err); } + /* + * Make sure the padding would not exceed + * the frag_length. + * + * Here we assume at least 24 bytes for the + * payload specific header the value of + * DCERPC_{REQUEST,RESPONSE}_LENGTH. + * + * We use this also for BIND_* and ALTER_* pdus. + * + * We need this check before we ignore possible + * invalid values. See also bug #11982. + */ + tmp_length = pkt->frag_length; + tmp_length -= DCERPC_REQUEST_LENGTH; + tmp_length -= DCERPC_AUTH_TRAILER_LENGTH; + tmp_length -= pkt->auth_length; + if (tmp_length < auth->auth_pad_length) { + DEBUG(1, (__location__ ": ERROR: pad length to large. " + "max %u got %u\n", + (unsigned)tmp_length, + (unsigned)auth->auth_pad_length)); + talloc_free(ndr); + ZERO_STRUCTP(auth); + return NT_STATUS_RPC_PROTOCOL_ERROR; + } + if (data_and_pad < auth->auth_pad_length) { DEBUG(1, (__location__ ": ERROR: pad length mismatch. " "Calculated %u got %u\n", -- 1.9.1 From 4813621296c196efc87e6981580ef972dcfac11d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2016 16:26:56 +0200 Subject: [PATCH 5/5] librpc/rpc: ignore invalid auth_pad_length values in BIND, ALTER and AUTH3 pdus This is a workarround for a bug in old Samba releases. For BIND_ACK <= 3.5.x and for ALTER_RESP <= 4.2.x (see bug #11061). BUG: https://bugzilla.samba.org/show_bug.cgi?id=11982 Signed-off-by: Stefan Metzmacher --- librpc/rpc/dcerpc_util.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index 768cb3d..349f72e 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -184,6 +184,22 @@ NTSTATUS dcerpc_pull_auth_trailer(const struct ncacn_packet *pkt, return NT_STATUS_RPC_PROTOCOL_ERROR; } + /* + * This is a workarround for a bug in old + * Samba releases. For BIND_ACK <= 3.5.x + * and for ALTER_RESP <= 4.2.x (see bug #11061) + * + * See also bug #11982. + */ + if (auth_data_only && data_and_pad == 0 && + data_and_pad < auth->auth_pad_length) { + /* + * we need to ignore invalid auth_pad_length + * values for BIND_* and ALTER_* pdus. + */ + auth->auth_pad_length = 0; + } + if (data_and_pad < auth->auth_pad_length) { DEBUG(1, (__location__ ": ERROR: pad length mismatch. " "Calculated %u got %u\n", -- 1.9.1