From: Micah Morton <mortonm@chromium.org>
Date: Fri, 8 Jun 2018 18:55:30 +0000 (-0700)
Subject: Allow charon to change group on files before dropping caps
X-Git-Tag: 5.7.0dr2~4
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4f1d2f2b095e523cf508749e7a5db9fc61fc2de9;p=thirdparty%2Fstrongswan.git

Allow charon to change group on files before dropping caps

Allow charon to start as a non-root user without CAP_CHOWN and still be
able to change the group on files that need to be accessed by charon
after capabilities have been dropped. This requires the user charon starts
as to have access to socket/pidfile directory as well as belong to the
group that charon will run as after dropping capabilities.

Closes strongswan/strongswan#105.
---

diff --git a/src/charon/charon.c b/src/charon/charon.c
index 180486746c..19f6c4cf79 100644
--- a/src/charon/charon.c
+++ b/src/charon/charon.c
@@ -231,15 +231,24 @@ static bool check_pidfile()
 			DBG1(DBG_LIB, "setting FD_CLOEXEC for '"PID_FILE"' failed: %s",
 				 strerror(errno));
 		}
-		/* Only fchown() the pidfile if we have CAP_CHOWN. Otherwise,
-		 * directory permissions should allow pidfile to be accessed
-		 * by the UID/GID under which the charon daemon will run. */
+		/* Only change owner of the pidfile if we have CAP_CHOWN. Otherwise,
+		 * attempt to change group of pidfile to group under which charon
+		 * runs after dropping caps. This requires the user that charon
+		 * starts as to:
+		 * a) Have write access to the socket dir.
+		 * b) Belong to the group that charon will run under after dropping
+		 *    caps. */
 		if (lib->caps->check(lib->caps, CAP_CHOWN))
 		{
 			ignore_result(fchown(fd,
 								 lib->caps->get_uid(lib->caps),
 								 lib->caps->get_gid(lib->caps)));
 		}
+		else
+		{
+			ignore_result(fchown(fd, -1,
+								 lib->caps->get_gid(lib->caps)));
+		}
 		fprintf(pidfile, "%d\n", getpid());
 		fflush(pidfile);
 		return FALSE;
diff --git a/src/libstrongswan/networking/streams/stream_service_unix.c b/src/libstrongswan/networking/streams/stream_service_unix.c
index a9b71d6fd1..ef967e8171 100644
--- a/src/libstrongswan/networking/streams/stream_service_unix.c
+++ b/src/libstrongswan/networking/streams/stream_service_unix.c
@@ -59,13 +59,27 @@ stream_service_t *stream_service_create_unix(char *uri, int backlog)
 		return NULL;
 	}
 	umask(old);
-	/* only attempt to chown() socket if we have CAP_CHOWN */
-	if (lib->caps->check(lib->caps, CAP_CHOWN) &&
-		chown(addr.sun_path, lib->caps->get_uid(lib->caps),
-			  lib->caps->get_gid(lib->caps)) != 0)
+	/* Only attempt to change owner of socket if we have CAP_CHOWN. Otherwise,
+	 * attempt to change group of socket to group under which charon runs after
+	 * dropping caps. This requires the user that charon starts as to:
+	 * a) Have write access to the socket dir.
+	 * b) Belong to the group that charon will run under after dropping caps. */
+	if (lib->caps->check(lib->caps, CAP_CHOWN))
 	{
-		DBG1(DBG_NET, "changing socket permissions for '%s' failed: %s",
-			 uri, strerror(errno));
+		if (chown(addr.sun_path, lib->caps->get_uid(lib->caps),
+				  lib->caps->get_gid(lib->caps)) != 0)
+		{
+			DBG1(DBG_NET, "changing socket owner/group for '%s' failed: %s",
+				 uri, strerror(errno));
+		}
+	}
+	else
+	{
+		if (chown(addr.sun_path, -1, lib->caps->get_gid(lib->caps)) != 0)
+		{
+			DBG1(DBG_NET, "changing socket group for '%s' failed: %s",
+				 uri, strerror(errno));
+		}
 	}
 	if (listen(fd, backlog) < 0)
 	{