The Samba-Bugzilla – Attachment 12213 Details for
Bug 11982
Invalid auth_pad_length is not ignored for BIND_* and ALTER_* pdus
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-2-test
tmp42.diff.txt (text/plain), 13.66 KB, created by
Stefan Metzmacher
on 2016-06-26 19:32:54 UTC
(
hide
)
Description:
Patches for v4-2-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2016-06-26 19:32:54 UTC
Size:
13.66 KB
patch
obsolete
>From 9be17c09b2d33e17920ff14f52227008e308e883 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 22 Jun 2016 20:38:01 +0200 >Subject: [PATCH 1/7] dcerpc.idl: remove unused DCERPC_NCACN_PAYLOAD_MAX_SIZE > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11948 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit d9e242e9035c15e49b041afc61e5a4a08877f289) >--- > librpc/idl/dcerpc.idl | 1 - > 1 file changed, 1 deletion(-) > >diff --git a/librpc/idl/dcerpc.idl b/librpc/idl/dcerpc.idl >index 5e0f919..527804d 100644 >--- a/librpc/idl/dcerpc.idl >+++ b/librpc/idl/dcerpc.idl >@@ -535,7 +535,6 @@ interface dcerpc > const uint32 DCERPC_FRAG_MAX_SIZE = 5840; > const uint8 DCERPC_AUTH_LEN_OFFSET = 10; > const uint8 DCERPC_NCACN_PAYLOAD_OFFSET = 16; >- const uint32 DCERPC_NCACN_PAYLOAD_MAX_SIZE = 0x400000; /* 4 MByte */ > > /* > * See [MS-RPCE] 3.3.3.5.4 Maximum Server Input Data Size >-- >1.9.1 > > >From 41086e500805286e40b3e7d2f4fa79e5aa14f55a Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 20 Jun 2016 16:11:37 +0200 >Subject: [PATCH 2/7] 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> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 505a4e68d96e6fb3d8c7493632ecb4b0fc6caa9d) >--- > 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 c96d90f050e5e6c86dbecd5fb1c84f1a93d4df2f Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 20 Jun 2016 16:16:23 +0200 >Subject: [PATCH 3/7] 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> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit e05c732c6074df2524403ad7bb30eade91443525) >--- > 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 4225e1d..de3f425 100644 >--- a/source4/librpc/rpc/dcerpc.c >+++ b/source4/librpc/rpc/dcerpc.c >@@ -1413,12 +1413,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; > } >@@ -2433,12 +2431,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 d80c2865b642d9e71145d87b90841151e9373bbe Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 20 Jun 2016 16:17:45 +0200 >Subject: [PATCH 4/7] 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> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit f386e81b982cd551313eb9c0f7d2f70d65515d80) >--- > 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 86808c28883a67e192e864c5756f4a93c794d570 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 20 Jun 2016 16:25:12 +0200 >Subject: [PATCH 5/7] 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> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 3f7e3ed8a276f16aaed87c1f3cd5b9781aa7e1af) >--- > librpc/rpc/dcerpc_util.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > >diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c >index 4d82e9a5..ee7b307 100644 >--- a/librpc/rpc/dcerpc_util.c >+++ b/librpc/rpc/dcerpc_util.c >@@ -95,6 +95,7 @@ NTSTATUS dcerpc_pull_auth_trailer(const struct ncacn_packet *pkt, > uint16_t data_and_pad; > uint16_t auth_length; > uint32_t tmp_length; >+ uint32_t max_pad_len = 0; > > ZERO_STRUCTP(auth); > if (_auth_length != NULL) { >@@ -157,6 +158,42 @@ 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_*, ALTER_* and AUTH3 pdus. >+ * >+ * We need this check before we ignore possible >+ * invalid values. See also bug #11982. >+ * >+ * This check is mainly used to generate the correct >+ * error for BIND_*, ALTER_* and AUTH3 pdus. >+ * >+ * We always have the 'if (data_and_pad < auth->auth_pad_length)' >+ * protection for REQUEST and RESPONSE pdus, where the >+ * auth_pad_length field is actually used by the caller. >+ */ >+ tmp_length = DCERPC_REQUEST_LENGTH; >+ tmp_length += DCERPC_AUTH_TRAILER_LENGTH; >+ tmp_length += pkt->auth_length; >+ if (tmp_length < pkt->frag_length) { >+ max_pad_len = pkt->frag_length - tmp_length; >+ } >+ if (max_pad_len < auth->auth_pad_length) { >+ DEBUG(1, (__location__ ": ERROR: pad length to large. " >+ "max %u got %u\n", >+ (unsigned)max_pad_len, >+ (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 332347446cddef190be9a07897fb1ad70b878eb5 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 20 Jun 2016 16:26:56 +0200 >Subject: [PATCH 6/7] 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> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit aef032302863e5f3a888dbf4c52b21d561a0dff4) >--- > 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 ee7b307..df14948 100644 >--- a/librpc/rpc/dcerpc_util.c >+++ b/librpc/rpc/dcerpc_util.c >@@ -194,6 +194,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 && >+ auth->auth_pad_length > 0) { >+ /* >+ * we need to ignore invalid auth_pad_length >+ * values for BIND_*, ALTER_* and AUTH3 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 > > >From 736877a120203bcf8f7b5e553c7b1bb6b9f36b47 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 23 Jun 2016 13:50:39 +0200 >Subject: [PATCH 7/7] s4:rpc_server: generate the correct error when we got an > invalid auth_pad_length on BIND,ALTER,AUTH3 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11982 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 7d8edcc24148658e92729b3d155e432994e27525) >--- > source4/rpc_server/dcerpc_server.c | 13 ++++++++++--- > source4/rpc_server/dcesrv_auth.c | 18 ++++++++++++++++++ > 2 files changed, 28 insertions(+), 3 deletions(-) > >diff --git a/source4/rpc_server/dcerpc_server.c b/source4/rpc_server/dcerpc_server.c >index 8439d84..c6b992e 100644 >--- a/source4/rpc_server/dcerpc_server.c >+++ b/source4/rpc_server/dcerpc_server.c >@@ -804,6 +804,11 @@ static NTSTATUS dcesrv_bind(struct dcesrv_call_state *call) > > TALLOC_FREE(call->context); > >+ if (call->fault_code == DCERPC_NCA_S_PROTO_ERROR) { >+ return dcesrv_bind_nak(call, >+ DCERPC_BIND_NAK_REASON_PROTOCOL_VERSION_NOT_SUPPORTED); >+ } >+ > if (auth->auth_level != DCERPC_AUTH_LEVEL_NONE) { > /* > * We only give INVALID_AUTH_TYPE if the auth_level was >@@ -936,6 +941,9 @@ static NTSTATUS dcesrv_auth3(struct dcesrv_call_state *call) > /* handle the auth3 in the auth code */ > if (!dcesrv_auth_auth3(call)) { > call->conn->auth_state.auth_invalid = true; >+ if (call->fault_code != 0) { >+ return dcesrv_fault_disconnect(call, call->fault_code); >+ } > } > > talloc_free(call); >@@ -1105,9 +1113,8 @@ static NTSTATUS dcesrv_alter(struct dcesrv_call_state *call) > > auth_ok = dcesrv_auth_alter(call); > if (!auth_ok) { >- if (call->in_auth_info.auth_type == DCERPC_AUTH_TYPE_NONE) { >- return dcesrv_fault_disconnect(call, >- DCERPC_FAULT_ACCESS_DENIED); >+ if (call->fault_code != 0) { >+ return dcesrv_fault_disconnect(call, call->fault_code); > } > } > >diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c >index 802876b..74a62df 100644 >--- a/source4/rpc_server/dcesrv_auth.c >+++ b/source4/rpc_server/dcesrv_auth.c >@@ -56,6 +56,12 @@ bool dcesrv_auth_bind(struct dcesrv_call_state *call) > &call->in_auth_info, > NULL, true); > if (!NT_STATUS_IS_OK(status)) { >+ /* >+ * This will cause a >+ * DCERPC_BIND_NAK_REASON_PROTOCOL_VERSION_NOT_SUPPORTED >+ * in the caller >+ */ >+ call->fault_code = DCERPC_NCA_S_PROTO_ERROR; > return false; > } > >@@ -257,6 +263,11 @@ 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, NULL, true); > if (!NT_STATUS_IS_OK(status)) { >+ /* >+ * Windows returns DCERPC_NCA_S_FAULT_REMOTE_NO_MEMORY >+ * instead of DCERPC_NCA_S_PROTO_ERROR. >+ */ >+ call->fault_code = DCERPC_NCA_S_FAULT_REMOTE_NO_MEMORY; > return false; > } > >@@ -332,6 +343,7 @@ bool dcesrv_auth_alter(struct dcesrv_call_state *call) > } > > if (dce_conn->auth_state.auth_finished) { >+ call->fault_code = DCERPC_FAULT_ACCESS_DENIED; > return false; > } > >@@ -343,6 +355,12 @@ 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, NULL, true); > if (!NT_STATUS_IS_OK(status)) { >+ call->fault_code = DCERPC_NCA_S_PROTO_ERROR; >+ return false; >+ } >+ >+ if (call->in_auth_info.auth_type == DCERPC_AUTH_TYPE_NONE) { >+ call->fault_code = DCERPC_FAULT_ACCESS_DENIED; > return false; > } > >-- >1.9.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=12213">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
gd
:
review+
Actions:
View
Attachments on
bug 11982
:
12192
|
12209
|
12211
|
12213
|
12308