[Patches] The way to remove gensec_update_ev()

Stefan Metzmacher metze at samba.org
Fri Jun 16 23:23:40 UTC 2017


Am 15.06.2017 um 05:11 schrieb Andrew Bartlett:
> On Wed, 2017-06-14 at 17:39 +0200, Stefan Metzmacher wrote:
>> Hi,
>>
>> just for the record:
>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
>> master3-gensec

Here have basic support for NTLMSSP for trusts.

>> Contains an async version of spnego.c, it already passed:
>> make -j test FAIL_IMMEDIATELY=1 SOCKET_WRAPPER_KEEP_PCAP=1
>> TESTS="spnego"
>>
>> I'm running private autobuilds now...
>>
>> Then I just need to put the 128 commits into some
>> more useful once:-)
> 
> Nice :-)
> 
> BTW I've pushed master3-gensec-ok to autobuild with my review.

The master3-gensec-ok branch has the attached patches
ready for master.

Please review and push:-)

Thanks!
metze
-------------- next part --------------
From cb9a8abb8d39def8438c3ef616d75c536ad8985f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 16:21:56 +0200
Subject: [PATCH 01/10] auth/gensec: clear the update_busy_ptr in
 gensec_subcontext_start()

This is required to support async subcontexts.

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

diff --git a/auth/gensec/gensec_start.c b/auth/gensec/gensec_start.c
index c1affd62..6a12935 100644
--- a/auth/gensec/gensec_start.c
+++ b/auth/gensec/gensec_start.c
@@ -616,6 +616,7 @@ _PUBLIC_ NTSTATUS gensec_subcontext_start(TALLOC_CTX *mem_ctx,
 	(**gensec_security) = *parent;
 	(*gensec_security)->ops = NULL;
 	(*gensec_security)->private_data = NULL;
+	(*gensec_security)->update_busy_ptr = NULL;
 
 	(*gensec_security)->subcontext = true;
 	(*gensec_security)->want_features = parent->want_features;
-- 
1.9.1


From 00771d1258a87640ebe42f63ebdd5394767b9bf2 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 10:39:26 +0200
Subject: [PATCH 02/10] auth/gensec: add GENSEC_UPDATE_IS_NTERROR() helper
 macro

This allows us to write clearer code that
checks for NT_STATUS_OK and NT_STATUS_MORE_PROCESSING_REQUIRED.

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

diff --git a/auth/gensec/gensec.h b/auth/gensec/gensec.h
index da3e0fd..a466f27 100644
--- a/auth/gensec/gensec.h
+++ b/auth/gensec/gensec.h
@@ -147,6 +147,12 @@ struct tevent_req *gensec_update_send(TALLOC_CTX *mem_ctx,
 				      struct gensec_security *gensec_security,
 				      const DATA_BLOB in);
 NTSTATUS gensec_update_recv(struct tevent_req *req, TALLOC_CTX *out_mem_ctx, DATA_BLOB *out);
+
+#define GENSEC_UPDATE_IS_NTERROR(status) ( \
+	!NT_STATUS_IS_OK(status) && \
+	!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) \
+	)
+
 /**
  * @brief Ask for features for a following authentication
  *
-- 
1.9.1


From 4ab80bee6feb3972f3b25fb6c706014db5b1a453 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 16 May 2017 00:01:07 +0200
Subject: [PATCH 03/10] s4:libcli/smb_composite: simplify gensec_update_ev()
 handling in session_setup_spnego()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 9f989f2..7f748c1 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -519,19 +519,22 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 		return status;
 	}
 
+	state->setup.spnego.out.secblob =
+			session->transport->negotiate.secblob;
 	if (session->transport->negotiate.secblob.length) {
 		chosen_oid = GENSEC_OID_SPNEGO;
 		status = gensec_start_mech_by_oid(session->gensec, chosen_oid);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(1, ("Failed to start set GENSEC client mechanism %s: %s\n",
 				  gensec_get_name_by_oid(session->gensec, chosen_oid), nt_errstr(status)));
+			state->setup.spnego.out.secblob = data_blob_null;
 			chosen_oid = GENSEC_OID_NTLMSSP;
 			status = gensec_start_mech_by_oid(session->gensec, chosen_oid);
 			if (!NT_STATUS_IS_OK(status)) {
 				DEBUG(1, ("Failed to start set (fallback) GENSEC client mechanism %s: %s\n",
 					  gensec_get_name_by_oid(session->gensec, chosen_oid), 
 					  nt_errstr(status)));
-			return status;
+				return status;
 			}
 		}
 	} else {
@@ -544,18 +547,10 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 		}
 	}
 
-	if (strequal(chosen_oid, GENSEC_OID_SPNEGO)) {
-		status = gensec_update_ev(session->gensec, state,
-				       c->event_ctx,
-				       session->transport->negotiate.secblob,
-				       &state->setup.spnego.in.secblob);
-	} else {
-		status = gensec_update_ev(session->gensec, state,
-				       c->event_ctx,
-				       data_blob(NULL, 0),
-				       &state->setup.spnego.in.secblob);
-
-	}
+	status = gensec_update_ev(session->gensec, state,
+				  c->event_ctx,
+				  state->setup.spnego.out.secblob,
+				  &state->setup.spnego.in.secblob);
 
 	if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
 	    !NT_STATUS_IS_OK(status)) {
-- 
1.9.1


From d8c6bb348a45d1e277eee8520d4003b581688f44 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 23:24:10 +0200
Subject: [PATCH 04/10] s4:libcli/smb_composite: move chosen_oid to
 state->chosen_oid

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 7f748c1..256d12b 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -35,6 +35,7 @@
 
 struct sesssetup_state {
 	union smb_sesssetup setup;
+	const char *chosen_oid;
 	NTSTATUS remote_status;
 	NTSTATUS gensec_status;
 	struct smb_composite_sesssetup *io;
@@ -522,28 +523,36 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 	state->setup.spnego.out.secblob =
 			session->transport->negotiate.secblob;
 	if (session->transport->negotiate.secblob.length) {
-		chosen_oid = GENSEC_OID_SPNEGO;
-		status = gensec_start_mech_by_oid(session->gensec, chosen_oid);
+		state->chosen_oid = GENSEC_OID_SPNEGO;
+		status = gensec_start_mech_by_oid(session->gensec,
+						  state->chosen_oid);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(1, ("Failed to start set GENSEC client mechanism %s: %s\n",
-				  gensec_get_name_by_oid(session->gensec, chosen_oid), nt_errstr(status)));
+				  gensec_get_name_by_oid(session->gensec,
+							 state->chosen_oid),
+				  nt_errstr(status)));
 			state->setup.spnego.out.secblob = data_blob_null;
-			chosen_oid = GENSEC_OID_NTLMSSP;
-			status = gensec_start_mech_by_oid(session->gensec, chosen_oid);
+			state->chosen_oid = GENSEC_OID_NTLMSSP;
+			status = gensec_start_mech_by_oid(session->gensec,
+							  state->chosen_oid);
 			if (!NT_STATUS_IS_OK(status)) {
 				DEBUG(1, ("Failed to start set (fallback) GENSEC client mechanism %s: %s\n",
-					  gensec_get_name_by_oid(session->gensec, chosen_oid), 
+					  gensec_get_name_by_oid(session->gensec,
+								 state->chosen_oid),
 					  nt_errstr(status)));
 				return status;
 			}
 		}
 	} else {
 		/* without a sec blob, means raw NTLMSSP */
