dovecot-2.2: auth: checkpassword code cleanup. Also fixed some e...

dovecot at dovecot.org dovecot at dovecot.org
Sun May 20 03:26:26 EEST 2012


details:   http://hg.dovecot.org/dovecot-2.2/rev/71b64b7b2e63
changeset: 14311:71b64b7b2e63
user:      Timo Sirainen <tss at iki.fi>
date:      Sat Mar 10 13:37:39 2012 +0200
description:
auth: checkpassword code cleanup. Also fixed some error handling.
Removes a lot of copy&pasted code between passdb and userdb.

diffstat:

 src/auth/checkpassword-reply.c  |   13 +-
 src/auth/db-checkpassword.c     |  426 ++++++++++++++++++++++++++++++++++-----
 src/auth/db-checkpassword.h     |   64 +----
 src/auth/passdb-checkpassword.c |  248 +---------------------
 src/auth/userdb-checkpassword.c |  232 ++-------------------
 5 files changed, 443 insertions(+), 540 deletions(-)

diffs (truncated from 1209 to 300 lines):

diff -r 7a26c427fc78 -r 71b64b7b2e63 src/auth/checkpassword-reply.c
--- a/src/auth/checkpassword-reply.c	Thu Mar 08 16:03:45 2012 +0200
+++ b/src/auth/checkpassword-reply.c	Sat Mar 10 13:37:39 2012 +0200
@@ -54,10 +54,21 @@
 	if (!gid_found)
 		str_printfa(str, "userdb_gid=%s\t",  dec2str(getgid()));
 
+	i_assert(str_len(str) > 0);
+
 	if (write_full(4, str_data(str), str_len(str)) < 0) {
 		i_error("checkpassword: write_full() failed: %m");
 		exit(111);
 	}
 	authorized = getenv("AUTHORIZED");
-	return authorized != NULL && strcmp(authorized, "2") == 0 ? 2 : 0;
+	if (authorized == NULL) {
+		/* authentication */
+		return 0;
+	} else if (strcmp(authorized, "2") == 0) {
+		/* successful passdb/userdb lookup */
+		return 2;
+	} else {
+		i_error("checkpassword: Script doesn't support passdb/userdb lookup");
+		return 111;
+	}
 }
diff -r 7a26c427fc78 -r 71b64b7b2e63 src/auth/db-checkpassword.c
--- a/src/auth/db-checkpassword.c	Thu Mar 08 16:03:45 2012 +0200
+++ b/src/auth/db-checkpassword.c	Sat Mar 10 13:37:39 2012 +0200
@@ -4,9 +4,47 @@
 
 #if defined(PASSDB_CHECKPASSWORD) || defined(USERDB_CHECKPASSWORD)
 
+#include "lib-signals.h"
+#include "buffer.h"
+#include "str.h"
+#include "ioloop.h"
+#include "hash.h"
+#include "execv-const.h"
+#include "env-util.h"
+#include "safe-memset.h"
+#include "child-wait.h"
 #include "var-expand.h"
 #include "db-checkpassword.h"
 
+#include <stdlib.h>
+#include <unistd.h>
+
+struct chkpw_auth_request {
+	struct db_checkpassword *db;
+	struct auth_request *request;
+	char *auth_password;
+
+	db_checkpassword_callback_t *callback;
+	void *context;
+
+	pid_t pid;
+	int fd_out, fd_in;
+	struct io *io_out, *io_in;
+
+	string_t *input_buf;
+	unsigned int write_pos;
+
+	int exit_status;
+	unsigned int exited:1;
+};
+
+struct db_checkpassword {
+	char *checkpassword_path, *checkpassword_reply_path;
+
+	struct hash_table *clients;
+	struct child_wait *child_wait;
+};
+
 static void env_put_extra_fields(const char *extra_fields)
 {
 	const char *const *tmp;
@@ -40,52 +78,143 @@
 	}
 }
 
