fds etc.: Support non-forking persistent children

Polypath is are going to want to spawn a persistent child process,
which will not exec.  This child ought not to hold onto the various
important fds.

Otherwise, if the main secnet process dies but the child does not (for
some reason), the network interfaces, udp sockets, etc., set up by the
old secnet will remain owned by the child.

Introduce a new PHASE for this purpose (currently never entered).
Provide a convenient common hook function for closing a single fd.

Add phase hooks to:
 * Close udp sockets (in the udp and polypath comm modules);
 * Close the pipes to userv-ipif (slip netlink module);
 * Close the tun device (tun netlink module);
 * Zero out data transport keys, to improve forward secrecy in case
   the subprocess leaks somehow.  (Sadly we can't conveniently find
   the asymmmetric crypto session key negotiation state to wipe it.)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/comm-common.h b/comm-common.h
index 13709c7940ba9440f86f5f746c8760537c0d5b2b..aa757e1eee851ab52240fab32f0ad4ab4b06581d 100644 (file)
--- a/comm-common.h
+++ b/comm-common.h
@@ -102,6 +102,7 @@ void udp_sock_experienced(struct log_if *lg, struct udpcommon *uc,
 
 void udp_socks_register(struct udpcommon *uc, struct udpsocks *socks);
 void udp_socks_deregister(struct udpcommon *uc, struct udpsocks *socks);
+void udp_socks_childpersist(struct udpcommon *uc, struct udpsocks *socks);
 
 #define UDP_APPLY_STANDARD(st,uc,desc)                                 \
     (uc)->use_proxy=False;                                             \

diff --git a/polypath.c b/polypath.c
index 58a89ee9f0cc978add45ca6a95d170103360b69a..b8676325d5304cc20dadaa17ad0d5fe3d46e81d5 100644 (file)
--- a/polypath.c
+++ b/polypath.c
@@ -506,6 +506,15 @@ static void polypath_phase_shutdown(void *sst, uint32_t newphase)
     }
 }
 
+static void polypath_phase_childpersist(void *sst, uint32_t newphase)
+{
+    struct polypath *st=sst;
+    struct interf *interf;
+
+    LIST_FOREACH(interf,&st->interfs,entry)
+       udp_socks_childpersist(&st->uc,&interf->socks);
+}
+
 #undef BAD
 #undef BADE
 
@@ -546,6 +555,7 @@ static list_t *polypath_apply(closure_t *self, struct cloc loc,
 
     add_hook(PHASE_RUN,         polypath_phase_startmonitor,st);
     add_hook(PHASE_SHUTDOWN,    polypath_phase_shutdown,    st);
+    add_hook(PHASE_CHILDPERSIST,polypath_phase_childpersist,st);
 
     return new_closure(&cc->cl);
 }

diff --git a/process.c b/process.c
index 77fe38e3127134077d8cbdbc3e6e460bcefaf942..c14dd694d977afe51dd742c9428303890cb4869d 100644 (file)
--- a/process.c
+++ b/process.c
@@ -236,6 +236,16 @@ void afterfork(void)
     sigprocmask(SIG_SETMASK,&emptyset,NULL);
 }
 
+void childpersist_closefd_hook(void *fd_vp, uint32_t newphase)
+{
+    int *fd_p=fd_vp;
+    int fd=*fd_p;
+    if (fd<0) return;
+    *fd_p=-1;
+    setnonblock(fd); /* in case close() might block */
+    close(fd); /* discard errors - we don't care, in the child */
+}
+
 static void signal_handler(int signum)
 {
     int saved_errno;

diff --git a/secnet.h b/secnet.h
index 76db6038c9e1ce9433849a1dabcfe609da221788..eb250a4a4fc3c38b3ababaef20ba595d23b9fa09 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -64,6 +64,12 @@ void afterfork(void);
 /* Must be called before exec in every child made after
    start_signal_handling.  Safe to call in earlier children too. */
 
+void childpersist_closefd_hook(void *fd_p, uint32_t newphase);
+/* Convenience hook function for use with add_hook PHASE_CHILDPERSIST.
+   With `int fd' in your state struct, pass fd_p=&fd.  The hook checks
+   whether fd>=0, so you can use it for an fd which is only sometimes
+   open.  This function will set fd to -1, so it is idempotent. */
+
 /***** CONFIGURATION support *****/
 
 extern bool_t just_check_config; /* If True then we're going to exit after
@@ -258,10 +264,18 @@ enum phase {
     PHASE_DROPPRIV,            /* Last chance for privileged operations */
     PHASE_RUN,
     PHASE_SHUTDOWN,            /* About to die; delete key material, etc. */
+    PHASE_CHILDPERSIST,        /* Forked long-term child: close fds, etc. */
     /* Keep this last: */
     NR_PHASES,
 };
 
+/* Each module should, in its CHILDPERSIST hooks, close all fds which
+   constitute ownership of important operating system resources, or
+   which are used for IPC with other processes who want to get the
+   usual disconnection effects if the main secnet process dies.
+   CHILDPERSIST hooks are not run if the child is going to exec;
+   so fds such as described above should be CLOEXEC too. */
+
 typedef void hook_fn(void *self, uint32_t newphase);
 bool_t add_hook(uint32_t phase, hook_fn *f, void *state);
 bool_t remove_hook(uint32_t phase, hook_fn *f, void *state);