-		chosen_oid = GENSEC_OID_NTLMSSP;
-		status = gensec_start_mech_by_oid(session->gensec, chosen_oid);
+		state->chosen_oid = GENSEC_OID_NTLMSSP;
+		status = gensec_start_mech_by_oid(session->gensec,
+						  state->chosen_oid);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(1, ("Failed to start set GENSEC client mechanism %s: %s\n",
-				  gensec_get_name_by_oid(session->gensec, chosen_oid), nt_errstr(status)));
+				  gensec_get_name_by_oid(session->gensec,
+							 state->chosen_oid),
+				  nt_errstr(status)));
 		}
 	}
 
@@ -555,7 +564,8 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 	if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
 	    !NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
-			  gensec_get_name_by_oid(session->gensec, chosen_oid), 
+			  gensec_get_name_by_oid(session->gensec,
+						 state->chosen_oid),
 			  nt_errstr(status)));
 		return status;
 	}
-- 
1.9.1


From a9adb74eb0d9bb1655e490191f70250ff74dab41 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 16 May 2017 00:10:33 +0200
Subject: [PATCH 05/10] s4:libcli/smb_composite: split out
 session_setup_spnego_restart() from session_setup_spnego()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 56 +++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 256d12b..5e649e4 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -466,28 +466,12 @@ static NTSTATUS session_setup_old(struct composite_context *c,
 	return (*req)->status;
 }
 
