ldpd: simplify initialization of the child processes

In order to have separate ASLR/cookies per process, ldpd calls exec()
in the child processes after fork() (this is also known as the fork+exec
model).

This is an important security feature but it makes the initialization
of the child processes a bit more complicated as they're not a copy of
the parent anymore, so all parameters given via command line are lost.

To solve this problem, we were creating an argv array by hand with all
necessary parameters and providing it to the exec() syscall. This works
but it's a very ugly solution. This patch introduces a different approach
to solve the problem: send an IMSG_INIT message to the child processes
with all parameters they need in order to initialize properly. This
makes adding additional initialization parameters much more convenient
and less error prone.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

8 files changed, 126 insertions, 123 deletions

diff --git a/ldpd/control.c b/ldpd/control.c
index 5c530e1b70..d40e0342c1 100644
--- a/ldpd/control.c
+++ b/ldpd/control.c
@@ -37,7 +37,7 @@ struct ctl_conns	 ctl_conns;
 static int		 control_fd;
 
 int
-control_init(void)
+control_init(char *path)
 {
 	struct sockaddr_un	 s_un;
 	int			 fd;
@@ -51,28 +51,28 @@ control_init(void)
 
 	memset(&s_un, 0, sizeof(s_un));
 	s_un.sun_family = AF_UNIX;
-	strlcpy(s_un.sun_path, ctl_sock_path, sizeof(s_un.sun_path));
+	strlcpy(s_un.sun_path, path, sizeof(s_un.sun_path));
 
-	if (unlink(ctl_sock_path) == -1)
+	if (unlink(path) == -1)
 		if (errno != ENOENT) {
-			log_warn("%s: unlink %s", __func__, ctl_sock_path);
+			log_warn("%s: unlink %s", __func__, path);
 			close(fd);
 			return (-1);
 		}
 
 	old_umask = umask(S_IXUSR|S_IXGRP|S_IWOTH|S_IROTH|S_IXOTH);
 	if (bind(fd, (struct sockaddr *)&s_un, sizeof(s_un)) == -1) {
-		log_warn("%s: bind: %s", __func__, ctl_sock_path);
+		log_warn("%s: bind: %s", __func__, path);
 		close(fd);
 		umask(old_umask);
 		return (-1);
 	}
 	umask(old_umask);
 
-	if (chmod(ctl_sock_path, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) == -1) {
+	if (chmod(path, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) == -1) {
 		log_warn("%s: chmod", __func__);
 		close(fd);
-		(void)unlink(ctl_sock_path);
+		(void)unlink(path);
 		return (-1);
 	}
 
@@ -93,11 +93,11 @@ control_listen(void)
 }
 
 void
-control_cleanup(void)
+control_cleanup(char *path)
 {
 	accept_del(control_fd);
 	close(control_fd);
-	unlink(ctl_sock_path);
+	unlink(path);
 }
 
 /* ARGSUSED */
diff --git a/ldpd/control.h b/ldpd/control.h
index 32c49fdf87..0e66a1636a 100644
--- a/ldpd/control.h
+++ b/ldpd/control.h
@@ -29,9 +29,9 @@ TAILQ_HEAD(ctl_conns, ctl_conn);
 
 extern struct ctl_conns ctl_conns;
 
-int	control_init(void);
+int	control_init(char *);
 int	control_listen(void);
-void	control_cleanup(void);
+void	control_cleanup(char *);
 int	control_imsg_relay(struct imsg *);
 
 #endif	/* _CONTROL_H_ */
diff --git a/ldpd/lde.c b/ldpd/lde.c
index 0865112ca6..a71f5e31aa 100644
--- a/ldpd/lde.c
+++ b/ldpd/lde.c
@@ -150,12 +150,9 @@ zclient_sync_init(u_short instance)
 
 /* label decision engine */
 void
-lde(const char *user, const char *group, u_short instance)
+lde(void)
 {
 	struct thread		 thread;
-	struct timeval		 now;
-
-	ldeconf = config_new_empty();
 
 #ifdef HAVE_SETPROCTITLE
 	setproctitle("label decision engine");
@@ -163,18 +160,6 @@ lde(const char *user, const char *group, u_short instance)
 	ldpd_process = PROC_LDE_ENGINE;
 	log_procname = log_procnames[PROC_LDE_ENGINE];
 
-	/* drop privileges */
-	if (user)
-		lde_privs.user = user;
-	if (group)
-		lde_privs.group = group;
-	zprivs_init(&lde_privs);
-
-#ifdef HAVE_PLEDGE
-	if (pledge("stdio recvfd unix", NULL) == -1)
-		fatal("pledge");
-#endif
-
 	master = thread_master_create();
 
 	/* setup signal handler */
@@ -193,19 +178,36 @@ lde(const char *user, const char *group, u_short instance)
 		fatal(NULL);
 	imsg_init(&iev_main_sync->ibuf, LDPD_FD_SYNC);
 
+	/* create base configuration */
+	ldeconf = config_new_empty();
+
+	/* Fetch next active thread. */
+	while (thread_fetch(master, &thread))
+		thread_call(&thread);
+}
+
+void
+lde_init(struct ldpd_init *init)
+{
+	/* drop privileges */
+	if (init->user)
+		lde_privs.user = init->user;
+	if (init->group)
+		lde_privs.group = init->group;
+	zprivs_init(&lde_privs);
+
+#ifdef HAVE_PLEDGE
+	if (pledge("stdio recvfd unix", NULL) == -1)
+		fatal("pledge");
+#endif
+
 	/* start the LIB garbage collector */
 	lde_gc_start_timer();
 
-	gettimeofday(&now, NULL);
-	global.uptime = now.tv_sec;
-
 	/* Init synchronous zclient and label list */
-	zclient_sync_init(instance);
+	zclient_serv_path_set(init->zclient_serv_path);
+	zclient_sync_init(init->instance);
 	lde_label_list_init();
-
-	/* Fetch next active thread. */
-	while (thread_fetch(master, &thread))
-		thread_call(&thread);
 }
 
 static void
@@ -551,6 +553,14 @@ lde_dispatch_parent(struct thread *thread)
 			iev_ldpe->handler_write = ldp_write_handler;
 			iev_ldpe->ev_write = NULL;
 			break;
+		case IMSG_INIT:
+			if (imsg.hdr.len != IMSG_HEADER_SIZE +
+			    sizeof(struct ldpd_init))
+				fatalx("INIT imsg with wrong len");
+
+			memcpy(&init, imsg.data, sizeof(init));
+			lde_init(&init);
+			break;
 		case IMSG_RECONF_CONF:
 			if ((nconf = malloc(sizeof(struct ldpd_conf))) ==
 			    NULL)
diff --git a/ldpd/lde.h b/ldpd/lde.h
index 57791cd1b0..3349d4ca00 100644
--- a/ldpd/lde.h
+++ b/ldpd/lde.h
@@ -139,7 +139,8 @@ extern struct nbr_tree	 lde_nbrs;
 extern struct thread	*gc_timer;
 
 /* lde.c */
-void		 lde(const char *, const char *, u_short instance);
+void		 lde(void);
+void		 lde_init(struct ldpd_init *);
 int		 lde_imsg_compose_parent(int, pid_t, void *, uint16_t);
 int		 lde_imsg_compose_ldpe(int, uint32_t, pid_t, void *, uint16_t);
 int		 lde_acl_check(char *, int, union ldpd_addr *, uint8_t);
diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c
index 47644d4449..bdf7097323 100644
--- a/ldpd/ldpd.c
+++ b/ldpd/ldpd.c
@@ -44,8 +44,7 @@
 #include "libfrr.h"
 
 static void		 ldpd_shutdown(void);
-static pid_t		 start_child(enum ldpd_process, char *, int, int,
-			    const char *, const char *, const char *, const char *);
+static pid_t		 start_child(enum ldpd_process, char *, int, int);
 static int		 main_dispatch_ldpe(struct thread *);
 static int		 main_dispatch_lde(struct thread *);
 static int		 main_imsg_send_ipc_sockets(struct imsgbuf *,
@@ -77,6 +76,7 @@ DEFINE_QOBJ_TYPE(l2vpn)
 DEFINE_QOBJ_TYPE(ldpd_conf)
 
 struct ldpd_global	 global;
+struct ldpd_init	 init;
 struct ldpd_conf	*ldpd_conf, *vty_conf;
 
 static struct imsgev	*iev_ldpe, *iev_ldpe_sync;
@@ -200,12 +200,7 @@ main(int argc, char *argv[])
 	int			 lflag = 0, eflag = 0;
 	int			 pipe_parent2ldpe[2], pipe_parent2ldpe_sync[2];
 	int			 pipe_parent2lde[2], pipe_parent2lde_sync[2];
-	char			*ctl_sock_custom_path = NULL;
 	char			*ctl_sock_name;
-	const char		*user = NULL;
-	const char		*group = NULL;
-	u_short			 instance = 0;
-	const char		*instance_char = NULL;
 
 	ldpd_process = PROC_MAIN;
 	log_procname = log_procnames[ldpd_process];
@@ -242,17 +237,14 @@ main(int argc, char *argv[])
 				 * sensible config
 				 */
 				ctl_sock_name = (char *)LDPD_SOCKET;
-			ctl_sock_custom_path = optarg;
-			strlcpy(ctl_sock_path, ctl_sock_custom_path,
-			    sizeof(ctl_sock_path));
+			strlcpy(ctl_sock_path, optarg, sizeof(ctl_sock_path));
 			strlcat(ctl_sock_path, "/", sizeof(ctl_sock_path));
 			strlcat(ctl_sock_path, ctl_sock_name,
 			    sizeof(ctl_sock_path));
 			break;
 		case 'n':
-			instance = atoi(optarg);
-			instance_char = optarg;
-			if (instance < 1)
+			init.instance = atoi(optarg);
+			if (init.instance < 1)
 				exit(0);
 			break;
 		case 'L':
@@ -267,8 +259,11 @@ main(int argc, char *argv[])
 		}
 	}
 
