conf: adopt the systemd semantics for per-service directories

Aaron Andersen points out in the #492 discussion that the *Directory
settings carry more contract than create-and-chown: per-directory
modes, specific ownership rules, and cleanup toggles.  Without them
config-dir was chowned to the service user, which systemd never does,
an existing directory with drifted ownership was left wrong, and the
runtime directory could not survive a restart.

Now matching systemd.exec(5), and where the man page is vague, the
code in setup_exec_directory():

  - each directory takes a matching -mode key, octal with the leading
    zero, default 0755.  The mode of the named directory is locked
    down again on every start, also when it already exists
  - config-dir is created but never chowned
  - the contents of an existing directory are left alone as long as
    the owner is right; on drift everything under it is chowned back
  - runtime-dir-preserve = no | restart | yes maps
    RuntimeDirectoryPreserve=.  A service still qualified to run when
    the runtime directory would be removed is restarting, not
    stopping, which is what svc_enabled() answers

The dir mechanics move to mksubsysd(), taking resolved ids, with
mksubsys() reduced to a name-resolving wrapper for the dbus plugin.
The child resolves uid/gid once for both directory setup and
privilege drop.

The symlink form, RuntimeDirectory=foo:bar, is not adopted.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

src/conf.c

@@ -274,6 +274,12 @@ static cfg_opt_t svc_opts[] = {
 	CFG_STR     ("cache-dir",    NULL, CFGF_NODEFAULT),
 	CFG_STR     ("logs-dir",     NULL, CFGF_NODEFAULT),
 	CFG_STR     ("config-dir",   NULL, CFGF_NODEFAULT),
+	CFG_INT     ("runtime-dir-mode",     0, CFGF_NODEFAULT),
+	CFG_INT     ("state-dir-mode",       0, CFGF_NODEFAULT),
+	CFG_INT     ("cache-dir-mode",       0, CFGF_NODEFAULT),
+	CFG_INT     ("logs-dir-mode",        0, CFGF_NODEFAULT),
+	CFG_INT     ("config-dir-mode",      0, CFGF_NODEFAULT),
+	CFG_STR     ("runtime-dir-preserve", NULL, CFGF_NODEFAULT),
 	CFG_STR_LIST("conflicts",    NULL, CFGF_NODEFAULT),
 	CFG_STR     ("if",           NULL, CFGF_NODEFAULT),
 	CFG_STR     ("tty",          NULL, CFGF_NODEFAULT),
@@ -1301,20 +1307,45 @@ static int if_translate(const char *str, char *buf, size_t len, char *file, cons
  */
 static void dirs_translate(cfg_t *sec, svc_t *svc, char *file)
 {
+	const char *str;
 	int i;
 
 	for (i = 0; i < NUM_SVCDIRS; i++) {
-		const char *str;
+		char key[32];
+		long num;
 
 		str = sec_getstr(sec, svcdirs[i].key, NULL);
-		if (!str || !str[0])
-			continue;
+		if (str && str[0]) {
+			if (service_set_dir(svc, &svcdirs[i], str))
+				logit(LOG_ERR, "%s: %s: %s '%s' %s, ignoring",
+				      file, cfg_title(sec), svcdirs[i].key, str,
+				      errno == ENAMETOOLONG ? "is too long"
+				      : "must be relative, no '..'");
+		}
 
-		if (service_set_dir(svc, &svcdirs[i], str))
-			logit(LOG_ERR, "%s: %s: %s '%s' %s, ignoring",
-			      file, cfg_title(sec), svcdirs[i].key, str,
-			      errno == ENAMETOOLONG ? "is too long"
-			      : "must be relative, no '..'");
+		snprintf(key, sizeof(key), "%s-mode", svcdirs[i].key);
+		if (sec_getint(sec, key, NULL, &num)) {
+			if (num & ~07777L)
+				logit(LOG_ERR, "%s: %s: %s %04lo is not a valid"
+				      " mode, ignoring.  Modes are octal,"
+				      " with the leading zero", file,
+				      cfg_title(sec), key, num);
+			else
+				svc->dir_mode[i] = (mode_t)num;
+		}
+	}
+
+	if ((str = sec_getstr(sec, "runtime-dir-preserve", NULL))) {
+		if (!strcmp(str, "no") || !strcmp(str, "false"))
+			svc->dir_preserve = SVC_DIR_PRESERVE_NO;
+		else if (!strcmp(str, "restart"))
+			svc->dir_preserve = SVC_DIR_PRESERVE_RESTART;
+		else if (!strcmp(str, "yes") || !strcmp(str, "true"))
+			svc->dir_preserve = SVC_DIR_PRESERVE_YES;
+		else
+			logit(LOG_ERR, "%s: %s: runtime-dir-preserve '%s' is not"
+			      " no, restart, or yes, using no", file,
+			      cfg_title(sec), str);
 	}
 }
 

src/service.c

@@ -577,11 +577,11 @@ static void set_uid(uid_t uid, svc_t *svc)
  * any post: script has run.  See issue #492.
  */
 const struct svcdir svcdirs[NUM_SVCDIRS] = {
-	{ "runtime-dir", "/run",       "RUNTIME_DIRECTORY",       offsetof(svc_t, runtime_dir) },
-	{ "state-dir",   "/var/lib",   "STATE_DIRECTORY",         offsetof(svc_t, state_dir)   },
-	{ "cache-dir",   "/var/cache", "CACHE_DIRECTORY",         offsetof(svc_t, cache_dir)   },
-	{ "logs-dir",    "/var/log",   "LOGS_DIRECTORY",          offsetof(svc_t, logs_dir)    },
-	{ "config-dir",  "/etc",       "CONFIGURATION_DIRECTORY", offsetof(svc_t, config_dir)  },
+	{ "runtime-dir", "/run",       "RUNTIME_DIRECTORY",       offsetof(svc_t, runtime_dir), 1 },
+	{ "state-dir",   "/var/lib",   "STATE_DIRECTORY",         offsetof(svc_t, state_dir),   1 },
+	{ "cache-dir",   "/var/cache", "CACHE_DIRECTORY",         offsetof(svc_t, cache_dir),   1 },
+	{ "logs-dir",    "/var/log",   "LOGS_DIRECTORY",          offsetof(svc_t, logs_dir),    1 },
+	{ "config-dir",  "/etc",       "CONFIGURATION_DIRECTORY", offsetof(svc_t, config_dir),  0 },
 };
 
 static char *svcdir_path(svc_t *svc, const struct svcdir *sd, char *path, size_t len)
@@ -618,12 +618,15 @@ int service_set_dir(svc_t *svc, const struct svcdir *sd, const char *name)
 	return 0;
 }
 
-static void service_mkdirs(svc_t *svc)
+static void service_mkdirs(svc_t *svc, uid_t uid, gid_t gid)
 {
 	char path[256];
 	size_t i;
 
 	for (i = 0; i < NELEMS(svcdirs); i++) {
+		uid_t u = svcdirs[i].chown ? uid : (uid_t)-1;
+		gid_t g = svcdirs[i].chown ? gid : 0;
+		struct stat st;
 		char *ptr;
 
 		ptr = svcdir_path(svc, &svcdirs[i], path, sizeof(path));
@@ -631,16 +634,21 @@ static void service_mkdirs(svc_t *svc)
 			continue;
 
 		/*
-		 * Script forks land here too, so an existing directory
-		 * is left alone -- mode and ownership are asserted at
-		 * creation only, a daemon may have tightened them.
+		 * Same rules as systemd: the named directory has its mode
+		 * locked down again on every start, but its contents are
+		 * only touched when the owner has drifted, then everything
+		 * under it is chowned back.  Script forks land here too,
+		 * so this must be idempotent.
 		 */
-		if (fisdir(ptr))
+		if (stat(ptr, &st) == 0) {
+			chmod(ptr, svc->dir_mode[i]);
+			if (u != (uid_t)-1 && (st.st_uid != u || st.st_gid != g))
+				chownr(ptr, u, g);
 			continue;
+		}
 
 		/* only the named directory is chowned, like systemd */
-		mkpath(ptr, 0755);
-		if (mksubsys(ptr, 0755, svc->username, svc->group))
+		if (mksubsysd(ptr, svc->dir_mode[i], u, g))
 			logit(LOG_WARNING, "%s: failed creating %s", svc_ident(svc, NULL, 0), ptr);
 	}
 }
@@ -661,8 +669,17 @@ static void service_rmdirs(svc_t *svc)
 	char path[256];
 
 	/* only the runtime directory, /run is tmpfs, the rest persist */
-	if (svcdir_path(svc, &svcdirs[0], path, sizeof(path)))
-		rmrf(path);
+	if (!svcdir_path(svc, &svcdirs[0], path, sizeof(path)))
+		return;
+
+	if (svc->dir_preserve == SVC_DIR_PRESERVE_YES)
+		return;
+
+	/* still qualified to run means this is a restart, not a stop */
+	if (svc->dir_preserve == SVC_DIR_PRESERVE_RESTART && svc_enabled(svc))
+		return;
+
+	rmrf(path);
 }
 
 static pid_t service_fork(svc_t *svc)
