From ebfc369325d2c22f833acf029b45694846aba023 Mon Sep 17 00:00:00 2001 From: reyk Date: Thu, 22 Oct 2015 15:55:18 +0000 Subject: [PATCH] iked hereby pledges that it will run with restricted system operations. This adds pledge(2) too all processes, including the iked parent process; the existing privsep design has been improved for better pledgeability. There haven't been any serious problems as it was already sane (eg. by receiving the PFKEYv2 and UDP sockets via fd passing). The control socket moved to an independent process to remove some abilities from the cert process. Committed in agreement with many but nobody was brave enough to OK it. Better testing will happen with having it in the tree. "It's the truth" deraadt@ "Let's see what happens" benno@ --- sbin/iked/ca.c | 18 +++++++++++++++++- sbin/iked/control.c | 27 ++++++++++++++++++++++++++- sbin/iked/iked.c | 43 ++++++++++++++++++++++++++++++++----------- sbin/iked/iked.h | 3 ++- sbin/iked/ikev2.c | 19 +++++++++++++++++-- sbin/iked/proc.c | 17 +++++++++++++---- sbin/iked/types.h | 8 +++----- 7 files changed, 110 insertions(+), 25 deletions(-) diff --git a/sbin/iked/ca.c b/sbin/iked/ca.c index 8fcfd3246d6..a03402a26d8 100644 --- a/sbin/iked/ca.c +++ b/sbin/iked/ca.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ca.c,v 1.38 2015/10/19 11:25:35 reyk Exp $ */ +/* $OpenBSD: ca.c,v 1.39 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -46,6 +46,7 @@ #include "iked.h" #include "ikev2.h" +void ca_run(struct privsep *, struct privsep_proc *, void *); void ca_reset(struct privsep *, struct privsep_proc *, void *); int ca_reload(struct iked *); @@ -116,6 +117,21 @@ caproc(struct privsep *ps, struct privsep_proc *p) return (proc_run(ps, p, procs, nitems(procs), ca_reset, store)); } +void +ca_run(struct privsep *ps, struct privsep_proc *p, void *arg) +{ + /* + * pledge in the ca process: + * stdio - for malloc and basic I/O including events. + * rpath - for certificate files. + * recvfd - for ocsp sockets. + */ + if (pledge("stdio rpath recvfd", NULL) == -1) + fatal("pledge"); + + ca_reset(ps, p, arg); +} + void ca_reset(struct privsep *ps, struct privsep_proc *p, void *arg) { diff --git a/sbin/iked/control.c b/sbin/iked/control.c index 267f6e5967d..39e82102fb9 100644 --- a/sbin/iked/control.c +++ b/sbin/iked/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.18 2015/10/19 11:27:35 reyk Exp $ */ +/* $OpenBSD: control.c,v 1.19 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -45,7 +45,32 @@ struct ctl_conn *control_connbyfd(int); void control_close(int, struct control_sock *); void control_dispatch_imsg(int, short, void *); +void control_dispatch_parent(int, short, void *); void control_imsg_forward(struct imsg *); +void control_run(struct privsep *, struct privsep_proc *, void *); + +static struct privsep_proc procs[] = { + { "parent", PROC_PARENT, NULL } +}; + +pid_t +control(struct privsep *ps, struct privsep_proc *p) +{ + return (proc_run(ps, p, procs, nitems(procs), control_run, NULL)); +} + +void +control_run(struct privsep *ps, struct privsep_proc *p, void *arg) +{ + /* + * pledge in the control process: + * stdio - for malloc and basic I/O including events. + * cpath - for unlinking the control socket. + * unix - for the control socket. + */ + if (pledge("stdio cpath unix", NULL) == -1) + fatal("pledge"); +} int control_init(struct privsep *ps, struct control_sock *cs) diff --git a/sbin/iked/iked.c b/sbin/iked/iked.c index bdc1b2b0cf4..e94e219f6da 100644 --- a/sbin/iked/iked.c +++ b/sbin/iked/iked.c @@ -1,4 +1,4 @@ -/* $OpenBSD: iked.c,v 1.27 2015/10/19 11:25:35 reyk Exp $ */ +/* $OpenBSD: iked.c,v 1.28 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -39,13 +39,14 @@ __dead void usage(void); void parent_shutdown(struct iked *); void parent_sig_handler(int, short, void *); -int parent_dispatch_ikev2(int, struct privsep_proc *, struct imsg *); int parent_dispatch_ca(int, struct privsep_proc *, struct imsg *); +int parent_dispatch_control(int, struct privsep_proc *, struct imsg *); int parent_configure(struct iked *); static struct privsep_proc procs[] = { - { "ikev2", PROC_IKEV2, parent_dispatch_ikev2, ikev2 }, - { "ca", PROC_CERT, parent_dispatch_ca, caproc, IKED_CA } + { "ca", PROC_CERT, parent_dispatch_ca, caproc, IKED_CA }, + { "control", PROC_CONTROL, parent_dispatch_control, control }, + { "ikev2", PROC_IKEV2, NULL, ikev2 } }; __dead void @@ -216,6 +217,24 @@ parent_configure(struct iked *env) if ((env->sc_opts & IKED_OPT_NONATT) == 0) config_setsocket(env, &ss, ntohs(IKED_NATT_PORT), PROC_IKEV2); + /* + * pledge in the parent process: + * It has to run fairly late to allow forking the processes and + * opening the PFKEY socket and the listening UDP sockets (once) + * that need the bypass ioctls that are never allowed by pledge. + * + * Other flags: + * stdio - for malloc and basic I/O including events. + * rpath - for reload to open and read the configuration files. + * proc - run kill to terminate its children safely. + * dns - for reload and ocsp connect. + * inet - for ocsp connect. + * route - for using interfaces in iked.conf (SIOCGIFGMEMB) + * sendfd - for ocsp sockets. + */ + if (pledge("stdio rpath proc dns inet route sendfd", NULL) == -1) + fatal("pledge"); + config_setcoupled(env, env->sc_decoupled ? 0 : 1); config_setmode(env, env->sc_passive ? 1 : 0); config_setocsp(env); @@ -329,18 +348,23 @@ parent_sig_handler(int sig, short event, void *arg) } int -parent_dispatch_ikev2(int fd, struct privsep_proc *p, struct imsg *imsg) +parent_dispatch_ca(int fd, struct privsep_proc *p, struct imsg *imsg) { + struct iked *env = p->p_ps->ps_env; + switch (imsg->hdr.type) { - default: + case IMSG_OCSP_FD: + ocsp_connect(env); break; + default: + return (-1); } - return (-1); + return (0); } int -parent_dispatch_ca(int fd, struct privsep_proc *p, struct imsg *imsg) +parent_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) { struct iked *env = p->p_ps->ps_env; int v; @@ -366,9 +390,6 @@ parent_dispatch_ca(int fd, struct privsep_proc *p, struct imsg *imsg) parent_reload(env, 0, str); free(str); break; - case IMSG_OCSP_FD: - ocsp_connect(env); - break; default: return (-1); } diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h index ea6da507422..afd4148e426 100644 --- a/sbin/iked/iked.h +++ b/sbin/iked/iked.h @@ -1,4 +1,4 @@ -/* $OpenBSD: iked.h,v 1.90 2015/10/19 11:25:35 reyk Exp $ */ +/* $OpenBSD: iked.h,v 1.91 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -612,6 +612,7 @@ struct iked_socket { void parent_reload(struct iked *, int, const char *); /* control.c */ +pid_t control(struct privsep *, struct privsep_proc *); int control_init(struct privsep *, struct control_sock *); int control_listen(struct control_sock *); void control_cleanup(struct control_sock *); diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c index 28071a2d0be..22e7661f855 100644 --- a/sbin/iked/ikev2.c +++ b/sbin/iked/ikev2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ikev2.c,v 1.127 2015/10/19 11:25:35 reyk Exp $ */ +/* $OpenBSD: ikev2.c,v 1.128 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -45,6 +45,7 @@ #include "eap.h" #include "dh.h" +void ikev2_run(struct privsep *, struct privsep_proc *, void *); int ikev2_dispatch_parent(int, struct privsep_proc *, struct imsg *); int ikev2_dispatch_cert(int, struct privsep_proc *, struct imsg *); @@ -136,7 +137,21 @@ static struct privsep_proc procs[] = { pid_t ikev2(struct privsep *ps, struct privsep_proc *p) { - return (proc_run(ps, p, procs, nitems(procs), NULL, NULL)); + return (proc_run(ps, p, procs, nitems(procs), ikev2_run, NULL)); +} + +void +ikev2_run(struct privsep *ps, struct privsep_proc *p, void *arg) +{ + /* + * pledge in the ikev2 process: + * stdio - for malloc and basic I/O including events. + * inet - for sendto with specified peer address. + * recvfd - for PFKEYv2 and the listening UDP sockets. + * In theory, recvfd could be dropped after getting the fds once. + */ + if (pledge("stdio inet recvfd", NULL) == -1) + fatal("pledge"); } int diff --git a/sbin/iked/proc.c b/sbin/iked/proc.c index 18b219b5e12..88561c7ac80 100644 --- a/sbin/iked/proc.c +++ b/sbin/iked/proc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: proc.c,v 1.23 2015/08/21 11:59:28 reyk Exp $ */ +/* $OpenBSD: proc.c,v 1.24 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010 - 2014 Reyk Floeter @@ -45,6 +45,7 @@ int proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid); void proc_shutdown(struct privsep_proc *); void proc_sig_handler(int, short, void *); void proc_range(struct privsep *, enum privsep_procid, int *, int *); +int proc_dispatch_null(int, struct privsep_proc *, struct imsg *); int proc_ispeer(struct privsep_proc *procs, unsigned int nproc, @@ -164,6 +165,8 @@ proc_open(struct privsep *ps, struct privsep_proc *p, for (proc = 0; proc < nproc; proc++) { procs[proc].p_ps = ps; procs[proc].p_env = ps->ps_env; + if (procs[proc].p_cb == NULL) + procs[proc].p_cb = proc_dispatch_null; for (i = 0; i < ps->ps_instances[src]; i++) { for (j = 0; j < ps->ps_instances[procs[proc].p_id]; @@ -332,7 +335,7 @@ proc_sig_handler(int sig, short event, void *arg) pid_t proc_run(struct privsep *ps, struct privsep_proc *p, struct privsep_proc *procs, unsigned int nproc, - void (*init)(struct privsep *, struct privsep_proc *, void *), void *arg) + void (*run)(struct privsep *, struct privsep_proc *, void *), void *arg) { pid_t pid; struct passwd *pw; @@ -427,8 +430,8 @@ proc_run(struct privsep *ps, struct privsep_proc *p, fatalx(p->p_title); } - if (init != NULL) - init(ps, p, arg); + if (run != NULL) + run(ps, p, arg); event_dispatch(); @@ -509,6 +512,12 @@ proc_dispatch(int fd, short event, void *arg) imsg_event_add(iev); } +int +proc_dispatch_null(int fd, struct privsep_proc *p, struct imsg *imsg) +{ + return (-1); +} + /* * imsg helper functions */ diff --git a/sbin/iked/types.h b/sbin/iked/types.h index fc43b54cf34..9c3be522067 100644 --- a/sbin/iked/types.h +++ b/sbin/iked/types.h @@ -1,4 +1,4 @@ -/* $OpenBSD: types.h,v 1.22 2015/10/19 11:25:35 reyk Exp $ */ +/* $OpenBSD: types.h,v 1.23 2015/10/22 15:55:18 reyk Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -113,14 +113,12 @@ enum imsg_type { enum privsep_procid { PROC_PARENT = 0, - PROC_IKEV2, + PROC_CONTROL, PROC_CERT, + PROC_IKEV2, PROC_MAX }; -/* Attach the control socket to the following process */ -#define PROC_CONTROL PROC_CERT - enum blockmodes { BM_NORMAL, BM_NONBLOCK -- 2.20.1