Possible patches to fix rpc client connections against older samba versions
Stefan Metzmacher
metze at samba.org
Mon Jun 20 20:27:09 UTC 2016
Hi,
here're possible fixes for https://bugzilla.samba.org/show_bug.cgi?id=11982
The problem is that e.g. Samba 3.5 uses
init_rpc_hdr_auth(&auth_info, DCERPC_AUTH_TYPE_SCHANNEL,
pauth_info->auth_level, RPC_HDR_AUTH_LEN, 1);
That means we always set auth_pad_length to 8 (RPC_HDR_AUTH_LEN), which
is wrong
for BIND_ACK and ALTER_RESP pdus.
The fixes a regression introduced in the following commit:
commit 8e19ce76dafef5ed00ad406b95bb739950063e14
Author: Stefan Metzmacher <metze at samba.org>
AuthorDate: Sun Jun 28 01:19:57 2015 +0200
Commit: Stefan Metzmacher <metze at samba.org>
CommitDate: Tue Apr 12 19:25:28 2016 +0200
CVE-2015-5370: librpc/rpc: simplify and harden
dcerpc_pull_auth_trailer()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11344
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Günther Deschner <gd at samba.org>
This pases autobuild and a hacked test I had, but I still need to
cleanup the test and do a manual test.
But you can start to review the attached patches.
Thanks!
metze
-------------- next part --------------
From 5d2c73d5fb081c71cb09ef9780029d2b71404683 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160620/1c488afd/signature.sig>
More information about the samba-technical
mailing list