[dovecot-cvs] dovecot/src/master auth-process.c,1.17,1.18 common.h,1.6,1.7 imap-process.c,1.14,1.15 login-process.c,1.19,1.20 main.c,1.17,1.18

[cras at procontrol.fi]

Update of /home/cvs/dovecot/src/master
In directory danu:/tmp/cvs-serv2284/master

Modified Files:
	auth-process.c common.h imap-process.c login-process.c main.c 
Log Message:
Drop root privileges earlier. Close syslog more later in imap-master when   
forking new processes, so that any errors get logged. Make sure that all   
errors show up in log files - use specific exit status codes if we can't
write to log file. Make sure imap and login processes always drop root
privileges even if master process didn't ask for it for some reason.
putenv() wasn't verified to succeed - luckily we never allowed large user
given data there.     



Index: auth-process.c
===================================================================
RCS file: /home/cvs/dovecot/src/master/auth-process.c,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -d -r1.17 -r1.18
--- auth-process.c	17 Dec 2002 03:00:44 -0000	1.17
+++ auth-process.c	18 Dec 2002 04:00:01 -0000	1.18
@@ -13,6 +13,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <pwd.h>
+#include <syslog.h>
 #include <sys/stat.h>
 
 typedef struct _WaitingRequest WaitingRequest;
@@ -255,13 +256,17 @@
 
 	restrict_process_size(config->process_size);
 
+	/* make sure we don't leak syslog fd, but do it last so that
+	   any errors above will be logged */
+	closelog();
+
 	/* hide the path, it's ugly */
 	argv[0] = strrchr(config->executable, '/');
 	if (argv[0] == NULL) argv[0] = config->executable; else argv[0]++;
 
 	execv(config->executable, (char **) argv);
 
-	i_fatal("execv(%s) failed: %m", argv[0]);
+	i_fatal_status(FATAL_EXEC, "execv(%s) failed: %m", argv[0]);
 	return -1;
 }
 

Index: common.h
===================================================================
RCS file: /home/cvs/dovecot/src/master/common.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- common.h	17 Dec 2002 03:00:44 -0000	1.6
+++ common.h	18 Dec 2002 04:00:01 -0000	1.7
@@ -37,7 +37,8 @@
 				      const char *system_user,
 				      const char *virtual_user,
 				      uid_t uid, gid_t gid, const char *home,
-				      int chroot, const char *env[]);
+				      int chroot, const char *mail,
+				      const char *login_tag);
 void imap_process_destroyed(pid_t pid);
 
 /* misc */

Index: imap-process.c
===================================================================
RCS file: /home/cvs/dovecot/src/master/imap-process.c,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -d -r1.14 -r1.15
--- imap-process.c	17 Dec 2002 03:00:44 -0000	1.14
+++ imap-process.c	18 Dec 2002 04:00:01 -0000	1.15
@@ -8,9 +8,9 @@
 
 #include <stdlib.h>
 #include <unistd.h>
-#include <sys/stat.h>
-#include <syslog.h>
 #include <grp.h>
+#include <syslog.h>
+#include <sys/stat.h>
 
 static unsigned int imap_process_count = 0;
 
@@ -105,12 +105,13 @@
 				      const char *system_user,
 				      const char *virtual_user,
 				      uid_t uid, gid_t gid, const char *home,
