From 1d5b85b0d6c5ac1f10ab26fbe372b58434f96fd6 Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Sun, 3 Oct 2021 17:19:33 +0200 Subject: [PATCH 1/7] Improve error handling when the called binary doesn't exist --- src/subprocess.c | 69 ++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/src/subprocess.c b/src/subprocess.c index b9fea33..28421a6 100644 --- a/src/subprocess.c +++ b/src/subprocess.c @@ -63,6 +63,8 @@ static int create_pipes(io_pipes_t * pipes) { * @return 0 on success, -1 on error. */ static int setup_pipes_for_child(io_pipes_t const * pipes) { + int ret = 0; + if (dup2(pipes->child_in, STDIN_FILENO) < 0) { perror("Redirecting stdin"); goto exit_0; @@ -78,19 +80,27 @@ static int setup_pipes_for_child(io_pipes_t const * pipes) { goto exit_0; } - if (close(pipes->parent_out) != 0) + if (close(pipes->parent_out) != 0) { perror("Closing pipe in child"); + ret = -1; + } - if (close(pipes->child_in) != 0) + if (close(pipes->child_in) != 0) { perror("Closing pipe in child"); + ret = -1; + } - if (close(pipes->child_out) != 0) + if (close(pipes->child_out) != 0) { perror("Closing pipe in child"); + ret = -1; + } - if (close(pipes->parent_in) != 0) + if (close(pipes->parent_in) != 0) { perror("Closing pipe in child"); + ret = -1; + } - return 0; + return ret; exit_0: return -1; @@ -105,11 +115,17 @@ static int setup_pipes_for_child(io_pipes_t const * pipes) { * @return 0 on success, -1 on error. */ static int setup_pipes_for_parent(io_pipes_t const * pipes) { - if (close(pipes->child_in) != 0) + int ret = 0; + + if (close(pipes->child_in) != 0) { perror("Closing pipe in parent"); - if (close(pipes->child_out) != 0) + ret = -1; + } + if (close(pipes->child_out) != 0) { perror("Closing pipe in parent"); - return 0; + ret = -1; + } + return ret; } @@ -217,9 +233,10 @@ int run( ssize_t out_size_ = 0; int status; + int ret = 0; if (create_pipes(&pipes) != 0) - goto exit_0; + return -1; child_pid = fork(); @@ -237,37 +254,40 @@ int run( exit(255); } - // execve() is mistyped for backward compatibility + // execve() is mistyped for backward compatibility, + // so need to const-cast here. execve(filename, (char * const *)argv, (char * const *)env); perror("Executing command"); exit(255); } else if (child_pid > 0) { // parent - if (setup_pipes_for_parent(&pipes) != 0) + if (setup_pipes_for_parent(&pipes) != 0) { goto exit_0; + } if (in_buf != NULL) { if (write_all(pipes.parent_out, in_buf, in_size) != 0) { perror("Writing to stdin in parent"); - goto exit_2; + ret = -1; } } if (close(pipes.parent_out) != 0) { perror("Closing stdin pipe in parent"); - goto exit_1; + ret = -1; }; if (out_buf != NULL) { if (read_all(pipes.parent_in, &out_buf_, &out_size_) != 0) { - goto exit_1; + fprintf(stderr, "Error reading from stdout/stderr"); + ret = -1; } } if (close(pipes.parent_in) != 0) { perror("Closing stdout/err pipe in parent"); - goto exit_0; + ret = -1; } waitpid(child_pid, &status, 0); @@ -279,23 +299,16 @@ int run( *out_buf = out_buf_; *out_size = out_size_; - return 0; - } - else { - // fork error - close(pipes.parent_out); - close(pipes.child_in); - close(pipes.child_out); - close(pipes.parent_in); + return ret; + } + // fork error if we get here, try to clean up -exit_2: +exit_0: close(pipes.parent_out); - -exit_1: + close(pipes.child_in); + close(pipes.child_out); close(pipes.parent_in); - -exit_0: return -1; } From b7f398063091791b4ac8f224b911f88ed9a8f78f Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Mon, 4 Oct 2021 14:17:13 +0200 Subject: [PATCH 2/7] Add Docker image --- Dockerfile | 27 +++++++++++++++++++++++++++ Makefile | 11 ++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 Dockerfile diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..cd83a28 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,27 @@ +FROM ubuntu:20.04 + +RUN \ + apt-get update && \ + apt-get install -y gcc make iproute2 wireguard-tools libcap2 libcap2-bin libcap-dev && \ + apt-get clean && \ + rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* + +RUN \ + mkdir /usr/local/src/net-admin-helper && \ + mkdir /usr/local/src/net-admin-helper/bin + +COPY Makefile /usr/local/src/net-admin-helper/Makefile +COPY config.h /usr/local/src/net-admin-helper/config.h +COPY src /usr/local/src/net-admin-helper/src + +RUN \ + cd /usr/local/src/net-admin-helper && \ + make && make setcap && \ + mv /usr/local/src/net-admin-helper/bin/net-admin-helper /usr/local/bin/net-admin-helper + +RUN rm -r /usr/local/src/net-admin-helper + +RUN apt-get remove -y gcc make libcap-dev && apt-get -y autoremove + +USER nobody + diff --git a/Makefile b/Makefile index 0f60d1e..e50349c 100644 --- a/Makefile +++ b/Makefile @@ -21,9 +21,18 @@ setcap: bin/net-admin-helper setcap 'cap_net_admin,cap_sys_ptrace,cap_sys_admin=p' bin/net-admin-helper +export DOCKER_BUILDKIT = 1 + +.PHONY: docker +docker: + docker build . -t net-admin-helper:latest + docker save net-admin-helper:latest | gzip -1 -c >bin/net-admin-helper.tar.gz + + .PHONY: clean clean: - rm -f bin/* + -rm -f bin/* + -docker rmi net-admin-helper:latest bin/main.o: config.h src/container_wireguard.h From b4ee3b2e215c8b9685b61d051381753768d9d245 Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Mon, 4 Oct 2021 14:40:07 +0200 Subject: [PATCH 3/7] Do not zip the Docker image, so it can be loaded directly --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e50349c..f31711f 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ export DOCKER_BUILDKIT = 1 .PHONY: docker docker: docker build . -t net-admin-helper:latest - docker save net-admin-helper:latest | gzip -1 -c >bin/net-admin-helper.tar.gz + docker save net-admin-helper:latest >bin/net-admin-helper.tar .PHONY: clean From 2be2b187880ee1437a686557aeacb295d80c65c6 Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Tue, 5 Oct 2021 11:03:01 +0200 Subject: [PATCH 4/7] Update configuration example --- config.h.example | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/config.h.example b/config.h.example index 79117de..381ff02 100644 --- a/config.h.example +++ b/config.h.example @@ -11,11 +11,12 @@ #define WG "/usr/bin/wg" -/** Enable tasks. - * - * This selects tasks to enable. To enable a task, remove the `// ` at the start - * of its line. - */ +/** Settings for container WireGuard */ + +// Device name prefix, use e.g. your application name +#define CWG_PREFIX "cwg" + +// To enable a task, remove the `// ` at the start of its line. // #define ENABLE_CWG_CREATE // #define ENABLE_CWG_CONNECT // #define ENABLE_CWG_DESTROY From 9efa43ba2bbf47a64afe030d02394318097b76c6 Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Tue, 5 Oct 2021 11:07:44 +0200 Subject: [PATCH 5/7] Add missing IPC_LOCK capability for mlockall() --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f31711f..8e576a2 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,7 @@ setcap: bin/net-admin-helper chown root:root bin/net-admin-helper chmod 755 bin/net-admin-helper # Give it the needed capabilities - setcap 'cap_net_admin,cap_sys_ptrace,cap_sys_admin=p' bin/net-admin-helper + setcap 'cap_net_admin,cap_sys_ptrace,cap_sys_admin,cap_ipc_lock=p' bin/net-admin-helper export DOCKER_BUILDKIT = 1 From cde409010d87f7bde2326f9364ac733ceedd866d Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Tue, 5 Oct 2021 12:10:38 +0200 Subject: [PATCH 6/7] Refactor capability enabling and disabling --- src/capabilities.c | 79 +++------------------------------------ src/capabilities.h | 23 +++--------- src/container_wireguard.c | 9 +++-- 3 files changed, 17 insertions(+), 94 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 3df9afc..6cecafc 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -65,10 +65,9 @@ int set_ambient_capabilities() { } -/** Enable CAP_SYS_PTRACE so we can access a container network namespace. */ -int enable_cap_sys_ptrace() { +/** Enable a specific capability. */ +int enable_cap(cap_value_t cap) { cap_t caps; - cap_value_t cap_list[] = {CAP_SYS_PTRACE}; caps = cap_get_proc(); if (caps == NULL) { @@ -76,73 +75,7 @@ int enable_cap_sys_ptrace() { goto exit_0; } - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET) != 0) { - perror("Error setting effective capabilities"); - goto exit_0; - } - - if (cap_set_proc(caps) != 0) { - perror("Error setting capabilities"); - goto exit_0; - } - - if (cap_free(caps) != 0) { - perror("Error freeing capabilities"); - goto exit_0; - } - - return 0; - -exit_0: - return -1; -} - - -/** Disable CAP_SYS_PTRACE. */ -int disable_cap_sys_ptrace() { - cap_t caps; - cap_value_t cap_list[] = {CAP_SYS_PTRACE}; - - caps = cap_get_proc(); - if (caps == NULL) { - perror("Error getting capabilities"); - goto exit_0; - } - - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, cap_list, CAP_CLEAR) != 0) { - perror("Error setting effective capabilities"); - goto exit_0; - } - - if (cap_set_proc(caps) != 0) { - perror("Error setting capabilities"); - goto exit_0; - } - - if (cap_free(caps) != 0) { - perror("Error freeing capabilities"); - goto exit_0; - } - - return 0; - -exit_0: - return -1; -} - - -/** Enable CAP_SYS_ADMIN so we can enter a container network namespace. */ -int enable_cap_sys_admin() { - cap_t caps; - cap_value_t cap_list[] = {CAP_SYS_ADMIN}; - - caps = cap_get_proc(); - if (caps == NULL) { - perror("Error getting capabilities"); - goto exit_0; - } - - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET) != 0) { + if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap, CAP_SET) != 0) { perror("Error setting effective capabilities"); goto exit_0; } @@ -164,9 +97,9 @@ int enable_cap_sys_admin() { } -int disable_cap_sys_admin() { +/** Disable a specific capability. */ +int disable_cap(cap_value_t cap) { cap_t caps; - cap_value_t cap_list[] = {CAP_SYS_ADMIN}; caps = cap_get_proc(); if (caps == NULL) { @@ -174,7 +107,7 @@ int disable_cap_sys_admin() { goto exit_0; } - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, cap_list, CAP_CLEAR) != 0) { + if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap, CAP_CLEAR) != 0) { perror("Error setting effective capabilities"); goto exit_0; } diff --git a/src/capabilities.h b/src/capabilities.h index ba0b95a..8dc58b9 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -1,5 +1,7 @@ #pragma once +#include + /** Set ambient capabilities. * @@ -11,30 +13,17 @@ int set_ambient_capabilities(); -/** Enable CAP_SYS_PTRACE so we can access a container's network namespace. - * - * @return 0 on success, -1 on failure. - */ -int enable_cap_sys_ptrace(); - - -/** Disable CAP_SYS_PTRACE again. +/** Enable the given capability. * * @return 0 on success, -1 on failure. */ -int disable_cap_sys_ptrace(); +int enable_cap(cap_value_t cap); -/** Enable CAP_SYS_ADMIN so we can switch namespaces. +/** Disable the given capability. * * @return 0 on success, -1 on failure. */ -int enable_cap_sys_admin(); - +int disable_cap(cap_value_t cap); -/** Disable CAP_SYS_ADMIN again. - * - * @return 0 on success, -1 on failure. - */ -int disable_cap_sys_admin(); diff --git a/src/container_wireguard.c b/src/container_wireguard.c index 47f1ed5..3c28839 100644 --- a/src/container_wireguard.c +++ b/src/container_wireguard.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -140,9 +141,9 @@ static int cwg_set_ns(const char * netns_pid) { char netns_path[32]; snprintf(netns_path, 32, "/proc/%s/ns/net", netns_pid); - enable_cap_sys_ptrace(); + enable_cap(CAP_SYS_PTRACE); int netns_fd = open(netns_path, O_RDONLY | O_NONBLOCK); - disable_cap_sys_ptrace(); + disable_cap(CAP_SYS_PTRACE); if (netns_fd == -1) { fprintf(stderr, "When opening %s\n", netns_path); @@ -150,9 +151,9 @@ static int cwg_set_ns(const char * netns_pid) { goto exit_fail; } - enable_cap_sys_admin(); + enable_cap(CAP_SYS_ADMIN); int err = setns(netns_fd, CLONE_NEWNET); - disable_cap_sys_admin(); + disable_cap(CAP_SYS_ADMIN); if (err) { fprintf(stderr, "Could not enter namespace\n"); From 8bd39c6ca5fb4c7787bba72b060485e7cc57e3aa Mon Sep 17 00:00:00 2001 From: Lourens Veen Date: Tue, 5 Oct 2021 12:10:54 +0200 Subject: [PATCH 7/7] Enable IPC_LOCK capability before locking memory --- src/main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main.c b/src/main.c index 4989eb4..e21378f 100644 --- a/src/main.c +++ b/src/main.c @@ -1,15 +1,16 @@ #include #include #include +#include #include #include #include "config.h" +#include "capabilities.h" #include "container_wireguard.h" - void usage(const char * cmd) { fprintf(stderr, "Usage: %s \n\n", cmd); @@ -33,7 +34,9 @@ int main(int argc, char * argv[]) { // Ensure private keys and the like don't get swapped out to a potentially // unencrypted swap partition. - mlockall(MCL_FUTURE); + enable_cap(CAP_IPC_LOCK); + mlockall(MCL_CURRENT | MCL_FUTURE); + disable_cap(CAP_IPC_LOCK); DISPATCH_CWG_CREATE(argv[1]); DISPATCH_CWG_CONNECT(argv[1]);