server/: Make initialization errors be non-fatal and restartable.
authorMark Wooding <mdw@distorted.org.uk>
Sat, 19 May 2018 21:03:28 +0000 (22:03 +0100)
committerMark Wooding <mdw@distorted.org.uk>
Thu, 5 Sep 2019 12:07:21 +0000 (13:07 +0100)
It should now be possible to trap any error during startup, change any
part of the configuration, shut down the affected components, and
restart everything again, without taking down the whole process.

Mostly, this involves replacing the existing calls to `exit' with
goto-cleanup and return codes.  In some cases, the existing
functionality has been reordered to make cleanup easier.  I didn't leave
this for a separate commit, because including those changes here makes
it clearer what they're for and should make it easier to check that
they're the right fixes.

The details are:

  * admin.c (a_listen): Move resetting the `umask' to the end.

    Specifically, after the last part of the function that can fail.
    This avoids a double reset if the final part, the call to listen(2),
    fails.

  * admin.c (a_init): Move creation of the service table to the end.

    This isn't (currently) a thing that can be cleaned up, so do it only
    after the parts that can fail -- specifically, initializing ADNS.

  * keymgmt.c (kh_init): Use `kh->kf' as a flag to prevent double init.

    As mentioned earlier, this is cleared by the static initializer, so
    we can safely assume that `kh->kf' is null if and only if the
    keyhalf requires initialization.

    Also, reorder slightly, to establish the cache hashtable only after
    the keyring file has been read.

  * keymgmt.c (km_init): Refresh before fetching the master key.

    Suppose we fail to initialize because the master key is missing.
    The right fix is to update the keyring file with the proper key, and
    then retry.  But at this point the private keyhalf has been
    initialized; so we must force a refresh of the keyring data.

  * keymgmt.c (km_init): Be idempotent regarding the master key.

    If there's a master key cached, then don't clobber it if we can't
    find it again.  On the other hand, if we find a different one this
    time then switch.

server/admin.c
server/keymgmt.c
server/peer.c
server/privsep.c
server/standalone.c
server/tripe.h
server/tun-slip.c
server/tun-std.c

index 78188f5..ad44132 100644 (file)
@@ -2470,12 +2470,12 @@ void a_daemon(void) { flags |= F_DAEMON; }
  *             @gid_t g@ = group to own the socket
  *             @mode_t m@ = permissions to set on the socket
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Creates the admin listening socket.
  */
 
-void a_listen(const char *name, uid_t u, gid_t g, mode_t m)
+int a_listen(const char *name, uid_t u, gid_t g, mode_t m)
 {
   int fd;
   int n = 5;
@@ -2488,7 +2488,7 @@ void a_listen(const char *name, uid_t u, gid_t g, mode_t m)
   sz = strlen(name) + 1;
   if (sz > sizeof(sun.sun_path)) {
     a_warn("ADMIN", "admin-socket", "%s", name, "name-too-long", A_END);
-    exit(EXIT_FAILURE);
+    goto fail_0;
   }
   BURN(sun);
   sun.sun_family = AF_UNIX;
@@ -2502,7 +2502,7 @@ again:
   if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) {
     a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
           "create-failed", "?ERRNO", A_END);
-    exit(EXIT_FAILURE);
+    goto fail_1;
   }
   if (bind(fd, (struct sockaddr *)&sun, sz) < 0) {
     struct stat st;
@@ -2510,34 +2510,34 @@ again:
     if (errno != EADDRINUSE) {
       a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
             "bind-failed", "?ERRNO", A_END);
-      exit(EXIT_FAILURE);
+      goto fail_2;
     }
     if (!n) {
       a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
             "too-many-retries", A_END);
-      exit(EXIT_FAILURE);
+      goto fail_2;
     }
     n--;
     if (!connect(fd, (struct sockaddr *)&sun, sz)) {
       a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
             "already-in-use", A_END);
-      exit(EXIT_FAILURE);
+      goto fail_2;
     }
     if (errno != ECONNREFUSED) {
       a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
             "bind-failed", "?ERR", e, A_END);
-      exit(EXIT_FAILURE);
+      goto fail_2;
     }
     if (stat(sun.sun_path, &st)) {
       if (errno == ENOENT) { close(fd); goto again; }
       a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
             "stat-failed", "?ERRNO", A_END);