-				      int chroot, const char *env[])
+				      int chroot, const char *mail,
+				      const char *login_tag)
 {
-	static char *argv[] = { NULL, "-s", NULL, NULL };
+	static char *argv[] = { NULL, NULL, NULL };
 	char host[MAX_IP_LEN], title[1024];
 	pid_t pid;
-	int i, j, err, found_mail;
+	int i, j, err;
 
 	if (imap_process_count == set_max_imap_processes) {
 		i_error("Maximum number of imap processes exceeded");
@@ -151,28 +152,11 @@
 	}
 	(void)close(socket);
 
-	/* setup environment */
-        found_mail = FALSE;
-	for (; env[0] != NULL && env[1] != NULL; env += 2) {
-		if (strcmp(env[0], "MAIL") == 0) {
-			if (env[1] == NULL || *env[1] == '\0')
-				continue;
-
-			found_mail = TRUE;
-		}
-
-		env_put(t_strconcat(env[0], "=", env[1], NULL));
-	}
-
-	if (!found_mail && set_default_mail_env != NULL) {
-		const char *mail;
-
-		mail = expand_mail_env(set_default_mail_env,
-				       virtual_user, home);
-		env_put(t_strconcat("MAIL=", mail, NULL));
-	}
+	/* setup environment - set the most important environment first
+	   (paranoia about filling up environment without noticing) */
+	restrict_access_set_env(system_user, uid, gid, chroot ? home : NULL);
+	restrict_process_size(set_imap_process_size);
 
-	env_put(t_strconcat("USER=", virtual_user, NULL));
 	env_put(t_strconcat("HOME=", home, NULL));
 	env_put(t_strconcat("MAIL_CACHE_FIELDS=", set_mail_cache_fields, NULL));
 	env_put(t_strconcat("MAIL_NEVER_CACHE_FIELDS=",
@@ -200,16 +184,27 @@
 	if (set_mbox_read_dotlock)
 		env_put("MBOX_READ_DOTLOCK=1");
 
+	/* user given environment - may be malicious. virtual_user comes from
+	   auth process, but don't trust that too much either. Some auth
+	   mechanism might allow leaving extra data there. */
+	if (mail == NULL && set_default_mail_env != NULL) {
+		mail = expand_mail_env(set_default_mail_env,
+				       virtual_user, home);
+		env_put(t_strconcat("MAIL=", mail, NULL));
+	}
+
+	env_put(t_strconcat("MAIL=", mail, NULL));
+	env_put(t_strconcat("USER=", virtual_user, NULL));
+	env_put(t_strconcat("LOGIN_TAG=", login_tag, NULL));
+
 	if (set_verbose_proctitle && net_ip2host(ip, host) == 0) {
 		i_snprintf(title, sizeof(title), "[%s %s]", virtual_user, host);
-		argv[2] = title;
+		argv[1] = title;
 	}
 
-	/* setup access environment - needs to be done after
-	   clean_child_process() since it clears environment */
-	restrict_access_set_env(system_user, uid, gid, chroot ? home : NULL);
-
-	restrict_process_size(set_imap_process_size);
+	/* make sure we don't leak syslog fd, but do it last so that
+	   any errors above will be logged */
+	closelog();
 
 	/* hide the path, it's ugly */
 	argv[0] = strrchr(set_imap_executable, '/');
@@ -221,7 +216,7 @@
 	for (i = 0; i < 3; i++)
 		(void)close(i);
 
-	i_fatal("execv(%s) failed: %m", set_imap_executable);
+	i_fatal_status(FATAL_EXEC, "execv(%s) failed: %m", set_imap_executable);
 
 	/* not reached */
 	return 0;

Index: login-process.c
===================================================================
RCS file: /home/cvs/dovecot/src/master/login-process.c,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -d -r1.19 -r1.20
--- login-process.c	17 Dec 2002 03:00:44 -0000	1.19
+++ login-process.c	18 Dec 2002 04:00:01 -0000	1.20
@@ -53,18 +53,10 @@
 
 static void auth_callback(AuthCookieReplyData *cookie_reply, void *context)
 {
-	const char *env[] = {
-		"MAIL", NULL,
-		"LOGIN_TAG", NULL,
-		NULL
-	};
 	LoginAuthRequest *request = context;
         LoginProcess *process;
 	MasterReply reply;
 
-	env[1] = cookie_reply->mail;
-	env[3] = request->login_tag;
-
 	if (cookie_reply == NULL || !cookie_reply->success)
 		reply.result = MASTER_RESULT_FAILURE;
 	else {
@@ -75,7 +67,9 @@
 						   cookie_reply->uid,
 						   cookie_reply->gid,
 						   cookie_reply->home,
-						   cookie_reply->chroot, env);
+						   cookie_reply->chroot,
+						   cookie_reply->mail,
+						   request->login_tag);
 	}
 
 	/* reply to login */
@@ -331,13 +325,17 @@
 
 	restrict_process_size(set_login_process_size);
 
+	/* make sure we don't leak syslog fd, but do it last so that
+	   any errors above will be logged */
+	closelog();
+
 	/* hide the path, it's ugly */
 	argv[0] = strrchr(set_login_executable, '/');
 	if (argv[0] == NULL) argv[0] = set_login_executable; else argv[0]++;
 
 	execv(set_login_executable, (char **) argv);
 
-	i_fatal("execv(%s) failed: %m", argv[0]);
+	i_fatal_status(FATAL_EXEC, "execv(%s) failed: %m", argv[0]);
 	return -1;
 }
 

Index: main.c
===================================================================
RCS file: /home/cvs/dovecot/src/master/main.c,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -d -r1.17 -r1.18
--- main.c	12 Dec 2002 18:33:32 -0000	1.17
+++ main.c	18 Dec 2002 04:00:01 -0000	1.18
@@ -54,10 +54,11 @@
 	/* set the failure log */
 	if (set_log_path != NULL)
 		env_put(t_strconcat("IMAP_LOGFILE=", set_log_path, NULL));
+	else
+		env_put("IMAP_USE_SYSLOG=1");
+
 	if (set_log_timestamp != NULL)
 		env_put(t_strconcat("IMAP_LOGSTAMP=", set_log_timestamp, NULL));
-
-	closelog();
 }
 
 static void sig_quit(int signo __attr_unused__)
