Patches for https://bugzilla.samba.org/show_bug.cgi?id=11061

Stefan (metze) Metzmacher metze at samba.org
Mon Jun 22 14:23:10 MDT 2015


Hi,

here're patches for https://bugzilla.samba.org/show_bug.cgi?id=11061

The problem is that the source3 rpc server uses 8 byte aligned padding
relative
to the pdu start, while windows uses 16 byte aligned padding relative to the
payload start. The heimdal gss_wrap() (called in
gensec_gssapi_seal_packet()) code assumes the windows behaviour when
working in dce_style mode. Otherwise is generated a too short signature
68 bytes in this cases instead of the expected 76 bytes returned by
gensec_gssapi_sig_size().

Please review and push.

metze
-------------- next part --------------
From f9f0db5163eb2c7142c1cd26a6024d898c3549e6 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 14:46:53 +0200
Subject: [PATCH 01/16] auth/gensec: gensec_[un]seal_packet() should only work
 with GENSEC_FEATURE_DCE_STYLE

gensec_sig_size() also requires GENSEC_FEATURE_DCE_STYLE if
GENSEC_FEATURE_SEAL is negotiated.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/gensec.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/auth/gensec/gensec.c b/auth/gensec/gensec.c
index d9504f7..9fd5f25 100644
--- a/auth/gensec/gensec.c
+++ b/auth/gensec/gensec.c
@@ -41,9 +41,15 @@ _PUBLIC_ NTSTATUS gensec_unseal_packet(struct gensec_security *gensec_security,
 	if (!gensec_security->ops->unseal_packet) {
 		return NT_STATUS_NOT_IMPLEMENTED;
 	}
+	if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_SIGN)) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
 	if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_SEAL)) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
+	if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_DCE_STYLE)) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
 
 	return gensec_security->ops->unseal_packet(gensec_security,
 						   data, length,
@@ -81,6 +87,9 @@ _PUBLIC_ NTSTATUS gensec_seal_packet(struct gensec_security *gensec_security,
 	if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_SIGN)) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
+	if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_DCE_STYLE)) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
 
 	return gensec_security->ops->seal_packet(gensec_security, mem_ctx, data, length, whole_pdu, pdu_length, sig);
 }
@@ -109,6 +118,11 @@ _PUBLIC_ size_t gensec_sig_size(struct gensec_security *gensec_security, size_t
 	if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_SIGN)) {
 		return 0;
 	}
+	if (gensec_have_feature(gensec_security, GENSEC_FEATURE_SEAL)) {
+		if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_DCE_STYLE)) {
+			return 0;
+		}
+	}
 
 	return gensec_security->ops->sig_size(gensec_security, data_size);
 }
-- 
1.9.1


From d17709fbfc9e5facca97b311842ca0e446b7f449 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 20 Jun 2015 16:19:31 +0200
Subject: [PATCH 02/16] auth/gensec: make sure gensec_start_mech_by_authtype()
 resets SIGN/SEAL before starting

