Mercurial > dovecot > original-hg > dovecot-1.2

changeset 3171:8a3b57385eca HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Added state variable for auth_request and several assertions to make sure the state is always valid. Fixed assert crash when a user having cached passdb entry expired tried to authenticate.
author Timo Sirainen <tss@iki.fi>
date Sat, 05 Mar 2005 13:48:13 +0200
parents 42e1e9dbf19e
children b951764080cc
files src/auth/auth-request-handler.c src/auth/auth-request.c src/auth/auth-request.h src/auth/passdb-blocking.c src/auth/passdb-cache.c src/auth/passdb-cache.h
diffstat 6 files changed, 59 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/src/auth/auth-request-handler.c	Sat Mar 05 12:25:09 2005 +0200
+++ b/src/auth/auth-request-handler.c	Sat Mar 05 13:48:13 2005 +0200
@@ -384,6 +384,10 @@
         struct auth_request_handler *handler = request->context;
 	string_t *reply;
 
+	i_assert(request->state == AUTH_REQUEST_STATE_USERDB);
+
+	request->state = AUTH_REQUEST_STATE_FINISHED;
+
 	reply = t_str_new(256);
 	if (handler->prepend_connect_uid)
 		str_printfa(reply, "%u\t", request->connect_uid);
@@ -422,7 +426,8 @@
 	auth_request_ref(request);
 	auth_request_handler_remove(handler, request);
 