-
-/*
-  Modern, all singing, all dancing extended security (and possibly SPNEGO) request
-*/
-static NTSTATUS session_setup_spnego(struct composite_context *c,
-				     struct smbcli_session *session, 
-				     struct smb_composite_sesssetup *io,
-				     struct smbcli_request **req) 
+static NTSTATUS session_setup_spnego_restart(struct composite_context *c,
+					     struct smbcli_session *session,
+					     struct smb_composite_sesssetup *io)
 {
 	struct sesssetup_state *state = talloc_get_type(c->private_data, struct sesssetup_state);
 	NTSTATUS status;
-	const char *chosen_oid = NULL;
-
-	state->setup.spnego.level           = RAW_SESSSETUP_SPNEGO;
-	state->setup.spnego.in.bufsize      = session->transport->options.max_xmit;
-	state->setup.spnego.in.mpx_max      = session->transport->options.max_mux;
-	state->setup.spnego.in.vc_num       = 1;
-	state->setup.spnego.in.sesskey      = io->in.sesskey;
-	state->setup.spnego.in.capabilities = io->in.capabilities;
-	state->setup.spnego.in.os           = "Unix";
-	state->setup.spnego.in.lanman       = talloc_asprintf(state, "Samba %s", SAMBA_VERSION_STRING);
-	state->setup.spnego.in.workgroup    = io->in.workgroup;
 
 	status = gensec_client_start(session, &session->gensec,
 				     io->in.gensec_settings);
@@ -553,9 +537,43 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 				  gensec_get_name_by_oid(session->gensec,
 							 state->chosen_oid),
 				  nt_errstr(status)));
+			return status;
 		}
 	}
 
+	state->gensec_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+	state->remote_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+	return NT_STATUS_OK;
+}
+
+/*
+  Modern, all singing, all dancing extended security (and possibly SPNEGO) request
+*/
+static NTSTATUS session_setup_spnego(struct composite_context *c,
+				     struct smbcli_session *session,
+				     struct smb_composite_sesssetup *io,
+				     struct smbcli_request **req)
+{
+	struct sesssetup_state *state = talloc_get_type(c->private_data, struct sesssetup_state);
+	NTSTATUS status;
+
+	status = session_setup_spnego_restart(c, session, io);
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(1, ("session_setup_spnego_restart() failed: %s\n",
+			  nt_errstr(status)));
+		return status;
+	}
+
+	state->setup.spnego.level           = RAW_SESSSETUP_SPNEGO;
+	state->setup.spnego.in.bufsize      = session->transport->options.max_xmit;
+	state->setup.spnego.in.mpx_max      = session->transport->options.max_mux;
+	state->setup.spnego.in.vc_num       = 1;
+	state->setup.spnego.in.sesskey      = io->in.sesskey;
+	state->setup.spnego.in.capabilities = io->in.capabilities;
+	state->setup.spnego.in.os           = "Unix";
+	state->setup.spnego.in.lanman       = talloc_asprintf(state, "Samba %s", SAMBA_VERSION_STRING);
+	state->setup.spnego.in.workgroup    = io->in.workgroup;
+
 	status = gensec_update_ev(session->gensec, state,
 				  c->event_ctx,
 				  state->setup.spnego.out.secblob,
-- 
1.9.1


From af96cb94b69473e2f6bfdbea580b0b0eede0796b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 16 May 2017 00:16:14 +0200
Subject: [PATCH 06/10] s4:libcli/smb_composite: move
 session_setup_spnego_restart() to the callers of session_setup_spnego()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 5e649e4..6d64629 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -61,6 +61,9 @@ static NTSTATUS session_setup_nt1(struct composite_context *c,
 				  struct smbcli_session *session, 
 				  struct smb_composite_sesssetup *io,
 				  struct smbcli_request **req); 
+static NTSTATUS session_setup_spnego_restart(struct composite_context *c,
+					     struct smbcli_session *session,
+					     struct smb_composite_sesssetup *io);
 static NTSTATUS session_setup_spnego(struct composite_context *c,
 				     struct smbcli_session *session, 
 				     struct smb_composite_sesssetup *io,
@@ -161,6 +164,15 @@ static void request_handler(struct smbcli_request *req)
 			}
 			if (cli_credentials_failed_kerberos_login(state->io->in.credentials, principal, &state->logon_retries) ||
 			    cli_credentials_wrong_password(state->io->in.credentials)) {
+				nt_status = session_setup_spnego_restart(c, session, state->io);
+				if (!NT_STATUS_IS_OK(nt_status)) {
+					DEBUG(1, ("session_setup_spnego_restart() failed: %s\n",
+						  nt_errstr(nt_status)));
+					c->status = nt_status;
+					composite_error(c, c->status);
+					return;
+				}
+
 				nt_status = session_setup_spnego(c, session, 
 								      state->io, 
 								      &state->req);
@@ -557,13 +569,6 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 	struct sesssetup_state *state = talloc_get_type(c->private_data, struct sesssetup_state);
 	NTSTATUS status;
 
-	status = session_setup_spnego_restart(c, session, io);
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(1, ("session_setup_spnego_restart() failed: %s\n",
-			  nt_errstr(status)));
-		return status;
-	}
-
 	state->setup.spnego.level           = RAW_SESSSETUP_SPNEGO;
 	state->setup.spnego.in.bufsize      = session->transport->options.max_xmit;
 	state->setup.spnego.in.mpx_max      = session->transport->options.max_mux;
@@ -644,6 +649,15 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se
 		   !(io->in.capabilities & CAP_EXTENDED_SECURITY)) {
 		status = session_setup_nt1(c, session, io, &state->req);
 	} else {
+		status = session_setup_spnego_restart(c, session, io);
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(1, ("session_setup_spnego_restart() failed: %s\n",
+				  nt_errstr(status)));
+			c->status = status;
+			composite_error(c, c->status);
+			return c;
+		}
+
 		status = session_setup_spnego(c, session, io, &state->req);
 	}
 