-	user = ldpd_privs.user;
-	group = ldpd_privs.group;
+	strlcpy(init.user, ldpd_privs.user, sizeof(init.user));
+	strlcpy(init.group, ldpd_privs.group, sizeof(init.group));
+	strlcpy(init.ctl_sock_path, ctl_sock_path, sizeof(init.ctl_sock_path));
+	strlcpy(init.zclient_serv_path, zclient_serv_path_get(),
+	    sizeof(init.zclient_serv_path));
 
 	argc -= optind;
 	argv += optind;
@@ -286,9 +281,9 @@ main(int argc, char *argv[])
 	    LOG_CONS | LOG_NDELAY | LOG_PID, LOG_DAEMON);
 
 	if (lflag)
-		lde(user, group, instance);
+		lde();
 	else if (eflag)
-		ldpe(user, group, ctl_sock_path);
+		ldpe();
 
 	if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_parent2ldpe) == -1)
 		fatal("socketpair");
@@ -317,11 +312,9 @@ main(int argc, char *argv[])
 
 	/* start children */
 	lde_pid = start_child(PROC_LDE_ENGINE, saved_argv0,
-	    pipe_parent2lde[1], pipe_parent2lde_sync[1],
-	    user, group, ctl_sock_custom_path, instance_char);
+	    pipe_parent2lde[1], pipe_parent2lde_sync[1]);
 	ldpe_pid = start_child(PROC_LDP_ENGINE, saved_argv0,
-	    pipe_parent2ldpe[1], pipe_parent2ldpe_sync[1],
-	    user, group, ctl_sock_custom_path, instance_char);
+	    pipe_parent2ldpe[1], pipe_parent2ldpe_sync[1]);
 
 	/* drop privileges */
 	zprivs_init(&ldpd_privs);
