From 4436af55e55e79b496264fe114039fbc1198d71f Mon Sep 17 00:00:00 2001 From: Kirill Isakov Date: Fri, 22 Apr 2022 21:40:54 +0600 Subject: [PATCH] Add basic pledge/unveil sandbox on OpenBSD --- doc/tinc.conf.5.in | 58 ++++++++++ meson.build | 1 + meson_options.txt | 5 + src/bsd/meson.build | 4 + src/bsd/openbsd/meson.build | 6 + src/bsd/openbsd/sandbox.c | 31 +++++ src/bsd/openbsd/sandbox.h | 31 +++++ src/bsd/openbsd/tincctl.c | 40 +++++++ src/bsd/openbsd/tincd.c | 171 ++++++++++++++++++++++++++++ src/device.h | 2 + src/dummy_device.c | 4 +- src/meson.build | 5 + src/net_setup.c | 36 +++++- src/sandbox.c | 19 ++++ src/sandbox.h | 32 ++++++ src/script.c | 5 + src/subnet.c | 5 + src/tincctl.c | 8 ++ src/tincd.c | 49 +++++++- src/utils.c | 4 + src/utils.h | 3 + test/integration/basic.py | 2 + test/integration/commandline.py | 2 + test/integration/meson.build | 1 + test/integration/proxy.py | 24 ++-- test/integration/sandbox.py | 147 ++++++++++++++++++++++++ test/integration/scripts.py | 23 +++- test/integration/security.py | 2 + test/integration/splice.py | 3 + test/integration/testlib/event.py | 3 + test/integration/testlib/feature.py | 9 ++ test/integration/testlib/path.py | 1 + test/integration/testlib/proc.py | 1 + test/integration/testlib/util.py | 9 ++ test/unit/test_proxy.c | 3 + test/unit/test_utils.c | 14 +++ 36 files changed, 742 insertions(+), 21 deletions(-) create mode 100644 src/bsd/openbsd/meson.build create mode 100644 src/bsd/openbsd/sandbox.c create mode 100644 src/bsd/openbsd/sandbox.h create mode 100644 src/bsd/openbsd/tincctl.c create mode 100644 src/bsd/openbsd/tincd.c create mode 100644 src/sandbox.c create mode 100644 src/sandbox.h create mode 100755 test/integration/sandbox.py create mode 100755 test/integration/testlib/feature.py diff --git a/doc/tinc.conf.5.in b/doc/tinc.conf.5.in index d7aa7d99..f0e765dc 100644 --- a/doc/tinc.conf.5.in +++ b/doc/tinc.conf.5.in @@ -492,6 +492,47 @@ the interaction of replay tracking with underlying real packet loss and/or reordering. Setting this to zero will disable replay tracking completely and pass all traffic, but leaves tinc vulnerable to replay-based attacks on your traffic. +.It Va Sandbox Li = off | normal | high Po normal Pc +Use process sandbox on some operating systems where it is supported (currently that's OpenBSD). +Using this directive on other operating systems with levels higher than +.Ar off +will cause +.Nm tincd +to exit with an error. +The goal is to limit the impact of possible remote attacks against the +.Nm tincd +daemon by running it with lowest privileges necessary for the required features to work. +The following levels are provided: +.Bl -tag -width indent +.It off +Disable sandbox. +No restrictions are put on +.Nm tincd , +all functionality works as if this feature did not exist. +.It normal +The default level which aims to be safe for most users. +Adds some level of protection with only minor reductions in functionality. +For example, executables located in non-standard paths may not be available as +.Nm tincd +scripts or +.Ar exec +proxies, and configuration reloading may not work for some variables, forcing you to restart +.Nm tincd +to apply new settings. +.It high +Fully disables +.Ar exec +proxies and +.Nm tincd +scripts, with the exception of initial +.Nm tinc-up +and +.Nm subnet-up . +This allows +.Nm tincd +to block large parts of operating system interface that may be useful to attackers. +Strongly consider using this level if you need neither of these features. +.El .It Va StrictSubnets Li = yes | no Po no Pc Bq experimental When this option is enabled tinc will only use Subnet statements which are present in the host config files in the local @@ -770,6 +811,23 @@ script is called, this is set to the invitation URL that has been created. .Pp Do not forget that under UNIX operating systems, you have to make the scripts executable, using the command .Nm chmod Li a+x Pa script . +.Pp +Here's the list of script configuration variables in alphabetical order. +.Bl -tag -width indent +.It Va ScriptsExtension Li = Ar .extension Pq empty +File extension to use for +.Nm tincd +scripts. For example, +.Ar .py , +.Ar .pl , +or +.Ar .rb . +Please note than it is simply concatenated with the script name and the dot is not added automatically. +.It Va ScriptsInterpreter Li = Pa /path/to/interpreter Pq empty +Used as an interpreter for scripts started by +.Nm tincd +by prepending it to the start of the command line. +If the variable is empty (which is the default), scripts are executed directly. .Sh FILES The most important files are: .Bl -tag -width indent diff --git a/meson.build b/meson.build index 43ae9744..82aa7223 100644 --- a/meson.build +++ b/meson.build @@ -20,6 +20,7 @@ opt_lz4 = get_option('lz4') opt_lzo = get_option('lzo') opt_miniupnpc = get_option('miniupnpc') opt_readline = get_option('readline') +opt_sandbox = get_option('sandbox') opt_static = get_option('static') opt_systemd = get_option('systemd') opt_tests = get_option('tests') diff --git a/meson_options.txt b/meson_options.txt index bb463ce1..0df57950 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -89,3 +89,8 @@ option('jumbograms', value: false, description: 'support for jumbograms (packets up to 9000 bytes)') +option('sandbox', + type: 'feature', + value: 'auto', + description: 'use sandboxing on platforms that support it') + diff --git a/src/bsd/meson.build b/src/bsd/meson.build index 690e7373..d6205f4f 100644 --- a/src/bsd/meson.build +++ b/src/bsd/meson.build @@ -13,6 +13,10 @@ check_functions += [ src_tincd += files('device.c') +if os_name == 'openbsd' + subdir('openbsd') +endif + if os_name == 'darwin' dep_tunemu = dependency('tunemu', required: opt_tunemu, static: static) dep_pcap = dependency('pcap', required: opt_tunemu, static: static) diff --git a/src/bsd/openbsd/meson.build b/src/bsd/openbsd/meson.build new file mode 100644 index 00000000..709cf506 --- /dev/null +++ b/src/bsd/openbsd/meson.build @@ -0,0 +1,6 @@ +if not opt_sandbox.disabled() + src_lib_common += files('sandbox.c') + src_tinc += files('tincctl.c') + src_tincd += files('tincd.c') + cdata.set('HAVE_SANDBOX', 1) +endif diff --git a/src/bsd/openbsd/sandbox.c b/src/bsd/openbsd/sandbox.c new file mode 100644 index 00000000..180571bb --- /dev/null +++ b/src/bsd/openbsd/sandbox.c @@ -0,0 +1,31 @@ +#include "../../system.h" + +#include "sandbox.h" +#include "../../logger.h" + +void allow_path(const char *path, const char *priv) { + if(path) { + logger(DEBUG_ALWAYS, LOG_DEBUG, "Allowing path %s with %s", path, priv); + + if(unveil(path, priv)) { + logger(DEBUG_ALWAYS, LOG_ERR, "unveil(%s, %s) failed: %s", path, priv, strerror(errno)); + } + } +} + +void allow_paths(const unveil_path_t paths[]) { + // Since some path variables may contain NULL, we check priv here. + // If a NULL path is seen, just skip it. + for(const unveil_path_t *p = paths; p->priv; ++p) { + allow_path(p->path, p->priv); + } +} + +bool restrict_privs(const char *promises, const char *execpromises) { + if(pledge(promises, execpromises)) { + logger(DEBUG_ALWAYS, LOG_ERR, "pledge(%s, %s) failed: %s", promises, execpromises, strerror(errno)); + return false; + } else { + return true; + } +} diff --git a/src/bsd/openbsd/sandbox.h b/src/bsd/openbsd/sandbox.h new file mode 100644 index 00000000..1b6ee13b --- /dev/null +++ b/src/bsd/openbsd/sandbox.h @@ -0,0 +1,31 @@ +#ifndef TINC_BSD_OPENBSD_SANDBOX_H +#define TINC_BSD_OPENBSD_SANDBOX_H + +#include "../../system.h" + +typedef struct unveil_path_t { + const char *path; + const char *priv; +} unveil_path_t; + +// No restrictions +static const char *PROMISES_ALL = NULL; + +// Full restrictions; children can call nothing but exit() +static const char *PROMISES_NONE = ""; + +// Allow access to the paths with the specified privileges. Can be called multiple times. +// This is a thin logging wrapper around unveil(2). +// Paths that are equal to NULL are skipped. The last path in the array must be (NULL, NULL). +// Note that after the last call to this function you should lock access to +// unveil(2) using pledge(2) to prevent the program from undoing the sandbox. +extern void allow_paths(const unveil_path_t paths[]); + +// Allow access to a single path. Logging wrapper around unveil(). +extern void allow_path(const char *path, const char *priv); + +// Restrict privileges. Can be called multiple times to further restrict (but not regain) them. +// It's a thin logging wrapper around unveil(2), see man page for details. +extern bool restrict_privs(const char *promises, const char *execpromises); + +#endif // TINC_BSD_OPENBSD_SANDBOX_H diff --git a/src/bsd/openbsd/tincctl.c b/src/bsd/openbsd/tincctl.c new file mode 100644 index 00000000..5dadfd02 --- /dev/null +++ b/src/bsd/openbsd/tincctl.c @@ -0,0 +1,40 @@ +#include "../../system.h" + +#include "sandbox.h" +#include "../../sandbox.h" + +static const char *promises = + "stdio" // General I/O + " rpath" // Read configs & keys + " wpath" // Write same + " cpath" // Create same + " fattr" // chmod() same + " proc" // Check that tincd is running with kill() + " dns" // Resolve domain names + " inet" // Check that port is available + " unix" // Control connection to tincd + " exec" // Start tincd +#if defined(HAVE_CURSES) || defined(HAVE_READLINE) + " tty" +#endif + ; + +static sandbox_level_t current_level = SANDBOX_NONE; + +void sandbox_set_level(sandbox_level_t level) { + current_level = level; +} + +bool sandbox_enter() { + if(current_level == SANDBOX_NONE) { + return true; + } else { + return restrict_privs(promises, PROMISES_ALL); + } +} + +bool sandbox_can(sandbox_action_t action, sandbox_time_t when) { + (void)action; + (void)when; + return true; +} diff --git a/src/bsd/openbsd/tincd.c b/src/bsd/openbsd/tincd.c new file mode 100644 index 00000000..0b49ea9e --- /dev/null +++ b/src/bsd/openbsd/tincd.c @@ -0,0 +1,171 @@ +#include "../../system.h" + +#include +#include + +#include "sandbox.h" +#include "../../device.h" +#include "../../logger.h" +#include "../../names.h" +#include "../../net.h" +#include "../../sandbox.h" +#include "../../script.h" +#include "../../xalloc.h" +#include "../../proxy.h" + +static sandbox_level_t current_level = SANDBOX_NONE; +static bool can_use_new_paths = true; +static bool entered = false; + +static bool chrooted(void) { + return !(confbase && *confbase); +} + +static void create_conf_subdir(const char *name, mode_t mode) { + char path[PATH_MAX]; + snprintf(path, sizeof(path), "%s/%s", confbase, name); + mkdir(path, mode); +} + +static void open_conf_subdir(const char *name, const char *privs) { + char path[PATH_MAX]; + snprintf(path, sizeof(path), "%s/%s", confbase, name); + allow_path(path, privs); +} + +static void open_common_paths(bool can_exec) { + // Dummy device uses a fake path, skip it + const char *dev = strcasecmp(device, DEVICE_DUMMY) ? device : NULL; + + // These calls must be done before the first unveil() for two reasons: + // 1. the first unveil() blocks access to all other paths. + // 2. unveil() remembers the exact directory and won't allow access if it's (re)created. + create_conf_subdir("cache", 0777); + create_conf_subdir("hosts", 0777); + create_conf_subdir("invitations", 0700); + + const unveil_path_t paths[] = { + {"/dev/random", "r"}, + {"/dev/urandom", "r"}, + {confbase, can_exec ? "rx" : "r"}, + {dev, "rw"}, + {logfilename, "rwc"}, + {pidfilename, "rwc"}, + {unixsocketname, "rwc"}, + {NULL, NULL}, + }; + allow_paths(paths); + + open_conf_subdir("cache", "rwc"); + open_conf_subdir("hosts", can_exec ? "rwxc" : "rwc"); + open_conf_subdir("invitations", "rwc"); +} + +static void open_exec_paths(void) { + // proxyhost was checked previously. If we're here, proxyhost + // contains the path to the executable, and nothing else. + const char *proxy_exec = proxytype == PROXY_EXEC ? proxyhost : NULL; + + const unveil_path_t bin_paths[] = { + {"/bin", "rx"}, + {"/sbin", "rx"}, + {"/usr/bin", "rx"}, + {"/usr/sbin", "rx"}, + {"/usr/local/bin", "rx"}, + {"/usr/local/sbin", "rx"}, + {scriptinterpreter, "rx"}, + {proxy_exec, "rx"}, + {NULL, NULL}, + }; + allow_paths(bin_paths); +} + +static bool sandbox_privs(bool can_exec) { + // no mcast since multicasting should be set up by now + char promises[512] = + "stdio" // General I/O, both disk and network + " rpath" // Read files and directories + " wpath" // Write files and directories + " cpath" // Create new ones + " dns" // Resolve domain names + " inet" // Make network connections + " unix"; // Control socket connections from tinc CLI + + if(can_exec) { + // fork() and execve() for scripts and exec proxies + const char *exec = " proc exec"; + size_t n = strlcat(promises, exec, sizeof(promises)); + assert(n < sizeof(promises)); + } + + return restrict_privs(promises, can_exec ? PROMISES_ALL : PROMISES_NONE); +} + +static void sandbox_paths(bool can_exec) { + if(chrooted()) { + logger(DEBUG_ALWAYS, LOG_DEBUG, "chroot is used. Disabling path sandbox."); + return; + } + + open_common_paths(can_exec); + can_use_new_paths = false; + + if(can_exec) { + if(proxytype == PROXY_EXEC && !access(proxyhost, X_OK)) { + logger(DEBUG_ALWAYS, LOG_WARNING, "Looks like a shell expression was used for exec proxy. Using weak path sandbox."); + allow_path("/", "rx"); + } else { + open_exec_paths(); + } + } +} + +static bool sandbox_can_after_enter(sandbox_action_t action) { + switch(action) { + case START_PROCESSES: + return current_level < SANDBOX_HIGH; + + case USE_NEW_PATHS: + return can_use_new_paths; + + default: + abort(); + } +} + +bool sandbox_can(sandbox_action_t action, sandbox_time_t when) { + if(when == AFTER_SANDBOX || entered) { + return sandbox_can_after_enter(action); + } else { + return true; + } +} + +void sandbox_set_level(sandbox_level_t level) { + assert(!entered); + current_level = level; +} + +bool sandbox_enter() { + assert(!entered); + entered = true; + + if(current_level == SANDBOX_NONE) { + logger(DEBUG_ALWAYS, LOG_DEBUG, "Sandbox is disabled"); + return true; + } + + bool can_exec = sandbox_can_after_enter(START_PROCESSES); + + sandbox_paths(can_exec); + + if(sandbox_privs(can_exec)) { + logger(DEBUG_ALWAYS, LOG_DEBUG, "Entered sandbox at level %d", current_level); + return true; + } + + logger(DEBUG_ALWAYS, LOG_ERR, "Could not enter sandbox. Set a lower level or disable it in tinc.conf"); + current_level = SANDBOX_NONE; + + return false; +} diff --git a/src/device.h b/src/device.h index c85671b3..32c0b875 100644 --- a/src/device.h +++ b/src/device.h @@ -27,6 +27,8 @@ extern int device_fd; extern char *device; extern char *iface; +#define DEVICE_DUMMY "dummy" + typedef struct devops_t { bool (*setup)(void); void (*close)(void); diff --git a/src/dummy_device.c b/src/dummy_device.c index 94c71ea0..4ffaf38f 100644 --- a/src/dummy_device.c +++ b/src/dummy_device.c @@ -27,8 +27,8 @@ static const char *device_info = "dummy device"; static bool setup_device(void) { - device = xstrdup("dummy"); - iface = xstrdup("dummy"); + device = xstrdup(DEVICE_DUMMY); + iface = xstrdup(DEVICE_DUMMY); logger(DEBUG_ALWAYS, LOG_INFO, "%s (%s) is a %s", device, iface, device_info); return true; } diff --git a/src/meson.build b/src/meson.build index adf82f62..564ef6fc 100644 --- a/src/meson.build +++ b/src/meson.build @@ -368,6 +368,11 @@ endif subdir('include') +have_sandbox = cdata.has('HAVE_SANDBOX') +if not have_sandbox + src_lib_common += 'sandbox.c' +endif + lib_crypto = static_library( 'tinc_crypto', sources: src_lib_crypto, diff --git a/src/net_setup.c b/src/net_setup.c index 03186134..3ac5a679 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -45,6 +45,7 @@ #include "utils.h" #include "xalloc.h" #include "keys.h" +#include "sandbox.h" #ifdef HAVE_MINIUPNPC #include "upnp.h" @@ -230,11 +231,25 @@ char *get_name(void) { return returned_name; } -bool setup_myself_reloadable(void) { - free(scriptinterpreter); - scriptinterpreter = NULL; +static void read_interpreter(void) { + char *interpreter = NULL; + get_config_string(lookup_config(&config_tree, "ScriptsInterpreter"), &interpreter); + + if(!interpreter || (sandbox_can(START_PROCESSES, AFTER_SANDBOX) && sandbox_can(USE_NEW_PATHS, AFTER_SANDBOX))) { + free(scriptinterpreter); + scriptinterpreter = interpreter; + return; + } + + if(!string_eq(interpreter, scriptinterpreter)) { + logger(DEBUG_ALWAYS, LOG_NOTICE, "Not changing ScriptsInterpreter because of sandbox."); + } - get_config_string(lookup_config(&config_tree, "ScriptsInterpreter"), &scriptinterpreter); + free(interpreter); +} + +bool setup_myself_reloadable(void) { + read_interpreter(); free(scriptextension); @@ -264,7 +279,12 @@ bool setup_myself_reloadable(void) { } else if(!strcasecmp(proxy, "http")) { proxytype = PROXY_HTTP; } else if(!strcasecmp(proxy, "exec")) { - proxytype = PROXY_EXEC; + if(sandbox_can(START_PROCESSES, AFTER_SANDBOX)) { + proxytype = PROXY_EXEC; + } else { + logger(DEBUG_ALWAYS, LOG_ERR, "Cannot use exec proxies with current sandbox level."); + return false; + } } else { logger(DEBUG_ALWAYS, LOG_ERR, "Unknown proxy type %s!", proxy); free_string(proxy); @@ -295,6 +315,10 @@ bool setup_myself_reloadable(void) { return false; } + if(!sandbox_can(USE_NEW_PATHS, AFTER_SANDBOX)) { + logger(DEBUG_ALWAYS, LOG_NOTICE, "Changed exec proxy may fail to work because of sandbox."); + } + proxyhost = xstrdup(space); break; @@ -1022,7 +1046,7 @@ static bool setup_myself(void) { devops = os_devops; if(get_config_string(lookup_config(&config_tree, "DeviceType"), &type)) { - if(!strcasecmp(type, "dummy")) { + if(!strcasecmp(type, DEVICE_DUMMY)) { devops = dummy_devops; } else if(!strcasecmp(type, "raw_socket")) { devops = raw_socket_devops; diff --git a/src/sandbox.c b/src/sandbox.c new file mode 100644 index 00000000..d846e174 --- /dev/null +++ b/src/sandbox.c @@ -0,0 +1,19 @@ +#include "system.h" + +#include "sandbox.h" + +// Stubs for platforms without sandbox support to avoid using lots of #ifdefs. + +bool sandbox_can(sandbox_action_t action, sandbox_time_t when) { + (void)action; + (void)when; + return true; +} + +void sandbox_set_level(sandbox_level_t level) { + (void)level; +} + +bool sandbox_enter(void) { + return true; +} diff --git a/src/sandbox.h b/src/sandbox.h new file mode 100644 index 00000000..66f204c4 --- /dev/null +++ b/src/sandbox.h @@ -0,0 +1,32 @@ +#ifndef TINC_SANDBOX_H +#define TINC_SANDBOX_H + +#include "system.h" + +typedef enum sandbox_level_t { + SANDBOX_NONE, + SANDBOX_NORMAL, + SANDBOX_HIGH, +} sandbox_level_t; + +typedef enum sandbox_action_t { + START_PROCESSES, // Start child processes + USE_NEW_PATHS, // Access to filesystem paths that were not known at the start of the process +} sandbox_action_t; + +typedef enum sandbox_time_t { + AFTER_SANDBOX, // Check if the action can be performed after entering sandbox + RIGHT_NOW, // Check if the action can be performed right now +} sandbox_time_t; + +// Check if the current process has enough privileges to perform the action +extern bool sandbox_can(sandbox_action_t action, sandbox_time_t when); + +// Set the expected sandbox level. Call sandbox_enter() to actually apply it. +extern void sandbox_set_level(sandbox_level_t level); + +// Enter sandbox using the passed level. Returns true if successful. +// Obviously, this is a one-way function, there's no way to reverse it. +extern bool sandbox_enter(void); + +#endif // TINC_SANDBOX_H diff --git a/src/script.c b/src/script.c index 3f44bf9f..2f2c30bf 100644 --- a/src/script.c +++ b/src/script.c @@ -26,6 +26,7 @@ #include "names.h" #include "script.h" #include "xalloc.h" +#include "sandbox.h" #ifdef HAVE_PUTENV static void unputenv(const char *p) { @@ -141,6 +142,10 @@ void environment_exit(environment_t *env) { } bool execute_script(const char *name, environment_t *env) { + if(!sandbox_can(START_PROCESSES, RIGHT_NOW)) { + return false; + } + char scriptname[PATH_MAX]; char *command; diff --git a/src/subnet.c b/src/subnet.c index 1ddf11e5..94000cc0 100644 --- a/src/subnet.c +++ b/src/subnet.c @@ -31,6 +31,7 @@ #include "script.h" #include "subnet.h" #include "xalloc.h" +#include "sandbox.h" /* lists type of subnet */ uint32_t hash_seed; @@ -321,6 +322,10 @@ subnet_t *lookup_subnet_ipv6(const ipv6_t *address) { } void subnet_update(node_t *owner, subnet_t *subnet, bool up) { + if(!sandbox_can(START_PROCESSES, RIGHT_NOW)) { + return; + } + char netstr[MAXNETSTR]; char *address, *port; char empty[] = ""; diff --git a/src/tincctl.c b/src/tincctl.c index afe19cd2..a0a02810 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -41,6 +41,7 @@ #include "subnet.h" #include "keys.h" #include "random.h" +#include "sandbox.h" #include "pidfile.h" #include "console.h" @@ -119,6 +120,9 @@ static void version(void) { #endif #ifndef DISABLE_LEGACY " legacy_protocol" +#endif +#ifdef HAVE_SANDBOX + " sandbox" #endif "\n\n" "Copyright (C) 1998-2018 Ivo Timmermans, Guus Sliepen and others.\n" @@ -1695,6 +1699,7 @@ const var_t variables[] = { {"ProcessPriority", VAR_SERVER}, {"Proxy", VAR_SERVER}, {"ReplayWindow", VAR_SERVER | VAR_SAFE}, + {"Sandbox", VAR_SERVER}, {"ScriptsExtension", VAR_SERVER}, {"ScriptsInterpreter", VAR_SERVER}, {"StrictSubnets", VAR_SERVER | VAR_SAFE}, @@ -3350,6 +3355,9 @@ int main(int argc, char *argv[]) { crypto_init(); prng_init(); + sandbox_set_level(SANDBOX_NORMAL); + sandbox_enter(); + int result = run_command(argc, argv); random_exit(); diff --git a/src/tincd.c b/src/tincd.c index edb03f51..539f5274 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -55,6 +55,7 @@ #include "xalloc.h" #include "version.h" #include "random.h" +#include "sandbox.h" /* If nonzero, display usage information and exit. */ static bool show_help = false; @@ -322,6 +323,44 @@ exit_fail: return false; } +static bool read_sandbox_level(void) { + sandbox_level_t level; + char *value = NULL; + + if(get_config_string(lookup_config(&config_tree, "Sandbox"), &value)) { + if(!strcasecmp("off", value)) { + level = SANDBOX_NONE; + } else if(!strcasecmp("normal", value)) { + level = SANDBOX_NORMAL; + } else if(!strcasecmp("high", value)) { + level = SANDBOX_HIGH; + } else { + logger(DEBUG_ALWAYS, LOG_ERR, "Bad sandbox value %s!", value); + free(value); + return false; + } + + free(value); + } else { +#ifdef HAVE_SANDBOX + level = SANDBOX_NORMAL; +#else + level = SANDBOX_NONE; +#endif + } + +#ifndef HAVE_SANDBOX + + if(level > SANDBOX_NONE) { + logger(DEBUG_ALWAYS, LOG_ERR, "Sandbox is used but is not supported on this platform"); + return false; + } + +#endif + sandbox_set_level(level); + return true; +} + static bool drop_privs(void) { #ifndef HAVE_WINDOWS uid_t uid = 0; @@ -373,7 +412,8 @@ static bool drop_privs(void) { } #endif - return true; + + return sandbox_enter(); } #ifdef HAVE_WINDOWS @@ -448,6 +488,9 @@ int main(int argc, char **argv) { #ifdef HAVE_MINIUPNPC " miniupnpc" #endif +#ifdef HAVE_SANDBOX + " sandbox" +#endif #ifdef ENABLE_UML " uml" #endif @@ -530,6 +573,10 @@ int main(int argc, char **argv) { return 1; } + if(!read_sandbox_level()) { + return 1; + } + if(debug_level == DEBUG_NOTHING) { int level = 0; diff --git a/src/utils.c b/src/utils.c index 5f40b8af..231938dc 100644 --- a/src/utils.c +++ b/src/utils.c @@ -373,3 +373,7 @@ FILE *fopenmask(const char *filename, const char *mode, mode_t perms) { return f; } +bool string_eq(const char *first, const char *second) { + return !first == !second && + !(first && second && strcmp(first, second)); +} diff --git a/src/utils.h b/src/utils.h index f02bb9f2..487058ae 100644 --- a/src/utils.h +++ b/src/utils.h @@ -78,4 +78,7 @@ char *absolute_path(const char *path) ATTR_MALLOC; extern FILE *fopenmask(const char *filename, const char *mode, mode_t perms) ATTR_DEALLOCATOR(fclose); +// NULL-safe wrapper around strcmp(). +extern bool string_eq(const char *first, const char *second); + #endif diff --git a/test/integration/basic.py b/test/integration/basic.py index aae3ff27..17642096 100755 --- a/test/integration/basic.py +++ b/test/integration/basic.py @@ -4,6 +4,7 @@ from testlib.test import Test from testlib.proc import Tinc +from testlib.feature import SANDBOX_LEVEL from testlib.log import log from testlib.script import Script from testlib import check @@ -18,6 +19,7 @@ def init(ctx: Test) -> Tinc: set Address localhost set Port 0 set DeviceType dummy + set Sandbox {SANDBOX_LEVEL} """ node.cmd(stdin=stdin) return node diff --git a/test/integration/commandline.py b/test/integration/commandline.py index 931b6d40..6781545e 100755 --- a/test/integration/commandline.py +++ b/test/integration/commandline.py @@ -11,6 +11,7 @@ from testlib import check, util, path from testlib.log import log from testlib.proc import Tinc, Script from testlib.test import Test +from testlib.feature import SANDBOX_LEVEL tinc_flags = ( (0, ("get", "name")), @@ -59,6 +60,7 @@ def init(ctx: Test) -> Tinc: set Port 0 set Address localhost set DeviceType dummy + set Sandbox {SANDBOX_LEVEL} """ tinc.cmd(stdin=stdin) tinc.add_script(Script.TINC_UP) diff --git a/test/integration/meson.build b/test/integration/meson.build index 5e82fe85..cf09cac5 100644 --- a/test/integration/meson.build +++ b/test/integration/meson.build @@ -7,6 +7,7 @@ tests = [ 'invite.py', 'invite_tinc_up.py', 'proxy.py', + 'sandbox.py', 'scripts.py', 'security.py', 'splice.py', diff --git a/test/integration/proxy.py b/test/integration/proxy.py index 93b51f58..c0e79710 100755 --- a/test/integration/proxy.py +++ b/test/integration/proxy.py @@ -4,7 +4,7 @@ import os import re -import tempfile +import time import typing as T import multiprocessing.connection as mp import logging @@ -14,11 +14,12 @@ import struct from threading import Thread from socketserver import ThreadingMixIn, TCPServer, StreamRequestHandler -from testlib import check, cmd, path +from testlib import check, cmd, path, util from testlib.proc import Tinc, Script from testlib.test import Test from testlib.util import random_string from testlib.log import log +from testlib.feature import HAVE_SANDBOX USERNAME = random_string(8) PASSWORD = random_string(8) @@ -382,12 +383,7 @@ import multiprocessing.connection as mp with mp.Client(("127.0.0.1", {port}), family="AF_INET") as client: client.send({{ **os.environ }}) """ - - file = tempfile.mktemp() - with open(file, "w", encoding="utf-8") as f: - f.write(code) - - return file + return util.temp_file(code) def test_proxy(ctx: Test, handler: T.Type[ProxyServer], user="", passw="") -> None: @@ -395,7 +391,10 @@ def test_proxy(ctx: Test, handler: T.Type[ProxyServer], user="", passw="") -> No foo, bar = init(ctx) - bar.add_script(foo.script_up) + if HAVE_SANDBOX: + for node in foo, bar: + node.cmd("set", "Sandbox", "high") + bar.add_script(Script.TINC_UP) bar.start() @@ -409,8 +408,11 @@ def test_proxy(ctx: Test, handler: T.Type[ProxyServer], user="", passw="") -> No worker.start() foo.cmd("set", "Proxy", handler.name, f"127.0.0.1 {port} {user} {passw}") + + foo.add_script(Script.TINC_UP) foo.cmd("start") - bar[foo.script_up].wait() + foo[Script.TINC_UP].wait() + time.sleep(1) foo.cmd("stop") bar.cmd("stop") @@ -436,7 +438,7 @@ def test_proxy_exec(ctx: Test) -> None: port = int(listener.address[1]) proxy = create_exec_proxy(port) - foo.cmd("set", "Proxy", "exec", f"{path.PYTHON_PATH} {path.PYTHON_CMD} {proxy}") + foo.cmd("set", "Proxy", "exec", f"{path.PYTHON_INTERPRETER} {proxy}") foo.cmd("start") with listener.accept() as conn: diff --git a/test/integration/sandbox.py b/test/integration/sandbox.py new file mode 100755 index 00000000..26f69286 --- /dev/null +++ b/test/integration/sandbox.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 + +"""Test that tincd works through proxies.""" + +import os +import time + +from testlib import check, cmd, path, util +from testlib.proc import Tinc, Script +from testlib.test import Test +from testlib.log import log +from testlib.feature import HAVE_SANDBOX + + +def init(ctx: Test, level: str) -> Tinc: + """Create a new tinc node.""" + + node = ctx.node() + + stdin = f""" + init {node} + set Address 127.0.0.1 + set Port 0 + set DeviceType dummy + set Sandbox {level} + """ + node.cmd(stdin=stdin) + + return node + + +def test_scripts_work(ctx: Test, level: str) -> None: + """Test that scripts work under the sandbox level.""" + foo = init(ctx, level) + foo.cmd("set", "Subnet", "1.2.3.4") + + for script in Script: + foo.add_script(script) + + foo.cmd("start") + foo[Script.TINC_UP].wait() + foo[Script.SUBNET_UP].wait() + + if os.name != "nt": + foo.cmd("set", "ScriptsInterpreter", path.PYTHON_PATH) + + foo.cmd("stop") + foo[Script.SUBNET_DOWN].wait() + foo[Script.TINC_DOWN].wait() + + +def test_high_scripts(ctx: Test) -> None: + """Test that only tinc-up/subnet-up work on highest isolation level.""" + foo = init(ctx, "high") + foo.cmd("set", "Subnet", "1.2.3.4") + + for script in Script: + foo.add_script(script) + + foo.cmd("start") + for script in Script.TINC_UP, Script.SUBNET_UP: + foo[script].wait() + + time.sleep(1) + foo.cmd("stop") + + while True: + try: + foo.cmd("pid", code=1) + break + except ValueError: + time.sleep(0.5) + + log.info("check that no other scripts were called") + for script in Script.SUBNET_DOWN, Script.TINC_DOWN: + check.false(foo[script].wait(0.01)) + + +def create_exec_proxy() -> str: + """Create a fake exec proxy that stops the test with an error.""" + code = f""" +import os +import signal + +os.kill({os.getpid()}, signal.SIGTERM) +""" + return util.temp_file(code) + + +def test_exec_proxy_does_not_start_on_high(ctx: Test) -> None: + """Check that tincd does not start if both exec proxy and high level are set.""" + foo = init(ctx, "high") + foo.cmd("set", "Proxy", "exec", path.PYTHON_INTERPRETER) + foo.cmd("start", code=1) + + +def test_bad_sandbox_level(ctx: Test, level: str) -> None: + """Check that tincd does not start if a bad sandbox level is used.""" + foo = init(ctx, level) + foo.cmd("start", code=1) + + +def test_exec_proxy_high(ctx: Test) -> None: + """Test that exec proxy does not work at maximum isolation.""" + foo, bar = init(ctx, "high"), init(ctx, "high") + + foo.add_script(Script.TINC_UP) + foo.start() + + proxy = create_exec_proxy() + foo.cmd("set", "Proxy", "exec", f"{path.PYTHON_INTERPRETER} {proxy}") + + cmd.exchange(foo, bar) + bar.cmd("set", f"{foo}.Port", str(foo.port)) + + bar.add_script(Script.TINC_UP) + bar.cmd("start") + bar[Script.TINC_UP].wait() + + time.sleep(1) + + bar.cmd("stop") + foo.cmd("stop") + + +with Test("all scripts work at level 'off'") as context: + test_scripts_work(context, "off") + +if HAVE_SANDBOX: + with Test("all scripts work at level 'normal'") as context: + test_scripts_work(context, "normal") + + with Test("only tinc-up and first subnet-up work at level 'high'") as context: + test_high_scripts(context) + + with Test("tincd does not start with exec proxy and level 'high'") as context: + test_exec_proxy_does_not_start_on_high(context) + + with Test("tincd does not start with bad sandbox level") as context: + test_bad_sandbox_level(context, "foobar") + + with Test("exec proxy does not work at level 'high'") as context: + test_exec_proxy_high(context) +else: + with Test("tincd does not start with bad sandbox level") as context: + for lvl in "normal", "high", "foobar": + test_bad_sandbox_level(context, lvl) diff --git a/test/integration/scripts.py b/test/integration/scripts.py index ef2fc7e6..7b1307e2 100755 --- a/test/integration/scripts.py +++ b/test/integration/scripts.py @@ -5,7 +5,7 @@ import os import typing as T -from testlib import check +from testlib import check, path from testlib.log import log from testlib.proc import Tinc, Script, ScriptType, TincScript from testlib.test import Test @@ -226,5 +226,26 @@ def run_tests(ctx: Test) -> None: test_stop_server(server, client) +def run_script_interpreter_test(ctx: Test) -> None: + """Check that tincd scripts run with a custom script interpreter.""" + foo = ctx.node() + stdin = f""" + init {foo} + set Port 0 + set DeviceType dummy + set ScriptsInterpreter {path.PYTHON_PATH} + """ + foo_up = foo.add_script(Script.TINC_UP) + foo.cmd(stdin=stdin) + + foo.cmd("start") + foo_up.wait() + foo.cmd("stop") + + with Test("scripts test") as context: run_tests(context) + +if os.name != "nt": + with Test("works with ScriptInterpreter") as context: + run_script_interpreter_test(context) diff --git a/test/integration/security.py b/test/integration/security.py index 11f42f88..bfaaa733 100755 --- a/test/integration/security.py +++ b/test/integration/security.py @@ -9,6 +9,7 @@ from testlib import check from testlib.log import log from testlib.proc import Tinc, Script from testlib.test import Test +from testlib.feature import SANDBOX_LEVEL TIMEOUT = 2 @@ -97,6 +98,7 @@ def init(ctx: Test) -> Tinc: set PingTimeout {TIMEOUT} set AutoConnect no set Subnet 10.96.96.1 + set Sandbox {SANDBOX_LEVEL} """ foo.cmd(stdin=stdin) diff --git a/test/integration/splice.py b/test/integration/splice.py index 578845fb..ce8136aa 100755 --- a/test/integration/splice.py +++ b/test/integration/splice.py @@ -10,6 +10,7 @@ from testlib import check, cmd, path from testlib.log import log from testlib.proc import Tinc, Script from testlib.test import Test +from testlib.feature import SANDBOX_LEVEL def init(ctx: Test, *options: str) -> T.Tuple[Tinc, Tinc]: @@ -26,6 +27,7 @@ def init(ctx: Test, *options: str) -> T.Tuple[Tinc, Tinc]: set Address localhost set AutoConnect no set Subnet 10.96.96.1 + set Sandbox {SANDBOX_LEVEL} {custom} """ foo.cmd(stdin=stdin) @@ -37,6 +39,7 @@ def init(ctx: Test, *options: str) -> T.Tuple[Tinc, Tinc]: set DeviceType dummy set AutoConnect no set Subnet 10.96.96.2 + set Sandbox {SANDBOX_LEVEL} {custom} """ bar.cmd(stdin=stdin) diff --git a/test/integration/testlib/event.py b/test/integration/testlib/event.py index 4acd375b..d52cedda 100755 --- a/test/integration/testlib/event.py +++ b/test/integration/testlib/event.py @@ -40,6 +40,9 @@ class Notification: if _MONOTONIC_IS_SYSTEMWIDE: self.update_time() + def __str__(self) -> str: + return f"{self.test}/{self.node}/{self.script}" + def update_time(self) -> None: """Update creation time if it was not assigned previously.""" if self.created_at is None: diff --git a/test/integration/testlib/feature.py b/test/integration/testlib/feature.py new file mode 100755 index 00000000..44dd7b1b --- /dev/null +++ b/test/integration/testlib/feature.py @@ -0,0 +1,9 @@ +"""Some hardcoded constants.""" + +from .proc import Feature, Tinc + +# True if tincd has sandbox support +HAVE_SANDBOX = Feature.SANDBOX in Tinc().features + +# Maximum supported sandbox level +SANDBOX_LEVEL = "high" if Feature.SANDBOX in Tinc().features else "off" diff --git a/test/integration/testlib/path.py b/test/integration/testlib/path.py index 4a90ac9e..a4d31207 100755 --- a/test/integration/testlib/path.py +++ b/test/integration/testlib/path.py @@ -24,6 +24,7 @@ SPTPS_TEST_PATH = str(env["SPTPS_TEST_PATH"]) SPTPS_KEYPAIR_PATH = str(env["SPTPS_KEYPAIR_PATH"]) PYTHON_CMD = "runpython" if "meson.exe" in PYTHON_PATH.lower() else "" +PYTHON_INTERPRETER = f"{PYTHON_PATH} {PYTHON_CMD}".rstrip() def _check() -> bool: diff --git a/test/integration/testlib/proc.py b/test/integration/testlib/proc.py index 1faea550..a0e6c458 100755 --- a/test/integration/testlib/proc.py +++ b/test/integration/testlib/proc.py @@ -50,6 +50,7 @@ class Feature(Enum): OPENSSL = "openssl" READLINE = "readline" TUNEMU = "tunemu" + SANDBOX = "sandbox" UML = "uml" VDE = "vde" diff --git a/test/integration/testlib/util.py b/test/integration/testlib/util.py index f0849b95..344958e3 100755 --- a/test/integration/testlib/util.py +++ b/test/integration/testlib/util.py @@ -7,6 +7,7 @@ import random import string import socket import typing as T +import tempfile from pathlib import Path from . import check @@ -32,6 +33,14 @@ def random_port() -> int: log.debug("could not bind to random port %d", port, exc_info=ex) +def temp_file(content: str) -> str: + """Create a temporary file and write text content into it.""" + file = tempfile.mktemp() + with open(file, "w", encoding="utf-8") as f: + f.write(content) + return file + + def remove_file(path: T.Union[str, Path]) -> bool: """Try to remove file without failing if it does not exist.""" try: diff --git a/test/unit/test_proxy.c b/test/unit/test_proxy.c index a5b30bd1..affbd3b5 100644 --- a/test/unit/test_proxy.c +++ b/test/unit/test_proxy.c @@ -19,6 +19,9 @@ static int teardown(void **state) { free(proxypass); proxypass = NULL; + free(proxyhost); + proxyhost = NULL; + return 0; } diff --git a/test/unit/test_utils.c b/test/unit/test_utils.c index a66541e7..51fc83ae 100644 --- a/test/unit/test_utils.c +++ b/test/unit/test_utils.c @@ -105,6 +105,19 @@ static int setup_path_unix(void **state) { return 0; } +static void test_string_eq(void **state) { + (void)state; + + assert_true(string_eq(NULL, NULL)); + assert_true(string_eq("", "")); + assert_true(string_eq("\tfoo 123", "\tfoo 123")); + + assert_false(string_eq(NULL, "")); + assert_false(string_eq("", NULL)); + assert_false(string_eq("foo", "FOO")); + assert_false(string_eq("foo", " foo")); +} + int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test_setup(test_unix_absolute_path_on_absolute_returns_it, setup_path_unix), @@ -117,6 +130,7 @@ int main(void) { cmocka_unit_test(test_is_decimal_pass_simple), cmocka_unit_test(test_is_decimal_pass_signs), cmocka_unit_test(test_is_decimal_pass_whitespace_prefix), + cmocka_unit_test(test_string_eq), }; #ifdef HAVE_WINDOWS -- 2.20.1