bug #199.

git-svn-id: file:///svn/unbound/trunk@1212 be551aaa-1e26-0410-a405-d3ace91eadb9

daemon/unbound.c

@@ -261,23 +261,21 @@ writepid (const char* pidfile, pid_t pid)
 
 /**
  * check old pid file.
- * @param cfg: the config settings
+ * @param pidfile: the file name of the pid file.
+ * @param inchroot: if pidfile is inchroot and we can thus expect to
+ *	be able to delete it.
  */
 static void
-checkoldpid(struct config_file* cfg)
+checkoldpid(char* pidfile, int inchroot)
 {
 	pid_t old;
-	char* file = cfg->pidfile;
-	if(cfg->chrootdir && cfg->chrootdir[0] &&
-		strncmp(file, cfg->chrootdir, strlen(cfg->chrootdir))==0) {
-		file += strlen(cfg->chrootdir);
-	}
-	if((old = readpid(file)) != -1) {
+	if((old = readpid(pidfile)) != -1) {
 		/* see if it is still alive */
 		if(kill(old, 0) == 0 || errno == EPERM)
 			log_warn("unbound is already running as pid %u.", 
 				(unsigned)old);
-		else	log_warn("did not exit gracefully last time (%u)", 
+		else	if(inchroot)
+			log_warn("did not exit gracefully last time (%u)", 
 				(unsigned)old);
 	}
 }
@@ -285,18 +283,16 @@ checkoldpid(struct config_file* cfg)
 
 /** detach from command line */
 static void
-detach(struct config_file* cfg)
+detach(void)
 {
 #ifdef HAVE_WORKING_FORK
-	int fd, err;
+	int fd;
 	/* Take off... */
 	switch (fork()) {
 		case 0:
 			break;
 		case -1:
-			err=errno;
-			unlink(cfg->pidfile);
-			fatal_exit("fork failed: %s", strerror(err));
+			fatal_exit("fork failed: %s", strerror(errno));
 		default:
 			/* exit interactive session */
 			exit(0);
@@ -313,14 +309,12 @@ detach(struct config_file* cfg)
 		if (fd > 2)
 			(void)close(fd);
 	}
-#else
-	(void)cfg;
 #endif /* HAVE_WORKING_FORK */
 }
 
 /** daemonize, drop user priviliges and chroot if needed */
 static void
-do_chroot(struct daemon* daemon, struct config_file* cfg, int debug_mode,
+perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 	char** cfgfile)
 {
 #ifdef HAVE_GETPWNAM
@@ -331,7 +325,6 @@ do_chroot(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 	memset(&gid, 112, sizeof(gid));
 	log_assert(cfg);
 
-	/* daemonize last to be able to print error to user */
 	if(cfg->username && cfg->username[0]) {
 		struct passwd *pwd;
 		if((pwd = getpwnam(cfg->username)) == NULL)
@@ -341,6 +334,69 @@ do_chroot(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 		endpwent();
 	}
 #endif
+
+	/* init syslog (as root) if needed, before daemonize, otherwise
+	 * a fork error could not be printed since daemonize closed stderr.*/
+	if(cfg->use_syslog)
+		log_init(cfg->logfile, cfg->use_syslog, cfg->chrootdir);
+	else if(cfg->logfile && cfg->logfile[0]) {
+		/* open logfile temporary with root permissions, to log
+		 * errors happening after daemonizing, that closes stderr.
+		 * After all that we reopen the logfile with less permits. */
+		char* lf = fname_after_chroot(cfg->logfile, cfg, 1);
+		if(!lf) fatal_exit("logfile malloc: out of memory");
+		log_init(lf, 0, NULL);
+		free(lf);
+	}
+
+#ifdef HAVE_KILL
+	/* check old pid file before forking */
+	if(cfg->pidfile && cfg->pidfile[0]) {
+		/* calculate position of pidfile */
+		if(cfg->pidfile[0] == '/')
+			daemon->pidfile = strdup(cfg->pidfile);
+		else	daemon->pidfile = fname_after_chroot(cfg->pidfile, 
+				cfg, 1);
+		if(!daemon->pidfile)
+			fatal_exit("pidfile alloc: out of memory");
+		checkoldpid(daemon->pidfile,
+			/* true if pidfile is inside chrootdir, or nochroot */
+			!(cfg->chrootdir && cfg->chrootdir[0]) ||
+			(cfg->chrootdir && cfg->chrootdir[0] &&
+			strncmp(daemon->pidfile, cfg->chrootdir,
+				strlen(cfg->chrootdir))==0));
+	}
+#endif
+
+	/* daemonize because pid is needed by the writepid func */
+	if(!debug_mode && cfg->do_daemonize) {
+		detach();
+	}
+
+	/* write new pidfile (while still root, so can be outside chroot) */
+#ifdef HAVE_KILL
+	if(cfg->pidfile && cfg->pidfile[0]) {
+		writepid(daemon->pidfile, getpid());
+		if(!(cfg->chrootdir && cfg->chrootdir[0]) || 
+			(cfg->chrootdir && cfg->chrootdir[0] && 
+			strncmp(daemon->pidfile, cfg->chrootdir, 
+			strlen(cfg->chrootdir))==0)) {
+			/* delete of pidfile could potentially work,
+			 * chown to get permissions */
+			if(cfg->username && cfg->username[0]) {
+			if(chown(daemon->pidfile, uid, gid) == -1) {
+				fatal_exit("cannot chown %u.%u %s: %s",
+					(unsigned)uid, (unsigned)gid,
+					daemon->pidfile, strerror(errno));
+			}
+			}
+		}
+	}
+#else
+	(void)daemon;
+#endif
+
+	/* box into the chroot */
 #ifdef HAVE_CHROOT
 	if(cfg->chrootdir && cfg->chrootdir[0]) {
 		if(chdir(cfg->chrootdir)) {
@@ -359,6 +415,7 @@ do_chroot(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 #else
 	(void)cfgfile;
 #endif
+	/* change to working directory inside chroot */
 	if(cfg->directory && cfg->directory[0]) {
 		char* dir = cfg->directory;
 		if(cfg->chrootdir && cfg->chrootdir[0] &&
@@ -373,6 +430,8 @@ do_chroot(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 			verbose(VERB_QUERY, "chdir to %s", dir);
 		}
 	}
+
+	/* drop permissions after chroot, getpwnam, pidfile, syslog done*/
 #ifdef HAVE_GETPWNAM
 	if(cfg->username && cfg->username[0]) {
 #ifdef HAVE_SETRESGID
@@ -397,31 +456,10 @@ do_chroot(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 			cfg->username);
 	}
 #endif
-#ifdef HAVE_KILL
-	/* check old pid file before forking */
-	if(cfg->pidfile && cfg->pidfile[0]) {
-		checkoldpid(cfg);
-	}
-#endif
-
-	/* init logfile just before fork */
-	log_init(cfg->logfile, cfg->use_syslog, cfg->chrootdir);
-	if(!debug_mode && cfg->do_daemonize) {
-		detach(cfg);
-	}
-#ifdef HAVE_KILL
-	if(cfg->pidfile && cfg->pidfile[0]) {
-		char* pf = cfg->pidfile;
-		if(cfg->chrootdir && cfg->chrootdir[0] &&
-			strncmp(pf, cfg->chrootdir, strlen(cfg->chrootdir))==0)
-			pf += strlen(cfg->chrootdir);
-		writepid(pf, getpid());
-		if(!(daemon->pidfile = strdup(pf)))
-			log_err("pidf: malloc failed");
-	}
-#else
-	(void)daemon;
-#endif
+	/* file logging inited after chroot,chdir,setuid is done so that 
+	 * it would succeed on SIGHUP as well */
+	if(!cfg->use_syslog)
+		log_init(cfg->logfile, cfg->use_syslog, cfg->chrootdir);
 }
 
 /**
@@ -436,12 +474,12 @@ run_daemon(char* cfgfile, int cmdline_verbose, int debug_mode)
 {
 	struct config_file* cfg = NULL;
 	struct daemon* daemon = NULL;
-	int done_chroot = 0;
+	int done_setup = 0;
 
 	if(!(daemon = daemon_init()))
 		fatal_exit("alloc failure");
 	while(!daemon->need_to_exit) {
-		if(done_chroot)
+		if(done_setup)
 			verbose(VERB_OPS, "Restart of %s.", PACKAGE_STRING);
 		else	verbose(VERB_OPS, "Start of %s.", PACKAGE_STRING);
 
@@ -459,9 +497,9 @@ run_daemon(char* cfgfile, int cmdline_verbose, int debug_mode)
 		/* prepare */
 		if(!daemon_open_shared_ports(daemon))
 			fatal_exit("could not open ports");
-		if(!done_chroot) { 
-			do_chroot(daemon, cfg, debug_mode, &cfgfile); 
-			done_chroot = 1; 
+		if(!done_setup) { 
+			perform_setup(daemon, cfg, debug_mode, &cfgfile); 
+			done_setup = 1; 
 		} else log_init(cfg->logfile, cfg->use_syslog, cfg->chrootdir);
 		/* work */
 		daemon_fork(daemon);
@@ -472,8 +510,21 @@ run_daemon(char* cfgfile, int cmdline_verbose, int debug_mode)
 		config_delete(cfg);
 	}
 	verbose(VERB_ALGO, "Exit cleanup.");
-	if(daemon->pidfile)
-		unlink(daemon->pidfile);
+	/* this unlink may not work if the pidfile is located outside
+	 * of the chroot/workdir or we no longer have permissions */
+	if(daemon->pidfile) {
+		int fd;
+		char* pf = daemon->pidfile;
+		if(cfg->chrootdir && cfg->chrootdir[0] &&
+			strncmp(pf, cfg->chrootdir, strlen(cfg->chrootdir)==0))
+			pf += strlen(cfg->chrootdir);
+		/* truncate pidfile */
+		fd = open(pf, O_WRONLY | O_TRUNC, 0644);
+		if(fd != -1)
+			close(fd);
+		/* delete pidfile */
+		unlink(pf);
+	}
 	daemon_delete(daemon);
 }
 

doc/Changelog

@@ -1,6 +1,8 @@
 27 August 2008: Wouter
 	- daemon(3) is causing problems for people. Reverting the patch.
 	  bug#200, and 199 and 203 contain sideline discussion on it.
+	- bug#199: pidfile can be outside chroot. openlog is done before
+	  chroot and drop permissions.
 
 26 August 2008: Wouter
 	- test for insecure zone when DLV is in use, also does negative cache.

doc/example.conf.in

@@ -150,15 +150,17 @@ server:
 	# chroot has been performed the now defunct portion of the config 
 	# file path is removed to be able to reread the config after a reload. 
 	#
-	# All other file paths (working dir, pidfile, logfile, roothints,
+	# All other file paths (working dir, logfile, roothints, and
 	# key files) can be specified in several ways:
 	# 	o as an absolute path relative to the new root.
 	# 	o as a relative path to the working directory.
 	# 	o as an absolute path relative to the original root.
 	# In the last case the path is adjusted to remove the unused portion.
 	#
-	# Additionally, unbound may need to access /dev/random (for entropy)
-	# and to /dev/log (if you use syslog) from inside the chroot.
+	# The pid file can be absolute and outside of the chroot, it is 
+	# written just prior to performing the chroot and dropping permissions.
+	#
+	# Additionally, unbound may need to access /dev/random (for entropy).
 	# How to do this is specific to your OS.
 	#
 	# If you give "" no chroot is performed. The path must not end in a /.
@@ -182,7 +184,7 @@ server:
 	# log to, with identity "unbound". If yes, it overrides the logfile.
 	# use-syslog: yes 
 
-	# the pid file.
+	# the pid file. Can be an absolute path outside of chroot/work dir.
 	# pidfile: "@UNBOUND_PIDFILE@"
 	
 	# file to read root hints from.

doc/unbound.conf.5.in

@@ -49,7 +49,7 @@ example.conf file with all the options.
 # unbound.conf(5) config file for unbound(8).
 server:
 	directory: "/etc/unbound"
-	username: unbound 	# make sure it can write to pidfile.
+	username: unbound
 	# make sure unbound can access entropy from inside the chroot.
 	# e.g. on linux the use these commands (on BSD, devfs(8) is used):
 	#      mount --bind -n /dev/random /etc/unbound/dev/random
@@ -253,15 +253,20 @@ commandline) as a full path from the original root. After the
 chroot has been performed the now defunct portion of the config 
 file path is removed to be able to reread the config after a reload. 
 .IP
-All other file paths (working dir, pidfile, logfile, roothints,
+All other file paths (working dir, logfile, roothints, and
 key files) can be specified in several ways:
 as an absolute path relative to the new root,
 as a relative path to the working directory, or
 as an absolute path relative to the original root.
 In the last case the path is adjusted to remove the unused portion.
 .IP
+The pidfile can be either a relative path to the working directory, or
+an absolute path relative to the original root. It is written just prior
+to chroot and dropping permissions. This allows the pidfile to be 
+/var/run/unbound.pid and the chroot to be /var/unbound, for example.
+.IP
 Additionally, unbound may need to access /dev/random (for entropy)
-and to /dev/log (if you use syslog) from inside the chroot.
+from inside the chroot.
 .IP
 If given a chroot is done to the given directory. The default is 
 "@UNBOUND_CHROOT_DIR@". If you give "" no chroot is performed.

smallapp/unbound-checkconf.c

@@ -216,64 +216,17 @@ is_dir(const char* fname)
 	return 1;
 }
 
-/** convert a filename to full pathname in original filesys
- * @param fname: the path name to convert.
- * 	Must not be null or empty.
- * @param cfg: config struct for chroot and chdir (if set).
- * @param use_chdir: if false, only chroot is applied.
- * @return pointer to static buffer which is: [chroot][chdir]fname
- */
-static char*
-fname_after_chroot(const char* fname, struct config_file* cfg, int use_chdir)
-{
-	static char buf[1024];
-	int slashit = 0;
-	buf[0] = 0;
-	if(cfg->chrootdir && cfg->chrootdir[0] && 
-		strncmp(cfg->chrootdir, fname, strlen(cfg->chrootdir)) == 0) {
-		/* already full pathname, return it */
-		strncpy(buf, fname, sizeof(buf)-1);
-		buf[sizeof(buf)-1] = 0;
-		return buf;
-	}
-	/* chroot */
-	if(cfg->chrootdir && cfg->chrootdir[0]) {
-		/* start with chrootdir */
-		strncpy(buf, cfg->chrootdir, sizeof(buf)-1);
-		slashit = 1;
-	}
-	/* chdir */
-	if(fname[0] == '/' || !use_chdir) {
-		/* full path, no chdir */
-	} else if(cfg->directory && cfg->directory[0]) {
-		/* prepend chdir */
-		if(slashit && cfg->directory[0] != '/')
-			strncat(buf, "/", sizeof(buf)-strlen(buf)-1);
-		if(strncmp(cfg->chrootdir, cfg->directory, 
-			strlen(cfg->chrootdir)) == 0)
-			strncat(buf, cfg->directory+strlen(cfg->chrootdir), 
-				   sizeof(buf)-strlen(buf)-1);
-		else strncat(buf, cfg->directory, sizeof(buf)-strlen(buf)-1);
-		slashit = 1;
-	}
-	/* fname */
-	if(slashit && fname[0] != '/')
-		strncat(buf, "/", sizeof(buf)-strlen(buf)-1);
-	strncat(buf, fname, sizeof(buf)-strlen(buf)-1);
-	buf[sizeof(buf)-1] = 0;
-	return buf;
-}
-
 /** get base dir of a fname */
 static char*
-basedir(const char* fname, struct config_file* cfg)
+basedir(char* fname)
 {
-	char* d = fname_after_chroot(fname, cfg, 1);
-	char* rev = strrchr(d, '/');
+	char* rev;
+	if(!fname) fatal_exit("out of memory");
+	rev = strrchr(fname, '/');
 	if(!rev) return NULL;
-	if(d == rev) return NULL;
+	if(fname == rev) return NULL;
 	rev[0] = 0;
-	return d;
+	return fname;
 }
 
 /** check chroot for a file string */
@@ -283,12 +236,13 @@ check_chroot_string(const char* desc, char** ss,
 {
 	char* str = *ss;
 	if(str && str[0]) {
-		if(!is_file(fname_after_chroot(str, cfg, 1))) {
+		*ss = fname_after_chroot(str, cfg, 1);
+		if(!*ss) fatal_exit("out of memory");
+		if(!is_file(*ss)) {
 			fatal_exit("%s: \"%s\" does not exist in chrootdir %s", 
 				desc, str, chrootdir);
 		}
 		/* put in a new full path for continued checking */
-		*ss = strdup(fname_after_chroot(str, cfg, 1));
 		free(str);
 	}
 }
@@ -343,21 +297,28 @@ morechecks(struct config_file* cfg, char* fname)
 			fatal_exit("config file %s is not inside chroot %s",
 				buf, cfg->chrootdir);
 	}
-	if(cfg->directory && cfg->directory[0] && !is_dir(
-		fname_after_chroot(cfg->directory, cfg, 0))) {
-		fatal_exit("bad chdir directory");
+	if(cfg->directory && cfg->directory[0]) {
+		char* ad = fname_after_chroot(cfg->directory, cfg, 0);
+		if(!ad) fatal_exit("out of memory");
+		if(!is_dir(ad)) fatal_exit("bad chdir directory");
+		free(ad);
 	}
 	if( (cfg->chrootdir && cfg->chrootdir[0]) ||
 	    (cfg->directory && cfg->directory[0])) {
-		if(cfg->pidfile && cfg->pidfile[0] &&
-		   basedir(cfg->pidfile, cfg) &&
-		   !is_dir(basedir(cfg->pidfile, cfg))) {
-			fatal_exit("pidfile directory does not exist");
+		if(cfg->pidfile && cfg->pidfile[0]) {
+			char* ad = (cfg->pidfile[0]=='/')?strdup(cfg->pidfile):
+				fname_after_chroot(cfg->pidfile, cfg, 1);
+			char* bd = basedir(ad);
+			if(bd && !is_dir(bd))
+				fatal_exit("pidfile directory does not exist");
+			free(ad);
 		}
-		if(cfg->logfile && cfg->logfile[0] &&
-		   basedir(cfg->logfile, cfg) &&
-		   !is_dir(basedir(cfg->logfile, cfg))) {
-			fatal_exit("logfile directory does not exist");
+		if(cfg->logfile && cfg->logfile[0]) {
+			char* ad = fname_after_chroot(cfg->logfile, cfg, 1);
+			char* bd = basedir(ad);
+			if(bd && !is_dir(bd))
+				fatal_exit("logfile directory does not exist");
+			free(ad);
 		}
 	}
 

util/config_file.c

@@ -739,3 +739,96 @@ config_apply(struct config_file* config)
 	MAX_TTL = (uint32_t)config->max_ttl;
 }
 
+/** 
+ * Calculate string length of full pathname in original filesys
+ * @param fname: the path name to convert.
+ * 	Must not be null or empty.
+ * @param cfg: config struct for chroot and chdir (if set).
+ * @param use_chdir: if false, only chroot is applied.
+ * @return length of string.
+ *	remember to allocate one more for 0 at end in mallocs.
+ */
+static size_t
+strlen_after_chroot(const char* fname, struct config_file* cfg, int use_chdir)
+{
+	size_t len = 0;
+	int slashit = 0;
+	if(cfg->chrootdir && cfg->chrootdir[0] && 
+		strncmp(cfg->chrootdir, fname, strlen(cfg->chrootdir)) == 0) {
+		/* already full pathname, return it */
+		return strlen(fname);
+	}
+	/* chroot */
+	if(cfg->chrootdir && cfg->chrootdir[0]) {
+		/* start with chrootdir */
+		len += strlen(cfg->chrootdir);
+		slashit = 1;
+	}
+	/* chdir */
+	if(fname[0] == '/' || !use_chdir) {
+		/* full path, no chdir */
+	} else if(cfg->directory && cfg->directory[0]) {
+		/* prepend chdir */
+		if(slashit && cfg->directory[0] != '/')
+			len++;
+		if(cfg->chrootdir && cfg->chrootdir[0] && 
+			strncmp(cfg->chrootdir, cfg->directory, 
+			strlen(cfg->chrootdir)) == 0)
+			len += strlen(cfg->directory)-strlen(cfg->chrootdir);
+		else	len += strlen(cfg->directory);
+		slashit = 1;
+	}
+	/* fname */
+	if(slashit && fname[0] != '/')
+		len++;
+	len += strlen(fname);
+	return len;
+}
+
+char*
+fname_after_chroot(const char* fname, struct config_file* cfg, int use_chdir)
+{
+	size_t len = strlen_after_chroot(fname, cfg, use_chdir);
+	int slashit = 0;
+	char* buf = (char*)malloc(len+1);
+	if(!buf)
+		return NULL;
+	buf[0] = 0;
+	/* is fname already in chroot ? */
+	if(cfg->chrootdir && cfg->chrootdir[0] && 
+		strncmp(cfg->chrootdir, fname, strlen(cfg->chrootdir)) == 0) {
+		/* already full pathname, return it */
+		strncpy(buf, fname, len);
+		buf[len] = 0;
+		return buf;
+	}
+	/* chroot */
+	if(cfg->chrootdir && cfg->chrootdir[0]) {
+		/* start with chrootdir */
+		strncpy(buf, cfg->chrootdir, len);
+		slashit = 1;
+	}
+	/* chdir */
+	if(fname[0] == '/' || !use_chdir) {
+		/* full path, no chdir */
+	} else if(cfg->directory && cfg->directory[0]) {
+		/* prepend chdir */
+		if(slashit && cfg->directory[0] != '/')
+			strncat(buf, "/", len-strlen(buf));
+		/* is the directory already in the chroot? */
+		if(cfg->chrootdir && cfg->chrootdir[0] && 
+			strncmp(cfg->chrootdir, cfg->directory, 
+			strlen(cfg->chrootdir)) == 0)
+			strncat(buf, cfg->directory+strlen(cfg->chrootdir), 
+				   len-strlen(buf));
+		else strncat(buf, cfg->directory, len-strlen(buf));
+		slashit = 1;
+	}
+	/* fname */
+	if(slashit && fname[0] != '/')
+		strncat(buf, "/", len-strlen(buf));
+	strncat(buf, fname, len-strlen(buf));
+	buf[len] = 0;
+	return buf;
+}
+

util/config_file.h

@@ -384,6 +384,18 @@ int cfg_condense_ports(struct config_file* cfg, int** avail);
  */
 int cfg_scan_ports(int* avail, int num);
 
+/** 
+ * Convert a filename to full pathname in original filesys
+ * @param fname: the path name to convert.
+ *      Must not be null or empty.
+ * @param cfg: config struct for chroot and chdir (if set).
+ * @param use_chdir: if false, only chroot is applied.
+ * @return pointer to malloced buffer which is: [chroot][chdir]fname
+ *      or NULL on malloc failure.
+ */
+char* fname_after_chroot(const char* fname, struct config_file* cfg, 
+	int use_chdir);
+
 /**
  * Used during options parsing
  */