@@ -389,6 +382,7 @@ main(int argc, char *argv[])
 
 	if (main_imsg_send_ipc_sockets(&iev_ldpe->ibuf, &iev_lde->ibuf))
 		fatal("could not establish imsg links");
+	main_imsg_compose_both(IMSG_INIT, &init, sizeof(init));
 	main_imsg_compose_both(IMSG_DEBUG_UPDATE, &ldp_debug,
 	    sizeof(ldp_debug));
 	main_imsg_send_config(ldpd_conf);
@@ -453,11 +447,9 @@ ldpd_shutdown(void)
 }
 
 static pid_t
-start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync,
-    const char *user, const char *group, const char *ctl_sock_custom_path,
-    const char *instance)
+start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync)
 {
-	char	*argv[13];
+	char	*argv[3];
 	int	 argc = 0;
 	pid_t	 pid;
 
@@ -488,26 +480,6 @@ start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync,
 		argv[argc++] = (char *)"-E";
 		break;
 	}
-	if (user) {
-		argv[argc++] = (char *)"-u";
-		argv[argc++] = (char *)user;
-	}
-	if (group) {
-		argv[argc++] = (char *)"-g";
-		argv[argc++] = (char *)group;
-	}
-	if (ctl_sock_custom_path) {
-		argv[argc++] = (char *)"--ctl_socket";
-		argv[argc++] = (char *)ctl_sock_custom_path;
-	}
-	/* zclient serv path */
-	argv[argc++] = (char *)"-z";
-	argv[argc++] = (char *)zclient_serv_path_get();
-	/* instance */
-	if (instance) {
-		argv[argc++] = (char *)"-n";
-		argv[argc++] = (char *)instance;
-	}
 	argv[argc++] = NULL;
 
 	execvp(argv0, argv);
