iked hereby pledges that it will run with restricted system
authorreyk <reyk@openbsd.org>
Thu, 22 Oct 2015 15:55:18 +0000 (15:55 +0000)
committerreyk <reyk@openbsd.org>
Thu, 22 Oct 2015 15:55:18 +0000 (15:55 +0000)
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
sbin/iked/control.c
sbin/iked/iked.c
sbin/iked/iked.h
sbin/iked/ikev2.c
sbin/iked/proc.c
sbin/iked/types.h

index 8fcfd32..a03402a 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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)
 {
index 267f6e5..39e8210 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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)
index bdc1b2b..e94e219 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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);
        }
index ea6da50..afd4148 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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 *);
index 28071a2..22e7661 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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
index 18b219b..88561c7 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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
  */
index fc43b54..9c3be52 100644 (file)
@@ -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 <reyk@openbsd.org>
@@ -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