s4-rpc: be more careful about DCERPC auth padding

Cope with a wider range of auth padding in dcerpc bind_ack and
alter_context packets. We now use a helper function that calculates
the right auth padding.

3 files changed, 107 insertions, 107 deletions

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index c006693bab..e7b181c50b 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -223,11 +223,9 @@ static NTSTATUS ncacn_pull_request_auth(struct dcerpc_connection *c, TALLOC_CTX
 					DATA_BLOB *raw_packet,
 					struct ncacn_packet *pkt)
 {
-	struct ndr_pull *ndr;
 	NTSTATUS status;
 	struct dcerpc_auth auth;
-	DATA_BLOB auth_blob;
-	enum ndr_err_code ndr_err;
+	uint32_t auth_length;
 
 	if (!c->security_state.auth_info ||
 	    !c->security_state.generic_state) {
@@ -254,33 +252,12 @@ static NTSTATUS ncacn_pull_request_auth(struct dcerpc_connection *c, TALLOC_CTX
 		return NT_STATUS_INVALID_LEVEL;
 	}
 
-	auth_blob.length = 8 + pkt->auth_length;
-
-	/* check for a valid length */
-	if (pkt->u.response.stub_and_verifier.length < auth_blob.length) {
-		return NT_STATUS_INFO_LENGTH_MISMATCH;
-	}
-
-	auth_blob.data = 
-		pkt->u.response.stub_and_verifier.data + 
-		pkt->u.response.stub_and_verifier.length - auth_blob.length;
-	pkt->u.response.stub_and_verifier.length -= auth_blob.length;
+	status = dcerpc_pull_auth_trailer(pkt, mem_ctx,
+					  &pkt->u.response.stub_and_verifier,
+					  &auth, &auth_length, false);
+	NT_STATUS_NOT_OK_RETURN(status);
 
-	/* pull the auth structure */
-	ndr = ndr_pull_init_flags(c, &auth_blob, mem_ctx);
-	if (!ndr) {
-		return NT_STATUS_NO_MEMORY;
-	}
-
-	if (!(pkt->drep[0] & DCERPC_DREP_LE)) {
-		ndr->flags |= LIBNDR_FLAG_BIGENDIAN;
-	}
-
-	ndr_err = ndr_pull_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, &auth);
-	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-		return ndr_map_error2ntstatus(ndr_err);
-	}
-	status = NT_STATUS_OK;
+	pkt->u.response.stub_and_verifier.length -= auth_length;
 
 	/* check signature or unseal the packet */
 	switch (c->security_state.auth_info->auth_level) {
@@ -317,7 +294,7 @@ static NTSTATUS ncacn_pull_request_auth(struct dcerpc_connection *c, TALLOC_CTX
 		break;
 	}
 	
-	/* remove the indicated amount of paddiing */
+	/* remove the indicated amount of padding */
 	if (pkt->u.response.stub_and_verifier.length < auth.auth_pad_length) {
 		return NT_STATUS_INFO_LENGTH_MISMATCH;
 	}
@@ -600,7 +577,6 @@ static void dcerpc_recv_data(struct dcerpc_connection *conn, DATA_BLOB *blob, NT
 	dcerpc_request_recv_data(conn, blob, &pkt);
 }
 
-
 /*
   Receive a bind reply from the transport
 */
@@ -643,17 +619,14 @@ static void dcerpc_bind_recv_handler(struct rpc_request *req,
 	}
 
 	/* the bind_ack might contain a reply set of credentials */
-	if (conn->security_state.auth_info &&
-	    pkt->u.bind_ack.auth_info.length) {
-		enum ndr_err_code ndr_err;
-		ndr_err = ndr_pull_struct_blob(
-			&pkt->u.bind_ack.auth_info, conn,
-			NULL,
-			conn->security_state.auth_info,
-			(ndr_pull_flags_fn_t)ndr_pull_dcerpc_auth);
-		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-			c->status = ndr_map_error2ntstatus(ndr_err);
-			if (!composite_is_ok(c)) return;
+	if (conn->security_state.auth_info && pkt->u.bind_ack.auth_info.length) {
+		NTSTATUS status;
+		uint32_t auth_length;
+		status = dcerpc_pull_auth_trailer(pkt, conn, &pkt->u.bind_ack.auth_info,
+						  conn->security_state.auth_info, &auth_length, true);
+		if (!NT_STATUS_IS_OK(status)) {
+			composite_error(c, status);
+			return;
 		}
 	}
 
@@ -791,7 +764,6 @@ NTSTATUS dcerpc_auth3(struct dcerpc_pipe *p,
 	pkt.pfc_flags = DCERPC_PFC_FLAG_FIRST | DCERPC_PFC_FLAG_LAST;
 	pkt.call_id = next_call_id(p->conn);
 	pkt.auth_length = 0;
-	pkt.u.auth3._pad = 0;
 	pkt.u.auth3.auth_info = data_blob(NULL, 0);
 
 	if (p->binding->flags & DCERPC_CONCURRENT_MULTIPLEX) {
@@ -1610,15 +1582,14 @@ static void dcerpc_alter_recv_handler(struct rpc_request *req,
 	/* the alter_resp might contain a reply set of credentials */
 	if (recv_pipe->conn->security_state.auth_info &&
 	    pkt->u.alter_resp.auth_info.length) {
-		enum ndr_err_code ndr_err;
-		ndr_err = ndr_pull_struct_blob(
-			&pkt->u.alter_resp.auth_info, recv_pipe,
-			NULL,
-			recv_pipe->conn->security_state.auth_info,
-			(ndr_pull_flags_fn_t)ndr_pull_dcerpc_auth);
-		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-			c->status = ndr_map_error2ntstatus(ndr_err);
-			if (!composite_is_ok(c)) return;
+		struct dcerpc_connection *conn = recv_pipe->conn;
+		NTSTATUS status;
+		uint32_t auth_length;
+		status = dcerpc_pull_auth_trailer(pkt, conn, &pkt->u.alter_resp.auth_info,
+						  conn->security_state.auth_info, &auth_length, true);
+		if (!NT_STATUS_IS_OK(status)) {
+			composite_error(c, status);
+			return;
 		}
 	}
 
diff --git a/source4/librpc/rpc/dcerpc_util.c b/source4/librpc/rpc/dcerpc_util.c
index 86c91535e7..1985cb9e18 100644
--- a/source4/librpc/rpc/dcerpc_util.c
+++ b/source4/librpc/rpc/dcerpc_util.c
@@ -83,6 +83,10 @@ NTSTATUS ncacn_push_auth(DATA_BLOB *blob, TALLOC_CTX *mem_ctx,
 	}
 
 	if (auth_info) {
+		ndr_err = ndr_push_zero(ndr, auth_info->auth_pad_length);
+		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+			return ndr_map_error2ntstatus(ndr_err);
+		}
 		ndr_err = ndr_push_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, auth_info);
 		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 			return ndr_map_error2ntstatus(ndr_err);
@@ -750,3 +754,59 @@ _PUBLIC_ NTSTATUS dcerpc_secondary_context(struct dcerpc_pipe *p,
 
 	return NT_STATUS_OK;
 }
+
+
+/*
+  pull an dcerpc_auth structure, taking account of any auth padding in
+  the blob at the end of the structure
+ */
+NTSTATUS dcerpc_pull_auth_trailer(struct ncacn_packet *pkt,
+				  TALLOC_CTX *mem_ctx,
+				  DATA_BLOB *pkt_auth_blob,
+				  struct dcerpc_auth *auth,
+				  uint32_t *auth_length,
+				  bool check_pad)
+{
+	struct ndr_pull *ndr;
+	enum ndr_err_code ndr_err;
+	uint32_t pad;
+
+	pad = pkt_auth_blob->length - (DCERPC_AUTH_TRAILER_LENGTH + pkt->auth_length);
+	*auth_length = pkt_auth_blob->length - pad;
+
+	ndr = ndr_pull_init_blob(pkt_auth_blob, mem_ctx, NULL);
+	if (!ndr) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	if (!(pkt->drep[0] & DCERPC_DREP_LE)) {
+		ndr->flags |= LIBNDR_FLAG_BIGENDIAN;
+	}
+
+	ndr_err = ndr_pull_advance(ndr, pad);
+	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+		talloc_free(ndr);
+		return ndr_map_error2ntstatus(ndr_err);
+	}
+
+	ndr_err = ndr_pull_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, auth);
+	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+		talloc_free(ndr);
+		return ndr_map_error2ntstatus(ndr_err);
+	}
+
+	if (check_pad && pad != auth->auth_pad_length) {
+		DEBUG(1,(__location__ ": WARNING: pad length mismatch. Calculated %u  got %u\n",
+			 (unsigned)pad, (unsigned)auth->auth_pad_length));
+	}
+
+	DEBUG(6,(__location__ ": auth_pad_length %u\n",
+		 (unsigned)auth->auth_pad_length));
+
+	talloc_steal(mem_ctx, auth->credentials.data);
+	talloc_free(ndr);
+
+	return NT_STATUS_OK;
+}
+
+
diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c
index 3499ca2187..d56dd12ac5 100644
--- a/source4/rpc_server/dcesrv_auth.c
+++ b/source4/rpc_server/dcesrv_auth.c
@@ -42,7 +42,7 @@ 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;
-	enum ndr_err_code ndr_err;
+	uint32_t auth_length;
 
 	if (pkt->u.bind.auth_info.length == 0) {
 		dce_conn->auth_state.auth_info = NULL;
@@ -54,14 +54,9 @@ bool dcesrv_auth_bind(struct dcesrv_call_state *call)
 		return false;
 	}
 
-	ndr_err = ndr_pull_struct_blob(&pkt->u.bind.auth_info,
-				       call, NULL,
-				       dce_conn->auth_state.auth_info,
-				       (ndr_pull_flags_fn_t)ndr_pull_dcerpc_auth);
-	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-		return false;
-	}
-
+	status = dcerpc_pull_auth_trailer(pkt, call, &pkt->u.bind.auth_info,
+					  dce_conn->auth_state.auth_info,
+					  &auth_length, false);
 	server_credentials 
 		= cli_credentials_init(call);
 	if (!server_credentials) {
@@ -155,7 +150,7 @@ bool dcesrv_auth_auth3(struct dcesrv_call_state *call)
 	struct ncacn_packet *pkt = &call->pkt;
 	struct dcesrv_connection *dce_conn = call->conn;
 	NTSTATUS status;
-	enum ndr_err_code ndr_err;
+	uint32_t auth_length;
 
 	/* We can't work without an existing gensec state, and an new blob to feed it */
 	if (!dce_conn->auth_state.auth_info ||
@@ -164,11 +159,9 @@ bool dcesrv_auth_auth3(struct dcesrv_call_state *call)
 		return false;
 	}
 
-	ndr_err = ndr_pull_struct_blob(&pkt->u.auth3.auth_info,
-				       call, NULL,
-				       dce_conn->auth_state.auth_info,
-				       (ndr_pull_flags_fn_t)ndr_pull_dcerpc_auth);
-	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+	status = dcerpc_pull_auth_trailer(pkt, call, &pkt->u.auth3.auth_info,
+					  dce_conn->auth_state.auth_info, &auth_length, true);
+	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
 
@@ -205,7 +198,8 @@ bool dcesrv_auth_alter(struct dcesrv_call_state *call)
 {
 	struct ncacn_packet *pkt = &call->pkt;
 	struct dcesrv_connection *dce_conn = call->conn;
-	enum ndr_err_code ndr_err;
+	NTSTATUS status;
+	uint32_t auth_length;
 
 	/* on a pure interface change there is no auth blob */
 	if (pkt->u.alter.auth_info.length == 0) {
@@ -222,11 +216,10 @@ bool dcesrv_auth_alter(struct dcesrv_call_state *call)
 		return false;
 	}
 
-	ndr_err = ndr_pull_struct_blob(&pkt->u.alter.auth_info,
-				       call, NULL,
-				       dce_conn->auth_state.auth_info,
-				       (ndr_pull_flags_fn_t)ndr_pull_dcerpc_auth);
-	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+	status = dcerpc_pull_auth_trailer(pkt, call, &pkt->u.alter.auth_info,
+					  dce_conn->auth_state.auth_info,
+					  &auth_length, true);
+	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
 
@@ -286,11 +279,9 @@ bool dcesrv_auth_request(struct dcesrv_call_state *call, DATA_BLOB *full_packet)
 {
 	struct ncacn_packet *pkt = &call->pkt;
 	struct dcesrv_connection *dce_conn = call->conn;
-	DATA_BLOB auth_blob;
 	struct dcerpc_auth auth;
-	struct ndr_pull *ndr;
 	NTSTATUS status;
-	enum ndr_err_code ndr_err;
+	uint32_t auth_length;
 	size_t hdr_size = DCERPC_REQUEST_LENGTH;
 
 	if (!dce_conn->auth_state.auth_info ||
@@ -298,6 +289,10 @@ bool dcesrv_auth_request(struct dcesrv_call_state *call, DATA_BLOB *full_packet)
 		return true;
 	}
 
+	if (pkt->pfc_flags & DCERPC_PFC_FLAG_OBJECT_UUID) {
+		hdr_size += 16;
+	}
+
 	switch (dce_conn->auth_state.auth_info->auth_level) {
 	case DCERPC_AUTH_LEVEL_PRIVACY:
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
@@ -318,38 +313,14 @@ bool dcesrv_auth_request(struct dcesrv_call_state *call, DATA_BLOB *full_packet)
 		return false;
 	}
 
-	auth_blob.length = 8 + pkt->auth_length;
-
-	/* check for a valid length */
-	if (pkt->u.request.stub_and_verifier.length < auth_blob.length) {
-		return false;
-	}
-
-	auth_blob.data = 
-		pkt->u.request.stub_and_verifier.data + 
-		pkt->u.request.stub_and_verifier.length - auth_blob.length;
-	pkt->u.request.stub_and_verifier.length -= auth_blob.length;
-
-	/* pull the auth structure */
-	ndr = ndr_pull_init_blob(&auth_blob, call, lp_iconv_convenience(call->conn->dce_ctx->lp_ctx));
-	if (!ndr) {
+	status = dcerpc_pull_auth_trailer(pkt, call,
+					  &pkt->u.request.stub_and_verifier,
+					  &auth, &auth_length, false);
+	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
 
-	if (!(pkt->drep[0] & DCERPC_DREP_LE)) {
-		ndr->flags |= LIBNDR_FLAG_BIGENDIAN;
-	}
-
-	if (pkt->pfc_flags & DCERPC_PFC_FLAG_OBJECT_UUID) {
-		ndr->flags |= LIBNDR_FLAG_OBJECT_PRESENT;
-		hdr_size += 16;
-	}
-
-	ndr_err = ndr_pull_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, &auth);
-	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-		talloc_free(ndr);
-		return false;
-	}
+	pkt->u.request.stub_and_verifier.length -= auth_length;
 
 	/* check signature or unseal the packet */
 	switch (dce_conn->auth_state.auth_info->auth_level) {
@@ -388,11 +359,9 @@ bool dcesrv_auth_request(struct dcesrv_call_state *call, DATA_BLOB *full_packet)
 
 	/* remove the indicated amount of padding */
 	if (pkt->u.request.stub_and_verifier.length < auth.auth_pad_length) {
-		talloc_free(ndr);
 		return false;
 	}
 	pkt->u.request.stub_and_verifier.length -= auth.auth_pad_length;
-	talloc_free(ndr);
 
 	return NT_STATUS_IS_OK(status);
 }