diff mbox series

[8/9] Avoid re-opening and parsing swupdate.cfg for each subprocess

Message ID 20210126131412.3567-9-michael.adler@siemens.com
State Changes Requested
Headers show
Series Overhaul swupdate.cfg handling | expand

Commit Message

Michael Adler Jan. 26, 2021, 1:14 p.m. UTC
This is achieved by passing uid and gid to start_subprocess,
i.e. an inversion of control.

Signed-off-by: Michael Adler <michael.adler@siemens.com>
Signed-off-by: Christian Storm <christian.storm@siemens.com>
---
 core/pctl.c                 | 20 ++++++++++----------
 core/swupdate.c             | 33 ++++++++++++++++++++++++++++-----
 corelib/swupdate_settings.c |  6 +++---
 include/pctl.h              |  9 +++++++--
 include/swupdate_settings.h |  4 ++--
 5 files changed, 50 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/core/pctl.c b/core/pctl.c
index 01ad540..481f077 100644
--- a/core/pctl.c
+++ b/core/pctl.c
@@ -165,19 +165,16 @@  static int spawn_process(struct swupdate_task *task,
 	}
 }
 
-static void start_swupdate_subprocess(sourcetype type,
-			const char *name, const char *cfgfile,
+static void start_swupdate_subprocess(sourcetype type, const char *name,
+			uid_t run_as_userid, gid_t run_as_groupid,
+			const char* cfgfile,
 			int argc, char **argv,
 			swupdate_process start,
 			const char *cmdline)
 {
-	uid_t uid;
-	gid_t gid;
-
-	read_settings_user_id(cfgfile, name, &uid, &gid);
 	procs[nprocs].name = name;
 	procs[nprocs].type = type;
-	if (spawn_process(&procs[nprocs], uid, gid, cfgfile, argc, argv, start, cmdline) < 0) {
+	if (spawn_process(&procs[nprocs], run_as_userid, run_as_groupid, cfgfile, argc, argv, start, cmdline) < 0) {
 		ERROR("Spawning %s failed, exiting process...", name);
 		exit(1);
 	}
@@ -188,19 +185,22 @@  static void start_swupdate_subprocess(sourcetype type,
 
 
 void start_subprocess_from_file(sourcetype type, const char *name,
+			uid_t run_as_userid, gid_t run_as_groupid,
 			const char *cfgfile,
 			int argc, char **argv,
 			const char *cmdline)
 {
-	start_swupdate_subprocess(type, name, cfgfile, argc, argv, NULL, cmdline);
+	start_swupdate_subprocess(type, name, run_as_userid, run_as_groupid, cfgfile, argc, argv, NULL, cmdline);
 }
 
-void start_subprocess(sourcetype type, const char *name, const char *cfgfile,
+void start_subprocess(sourcetype type, const char *name,
+			uid_t run_as_userid, gid_t run_as_groupid,
+			const char *cfgfile,
 			int argc, char **argv,
 			swupdate_process start)
 {
 
-	start_swupdate_subprocess(type, name, cfgfile, argc, argv, start, NULL);
+	start_swupdate_subprocess(type, name, run_as_userid, run_as_groupid, cfgfile, argc, argv, start, NULL);
 }
 
 /*
diff --git a/core/swupdate.c b/core/swupdate.c
index a76e670..3729e32 100644
--- a/core/swupdate.c
+++ b/core/swupdate.c
@@ -545,6 +545,7 @@  int main(int argc, char **argv)
 		}
 	}
 
+	swupdate_cfg_handle *p_handle = NULL;
 	/* Load configuration file */
 	if (cfgfname != NULL) {
 		swupdate_cfg_handle handle;
@@ -584,7 +585,8 @@  int main(int argc, char **argv)
 
 		get_sw_versions(&handle, &swcfg);
 
-		swupdate_cfg_destroy(&handle);
+		// save the handle for later usage
+		p_handle = &handle;
 	} else {
 		get_sw_versions(NULL, &swcfg);
 	}
@@ -847,7 +849,10 @@  int main(int argc, char **argv)
 	/* Start embedded web server */
 #if defined(CONFIG_MONGOOSE)
 	if (opt_w) {
-		start_subprocess(SOURCE_WEBSERVER, "webserver",
+		uid_t uid;
+		gid_t gid;
+		read_settings_user_id(p_handle, "webserver", &uid, &gid);
+		start_subprocess(SOURCE_WEBSERVER, "webserver", uid, gid,
 				 cfgfname, ac, av,
 				 start_mongoose);
 		freeargs(av);
@@ -856,7 +861,10 @@  int main(int argc, char **argv)
 
 #if defined(CONFIG_SURICATTA)
 	if (opt_u) {
-		start_subprocess(SOURCE_SURICATTA, "suricatta",
+		uid_t uid;
+		gid_t gid;
+		read_settings_user_id(p_handle, "suricatta", &uid, &gid);
+		start_subprocess(SOURCE_SURICATTA, "suricatta", uid, gid,
 				 cfgfname, argcount,
 				 argvalues, start_suricatta);
 
@@ -866,7 +874,10 @@  int main(int argc, char **argv)
 
 #ifdef CONFIG_DOWNLOAD
 	if (opt_d) {
-		start_subprocess(SOURCE_DOWNLOADER, "download",
+		uid_t uid;
+		gid_t gid;
+		read_settings_user_id(p_handle, "download", &uid, &gid);
+		start_subprocess(SOURCE_DOWNLOADER, "download", uid, gid,
 				 cfgfname, dwlac,
 				 dwlav, start_download);
 		freeargs(dwlav);
@@ -883,7 +894,10 @@  int main(int argc, char **argv)
 
 		dwlav[dwlac] = NULL;
 
-		start_subprocess_from_file(SOURCE_UNKNOWN, proc->name,
+		uid_t uid;
+		gid_t gid;
+		read_settings_user_id(p_handle, proc->name, &uid, &gid);
+		start_subprocess_from_file(SOURCE_UNKNOWN, proc->name, uid, gid,
 					   cfgfname, dwlac,
 					   dwlav, dwlav[0]);
 
@@ -910,6 +924,15 @@  int main(int argc, char **argv)
 	sa.sa_handler = sigterm_handler;
 	sigaction(SIGTERM, &sa, NULL);
 
+	/*
+	 * This process is done with the config file.
+	 * Other processes will have to re-open it if they need some settings.
+	 * This is necessary for security reasons since they may run under
+	 * different privileges.
+	 */
+	if (p_handle != NULL)
+		swupdate_cfg_destroy(p_handle);
+
 	/*
 	 * Go into supervisor loop
 	 */
diff --git a/corelib/swupdate_settings.c b/corelib/swupdate_settings.c
index f06ecf7..1669f37 100644
--- a/corelib/swupdate_settings.c
+++ b/corelib/swupdate_settings.c
@@ -94,7 +94,7 @@  static int get_run_as(void *elem, void *data)
 	return 0;
 }
 
-int read_settings_user_id(const char *filename, const char *module, uid_t *userid, gid_t *groupid)
+int read_settings_user_id(swupdate_cfg_handle *handle, const char *module, uid_t *userid, gid_t *groupid)
 {
 	struct run_as ids;
 	int ret;
@@ -102,7 +102,7 @@  int read_settings_user_id(const char *filename, const char *module, uid_t *useri
 	*userid = ids.userid = getuid();
 	*groupid = ids.groupid = getgid();
 
-	ret = read_module_settings(filename, module, get_run_as, &ids);
+	ret = swupdate_cfg_read_module_settings(handle, module, get_run_as, &ids);
 	if (ret)
 		return -EINVAL;
 
@@ -165,7 +165,7 @@  int swupdate_cfg_read_module_settings(swupdate_cfg_handle *handle, const char *m
 {
 	config_setting_t *elem;
 
-	if (!fcn)
+	if (handle == NULL || !fcn)
 		return -EINVAL;
 
 	TRACE("Reading config file settings for module %s", module);
diff --git a/include/pctl.h b/include/pctl.h
index de5f3b5..84cf549 100644
--- a/include/pctl.h
+++ b/include/pctl.h
@@ -9,6 +9,7 @@ 
 #define _SWUPDATE_PCTL_H
 
 #include <swupdate_status.h>
+#include <sys/types.h>
 
 extern int pid;
 extern int sw_sockfd;
@@ -28,11 +29,15 @@  pthread_t start_thread(void *(* start_routine) (void *), void *arg);
 
 typedef int (*swupdate_process)(const char *cfgname, int argc, char **argv);
 
-void start_subprocess(sourcetype type, const char *name, const char *cfgfile,
+void start_subprocess(sourcetype type, const char *name,
+			uid_t run_as_userid, gid_t run_as_groupid,
+			const char *cfgfile,
 			int argc, char **argv,
 			swupdate_process start);
 
-void start_subprocess_from_file(sourcetype type, const char *name, const char *cfgfile,
+void start_subprocess_from_file(sourcetype type, const char *name,
+			uid_t run_as_userid, gid_t run_as_groupid,
+			const char *cfgfile,
 			int argc, char **argv,
 			const char *cmd);
 
diff --git a/include/swupdate_settings.h b/include/swupdate_settings.h
index c8e4f1d..d98c73e 100644
--- a/include/swupdate_settings.h
+++ b/include/swupdate_settings.h
@@ -25,7 +25,7 @@  void swupdate_cfg_destroy(swupdate_cfg_handle *handle);
 int swupdate_cfg_read_module_settings(swupdate_cfg_handle *handle, const char *module, settings_callback fcn, void *data);
 
 int read_module_settings(const char *filename, const char *module, settings_callback fcn, void *data);
-int read_settings_user_id(const char *filename, const char *module, uid_t *userid, gid_t *groupid);
+int read_settings_user_id(swupdate_cfg_handle *handle, const char *module, uid_t *userid, gid_t *groupid);
 int settings_into_dict(void *settings, void *data);
 #else
 
@@ -42,7 +42,7 @@  static inline int read_module_settings(const char __attribute__ ((__unused__))*f
 /*
  * Without LIBCONFIG, let run with current user
  */
-static inline int read_settings_user_id(const char __attribute__ ((__unused__))*filename,
+static inline int read_settings_user_id(swupdate_cfg_handle __attribute__ ((__unused__))*handle,
 					const char __attribute__ ((__unused__))*module,
 					uid_t *userid, gid_t *groupid)
 {