[Patches] The way to remove gensec_update_ev()

Stefan Metzmacher metze at samba.org
Tue Jun 27 06:42:42 UTC 2017


Hi,

>> here're some preparation patches which passed autobuild twice.

and some more just passed a private autobuild...

Please review and push:-)

Thanks!
metze

-------------- next part --------------
From 845a72d05576dc56e3323286d7b2ad711bc25cfd Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 17 Jun 2017 00:26:18 +0200
Subject: [PATCH 1/5] s4:auth_winbind: fix error checking in
 winbind_check_password()

We need to handle every error instead of just NT_STATUS_NO_SUCH_USER,
the callers also doesn't require NT_STATUS_NOT_IMPLEMENTED anymore.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/ntlm/auth_winbind.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c
index 7308a17..65ad9b6 100644
--- a/source4/auth/ntlm/auth_winbind.c
+++ b/source4/auth/ntlm/auth_winbind.c
@@ -189,9 +189,11 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 	status = dcerpc_winbind_SamLogon_r(irpc_handle, s, &s->req);
 	NT_STATUS_NOT_OK_RETURN(status);
 
-	if (NT_STATUS_EQUAL(s->req.out.result, NT_STATUS_NO_SUCH_USER) &&
-	    !s->req.out.authoritative) {
-		return NT_STATUS_NOT_IMPLEMENTED;
+	if (!NT_STATUS_IS_OK(s->req.out.result)) {
+		if (!s->req.out.authoritative) {
+			*authoritative = false;
+		}
+		return s->req.out.result;
 	}
 
 	/*
@@ -199,7 +201,7 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 	 * This means that lockouts happen at a badPwdCount earlier than
 	 * normal, but makes it more fault tolerant.
 	 */
-	if (NT_STATUS_IS_OK(s->req.out.result)) {
+	{
 		const char *account_name = user_info->mapped.account_name;
 		const char *p = NULL;
 		p = strchr_m(account_name, '@');
-- 
1.9.1


From 8165579522c2b119194702e0f1fae7dcb53cbe1c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 17 Jun 2017 00:29:25 +0200
Subject: [PATCH 2/5] s4:auth_winbind: rename 's' to 'state' in
 winbind_check_password()

This prepares the conversion to winbind_check_password_send/recv()
where the internal state is called 'winbind_check_password_state'
as 'state'.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/ntlm/auth_winbind.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c
index 65ad9b6..59c0ed6 100644
--- a/source4/auth/ntlm/auth_winbind.c
+++ b/source4/auth/ntlm/auth_winbind.c
@@ -103,9 +103,9 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 				       struct auth_user_info_dc **user_info_dc,
 				       bool *authoritative)
 {
+	struct winbind_check_password_state *state = NULL;
 	NTSTATUS status;
 	struct dcerpc_binding_handle *irpc_handle;
-	struct winbind_check_password_state *s;
 	const struct auth_usersupplied_info *user_info_new;
 	struct netr_IdentityInfo *identity_info;
 	struct ldb_dn *domain_dn;
@@ -117,10 +117,10 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	s = talloc(mem_ctx, struct winbind_check_password_state);
-	NT_STATUS_HAVE_NO_MEMORY(s);
+	state = talloc(mem_ctx, struct winbind_check_password_state);
+	NT_STATUS_HAVE_NO_MEMORY(state);
 
-	irpc_handle = irpc_binding_handle_by_name(s, ctx->auth_ctx->msg_ctx,
+	irpc_handle = irpc_binding_handle_by_name(state, ctx->auth_ctx->msg_ctx,
 						  "winbind_server",
 						  &ndr_table_winbind);
 	if (irpc_handle == NULL) {
@@ -133,30 +133,30 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 	if (user_info->flags & USER_INFO_INTERACTIVE_LOGON) {
 		struct netr_PasswordInfo *password_info;
 
-		status = encrypt_user_info(s, ctx->auth_ctx, AUTH_PASSWORD_HASH,
+		status = encrypt_user_info(state, ctx->auth_ctx, AUTH_PASSWORD_HASH,
 					   user_info, &user_info_new);
 		NT_STATUS_NOT_OK_RETURN(status);
 		user_info = user_info_new;
 
-		password_info = talloc(s, struct netr_PasswordInfo);
+		password_info = talloc_zero(state, struct netr_PasswordInfo);
 		NT_STATUS_HAVE_NO_MEMORY(password_info);
 
 		password_info->lmpassword = *user_info->password.hash.lanman;
 		password_info->ntpassword = *user_info->password.hash.nt;
 
 		identity_info = &password_info->identity_info;
-		s->req.in.logon_level	= 1;
-		s->req.in.logon.password= password_info;
+		state->req.in.logon_level	= 1;
+		state->req.in.logon.password= password_info;
 	} else {
 		struct netr_NetworkInfo *network_info;
 		uint8_t chal[8];
 
-		status = encrypt_user_info(s, ctx->auth_ctx, AUTH_PASSWORD_RESPONSE,
+		status = encrypt_user_info(state, ctx->auth_ctx, AUTH_PASSWORD_RESPONSE,
 					   user_info, &user_info_new);
 		NT_STATUS_NOT_OK_RETURN(status);
 		user_info = user_info_new;
 
-		network_info = talloc(s, struct netr_NetworkInfo);
+		network_info = talloc_zero(state, struct netr_NetworkInfo);
 		NT_STATUS_HAVE_NO_MEMORY(network_info);
 
 		status = auth_get_challenge(ctx->auth_ctx, chal);
@@ -171,8 +171,8 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 		network_info->lm.data	= user_info->password.response.lanman.data;
 
 		identity_info = &network_info->identity_info;
-		s->req.in.logon_level	= 2;
-		s->req.in.logon.network = network_info;
+		state->req.in.logon_level	= 2;
+		state->req.in.logon.network = network_info;
 	}
 
 	identity_info->domain_name.string	= user_info->client.domain_name;
@@ -182,18 +182,18 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 	identity_info->account_name.string	= user_info->client.account_name;
 	identity_info->workstation.string	= user_info->workstation_name;
 
-	s->req.in.validation_level	= 3;
+	state->req.in.validation_level = 3;
 
 	/* Note: this makes use of nested event loops... */
 	dcerpc_binding_handle_set_sync_ev(irpc_handle, ctx->auth_ctx->event_ctx);
-	status = dcerpc_winbind_SamLogon_r(irpc_handle, s, &s->req);
+	status = dcerpc_winbind_SamLogon_r(irpc_handle, state, &state->req);
 	NT_STATUS_NOT_OK_RETURN(status);
 
-	if (!NT_STATUS_IS_OK(s->req.out.result)) {
-		if (!s->req.out.authoritative) {
+	if (!NT_STATUS_IS_OK(state->req.out.result)) {
+		if (!state->req.out.authoritative) {
 			*authoritative = false;
 		}
-		return s->req.out.result;
+		return state->req.out.result;
 	}
 
 	/*
@@ -234,8 +234,8 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx,
 
 	status = make_user_info_dc_netlogon_validation(mem_ctx,
 						      user_info->client.account_name,
-						      s->req.in.validation_level,
-						      &s->req.out.validation,
+						      state->req.in.validation_level,
+						      &state->req.out.validation,
 						       true, /* This user was authenticated */
 						      user_info_dc);
 	NT_STATUS_NOT_OK_RETURN(status);
-- 
1.9.1


From 37f0070d90c9a3510acd24df69b24b36a6008bc6 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 16 Jun 2017 22:46:27 +0200
Subject: [PATCH 3/5] s4:auth/ntlm: move auth_check_password_wrapper() further
 down

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/ntlm/auth.c | 74 ++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c
index cbe49a1..580834f5 100644
--- a/source4/auth/ntlm/auth.c
+++ b/source4/auth/ntlm/auth.c
@@ -186,43 +186,6 @@ _PUBLIC_ NTSTATUS auth_check_password(struct auth4_context *auth_ctx,
 	return status;
 }
 
-static NTSTATUS auth_check_password_wrapper(struct auth4_context *auth_ctx,
-					    TALLOC_CTX *mem_ctx,
-					    const struct auth_usersupplied_info *user_info,
-					    uint8_t *pauthoritative,
-					    void **server_returned_info,
-					    DATA_BLOB *user_session_key, DATA_BLOB *lm_session_key)
-{
-	struct auth_user_info_dc *user_info_dc;
-	NTSTATUS status;
-
-	status = auth_check_password(auth_ctx, mem_ctx, user_info,
-				     &user_info_dc, pauthoritative);
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-
-	*server_returned_info = user_info_dc;
-
-	if (user_session_key) {
-		DEBUG(10, ("Got NT session key of length %u\n",
-			   (unsigned)user_info_dc->user_session_key.length));
-		*user_session_key = user_info_dc->user_session_key;
-		talloc_steal(mem_ctx, user_session_key->data);
-		user_info_dc->user_session_key = data_blob_null;
-	}
-
-	if (lm_session_key) {
-		DEBUG(10, ("Got LM session key of length %u\n",
-			   (unsigned)user_info_dc->lm_session_key.length));
-		*lm_session_key = user_info_dc->lm_session_key;
-		talloc_steal(mem_ctx, lm_session_key->data);
-		user_info_dc->lm_session_key = data_blob_null;
-	}
-
-	return NT_STATUS_OK;
-}
-
 struct auth_check_password_state {
 	struct auth4_context *auth_ctx;
 	const struct auth_usersupplied_info *user_info;
@@ -497,6 +460,43 @@ _PUBLIC_ NTSTATUS auth_check_password_recv(struct tevent_req *req,
 	return NT_STATUS_OK;
 }
 
+static NTSTATUS auth_check_password_wrapper(struct auth4_context *auth_ctx,
+					    TALLOC_CTX *mem_ctx,
+					    const struct auth_usersupplied_info *user_info,
+					    uint8_t *pauthoritative,
+					    void **server_returned_info,
+					    DATA_BLOB *user_session_key, DATA_BLOB *lm_session_key)
+{
+	struct auth_user_info_dc *user_info_dc;
+	NTSTATUS status;
+
+	status = auth_check_password(auth_ctx, mem_ctx, user_info,
+				     &user_info_dc, pauthoritative);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	*server_returned_info = user_info_dc;
+
+	if (user_session_key) {
+		DEBUG(10, ("Got NT session key of length %u\n",
+			   (unsigned)user_info_dc->user_session_key.length));
+		*user_session_key = user_info_dc->user_session_key;
+		talloc_steal(mem_ctx, user_session_key->data);
+		user_info_dc->user_session_key = data_blob_null;
+	}
+
+	if (lm_session_key) {
+		DEBUG(10, ("Got LM session key of length %u\n",
+			   (unsigned)user_info_dc->lm_session_key.length));
+		*lm_session_key = user_info_dc->lm_session_key;
+		talloc_steal(mem_ctx, lm_session_key->data);
+		user_info_dc->lm_session_key = data_blob_null;
+	}
+
+	return NT_STATUS_OK;
+}
+
  /* Wrapper because we don't want to expose all callers to needing to
   * know that session_info is generated from the main ldb, and because
   * we need to break a depenency loop between the DCE/RPC layer and the
-- 
1.9.1


From 82e56f1da5e81803c506dea18a5aca49e51136ca Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 17 Jun 2017 00:05:22 +0200
Subject: [PATCH 4/5] s4:auth/ntlm: introduce auth_check_password_next()

This prepares real async handling in the backends.

Check with git show -w.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/ntlm/auth.c | 98 ++++++++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 54 deletions(-)

diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c
index 580834f5..285edd9 100644
--- a/source4/auth/ntlm/auth.c
+++ b/source4/auth/ntlm/auth.c
@@ -194,9 +194,8 @@ struct auth_check_password_state {
 	uint8_t authoritative;
 };
 
-static void auth_check_password_async_trigger(struct tevent_context *ev,
-					      struct tevent_immediate *im,
-					      void *private_data);
+static void auth_check_password_next(struct tevent_req *req);
+
 /**
  * Check a user's Plaintext, LM or NTLM password.
  * async send hook
@@ -233,7 +232,6 @@ _PUBLIC_ struct tevent_req *auth_check_password_send(TALLOC_CTX *mem_ctx,
 	/* if all the modules say 'not for me' this is reasonable */
 	NTSTATUS nt_status;
 	uint8_t chal[8];
-	struct tevent_immediate *im;
 
 	DEBUG(3,("auth_check_password_send: "
 		 "Checking password for unmapped user [%s]\\[%s]@[%s]\n",
@@ -249,9 +247,9 @@ _PUBLIC_ struct tevent_req *auth_check_password_send(TALLOC_CTX *mem_ctx,
 	/*
 	 * We are authoritative by default.
 	 */
-	state->authoritative	= 1;
 	state->auth_ctx		= auth_ctx;
 	state->user_info	= user_info;
+	state->authoritative	= 1;
 
 	if (!user_info->mapped_state) {
 		struct auth_usersupplied_info *user_info_tmp;
@@ -312,75 +310,67 @@ _PUBLIC_ struct tevent_req *auth_check_password_send(TALLOC_CTX *mem_ctx,
 	dump_data(5, auth_ctx->challenge.data.data,
 		  auth_ctx->challenge.data.length);
 
-	im = tevent_create_immediate(state);
-	if (tevent_req_nomem(im, req)) {
+	state->method = state->auth_ctx->methods;
+	auth_check_password_next(req);
+	if (!tevent_req_is_in_progress(req)) {
 		return tevent_req_post(req, ev);
 	}
 
-	tevent_schedule_immediate(im,
-				  auth_ctx->event_ctx,
-				  auth_check_password_async_trigger,
-				  req);
 	return req;
 }
 
-static void auth_check_password_async_trigger(struct tevent_context *ev,
-					      struct tevent_immediate *im,
-					      void *private_data)
+static void auth_check_password_next(struct tevent_req *req)
 {
-	struct tevent_req *req =
-		talloc_get_type_abort(private_data, struct tevent_req);
 	struct auth_check_password_state *state =
 		tevent_req_data(req, struct auth_check_password_state);
-	NTSTATUS status;
-	struct auth_method_context *method;
+	struct tevent_req *subreq = NULL;
 	bool authoritative = true;
+	NTSTATUS status;
 
-	status = NT_STATUS_OK;
-
-	for (method=state->auth_ctx->methods; method; method = method->next) {
-		authoritative = true;
-
-		/* we fill in state->method here so debug messages in
-		   the callers know which method failed */
-		state->method = method;
-
-		/* check if the module wants to check the password */
-		status = method->ops->want_check(method, req, state->user_info);
-		if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) {
-			DEBUG(11,("auth_check_password_send: "
-				  "%s doesn't want to check\n",
-				  method->ops->name));
-			continue;
-		}
+	if (state->method == NULL) {
+		state->authoritative = 0;
+		tevent_req_nterror(req, NT_STATUS_NO_SUCH_USER);
+		return;
+	}
 
-		if (tevent_req_nterror(req, status)) {
-			return;
-		}
+	/* check if the module wants to check the password */
+	status = state->method->ops->want_check(state->method, state,
+						state->user_info);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) {
+		DEBUG(11,("auth_check_password_send: "
+			  "%s doesn't want to check\n",
+			  state->method->ops->name));
+		state->method = state->method->next;
+		auth_check_password_next(req);
+		return;
+	}
 
-		status = method->ops->check_password(method,
-						     state,
-						     state->user_info,
-						     &state->user_info_dc,
-						     &authoritative);
-		if (!authoritative ||
-		    NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) {
-			DEBUG(11,("auth_check_password_send: "
-				  "%s passes to the next method\n",
-				  method->ops->name));
-			continue;
-		}
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
 
-		/* the backend has handled the request */
-		break;
+	if (state->method->ops->check_password == NULL) {
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
 	}
 
+	status = state->method->ops->check_password(state->method,
+						    state,
+						    state->user_info,
+						    &state->user_info_dc,
+						    &authoritative);
 	if (!authoritative ||
 	    NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) {
-		state->authoritative = 0;
-		status = NT_STATUS_NO_SUCH_USER;
+		DEBUG(11,("auth_check_password_send: "
+			  "%s passes to the next method\n",
+			  state->method->ops->name));
+		state->method = state->method->next;
+		auth_check_password_next(req);
+		return;
 	}
 
+	/* the backend has handled the request */
+
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
-- 
1.9.1


From e1c78d44077d987918d83c393008ef444a9ab2e4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sat, 17 Jun 2017 00:05:22 +0200
Subject: [PATCH 5/5] s4:auth/ntlm: allow auth_operations to specify
 check_password_send/recv()

This prepares real async handling in the backends.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/auth.h      |  9 ++++++++-
 source4/auth/ntlm/auth.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/source4/auth/auth.h b/source4/auth/auth.h
index 2dc0d8c..f88489b 100644
--- a/source4/auth/auth.h
+++ b/source4/auth/auth.h
@@ -65,7 +65,14 @@ struct auth_operations {
 				   const struct auth_usersupplied_info *user_info,
 				   struct auth_user_info_dc **interim_info,
 				   bool *authoritative);
-
+	struct tevent_req *(*check_password_send)(TALLOC_CTX *mem_ctx,
+				struct tevent_context *ev,
+				struct auth_method_context *ctx,
+				const struct auth_usersupplied_info *user_info);
+	NTSTATUS (*check_password_recv)(struct tevent_req *subreq,
+				TALLOC_CTX *mem_ctx,
+				struct auth_user_info_dc **interim_info,
+				bool *authoritative);
 
 	/* Lookup a 'session info interim' return based only on the principal or DN */
 	NTSTATUS (*get_user_info_dc_principal)(TALLOC_CTX *mem_ctx,
diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c
index 285edd9..0e76348 100644
--- a/source4/auth/ntlm/auth.c
+++ b/source4/auth/ntlm/auth.c
@@ -187,6 +187,7 @@ _PUBLIC_ NTSTATUS auth_check_password(struct auth4_context *auth_ctx,
 }
 
 struct auth_check_password_state {
+	struct tevent_context *ev;
 	struct auth4_context *auth_ctx;
 	const struct auth_usersupplied_info *user_info;
 	struct auth_user_info_dc *user_info_dc;
@@ -247,6 +248,7 @@ _PUBLIC_ struct tevent_req *auth_check_password_send(TALLOC_CTX *mem_ctx,
 	/*
 	 * We are authoritative by default.
 	 */
+	state->ev		= ev;
 	state->auth_ctx		= auth_ctx;
 	state->user_info	= user_info;
 	state->authoritative	= 1;
@@ -319,6 +321,8 @@ _PUBLIC_ struct tevent_req *auth_check_password_send(TALLOC_CTX *mem_ctx,
 	return req;
 }
 
+static void auth_check_password_done(struct tevent_req *subreq);
+
 static void auth_check_password_next(struct tevent_req *req)
 {
 	struct auth_check_password_state *state =
@@ -349,6 +353,20 @@ static void auth_check_password_next(struct tevent_req *req)
 		return;
 	}
 
+	if (state->method->ops->check_password_send != NULL) {
+		subreq = state->method->ops->check_password_send(state,
+								 state->ev,
+								 state->method,
+								 state->user_info);
+		if (tevent_req_nomem(subreq, req)) {
+			return;
+		}
+		tevent_req_set_callback(subreq,
+					auth_check_password_done,
+					req);
+		return;
+	}
+
 	if (state->method->ops->check_password == NULL) {
 		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
 		return;
@@ -378,6 +396,40 @@ static void auth_check_password_next(struct tevent_req *req)
 	tevent_req_done(req);
 }
 
+static void auth_check_password_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct auth_check_password_state *state =
+		tevent_req_data(req,
+		struct auth_check_password_state);
+	bool authoritative = true;
+	NTSTATUS status;
+
+	status = state->method->ops->check_password_recv(subreq, state,
+							 &state->user_info_dc,
+							 &authoritative);
+	TALLOC_FREE(subreq);
+	if (!authoritative ||
+	    NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) {
+		DEBUG(11,("auth_check_password_send: "
+			  "%s passes to the next method\n",
+			  state->method->ops->name));
+		state->method = state->method->next;
+		auth_check_password_next(req);
+		return;
+	}
+
+	/* the backend has handled the request */
+
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+
+	tevent_req_done(req);
+}
+
 /**
  * Check a user's Plaintext, LM or NTLM password.
  * async receive function
-- 
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/20170627/186363f5/signature-0001.sig>


More information about the samba-technical mailing list