Attachment 12192 Details for Bug 11982 – Possible patches for master

[patch] Possible patches for master

tmp.diff.txt (text/plain), 8.09 KB, created by Stefan Metzmacher on 2016-06-20 20:28:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2016-06-20 20:28:34 UTC

Size: 8.09 KB

patch

obsolete

>From 5d2c73d5fb081c71cb09ef9780029d2b71404683 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>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 <metze@samba.org>
>---
> 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 <metze@samba.org>
>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 <metze@samba.org>
>---
> 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 <metze@samba.org>
>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 <metze@samba.org>
>---
> 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 <metze@samba.org>
>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 <metze@samba.org>
>---
> 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 <metze@samba.org>
>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 <metze@samba.org>
>---
> 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
>

Flags:

gd: review+

Actions: View

Attachments on bug 11982: 12192 | 12209 | 12211 | 12213 | 12308