@@ -686,8 +703,6 @@ static pid_t service_fork(svc_t *svc)
 
 	if (pid == 0) {
 		char *home = NULL;
-
-		service_mkdirs(svc);
 #ifdef ENABLE_STATIC
 		int uid = 0; /* XXX: Fix better warning that dropprivs is disabled. */
 		int gid = 0;
@@ -707,6 +722,7 @@ static pid_t service_fork(svc_t *svc)
 			return -1;
 		}
 #endif
+		service_mkdirs(svc, uid, gid);
 		if (svc_is_tty(svc))
 			setprocnm("getty");
 
@@ -2372,8 +2388,11 @@ svc_t *service_register(int type, char *cfg, struct rlimit rlimit[], char *file)
 		memset(svc->capabilities, 0, sizeof(svc->capabilities));
 
 	/* block format only, set by conf.c after registration */
-	for (int i = 0; i < NUM_SVCDIRS; i++)
+	for (int i = 0; i < NUM_SVCDIRS; i++) {
 		memset((char *)svc + svcdirs[i].off, 0, MAX_ARG_LEN);
+		svc->dir_mode[i] = 0755;
+	}
+	svc->dir_preserve = SVC_DIR_PRESERVE_NO;
 
 	if (!svc_is_tty(svc) && ctty) {
 		char *dev = ctty;

src/service.h

@@ -38,8 +38,8 @@ struct svcdir {
 	const char *base;
 	const char *env;
 	size_t      off;	/* offsetof() in svc_t */
+	int         chown;	/* all but config-dir, like systemd */
 };
-#define NUM_SVCDIRS 5
 extern const struct svcdir svcdirs[NUM_SVCDIRS];
 
 int	  service_set_dir	 (svc_t *svc, const struct svcdir *sd, const char *name);

src/svc.h

@@ -201,11 +201,17 @@ typedef struct svc {
 
 	/* Directories set up for the service, block format only, the
 	 * name is resolved under a fixed base, e.g. /run/NAME */
+#define NUM_SVCDIRS 5
 	char	       runtime_dir[MAX_ARG_LEN];
 	char	       state_dir[MAX_ARG_LEN];
 	char	       cache_dir[MAX_ARG_LEN];
 	char	       logs_dir[MAX_ARG_LEN];
 	char	       config_dir[MAX_ARG_LEN];
+	mode_t	       dir_mode[NUM_SVCDIRS];
+#define SVC_DIR_PRESERVE_NO      0	/* remove runtime-dir on stop  */
+#define SVC_DIR_PRESERVE_RESTART 1	/* keep it across restarts     */
+#define SVC_DIR_PRESERVE_YES     2	/* never remove it             */
+	char	       dir_preserve;
 
 	/* Command, arguments and service description */
 	char	       cmd[MAX_CMD_LEN];

src/util.c

@@ -325,6 +325,33 @@ static int rmrf_cb(const char *fpath, const struct stat *sb, int tflag, struct F
 	return 0;
 }
 
+/* nftw() cannot pass user data, see also tmpfiles.c do_clean() */
+static uid_t chownr_uid;
+static gid_t chownr_gid;
+
+static int chownr_cb(const char *fpath, const struct stat *sb, int tflag, struct FTW *ftw)
+{
+	(void)tflag;
+	(void)ftw;
+
+	if (sb->st_uid == chownr_uid && sb->st_gid == chownr_gid)
+		return 0;
+
+	if (lchown(fpath, chownr_uid, chownr_gid))
+		warn("Failed chown(%s, %d, %d)", fpath, (int)chownr_uid, (int)chownr_gid);
+
+	return 0;
+}
+
+/* chown -R */
+int chownr(const char *path, uid_t uid, gid_t gid)
+{
+	chownr_uid = uid;
+	chownr_gid = gid;
+
+	return nftw(path, chownr_cb, 20, FTW_PHYS);
+}
+
 /* empty a directory but keep it, silently ignores a missing path */
 int rmcontents(const char *path)
 {
@@ -347,34 +374,43 @@ int rmrf(const char *path)
 	return 0;
 }
 
-int mksubsys(const char *dir, mode_t mode, char *user, char *group)
+/*
+ * Like mksubsys() but with the ids already resolved, uid -1 skips the
+ * chown.  Parents are created 0755, only the leaf gets @mode.
+ */
+int mksubsysd(const char *dir, mode_t mode, uid_t uid, gid_t gid)
 {
 	mode_t omask;
-	int uid, gid;
-	int rc = 0;
+	int rc;
 
 	omask = umask(0);
 
-	rc = makedir(dir, mode);
-	if (rc && errno == EEXIST)
+	rc = mkpath(dir, 0755);
+	if (!rc) {
 		rc = chmod(dir, mode);
-
-	uid = getuser(user, NULL);
-	if (uid >= 0) {
-		gid = getgroup(group);
-		if (gid < 0)
-			gid = 0;
-
-		if (chown(dir, uid, gid))
-			err(1, "Failed chown(%s, %d, %d)", dir, uid, gid);
-	} else
-		warnx("Cannot find user %s, %s is owned by root", user, dir);
+		if (!rc && uid != (uid_t)-1 && chown(dir, uid, gid))
+			err(1, "Failed chown(%s, %d, %d)", dir, (int)uid, (int)gid);
+	}
 
 	umask(omask);
 
 	return rc;
 }
 
+int mksubsys(const char *dir, mode_t mode, char *user, char *group)
+{
+	int uid, gid;
+
+	uid = getuser(user, NULL);
+	gid = getgroup(group);
+	if (gid < 0)
+		gid = 0;
+	if (uid < 0)
+		warnx("Cannot find user %s, %s is owned by root", user, dir);
+
+	return mksubsysd(dir, mode, uid < 0 ? (uid_t)-1 : (uid_t)uid, (gid_t)gid);
+}
+
 /*
  * Read an open stream to EOF into a malloc()'ed, NUL terminated buffer.
  *

src/util.h

@@ -72,6 +72,8 @@ int   getcuser     (char *buf, size_t len);
 int   getcgroup    (char *buf, size_t len);
 
 int   mksubsys     (const char *dir, mode_t mode, char *user, char *group);
+int   mksubsysd    (const char *dir, mode_t mode, uid_t uid, gid_t gid);
+int   chownr       (const char *path, uid_t uid, gid_t gid);
 int   rmcontents   (const char *path);
 int   rmrf         (const char *path);