We want to set GENSEC_FEATURE_SIGN and GENSEC_FEATURE_SEAL based on the given
auth_level and should not have GENSEC_FEATURE_SEAL if
DCERPC_AUTH_LEVEL_INTEGRITY is desired.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/gensec_start.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/auth/gensec/gensec_start.c b/auth/gensec/gensec_start.c
index 955cc36..be31697 100644
--- a/auth/gensec/gensec_start.c
+++ b/auth/gensec/gensec_start.c
@@ -724,6 +724,12 @@ _PUBLIC_ NTSTATUS gensec_start_mech_by_authtype(struct gensec_security *gensec_s
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 	gensec_security->dcerpc_auth_level = auth_level;
+	/*
+	 * We need to reset sign/seal in order to reset it.
+	 * We may got some default features inherited by the credentials
+	 */
+	gensec_security->want_features &= ~GENSEC_FEATURE_SIGN;
+	gensec_security->want_features &= ~GENSEC_FEATURE_SEAL;
 	gensec_want_feature(gensec_security, GENSEC_FEATURE_DCE_STYLE);
 	gensec_want_feature(gensec_security, GENSEC_FEATURE_ASYNC_REPLIES);
 	if (auth_level == DCERPC_AUTH_LEVEL_INTEGRITY) {
-- 
1.9.1


From f63385e582820fea3f7f034a4ea6b6a9498f9157 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 16:48:48 +0200
Subject: [PATCH 03/16] dcerpc.idl: add DCERPC_AUTH_PAD_ALIGNMENT (=16)

Windows pads the payload aligned to 16 bytes.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 librpc/idl/dcerpc.idl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/librpc/idl/dcerpc.idl b/librpc/idl/dcerpc.idl
index 4dad126..67f4b9d 100644
--- a/librpc/idl/dcerpc.idl
+++ b/librpc/idl/dcerpc.idl
@@ -259,6 +259,7 @@ interface dcerpc
 	} dcerpc_auth;
 
 	const uint8 DCERPC_AUTH_TRAILER_LENGTH = 8;
+	const uint8 DCERPC_AUTH_PAD_ALIGNMENT = 16;
 
 	typedef [public] struct {
 		[value(0)]	      uint32    _pad;
-- 
1.9.1


From 31c6cd05c4cb1e7a91cdffa0137801f33da37302 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 20 Jun 2015 17:43:47 +0200
Subject: [PATCH 04/16] librpc/rpc: add DCERPC_AUTH_PAD_LENGTH(stub_length)
 helper macro

This calculates the required padding DCERPC_AUTH_PAD_ALIGNMENT
and the stub_length.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 librpc/rpc/rpc_common.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/librpc/rpc/rpc_common.h b/librpc/rpc/rpc_common.h
index 1b54b80..61a8eab 100644
--- a/librpc/rpc/rpc_common.h
+++ b/librpc/rpc/rpc_common.h
@@ -372,4 +372,9 @@ bool dcerpc_sec_verification_trailer_check(
 		const struct dcerpc_sec_vt_pcontext *pcontext,
 		const struct dcerpc_sec_vt_header2 *header2);
 
+#define DCERPC_AUTH_PAD_LENGTH(stub_length) (\
+	(((stub_length) % DCERPC_AUTH_PAD_ALIGNMENT) > 0)?\
+	(DCERPC_AUTH_PAD_ALIGNMENT - (stub_length) % DCERPC_AUTH_PAD_ALIGNMENT):\
+	0)
+
 #endif /* __DEFAULT_LIBRPC_RPCCOMMON_H__ */
-- 
1.9.1


From 36d8fe7dab433ddb1383cb203c3dd4045bcad2cb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 16:55:39 +0200
Subject: [PATCH 05/16] s3:librpc/rpc: allow up to DCERPC_AUTH_PAD_ALIGNMENT
 padding bytes in dcerpc_add_auth_footer()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/librpc/rpc/dcerpc_helpers.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/librpc/rpc/dcerpc_helpers.c b/source3/librpc/rpc/dcerpc_helpers.c
index a9b24c8..5f2b94c 100644
--- a/source3/librpc/rpc/dcerpc_helpers.c
+++ b/source3/librpc/rpc/dcerpc_helpers.c
@@ -422,7 +422,7 @@ NTSTATUS dcerpc_add_auth_footer(struct pipe_auth_data *auth,
 				size_t pad_len, DATA_BLOB *rpc_out)
 {
 	struct gensec_security *gensec_security;
-	char pad[CLIENT_NDR_PADDING_SIZE] = { 0, };
+	const char pad[DCERPC_AUTH_PAD_ALIGNMENT] = { 0, };
 	DATA_BLOB auth_info;
 	DATA_BLOB auth_blob;
 	NTSTATUS status;
@@ -432,6 +432,8 @@ NTSTATUS dcerpc_add_auth_footer(struct pipe_auth_data *auth,
 	}
 
 	if (pad_len) {
+		SMB_ASSERT(pad_len <= ARRAY_SIZE(pad));
+
 		/* Copy the sign/seal padding data. */
 		if (!data_blob_append(NULL, rpc_out, pad, pad_len)) {
 			return NT_STATUS_NO_MEMORY;
-- 
1.9.1


From 3d7bfa867998561168b2d92dde0c33a64504c498 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 15:52:11 +0200
Subject: [PATCH 06/16] s3:librpc/rpc: fix padding calculation in
 dcerpc_guess_sizes()

The padding needs to be relative to the payload start not to the pdu start.
We also need align the padding to DCERPC_AUTH_PAD_ALIGNMENT (16 bytes).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/librpc/rpc/dcerpc.h         |  2 +-
 source3/librpc/rpc/dcerpc_helpers.c | 22 +++++++++-------------
 source3/rpc_client/cli_pipe.c       |  1 -
 source3/rpc_server/srv_pipe.c       |  1 -
 4 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/source3/librpc/rpc/dcerpc.h b/source3/librpc/rpc/dcerpc.h
index 42429a1..e7d66b7 100644
--- a/source3/librpc/rpc/dcerpc.h
+++ b/source3/librpc/rpc/dcerpc.h
@@ -75,7 +75,7 @@ NTSTATUS dcerpc_pull_dcerpc_auth(TALLOC_CTX *mem_ctx,
 				 bool bigendian);
 NTSTATUS dcerpc_guess_sizes(struct pipe_auth_data *auth,
 			    size_t header_len, size_t data_left,
-			    size_t max_xmit_frag, size_t pad_alignment,
+			    size_t max_xmit_frag,
 			    size_t *data_to_send, size_t *frag_len,
 			    size_t *auth_len, size_t *pad_len);
 NTSTATUS dcerpc_add_auth_footer(struct pipe_auth_data *auth,
diff --git a/source3/librpc/rpc/dcerpc_helpers.c b/source3/librpc/rpc/dcerpc_helpers.c
index 5f2b94c..1193baa 100644
--- a/source3/librpc/rpc/dcerpc_helpers.c
+++ b/source3/librpc/rpc/dcerpc_helpers.c
@@ -225,7 +225,6 @@ NTSTATUS dcerpc_pull_dcerpc_auth(TALLOC_CTX *mem_ctx,
 * @param header_len	The length of the packet header
 * @param data_left	The data left in the send buffer
 * @param max_xmit_frag	The max fragment size.
-* @param pad_alignment	The NDR padding size.
 * @param data_to_send	[out] The max data we will send in the pdu
 * @param frag_len	[out] The total length of the fragment
 * @param auth_len	[out] The length of the auth trailer
@@ -235,7 +234,7 @@ NTSTATUS dcerpc_pull_dcerpc_auth(TALLOC_CTX *mem_ctx,
 */
 NTSTATUS dcerpc_guess_sizes(struct pipe_auth_data *auth,
 			    size_t header_len, size_t data_left,
-			    size_t max_xmit_frag, size_t pad_alignment,
+			    size_t max_xmit_frag,
 			    size_t *data_to_send, size_t *frag_len,
 			    size_t *auth_len, size_t *pad_len)
 {
@@ -277,26 +276,23 @@ NTSTATUS dcerpc_guess_sizes(struct pipe_auth_data *auth,
 	case DCERPC_AUTH_TYPE_KRB5:
 	case DCERPC_AUTH_TYPE_SCHANNEL:
 		gensec_security = auth->auth_ctx;
-		*auth_len = gensec_sig_size(gensec_security, max_len);
+		mod_len = (max_len % DCERPC_AUTH_PAD_ALIGNMENT);
+		*auth_len = gensec_sig_size(gensec_security, max_len - mod_len);
+		if (*auth_len == 0) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
 		break;
 	default:
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
 	max_len -= *auth_len;
+	mod_len = (max_len % DCERPC_AUTH_PAD_ALIGNMENT);
+	max_len -= mod_len;
 
 	*data_to_send = MIN(max_len, data_left);
 
-	mod_len = (header_len + *data_to_send) % pad_alignment;
-	if (mod_len) {
-		*pad_len = pad_alignment - mod_len;
-	} else {
-		*pad_len = 0;
-	}
-
-	if (*data_to_send + *pad_len > max_len) {
-		*data_to_send -= pad_alignment;
-	}
+	*pad_len = DCERPC_AUTH_PAD_LENGTH(*data_to_send);
 
 	*frag_len = header_len + *data_to_send + *pad_len
 			+ DCERPC_AUTH_TRAILER_LENGTH + *auth_len;
diff --git a/source3/rpc_client/cli_pipe.c b/source3/rpc_client/cli_pipe.c
index d0fb774..f642d30 100644
--- a/source3/rpc_client/cli_pipe.c
+++ b/source3/rpc_client/cli_pipe.c
@@ -1398,7 +1398,6 @@ static NTSTATUS prepare_next_frag(struct rpc_api_pipe_req_state *state,
 	status = dcerpc_guess_sizes(state->cli->auth,
 				    DCERPC_REQUEST_LENGTH, total_left,
 				    state->cli->max_xmit_frag,
-				    CLIENT_NDR_PADDING_SIZE,
 				    &total_thistime,
 				    &frag_len, &auth_len, &pad_len);
 	if (!NT_STATUS_IS_OK(status)) {
diff --git a/source3/rpc_server/srv_pipe.c b/source3/rpc_server/srv_pipe.c
index 63323f8..77592a4 100644
--- a/source3/rpc_server/srv_pipe.c
+++ b/source3/rpc_server/srv_pipe.c
@@ -143,7 +143,6 @@ static NTSTATUS create_next_packet(TALLOC_CTX *mem_ctx,
 				    DCERPC_RESPONSE_LENGTH,
 				    data_left,
 				    RPC_MAX_PDU_FRAG_LEN,
-				    SERVER_NDR_PADDING_SIZE,
 				    &data_to_send, &frag_len,
 				    &auth_len, &pad_len);
 	if (!NT_STATUS_IS_OK(status)) {
-- 
1.9.1


From 54c82d4c80e79c889808c7c3b7ead441441c1dae Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 22:09:57 +0200
Subject: [PATCH 07/16] s3:rpc_server: remove pad handling from
 api_pipe_alter_context()

This is not needed and windows doesn't use it.
The padding is for the payload in request and response.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/rpc_server/srv_pipe.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/source3/rpc_server/srv_pipe.c b/source3/rpc_server/srv_pipe.c
index 77592a4..4ffaa0d 100644
--- a/source3/rpc_server/srv_pipe.c
+++ b/source3/rpc_server/srv_pipe.c
@@ -943,7 +943,6 @@ static bool api_pipe_alter_context(struct pipes_struct *p,
 	struct dcerpc_ack_ctx bind_ack_ctx;
 	DATA_BLOB auth_resp = data_blob_null;
 	DATA_BLOB auth_blob = data_blob_null;
-	int pad_len = 0;
 	struct gensec_security *gensec_security;
 
 	DEBUG(5,("api_pipe_alter_context: make response. %d\n", __LINE__));
@@ -1080,19 +1079,10 @@ static bool api_pipe_alter_context(struct pipes_struct *p,
 	}
 
 	if (auth_resp.length) {
-
-		/* Work out any padding needed before the auth footer. */
-		pad_len = p->out_data.frag.length % SERVER_NDR_PADDING_SIZE;
-		if (pad_len) {
-			pad_len = SERVER_NDR_PADDING_SIZE - pad_len;
-			DEBUG(10, ("auth pad_len = %u\n",
-				   (unsigned int)pad_len));
-		}
-
 		status = dcerpc_push_dcerpc_auth(pkt,
 						 auth_info.auth_type,
 						 auth_info.auth_level,
-						 pad_len,
+						 0, /* pad_len */
 						 1, /* auth_context_id */
 						 &auth_resp,
 						 &auth_blob);
@@ -1106,22 +1096,9 @@ static bool api_pipe_alter_context(struct pipes_struct *p,
 	 * the dcerpc header */
 	dcerpc_set_frag_length(&p->out_data.frag,
 				p->out_data.frag.length +
-					pad_len + auth_blob.length);
+				auth_blob.length);
 
 	if (auth_resp.length) {
-		if (pad_len) {
-			char pad[SERVER_NDR_PADDING_SIZE];
-			memset(pad, '\0', SERVER_NDR_PADDING_SIZE);
-			if (!data_blob_append(p->mem_ctx,
-						&p->out_data.frag,
-						pad, pad_len)) {
-				DEBUG(0, ("api_pipe_bind_req: failed to add "
-					  "%u bytes of pad data.\n",
-					  (unsigned int)pad_len));
-				goto err_exit;
-			}
-		}
-
 		if (!data_blob_append(p->mem_ctx, &p->out_data.frag,
 					auth_blob.data, auth_blob.length)) {
 			DEBUG(0, ("Append of auth info failed.\n"));
-- 
1.9.1


From 791127302e651622849d2f90212acc317747ad69 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 22:23:01 +0200
Subject: [PATCH 08/16] s3:include: remove used unused
 {CLIENT,SERVER}_NDR_PADDING_SIZE

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/include/local.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/source3/include/local.h b/source3/include/local.h
index 85f0861..5963eb0 100644
--- a/source3/include/local.h
+++ b/source3/include/local.h
@@ -204,7 +204,4 @@
 /* Maximum size of RPC data we will accept for one call. */
 #define MAX_RPC_DATA_SIZE (15*1024*1024)
 
-#define CLIENT_NDR_PADDING_SIZE 8
-#define SERVER_NDR_PADDING_SIZE 8
-
 #endif
-- 
1.9.1


From ce09866db5645826b28ad8d31b23176a52238165 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 22:35:44 +0200
Subject: [PATCH 09/16] s4:librpc/rpc: let dcerpc_ship_next_request() use
 DCERPC_AUTH_PAD_ALIGNMENT define

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/librpc/rpc/dcerpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index be9a44c..3f8189d 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -1688,7 +1688,7 @@ static void dcerpc_ship_next_request(struct dcecli_connection *c)
 			chunk_size -= sig_size;
 		}
 	}
-	chunk_size -= (chunk_size % 16);
+	chunk_size -= (chunk_size % DCERPC_AUTH_PAD_ALIGNMENT);
 
 	pkt.ptype = DCERPC_PKT_REQUEST;
 	pkt.call_id = req->call_id;
-- 
1.9.1


From 7337bf1b73c8d31b9f52f14755a560e2ba82a03a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 22:35:44 +0200
Subject: [PATCH 10/16] s4:librpc/rpc: let dcerpc_ship_next_request() use a
 sig_size for a padded payload

The sig_size could differ depending on the aligment/padding.
So should use the same alignment as we use for the payload.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/librpc/rpc/dcerpc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index 3f8189d..7194074 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -1681,8 +1681,13 @@ static void dcerpc_ship_next_request(struct dcecli_connection *c)
 	chunk_size -= DCERPC_REQUEST_LENGTH;
 	if (c->security_state.auth_info &&
 	    c->security_state.generic_state) {
+		size_t max_payload = chunk_size;
+
+		max_payload -= DCERPC_AUTH_TRAILER_LENGTH;
+		max_payload -= (max_payload % DCERPC_AUTH_PAD_ALIGNMENT);
+
 		sig_size = gensec_sig_size(c->security_state.generic_state,
-					   p->conn->srv_max_recv_frag);
+					   max_payload);
 		if (sig_size) {
 			chunk_size -= DCERPC_AUTH_TRAILER_LENGTH;
 			chunk_size -= sig_size;
-- 
1.9.1


From a95f9be6a6521a1738b5fee96c920e10a7e55baa Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 20 Jun 2015 17:47:14 +0200
Subject: [PATCH 11/16] s4:librpc/rpc: let ncacn_push_request_sign() handle
 sig_size == 0 with auth_info as internal error

Don't send plaintext on the wire because of an internal error...

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/librpc/rpc/dcerpc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index 7194074..f1c6d47 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -832,13 +832,16 @@ static NTSTATUS ncacn_push_request_sign(struct dcecli_connection *c,
 	size_t hdr_size = DCERPC_REQUEST_LENGTH;
 
 	/* non-signed packets are simpler */
-	if (sig_size == 0) {
+	if (c->security_state.auth_info == NULL) {
 		return ncacn_push_auth(blob, mem_ctx, pkt, NULL);
 	}
 
 	switch (c->security_state.auth_info->auth_level) {
 	case DCERPC_AUTH_LEVEL_PRIVACY:
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
+		if (sig_size == 0) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
 		break;
 
 	case DCERPC_AUTH_LEVEL_CONNECT:
-- 
1.9.1


From a673fce3c94d8d0613cfb2701f4fbb7069244e37 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 20 Jun 2015 17:49:02 +0200
Subject: [PATCH 12/16] s4:librpc/rpc: fix padding caclucation in
 ncacn_push_request_sign()

This is simplified by using DCERPC_AUTH_PAD_LENGTH() and changes the behaviour
so that we will use no padding if the stub_length is already aligned
to DCERPC_AUTH_PAD_ALIGNMENT (16 bytes).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/librpc/rpc/dcerpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index f1c6d47..6e3410b 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -884,7 +884,7 @@ static NTSTATUS ncacn_push_request_sign(struct dcecli_connection *c,
 	   whole packet, whereas w2k8 wants it relative to the start
 	   of the stub */
 	c->security_state.auth_info->auth_pad_length =
-		(16 - (pkt->u.request.stub_and_verifier.length & 15)) & 15;
+		DCERPC_AUTH_PAD_LENGTH(pkt->u.request.stub_and_verifier.length);
 	ndr_err = ndr_push_zero(ndr, c->security_state.auth_info->auth_pad_length);
 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 		return ndr_map_error2ntstatus(ndr_err);
-- 
1.9.1


From 534e19efe082eb4be1de18603472a6028c62c0f3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 22:35:44 +0200
Subject: [PATCH 13/16] s4:rpc_server: let dcesrv_reply() use
 DCERPC_AUTH_PAD_ALIGNMENT define

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/rpc_server/common/reply.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/rpc_server/common/reply.c b/source4/rpc_server/common/reply.c
index 92bd552..42830ef 100644
--- a/source4/rpc_server/common/reply.c
+++ b/source4/rpc_server/common/reply.c
@@ -194,7 +194,7 @@ _PUBLIC_ NTSTATUS dcesrv_reply(struct dcesrv_call_state *call)
 			chunk_size -= sig_size;
 		}
 	}
-	chunk_size -= (chunk_size % 16);
+	chunk_size -= (chunk_size % DCERPC_AUTH_PAD_ALIGNMENT);
 
 	do {
 		uint32_t length;
-- 
1.9.1


From d81043cb79f9c43a217da3120485ccb7339f5a82 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 19 Jun 2015 22:35:44 +0200
Subject: [PATCH 14/16] s4:rpc_server: let dcesrv_reply() use a sig_size for a
 padded payload

The sig_size could differ depending on the aligment/padding.
So should use the same alignment as we use for the payload.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/rpc_server/common/reply.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/source4/rpc_server/common/reply.c b/source4/rpc_server/common/reply.c
index 42830ef..007b680 100644
--- a/source4/rpc_server/common/reply.c
+++ b/source4/rpc_server/common/reply.c
@@ -187,8 +187,13 @@ _PUBLIC_ NTSTATUS dcesrv_reply(struct dcesrv_call_state *call)
 	chunk_size -= DCERPC_REQUEST_LENGTH;
 	if (call->conn->auth_state.auth_info &&
 	    call->conn->auth_state.gensec_security) {
+		size_t max_payload = chunk_size;
+
+		max_payload -= DCERPC_AUTH_TRAILER_LENGTH;
+		max_payload -= (max_payload % DCERPC_AUTH_PAD_ALIGNMENT);
+
 		sig_size = gensec_sig_size(call->conn->auth_state.gensec_security,
-					   call->conn->cli_max_recv_frag);
+					   max_payload);
 		if (sig_size) {
 			chunk_size -= DCERPC_AUTH_TRAILER_LENGTH;
 			chunk_size -= sig_size;
-- 
1.9.1


From 33fe0ba2c66f0bc9026d333f904d93b60b77adf3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 20 Jun 2015 17:47:14 +0200
Subject: [PATCH 15/16] s4:rpc_server: let dcesrv_auth_response() handle
 sig_size == 0 with auth_info as error

Don't send plaintext on the wire because of an internal error...

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/rpc_server/dcesrv_auth.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c
index d5aef49..e8b950d 100644
--- a/source4/rpc_server/dcesrv_auth.c
+++ b/source4/rpc_server/dcesrv_auth.c
@@ -442,7 +442,7 @@ bool dcesrv_auth_response(struct dcesrv_call_state *call,
 	DATA_BLOB creds2;
 
 	/* non-signed packets are simple */
-	if (sig_size == 0) {
+	if (dce_conn->auth_state.auth_info == NULL) {
 		status = ncacn_push_auth(blob, call, pkt, NULL);
 		return NT_STATUS_IS_OK(status);
 	}
@@ -450,6 +450,10 @@ bool dcesrv_auth_response(struct dcesrv_call_state *call,
 	switch (dce_conn->auth_state.auth_info->auth_level) {
 	case DCERPC_AUTH_LEVEL_PRIVACY:
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
+		if (sig_size == 0) {
+			return false;
+		}
+
 		break;
 
 	case DCERPC_AUTH_LEVEL_CONNECT:
-- 
1.9.1


From f674330e988e140900e813c51c4283d20f7b8e81 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 20 Jun 2015 17:49:02 +0200
Subject: [PATCH 16/16] s4:rpc_server: fix padding caclucation in
 dcesrv_auth_response()

This is simplified by using DCERPC_AUTH_PAD_LENGTH() and changes the behaviour
so that we will use no padding if the stub_length is already aligned
to DCERPC_AUTH_PAD_ALIGNMENT (16 bytes).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11061

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/rpc_server/dcesrv_auth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c
index e8b950d..374c2e0 100644
--- a/source4/rpc_server/dcesrv_auth.c
+++ b/source4/rpc_server/dcesrv_auth.c
@@ -492,7 +492,7 @@ bool dcesrv_auth_response(struct dcesrv_call_state *call,
 	   whole packet, whereas w2k8 wants it relative to the start
 	   of the stub */
 	dce_conn->auth_state.auth_info->auth_pad_length =
-		(16 - (pkt->u.response.stub_and_verifier.length & 15)) & 15;
+		DCERPC_AUTH_PAD_LENGTH(pkt->u.response.stub_and_verifier.length);
 	ndr_err = ndr_push_zero(ndr,
 				dce_conn->auth_state.auth_info->auth_pad_length);
 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-- 
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150622/b2e47e6f/attachment.pgp>


More information about the samba-technical mailing list