From 274f5abf24fd5c9c79dce4102c3dfa583a791559 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Tue, 18 Apr 2017 22:03:35 -0300 Subject: [PATCH] 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 --- ldpd/control.c | 18 +++++------ ldpd/control.h | 4 +-- ldpd/lde.c | 58 +++++++++++++++++++-------------- ldpd/lde.h | 3 +- ldpd/ldpd.c | 62 ++++++++++------------------------- ldpd/ldpd.h | 13 ++++++-- ldpd/ldpe.c | 88 ++++++++++++++++++++++++++++---------------------- ldpd/ldpe.h | 3 +- 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 *, -- 2.39.5