-- 
1.9.1


From d717ef86eac761e0b7e94ae2c32901f5cf3cfa49 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 16 May 2017 00:25:45 +0200
Subject: [PATCH 07/10] s4:libcli/smb_composite: move gensec_update_ev() out of
 session_setup_spnego()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 46 +++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 6d64629..9359200 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -173,6 +173,21 @@ static void request_handler(struct smbcli_request *req)
 					return;
 				}
 
+				nt_status = gensec_update_ev(session->gensec, state,
+							     c->event_ctx,
+							     state->setup.spnego.out.secblob,
+							     &state->setup.spnego.in.secblob);
+				if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
+					DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
+						  gensec_get_name_by_oid(session->gensec,
+									 state->chosen_oid),
+						  nt_errstr(nt_status)));
+					c->status = nt_status;
+					composite_error(c, c->status);
+					return;
+				}
+				state->gensec_status = nt_status;
+
 				nt_status = session_setup_spnego(c, session, 
 								      state->io, 
 								      &state->req);
@@ -567,7 +582,6 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 				     struct smbcli_request **req)
 {
 	struct sesssetup_state *state = talloc_get_type(c->private_data, struct sesssetup_state);
-	NTSTATUS status;
 
 	state->setup.spnego.level           = RAW_SESSSETUP_SPNEGO;
 	state->setup.spnego.in.bufsize      = session->transport->options.max_xmit;
@@ -579,21 +593,6 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 	state->setup.spnego.in.lanman       = talloc_asprintf(state, "Samba %s", SAMBA_VERSION_STRING);
 	state->setup.spnego.in.workgroup    = io->in.workgroup;
 
-	status = gensec_update_ev(session->gensec, state,
-				  c->event_ctx,
-				  state->setup.spnego.out.secblob,
-				  &state->setup.spnego.in.secblob);
-
-	if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
-	    !NT_STATUS_IS_OK(status)) {
-		DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
-			  gensec_get_name_by_oid(session->gensec,
-						 state->chosen_oid),
-			  nt_errstr(status)));
-		return status;
-	}
-	state->gensec_status = status;
-
 	*req = smb_raw_sesssetup_send(session, &state->setup);
 	if (!*req) {
 		return NT_STATUS_NO_MEMORY;
@@ -658,6 +657,21 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se
 			return c;
 		}
 
+		status = gensec_update_ev(session->gensec, state,
+					  c->event_ctx,
+					  state->setup.spnego.out.secblob,
+					  &state->setup.spnego.in.secblob);
+		if (GENSEC_UPDATE_IS_NTERROR(status)) {
+			DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
+				  gensec_get_name_by_oid(session->gensec,
+							 state->chosen_oid),
+				  nt_errstr(status)));
+			c->status = status;
+			composite_error(c, c->status);
+			return c;
+		}
+		state->gensec_status = status;
+
 		status = session_setup_spnego(c, session, io, &state->req);
 	}
 