-      exit(EXIT_FAILURE);
+      goto fail_2;
     }
     if (!S_ISSOCK(st.st_mode)) {
       a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
             "not-a-socket", A_END);
-      exit(EXIT_FAILURE);
+      goto fail_2;
     }
     T( trace(T_ADMIN, "admin: stale socket found; removing it"); )
     unlink(sun.sun_path);
@@ -2547,26 +2547,39 @@ again:
   if (chown(sun.sun_path, u, g)) {
     a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
           "chown-failed", "?ERRNO", A_END);
-    exit(EXIT_FAILURE);
+    goto fail_3;
   }
   if (chmod(sun.sun_path, m)) {
     a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
           "chmod-failed", "?ERRNO", A_END);
-    exit(EXIT_FAILURE);
+    goto fail_3;
   }
-  umask(omask);
   fdflags(fd, O_NONBLOCK, O_NONBLOCK, FD_CLOEXEC, FD_CLOEXEC);
   if (listen(fd, 5)) {
     a_warn("ADMIN", "admin-socket", "%s", sun.sun_path,
           "listen-failed", "?ERRNO", A_END);
-    exit(EXIT_FAILURE);
+    goto fail_3;
   }
+  umask(omask);
 
   /* --- Listen to the socket --- */
 
   sel_initfile(&sel, &sock, fd, SEL_READ, a_accept, 0);
   sel_addfile(&sock);
   sockname = name;
+
+  return (0);
+
+  /* --- Clean up if things go sideways --- */
+
+fail_3:
+  unlink(sun.sun_path);
+fail_2:
+  close(fd);
+fail_1:
+  umask(omask);
+fail_0:
+  return (-1);
 }
 
 /* --- @a_unlisten@ --- *
@@ -2633,21 +2646,17 @@ void a_signals(void)
  *
  * Arguments:  ---
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Creates the admin listening socket.
  */
 
-void a_init(void)
+int a_init(void)
 {
 #ifdef HAVE_LIBADNS
   int err;
 #endif
 
-  /* --- Create services table --- */
-
-  sym_create(&a_svcs);
-
   /* --- Prepare the background name resolver --- */
 
 #ifdef HAVE_LIBADNS
@@ -2657,12 +2666,20 @@ void a_init(void)
                        adns_if_noautosys),
                       0)) != 0) {
     a_warn("ADMIN", "adns-init-failed", "?ERRNO", A_END);
-    exit(EXIT_FAILURE);
+    return (-1);
   }
   sel_addhook(&sel, &hook, before_select, after_select, 0);
 #else
   bres_init(&sel);
 #endif
+
+  /* --- Create services table --- */
+
+  sym_create(&a_svcs);
+
+  /* --- All done --- */
+
+  return (0);
 }
 
 /*----- That's all, folks -------------------------------------------------*/
index 48e4de6..84d7010 100644 (file)
@@ -291,19 +291,21 @@ static int kh_reopen(keyhalf *kh)
  * Arguments:  @keyhalf *kh@ = pointer to keyhalf structure to set up
  *             @const char *kr@ = name of the keyring file
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on error.
  *
  * Use:                Initialize a keyhalf structure, maintaining the private or
  *             public keys.  Intended to be called during initialization:
  *             exits if there's some kind of problem.
  */
 
-static void kh_init(keyhalf *kh, const char *kr)
+static int kh_init(keyhalf *kh, const char *kr)
 {
+  if (kh->kf) return (0);
   kh->kr = xstrdup(kr);
+  if (kh_reopen(kh)) return (-1);
   fwatch_init(&kh->w, kr);
   sym_create(&kh->tab);
-  if (kh_reopen(kh)) exit(EXIT_FAILURE);
+  return (0);
 }
 
 /* --- @kh_load@ --- *
@@ -554,7 +556,7 @@ static void kh_clear(keyhalf *kh)
 /*----- Main code ---------------------------------------------------------*/
 
 char *tag_priv = 0;
-kdata *master;
+kdata *master = 0;
 
 /* --- @km_init@ --- *
  *
@@ -562,15 +564,16 @@ kdata *master;
  *             @const char *pubkr@ = public keyring file
  *             @const char *ptag@ = default private-key tag
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Initializes the key-management machinery, loading the
  *             keyrings and so on.
  */
 