@@ -76,10 +77,29 @@
         auth_processes_destroy_all();
 }
 
+static const char *get_exit_status_message(FatalExitStatus status)
+{
+	switch (status) {
+	case FATAL_LOGOPEN:
+		return "Can't open log file";
+	case FATAL_LOGWRITE:
+		return "Can't write to log file";
+	case FATAL_OUTOFMEM:
+		return "Out of memory";
+	case FATAL_EXEC:
+		return "exec() failed";
+
+	case FATAL_DEFAULT:
+		return NULL;
+	}
+
+	return NULL;
+}
+
 static void timeout_handler(void *context __attr_unused__,
 			    Timeout timeout __attr_unused__)
 {
-	const char *process_type_name;
+	const char *process_type_name, *msg;
 	pid_t pid;
 	int status, process_type;
 
@@ -104,8 +124,12 @@
 			status = WEXITSTATUS(status);
 			if (status != 0) {
 				login_process_abormal_exit(pid);
-				i_error("child %d (%s) returned error %d",
-					(int)pid, process_type_name, status);
+				msg = get_exit_status_message(status);
+				if (msg != NULL)
+					msg = t_strconcat(" (", msg, ")", NULL);
+				i_error("child %d (%s) returned error %d%s",
+					(int)pid, process_type_name,
+					status, msg);
 			}
 		} else if (WIFSIGNALED(status)) {
 			login_process_abormal_exit(pid);
@@ -182,25 +206,25 @@
 	fd_close_on_exec(imaps_fd, TRUE);
 }
 
-static void main_init(void)
+static void open_logfile(void)
 {
-	lib_init_signals(sig_quit);
+	if (set_log_path == NULL)
+		i_set_failure_syslog("imap-master", LOG_NDELAY, LOG_MAIL);
+	else {
+		/* log to file or stderr */
+		i_set_failure_file(set_log_path, "imap-master");
+		i_set_failure_timestamp_format(set_log_timestamp);
+	}
+}
 
+static void main_init(void)
+{
 	/* deny file access from everyone else except owner */
         (void)umask(0077);
 
-	if (set_log_path == NULL) {
-		openlog("imap-master", LOG_NDELAY, LOG_MAIL);
+	open_logfile();
 
-		i_set_panic_handler(i_syslog_panic_handler);
-		i_set_fatal_handler(i_syslog_fatal_handler);
-		i_set_error_handler(i_syslog_error_handler);
-		i_set_warning_handler(i_syslog_warning_handler);
-	} else {
-		/* log failures into specified log file */
-		i_set_failure_file(set_log_path, "imap-master");
-		i_set_failure_timestamp_format(set_log_timestamp);
-	}
+	lib_init_signals(sig_quit);
 
 	pids = hash_create(default_pool, 128, NULL, NULL);
 	to = timeout_add(100, timeout_handler, NULL);
@@ -240,7 +264,8 @@
 	if (pid != 0)
 		_exit(0);
 
-	setsid();
+	if (setsid() < 0)
+		i_fatal("setsid() failed: %m");
 }
 
 static void print_help(void)