-- 
1.9.1


From 6ade3960ca9e24a7b222debe79c2643e2fcd3cae Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 23:24:10 +0200
Subject: [PATCH 08/10] s4:libcli/smb_composite: make the first round to gensec
 async

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 94 ++++++++++++++++++++------------
 1 file changed, 58 insertions(+), 36 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 9359200..0c222d6 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -21,6 +21,7 @@
 */
 
 #include "includes.h"
+#include <tevent.h>
 #include "libcli/raw/libcliraw.h"
 #include "libcli/raw/raw_proto.h"
 #include "libcli/composite/composite.h"
@@ -34,6 +35,7 @@
 #include "libcli/smb/smbXcli_base.h"
 
 struct sesssetup_state {
+	struct smbcli_session *session;
 	union smb_sesssetup setup;
 	const char *chosen_oid;
 	NTSTATUS remote_status;
@@ -68,6 +70,8 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 				     struct smbcli_session *session, 
 				     struct smb_composite_sesssetup *io,
 				     struct smbcli_request **req);
+static void smb_composite_sesssetup_spnego_done1(struct tevent_req *subreq);
+
 
 /*
   handler for completion of a smbcli_request sub-request
@@ -164,6 +168,8 @@ static void request_handler(struct smbcli_request *req)
 			}
 			if (cli_credentials_failed_kerberos_login(state->io->in.credentials, principal, &state->logon_retries) ||
 			    cli_credentials_wrong_password(state->io->in.credentials)) {
+				struct tevent_req *subreq = NULL;
+
 				nt_status = session_setup_spnego_restart(c, session, state->io);
 				if (!NT_STATUS_IS_OK(nt_status)) {
 					DEBUG(1, ("session_setup_spnego_restart() failed: %s\n",
@@ -173,30 +179,16 @@ static void request_handler(struct smbcli_request *req)
 					return;
 				}
 
-				nt_status = gensec_update_ev(session->gensec, state,
-							     c->event_ctx,
-							     state->setup.spnego.out.secblob,
-							     &state->setup.spnego.in.secblob);
-				if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
-					DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
-						  gensec_get_name_by_oid(session->gensec,
-									 state->chosen_oid),
-						  nt_errstr(nt_status)));
-					c->status = nt_status;
-					composite_error(c, c->status);
-					return;
-				}
-				state->gensec_status = nt_status;
-
-				nt_status = session_setup_spnego(c, session, 
-								      state->io, 
-								      &state->req);
-				if (NT_STATUS_IS_OK(nt_status)) {
-					talloc_free(check_req);
-					c->status = nt_status;
-					composite_continue_smb(c, state->req, request_handler, c);
+				subreq = gensec_update_send(state, c->event_ctx,
+							    session->gensec,
+							    state->setup.spnego.out.secblob);
+				if (composite_nomem(subreq, c)) {
 					return;
 				}
+				tevent_req_set_callback(subreq,
+							smb_composite_sesssetup_spnego_done1,
+							c);
+				return;
 			}
 		}
 		if (!NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
@@ -630,6 +622,7 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se
 	if (composite_nomem(state, c)) return c;
 	c->private_data = state;
 
+	state->session = session;
 	state->io = io;
 
 	talloc_set_destructor(state, sesssetup_state_destructor);
@@ -648,6 +641,8 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se
 		   !(io->in.capabilities & CAP_EXTENDED_SECURITY)) {
 		status = session_setup_nt1(c, session, io, &state->req);
 	} else {
+		struct tevent_req *subreq = NULL;
+
 		status = session_setup_spnego_restart(c, session, io);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(1, ("session_setup_spnego_restart() failed: %s\n",
@@ -657,22 +652,16 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se
 			return c;
 		}
 
-		status = gensec_update_ev(session->gensec, state,
-					  c->event_ctx,
-					  state->setup.spnego.out.secblob,
-					  &state->setup.spnego.in.secblob);
-		if (GENSEC_UPDATE_IS_NTERROR(status)) {
-			DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
-				  gensec_get_name_by_oid(session->gensec,
-							 state->chosen_oid),
-				  nt_errstr(status)));
-			c->status = status;
-			composite_error(c, c->status);
+		subreq = gensec_update_send(state, c->event_ctx,
+					    session->gensec,
+					    state->setup.spnego.out.secblob);
+		if (composite_nomem(subreq, c)) {
 			return c;
 		}
-		state->gensec_status = status;
-
-		status = session_setup_spnego(c, session, io, &state->req);
+		tevent_req_set_callback(subreq,
+					smb_composite_sesssetup_spnego_done1,
+					c);
+		return c;
 	}
 
 	if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) || 
@@ -685,6 +674,39 @@ struct composite_context *smb_composite_sesssetup_send(struct smbcli_session *se
 	return c;
 }
 
+static void smb_composite_sesssetup_spnego_done1(struct tevent_req *subreq)
+{
+	struct composite_context *c =
+		tevent_req_callback_data(subreq,
+		struct composite_context);
+	struct sesssetup_state *state =
+		talloc_get_type_abort(c->private_data,
+		struct sesssetup_state);
+	NTSTATUS status;
+
+	status = gensec_update_recv(subreq, state,
+				    &state->setup.spnego.in.secblob);
+	TALLOC_FREE(subreq);
+	if (GENSEC_UPDATE_IS_NTERROR(status)) {
+		DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
+			  gensec_get_name_by_oid(state->session->gensec,
+						 state->chosen_oid),
+			  nt_errstr(status)));
+		c->status = status;
+		composite_error(c, c->status);
+		return;
+	}
+	state->gensec_status = status;
+
+	status = session_setup_spnego(c, state->session, state->io, &state->req);
+	if (!NT_STATUS_IS_OK(status)) {
+		c->status = status;
+		composite_error(c, c->status);
+		return;
+	}
+
+	composite_continue_smb(c, state->req, request_handler, c);
+}
 
 /*
   receive a composite session setup reply
-- 
1.9.1


From f21b1e0f1ae5f2d70e5976b0bece1f65cb0b2298 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 23:33:04 +0200
Subject: [PATCH 09/10] s4:libcli/smb_composite: add early returns to
 sesssetup.c:request_handler()

This makes it much clearer under which condutions the following code
operates.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 34 +++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 0c222d6..2f1ce4c 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -125,6 +125,10 @@ static void request_handler(struct smbcli_request *req)
 				}
 			}
 		}
+		if (!NT_STATUS_IS_OK(c->status)) {
+			composite_error(c, c->status);
+			return;
+		}
 		os = state->setup.old.out.os;
 		lanman = state->setup.old.out.lanman;
 		break;
@@ -146,6 +150,10 @@ static void request_handler(struct smbcli_request *req)
 				}
 			}
 		}
+		if (!NT_STATUS_IS_OK(c->status)) {
+			composite_error(c, c->status);
+			return;
+		}
 		os = state->setup.nt1.out.os;
 		lanman = state->setup.nt1.out.lanman;
 		break;
@@ -191,9 +199,9 @@ static void request_handler(struct smbcli_request *req)
 				return;
 			}
 		}
-		if (!NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
-		    !NT_STATUS_IS_OK(c->status)) {
-			break;
+		if (GENSEC_UPDATE_IS_NTERROR(c->status)) {
+			composite_error(c, c->status);
+			return;
 		}
 		if (NT_STATUS_EQUAL(state->gensec_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
 
@@ -208,9 +216,9 @@ static void request_handler(struct smbcli_request *req)
 							 state->setup.spnego.out.secblob,
 							 &state->setup.spnego.in.secblob);
 			c->status = state->gensec_status;
-			if (!NT_STATUS_EQUAL(c->status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
-			    !NT_STATUS_IS_OK(c->status)) {
-				break;
+			if (GENSEC_UPDATE_IS_NTERROR(c->status)) {
+				composite_error(c, c->status);
+				return;
 			}
 		} else {
 			state->setup.spnego.in.secblob = data_blob(NULL, 0);
@@ -225,7 +233,8 @@ static void request_handler(struct smbcli_request *req)
 
 			if (state->setup.spnego.in.secblob.length) {
 				c->status = NT_STATUS_INTERNAL_ERROR;
-				break;
+				composite_error(c, c->status);
+				return;
 			}
 			session_key_err = gensec_session_key(session->gensec, session, &session_key);
 			if (NT_STATUS_IS_OK(session_key_err)) {
@@ -238,7 +247,8 @@ static void request_handler(struct smbcli_request *req)
 								    session_key);
 			data_blob_free(&session_key);
 			if (!NT_STATUS_IS_OK(c->status)) {
-				break;
+				composite_error(c, c->status);
+				return;
 			}
 		}
 
@@ -264,7 +274,8 @@ static void request_handler(struct smbcli_request *req)
 
 	case RAW_SESSSETUP_SMB2:
 		c->status = NT_STATUS_INTERNAL_ERROR;
-		break;
+		composite_error(c, c->status);
+		return;
 	}
 
 	if (check_req) {
@@ -274,11 +285,12 @@ static void request_handler(struct smbcli_request *req)
 
 		ok = smb1cli_conn_check_signing(check_req->transport->conn,
 						check_req->in.buffer, 1);
+		TALLOC_FREE(check_req);
 		if (!ok) {
 			c->status = NT_STATUS_ACCESS_DENIED;
+			composite_error(c, c->status);
+			return;
 		}
-		talloc_free(check_req);
-		check_req = NULL;
 	}
 
 	if (!NT_STATUS_IS_OK(c->status)) {
-- 
1.9.1


From a7d84b8fdc2a789dc0aae31f931a12a200d71a13 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 15 Jun 2017 00:03:14 +0200
Subject: [PATCH 10/10] s4:libcli/smb_composite: make the additional
 gensec_update steps async

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/libcli/smb_composite/sesssetup.c | 151 ++++++++++++++++++++++++++-----
 1 file changed, 128 insertions(+), 23 deletions(-)

diff --git a/source4/libcli/smb_composite/sesssetup.c b/source4/libcli/smb_composite/sesssetup.c
index 2f1ce4c..6ee4929 100644
--- a/source4/libcli/smb_composite/sesssetup.c
+++ b/source4/libcli/smb_composite/sesssetup.c
@@ -42,6 +42,7 @@ struct sesssetup_state {
 	NTSTATUS gensec_status;
 	struct smb_composite_sesssetup *io;
 	struct smbcli_request *req;
+	struct smbcli_request *check_req;
 	unsigned int logon_retries;
 };
 
@@ -71,6 +72,7 @@ static NTSTATUS session_setup_spnego(struct composite_context *c,
 				     struct smb_composite_sesssetup *io,
 				     struct smbcli_request **req);
 static void smb_composite_sesssetup_spnego_done1(struct tevent_req *subreq);
+static void smb_composite_sesssetup_spnego_done2(struct tevent_req *subreq);
 
 
 /*
@@ -204,6 +206,7 @@ static void request_handler(struct smbcli_request *req)
 			return;
 		}
 		if (NT_STATUS_EQUAL(state->gensec_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+			struct tevent_req *subreq = NULL;
 
 			/* The status value here, from the earlier pass at GENSEC is
 			 * vital to the security of the system.  Even if the other end
@@ -211,15 +214,22 @@ static void request_handler(struct smbcli_request *req)
 			 * you must keep feeding it blobs, or else the remote
 			 * host/attacker might avoid mutal authentication
 			 * requirements */
