Clean up shell command handling

5 files changed, 130 insertions, 117 deletions

diff --git a/src/fastd.c b/src/fastd.c
index c6fec8b..9761715 100644
--- a/src/fastd.c
+++ b/src/fastd.c
@@ -270,19 +270,19 @@ void fastd_handle_receive(fastd_context_t *ctx, fastd_peer_t *peer, fastd_buffer
 }
 
 static inline void on_pre_up(fastd_context_t *ctx) {
-	fastd_shell_command_exec(ctx, &ctx->conf->on_pre_up, NULL, NULL, NULL, NULL);
+	fastd_shell_command_exec(ctx, &ctx->conf->on_pre_up, NULL, NULL, NULL);
 }
 
 static inline void on_up(fastd_context_t *ctx) {
-	fastd_shell_command_exec(ctx, &ctx->conf->on_up, NULL, NULL, NULL, NULL);
+	fastd_shell_command_exec(ctx, &ctx->conf->on_up, NULL, NULL, NULL);
 }
 
 static inline void on_down(fastd_context_t *ctx) {
-	fastd_shell_command_exec(ctx, &ctx->conf->on_down, NULL, NULL, NULL, NULL);
+	fastd_shell_command_exec(ctx, &ctx->conf->on_down, NULL, NULL, NULL);
 }
 
 static inline void on_post_down(fastd_context_t *ctx) {
-	fastd_shell_command_exec(ctx, &ctx->conf->on_post_down, NULL, NULL, NULL, NULL);
+	fastd_shell_command_exec(ctx, &ctx->conf->on_post_down, NULL, NULL, NULL);
 }
 
 static fastd_peer_group_t* init_peer_group(const fastd_peer_group_config_t *config, fastd_peer_group_t *parent) {
@@ -699,9 +699,6 @@ static void write_pid(fastd_context_t *ctx, pid_t pid) {
 }
 
 static void set_user(fastd_context_t *ctx) {
-	if (chdir("/"))
-		pr_error(ctx, "can't chdir to `/': %s", strerror(errno));
-
 	if (ctx->conf->user || ctx->conf->group) {
 		if (setgid(ctx->conf->gid) < 0)
 			exit_errno(ctx, "setgid");
@@ -899,6 +896,9 @@ int main(int argc, char *argv[]) {
 	if (conf.daemon)
 		status_fd = daemonize(&ctx);
 
+	if (chdir("/"))
+		pr_error(&ctx, "can't chdir to `/': %s", strerror(errno));
+
 	init_log(&ctx);
 
 #ifdef HAVE_LIBSODIUM
diff --git a/src/peer.c b/src/peer.c
index 9aeb81e..42268b7 100644
--- a/src/peer.c
+++ b/src/peer.c
@@ -31,11 +31,11 @@
 
 
 static inline void on_establish(fastd_context_t *ctx, const fastd_peer_t *peer) {
-	fastd_shell_command_exec(ctx, &ctx->conf->on_establish, peer, &peer->local_address, &peer->address, NULL);
+	fastd_shell_command_exec(ctx, &ctx->conf->on_establish, peer, &peer->local_address, &peer->address);
 }
 
 static inline void on_disestablish(fastd_context_t *ctx, const fastd_peer_t *peer) {
-	fastd_shell_command_exec(ctx, &ctx->conf->on_disestablish, peer, &peer->local_address, &peer->address, NULL);
+	fastd_shell_command_exec(ctx, &ctx->conf->on_disestablish, peer, &peer->local_address, &peer->address);
 }
 
 static inline void free_socket(fastd_context_t *ctx, fastd_peer_t *peer) {
@@ -574,11 +574,10 @@ bool fastd_peer_verify_temporary(fastd_context_t *ctx, fastd_peer_t *peer, const
 	if (!fastd_shell_command_isset(&ctx->conf->on_verify))
 		exit_bug(ctx, "tried to verify temporary peer without on-verify command");
 
-	fastd_shell_command_t cmd = ctx->conf->on_verify;
-	cmd.sync = true; /* TODO: async not supported yet */
+	/* TODO: async not supported yet */
 
 	int ret;
-	if (!fastd_shell_command_exec(ctx, &cmd, peer, local_addr, peer_addr, &ret))
+	if (!fastd_shell_command_exec_sync(ctx, &ctx->conf->on_verify, peer, local_addr, peer_addr, &ret))
 		return false;
 
 	if (WIFSIGNALED(ret)) {
diff --git a/src/protocols/ec25519_fhmqvc/handshake.c b/src/protocols/ec25519_fhmqvc/handshake.c
index 317501b..9339475 100644
--- a/src/protocols/ec25519_fhmqvc/handshake.c
+++ b/src/protocols/ec25519_fhmqvc/handshake.c
@@ -528,7 +528,7 @@ void fastd_protocol_ec25519_fhmqvc_handshake_init(fastd_context_t *ctx, const fa
 	fastd_handshake_add(ctx, &buffer, RECORD_SENDER_HANDSHAKE_KEY, PUBLICKEYBYTES, &ctx->protocol_state->handshake_key.key.public);
 
 	if (!peer || !fastd_peer_is_established(peer))
-		fastd_shell_command_exec(ctx, &ctx->conf->on_connect, peer, (local_addr && local_addr->sa.sa_family) ? local_addr : sock->bound_addr, remote_addr, NULL);
+		fastd_shell_command_exec(ctx, &ctx->conf->on_connect, peer, (local_addr && local_addr->sa.sa_family) ? local_addr : sock->bound_addr, remote_addr);
 
 	fastd_send_handshake(ctx, sock, local_addr, remote_addr, peer, buffer);
 }
diff --git a/src/shell.c b/src/shell.c
index e1161d4..0b9bef5 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -32,145 +32,158 @@
 #include <sys/wait.h>
 
 
-bool fastd_shell_command_exec(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr, int *ret) {
-	if (!fastd_shell_command_isset(command))
-		return true;
+static void shell_command_setenv(fastd_context_t *ctx, pid_t pid, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr) {
+	/* both INET6_ADDRSTRLEN and IFNAMESIZE already include space for the zero termination, so there is no need to add space for the '%' here. */
+	char buf[INET6_ADDRSTRLEN+IF_NAMESIZE];
 
-	bool ok = false;
-	char *cwd = get_current_dir_name();
+	snprintf(buf, sizeof(buf), "%u", (unsigned)pid);
+	setenv("FASTD_PID", buf, 1);
 
+	if (ctx->ifname) {
+		setenv("INTERFACE", ctx->ifname, 1);
+	}
+	else if (ctx->conf->ifname) {
+		char ifname[IF_NAMESIZE];
 
-	if (!chdir(command->dir)) {
-		/* both INET6_ADDRSTRLEN and IFNAMESIZE already include space for the zero termination, so there is no need to add space for the '%' here. */
-		char buf[INET6_ADDRSTRLEN+IF_NAMESIZE];
+		strncpy(ifname, ctx->conf->ifname, sizeof(ifname)-1);
+		ifname[sizeof(ifname)-1] = 0;
 
-		snprintf(buf, sizeof(buf), "%u", (unsigned)getpid());
-		setenv("FASTD_PID", buf, 1);
+		setenv("INTERFACE", ifname, 1);
+	}
+	else {
+		unsetenv("INTERFACE");
+	}
 
-		if (ctx->ifname) {
-			setenv("INTERFACE", ctx->ifname, 1);
-		}
-		else if (ctx->conf->ifname) {
-			char ifname[IF_NAMESIZE];
+	snprintf(buf, sizeof(buf), "%u", ctx->conf->mtu);
+	setenv("INTERFACE_MTU", buf, 1);
 
-			strncpy(ifname, ctx->conf->ifname, sizeof(ifname)-1);
-			ifname[sizeof(ifname)-1] = 0;
+	if (peer && peer->config && peer->config->name)
+		setenv("PEER_NAME", peer->config->name, 1);
+	else
+		unsetenv("PEER_NAME");
 
-			setenv("INTERFACE", ifname, 1);
-		}
-		else {
-			unsetenv("INTERFACE");
-		}
+	switch(local_addr ? local_addr->sa.sa_family : AF_UNSPEC) {
+	case AF_INET:
+		inet_ntop(AF_INET, &local_addr->in.sin_addr, buf, sizeof(buf));
+		setenv("LOCAL_ADDRESS", buf, 1);
+
+		snprintf(buf, sizeof(buf), "%u", ntohs(local_addr->in.sin_port));
+		setenv("LOCAL_PORT", buf, 1);
 
-		snprintf(buf, sizeof(buf), "%u", ctx->conf->mtu);
-		setenv("INTERFACE_MTU", buf, 1);
+		break;
 
-		if (peer && peer->config && peer->config->name)
-			setenv("PEER_NAME", peer->config->name, 1);
-		else
-			unsetenv("PEER_NAME");
+	case AF_INET6:
+		inet_ntop(AF_INET6, &local_addr->in6.sin6_addr, buf, sizeof(buf));
 
-		switch(local_addr ? local_addr->sa.sa_family : AF_UNSPEC) {
-		case AF_INET:
-			inet_ntop(AF_INET, &local_addr->in.sin_addr, buf, sizeof(buf));
-			setenv("LOCAL_ADDRESS", buf, 1);
+		if (IN6_IS_ADDR_LINKLOCAL(&local_addr->in6.sin6_addr)) {
+			if (if_indextoname(local_addr->in6.sin6_scope_id, buf+strlen(buf)+1))
+				buf[strlen(buf)] = '%';
+		}
 
-			snprintf(buf, sizeof(buf), "%u", ntohs(local_addr->in.sin_port));
-			setenv("LOCAL_PORT", buf, 1);
+		setenv("LOCAL_ADDRESS", buf, 1);
 
-			break;
+		snprintf(buf, sizeof(buf), "%u", ntohs(local_addr->in6.sin6_port));
+		setenv("LOCAL_PORT", buf, 1);
 
-		case AF_INET6:
-			inet_ntop(AF_INET6, &local_addr->in6.sin6_addr, buf, sizeof(buf));
+		break;
 
-			if (IN6_IS_ADDR_LINKLOCAL(&local_addr->in6.sin6_addr)) {
-				if (if_indextoname(local_addr->in6.sin6_scope_id, buf+strlen(buf)+1))
-					buf[strlen(buf)] = '%';
-			}
+	default:
+		unsetenv("LOCAL_ADDRESS");
+		unsetenv("LOCAL_PORT");
+	}
 
-			setenv("LOCAL_ADDRESS", buf, 1);
+	switch(peer_addr ? peer_addr->sa.sa_family : AF_UNSPEC) {
+	case AF_INET:
+		inet_ntop(AF_INET, &peer_addr->in.sin_addr, buf, sizeof(buf));
+		setenv("PEER_ADDRESS", buf, 1);
 
-			snprintf(buf, sizeof(buf), "%u", ntohs(local_addr->in6.sin6_port));
-			setenv("LOCAL_PORT", buf, 1);
+		snprintf(buf, sizeof(buf), "%u", ntohs(peer_addr->in.sin_port));
+		setenv("PEER_PORT", buf, 1);
 
-			break;
+		break;
 
-		default:
-			unsetenv("LOCAL_ADDRESS");
-			unsetenv("LOCAL_PORT");
+	case AF_INET6:
+		inet_ntop(AF_INET6, &peer_addr->in6.sin6_addr, buf, sizeof(buf));
+
+		if (IN6_IS_ADDR_LINKLOCAL(&peer_addr->in6.sin6_addr)) {
+			if (if_indextoname(peer_addr->in6.sin6_scope_id, buf+strlen(buf)+1))
+				buf[strlen(buf)] = '%';
 		}
 
-		switch(peer_addr ? peer_addr->sa.sa_family : AF_UNSPEC) {
-		case AF_INET:
-			inet_ntop(AF_INET, &peer_addr->in.sin_addr, buf, sizeof(buf));
-			setenv("PEER_ADDRESS", buf, 1);
+		setenv("PEER_ADDRESS", buf, 1);
+
+		snprintf(buf, sizeof(buf), "%u", ntohs(peer_addr->in6.sin6_port));
+		setenv("PEER_PORT", buf, 1);
 
-			snprintf(buf, sizeof(buf), "%u", ntohs(peer_addr->in.sin_port));
-			setenv("PEER_PORT", buf, 1);
+		break;
 
-			break;
+	default:
+		unsetenv("PEER_ADDRESS");
+		unsetenv("PEER_PORT");
+	}
 
-		case AF_INET6:
-			inet_ntop(AF_INET6, &peer_addr->in6.sin6_addr, buf, sizeof(buf));
+	ctx->conf->protocol->set_shell_env(ctx, peer);
+}
 
-			if (IN6_IS_ADDR_LINKLOCAL(&peer_addr->in6.sin6_addr)) {
-				if (if_indextoname(peer_addr->in6.sin6_scope_id, buf+strlen(buf)+1))
-					buf[strlen(buf)] = '%';
-			}
+static bool shell_command_do_exec(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr, pid_t *pid_ret) {
+	pid_t parent = getpid();
 
-			setenv("PEER_ADDRESS", buf, 1);
+	pid_t pid = fork();
+	if (pid < 0) {
+		pr_error_errno(ctx, "shell_command_do_exec: fork");
+		return false;
+	}
+	else if (pid > 0) {
+		if (pid_ret)
+			*pid_ret = pid;
 
-			snprintf(buf, sizeof(buf), "%u", ntohs(peer_addr->in6.sin6_port));
-			setenv("PEER_PORT", buf, 1);
+		return true;
+	}
 
-			break;
+	/* child process */
+	if (chdir(command->dir))
+		_exit(126);
 
-		default:
-			unsetenv("PEER_ADDRESS");
-			unsetenv("PEER_PORT");
-		}
+	shell_command_setenv(ctx, parent, peer, local_addr, peer_addr);
 
-		ctx->conf->protocol->set_shell_env(ctx, peer);
+	execl("/bin/sh", "sh", "-c", command->command, (char*)NULL);
+	_exit(127);
+}
 
-		if (command->sync) {
-			int result = system(command->command);
+bool fastd_shell_command_exec_sync(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr, int *ret) {
+	if (!fastd_shell_command_isset(command))
+		return true;
 
-			if (ret) {
-				*ret = result;
-				ok = true;
-			}
-			else {
-				if (WIFSIGNALED(result))
-					pr_error(ctx, "command exited with signal %i", WTERMSIG(result));
-				else if (WEXITSTATUS(result))
-					pr_warn(ctx, "command exited with status %i", WEXITSTATUS(result));
-				else
-					ok = true;
-			}
+	pid_t pid;
+	if (!shell_command_do_exec(ctx, command, peer, local_addr, peer_addr, &pid))
+		return false;
 
-		}
-		else {
-			pid_t pid = fork();
-			if (pid == 0) {
-				execl("/bin/sh", "sh", "-c", command->command, (char*)NULL);
-				_exit(127);
-			}
-			else if (pid > 0) {
-				ok = true;
-			}
-			else {
-				pr_error_errno(ctx, "fastd_shell_exec_command: fork");
-			}
-		}
+	int status;
+	if (waitpid(pid, &status, 0) <= 0) {
+		pr_error_errno(ctx, "fastd_shell_command_exec_sync: waitpid");
+		return false;
+	}
 
-		if(chdir(cwd))
-			pr_error(ctx, "can't chdir to `%s': %s", cwd, strerror(errno));
+	if (ret) {
+		*ret = status;
 	}
 	else {
-		pr_error(ctx, "can't chdir to `%s': %s", command->dir, strerror(errno));
+		if (WIFSIGNALED(status))
+			pr_warn(ctx, "command exited with signal %i", WTERMSIG(status));
+		else if (WEXITSTATUS(status))
+			pr_warn(ctx, "command exited with status %i", WEXITSTATUS(status));
 	}
 
-	free(cwd);
+	return true;
+}
+
+
+void fastd_shell_command_exec(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr) {
+	if (!fastd_shell_command_isset(command))
+		return;
 
-	return ok;
+	if (command->sync)
+		fastd_shell_command_exec_sync(ctx, command, peer, local_addr, peer_addr, NULL);
+	else
+		shell_command_do_exec(ctx, command, peer, local_addr, peer_addr, NULL);
 }
diff --git a/src/shell.h b/src/shell.h
index e6c3618..21a451b 100644
--- a/src/shell.h
+++ b/src/shell.h
@@ -58,4 +58,5 @@ static inline bool fastd_shell_command_isset(const fastd_shell_command_t *comman
 	return command->command;
 }
 
-bool fastd_shell_command_exec(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr, int *ret);
+bool fastd_shell_command_exec_sync(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr, int *ret);
+void fastd_shell_command_exec(fastd_context_t *ctx, const fastd_shell_command_t *command, const fastd_peer_t *peer, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *peer_addr);