[Patches] The way to remove gensec_update_ev() part7 (final!)

Stefan Metzmacher metze at samba.org
Thu Jul 20 22:27:17 UTC 2017


Hi,

and the final one that remove gensec_update_ev().

Most of them are already reviewed by Andreas.

They also passed private autobuilds a few times.

Please review and push:-)

Thanks!
metze

-------------- next part --------------
From 9119011271edb9a1fc52fbd1a25f0c56bae787f4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 01:52:09 +0200
Subject: [PATCH 01/17] auth/spnego: split gensec_spnego_create_negTokenInit()
 into subfunctions

This adds and uses the gensec_spnego_neg_loop() abstraction, which
abstracts start, step and finish hooks.

The start hook does the initial processing on the incoming paket and
may start the first possible subcontext. It indicates that
gensec_update() is required on the subcontext by returning
NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
caller should treat 'in_next' as const and don't attempt to free the
content.  NT_STATUS_OK indicates the finish hook should be invoked
directly withing the need of gensec_update() on the subcontext.
Every other error indicates an error that's returned to the caller.

The step hook processes the result of a failed gensec_update() and
can decide to ignore a failure or continue the negotiation by
setting up the next possible subcontext. It indicates that
gensec_update() is required on the subcontext by returning
NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
caller should treat 'in_next' as const and don't attempt to free the
content.  NT_STATUS_OK indicates the finish hook should be invoced
directly withing the need of gensec_update() on the subcontext.
Every other error indicated an error that's returned to the caller.

The finish hook processes the result of a successful gensec_update()
(NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED). It forms the
response pdu that will be returned from the toplevel gensec_update()
together with NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED. It
may also alter the state machine to prepare receiving the next pdu
from the peer.

This is the start of using this abstraction for the initial client or server
start with on empty input token from the peer.

This abstraction will be applied to all four other spnego states,
gensec_spnego_{client,server}_negToken{Init,Targ}() in the following
commits.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 auth/gensec/spnego.c | 325 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 253 insertions(+), 72 deletions(-)

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index aae3cab..a9b8a4f 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -47,6 +47,74 @@ enum spnego_state_position {
 	SPNEGO_DONE
 };
 