-			
-			state->gensec_status = gensec_update_ev(session->gensec, state, c->event_ctx,
-							 state->setup.spnego.out.secblob,
-							 &state->setup.spnego.in.secblob);
-			c->status = state->gensec_status;
-			if (GENSEC_UPDATE_IS_NTERROR(c->status)) {
-				composite_error(c, c->status);
+
+			subreq = gensec_update_send(state, c->event_ctx,
+						    session->gensec,
+						    state->setup.spnego.out.secblob);
+			if (composite_nomem(subreq, c)) {
 				return;
 			}
+			tevent_req_set_callback(subreq,
+						smb_composite_sesssetup_spnego_done2,
+						c);
+			if (NT_STATUS_IS_OK(state->remote_status)) {
+				state->check_req = check_req;
+			} else {
+				TALLOC_FREE(check_req);
+			}
+			return;
 		} else {
 			state->setup.spnego.in.secblob = data_blob(NULL, 0);
 		}
@@ -252,22 +262,6 @@ static void request_handler(struct smbcli_request *req)
 			}
 		}
 
-		if (state->setup.spnego.in.secblob.length) {
-			/* 
-			 * set the session->vuid value only for calling
-			 * smb_raw_sesssetup_send()
-			 */
-			uint16_t vuid = session->vuid;
-			session->vuid = state->io->out.vuid;
-			state->req = smb_raw_sesssetup_send(session, &state->setup);
-			session->vuid = vuid;
-			if (state->req &&
-			    !smb1cli_conn_signing_is_active(state->req->transport->conn)) {
-				state->req->sign_caller_checks = true;
-			}
-			composite_continue_smb(c, state->req, request_handler, c);
-			return;
-		}
 		os = state->setup.spnego.out.os;
 		lanman = state->setup.spnego.out.lanman;
 		break;
