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 5ab6e467f4ed2c5ed105404dc6193d0e742119a4..c2171f66a86faaac12fa3a8a1b3e7e7da1962629 100644 (file)
--- a/comm-common.h
+++ b/comm-common.h
@@ -92,6 +92,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 8b60ac2136eb9148c0a23af58c0f069cb07e7d94..fa7b9a4df10b59ea0e5603b90681ad08bf4c769b 100644 (file)
--- a/polypath.c
+++ b/polypath.c
@@ -505,6 +505,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 1cb4a21d8c5a73e740ad4faeb5631da46ff11202..355d657f9d7a03362592a87f00a4bd0f264918a1 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
@@ -255,10 +261,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 4c9a123f7a54765e577b7b7b6f73df950e87de7a..f983f101c66685c692bc2f5b0d6ef9d37a2bd14b 100644 (file)
--- a/site.c
+++ b/site.c
@@ -1900,6 +1900,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)
 {
@@ -2089,6 +2106,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 abfd922e0aa97ad39db6b0300083a0ee149dc93c..04c96456221ae278eff04be29256fe5a91648819 100644 (file)
--- a/slip.c
+++ b/slip.c
@@ -383,6 +383,9 @@ static void userv_invoke_userv(struct userv *st)
                  st->slip.nl.name,confirm);
        }
     }
+
+    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 476bdc281291ac216eb618881d8997272581b5cf..dca8db6c4f08b18e0e05e66e9adbbaeb0ee0a548 100644 (file)
--- a/udp.c
+++ b/udp.c
@@ -325,6 +325,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;
@@ -335,6 +348,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 ab4eee8de12c50e77987f90ed84322da419c579e..fb0a4316d8ea15534fa2ae66935170d230fc6a7b 100644 (file)
--- a/util.c
+++ b/util.c
@@ -202,7 +202,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)