-void km_init(const char *privkr, const char *pubkr, const char *ptag)
+int km_init(const char *privkr, const char *pubkr, const char *ptag)
 {
   const gchash *const *hh;
+  kdata *kd;
 
   for (hh = ghashtab; *hh; hh++) {
     if ((*hh)->hashsz > MAXHASHSZ) {
@@ -581,11 +584,17 @@ void km_init(const char *privkr, const char *pubkr, const char *ptag)
     }
   }
 
-  kh_init(&priv, privkr);
-  kh_init(&pub, pubkr);
+  if (kh_init(&priv, privkr) || kh_init(&pub, pubkr))
+    return (-1);
 
   tag_priv = ptag ? xstrdup(ptag) : 0;
-  if ((master = km_findpriv(ptag)) == 0) exit(EXIT_FAILURE);
+  kh_refresh(&priv);
+
+  if ((kd = km_findpriv(tag_priv)) == 0) return (-1);
+  if (master) km_unref(master);
+  master = kd;
+
+  return (0);
 }
 
 /* --- @km_reload@ --- *
index 278a6b2..9530683 100644 (file)
@@ -809,14 +809,14 @@ const addr *p_addr(peer *p) { return (&p->spec.sa); }
  *
  * Arguments:  @struct addrinfo *ailist@ = addresses to bind to
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
- * Use:                Initializes the peer system; creates the socket.
+ * Use:                Binds to the main UDP sockets.
  */
 
-void p_bind(struct addrinfo *ailist)
+int p_bind(struct addrinfo *ailist)
 {
-  int fd;
+  int fd = -1;
   int len = PKBUFSZ;
   int yes = 1;
   int i;
@@ -842,13 +842,13 @@ void p_bind(struct addrinfo *ailist)
     if ((fd = socket(ai->ai_family, SOCK_DGRAM, 0)) < 0) {
       a_warn("PEER", "-", "udp-socket", "%s", aftab[i].name,
             "create-failed", "?ERRNO", A_END);
-      exit(EXIT_FAILURE);
+      goto fail;
     }
     if (i == AFIX_INET6 &&
        setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &yes, sizeof(yes))) {
       a_warn("PEER", "-", "udp-socket", "%s", aftab[i].name,
             "set-v6only-failed", "?ERRNO", A_END);
-      exit(EXIT_FAILURE);
+      goto fail;
     }
     assert(ai->ai_addrlen <= sizeof(a));
     memcpy(&a, ai->ai_addr, ai->ai_addrlen);
@@ -856,13 +856,13 @@ void p_bind(struct addrinfo *ailist)
     if (bind(fd, &a.sa, addrsz(&a))) {
       a_warn("PEER", "-", "udp-socket", "%s", aftab[i].name,
             "bind-failed", "?ERRNO", A_END);
-      exit(EXIT_FAILURE);
+      goto fail;
     }
     if (setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &len, sizeof(len)) ||
        setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &len, sizeof(len))) {
       a_warn("PEER", "-", "udp-socket", "%s", aftab[i].name,
             "set-buffers-failed", "?ERRNO", A_END);
-      exit(EXIT_FAILURE);
+      goto fail;
     }
     fdflags(fd, O_NONBLOCK, O_NONBLOCK, FD_CLOEXEC, FD_CLOEXEC);
     if (port)
@@ -872,15 +872,22 @@ void p_bind(struct addrinfo *ailist)
       if (getsockname(fd, &a.sa, &sz)) {
        a_warn("PEER", "-", "udp-socket", "%s", aftab[i].name,
               "read-local-address-failed", "?ERRNO", A_END);
-       exit(EXIT_FAILURE);
+       goto fail;
       }
       udpsock[i].port = lastport = getport(&a);
     }
     T( trace(T_PEER, "peer: created %s socket", aftab[i].name); )
     sel_initfile(&sel, &udpsock[i].sf, fd, SEL_READ, p_read, 0);
     sel_addfile(&udpsock[i].sf);
+    fd = -1;
   }
 
+  return (0);
+
+fail:
+  if (fd != -1) close(fd);
+  p_unbind();
+  return (-1);
 }
 
 /* --- @p_unbind@ --- *
@@ -928,28 +935,29 @@ void p_init(void)
  *
  * Arguments:  @const tunnel_ops *tops@ = tunnel ops to add
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
- * Use:                Adds a tunnel class to the list of known classes.  If there
- *             is no current default tunnel, then this one is made the
- *             default.
+ * Use:                Adds a tunnel class to the list of known classes, if it
+ *             initializes properly.  If there is no current default tunnel,
+ *             then this one is made the default.
  *
  *             Does nothing if the tunnel class is already known.  So adding
  *             a bunch of tunnels takes quadratic time, but there will be
  *             too few to care about.
  */
 