@@ -720,6 +714,117 @@ static void smb_composite_sesssetup_spnego_done1(struct tevent_req *subreq)
 	composite_continue_smb(c, state->req, request_handler, c);
 }
 
+static void smb_composite_sesssetup_spnego_done2(struct tevent_req *subreq)
+{
+	struct composite_context *c =
+		tevent_req_callback_data(subreq,
+		struct composite_context);
+	struct sesssetup_state *state =
+		talloc_get_type_abort(c->private_data,
+		struct sesssetup_state);
+	struct smbcli_session *session = state->session;
+	NTSTATUS status;
+	const char *os = NULL;
+	const char *lanman = NULL;
+
+	status = gensec_update_recv(subreq, state,
+				    &state->setup.spnego.in.secblob);
+	TALLOC_FREE(subreq);
+	if (GENSEC_UPDATE_IS_NTERROR(status)) {
+		DEBUG(1, ("Failed initial gensec_update with mechanism %s: %s\n",
+			  gensec_get_name_by_oid(state->session->gensec,
+						 state->chosen_oid),
+			  nt_errstr(status)));
+		c->status = status;
+		composite_error(c, c->status);
+		return;
+	}
+	state->gensec_status = status;
+
+	if (NT_STATUS_IS_OK(state->remote_status)) {
+		if (state->setup.spnego.in.secblob.length) {
+			c->status = NT_STATUS_INTERNAL_ERROR;
+			composite_error(c, c->status);
+			return;
+		}
+	}
+
+	if (state->setup.spnego.in.secblob.length) {
+		/*
+		 * set the session->vuid value only for calling
+		 * smb_raw_sesssetup_send()
+		 */
+		uint16_t vuid = session->vuid;
+		session->vuid = state->io->out.vuid;
+		state->req = smb_raw_sesssetup_send(session, &state->setup);
+		session->vuid = vuid;
+		if (state->req &&
+		    !smb1cli_conn_signing_is_active(state->req->transport->conn)) {
+			state->req->sign_caller_checks = true;
+		}
+		composite_continue_smb(c, state->req, request_handler, c);
+		return;
+	}
+
+	if (cli_credentials_is_anonymous(state->io->in.credentials)) {
+		/*
+		 * anonymous => no signing
+		 */
+	} else if (NT_STATUS_IS_OK(state->remote_status)) {
+		NTSTATUS session_key_err;
+		DATA_BLOB session_key;
+
+		session_key_err = gensec_session_key(session->gensec, session, &session_key);
+		if (NT_STATUS_IS_OK(session_key_err)) {
+			smb1cli_conn_activate_signing(session->transport->conn,
+						      session_key,
+						      data_blob_null);
+		}
+
+		c->status = smb1cli_session_set_session_key(session->smbXcli,
+							    session_key);
+		data_blob_free(&session_key);
+		if (!NT_STATUS_IS_OK(c->status)) {
+			composite_error(c, c->status);
+			return;
+		}
+	}
+
+	os = state->setup.spnego.out.os;
+	lanman = state->setup.spnego.out.lanman;
+
+	if (state->check_req) {
+		struct smbcli_request *check_req = state->check_req;
+		bool ok;
+
+		check_req->sign_caller_checks = false;
+
+		ok = smb1cli_conn_check_signing(check_req->transport->conn,
+						check_req->in.buffer, 1);
+		TALLOC_FREE(check_req);
+		if (!ok) {
+			c->status = NT_STATUS_ACCESS_DENIED;
+			composite_error(c, c->status);
+			return;
+		}
+	}
+
+	if (os) {
+		session->os = talloc_strdup(session, os);
+		if (composite_nomem(session->os, c)) return;
+	} else {
+		session->os = NULL;
+	}
+	if (lanman) {
+		session->lanman = talloc_strdup(session, lanman);
+		if (composite_nomem(session->lanman, c)) return;
+	} else {
+		session->lanman = NULL;
+	}
+
+	composite_done(c);
+}
+
 /*
   receive a composite session setup reply
 */
-- 
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/20170617/abad23fe/signature.sig>


More information about the samba-technical mailing list