From: W.C.A. Wijngaards <wouter@nlnetlabs.nl>
Date: Wed, 27 Mar 2024 13:52:25 +0000 (+0100)
Subject: - Fix that the server does not chown the pidfile.
X-Git-Tag: release-1.20.0rc1~46
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6f82b5be4aeedf0daad6d3b0a7b529466d380697;p=thirdparty%2Funbound.git

- Fix that the server does not chown the pidfile.
---

diff --git a/contrib/rc_d_unbound b/contrib/rc_d_unbound
index 56516147f..9d98c5e05 100755
--- a/contrib/rc_d_unbound
+++ b/contrib/rc_d_unbound
@@ -22,4 +22,13 @@ pidfile=${unbound_pidfile:-"/usr/local/etc/unbound/unbound.pid"}
 command_args=${unbound_flags:-"-c /usr/local/etc/unbound/unbound.conf"}
 extra_commands="reload"
 
+if test "$1" = "stop" ; then
+	run_rc_command "$1"
+	ret=$?
+	if test $ret -eq 0; then
+		rm -f "$pidfile"
+	fi
+	exit $ret
+fi
+
 run_rc_command "$1"
diff --git a/contrib/unbound.init b/contrib/unbound.init
index c5bb52bb4..70ab0134e 100644
--- a/contrib/unbound.init
+++ b/contrib/unbound.init
@@ -75,6 +75,7 @@ stop() {
     retval=$?
     echo
     [ $retval -eq 0 ] && rm -f $lockfile
+    [ $retval -eq 0 ] && rm -f $pidfile
     if egrep -q '^/[^[:space:]]+[[:space:]]+'${rootdir}'/dev/log' /proc/mounts; then
 	umount ${rootdir}/dev/log >/dev/null 2>&1
     fi;
diff --git a/contrib/unbound.init_fedora b/contrib/unbound.init_fedora
index 989440341..75856777f 100644
--- a/contrib/unbound.init_fedora
+++ b/contrib/unbound.init_fedora
@@ -58,6 +58,7 @@ stop() {
     killproc -p $pidfile unbound
     retval=$?
     [ $retval -eq 0 ] && rm -f $lockfile
+    [ $retval -eq 0 ] && rm -f $pidfile
     for mountfile in /dev/log /dev/urandom /etc/localtime /etc/resolv.conf /var/run/unbound
     do
     if egrep -q '^/[^[:space:]]+[[:space:]]+'${rootdir}''${mountfile}'' /proc/mounts; then
diff --git a/contrib/unbound.init_yocto b/contrib/unbound.init_yocto
index 4eba752bc..e1a812448 100644
--- a/contrib/unbound.init_yocto
+++ b/contrib/unbound.init_yocto
@@ -75,6 +75,7 @@ stop() {
     retval=$?
     echo
     [ $retval -eq 0 ] && rm -f $lockfile
+    [ $retval -eq 0 ] && rm -f $pidfile
     if egrep -q '^/[^[:space:]]+[[:space:]]+'${rootdir}'/dev/log' /proc/mounts; then
 	umount ${rootdir}/dev/log >/dev/null 2>&1
     fi;
diff --git a/daemon/unbound.c b/daemon/unbound.c
index b5c7a0d20..d6c371571 100644
--- a/daemon/unbound.c
+++ b/daemon/unbound.c
@@ -366,9 +366,8 @@ readpid (const char* file)
 /** write pid to file. 
  * @param pidfile: file name of pid file.
  * @param pid: pid to write to file.
- * @return false on failure
  */
-static int
+static void
 writepid (const char* pidfile, pid_t pid)
 {
 	int fd;
@@ -383,7 +382,7 @@ writepid (const char* pidfile, pid_t pid)
 		, 0644)) == -1) {
 		log_err("cannot open pidfile %s: %s", 
 			pidfile, strerror(errno));
-		return 0;
+		return;
 	}
 	while(count < strlen(pidbuf)) {
 		ssize_t r = write(fd, pidbuf+count, strlen(pidbuf)-count);
@@ -393,17 +392,16 @@ writepid (const char* pidfile, pid_t pid)
 			log_err("cannot write to pidfile %s: %s",
 				pidfile, strerror(errno));
 			close(fd);
-			return 0;
+			return;
 		} else if(r == 0) {
 			log_err("cannot write any bytes to pidfile %s: "
 				"write returns 0 bytes written", pidfile);
 			close(fd);
-			return 0;
+			return;
 		}
 		count += r;
 	}
 	close(fd);
-	return 1;
 }
 
 /**
@@ -545,7 +543,15 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 				cfg, 1);
 		if(!daemon->pidfile)
 			fatal_exit("pidfile alloc: out of memory");
-		checkoldpid(daemon->pidfile, pidinchroot);
+		/* Check old pid if there is no username configured.
+		 * With a username, the assumption is that the privilege
+		 * drop makes a pidfile not removed when the server stopped
+		 * last time. The server does not chown the pidfile for it,
+		 * because that creates privilege escape problems, with the
+		 * pidfile writable by unprivileged users, but used by
+		 * privileged users. */
+		if(cfg->username && cfg->username[0])
+			checkoldpid(daemon->pidfile, pidinchroot);
 	}
 #endif
 
@@ -557,18 +563,7 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 	/* write new pidfile (while still root, so can be outside chroot) */
 #ifdef HAVE_KILL
 	if(cfg->pidfile && cfg->pidfile[0] && need_pidfile) {
-		if(writepid(daemon->pidfile, getpid())) {
-			if(cfg->username && cfg->username[0] && cfg_uid != (uid_t)-1 &&
-				pidinchroot) {
-#  ifdef HAVE_CHOWN
-				if(chown(daemon->pidfile, cfg_uid, cfg_gid) == -1) {
-					verbose(VERB_QUERY, "cannot chown %u.%u %s: %s",
-						(unsigned)cfg_uid, (unsigned)cfg_gid,
-						daemon->pidfile, strerror(errno));
-				}
-#  endif /* HAVE_CHOWN */
-			}
-		}
+		writepid(daemon->pidfile, getpid());
 	}
 #else
 	(void)daemon;
diff --git a/doc/Changelog b/doc/Changelog
index 05be2e56f..64b1bcf49 100644
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -7,6 +7,7 @@
 	- Fix to add unit test for lruhash space that exercises the routines.
 	- Fix that when the server truncates the pidfile, it does not follow
 	  symbolic links.
+	- Fix that the server does not chown the pidfile.
 
 25 March 2024: Yorgos
 	- Merge #831 from Pierre4012: Improve Windows NSIS installer