+struct spnego_state;
+struct spnego_neg_ops;
+struct spnego_neg_state;
+
+struct spnego_neg_state {
+	const struct spnego_neg_ops *ops;
+	const struct gensec_security_ops_wrapper *all_sec;
+	size_t all_idx;
+	const char * const *mech_types;
+	size_t mech_idx;
+};
+
+struct spnego_neg_ops {
+	const char *name;
+	/*
+	 * The start hook does the initial processing on the incoming paket and
+	 * may starts the first possible subcontext. It indicates that
+	 * gensec_update() is required on the subcontext by returning
+	 * NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
+	 * 'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
+	 * caller should treat 'in_next' as const and don't attempt to free the
+	 * content.  NT_STATUS_OK indicates the finish hook should be invoked
+	 * directly withing the need of gensec_update() on the subcontext.
+	 * Every other error indicates an error that's returned to the caller.
+	 */
+	NTSTATUS (*start_fn)(struct gensec_security *gensec_security,
+			     struct spnego_state *spnego_state,
+			     struct spnego_neg_state *n,
+			     struct spnego_data *spnego_in,
+			     TALLOC_CTX *in_mem_ctx,
+			     DATA_BLOB *in_next);
+	/*
+	 * The step hook processes the result of a failed gensec_update() and
+	 * can decide to ignore a failure and continue the negotiation by
+	 * setting up the next possible subcontext. It indicates that
+	 * gensec_update() is required on the subcontext by returning
+	 * NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
+	 * 'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
+	 * caller should treat 'in_next' as const and don't attempt to free the
+	 * content.  NT_STATUS_OK indicates the finish hook should be invoked
+	 * directly withing the need of gensec_update() on the subcontext.
+	 * Every other error indicates an error that's returned to the caller.
+	 */
+	NTSTATUS (*step_fn)(struct gensec_security *gensec_security,
+			    struct spnego_state *spnego_state,
+			    struct spnego_neg_state *n,
+			    struct spnego_data *spnego_in,
+			    NTSTATUS last_status,
+			    TALLOC_CTX *in_mem_ctx,
+			    DATA_BLOB *in_next);
+	/*
+	 * The finish hook processes the result of a successful gensec_update()
+	 * (NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED). It forms the
+	 * response pdu that will be returned from the toplevel gensec_update()
+	 * together with NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED. It
+	 * may also alter the state machine to prepare receiving the next pdu
+	 * from the peer.
+	 */
+	NTSTATUS (*finish_fn)(struct gensec_security *gensec_security,
+			      struct spnego_state *spnego_state,
+			      struct spnego_neg_state *n,
+			      struct spnego_data *spnego_in,
+			      NTSTATUS sub_status,
+			      const DATA_BLOB sub_out,
+			      TALLOC_CTX *out_mem_ctx,
+			      DATA_BLOB *out);
+};
+
 struct spnego_state {
 	enum spnego_message_type expected_packet;
 	enum spnego_state_position state_position;
@@ -77,6 +145,74 @@ struct spnego_state {
 	NTSTATUS out_status;
 };
 
+static struct spnego_neg_state *gensec_spnego_neg_state(TALLOC_CTX *mem_ctx,
+				       const const struct spnego_neg_ops *ops)
+{
+	struct spnego_neg_state *n = NULL;
+
+	n = talloc_zero(mem_ctx, struct spnego_neg_state);
+	if (n == NULL) {
+		return NULL;
+	}
+	n->ops = ops;
+
+	return n;
+}
+
+static NTSTATUS gensec_spnego_neg_loop(struct gensec_security *gensec_security,
+				       struct spnego_state *spnego_state,
+				       const const struct spnego_neg_ops *ops,
+				       struct tevent_context *ev,
+				       struct spnego_data *spnego_in,
+				       TALLOC_CTX *out_mem_ctx,
+				       DATA_BLOB *out)
+{
+	struct spnego_neg_state *n = NULL;
+	NTSTATUS status;
+	DATA_BLOB sub_in = data_blob_null;
+	DATA_BLOB sub_out = data_blob_null;
+
+	*out = data_blob_null;
+
+	n = gensec_spnego_neg_state(out_mem_ctx, ops);
+	if (n == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	status = n->ops->start_fn(gensec_security, spnego_state, n,
+				  spnego_in, n, &sub_in);
+	if (GENSEC_UPDATE_IS_NTERROR(status)) {
+		TALLOC_FREE(n);
+		return status;
+	}
+
+	while (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+		status = gensec_update_ev(spnego_state->sub_sec_security,
+					  n, ev, sub_in, &sub_out);
+		sub_in = data_blob_null;
+		if (NT_STATUS_IS_OK(status)) {
+			spnego_state->sub_sec_ready = true;
+		}
+		if (!GENSEC_UPDATE_IS_NTERROR(status)) {
+			break;
+		}
+		sub_out = data_blob_null;
+
+		status = n->ops->step_fn(gensec_security, spnego_state, n,
+					 spnego_in, status, n, &sub_in);
+		if (GENSEC_UPDATE_IS_NTERROR(status)) {
+			TALLOC_FREE(n);
+			return status;
+		}
+	}
+
+	status = n->ops->finish_fn(gensec_security, spnego_state, n,
+				   spnego_in, status, sub_out,
+				   out_mem_ctx, out);
+	TALLOC_FREE(n);
+	return status;
+}
+
 static void gensec_spnego_update_sub_abort(struct spnego_state *spnego_state)
 {
 	spnego_state->sub_sec_ready = false;
@@ -203,86 +339,59 @@ static NTSTATUS gensec_spnego_server_try_fallback(struct gensec_security *gensec
 	return NT_STATUS_INVALID_PARAMETER;
 }
 
-/** create a negTokenInit 
- *
- * This is the same packet, no matter if the client or server sends it first, but it is always the first packet
-*/
-static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec_security, 
-						  struct spnego_state *spnego_state,
-						  TALLOC_CTX *out_mem_ctx, 
-						  struct tevent_context *ev,
-						  DATA_BLOB *out)
+static NTSTATUS gensec_spnego_create_negTokenInit_start(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
 {
-	NTSTATUS status;
-	const char **mechTypes = NULL;
-	DATA_BLOB unwrapped_out = data_blob_null;
-	size_t all_idx = 0;
-	const struct gensec_security_ops_wrapper *all_sec;
-	const struct gensec_security_ops_wrapper *cur_sec = NULL;
-	const char **send_mech_types = NULL;
-	struct spnego_data spnego_out;
-	bool ok;
-
-	mechTypes = gensec_security_oids(gensec_security, 
-					 out_mem_ctx, GENSEC_OID_SPNEGO);
-	if (mechTypes == NULL) {
+	n->mech_idx = 0;
+	n->mech_types = gensec_security_oids(gensec_security, n,
+					     GENSEC_OID_SPNEGO);
+	if (n->mech_types == NULL) {
 		DBG_WARNING("gensec_security_oids() failed\n");
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	all_sec	= gensec_security_by_oid_list(gensec_security, 
-					      out_mem_ctx, 
-					      mechTypes,
-					      GENSEC_OID_SPNEGO);
-	if (all_sec == NULL) {
+	n->all_idx = 0;
+	n->all_sec = gensec_security_by_oid_list(gensec_security,
+						 n, n->mech_types,
+						 GENSEC_OID_SPNEGO);
+	if (n->all_sec == NULL) {
 		DBG_WARNING("gensec_security_by_oid_list() failed\n");
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	for (; all_sec[all_idx].op != NULL; all_idx++) {
+	return n->ops->step_fn(gensec_security, spnego_state, n,
+			       spnego_in, NT_STATUS_OK, in_mem_ctx, in_next);
+}
+
+static NTSTATUS gensec_spnego_create_negTokenInit_step(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS last_status,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
+{
+	if (!NT_STATUS_IS_OK(last_status)) {
+		const struct gensec_security_ops_wrapper *cur_sec =
+			&n->all_sec[n->all_idx];
+		const struct gensec_security_ops_wrapper *next_sec = NULL;
 		const char *next = NULL;
 		const char *principal = NULL;
 		int dbg_level = DBGLVL_WARNING;
+		NTSTATUS status = last_status;
 
-		cur_sec = &all_sec[all_idx];
-
-		status = gensec_subcontext_start(spnego_state,
-						 gensec_security,
-						 &spnego_state->sub_sec_security);
-		if (!NT_STATUS_IS_OK(status)) {
-			return status;
-		}
-		/* select the sub context */
-		status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
-						  cur_sec->op);
-		if (!NT_STATUS_IS_OK(status)) {
-			gensec_spnego_update_sub_abort(spnego_state);
-			continue;
-		}
-
-		if (spnego_state->state_position != SPNEGO_CLIENT_START) {
-			/*
-			 * The server doesn't generate an optimistic token.
-			 */
-			goto reply;
-		}
-
-		/* In the client, try and produce the first (optimistic) packet */
-		status = gensec_update_ev(spnego_state->sub_sec_security,
-					  out_mem_ctx,
-					  ev,
-					  data_blob_null,
-					  &unwrapped_out);
-		if (NT_STATUS_IS_OK(status)) {
-			spnego_state->sub_sec_ready = true;
-		}
-
-		if (!GENSEC_UPDATE_IS_NTERROR(status)) {
-			goto reply;
+		if (cur_sec[1].op != NULL) {
+			next_sec = &cur_sec[1];
 		}
 
-		if (cur_sec[1].op != NULL) {
-			next = cur_sec[1].op->name;
+		if (next_sec != NULL) {
+			next = next_sec->op->name;
 			dbg_level = DBGLVL_NOTICE;
 		}
 
@@ -315,23 +424,72 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
 		 * Pretend we never started it
 		 */
 		gensec_spnego_update_sub_abort(spnego_state);
+
+		/*
+		 * And try the next one...
+		 */
+		n->all_idx += 1;
+	}
+
+	for (; n->all_sec[n->all_idx].op != NULL; n->all_idx++) {
+		const struct gensec_security_ops_wrapper *cur_sec =
+			&n->all_sec[n->all_idx];
+		NTSTATUS status;
+
+		status = gensec_subcontext_start(spnego_state,
+						 gensec_security,
+						 &spnego_state->sub_sec_security);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+
+		/* select the sub context */
+		status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
+						  cur_sec->op);
+		if (!NT_STATUS_IS_OK(status)) {
+			gensec_spnego_update_sub_abort(spnego_state);
+			continue;
+		}
+
+		/* In the client, try and produce the first (optimistic) packet */
+		if (spnego_state->state_position == SPNEGO_CLIENT_START) {
+			*in_next = data_blob_null;
+			return NT_STATUS_MORE_PROCESSING_REQUIRED;
+		}
+
+		*in_next = data_blob_null;
+		return NT_STATUS_OK;
 	}
 
 	DBG_WARNING("Failed to setup SPNEGO negTokenInit request\n");
 	return NT_STATUS_INVALID_PARAMETER;
+}
+
+static NTSTATUS gensec_spnego_create_negTokenInit_finish(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS sub_status,
+					const DATA_BLOB sub_out,
+					TALLOC_CTX *out_mem_ctx,
+					DATA_BLOB *out)
+{
+	const struct gensec_security_ops_wrapper *cur_sec =
+			&n->all_sec[n->all_idx];
+	struct spnego_data spnego_out;
+	bool ok;
 
-reply:
 	spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
 
-	send_mech_types = gensec_security_oids_from_ops_wrapped(out_mem_ctx,
-								cur_sec);
-	if (send_mech_types == NULL) {
+	n->mech_types = gensec_security_oids_from_ops_wrapped(n, cur_sec);
+	if (n->mech_types == NULL) {
 		DBG_WARNING("gensec_security_oids_from_ops_wrapped() failed\n");
 		return NT_STATUS_NO_MEMORY;
 	}
 
 	ok = spnego_write_mech_types(spnego_state,
-				     send_mech_types,
+				     n->mech_types,
 				     &spnego_state->mech_types);
 	if (!ok) {
 		DBG_ERR("Failed to write mechTypes\n");
@@ -339,7 +497,7 @@ reply:
 	}
 
 	/* List the remaining mechs as options */
-	spnego_out.negTokenInit.mechTypes = send_mech_types;
+	spnego_out.negTokenInit.mechTypes = n->mech_types;
 	spnego_out.negTokenInit.reqFlags = data_blob_null;
 	spnego_out.negTokenInit.reqFlagsPadding = 0;
 
@@ -350,7 +508,7 @@ reply:
 		spnego_out.negTokenInit.mechListMIC = data_blob_null;
 	}
 
-	spnego_out.negTokenInit.mechToken = unwrapped_out;
+	spnego_out.negTokenInit.mechToken = sub_out;
 
 	if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
 		DBG_ERR("Failed to write NEG_TOKEN_INIT\n");
@@ -376,6 +534,29 @@ reply:
 	return NT_STATUS_MORE_PROCESSING_REQUIRED;
 }
 
+static const struct spnego_neg_ops gensec_spnego_create_negTokenInit_ops = {
+	.name      = "create_negTokenInit",
+	.start_fn  = gensec_spnego_create_negTokenInit_start,
+	.step_fn   = gensec_spnego_create_negTokenInit_step,
+	.finish_fn = gensec_spnego_create_negTokenInit_finish,
+};
+
+/** create a negTokenInit
+ *
+ * This is the same packet, no matter if the client or server sends it first, but it is always the first packet
+*/
+static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec_security,
+						  struct spnego_state *spnego_state,
+						  TALLOC_CTX *out_mem_ctx,
+						  struct tevent_context *ev,
+						  DATA_BLOB *out)
+{
+	struct spnego_data *spnego_in = NULL;
+	return gensec_spnego_neg_loop(gensec_security, spnego_state,
+				      &gensec_spnego_create_negTokenInit_ops,
+				      ev, spnego_in, out_mem_ctx, out);
+}
+
 static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
 						  struct spnego_state *spnego_state,
 						  struct tevent_context *ev,
-- 
1.9.1


From 6141565517e2d349baca14d51043d16b3a79d3f0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 12:59:43 +0200
Subject: [PATCH 02/17] auth/spnego: split gensec_spnego_client_negTokenInit()
 into subfunctions

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 auth/gensec/spnego.c | 183 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 112 insertions(+), 71 deletions(-)

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index a9b8a4f..caab065 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -557,25 +557,15 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
 				      ev, spnego_in, out_mem_ctx, out);
 }
 
-static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
+static NTSTATUS gensec_spnego_client_negTokenInit_start(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
 {
-	TALLOC_CTX *frame = talloc_stackframe();
-	DATA_BLOB sub_out = data_blob_null;
 	const char *tp = NULL;
-	const char * const *mech_types = NULL;
-	size_t all_idx = 0;
-	const struct gensec_security_ops_wrapper *all_sec = NULL;
-	struct spnego_data spnego_out;
-	const char *my_mechs[] = {NULL, NULL};
-	NTSTATUS status;
-	bool ok;
-
-	*out = data_blob_null;
 
 	/* The server offers a list of mechanisms */
 
@@ -587,62 +577,46 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 		}
 	}
 
-	mech_types = spnego_in->negTokenInit.mechTypes;
-	if (mech_types == NULL) {
-		TALLOC_FREE(frame);
+	n->mech_idx = 0;
+	n->mech_types = spnego_in->negTokenInit.mechTypes;
+	if (n->mech_types == NULL) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	all_sec = gensec_security_by_oid_list(gensec_security,
-					      frame, mech_types,
-					      GENSEC_OID_SPNEGO);
-	if (all_sec == NULL) {
+	n->all_idx = 0;
+	n->all_sec = gensec_security_by_oid_list(gensec_security,
+						 n, n->mech_types,
+						 GENSEC_OID_SPNEGO);
+	if (n->all_sec == NULL) {
 		DBG_WARNING("gensec_security_by_oid_list() failed\n");
-		TALLOC_FREE(frame);
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	for (; all_sec[all_idx].op; all_idx++) {
+	return n->ops->step_fn(gensec_security, spnego_state, n,
+			       spnego_in, NT_STATUS_OK, in_mem_ctx, in_next);
+}
+
+static NTSTATUS gensec_spnego_client_negTokenInit_step(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS last_status,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
+{
+	if (!NT_STATUS_IS_OK(last_status)) {
 		const struct gensec_security_ops_wrapper *cur_sec =
-			&all_sec[all_idx];
+			&n->all_sec[n->all_idx];
+		const struct gensec_security_ops_wrapper *next_sec = NULL;
 		const char *next = NULL;
 		const char *principal = NULL;
 		int dbg_level = DBGLVL_WARNING;
 		bool allow_fallback = false;
+		NTSTATUS status = last_status;
 
-		status = gensec_subcontext_start(spnego_state,
-						 gensec_security,
-						 &spnego_state->sub_sec_security);
-		if (!NT_STATUS_IS_OK(status)) {
-			TALLOC_FREE(frame);
-			return status;
-		}
-
-		/* select the sub context */
-		status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
-						  cur_sec->op);
-		if (!NT_STATUS_IS_OK(status)) {
-			/*
-			 * Pretend we never started it.
-			 */
-			gensec_spnego_update_sub_abort(spnego_state);
-			continue;
-		}
-
-		spnego_state->neg_oid = cur_sec->oid;
-
-		/*
-		 * As client we don't use an optimistic token from the server.
-		 */
-		status = gensec_update_ev(spnego_state->sub_sec_security,
-					  frame, ev, data_blob_null, &sub_out);
-		if (NT_STATUS_IS_OK(status)) {
-			spnego_state->sub_sec_ready = true;
-		}
-
-		if (!GENSEC_UPDATE_IS_NTERROR(status)) {
-			/* OK or MORE_PROCESSING_REQUIRED */
-			goto reply;
+		if (cur_sec[1].op != NULL) {
+			next_sec = &cur_sec[1];
 		}
 
 		/*
@@ -660,8 +634,8 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 			allow_fallback = true;
 		}
 
-		if (allow_fallback && cur_sec[1].op != NULL) {
-			next = cur_sec[1].op->name;
+		if (allow_fallback && next_sec != NULL) {
+			next = next_sec->op->name;
 			dbg_level = DBGLVL_NOTICE;
 		}
 
@@ -679,16 +653,14 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 		}
 
 		DBG_PREFIX(dbg_level, (
-			   "%s: creating NEG_TOKEN_INIT "
-			   "for %s failed (next[%s]): %s\n",
-			   spnego_state->sub_sec_security->ops->name,
+			   "%s: creating NEG_TOKEN_INIT for %s failed "
+			   "(next[%s]): %s\n", cur_sec->op->name,
 			   principal, next, nt_errstr(status)));
 
 		if (next == NULL) {
 			/*
 			 * A hard error without a possible fallback.
 			 */
-			TALLOC_FREE(frame);
 			return status;
 		}
 
@@ -696,13 +668,66 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 		 * Pretend we never started it.
 		 */
 		gensec_spnego_update_sub_abort(spnego_state);
+
+		/*
+		 * And try the next one...
+		 */
+		n->all_idx += 1;
+	}
+
+	for (; n->all_sec[n->all_idx].op != NULL; n->all_idx++) {
+		const struct gensec_security_ops_wrapper *cur_sec =
+			&n->all_sec[n->all_idx];
+		NTSTATUS status;
+
+		status = gensec_subcontext_start(spnego_state,
+						 gensec_security,
+						 &spnego_state->sub_sec_security);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+
+		/* select the sub context */
+		status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
+						  cur_sec->op);
+		if (!NT_STATUS_IS_OK(status)) {
+			gensec_spnego_update_sub_abort(spnego_state);
+			continue;
+		}
+
+		/*
+		 * Note that 'cur_sec' is temporary memory, but
+		 * cur_sec->oid points to a const string in the
+		 * backends gensec_security_ops structure.
+		 */
+		spnego_state->neg_oid = cur_sec->oid;
+
+		/*
+		 * As client we don't use an optimistic token from the server.
+		 * But try to produce one for the server.
+		 */
+		*in_next = data_blob_null;
+		return NT_STATUS_MORE_PROCESSING_REQUIRED;
 	}
 
 	DBG_WARNING("Could not find a suitable mechtype in NEG_TOKEN_INIT\n");
-	TALLOC_FREE(frame);
 	return NT_STATUS_INVALID_PARAMETER;
+}
+
+static NTSTATUS gensec_spnego_client_negTokenInit_finish(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS sub_status,
+					const DATA_BLOB sub_out,
+					TALLOC_CTX *out_mem_ctx,
+					DATA_BLOB *out)
+{
+	struct spnego_data spnego_out;
+	const char *my_mechs[] = {NULL, NULL};
+	bool ok;
 
- reply:
 	my_mechs[0] = spnego_state->neg_oid;
 	/* compose reply */
 	spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
@@ -714,7 +739,6 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 
 	if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
 		DBG_ERR("Failed to write SPNEGO reply to NEG_TOKEN_INIT\n");
-		TALLOC_FREE(frame);
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
@@ -723,7 +747,6 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 				     &spnego_state->mech_types);
 	if (!ok) {
 		DBG_ERR("failed to write mechTypes\n");
-		TALLOC_FREE(frame);
 		return NT_STATUS_NO_MEMORY;
 	}
 
@@ -731,10 +754,28 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 	spnego_state->expected_packet = SPNEGO_NEG_TOKEN_TARG;
 	spnego_state->state_position = SPNEGO_CLIENT_TARG;
 
-	TALLOC_FREE(frame);
 	return NT_STATUS_MORE_PROCESSING_REQUIRED;
 }
 
+static const struct spnego_neg_ops gensec_spnego_client_negTokenInit_ops = {
+	.name      = "client_negTokenInit",
+	.start_fn  = gensec_spnego_client_negTokenInit_start,
+	.step_fn   = gensec_spnego_client_negTokenInit_step,
+	.finish_fn = gensec_spnego_client_negTokenInit_finish,
+};
+
+static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
+						  struct spnego_state *spnego_state,
+						  struct tevent_context *ev,
+						  struct spnego_data *spnego_in,
+						  TALLOC_CTX *out_mem_ctx,
+						  DATA_BLOB *out)
+{
+	return gensec_spnego_neg_loop(gensec_security, spnego_state,
+				      &gensec_spnego_client_negTokenInit_ops,
+				      ev, spnego_in, out_mem_ctx, out);
+}
+
 static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec_security,
 						  struct spnego_state *spnego_state,
 						  struct tevent_context *ev,
-- 
1.9.1


From 1b2cfa2079cc85b727bdf9652813ac773444299b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 13:56:02 +0200
Subject: [PATCH 03/17] auth/spnego: split gensec_spnego_client_negTokenTarg()
 into subfunctions

Check with git show -U15

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

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index caab065..e5ca297 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -776,22 +776,17 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
 				      ev, spnego_in, out_mem_ctx, out);
 }
 
-static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
+static NTSTATUS gensec_spnego_client_negTokenTarg_start(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
 {
 	struct spnego_negTokenTarg *ta = &spnego_in->negTokenTarg;
-	DATA_BLOB sub_in = ta->responseToken;
-	DATA_BLOB mech_list_mic = data_blob_null;
-	DATA_BLOB sub_out = data_blob_null;
-	struct spnego_data spnego_out;
 	NTSTATUS status;
 
-	*out = data_blob_null;
-
 	spnego_state->num_targs++;
 
 	if (ta->negResult == SPNEGO_REJECT) {
@@ -891,8 +886,7 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
 			 * https://bugzilla.samba.org/show_bug.cgi?id=11994
 			 */
 			spnego_state->needs_mic_check = false;
-			status = NT_STATUS_OK;
-			goto client_response;
+			return NT_STATUS_OK;
 		}
 
 		status = gensec_check_packet(spnego_state->sub_sec_security,
@@ -908,22 +902,93 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
 		}
 		spnego_state->needs_mic_check = false;
 		spnego_state->done_mic_check = true;
-		goto client_response;
+		return NT_STATUS_OK;
 	}
 
 	if (!spnego_state->sub_sec_ready) {
-		status = gensec_update_ev(spnego_state->sub_sec_security,
-					  out_mem_ctx, ev,
-					  sub_in,
-					  &sub_out);
-		if (NT_STATUS_IS_OK(status)) {
-			spnego_state->sub_sec_ready = true;
-		}
-		if (!NT_STATUS_IS_OK(status)) {
-			goto client_response;
-		}
-	} else {
-		status = NT_STATUS_OK;
+		*in_next = ta->responseToken;
+		return NT_STATUS_MORE_PROCESSING_REQUIRED;
+	}
+
+	return NT_STATUS_OK;
+}
+
+static NTSTATUS gensec_spnego_client_negTokenTarg_step(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS last_status,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
+{
+	if (GENSEC_UPDATE_IS_NTERROR(last_status)) {
+		DBG_WARNING("SPNEGO(%s) login failed: %s\n",
+			    spnego_state->sub_sec_security->ops->name,
+			    nt_errstr(last_status));
+		return last_status;
+	}
+
+	/*
+	 * This should never be reached!
+	 * The step function is only called on errors!
+	 */
+	smb_panic(__location__);
+	return NT_STATUS_INTERNAL_ERROR;
+}
+
+static NTSTATUS gensec_spnego_client_negTokenTarg_finish(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS sub_status,
+					const DATA_BLOB sub_out,
+					TALLOC_CTX *out_mem_ctx,
+					DATA_BLOB *out)
+{
+	const struct spnego_negTokenTarg *ta =
+		&spnego_in->negTokenTarg;
+	DATA_BLOB mech_list_mic = data_blob_null;
+	NTSTATUS status;
+	struct spnego_data spnego_out;
+
+	status = sub_status;
+
+	if (!spnego_state->sub_sec_ready) {
+		/*
+		 * We're not yet ready to deal with signatures.
+		 */
+		goto client_response;
+	}
+
+	if (spnego_state->done_mic_check) {
+		/*
+		 * We already checked the mic,
+		 * either the in last round here
+		 * in gensec_spnego_client_negTokenTarg_finish()
+		 * or during this round in
+		 * gensec_spnego_client_negTokenTarg_start().
+		 *
+		 * Both cases we're sure we don't have to
+		 * call gensec_sign_packet().
+		 */
+		goto client_response;
+	}
+
+	if (spnego_state->may_skip_mic_check) {
+		/*
+		 * This can only be set during
+		 * the last round here in
+		 * gensec_spnego_client_negTokenTarg_finish()
+		 * below. And during this round
+		 * we already passed the checks in
+		 * gensec_spnego_client_negTokenTarg_start().
+		 *
+		 * So we need to skip to deal with
+		 * any signatures now.
+		 */
+		goto client_response;
 	}
 
 	if (!spnego_state->done_mic_check) {
@@ -1037,7 +1102,7 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
 
 	if (spnego_state->needs_mic_sign) {
 		status = gensec_sign_packet(spnego_state->sub_sec_security,
-					    out_mem_ctx,
+					    n,
 					    spnego_state->mech_types.data,
 					    spnego_state->mech_types.length,
 					    spnego_state->mech_types.data,
@@ -1052,13 +1117,6 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
 	}
 
  client_response:
-	if (GENSEC_UPDATE_IS_NTERROR(status)) {
-		DBG_WARNING("SPNEGO(%s) login failed: %s\n",
-			    spnego_state->sub_sec_security->ops->name,
-			    nt_errstr(status));
-		return status;
-	}
-
 	if (sub_out.length == 0 && mech_list_mic.length == 0) {
 		*out = data_blob_null;
 
@@ -1101,6 +1159,24 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
 	return NT_STATUS_MORE_PROCESSING_REQUIRED;
 }
 
+static const struct spnego_neg_ops gensec_spnego_client_negTokenTarg_ops = {
+	.name      = "client_negTokenTarg",
+	.start_fn  = gensec_spnego_client_negTokenTarg_start,
+	.step_fn   = gensec_spnego_client_negTokenTarg_step,
+	.finish_fn = gensec_spnego_client_negTokenTarg_finish,
+};
+
+static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec_security,
+						  struct spnego_state *spnego_state,
+						  struct tevent_context *ev,
+						  struct spnego_data *spnego_in,
+						  TALLOC_CTX *out_mem_ctx,
+						  DATA_BLOB *out)
+{
+	return gensec_spnego_neg_loop(gensec_security, spnego_state,
+				      &gensec_spnego_client_negTokenTarg_ops,
+				      ev, spnego_in, out_mem_ctx, out);
+}
 /** create a server negTokenTarg 
  *
  * This is the case, where the client is the first one who sends data
-- 
1.9.1


From 21cbd82863eba737c0fc2c8fce3abf531b3b3345 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 15:22:57 +0200
Subject: [PATCH 04/17] auth/spnego: split gensec_spnego_server_negTokenInit()
 into subfunctions

Check with git show -U15

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

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index e5ca297..5c6e58b 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1225,58 +1225,128 @@ static NTSTATUS gensec_spnego_server_response(struct spnego_state *spnego_state,
 	return nt_status;
 }
 
-static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
+static NTSTATUS gensec_spnego_server_negTokenInit_start(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
 {
-	TALLOC_CTX *frame = talloc_stackframe();
-	DATA_BLOB sub_out = data_blob_null;
-	DATA_BLOB mech_list_mic = data_blob_null;
-	const char * const *mech_types = NULL;
-	size_t all_idx = 0;
-	const struct gensec_security_ops_wrapper *all_sec = NULL;
-	size_t mech_idx = 0;
-	NTSTATUS status;
 	bool ok;
 
-	mech_types = spnego_in->negTokenInit.mechTypes;
-	if (mech_types == NULL) {
-		TALLOC_FREE(frame);
+	n->mech_idx = 0;
+	n->mech_types = spnego_in->negTokenInit.mechTypes;
+	if (n->mech_types == NULL) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	all_sec = gensec_security_by_oid_list(gensec_security, frame,
-					      mech_types, GENSEC_OID_SPNEGO);
-	if (all_sec == NULL) {
+	n->all_idx = 0;
+	n->all_sec = gensec_security_by_oid_list(gensec_security,
+						 n, n->mech_types,
+						 GENSEC_OID_SPNEGO);
+	if (n->all_sec == NULL) {
 		DBG_WARNING("gensec_security_by_oid_list() failed\n");
-		TALLOC_FREE(frame);
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	ok = spnego_write_mech_types(spnego_state, mech_types,
+	ok = spnego_write_mech_types(spnego_state,
+				     n->mech_types,
 				     &spnego_state->mech_types);
 	if (!ok) {
 		DBG_ERR("Failed to write mechTypes\n");
-		TALLOC_FREE(frame);
 		return NT_STATUS_NO_MEMORY;
 	}
 
+	return n->ops->step_fn(gensec_security, spnego_state, n,
+			       spnego_in, NT_STATUS_OK, in_mem_ctx, in_next);
+}
+
+static NTSTATUS gensec_spnego_server_negTokenInit_step(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS last_status,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
+{
+	if (!NT_STATUS_IS_OK(last_status)) {
+		const struct gensec_security_ops_wrapper *cur_sec =
+			&n->all_sec[n->all_idx];
+		const char *next_mech = n->mech_types[n->mech_idx+1];
+		const struct gensec_security_ops_wrapper *next_sec = NULL;
+		const char *next = NULL;
+		int dbg_level = DBGLVL_WARNING;
+		bool allow_fallback = false;
+		NTSTATUS status = last_status;
+		size_t i;
+
+		for (i = 0; next_mech != NULL && n->all_sec[i].op != NULL; i++) {
+			if (strcmp(next_mech, n->all_sec[i].oid) != 0) {
+				continue;
+			}
+
+			next_sec = &n->all_sec[i];
+			break;
+		}
+
+		if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER) ||
+		    NT_STATUS_EQUAL(status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO))
+		{
+			allow_fallback = true;
+		}
+
+		if (allow_fallback && next_sec != NULL) {
+			next = next_sec->op->name;
+			dbg_level = DBGLVL_NOTICE;
+		}
+
+		DBG_PREFIX(dbg_level, (
+			   "%s: parsing NEG_TOKEN_INIT content failed "
+			   "(next[%s]): %s\n", cur_sec->op->name,
+			   next, nt_errstr(status)));
+
+		if (next == NULL) {
+			/*
+			 * A hard error without a possible fallback.
+			 */
+			return status;
+		}
+
+		/*
+		 * Pretend we never started it
+		 */
+		gensec_spnego_update_sub_abort(spnego_state);
+
+		/*
+		 * And try the next one, based on the clients
+		 * mech type list...
+		 */
+		n->mech_idx += 1;
+	}
+
 	/*
-	 * First try the preferred mechs from the client.
+	 * we always reset all_idx here, as the negotiation is
+	 * done via mech_idx!
 	 */
-	for (; mech_types[mech_idx]; mech_idx++) {
-		const char *cur_mech = mech_types[mech_idx];
+	n->all_idx = 0;
+
+	for (; n->mech_types[n->mech_idx] != NULL; n->mech_idx++) {
+		const char *cur_mech = n->mech_types[n->mech_idx];
 		const struct gensec_security_ops_wrapper *cur_sec = NULL;
+		NTSTATUS status;
 		DATA_BLOB sub_in = data_blob_null;
+		size_t i;
 
-		for (all_idx = 0; all_sec[all_idx].op; all_idx++) {
-			if (strcmp(cur_mech, all_sec[all_idx].oid) == 0) {
-				cur_sec = &all_sec[all_idx];
-				break;
+		for (i = 0; n->all_sec[i].op != NULL; i++) {
+			if (strcmp(cur_mech, n->all_sec[i].oid) != 0) {
+				continue;
 			}
+
+			cur_sec = &n->all_sec[i];
+			n->all_idx = i;
+			break;
 		}
 
 		if (cur_sec == NULL) {
@@ -1287,7 +1357,6 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
 						 gensec_security,
 						 &spnego_state->sub_sec_security);
 		if (!NT_STATUS_IS_OK(status)) {
-			TALLOC_FREE(frame);
 			return status;
 		}
 
@@ -1302,58 +1371,48 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
 			continue;
 		}
 
-		if (mech_idx > 0) {
+		if (n->mech_idx == 0) {
+			/*
+			 * We can use the optimistic token.
+			 */
+			sub_in = spnego_in->negTokenInit.mechToken;
+		} else {
 			/*
 			 * Indicate the downgrade and request a
 			 * mic.
 			 */
 			spnego_state->downgraded = true;
 			spnego_state->mic_requested = true;
-			/* no optimistic token */
-			spnego_state->neg_oid = cur_sec->oid;
-			sub_out = data_blob_null;
-			status = NT_STATUS_MORE_PROCESSING_REQUIRED;
-			goto reply;
 		}
 
 		/*
-		 * Try the optimistic token from the client
+		 * Note that 'cur_sec' is temporary memory, but
+		 * cur_sec->oid points to a const string in the
+		 * backends gensec_security_ops structure.
 		 */
-		sub_in = spnego_in->negTokenInit.mechToken;
-		status = gensec_update_ev(spnego_state->sub_sec_security,
-					  frame, ev, sub_in, &sub_out);
-		if (NT_STATUS_IS_OK(status)) {
-			spnego_state->sub_sec_ready = true;
-		}
-		if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER) ||
-		    NT_STATUS_EQUAL(status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO)) {
-
-			DBG_WARNING("%s: NEG_TOKEN_INIT failed to parse contents: %s\n",
-				    cur_sec->op->name, nt_errstr(status));
-
-			/*
-			 * Pretend we never started it
-			 */
-			gensec_spnego_update_sub_abort(spnego_state);
-			continue;
-		}
-
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			DBG_WARNING("%s: NEG_TOKEN_INIT failed: %s\n",
-				    cur_sec->op->name, nt_errstr(status));
-			TALLOC_FREE(frame);
-			return status;
-		}
-
 		spnego_state->neg_oid = cur_sec->oid;
-		goto reply; /* OK or MORE PROCESSING */
+
+		/* we need some content from the mech */
+		*in_next = sub_in;
+		return NT_STATUS_MORE_PROCESSING_REQUIRED;
 	}
 
 	DBG_WARNING("Could not find a suitable mechtype in NEG_TOKEN_INIT\n");
-	TALLOC_FREE(frame);
 	return NT_STATUS_INVALID_PARAMETER;
+}
+
+static NTSTATUS gensec_spnego_server_negTokenInit_finish(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS sub_status,
+					const DATA_BLOB sub_out,
+					TALLOC_CTX *out_mem_ctx,
+					DATA_BLOB *out)
+{
+	DATA_BLOB mech_list_mic = data_blob_null;
 
- reply:
 	if (spnego_state->simulate_w2k) {
 		/*
 		 * Windows 2000 returns the unwrapped token
@@ -1366,14 +1425,31 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
 		mech_list_mic = sub_out;
 	}
 
-	status = gensec_spnego_server_response(spnego_state,
-					       out_mem_ctx,
-					       status,
-					       sub_out,
-					       mech_list_mic,
-					       out);
-	TALLOC_FREE(frame);
-	return status;
+	return gensec_spnego_server_response(spnego_state,
+					     out_mem_ctx,
+					     sub_status,
+					     sub_out,
+					     mech_list_mic,
+					     out);
+}
+
+static const struct spnego_neg_ops gensec_spnego_server_negTokenInit_ops = {
+	.name      = "server_negTokenInit",
+	.start_fn  = gensec_spnego_server_negTokenInit_start,
+	.step_fn   = gensec_spnego_server_negTokenInit_step,
+	.finish_fn = gensec_spnego_server_negTokenInit_finish,
+};
+
+static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec_security,
+						  struct spnego_state *spnego_state,
+						  struct tevent_context *ev,
+						  struct spnego_data *spnego_in,
+						  TALLOC_CTX *out_mem_ctx,
+						  DATA_BLOB *out)
+{
+	return gensec_spnego_neg_loop(gensec_security, spnego_state,
+				      &gensec_spnego_server_negTokenInit_ops,
+				      ev, spnego_in, out_mem_ctx, out);
 }
 
 static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec_security,
-- 
1.9.1


From 3fdded879db78c4d03dd9cdfb976121bb1d7be2d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 15:40:41 +0200
Subject: [PATCH 05/17] auth/spnego: split gensec_spnego_server_negTokenTarg()
 into subfunctions

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

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 5c6e58b..f5488ac 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1452,20 +1452,16 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
 				      ev, spnego_in, out_mem_ctx, out);
 }
 
-static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
+static NTSTATUS gensec_spnego_server_negTokenTarg_start(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
 {
 	const struct spnego_negTokenTarg *ta = &spnego_in->negTokenTarg;
-	DATA_BLOB sub_in = ta->responseToken;
-	DATA_BLOB mech_list_mic = data_blob_null;
-	DATA_BLOB sub_out = data_blob_null;
 	NTSTATUS status;
-	bool have_sign = true;
-	bool new_spnego = false;
 
 	spnego_state->num_targs++;
 
@@ -1494,26 +1490,78 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
 
 		spnego_state->needs_mic_check = false;
 		spnego_state->done_mic_check = true;
-		goto server_response;
+		return NT_STATUS_OK;
 	}
 
 	if (!spnego_state->sub_sec_ready) {
-		status = gensec_update_ev(spnego_state->sub_sec_security,
-					  out_mem_ctx, ev,
-					  sub_in, &sub_out);
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			DEBUG(2, ("SPNEGO login failed: %s\n",
-				  nt_errstr(status)));
-			return status;
-		}
-		if (NT_STATUS_IS_OK(status)) {
-			spnego_state->sub_sec_ready = true;
-		}
-		if (!NT_STATUS_IS_OK(status)) {
-			goto server_response;
-		}
-	} else {
-		status = NT_STATUS_OK;
+		*in_next = ta->responseToken;
+		return NT_STATUS_MORE_PROCESSING_REQUIRED;
+	}
+
+	return NT_STATUS_OK;
+}
+
+static NTSTATUS gensec_spnego_server_negTokenTarg_step(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS last_status,
+					TALLOC_CTX *in_mem_ctx,
+					DATA_BLOB *in_next)
+{
+	if (GENSEC_UPDATE_IS_NTERROR(last_status)) {
+		DBG_NOTICE("SPNEGO(%s) login failed: %s\n",
+			   spnego_state->sub_sec_security->ops->name,
+			   nt_errstr(last_status));
+		return last_status;
+	}
+
+	/*
+	 * This should never be reached!
+	 * The step function is only called on errors!
+	 */
+	smb_panic(__location__);
+	return NT_STATUS_INTERNAL_ERROR;
+}
+
+static NTSTATUS gensec_spnego_server_negTokenTarg_finish(
+					struct gensec_security *gensec_security,
+					struct spnego_state *spnego_state,
+					struct spnego_neg_state *n,
+					struct spnego_data *spnego_in,
+					NTSTATUS sub_status,
+					const DATA_BLOB sub_out,
+					TALLOC_CTX *out_mem_ctx,
+					DATA_BLOB *out)
+{
+	const struct spnego_negTokenTarg *ta = &spnego_in->negTokenTarg;
+	DATA_BLOB mech_list_mic = data_blob_null;
+	NTSTATUS status;
+	bool have_sign = true;
+	bool new_spnego = false;
+
+	status = sub_status;
+
+	if (!spnego_state->sub_sec_ready) {
+		/*
+		 * We're not yet ready to deal with signatures.
+		 */
+		goto server_response;
+	}
+
+	if (spnego_state->done_mic_check) {
+		/*
+		 * We already checked the mic,
+		 * either the in last round here
+		 * in gensec_spnego_server_negTokenTarg_finish()
+		 * or during this round in
+		 * gensec_spnego_server_negTokenTarg_start().
+		 *
+		 * Both cases we're sure we don't have to
+		 * call gensec_sign_packet().
+		 */
+		goto server_response;
 	}
 
 	have_sign = gensec_have_feature(spnego_state->sub_sec_security,
@@ -1551,7 +1599,7 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
 
 	if (spnego_state->needs_mic_sign) {
 		status = gensec_sign_packet(spnego_state->sub_sec_security,
-					    out_mem_ctx,
+					    n,
 					    spnego_state->mech_types.data,
 					    spnego_state->mech_types.length,
 					    spnego_state->mech_types.data,
@@ -1578,6 +1626,25 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
 					     out);
 }
 
+static const struct spnego_neg_ops gensec_spnego_server_negTokenTarg_ops = {
+	.name      = "server_negTokenTarg",
+	.start_fn  = gensec_spnego_server_negTokenTarg_start,
+	.step_fn   = gensec_spnego_server_negTokenTarg_step,
+	.finish_fn = gensec_spnego_server_negTokenTarg_finish,
+};
+
+static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec_security,
+						  struct spnego_state *spnego_state,
+						  struct tevent_context *ev,
+						  struct spnego_data *spnego_in,
+						  TALLOC_CTX *out_mem_ctx,
+						  DATA_BLOB *out)
+{
+	return gensec_spnego_neg_loop(gensec_security, spnego_state,
+				      &gensec_spnego_server_negTokenTarg_ops,
+				      ev, spnego_in, out_mem_ctx, out);
+}
+
 struct gensec_spnego_update_state {
 	struct tevent_context *ev;
 	struct gensec_security *gensec;
-- 
1.9.1


From 14d7f1b45569445481c2925d98f58771062a1159 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 11:01:23 +0200
Subject: [PATCH 06/17] auth/spnego: replace gensec_spnego_neg_loop() by real
 async processing of {start,step,finish}_fn()

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

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index f5488ac..3983cf5 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -159,60 +159,6 @@ static struct spnego_neg_state *gensec_spnego_neg_state(TALLOC_CTX *mem_ctx,
 	return n;
 }
 
-static NTSTATUS gensec_spnego_neg_loop(struct gensec_security *gensec_security,
-				       struct spnego_state *spnego_state,
-				       const const struct spnego_neg_ops *ops,
-				       struct tevent_context *ev,
-				       struct spnego_data *spnego_in,
-				       TALLOC_CTX *out_mem_ctx,
-				       DATA_BLOB *out)
-{
-	struct spnego_neg_state *n = NULL;
-	NTSTATUS status;
-	DATA_BLOB sub_in = data_blob_null;
-	DATA_BLOB sub_out = data_blob_null;
-
-	*out = data_blob_null;
-
-	n = gensec_spnego_neg_state(out_mem_ctx, ops);
-	if (n == NULL) {
-		return NT_STATUS_NO_MEMORY;
-	}
-
-	status = n->ops->start_fn(gensec_security, spnego_state, n,
-				  spnego_in, n, &sub_in);
-	if (GENSEC_UPDATE_IS_NTERROR(status)) {
-		TALLOC_FREE(n);
-		return status;
-	}
-
-	while (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-		status = gensec_update_ev(spnego_state->sub_sec_security,
-					  n, ev, sub_in, &sub_out);
-		sub_in = data_blob_null;
-		if (NT_STATUS_IS_OK(status)) {
-			spnego_state->sub_sec_ready = true;
-		}
-		if (!GENSEC_UPDATE_IS_NTERROR(status)) {
-			break;
-		}
-		sub_out = data_blob_null;
-
-		status = n->ops->step_fn(gensec_security, spnego_state, n,
-					 spnego_in, status, n, &sub_in);
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			TALLOC_FREE(n);
-			return status;
-		}
-	}
-
-	status = n->ops->finish_fn(gensec_security, spnego_state, n,
-				   spnego_in, status, sub_out,
-				   out_mem_ctx, out);
-	TALLOC_FREE(n);
-	return status;
-}
-
 static void gensec_spnego_update_sub_abort(struct spnego_state *spnego_state)
 {
 	spnego_state->sub_sec_ready = false;
@@ -541,22 +487,6 @@ static const struct spnego_neg_ops gensec_spnego_create_negTokenInit_ops = {
 	.finish_fn = gensec_spnego_create_negTokenInit_finish,
 };
 
-/** create a negTokenInit
- *
- * This is the same packet, no matter if the client or server sends it first, but it is always the first packet
-*/
-static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  TALLOC_CTX *out_mem_ctx,
-						  struct tevent_context *ev,
-						  DATA_BLOB *out)
-{
-	struct spnego_data *spnego_in = NULL;
-	return gensec_spnego_neg_loop(gensec_security, spnego_state,
-				      &gensec_spnego_create_negTokenInit_ops,
-				      ev, spnego_in, out_mem_ctx, out);
-}
-
 static NTSTATUS gensec_spnego_client_negTokenInit_start(
 					struct gensec_security *gensec_security,
 					struct spnego_state *spnego_state,
@@ -764,18 +694,6 @@ static const struct spnego_neg_ops gensec_spnego_client_negTokenInit_ops = {
 	.finish_fn = gensec_spnego_client_negTokenInit_finish,
 };
 
-static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
-{
-	return gensec_spnego_neg_loop(gensec_security, spnego_state,
-				      &gensec_spnego_client_negTokenInit_ops,
-				      ev, spnego_in, out_mem_ctx, out);
-}
-
 static NTSTATUS gensec_spnego_client_negTokenTarg_start(
 					struct gensec_security *gensec_security,
 					struct spnego_state *spnego_state,
@@ -1166,17 +1084,6 @@ static const struct spnego_neg_ops gensec_spnego_client_negTokenTarg_ops = {
 	.finish_fn = gensec_spnego_client_negTokenTarg_finish,
 };
 
-static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
-{
-	return gensec_spnego_neg_loop(gensec_security, spnego_state,
-				      &gensec_spnego_client_negTokenTarg_ops,
-				      ev, spnego_in, out_mem_ctx, out);
-}
 /** create a server negTokenTarg 
  *
  * This is the case, where the client is the first one who sends data
@@ -1440,18 +1347,6 @@ static const struct spnego_neg_ops gensec_spnego_server_negTokenInit_ops = {
 	.finish_fn = gensec_spnego_server_negTokenInit_finish,
 };
 
-static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
-{
-	return gensec_spnego_neg_loop(gensec_security, spnego_state,
-				      &gensec_spnego_server_negTokenInit_ops,
-				      ev, spnego_in, out_mem_ctx, out);
-}
-
 static NTSTATUS gensec_spnego_server_negTokenTarg_start(
 					struct gensec_security *gensec_security,
 					struct spnego_state *spnego_state,
@@ -1633,18 +1528,6 @@ static const struct spnego_neg_ops gensec_spnego_server_negTokenTarg_ops = {
 	.finish_fn = gensec_spnego_server_negTokenTarg_finish,
 };
 
-static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec_security,
-						  struct spnego_state *spnego_state,
-						  struct tevent_context *ev,
-						  struct spnego_data *spnego_in,
-						  TALLOC_CTX *out_mem_ctx,
-						  DATA_BLOB *out)
-{
-	return gensec_spnego_neg_loop(gensec_security, spnego_state,
-				      &gensec_spnego_server_negTokenTarg_ops,
-				      ev, spnego_in, out_mem_ctx, out);
-}
-
 struct gensec_spnego_update_state {
 	struct tevent_context *ev;
 	struct gensec_security *gensec;
@@ -1661,6 +1544,8 @@ struct gensec_spnego_update_state {
 		DATA_BLOB out;
 	} sub;
 
+	struct spnego_neg_state *n;
+
 	NTSTATUS status;
 	DATA_BLOB out;
 };
@@ -1965,9 +1850,8 @@ static void gensec_spnego_update_pre(struct tevent_req *req)
 	struct gensec_spnego_update_state *state =
 		tevent_req_data(req,
 		struct gensec_spnego_update_state);
-	struct gensec_security *gensec_security = state->gensec;
 	struct spnego_state *spnego_state = state->spnego;
-	struct tevent_context *ev = state->ev;
+	const struct spnego_neg_ops *ops = NULL;
 	NTSTATUS status;
 
 	state->sub.needed = false;
@@ -1986,65 +1870,29 @@ static void gensec_spnego_update_pre(struct tevent_req *req)
 	case SPNEGO_CLIENT_START:
 		if (state->spnego_in == NULL) {
 			/* client to produce negTokenInit */
-			status = gensec_spnego_create_negTokenInit(gensec_security,
-							spnego_state, state, ev,
-							&spnego_state->out_frag);
-			if (GENSEC_UPDATE_IS_NTERROR(status)) {
-				tevent_req_nterror(req, status);
-				return;
-			}
+			ops = &gensec_spnego_create_negTokenInit_ops;
 			break;
 		}
 
-		status = gensec_spnego_client_negTokenInit(gensec_security,
-							spnego_state, ev,
-							state->spnego_in, state,
-							&spnego_state->out_frag);
+		ops = &gensec_spnego_client_negTokenInit_ops;
 		break;
 
 	case SPNEGO_CLIENT_TARG:
-		status = gensec_spnego_client_negTokenTarg(gensec_security,
-							spnego_state, ev,
-							state->spnego_in, state,
-							&spnego_state->out_frag);
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			tevent_req_nterror(req, status);
-			return;
-		}
+		ops = &gensec_spnego_client_negTokenTarg_ops;
 		break;
 
 	case SPNEGO_SERVER_START:
 		if (state->spnego_in == NULL) {
 			/* server to produce negTokenInit */
-			status = gensec_spnego_create_negTokenInit(gensec_security,
-							spnego_state, state, ev,
-							&spnego_state->out_frag);
-			if (GENSEC_UPDATE_IS_NTERROR(status)) {
-				tevent_req_nterror(req, status);
-				return;
-			}
+			ops = &gensec_spnego_create_negTokenInit_ops;
 			break;
 		}
 
-		status = gensec_spnego_server_negTokenInit(gensec_security,
-							spnego_state, ev,
-							state->spnego_in, state,
-							&spnego_state->out_frag);
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			tevent_req_nterror(req, status);
-			return;
-		}
+		ops = &gensec_spnego_server_negTokenInit_ops;
 		break;
 
 	case SPNEGO_SERVER_TARG:
-		status = gensec_spnego_server_negTokenTarg(gensec_security,
-							spnego_state, ev,
-							state->spnego_in, state,
-							&spnego_state->out_frag);
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			tevent_req_nterror(req, status);
-			return;
-		}
+		ops = &gensec_spnego_server_negTokenTarg_ops;
 		break;
 
 	default:
@@ -2052,7 +1900,31 @@ static void gensec_spnego_update_pre(struct tevent_req *req)
 		return;
 	}
 
-	spnego_state->out_status = status;
+	state->n = gensec_spnego_neg_state(state, ops);
+	if (tevent_req_nomem(state->n, req)) {
+		return;
+	}
+
+	status = ops->start_fn(state->gensec, spnego_state, state->n,
+			       state->spnego_in, state, &state->sub.in);
+	if (GENSEC_UPDATE_IS_NTERROR(status)) {
+		tevent_req_nterror(req, status);
+		return;
+	}
+
+	if (NT_STATUS_IS_OK(status)) {
+		/*
+		 * Call finish_fn() with an empty
+		 * blob and NT_STATUS_OK.
+		 */
+		state->sub.status = NT_STATUS_OK;
+	} else {
+		/*
+		 * MORE_PROCESSING_REQUIRED =>
+		 * we need to call gensec_update_send().
+		 */
+		state->sub.needed = true;
+	}
 }
 
 static void gensec_spnego_update_done(struct tevent_req *subreq)
@@ -2080,6 +1952,7 @@ static void gensec_spnego_update_post(struct tevent_req *req)
 		tevent_req_data(req,
 		struct gensec_spnego_update_state);
 	struct spnego_state *spnego_state = state->spnego;
+	const struct spnego_neg_ops *ops = NULL;
 	NTSTATUS status;
 
 	state->sub.in = data_blob_null;
@@ -2093,11 +1966,78 @@ static void gensec_spnego_update_post(struct tevent_req *req)
 		goto respond;
 	}
 
-	/*
-	 * For now just handle the sync processing done
-	 * in gensec_spnego_update_pre()
-	 */
-	status = spnego_state->out_status;
+	ops = state->n->ops;
+
+	if (GENSEC_UPDATE_IS_NTERROR(state->sub.status)) {
+
+
+		/*
+		 * gensec_update_recv() returned an error,
+		 * let's see if the step_fn() want to
+		 * handle it and negotiate something else.
+		 */
+
+		status = ops->step_fn(state->gensec,
+				      spnego_state,
+				      state->n,
+				      state->spnego_in,
+				      state->sub.status,
+				      state,
+				      &state->sub.in);
+		if (GENSEC_UPDATE_IS_NTERROR(status)) {
+			tevent_req_nterror(req, status);
+			return;
+		}
+
+		state->sub.out = data_blob_null;
+		state->sub.status = NT_STATUS_INTERNAL_ERROR;
+
+		if (NT_STATUS_IS_OK(status)) {
+			/*
+			 * Call finish_fn() with an empty
+			 * blob and NT_STATUS_OK.
+			 */
+			state->sub.status = NT_STATUS_OK;
+		} else {
+			/*
+			 * MORE_PROCESSING_REQUIRED...
+			 */
+			state->sub.needed = true;
+		}
+	}
+
+	if (state->sub.needed) {
+		struct tevent_req *subreq = NULL;
+
+		/*
+		 * We may need one more roundtrip...
+		 */
+		subreq = gensec_update_send(state, state->ev,
+					    spnego_state->sub_sec_security,
+					    state->sub.in);
+		if (tevent_req_nomem(subreq, req)) {
+			return;
+		}
+		tevent_req_set_callback(subreq,
+					gensec_spnego_update_done,
+					req);
+		state->sub.needed = false;
+		return;
+	}
+
+	status = ops->finish_fn(state->gensec,
+				spnego_state,
+				state->n,
+				state->spnego_in,
+				state->sub.status,
+				state->sub.out,
+				spnego_state,
+				&spnego_state->out_frag);
+	TALLOC_FREE(state->n);
+	if (GENSEC_UPDATE_IS_NTERROR(status)) {
+		tevent_req_nterror(req, status);
+		return;
+	}
 
 	if (NT_STATUS_IS_OK(status)) {
 		bool reset_full = true;
-- 
1.9.1


From c644ecdd5941de94b4c5a0acfefb478045a65ad1 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 19 Jul 2017 10:53:30 +0200
Subject: [PATCH 07/17] auth/spnego: Rename gensec_spnego_update_sub_abort()

The name is not ideal as someone might think we will panic and abort the
process. So rename it to gensec_spnego_reset_sub_sec().

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/spnego.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 3983cf5..959118e 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -159,7 +159,7 @@ static struct spnego_neg_state *gensec_spnego_neg_state(TALLOC_CTX *mem_ctx,
 	return n;
 }
 
-static void gensec_spnego_update_sub_abort(struct spnego_state *spnego_state)
+static void gensec_spnego_reset_sub_sec(struct spnego_state *spnego_state)
 {
 	spnego_state->sub_sec_ready = false;
 	TALLOC_FREE(spnego_state->sub_sec_security);
@@ -369,7 +369,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit_step(
 		/*
 		 * Pretend we never started it
 		 */
-		gensec_spnego_update_sub_abort(spnego_state);
+		gensec_spnego_reset_sub_sec(spnego_state);
 
 		/*
 		 * And try the next one...
@@ -393,7 +393,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit_step(
 		status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
 						  cur_sec->op);
 		if (!NT_STATUS_IS_OK(status)) {
-			gensec_spnego_update_sub_abort(spnego_state);
+			gensec_spnego_reset_sub_sec(spnego_state);
 			continue;
 		}
 
@@ -597,7 +597,7 @@ static NTSTATUS gensec_spnego_client_negTokenInit_step(
 		/*
 		 * Pretend we never started it.
 		 */
-		gensec_spnego_update_sub_abort(spnego_state);
+		gensec_spnego_reset_sub_sec(spnego_state);
 
 		/*
 		 * And try the next one...
@@ -621,7 +621,7 @@ static NTSTATUS gensec_spnego_client_negTokenInit_step(
 		status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
 						  cur_sec->op);
 		if (!NT_STATUS_IS_OK(status)) {
-			gensec_spnego_update_sub_abort(spnego_state);
+			gensec_spnego_reset_sub_sec(spnego_state);
 			continue;
 		}
 
@@ -756,7 +756,7 @@ static NTSTATUS gensec_spnego_client_negTokenTarg_start(
 			   client_mech, client_oid, server_mech, server_oid);
 
 		spnego_state->downgraded = true;
-		gensec_spnego_update_sub_abort(spnego_state);
+		gensec_spnego_reset_sub_sec(spnego_state);
 
 		status = gensec_subcontext_start(spnego_state,
 						 gensec_security,
@@ -1224,7 +1224,7 @@ static NTSTATUS gensec_spnego_server_negTokenInit_step(
 		/*
 		 * Pretend we never started it
 		 */
-		gensec_spnego_update_sub_abort(spnego_state);
+		gensec_spnego_reset_sub_sec(spnego_state);
 
 		/*
 		 * And try the next one, based on the clients
@@ -1274,7 +1274,7 @@ static NTSTATUS gensec_spnego_server_negTokenInit_step(
 			/*
 			 * Pretend we never started it
 			 */
-			gensec_spnego_update_sub_abort(spnego_state);
+			gensec_spnego_reset_sub_sec(spnego_state);
 			continue;
 		}
 
-- 
1.9.1


From 128b17af7ad7c1832f31c6763831104a30e193a3 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 19 Jul 2017 11:02:39 +0200
Subject: [PATCH 08/17] auth/spnego: Use talloc_get_type_abort() in
 gsensec_spnego_update_in()

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/spnego.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 959118e..3289c67 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1741,7 +1741,9 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
 					const DATA_BLOB in, TALLOC_CTX *mem_ctx,
 					DATA_BLOB *full_in)
 {
-	struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
+	struct spnego_state *spnego_state =
+		talloc_get_type_abort(gensec_security->private_data,
+		struct spnego_state);
 	size_t expected;
 	bool ok;
 
-- 
1.9.1


From 41dfb83cd6eb5c8a4068b1cd766901069f31cc6a Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 19 Jul 2017 11:05:32 +0200
Subject: [PATCH 09/17] auth/spnego: Use talloc_get_type_abort() in
 gsensec_spnego_update_out()

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/spnego.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 3289c67..b00a6a3 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -2072,7 +2072,9 @@ static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security
 					 TALLOC_CTX *out_mem_ctx,
 					 DATA_BLOB *_out)
 {
-	struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
+	struct spnego_state *spnego_state =
+		talloc_get_type_abort(gensec_security->private_data,
+		struct spnego_state);
 	DATA_BLOB out = data_blob_null;
 	bool ok;
 
-- 
1.9.1


From b9a04a80af987f4d5a6083c4e3aaaa6082ee3f64 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 20 Jul 2017 14:16:44 +0200
Subject: [PATCH 10/17] tevent: avoid calling talloc_get_name(NULL) in
 tevent_req_default_print()

We have the same information available under req->internal.private_type.

This way it's possible to call tevent_req_print() after
tevent_req_received() was called.

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

diff --git a/lib/tevent/tevent_req.c b/lib/tevent/tevent_req.c
index 22f7a4f..155746e 100644
--- a/lib/tevent/tevent_req.c
+++ b/lib/tevent/tevent_req.c
@@ -36,7 +36,7 @@ char *tevent_req_default_print(struct tevent_req *req, TALLOC_CTX *mem_ctx)
 			       req->internal.state,
 			       (unsigned long long)req->internal.error,
 			       (unsigned long long)req->internal.error,
-			       talloc_get_name(req->data),
+			       req->internal.private_type,
 			       req->data,
 			       req->internal.timer,
 			       req->internal.finish_location
-- 
1.9.1


From 8fb4181537a33ea3e6a2b12938f6204bc1b93f01 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 20 Jul 2017 14:20:03 +0200
Subject: [PATCH 11/17] tevent: handle passing req = NULL to tevent_req_print()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 lib/tevent/tevent_req.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/tevent/tevent_req.c b/lib/tevent/tevent_req.c
index 155746e..15754d3 100644
--- a/lib/tevent/tevent_req.c
+++ b/lib/tevent/tevent_req.c
@@ -45,6 +45,10 @@ char *tevent_req_default_print(struct tevent_req *req, TALLOC_CTX *mem_ctx)
 
 char *tevent_req_print(TALLOC_CTX *mem_ctx, struct tevent_req *req)
 {
+	if (req == NULL) {
+		return talloc_strdup(mem_ctx, "tevent_req[NULL]");
+	}
+
 	if (!req->private_print) {
 		return tevent_req_default_print(req, mem_ctx);
 	}
-- 
1.9.1


From 3ff26dfe0a8f6dcf4e072b8818642148492b7601 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 20 Jul 2017 15:42:58 +0200
Subject: [PATCH 12/17] auth/gensec: add some useful debugging to
 gensec_update_send/gensec_update_done

This makes it easier to spot problems with all the abstraction and async layers.

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

diff --git a/auth/gensec/gensec.c b/auth/gensec/gensec.c
index 014516f..6569c72 100644
--- a/auth/gensec/gensec.c
+++ b/auth/gensec/gensec.c
@@ -454,6 +454,9 @@ _PUBLIC_ struct tevent_req *gensec_update_send(TALLOC_CTX *mem_ctx,
 	}
 	tevent_req_set_callback(subreq, gensec_update_done, req);
 
+	DBG_DEBUG("%s[%p]: subreq: %p\n", state->ops->name,
+		  state->gensec_security, subreq);
+
 	return req;
 }
 
@@ -484,15 +487,35 @@ static void gensec_update_done(struct tevent_req *subreq)
 		tevent_req_data(req,
 		struct gensec_update_state);
 	NTSTATUS status;
+	const char *debug_subreq = NULL;
+
+	if (CHECK_DEBUGLVL(DBGLVL_DEBUG)) {
+		/*
+		 * We need to call tevent_req_print()
+		 * before calling the _recv function,
+		 * before tevent_req_received() was called.
+		 * in order to print the pointer value of
+		 * the subreq state.
+		 */
+		debug_subreq = tevent_req_print(state, subreq);
+	}
 
 	status = state->ops->update_recv(subreq, state, &state->out);
 	TALLOC_FREE(subreq);
 	state->status = status;
-	if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-		tevent_req_done(req);
+	if (GENSEC_UPDATE_IS_NTERROR(status)) {
+		DBG_INFO("%s[%p]: %s%s%s\n", state->ops->name,
+			 state->gensec_security, nt_errstr(status),
+			 debug_subreq ? " " : "",
+			 debug_subreq ? debug_subreq : "");
+		tevent_req_nterror(req, status);
 		return;
 	}
-	if (tevent_req_nterror(req, status)) {
+	DBG_DEBUG("%s[%p]: %s %s\n", state->ops->name,
+		  state->gensec_security, nt_errstr(status),
+		  debug_subreq);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+		tevent_req_done(req);
 		return;
 	}
 
-- 
1.9.1


From 264d0f8b5bfbc061f7bdf1d41d1594002011d2af Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 20 Jul 2017 23:28:51 +0200
Subject: [PATCH 13/17] auth/gensec: introduce gensec_security_ops.glue in
 order to avoid depending on GENSEC_OID_SPNEGO being special

In future we have get more backends that can negotiate other backends,
we should keep all of them even if we require kerberos.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/gensec_internal.h |  1 +
 auth/gensec/gensec_start.c    | 11 ++++-------
 auth/gensec/spnego.c          |  3 ++-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/auth/gensec/gensec_internal.h b/auth/gensec/gensec_internal.h
index c73be11..911b48b 100644
--- a/auth/gensec/gensec_internal.h
+++ b/auth/gensec/gensec_internal.h
@@ -86,6 +86,7 @@ struct gensec_security_ops {
 	bool enabled;
 	bool kerberos;
 	enum gensec_priority priority;
+	bool glue;
 };
 
 struct gensec_security_ops_wrapper {
diff --git a/auth/gensec/gensec_start.c b/auth/gensec/gensec_start.c
index 6a12935..4276620 100644
--- a/auth/gensec/gensec_start.c
+++ b/auth/gensec/gensec_start.c
@@ -98,15 +98,12 @@ _PUBLIC_ const struct gensec_security_ops **gensec_use_kerberos_mechs(TALLOC_CTX
 
 	j = 0;
 	for (i=0; old_gensec_list && old_gensec_list[i]; i++) {
-		int oid_idx;
 		bool keep = false;
 
-		for (oid_idx = 0; old_gensec_list[i]->oid && old_gensec_list[i]->oid[oid_idx]; oid_idx++) {
-			if (strcmp(old_gensec_list[i]->oid[oid_idx], GENSEC_OID_SPNEGO) == 0) {
-				keep = true;
-				break;
-			}
-		}
+		/*
+		 * We want to keep SPNGEO and other backends
+		 */
+		keep = old_gensec_list[i]->glue;
 
 		if (old_gensec_list[i]->auth_type == DCERPC_AUTH_TYPE_SCHANNEL) {
 			keep = keep_schannel;
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index b00a6a3..09f2d742 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -2182,7 +2182,8 @@ static const struct gensec_security_ops gensec_spnego_security_ops = {
 	.expire_time      = gensec_child_expire_time,
 	.final_auth_type  = gensec_child_final_auth_type,
 	.enabled          = true,
-	.priority         = GENSEC_SPNEGO
+	.priority         = GENSEC_SPNEGO,
+	.glue             = true,
 };
 
 _PUBLIC_ NTSTATUS gensec_spnego_init(TALLOC_CTX *ctx)
-- 
1.9.1


From 02d6ca5cb79bd77a756510d5fa08dbc52162def7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 11 May 2017 15:34:08 +0200
Subject: [PATCH 14/17] s4:lib/http: rewrite http_send_auth_request_*() using
 gensec_update_send/recv

The new logic makes it much clearer that we have a loop of

gensec_update_send()
gensec_update_recv()
http_send_request_send()
http_send_request_recv()
http_read_response_send()
http_read_response_recv()

Until the local gensec and the server are ready.

I've tested this against Windows 2008R2 like this:

bin/smbtorture \
  -W BLA --realm=BLA.BASE \
  -s /dev/null -Uadministrator%A1b2C3d4 \
  ncacn_http:w2k8r2-219[593,RpcProxy=w2k8r2-219.bla.base,HttpUseTls=false,HttpAuthOption=basic] \
  rpc.epmapper.epmapper.Lookup_simple \

and:

bin/smbtorture \
  -W BLA --realm=BLA.BASE \
  -s /dev/null -Uadministrator%A1b2C3d4 \
  ncacn_http:w2k8r2-219[593,RpcProxy=w2k8r2-219.bla.base,HttpUseTls=false,HttpAuthOption=ntlm] \
  rpc.epmapper.epmapper.Lookup_simple \

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/lib/http/http.h      |   2 +-
 source4/lib/http/http_auth.c | 337 +++++++++++++++++++------------------------
 2 files changed, 153 insertions(+), 186 deletions(-)

diff --git a/source4/lib/http/http.h b/source4/lib/http/http.h
index 75a04b1..a601a03 100644
--- a/source4/lib/http/http.h
+++ b/source4/lib/http/http.h
@@ -111,7 +111,7 @@ struct tevent_req *http_send_auth_request_send(TALLOC_CTX *,
 					       struct tevent_context *,
 					       struct tstream_context *,
 					       struct tevent_queue *,
-					       struct http_request *,
+					       const struct http_request *,
 					       struct cli_credentials *,
 					       struct loadparm_context *,
 					       enum http_auth_method);
diff --git a/source4/lib/http/http_auth.c b/source4/lib/http/http_auth.c
index b6f102f..3d2148e 100644
--- a/source4/lib/http/http_auth.c
+++ b/source4/lib/http/http_auth.c
@@ -32,7 +32,7 @@
 /**
  * Copy the request headers from src to dst
  */
-static NTSTATUS http_copy_header(struct http_request *src,
+static NTSTATUS http_copy_header(const struct http_request *src,
 				 struct http_request *dst)
 {
 	struct http_header *h;
@@ -78,278 +78,245 @@ static NTSTATUS http_parse_auth_response(enum http_auth_method auth,
 	return NT_STATUS_NOT_SUPPORTED;
 }
 
-/*
- * Create the next authentication request to send to server if authentication
- * is not completed. If it is completed, attachs the 'Authorization' header
- * to the original request.
- */
-static NTSTATUS http_create_auth_request(TALLOC_CTX *mem_ctx,
-					 struct gensec_security *gensec_ctx,
-					 struct tevent_context *ev,
-					 enum http_auth_method auth,
-					 struct http_request *original_request,
-					 struct http_request *auth_response,
-					 struct http_request **auth_request)
-{
-	NTSTATUS status;
-	DATA_BLOB in, out;
-
-	if (auth_response) {
-		status = http_parse_auth_response(auth, auth_response, &in);
-		if (!NT_STATUS_IS_OK(status)) {
-			return status;
-		}
-	} else {
-		in = data_blob_null;
-	}
-
-	status = gensec_update_ev(gensec_ctx, mem_ctx, ev, in, &out);
-	if (NT_STATUS_IS_OK(status)) {
-		if (out.length) {
-			http_add_header(original_request,
-					&original_request->headers,
-					"Authorization", (char*)out.data);
-		}
-	}
+struct http_auth_state {
+	struct tevent_context *ev;
 
-	if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-		NTSTATUS status2;
-
-		*auth_request = talloc_zero(mem_ctx, struct http_request);
-		if (*auth_request == NULL) {
-			return NT_STATUS_NO_MEMORY;
-		}
+	struct tstream_context *stream;
+	struct tevent_queue *send_queue;
 
-		status2 = http_copy_header(original_request, *auth_request);
-		if (!NT_STATUS_IS_OK(status2)) {
-			talloc_free(*auth_request);
-			return status2;
-		}
-
-		http_replace_header(*auth_request, &((*auth_request)->headers),
-				    "Content-Length", "0");
-		if (out.length) {
-			http_add_header(*auth_request,
-					&((*auth_request)->headers),
-					"Authorization", (char*)out.data);
-		}
-	}
+	enum http_auth_method auth;
 
-	return status;
-}
+	struct gensec_security *gensec_ctx;
+	NTSTATUS gensec_status;
 
-struct http_auth_state
-{
-	struct loadparm_context	*lp_ctx;
-	struct tevent_context	*ev;
-	struct tstream_context	*stream;
-	struct tevent_queue	*send_queue;
-	struct cli_credentials  *credentials;
-	struct http_request	*original_request;
-	struct gensec_security	*gensec_ctx;
-	NTSTATUS		gensec_status;
-	enum http_auth_method	auth;
-
-	int			sys_errno;
-	int			nwritten;
+	const struct http_request *original_request;
+	struct http_request *next_request;
+	struct http_request *auth_response;
 };
 
 
-static void http_send_auth_request_done(struct tevent_req *);
+static void http_send_auth_request_gensec_done(struct tevent_req *subreq);
+static void http_send_auth_request_http_req_done(struct tevent_req *subreq);
+static void http_send_auth_request_http_rep_done(struct tevent_req *subreq);
+
 struct tevent_req *http_send_auth_request_send(TALLOC_CTX *mem_ctx,
 					       struct tevent_context *ev,
 					       struct tstream_context *stream,
 					       struct tevent_queue *send_queue,
-					       struct http_request *original_request,
+					       const struct http_request *original_request,
 					       struct cli_credentials *credentials,
 					       struct loadparm_context *lp_ctx,
 					       enum http_auth_method auth)
 {
-	struct tevent_req *req;
-	struct tevent_req *subreq;
-	struct http_auth_state *state;
+	struct tevent_req *req = NULL;
+	struct http_auth_state *state = NULL;
+	struct tevent_req *subreq = NULL;
+	DATA_BLOB gensec_in = data_blob_null;
 	NTSTATUS status;
-	struct http_request *auth_request = NULL;
-	struct http_request *request_to_send;
+	const char *mech_name = NULL;
 
 	req = tevent_req_create(mem_ctx, &state, struct http_auth_state);
 	if (req == NULL) {
 		return NULL;
 	}
-
 	state->ev = ev;
 	state->stream = stream;
 	state->send_queue = send_queue;
-	state->original_request = original_request;
-	state->credentials = credentials;
-	state->lp_ctx = lp_ctx;
 	state->auth = auth;
+	state->original_request = original_request;
 
 	status = gensec_init();
-	if (!NT_STATUS_IS_OK(status)) {
-		goto post_status;
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
 	}
+
 	status = gensec_client_start(state, &state->gensec_ctx,
 			             lpcfg_gensec_settings(state, lp_ctx));
-	if (!NT_STATUS_IS_OK(status)) {
-		goto post_status;
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
 	}
+
 	status = gensec_set_credentials(state->gensec_ctx, credentials);
-	if (!NT_STATUS_IS_OK(status)) {
-		goto post_status;
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
 	}
 
 	switch (state->auth) {
 	case HTTP_AUTH_BASIC:
-		status = gensec_start_mech_by_name(state->gensec_ctx,
-						   "http_basic");
-		if (!NT_STATUS_IS_OK(status)) {
-			goto post_status;
-		}
+		mech_name = "http_basic";
 		break;
 	case HTTP_AUTH_NTLM:
-		status = gensec_start_mech_by_name(state->gensec_ctx,
-						   "http_ntlm");
-		if (!NT_STATUS_IS_OK(status)) {
-			goto post_status;
-		}
+		mech_name = "http_ntlm";
 		break;
 	default:
 		tevent_req_nterror(req, NT_STATUS_NOT_SUPPORTED);
 		return tevent_req_post(req, ev);
 	}
 
-	/*
-	 * Store the gensec status to read the server response on callback
-	 * if more processing is required
-	*/
-	state->gensec_status = http_create_auth_request(state,
-							state->gensec_ctx,
-							state->ev,
-							state->auth,
-							state->original_request,
-							NULL,
-							&auth_request);
-	if (!NT_STATUS_IS_OK(state->gensec_status) &&
-	    !NT_STATUS_EQUAL(state->gensec_status,
-			     NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-		goto post_status;
+	status = gensec_start_mech_by_name(state->gensec_ctx, mech_name);
+	if (tevent_req_nterror(req, status)) {
+		return tevent_req_post(req, ev);
 	}
 
-	/*
-	 * If no more processing is necessary, the http_create_auth_request
-	 * function will attach the authentication header to the original
-	 * request
-	 */
-	request_to_send = NT_STATUS_IS_OK(state->gensec_status) ?
-				state->original_request : auth_request;
-
-	subreq = http_send_request_send(state, ev, stream, send_queue,
-					request_to_send);
+	subreq = gensec_update_send(state, state->ev,
+				    state->gensec_ctx,
+				    gensec_in);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
-	tevent_req_set_callback(subreq, http_send_auth_request_done, req);
+	tevent_req_set_callback(subreq, http_send_auth_request_gensec_done, req);
+
 	return req;
-post_status:
-	tevent_req_nterror(req, status);
-	return tevent_req_post(req, ev);
 }
 
-static void http_send_auth_request_done2(struct tevent_req *subreq);
-static void http_send_auth_request_done(struct tevent_req *subreq)
+static void http_send_auth_request_gensec_done(struct tevent_req *subreq)
 {
-	NTSTATUS		status;
-	struct tevent_req	*req;
-	struct http_auth_state	*state;
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct http_auth_state *state =
+		tevent_req_data(req,
+		struct http_auth_state);
+	DATA_BLOB gensec_out = data_blob_null;
+	NTSTATUS status;
+	int ret;
 
-	req = tevent_req_callback_data(subreq, struct tevent_req);
-	state = tevent_req_data(req, struct http_auth_state);
+	TALLOC_FREE(state->auth_response);
 
-	status = http_send_request_recv(subreq);
+	status = gensec_update_recv(subreq, state, &gensec_out);
 	TALLOC_FREE(subreq);
+	state->gensec_status = status;
+	if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+		status = NT_STATUS_OK;
+	}
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 
-	/* If no more processing required, it is done */
-	if (NT_STATUS_IS_OK(state->gensec_status)) {
-		tevent_req_done(req);
+	state->next_request = talloc_zero(state, struct http_request);
+	if (tevent_req_nomem(state->next_request, req)) {
 		return;
 	}
 
-	/* If more processing required, read the response from server */
-	if (NT_STATUS_EQUAL(state->gensec_status,
-			    NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-		subreq = http_read_response_send(state, state->ev,
-						 state->stream);
-		if (tevent_req_nomem(subreq, req)) {
+	status = http_copy_header(state->original_request, state->next_request);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+
+	if (!NT_STATUS_IS_OK(state->gensec_status)) {
+		/*
+		 * More preprocessing required before we
+		 * can include the content.
+		 */
+		ret = http_replace_header(state->next_request,
+					  &state->next_request->headers,
+					  "Content-Length", "0");
+		if (ret != 0) {
+			tevent_req_oom(req);
+			return;
+		}
+	} else {
+		state->next_request->body = state->original_request->body;
+	}
+
+	if (gensec_out.length > 0) {
+		ret = http_add_header(state->next_request,
+				      &state->next_request->headers,
+				      "Authorization",
+				      (char *)gensec_out.data);
+		if (ret != 0) {
+			tevent_req_oom(req);
 			return;
 		}
-		tevent_req_set_callback(subreq, http_send_auth_request_done2,
-					req);
+		data_blob_free(&gensec_out);
+	}
+
+	subreq = http_send_request_send(state, state->ev,
+					state->stream,
+					state->send_queue,
+					state->next_request);
+	if (tevent_req_nomem(subreq, req)) {
+		return;
+	}
+	tevent_req_set_callback(subreq,
+				http_send_auth_request_http_req_done,
+				req);
+}
+
+static void http_send_auth_request_http_req_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct http_auth_state *state =
+		tevent_req_data(req,
+		struct http_auth_state);
+	NTSTATUS status;
+
+	TALLOC_FREE(state->next_request);
+
+	status = http_send_request_recv(subreq);
+	TALLOC_FREE(subreq);
+	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 
 	/*
-	 * If gensec status is not NT_STATUS_OK neither
-	 * NT_STATUS_MORE_PROCESSING_REQUIRED , it is an error
+	 * If no more processing required, it is done
+	 *
+	 * The caller will use http_read_response_send/recv
+	 * in order to get the high level response.
 	 */
-	tevent_req_nterror(req, state->gensec_status);
+	if (NT_STATUS_IS_OK(state->gensec_status)) {
+		tevent_req_done(req);
+		return;
+	}
+
+	/* If more processing required, read the response from server */
+	subreq = http_read_response_send(state, state->ev,
+					 state->stream);
+	if (tevent_req_nomem(subreq, req)) {
+		return;
+	}
+	tevent_req_set_callback(subreq,
+				http_send_auth_request_http_rep_done,
+				req);
 }
 
-static void http_send_auth_request_done2(struct tevent_req *subreq)
+static void http_send_auth_request_http_rep_done(struct tevent_req *subreq)
 {
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct http_auth_state *state =
+		tevent_req_data(req,
+		struct http_auth_state);
+	DATA_BLOB gensec_in = data_blob_null;
 	NTSTATUS status;
-	struct tevent_req	*req;
-	struct http_auth_state	*state;
-	struct http_request *auth_response;
-	struct http_request *auth_request = NULL;
-	struct http_request *request_to_send;
 
-	req = tevent_req_callback_data(subreq, struct tevent_req);
-	state = tevent_req_data(req, struct http_auth_state);
-
-	status = http_read_response_recv(subreq, state, &auth_response);
+	status = http_read_response_recv(subreq, state,
+					 &state->auth_response);
 	TALLOC_FREE(subreq);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 
-	state->gensec_status = http_create_auth_request(state,
-							state->gensec_ctx,
-							state->ev,
-							state->auth,
-							state->original_request,
-							auth_response,
-							&auth_request);
-	if (!NT_STATUS_IS_OK(state->gensec_status) &&
-	    !NT_STATUS_EQUAL(state->gensec_status,
-			     NT_STATUS_MORE_PROCESSING_REQUIRED)) {
-		tevent_req_nterror(req, status);
+	status = http_parse_auth_response(state->auth,
+					  state->auth_response,
+					  &gensec_in);
+	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 
-	/*
-	 * If no more processing is necessary, the http_create_auth_request
-	 * function will attach the authentication header to the original
-	 * request
-	 */
-	request_to_send = NT_STATUS_IS_OK(state->gensec_status) ?
-				state->original_request : auth_request;
-
-	subreq = http_send_request_send(state,
-					state->ev,
-					state->stream,
-					state->send_queue,
-					request_to_send);
+	subreq = gensec_update_send(state, state->ev,
+				    state->gensec_ctx,
+				    gensec_in);
 	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
-	tevent_req_set_callback(subreq, http_send_auth_request_done, req);
+	tevent_req_set_callback(subreq, http_send_auth_request_gensec_done, req);
 }
 
-
 NTSTATUS http_send_auth_request_recv(struct tevent_req *req)
 {
 	NTSTATUS status;
-- 
1.9.1


From ecafe0f0a5fa34b8584620cf87494410e0feb669 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 11 May 2017 13:16:16 +0200
Subject: [PATCH 15/17] auth/gensec: make use of gensec_update_send/recv in
 gensec_update_ev()

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

diff --git a/auth/gensec/gensec.c b/auth/gensec/gensec.c
index 6569c72..6a747ca 100644
--- a/auth/gensec/gensec.c
+++ b/auth/gensec/gensec.c
@@ -325,15 +325,10 @@ _PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
 				   const DATA_BLOB in, DATA_BLOB *out)
 {
 	NTSTATUS status;
-	const struct gensec_security_ops *ops = gensec_security->ops;
 	TALLOC_CTX *frame = NULL;
 	struct tevent_req *subreq = NULL;
 	bool ok;
 
-	if (gensec_security->child_security != NULL) {
-		return NT_STATUS_INVALID_PARAMETER;
-	}
-
 	frame = talloc_stackframe();
 
 	if (ev == NULL) {
@@ -350,7 +345,7 @@ _PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
 		tevent_loop_allow_nesting(ev);
 	}
 
-	subreq = ops->update_send(frame, ev, gensec_security, in);
+	subreq = gensec_update_send(frame, ev, gensec_security, in);
 	if (subreq == NULL) {
 		status = NT_STATUS_NO_MEMORY;
 		goto fail;
@@ -359,20 +354,7 @@ _PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
 	if (!ok) {
 		goto fail;
 	}
-	status = ops->update_recv(subreq, out_mem_ctx, out);
-	if (!NT_STATUS_IS_OK(status)) {
-		goto fail;
-	}
-
-	/*
-	 * Because callers using the
-	 * gensec_start_mech_by_auth_type() never call
-	 * gensec_want_feature(), it isn't sensible for them
-	 * to have to call gensec_have_feature() manually, and
-	 * these are not points of negotiation, but are
-	 * asserted by the client
-	 */
-	status = gensec_verify_features(gensec_security);
+	status = gensec_update_recv(subreq, out_mem_ctx, out);
  fail:
 	TALLOC_FREE(frame);
 	return status;
-- 
1.9.1


From 189e3dba531eab985a4f80b112cc7cd9e151f4da Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 11 May 2017 14:22:27 +0200
Subject: [PATCH 16/17] auth/gensec: don't allow gensec_update[_ev] to be
 called on a subcontext

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

diff --git a/auth/gensec/gensec.c b/auth/gensec/gensec.c
index 6a747ca..f3969b4 100644
--- a/auth/gensec/gensec.c
+++ b/auth/gensec/gensec.c
@@ -329,6 +329,13 @@ _PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
 	struct tevent_req *subreq = NULL;
 	bool ok;
 
+	if (gensec_security->subcontext) {
+		/*
+		 * gensec modules are not allowed to call the sync version.
+		 */
+		return NT_STATUS_INTERNAL_ERROR;
+	}
+
 	frame = talloc_stackframe();
 
 	if (ev == NULL) {
-- 
1.9.1


From b9985e8911eea357c6b52437f4d5391089cb8bef Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 15 Jun 2017 00:05:29 +0200
Subject: [PATCH 17/17] auth/gensec: finally remove unused gensec_update_ev()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 auth/gensec/gensec.c | 56 +++++++++++++++++++++-------------------------------
 auth/gensec/gensec.h |  4 ----
 2 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/auth/gensec/gensec.c b/auth/gensec/gensec.c
index f3969b4..61bff22 100644
--- a/auth/gensec/gensec.c
+++ b/auth/gensec/gensec.c
@@ -319,13 +319,23 @@ static NTSTATUS gensec_verify_features(struct gensec_security *gensec_security)
 	return NT_STATUS_OK;
 }
 
-_PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
-				   TALLOC_CTX *out_mem_ctx,
-				   struct tevent_context *ev,
-				   const DATA_BLOB in, DATA_BLOB *out)
+/**
+ * Next state function for the GENSEC state machine
+ *
+ * @param gensec_security GENSEC State
+ * @param out_mem_ctx The TALLOC_CTX for *out to be allocated on
+ * @param in The request, as a DATA_BLOB
+ * @param out The reply, as an talloc()ed DATA_BLOB, on *out_mem_ctx
+ * @return Error, MORE_PROCESSING_REQUIRED if a reply is sent,
+ *                or NT_STATUS_OK if the user is authenticated.
+ */
+_PUBLIC_ NTSTATUS gensec_update(struct gensec_security *gensec_security,
+				TALLOC_CTX *out_mem_ctx,
+				const DATA_BLOB in, DATA_BLOB *out)
 {
 	NTSTATUS status;
 	TALLOC_CTX *frame = NULL;
+	struct tevent_context *ev = NULL;
 	struct tevent_req *subreq = NULL;
 	bool ok;
 
@@ -338,20 +348,18 @@ _PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
 
 	frame = talloc_stackframe();
 
+	ev = samba_tevent_context_init(frame);
 	if (ev == NULL) {
-		ev = samba_tevent_context_init(frame);
-		if (ev == NULL) {
-			status = NT_STATUS_NO_MEMORY;
-			goto fail;
-		}
-
-		/*
-		 * TODO: remove this hack once the backends
-		 * are fixed.
-		 */
-		tevent_loop_allow_nesting(ev);
+		status = NT_STATUS_NO_MEMORY;
+		goto fail;
 	}
 
+	/*
+	 * TODO: remove this hack once the backends
+	 * are fixed.
+	 */
+	tevent_loop_allow_nesting(ev);
+
 	subreq = gensec_update_send(frame, ev, gensec_security, in);
 	if (subreq == NULL) {
 		status = NT_STATUS_NO_MEMORY;
@@ -367,24 +375,6 @@ _PUBLIC_ NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
 	return status;
 }
 
-/**
- * Next state function for the GENSEC state machine
- *
- * @param gensec_security GENSEC State
- * @param out_mem_ctx The TALLOC_CTX for *out to be allocated on
- * @param in The request, as a DATA_BLOB
- * @param out The reply, as an talloc()ed DATA_BLOB, on *out_mem_ctx
- * @return Error, MORE_PROCESSING_REQUIRED if a reply is sent,
- *                or NT_STATUS_OK if the user is authenticated.
- */
-
-_PUBLIC_ NTSTATUS gensec_update(struct gensec_security *gensec_security,
-				TALLOC_CTX *out_mem_ctx,
-				const DATA_BLOB in, DATA_BLOB *out)
-{
-	return gensec_update_ev(gensec_security, out_mem_ctx, NULL, in, out);
-}
-
 struct gensec_update_state {
 	const struct gensec_security_ops *ops;
 	struct gensec_security *gensec_security;
diff --git a/auth/gensec/gensec.h b/auth/gensec/gensec.h
index a466f27..d424067 100644
--- a/auth/gensec/gensec.h
+++ b/auth/gensec/gensec.h
@@ -138,10 +138,6 @@ size_t gensec_max_update_size(struct gensec_security *gensec_security);
 NTSTATUS gensec_update(struct gensec_security *gensec_security,
 		       TALLOC_CTX *out_mem_ctx,
 		       const DATA_BLOB in, DATA_BLOB *out);
-NTSTATUS gensec_update_ev(struct gensec_security *gensec_security,
-			  TALLOC_CTX *out_mem_ctx,
-			  struct tevent_context *ev,
-			  const DATA_BLOB in, DATA_BLOB *out);
 struct tevent_req *gensec_update_send(TALLOC_CTX *mem_ctx,
 				      struct tevent_context *ev,
 				      struct gensec_security *gensec_security,
-- 
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/20170721/bb33810a/signature-0001.sig>


More information about the samba-technical mailing list