ipcpd: Fix request handling at shutdown

The IPCP states were not entirely correct causing some operations to
be serviced during shutdown. This caused some use-after-free in the
pff. States in the IPCP are now correctly set. IRMd states updated to
the same strategy. The IRMd registry tracks if the IPCP was ENROLLED
or BOOTSTRAPPED, the IPCP just goes to OPERATIONAL.

IPCP state diagram::

NULL -> init() -> INIT -> start() -> BOOT ->
     bootstrap/enroll() -> OPERATIONAL -> shutdown()
     -> SHUTDOWN -> stop_components() -> BOOT ->
stop() -> INIT -> fini() -> NULL

Signed-off-by: Dimitri Staessens <dimitri@ouroboros.rocks>

9 files changed, 69 insertions, 57 deletions

diff --git a/src/ipcpd/ipcp.c b/src/ipcpd/ipcp.c
index 7fe3e7eb..a47b6226 100644
--- a/src/ipcpd/ipcp.c
+++ b/src/ipcpd/ipcp.c
@@ -302,7 +302,7 @@ static void * acceptloop(void * o)
         (void) o;
 
         while (ipcp_get_state() != IPCP_SHUTDOWN &&
-               ipcp_get_state() != IPCP_NULL) {
+               ipcp_get_state() != IPCP_INIT) {
                 struct cmd * cmd;
 
                 csockfd = accept(ipcpd.sockfd, 0, 0);
@@ -444,11 +444,11 @@ static void do_bootstrap(ipcp_config_msg_t * conf_msg,
                 return;
         }
 
-        if (ipcp_get_state() != IPCP_INIT) {
+        if (ipcp_get_state() != IPCP_BOOT) {
 
                 log_err("Failed to bootstrap: IPCP in state <%s>, need <%s>.",
                         ipcp_state_str[ipcp_get_state()],
-                        ipcp_state_str[IPCP_INIT]);
+                        ipcp_state_str[IPCP_BOOT]);
                 ret_msg->result = -EIPCPSTATE;
                 return;
         }
@@ -476,14 +476,13 @@ static void do_bootstrap(ipcp_config_msg_t * conf_msg,
         strcpy(ipcpd.layer_name, info->name);
         ipcpd.dir_hash_algo = (enum hash_algo) info->dir_hash_algo;
         ret_msg->layer_info = layer_info_s_to_msg(info);
+        ipcp_set_state(IPCP_OPERATIONAL);
 
         log_info("Finished bootstrapping in %s.", info->name);
         log_info("  type: %s", ipcp_type_str[ipcpd.type]);
         log_info("  hash: %s [%zd bytes]",
                 dir_hash_str[ipcpd.dir_hash_algo],
                 ipcp_dir_hash_len());
-
-        ipcp_set_state(IPCP_OPERATIONAL);
 }
 
 static void do_enroll(const char * dst,
@@ -499,10 +498,10 @@ static void do_enroll(const char * dst,
                 return;
         }
 
-        if (ipcp_get_state() != IPCP_INIT) {
+        if (ipcp_get_state() != IPCP_BOOT) {
                 log_err("Failed to enroll: IPCP in state <%s>, need <%s>.",
                         ipcp_state_str[ipcp_get_state()],
-                        ipcp_state_str[IPCP_INIT]);
+                        ipcp_state_str[IPCP_BOOT]);
                 ret_msg->result = -EIPCPSTATE;
                 return;
         }
@@ -949,7 +948,6 @@ int ipcp_init(int               argc,
 
         log_init(log);
 
-        ipcpd.state = IPCP_NULL;
         ipcpd.type  = type;
 
 #if defined (__linux__)
@@ -1031,6 +1029,9 @@ int ipcp_init(int               argc,
 
         ipcp_set_state(IPCP_INIT);
 
+        log_info("IPCP %s %d initialized.", ipcp_type_str[ipcpd.type],
+                 getpid());
+
         return 0;
 
  fail_tpm_create:
@@ -1075,7 +1076,9 @@ int ipcp_start(void)
         info.pid  = getpid();
         info.type = ipcpd.type;
         strcpy(info.name, ipcpd.name);
-        info.state = IPCP_OPERATIONAL;
+        info.state = IPCP_BOOT;
+
+        ipcp_set_state(IPCP_BOOT);
 
         if (tpm_start(ipcpd.tpm)) {
                 log_err("Failed to start threadpool manager.");
@@ -1087,8 +1090,6 @@ int ipcp_start(void)
                 goto fail_acceptor;
         }
 
-        info.state = IPCP_OPERATIONAL;
-
         if (ipcp_create_r(&info)) {
                 log_err("Failed to notify IRMd we are initialized.");
                 goto fail_create_r;
@@ -1103,8 +1104,7 @@ int ipcp_start(void)
         tpm_stop(ipcpd.tpm);
  fail_tpm_start:
         tpm_destroy(ipcpd.tpm);
-        ipcp_set_state(IPCP_NULL);
-        info.state = IPCP_NULL;
+        ipcp_set_state(IPCP_INIT);
         ipcp_create_r(&info);
         return -1;
 }
@@ -1124,7 +1124,7 @@ void ipcp_sigwait(void)
         sigaddset(&sigset, SIGTERM);
         sigaddset(&sigset, SIGPIPE);
 
-        while(ipcp_get_state() != IPCP_NULL &&
+        while(ipcp_get_state() != IPCP_INIT &&
               ipcp_get_state() != IPCP_SHUTDOWN) {
 #ifdef __APPLE__
                 if (sigwait(&sigset, &sig) < 0) {
@@ -1149,8 +1149,8 @@ void ipcp_sigwait(void)
                         /* FALLTHRU */
                 case SIGQUIT:
                         if (info.si_pid == ipcpd.irmd_pid) {
-                                if (ipcp_get_state() == IPCP_INIT)
-                                        ipcp_set_state(IPCP_NULL);
+                                if (ipcp_get_state() == IPCP_BOOT)
+                                        ipcp_set_state(IPCP_INIT);
 
                                 if (ipcp_get_state() == IPCP_OPERATIONAL)
                                         ipcp_set_state(IPCP_SHUTDOWN);
@@ -1173,6 +1173,8 @@ void ipcp_stop(void)
         pthread_join(ipcpd.acceptor, NULL);
 
         tpm_stop(ipcpd.tpm);
+
+        ipcp_set_state(IPCP_INIT);
 }
 
 void ipcp_fini(void)
@@ -1200,6 +1202,8 @@ void ipcp_fini(void)
         log_info("IPCP %d out.", getpid());
 
         log_fini();
+
+        ipcpd.state = IPCP_NULL;
 }
 
 void ipcp_set_state(enum ipcp_state state)
diff --git a/src/ipcpd/unicast/dir/dht.c b/src/ipcpd/unicast/dir/dht.c
index f7de7bb7..d0523eee 100644
--- a/src/ipcpd/unicast/dir/dht.c
+++ b/src/ipcpd/unicast/dir/dht.c
@@ -39,7 +39,6 @@
 #include <ouroboros/errno.h>
 #include <ouroboros/logs.h>
 #include <ouroboros/list.h>
-#include <ouroboros/notifier.h>
 #include <ouroboros/random.h>
 #include <ouroboros/rib.h>
 #include <ouroboros/time.h>
diff --git a/src/ipcpd/unicast/main.c b/src/ipcpd/unicast/main.c
index c2348242..cd0977d4 100644
--- a/src/ipcpd/unicast/main.c
+++ b/src/ipcpd/unicast/main.c
@@ -172,7 +172,7 @@ static void stop_components(void)
 
         dt_stop();
 
-        ipcp_set_state(IPCP_INIT);
+        ipcp_set_state(IPCP_BOOT);
 }
 
 static int unicast_ipcp_enroll(const char *        dst,
@@ -308,11 +308,12 @@ int main(int    argc,
 
         if (ipcp_get_state() == IPCP_SHUTDOWN) {
                 stop_components();
+                ipcp_stop();
                 finalize_components();
+        } else {
+                ipcp_stop();
         }
 
-        ipcp_stop();
-
         enroll_fini();
 
         connmgr_fini();
diff --git a/src/irmd/ipcp.c b/src/irmd/ipcp.c
index 1e8980fb..fdee6a75 100644
--- a/src/irmd/ipcp.c
+++ b/src/irmd/ipcp.c
@@ -94,11 +94,14 @@ ipcp_msg_t * send_recv_ipcp_msg(pid_t        pid,
                 return NULL;
 
         spath = sock_path(pid, IPCP_SOCK_PATH_PREFIX);
-        if (spath == NULL)
+        if (spath == NULL) {
+                log_err("Failed to get IPCP socket path for pid %d.", pid);
                 return NULL;
+        }
 
         sockfd = client_socket_open(spath);
         if (sockfd < 0) {
+                log_err("Failed to open client socket at %s.", spath);
                 free(spath);
                 return NULL;
         }
diff --git a/src/irmd/main.c b/src/irmd/main.c
index e799666f..6fa6ad02 100644
--- a/src/irmd/main.c
+++ b/src/irmd/main.c
@@ -80,6 +80,7 @@
 
 enum irm_state {
         IRMD_NULL = 0,
+        IRMD_INIT,
         IRMD_RUNNING,
         IRMD_SHUTDOWN
 };
@@ -234,7 +235,7 @@ static pid_t spawn_ipcp(struct ipcp_info * info)
         }
 
         info->pid   = pid;
-        info->state = IPCP_BOOT;
+        info->state = IPCP_INIT;
 
         return 0;
 }
@@ -337,7 +338,7 @@ int bootstrap_ipcp(pid_t                pid,
                 goto fail;
         }
 
-        info.state = IPCP_BOOTSTRAPPED;
+        info.state = IPCP_BOOT;
 
         if (reg_set_layer_for_ipcp(&info, &layer) < 0) {
                 log_err("Failed to set layer info for IPCP.");
@@ -369,6 +370,8 @@ int enroll_ipcp(pid_t        pid,
                 goto fail;
         }
 
+        info.state = IPCP_BOOT;
+
         if (reg_set_layer_for_ipcp(&info, &layer) < 0) {
                 log_err("Failed to set layer info for IPCP.");
                 goto fail;
@@ -2303,6 +2306,9 @@ static int irm_init(void)
 
         gcry_control(GCRYCTL_INITIALIZATION_FINISHED);
 #endif
+
+        irmd_set_state(IRMD_INIT);
+
         return 0;
 
 #ifdef HAVE_LIBGCRYPT
@@ -2343,7 +2349,7 @@ static void irm_fini(void)
         struct timespec wait = TIMESPEC_INIT_MS(1);
         int    retries = 5;
 #endif
-        if (irmd_get_state() != IRMD_NULL)
+        if (irmd_get_state() != IRMD_INIT)
                 log_warn("Unsafe destroy.");
 
         pthread_mutex_lock(&irmd.auth.mtx);
@@ -2394,6 +2400,8 @@ static void irm_fini(void)
                 log_err("Failed to remove " FUSE_PREFIX);
 #endif
         assert(list_is_empty(&irmd.cmds));
+
+        irmd.state = IRMD_NULL;
 }
 
 static void usage(void)
@@ -2409,11 +2417,11 @@ static void usage(void)
 
 static int irm_start(void)
 {
+        irmd_set_state(IRMD_RUNNING);
+
         if (tpm_start(irmd.tpm))
                 goto fail_tpm_start;
 
-        irmd_set_state(IRMD_RUNNING);
-
         if (pthread_create(&irmd.irm_sanitize, NULL, irm_sanitize, NULL))
                 goto fail_irm_sanitize;
 
@@ -2428,9 +2436,9 @@ static int irm_start(void)
         pthread_cancel(irmd.irm_sanitize);
         pthread_join(irmd.irm_sanitize, NULL);
  fail_irm_sanitize:
-        irmd_set_state(IRMD_NULL);
         tpm_stop(irmd.tpm);
  fail_tpm_start:
+        irmd_set_state(IRMD_INIT);
         return -1;
 }
 
@@ -2471,7 +2479,7 @@ static void irm_stop(void)
 
         tpm_stop(irmd.tpm);
 
-        irmd_set_state(IRMD_NULL);
+        irmd_set_state(IRMD_INIT);
 }
 
 static void irm_argparse(int     argc,
diff --git a/src/irmd/reg/ipcp.c b/src/irmd/reg/ipcp.c
index 6580cb5b..474527a4 100644
--- a/src/irmd/reg/ipcp.c
+++ b/src/irmd/reg/ipcp.c
@@ -40,7 +40,7 @@ struct reg_ipcp * reg_ipcp_create(const struct ipcp_info * info)
         struct reg_ipcp * ipcp;
 
         assert(info != NULL);
-        assert(info->state == IPCP_BOOT);
+        assert(info->state == IPCP_INIT);
 
         ipcp = malloc(sizeof(*ipcp));
         if (ipcp == NULL) {
@@ -54,7 +54,7 @@ struct reg_ipcp * reg_ipcp_create(const struct ipcp_info * info)
         list_head_init(&ipcp->next);
 
         ipcp->info = *info;
-        ipcp->info.state = IPCP_BOOT;
+        ipcp->info.state = IPCP_INIT;
 
         strcpy(ipcp->layer.name, "Not enrolled.");
 
@@ -77,7 +77,7 @@ void reg_ipcp_update(struct reg_ipcp *        ipcp,
                      const struct ipcp_info * info)
 {
         assert(ipcp != NULL);
-        assert(info->state != IPCP_INIT);
+        assert(info->state != IPCP_NULL);
 
         ipcp->info = *info;
 }
@@ -86,7 +86,7 @@ void reg_ipcp_set_layer(struct reg_ipcp *         ipcp,
                         const struct layer_info * info)
 {
         assert(ipcp != NULL);
-        assert(ipcp->info.state == IPCP_OPERATIONAL);
+        assert(ipcp->info.state == IPCP_BOOT);
 
         ipcp->layer = *info;
 }
diff --git a/src/irmd/reg/reg.c b/src/irmd/reg/reg.c
index 0d4c6f24..91385260 100644
--- a/src/irmd/reg/reg.c
+++ b/src/irmd/reg/reg.c
@@ -732,7 +732,7 @@ int reg_create_ipcp(const struct ipcp_info * info)
 
         assert(info != NULL);
         assert(info->pid != 0);
-        assert(info->state == IPCP_BOOT);
+        assert(info->state == IPCP_INIT);
 
         pthread_mutex_lock(&reg.mtx);
 
@@ -1582,8 +1582,7 @@ int reg_set_layer_for_ipcp(struct ipcp_info *        info,
         struct reg_ipcp * ipcp;
 
         assert(info != NULL);
-        assert(info->state > IPCP_BOOT);
-        assert(info->state < IPCP_SHUTDOWN);
+        assert(info->state == IPCP_BOOT);
 
         pthread_mutex_lock(&reg.mtx);
 
@@ -2060,7 +2059,7 @@ int reg_wait_ipcp_boot(struct ipcp_info *      info,
         int               ret;
         bool              stop = false;
 
-        assert(info->state == IPCP_BOOT);
+        assert(info->state == IPCP_INIT);
 
         pthread_mutex_lock(&reg.mtx);
 
@@ -2080,16 +2079,18 @@ int reg_wait_ipcp_boot(struct ipcp_info *      info,
                         ret = -1;
                         stop = true;
                         break;
+                case IPCP_BOOT:
+                        /* FALLTHRU*/
                 case IPCP_OPERATIONAL:
                         ret  = 0;
                         stop = true;
                         break;
-                case IPCP_BOOT:
+                case IPCP_INIT:
                         ret = -__timedwait(&reg.cond, &reg.mtx, abstime);
                         break;
                 default:
                         assert(false);
-                        continue; /* Shut up static analyzer. */
+                        break; /* Shut up static analyzer. */
                 }
 
                 ipcp = __reg_get_ipcp(info->pid);
diff --git a/src/irmd/reg/tests/ipcp_test.c b/src/irmd/reg/tests/ipcp_test.c
index fb8ba71b..d7d8e524 100644
--- a/src/irmd/reg/tests/ipcp_test.c
+++ b/src/irmd/reg/tests/ipcp_test.c
@@ -31,7 +31,7 @@ static int test_reg_ipcp_create(void)
         struct reg_ipcp *  ipcp;
         struct ipcp_info   info = {
                 .pid   = TEST_PID,
-                .state = IPCP_BOOT
+                .state = IPCP_INIT
         };
         struct layer_info layer = {
                 .name = "testlayer",
@@ -51,7 +51,7 @@ static int test_reg_ipcp_create(void)
                 goto fail;
         }
 
-        ipcp->info.state = IPCP_OPERATIONAL;
+        ipcp->info.state = IPCP_BOOT;
 
         reg_ipcp_set_layer(ipcp, &layer);
 
@@ -60,11 +60,6 @@ static int test_reg_ipcp_create(void)
                 goto fail;
         }
 
-        if (ipcp->info.state != IPCP_OPERATIONAL) {
-                printf("IPCP state was not set.\n");
-                goto fail;
-        }
-
         reg_ipcp_destroy(ipcp);
 
         TEST_SUCCESS();
diff --git a/src/irmd/reg/tests/reg_test.c b/src/irmd/reg/tests/reg_test.c
index 280ea9a8..e51fdfc0 100644
--- a/src/irmd/reg/tests/reg_test.c
+++ b/src/irmd/reg/tests/reg_test.c
@@ -482,7 +482,7 @@ static int test_reg_create_ipcp(void)
         struct ipcp_info info = {
                 .name  = TEST_IPCP,
                 .pid   = TEST_PID,
-                .state = IPCP_BOOT /* set by spawn_ipcp */
+                .state = IPCP_INIT /* set by spawn_ipcp */
         };
 
         TEST_START();
@@ -543,7 +543,7 @@ static int test_reg_list_ipcps(void)
         for (i = 0; i < 10; i++) {
                 struct ipcp_info info = {
                         .pid   = TEST_PID + i,
-                        .state = IPCP_BOOT /* set by spawn_ipcp */
+                        .state = IPCP_INIT /* set by spawn_ipcp */
                 };
 
                 sprintf(info.name, "%s%d", TEST_IPCP, i);
@@ -601,7 +601,7 @@ static int test_insert_ipcps(void)
                 sprintf(info.name, "%s-%ld", TEST_IPCP, i);
                 info.pid   = TEST_PID + rand() % 10000;
                 info.type  = rand() % IPCP_INVALID;
-                info.state = IPCP_BOOT; /* set by spawn_ipcp */
+                info.state = IPCP_INIT; /* set by spawn_ipcp */
 
                 if (reg_create_ipcp(&info) < 0) {
                         printf("Failed to create ipcp %s.\n", info.name);
@@ -653,7 +653,7 @@ static int test_set_layer(void)
         struct ipcp_info info = {
                 .name  = TEST_IPCP,
                 .pid   = TEST_PID,
-                .state = IPCP_BOOT /* set by spawn_ipcp */
+                .state = IPCP_INIT /* set by spawn_ipcp */
         };
         struct layer_info layer = {
                 .name = TEST_LAYER,
@@ -677,8 +677,9 @@ static int test_set_layer(void)
         }
 
         ipcp = __reg_get_ipcp(info.pid);
-        ipcp->info.state = IPCP_OPERATIONAL;
-        info.state = IPCP_ENROLLED;
+
+        ipcp->info.state = IPCP_BOOT;
+        info.state = IPCP_BOOT;
 
         reg_set_layer_for_ipcp(&info, &layer);
 
@@ -1407,7 +1408,7 @@ static int test_wait_ipcp_boot_timeout(void)
         struct ipcp_info info = {
                 .name  = TEST_IPCP,
                 .pid   = TEST_PID,
-                .state = IPCP_BOOT  /* set by spawn_ipcp */
+                .state = IPCP_INIT  /* set by spawn_ipcp */
         };
 
         TEST_START();
@@ -1462,12 +1463,12 @@ static int test_wait_ipcp_boot_fail(void)
         struct ipcp_info info = {
                 .name  = TEST_IPCP,
                 .pid   = TEST_PID,
-                .state = IPCP_BOOT /* set by spawn_ipcp */
+                .state = IPCP_INIT /* set by spawn_ipcp */
         };
         struct ipcp_info resp_info = {
                 .name  = TEST_IPCP,
                 .pid   = TEST_PID,
-                .state = IPCP_NULL
+                .state = IPCP_INIT
         };
 
         TEST_START();
@@ -1487,7 +1488,7 @@ static int test_wait_ipcp_boot_fail(void)
         clock_gettime(PTHREAD_COND_CLOCK, &abstime);
         ts_add(&abstime, &timeo, &abstime);
 
-        info.state = IPCP_BOOT;
+        info.state = IPCP_INIT;
 
         if (reg_wait_ipcp_boot(&info, &abstime) == 0) {
                 printf("IPCP boot reported success.\n");
@@ -1524,7 +1525,7 @@ static int test_wait_ipcp_boot_success(void)
         struct ipcp_info info = {
                 .name  = TEST_IPCP,
                 .pid   = TEST_PID,
-                .state = IPCP_BOOT /* set by spawn_ipcp */
+                .state = IPCP_INIT /* set by spawn_ipcp */
         };
         struct ipcp_info resp_info = {
                 .name  = TEST_IPCP,
@@ -1549,7 +1550,7 @@ static int test_wait_ipcp_boot_success(void)
         clock_gettime(PTHREAD_COND_CLOCK, &abstime);
         ts_add(&abstime, &timeo, &abstime);
 
-        info.state = IPCP_BOOT;
+        info.state = IPCP_INIT;
 
         if (reg_wait_ipcp_boot(&info, &abstime) < 0) {
                 printf("IPCP boot failed.\n");