-	if (!request->successful) {
+	if (request->state != AUTH_REQUEST_STATE_FINISHED ||
+	    !request->successful) {
 		i_error("Master requested unfinished authentication request "
 			"%u.%u", handler->client_pid, client_id);
 		str_printfa(reply, "NOTFOUND\t%u", id);
@@ -431,6 +436,7 @@
 		/* the request isn't being referenced anywhere anymore,
 		   so we can do a bit of kludging.. replace the request's
 		   old client_id with master's id. */
+		request->state = AUTH_REQUEST_STATE_USERDB;
 		request->id = id;
 		request->context = handler;
 
@@ -449,6 +455,7 @@
 	size /= sizeof(*auth_request);
 
 	for (i = 0; i < size; i++) {
+		i_assert(auth_request[i]->state == AUTH_REQUEST_STATE_FINISHED);
 		auth_request[i]->callback(auth_request[i],
 					  AUTH_CLIENT_RESULT_FAILURE, NULL, 0);
 		auth_request_unref(auth_request[i]);
--- a/src/auth/auth-request.c	Sat Mar 05 12:25:09 2005 +0200
+++ b/src/auth/auth-request.c	Sat Mar 05 13:48:13 2005 +0200
@@ -23,6 +23,7 @@
 	struct auth_request *request;
 
 	request = mech->auth_new();
+	request->state = AUTH_REQUEST_STATE_NEW;
 
 	request->refcount = 1;
 	request->created = ioloop_time;
@@ -37,9 +38,9 @@
 void auth_request_success(struct auth_request *request,
 			  const void *data, size_t data_size)
 {
-	i_assert(!request->finished);
-	request->finished = TRUE;
+	i_assert(request->state == AUTH_REQUEST_STATE_MECH_CONTINUE);
 
+	request->state = AUTH_REQUEST_STATE_FINISHED;
 	request->successful = TRUE;
 	request->callback(request, AUTH_CLIENT_RESULT_SUCCESS,
 			  data, data_size);
@@ -47,9 +48,9 @@
 
 void auth_request_fail(struct auth_request *request)
 {
-	i_assert(!request->finished);
-	request->finished = TRUE;
+	i_assert(request->state == AUTH_REQUEST_STATE_MECH_CONTINUE);
 
+	request->state = AUTH_REQUEST_STATE_FINISHED;
 	request->callback(request, AUTH_CLIENT_RESULT_FAILURE, NULL, 0);
 }
 
@@ -89,12 +90,17 @@
 void auth_request_initial(struct auth_request *request,
 			  const unsigned char *data, size_t data_size)
 {
+	i_assert(request->state == AUTH_REQUEST_STATE_NEW);
+
+	request->state = AUTH_REQUEST_STATE_MECH_CONTINUE;
 	request->mech->auth_initial(request, data, data_size);
 }
 
 void auth_request_continue(struct auth_request *request,
 			   const unsigned char *data, size_t data_size)
 {
+	i_assert(request->state == AUTH_REQUEST_STATE_MECH_CONTINUE);
+
 	request->mech->auth_continue(request, data, data_size);
 }
 
@@ -149,7 +155,10 @@
 					struct auth_request *request)
 {
 	const char *cache_key;
-	int expired;
+
+	i_assert(request->state == AUTH_REQUEST_STATE_PASSDB);
+
+	request->state = AUTH_REQUEST_STATE_MECH_CONTINUE;
 
         auth_request_save_cache(request, result);
 
@@ -161,7 +170,7 @@
 		   expired record. */
 		if (passdb_cache_verify_plain(request, cache_key,
 					      request->mech_password,
-					      &result, &expired)) {
+					      &result, TRUE)) {
 			request->private_callback.verify_plain(result, request);
 			safe_memset(request->mech_password, 0,
 				    strlen(request->mech_password));
@@ -193,7 +202,8 @@
 	struct passdb_module *passdb = request->auth->passdb;
 	enum passdb_result result;
 	const char *cache_key;
-	int expired;
+
+	i_assert(request->state == AUTH_REQUEST_STATE_MECH_CONTINUE);
 
 	request->mech_password = p_strdup(request->pool, password);
 	request->private_callback.verify_plain = callback;
@@ -201,12 +211,14 @@
 	cache_key = passdb_cache == NULL ? NULL : passdb->cache_key;
 	if (cache_key != NULL) {
 		if (passdb_cache_verify_plain(request, cache_key, password,
-					      &result, &expired) && !expired) {
+					      &result, FALSE)) {
 			callback(result, request);
 			return;
 		}
 	}
 
+	request->state = AUTH_REQUEST_STATE_PASSDB;
+
 	if (passdb->blocking)
 		passdb_blocking_verify_plain(request);
 	else {
@@ -220,8 +232,10 @@
 					      struct auth_request *request)
 {
 	const char *cache_key, *scheme;
-	int expired;
 
+	i_assert(request->state == AUTH_REQUEST_STATE_PASSDB);
+
+	request->state = AUTH_REQUEST_STATE_MECH_CONTINUE;
         auth_request_save_cache(request, result);
 
 	if (request->passdb_password != NULL) {
@@ -237,7 +251,7 @@
 		   expired record. */
 		if (passdb_cache_lookup_credentials(request, cache_key,
 						    &credentials, &scheme,
-						    &expired)) {
+						    TRUE)) {
 			passdb_handle_credentials(credentials != NULL ?
 				PASSDB_RESULT_OK : PASSDB_RESULT_USER_UNKNOWN,
 				request->credentials, credentials, scheme,
@@ -257,13 +271,13 @@
 {
 	struct passdb_module *passdb = request->auth->passdb;
 	const char *cache_key, *result, *scheme;
-	int expired;
+
+	i_assert(request->state == AUTH_REQUEST_STATE_MECH_CONTINUE);
 
 	cache_key = passdb_cache == NULL ? NULL : passdb->cache_key;
 	if (cache_key != NULL) {
 		if (passdb_cache_lookup_credentials(request, cache_key,
-						    &result, &scheme,
-						    &expired) && !expired) {
+						    &result, &scheme, FALSE)) {
 			passdb_handle_credentials(result != NULL ?
 						  PASSDB_RESULT_OK :
 						  PASSDB_RESULT_USER_UNKNOWN,
@@ -273,6 +287,7 @@
 		}
 	}
 
+	request->state = AUTH_REQUEST_STATE_PASSDB;
 	request->credentials = credentials;
 	request->private_callback.lookup_credentials = callback;
 
--- a/src/auth/auth-request.h	Sat Mar 05 12:25:09 2005 +0200
+++ b/src/auth/auth-request.h	Sat Mar 05 13:48:13 2005 +0200
@@ -8,10 +8,19 @@
 
 struct auth_client_connection;
 
+enum auth_request_state {
+	AUTH_REQUEST_STATE_NEW,
+	AUTH_REQUEST_STATE_PASSDB,
+	AUTH_REQUEST_STATE_MECH_CONTINUE,
+	AUTH_REQUEST_STATE_FINISHED,
+	AUTH_REQUEST_STATE_USERDB
+};
+
 struct auth_request {
 	int refcount;
 
 	pool_t pool;
+        enum auth_request_state state;
 	char *user;
 	char *mech_password; /* set if verify_plain() is called */
 	char *passdb_password; /* set after password lookup if successful */
@@ -40,7 +49,6 @@
 
 	unsigned int successful:1;
 	unsigned int internal_failure:1;
-	unsigned int finished:1;
 	unsigned int delayed_failure:1;
 	unsigned int accept_input:1;
 	unsigned int no_failure_delay:1;
--- a/src/auth/passdb-blocking.c	Sat Mar 05 12:25:09 2005 +0200
+++ b/src/auth/passdb-blocking.c	Sat Mar 05 13:48:13 2005 +0200
@@ -87,6 +87,8 @@
 {
 	string_t *str;
 
+	i_assert(request->extra_fields == NULL);
+
 	str = t_str_new(64);
 	str_append(str, "PASSV\t");
 	str_append(str, request->mech_password);
@@ -118,6 +120,8 @@
 {
 	string_t *str;
 
+	i_assert(request->extra_fields == NULL);
+
 	str = t_str_new(64);
 	str_printfa(str, "PASSL\t%d\t", request->credentials);
 	auth_request_export(request, str);
--- a/src/auth/passdb-cache.c	Sat Mar 05 12:25:09 2005 +0200
+++ b/src/auth/passdb-cache.c	Sat Mar 05 13:48:13 2005 +0200
@@ -34,17 +34,17 @@
 
 int passdb_cache_verify_plain(struct auth_request *request, const char *key,
 			      const char *password,
-			      enum passdb_result *result_r, int *expired_r)
+			      enum passdb_result *result_r, int use_expired)
 {
 	const char *value, *cached_pw, *scheme, *const *list;
-	int ret;
+	int ret, expired;
 
 	if (passdb_cache == NULL)
 		return FALSE;
 
 	/* value = password \t ... */
-	value = auth_cache_lookup(passdb_cache, request, key, expired_r);
-	if (value == NULL)
+	value = auth_cache_lookup(passdb_cache, request, key, &expired);
+	if (value == NULL || (expired && !use_expired))
 		return FALSE;
 
 	if (*value == '\0') {
@@ -76,15 +76,16 @@
 
 int passdb_cache_lookup_credentials(struct auth_request *request,
 				    const char *key, const char **result_r,
-				    const char **scheme_r, int *expired_r)
+				    const char **scheme_r, int use_expired)
 {
 	const char *value, *const *list;
+	int expired;
 
 	if (passdb_cache == NULL)
 		return FALSE;
 
-	value = auth_cache_lookup(passdb_cache, request, key, expired_r);
-	if (value == NULL)
+	value = auth_cache_lookup(passdb_cache, request, key, &expired);
+	if (value == NULL || (expired && !use_expired))
 		return FALSE;
 
 	if (*value == '\0') {
--- a/src/auth/passdb-cache.h	Sat Mar 05 12:25:09 2005 +0200
+++ b/src/auth/passdb-cache.h	Sat Mar 05 13:48:13 2005 +0200
@@ -8,10 +8,10 @@
 
 int passdb_cache_verify_plain(struct auth_request *request, const char *key,
 			      const char *password,
-			      enum passdb_result *result_r, int *expired_r);
+			      enum passdb_result *result_r, int use_expired);
 int passdb_cache_lookup_credentials(struct auth_request *request,
 				    const char *key, const char **result_r,
-				    const char **scheme_r, int *expired_r);
+				    const char **scheme_r, int use_expired);
 
 void passdb_cache_init(void);
 void passdb_cache_deinit(void);