diff --git a/site.c b/site.c
index 935bf38f76fb0aac273e49ae4d17865f7cf652af..1c57a8a66b6296ca305b7efab2d4cf9035838c56 100644 (file)
--- a/site.c
+++ b/site.c
@@ -1901,6 +1901,23 @@ static void site_phase_hook(void *sst, uint32_t newphase)
     send_msg7(st,"shutting down");
 }
 
+static void site_childpersist_clearkeys(void *sst, uint32_t newphase)
+{
+    struct site *st=sst;
+    dispose_transform(&st->current.transform);
+    dispose_transform(&st->auxiliary_key.transform);
+    dispose_transform(&st->new_transform);
+    /* Not much point overwiting the signing key, since we loaded it
+       from disk, and it is only valid prospectively if at all,
+       anyway. */
+    /* XXX it would be best to overwrite the DH state, because that
+       _is_ relevant to forward secrecy.  However we have no
+       convenient interface for doing that and in practice gmp has
+       probably dribbled droppings all over the malloc arena.  A good
+       way to fix this would be to have a privsep child for asymmetric
+       crypto operations, but that's a task for another day. */
+}
+
 static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
                          list_t *args)
 {
@@ -2090,6 +2107,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     enter_state_stop(st);
 
     add_hook(PHASE_SHUTDOWN,site_phase_hook,st);
+    add_hook(PHASE_CHILDPERSIST,site_childpersist_clearkeys,st);
 
     return new_closure(&st->cl);
 }

diff --git a/slip.c b/slip.c
index e555f471218b710b12ecbe35a075ec0660533cd1..fad0a7ee394fa2a764e9853129f68cc19dac55b8 100644 (file)
--- a/slip.c
+++ b/slip.c
@@ -383,6 +383,9 @@ static void userv_invoke_userv(struct userv *st)
     }
     setnonblock(st->txfd);
     setnonblock(st->rxfd);
+
+    add_hook(PHASE_CHILDPERSIST,childpersist_closefd_hook,&st->txfd);
+    add_hook(PHASE_CHILDPERSIST,childpersist_closefd_hook,&st->rxfd);
 }
 
 static void userv_kill_userv(struct userv *st)

diff --git a/tun.c b/tun.c
index 2c84ed0dace4d51a35c16071a7926e59859159be..b3c69a2351e0fea272c4fef404c50d585b8159d5 100644 (file)
--- a/tun.c
+++ b/tun.c
@@ -442,6 +442,8 @@ static void tun_phase_hook(void *sst, uint32_t newphase)
        tun_set_route(st,r);
     }
 
+    add_hook(PHASE_CHILDPERSIST,childpersist_closefd_hook,&st->fd);
+
     /* Register for poll() */
     register_for_poll(st, tun_beforepoll, tun_afterpoll, st->nl.name);
 }

diff --git a/udp.c b/udp.c
index 706e07786d7dc76b99f2aaaa1677bc8832067cfb..e62f5c146332852fb9a4d083d50b2ecce0e15c14 100644 (file)
--- a/udp.c
+++ b/udp.c
@@ -340,6 +340,19 @@ void udp_socks_deregister(struct udpcommon *uc, struct udpsocks *socks)
     deregister_for_poll(socks->interest);
 }
 
+void udp_socks_childpersist(struct udpcommon *uc, struct udpsocks *socks)
+{
+    int i;
+    for (i=0; i<socks->n_socks; i++)
+       udp_destroy_socket(uc,&socks->socks[i]);
+}
+
+static void udp_childpersist_hook(void *sst, uint32_t new_phase)
+{
+    struct udp *st=sst;
+    udp_socks_childpersist(&st->uc,&st->socks);
+}
+
 static void udp_phase_hook(void *sst, uint32_t new_phase)
 {
     struct udp *st=sst;
@@ -350,6 +363,8 @@ static void udp_phase_hook(void *sst, uint32_t new_phase)
        udp_make_socket(uc,&socks->socks[i],M_FATAL);
 
     udp_socks_register(uc,socks);
+
+    add_hook(PHASE_CHILDPERSIST,udp_childpersist_hook,st);
 }
 
 static list_t *udp_apply(closure_t *self, struct cloc loc, dict_t *context,

diff --git a/util.c b/util.c
index 12e52085735508950874bacc4af2dcce571a7a66..d843859f40dcb5fe6593592547888c98b63bbda2 100644 (file)
--- a/util.c
+++ b/util.c
@@ -204,7 +204,8 @@ static const char *phases[NR_PHASES]={
     "PHASE_GETRESOURCES",
     "PHASE_DROPPRIV",
     "PHASE_RUN",
-    "PHASE_SHUTDOWN"
+    "PHASE_SHUTDOWN",
+    "PHASE_CHILDPERSIST"
 };
 
 void enter_phase(uint32_t new_phase)