-void p_addtun(const tunnel_ops *tops)
+int p_addtun(const tunnel_ops *tops)
 {
   struct tunnel_node *tn;
 
   for (tn = tunnels; tn; tn = tn->next)
-    if (tn->tops == tops) return;
-  tops->init();
+    if (tn->tops == tops) return (0);
+  if (tops->init()) return (-1);
   tn = CREATE(struct tunnel_node);
   tn->next = 0; tn->tops = tops;
   *tunnels_tail = tn; tunnels_tail = &tn->next;
   if (!dflttun) dflttun = tops;
+  return (0);
 }
 
 /* --- @p_setdflttun@ --- *
index 56b22be..ffe6404 100644 (file)
@@ -163,13 +163,13 @@ static void reap(int sig, void *p)
  *
  * Arguments:  @int detachp@ = whether to detach the child from its terminal
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Separates off the privileged tunnel-opening service from the
  *             rest of the server.
  */
 
-void ps_split(int detachp)
+int ps_split(int detachp)
 {
   pid_t kid;
   int fd[2];
@@ -178,7 +178,7 @@ void ps_split(int detachp)
 
   if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd)) {
     a_warn("PRIVSEP", "socketpair-create-failed", "?ERRNO", A_END);
-    exit(EXIT_FAILURE);
+    return (-1);
   }
   helper = getenv("TRIPE_PRIVHELPER");
   if (!helper) helper = PRIVSEP_HELPER;
@@ -200,6 +200,7 @@ void ps_split(int detachp)
   T( trace(T_PRIVSEP, "privsep: forked child successfully"); )
   close(fd[0]);
   pc_fd = fd[1];
+  return (0);
 }
 
 /* --- @ps_quit@ --- *
index 9b4976c..b6f3487 100644 (file)
@@ -287,22 +287,23 @@ int main(int argc, char *argv[])
 
   p_init();
   for (i = 0; i < N(tunnels); i++)
-    p_addtun(tunnels[i]);
+    if (p_addtun(tunnels[i])) exit(EXIT_FAILURE);
   if (dflt) p_setdflttun(dflt);
-  p_bind(ailist); freeaddrinfo(ailist);
+  if (p_bind(ailist)) exit(EXIT_FAILURE);
+  freeaddrinfo(ailist);
 
   for (i = 0; tunnels[i]; i++) {
     if (tunnels[i]->flags&TUNF_PRIVOPEN) {
-      ps_split(f & f_daemon);
+      if (ps_split(f & f_daemon)) exit(EXIT_FAILURE);
       break;
     }
   }
 
-  a_init();
+  if (a_init()) exit(EXIT_FAILURE);
   a_signals();
-  a_listen(csock, u, g, csockmode);
+  if (a_listen(csock, u, g, csockmode)) exit(EXIT_FAILURE);
   u_setugid(u, g);
-  km_init(kr_priv, kr_pub, tag_priv);
+  if (km_init(kr_priv, kr_pub, tag_priv)) exit(EXIT_FAILURE);
   kx_init();
   if (f & f_daemon) {
     if (daemonize()) {
index fd0f44e..0e336f3 100644 (file)
@@ -598,7 +598,7 @@ typedef struct tunnel_ops {
   const char *name;                    /* Name of this tunnel driver */
   unsigned flags;                      /* Various interesting flags */
 #define TUNF_PRIVOPEN 1u               /*   Need helper to open file */
-  void (*init)(void);                  /* Initializes the system */
+  int (*init)(void);                   /* Initializes the system */
   tunnel *(*create)(struct peer */*p*/, int /*fd*/, char **/*ifn*/);
                                        /* Initializes a new tunnel */
   void (*setifname)(tunnel */*t*/, const char */*ifn*/);
@@ -841,14 +841,14 @@ extern unsigned tr_flags;         /* Trace options flags */
  *             @const char *pubkr@ = public keyring file
  *             @const char *ptag@ = default private-key tag
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Initializes the key-management machinery, loading the
  *             keyrings and so on.
  */
 