diff --git a/ldpd/ldpd.h b/ldpd/ldpd.h
index ad83b9aad3..cba1939d98 100644
--- a/ldpd/ldpd.h
+++ b/ldpd/ldpd.h
@@ -145,7 +145,16 @@ enum imsg_type {
 	IMSG_RECONF_END,
 	IMSG_DEBUG_UPDATE,
 	IMSG_LOG,
-	IMSG_ACL_CHECK
+	IMSG_ACL_CHECK,
+	IMSG_INIT
+};
+
+struct ldpd_init {
+	char		 user[256];
+	char		 group[256];
+	char		 ctl_sock_path[MAXPATHLEN];
+	char		 zclient_serv_path[MAXPATHLEN];
+	u_short		 instance;
 };
 
 union ldpd_addr {
@@ -508,7 +517,6 @@ struct ldpd_af_global {
 struct ldpd_global {
 	int			 cmd_opts;
 	int			 sighup;
-	time_t			 uptime;
 	struct in_addr		 rtr_id;
 	struct ldpd_af_global	 ipv4;
 	struct ldpd_af_global	 ipv6;
@@ -652,6 +660,7 @@ struct ctl_pw {
 
 extern struct ldpd_conf		*ldpd_conf, *vty_conf;
 extern struct ldpd_global	 global;
+extern struct ldpd_init		 init;
 
 /* parse.y */
 struct ldpd_conf	*parse_config(char *);
diff --git a/ldpd/ldpe.c b/ldpd/ldpe.c
index e82416276b..1248d4f86e 100644
--- a/ldpd/ldpe.c
+++ b/ldpd/ldpe.c
@@ -103,47 +103,17 @@ static struct quagga_signal_t ldpe_signals[] =
 
 /* label distribution protocol engine */
 void
-ldpe(const char *user, const char *group, const char *ctl_path)
+ldpe(void)
 {
 	struct thread		 thread;
 
-	leconf = config_new_empty();
-
 #ifdef HAVE_SETPROCTITLE
 	setproctitle("ldp engine");
 #endif
 	ldpd_process = PROC_LDP_ENGINE;
 	log_procname = log_procnames[ldpd_process];
 
-	LIST_INIT(&global.addr_list);
-	RB_INIT(&global.adj_tree);
-	TAILQ_INIT(&global.pending_conns);
-	if (inet_pton(AF_INET, AllRouters_v4, &global.mcast_addr_v4) != 1)
-		fatal("inet_pton");
-	if (inet_pton(AF_INET6, AllRouters_v6, &global.mcast_addr_v6) != 1)
-		fatal("inet_pton");
-#ifdef __OpenBSD__
-	global.pfkeysock = pfkey_init();
-#endif
-
-	/* drop privileges */
-	if (user)
-		ldpe_privs.user = user;
-	if (group)
-		ldpe_privs.group = group;
-	zprivs_init(&ldpe_privs);
-
-	strlcpy(ctl_sock_path, ctl_path, sizeof(ctl_sock_path));
-	if (control_init() == -1)
-		fatalx("control socket setup failed");
-
-#ifdef HAVE_PLEDGE
-	if (pledge("stdio cpath inet mcast recvfd", NULL) == -1)
-		fatal("pledge");
-#endif
-
   	master = thread_master_create();
-	accept_init();
 
 	/* setup signal handler */
 	signal_init(master, array_size(ldpe_signals), ldpe_signals);
@@ -161,7 +131,45 @@ ldpe(const char *user, const char *group, const char *ctl_path)
 		fatal(NULL);
 	imsg_init(&iev_main_sync->ibuf, LDPD_FD_SYNC);
 
+	/* create base configuration */
+	leconf = config_new_empty();
+
+	/* Fetch next active thread. */
+	while (thread_fetch(master, &thread))
+		thread_call(&thread);
+}
+
+void
+ldpe_init(struct ldpd_init *init)
+{
+	/* drop privileges */
+	if (init->user)
+		ldpe_privs.user = init->user;
+	if (init->group)
+		ldpe_privs.group = init->group;
+	zprivs_init(&ldpe_privs);
+
+	/* listen on ldpd control socket */
+	strlcpy(ctl_sock_path, init->ctl_sock_path, sizeof(ctl_sock_path));
+	if (control_init(ctl_sock_path) == -1)
+		fatalx("control socket setup failed");
+	TAILQ_INIT(&ctl_conns);
+	control_listen();
+
+#ifdef HAVE_PLEDGE
+	if (pledge("stdio cpath inet mcast recvfd", NULL) == -1)
+		fatal("pledge");
+#endif
+
+	LIST_INIT(&global.addr_list);
+	RB_INIT(&global.adj_tree);
+	TAILQ_INIT(&global.pending_conns);
+	if (inet_pton(AF_INET, AllRouters_v4, &global.mcast_addr_v4) != 1)
+		fatal("inet_pton");
+	if (inet_pton(AF_INET6, AllRouters_v6, &global.mcast_addr_v6) != 1)
+		fatal("inet_pton");
 #ifdef __OpenBSD__
+	global.pfkeysock = pfkey_init();
 	if (sysdep.no_pfkey == 0)
 		pfkey_ev = thread_add_read(master, ldpe_dispatch_pfkey,
 		    NULL, global.pfkeysock);
@@ -175,16 +183,10 @@ ldpe(const char *user, const char *group, const char *ctl_path)
 	global.ipv6.ldp_edisc_socket = -1;
 	global.ipv6.ldp_session_socket = -1;
 
-	/* listen on ldpd control socket */
-	TAILQ_INIT(&ctl_conns);
-	control_listen();
-
 	if ((pkt_ptr = calloc(1, IBUF_READ_SIZE)) == NULL)
 		fatal(__func__);
 
-	/* Fetch next active thread. */
-	while (thread_fetch(master, &thread))
-		thread_call(&thread);
+	accept_init();
 }
 
 static void
@@ -203,7 +205,7 @@ ldpe_shutdown(void)
 	msgbuf_clear(&iev_main_sync->ibuf.w);
 	close(iev_main_sync->ibuf.fd);
 
-	control_cleanup();
+	control_cleanup(ctl_sock_path);
 	config_clear(leconf);
 
 #ifdef __OpenBSD__
@@ -352,6 +354,14 @@ ldpe_dispatch_main(struct thread *thread)
 			iev_lde->handler_write = ldp_write_handler;
 			iev_lde->ev_write = NULL;
 			break;
+		case IMSG_INIT:
+			if (imsg.hdr.len != IMSG_HEADER_SIZE +
+			    sizeof(struct ldpd_init))
+				fatalx("INIT imsg with wrong len");
+
+			memcpy(&init, imsg.data, sizeof(init));
+			ldpe_init(&init);
+			break;
 		case IMSG_CLOSE_SOCKETS:
 			af = imsg.hdr.peerid;
 
diff --git a/ldpd/ldpe.h b/ldpd/ldpe.h
index a3f41a8b9f..2c1ba46c6b 100644
--- a/ldpd/ldpe.h
+++ b/ldpd/ldpe.h
@@ -195,7 +195,8 @@ int	 tlv_decode_fec_elm(struct nbr *, struct ldp_msg *, char *,
 	    uint16_t, struct map *);
 
 /* ldpe.c */
-void		 ldpe(const char *, const char *, const char *);
+void		 ldpe(void);
+void		 ldpe_init(struct ldpd_init *);
 int		 ldpe_imsg_compose_parent(int, pid_t, void *,
 		    uint16_t);
 int		 ldpe_imsg_compose_lde(int, uint32_t, pid_t, void *,