-void checkpassword_request_free(struct chkpw_auth_request *request)
+static void checkpassword_request_free(struct chkpw_auth_request **_request)
 {
+	struct chkpw_auth_request *request = *_request;
+
+	*_request = NULL;
+
+	if (!request->exited)
+		child_wait_remove_pid(request->db->child_wait, request->pid);
 	checkpassword_request_close(request);
-	if (request->input_buf != NULL)
-		str_free(&request->input_buf);
 
-	if (request->password != NULL) {
-		safe_memset(request->password, 0, strlen(request->password));
-		i_free(request->password);
+	if (request->auth_password != NULL) {
+		safe_memset(request->auth_password, 0,
+			    strlen(request->auth_password));
+		i_free(request->auth_password);
 	}
+	auth_request_unref(&request->request);
+	str_free(&request->input_buf);
 	i_free(request);
 }
 
-enum checkpassword_sigchld_handler_result
-checkpassword_sigchld_handler(const struct child_wait_status *child_wait_status,
-			      struct chkpw_auth_request *request)
+static void checkpassword_finish(struct chkpw_auth_request **_request,
+				 enum db_checkpassword_status status)
 {
-	int status = child_wait_status->status;
-	pid_t pid = child_wait_status->pid;
+	struct chkpw_auth_request *request = *_request;
 
-	if (request == NULL) {
-		i_error("checkpassword: sighandler called for unknown child %s",
-			dec2str(pid));
-		return SIGCHLD_RESULT_UNKNOWN_CHILD;
+	*_request = NULL;
+	request->callback(request->request, status, request->context);
+	checkpassword_request_free(&request);
+}
+
+static void checkpassword_internal_failure(struct chkpw_auth_request **request)
+{
+	checkpassword_finish(request, DB_CHECKPASSWORD_STATUS_INTERNAL_FAILURE);
+}
+
+static void
+checkpassword_request_finish_auth(struct chkpw_auth_request *request)
+{
+	switch (request->exit_status) {
+	/* vpopmail exit codes: */
+	case 3:		/* password fail / vpopmail user not found */
+	case 12: 	/* null user name given */
+	case 13:	/* null password given */
+	case 15:	/* user has no password */
+	case 20:	/* invalid user/domain characters */
+	case 21:	/* system user not found */
+	case 22:	/* system user shadow entry not found */
+	case 23:	/* system password fail */
+
+	/* standard checkpassword exit codes: */
+	case 1:
+		/* (1 is additionally defined in vpopmail for
+		   "pop/smtp/webmal/ imap/access denied") */
+		auth_request_log_info(request->request, "checkpassword",
+				      "Login failed (status=%d)",
+				      request->exit_status);
+		checkpassword_finish(&request, DB_CHECKPASSWORD_STATUS_FAILURE);
+		break;
+	case 0:
+		if (request->input_buf->used == 0) {
+			auth_request_log_error(request->request, "checkpassword",
+					       "Received no input");
+			checkpassword_internal_failure(&request);
+			break;
+		}
+
+		auth_request_set_fields(request->request,
+			t_strsplit(str_c(request->input_buf), "\t"), NULL);
+		checkpassword_finish(&request, DB_CHECKPASSWORD_STATUS_OK);
+		break;
+	case 2:
+		/* checkpassword is called with wrong parameters? unlikely */
+		auth_request_log_error(request->request, "checkpassword",
+			"Child %s exited with status 2 (tried to use "
+			"userdb-only checkpassword program for passdb?)",
+			dec2str(request->pid));
+		checkpassword_internal_failure(&request);
+		break;
+	case 111:
+		/* temporary problem, treat as internal error */
+	default:
+		/* whatever error.. */
+		auth_request_log_error(request->request, "checkpassword",
+			"Child %s exited with status %d",
+			dec2str(request->pid), request->exit_status);
+		checkpassword_internal_failure(&request);
+		break;
 	}
+}
 
-	if (WIFSIGNALED(status)) {
-		i_error("checkpassword: Child %s died with signal %d",
-			dec2str(pid), WTERMSIG(status));
-		return SIGCHLD_RESULT_DEAD_CHILD;
-	} else if (WIFEXITED(status)) {
-		request->exited = TRUE;
-		request->exit_status = WEXITSTATUS(status);
+static void
+checkpassword_request_finish_lookup(struct chkpw_auth_request *request)
+{
+	switch (request->exit_status) {
+	case 3:
+		/* User does not exist. */
+		auth_request_log_info(request->request, "userdb-checkpassword",
+				      "User unknown");
+		checkpassword_finish(&request, DB_CHECKPASSWORD_STATUS_FAILURE);
+		break;
+	case 2:
+		/* This is intentionally not 0. checkpassword-reply exits with
+		   2 on success when AUTHORIZED is set. */
+		if (request->input_buf->used == 0) {
+			auth_request_log_error(request->request, "checkpassword",
+					       "Received no input");
+			checkpassword_internal_failure(&request);
+			break;
+		}
 
-		auth_request_log_debug(request->request,
-				       "checkpassword", "exit_status=%d",
-				       request->exit_status);
-		return SIGCHLD_RESULT_OK;
-	} else {
-		/* shouldn't happen */
-		auth_request_log_debug(request->request, "checkpassword",
-				       "Child exited with status=%d", status);
-		return SIGCHLD_RESULT_UNKNOWN_ERROR;
+		auth_request_set_fields(request->request,
+			t_strsplit(str_c(request->input_buf), "\t"), NULL);
+		checkpassword_finish(&request, DB_CHECKPASSWORD_STATUS_OK);
+		break;
+	default:
+		/* whatever error... */
+		auth_request_log_error(request->request, "userdb-checkpassword",
+			"Child %s exited with status %d",
+			dec2str(request->pid), request->exit_status);
+		checkpassword_internal_failure(&request);
+		break;
 	}
 }
 
+static void
+checkpassword_request_half_finish(struct chkpw_auth_request *request)
+{
+	/* the process must have exited, and the input fd must have closed */
+	if (!request->exited || request->fd_in != -1)
+		return;
+
+	if (request->auth_password != NULL)
+		checkpassword_request_finish_auth(request);
+	else
+		checkpassword_request_finish_lookup(request);
+}
+
 static void env_put_auth_vars(struct auth_request *request)
 {
 	const struct var_expand_table *tab;
@@ -101,7 +230,7 @@
 	}
 }
 
-void checkpassword_setup_env(struct auth_request *request)
+static void checkpassword_setup_env(struct auth_request *request)
 {
 	/* Besides passing the standard username and password in a
 	   pipe, also pass some other possibly interesting information
@@ -146,7 +275,7 @@
 	env_put_auth_vars(request);
 }
 
-const char *
+static const char *
 checkpassword_get_cmd(struct auth_request *request, const char *args,
 		      const char *checkpassword_reply_path)
 {
@@ -158,33 +287,34 @@
 	return t_strconcat(str_c(str), " ", checkpassword_reply_path, NULL);
 }
 
-void checkpassword_child_input(struct chkpw_auth_request *request)
+static void checkpassword_child_input(struct chkpw_auth_request *request)
 {
 	unsigned char buf[1024];
 	ssize_t ret;
 
 	ret = read(request->fd_in, buf, sizeof(buf));
-	if (ret <= 0) {
-		if (ret < 0) {
-			auth_request_log_error(request->request,
-				"checkpassword", "read() failed: %m");
-		}
+	if (ret > 0) {
+		str_append_n(request->input_buf, buf, ret);
+		return;
+	}
 
-		auth_request_log_debug(request->request, "checkpassword",
-				       "Received no input");
-		checkpassword_request_close(request);
-		request->half_finish_callback(request);
+	if (ret < 0) {
+		auth_request_log_error(request->request,
+			"checkpassword", "read() failed: %m");
+		checkpassword_internal_failure(&request);
+	} else if (strchr(str_c(request->input_buf), '\n') != NULL) {


More information about the dovecot-cvs mailing list