-extern void km_init(const char */*privkr*/, const char */*pubkr*/,
-                   const char */*ptag*/);
+extern int km_init(const char */*privkr*/, const char */*pubkr*/,
+                  const char */*ptag*/);
 
 /* --- @km_reload@ --- *
  *
@@ -1354,13 +1354,13 @@ extern void a_daemon(void);
  *             @gid_t g@ = group to own the socket
  *             @mode_t m@ = permissions to set on the socket
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Creates the admin listening socket.
  */
 
-extern void a_listen(const char */*sock*/,
-                    uid_t /*u*/, gid_t /*g*/, mode_t /*m*/);
+extern int a_listen(const char */*sock*/,
+                   uid_t /*u*/, gid_t /*g*/, mode_t /*m*/);
 
 /* --- @a_unlisten@ --- *
  *
@@ -1409,12 +1409,12 @@ extern void a_signals(void);
  *             @gid_t g@ = group to own the socket
  *             @mode_t m@ = permissions to set on the socket
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Creates the admin listening socket.
  */
 
-extern void a_init(void);
+extern int a_init(void);
 
 /*----- Mapping with addresses as keys ------------------------------------*/
 
@@ -1514,13 +1514,13 @@ extern int ps_tunfd(const tunnel_ops */*tops*/, char **/*ifn*/);
  *
  * Arguments:  @int detachp@ = whether to detach the child from its terminal
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Separates off the privileged tunnel-opening service from the
  *             rest of the server.
  */
 
-extern void ps_split(int /*detachp*/);
+extern int ps_split(int /*detachp*/);
 
 /* --- @ps_quit@ --- *
  *
@@ -1707,12 +1707,12 @@ extern const addr *p_addr(peer */*p*/);
  *
  * Arguments:  @struct addrinfo *ailist@ = addresses to bind to
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Binds to the main UDP sockets.
  */
 
-extern void p_bind(struct addrinfo */*ailist*/);
+extern int p_bind(struct addrinfo */*ailist*/);
 
 /* --- @p_unbind@ --- *
  *
@@ -1741,18 +1741,18 @@ extern void p_init(void);
  *
  * Arguments:  @const tunnel_ops *tops@ = tunnel ops to add
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
- * Use:                Adds a tunnel class to the list of known classes.  If there
- *             is no current default tunnel, then this one is made the
- *             default.
+ * Use:                Adds a tunnel class to the list of known classes, if it
+ *             initializes properly.  If there is no current default tunnel,
+ *             then this one is made the default.
  *
  *             Does nothing if the tunnel class is already known.  So adding
  *             a bunch of tunnels takes quadratic time, but there will be
  *             too few to care about.
  */
 
-extern void p_addtun(const tunnel_ops */*tops*/);
+extern int p_addtun(const tunnel_ops */*tops*/);
 
 /* --- @p_setdflttun@ --- *
  *
index d4b35d0..29f42d5 100644 (file)
@@ -179,13 +179,13 @@ static void t_read(int fd, unsigned mode, void *v)
  *
  * Arguments:  ---
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Initializes the tunneling system.  Maybe this will require
  *             opening file descriptors or something.
  */
 
-static void t_init(void)
+static int t_init(void)
 {
   char *p, *q;
   dstr d = DSTR_INIT;
@@ -194,7 +194,7 @@ static void t_init(void)
   size_t n;
 
   if ((p = getenv("TRIPE_SLIPIF")) == 0)
-    return;
+    return (0);
 
   /* --- Build the list of available interfaces --- */
 
@@ -240,10 +240,11 @@ static void t_init(void)
       break;
     p++;
   }
-  return;
+  return (0);
 
 whine:
   a_warn("TUN", "-", "slip", "bad-interface-list", A_END);
+  return (-1);
 }
 
 /* --- @t_broken@ --- *
index 21b2aab..4a9643e 100644 (file)
@@ -74,13 +74,13 @@ static void t_read(int fd, unsigned mode, void *v)
  *
  * Arguments:  ---
  *
- * Returns:    ---
+ * Returns:    Zero on success, @-1@ on failure.
  *
  * Use:                Initializes the tunneling system.  Maybe this will require
  *             opening file descriptors or something.
  */
 
-static void t_init(void) { return; }
+static int t_init(void) { return (0); }
 
 /* --- @t_create@ --- *
  *