From 4f1d2f2b095e523cf508749e7a5db9fc61fc2de9 Mon Sep 17 00:00:00 2001
From: Micah Morton <mortonm@chromium.org>
Date: Fri, 8 Jun 2018 11:55:30 -0700
Subject: [PATCH] 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.
---
 src/charon/charon.c                           | 15 ++++++++---
 .../networking/streams/stream_service_unix.c  | 26 ++++++++++++++-----
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/src/charon/charon.c b/src/charon/charon.c
index 180486746..19f6c4cf7 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 a9b71d6fd..ef967e817 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)
 	{