From c4072a7e952dd66dcd1fa14e4fdbfa0754986dee Mon Sep 17 00:00:00 2001 From: Albert S Date: Sun, 3 Oct 2021 23:46:40 +0200 Subject: [PATCH] Sandbox: Remove multiple stages While interesitng in theory, there is nothing to be gained here, because we don't really have user input at those early stages. As we are also not a privileged process, those early stage sandboxes in the end are not worth it, since they increase complexity while there is no benefit in practise. So, reduce those 3 stages to a single one (enable()), which we activate after CLI server has launched. --- qswiki.cpp | 35 +++++++---------- sandbox/sandbox-linux.cpp | 82 +++++++-------------------------------- sandbox/sandbox-linux.h | 4 +- sandbox/sandbox-openbsd.h | 6 +-- sandbox/sandbox.h | 13 +------ 5 files changed, 30 insertions(+), 110 deletions(-) diff --git a/qswiki.cpp b/qswiki.cpp index f34a85c..bdb527d 100644 --- a/qswiki.cpp +++ b/qswiki.cpp @@ -123,29 +123,12 @@ int main(int argc, char **argv) return 1; } std::string configpath = std::filesystem::absolute(configfilepath).string(); - if(!sandbox->enableForInit()) - { - Logger::error() << "Sandboxing for init mode could not be activated."; - exit(EXIT_FAILURE); - } try { ConfigReader configreader(configpath); Config config = configreader.readConfig(); - // TODO: config.connectiontring only works as long as we only support sqlite of course - if(!sandbox->enablePreWorker({ - config.configVarResolver.getConfig("cache_fs_dir"), - config.templatepath, - std::filesystem::path(config.logfile).parent_path(), - std::filesystem::path(config.connectionstring).parent_path(), - })) - { - Logger::error() << "Sandboxing for pre worker stage could not be activated."; - exit(EXIT_FAILURE); - } - setup_signal_handlers(); std::fstream logstream; @@ -162,6 +145,19 @@ int main(int argc, char **argv) console.startInteractive(); return 0; } + + // TODO: config.connectiontring only works as long as we only support sqlite of course + if(!sandbox->enable({ + config.configVarResolver.getConfig("cache_fs_dir"), + config.templatepath, + std::filesystem::path(config.logfile).parent_path(), + std::filesystem::path(config.connectionstring).parent_path(), + })) + { + Logger::error() << "Sandboxing for worker could not be enabled!"; + exit(EXIT_FAILURE); + } + CLIServer cliServer{cliHandler}; if(!cliServer.detachServer(socketPath)) { @@ -195,11 +191,6 @@ int main(int argc, char **argv) auto interface = createGateway(config); - if(!sandbox->enableForWorker()) - { - Logger::error() << "Sandboxing for worker could not be enabled!"; - exit(EXIT_FAILURE); - } interface->work(requestWorker); } catch(const std::exception &e) diff --git a/sandbox/sandbox-linux.cpp b/sandbox/sandbox-linux.cpp index 6c2dff9..fef03be 100644 --- a/sandbox/sandbox-linux.cpp +++ b/sandbox/sandbox-linux.cpp @@ -23,68 +23,6 @@ * obvious systemcalls. To whitelist, we need to analyse our * dependencies (http library, sqlite wrapper, sqlite lib etc.) */ -bool SandboxLinux::enableForInit() -{ - umask(0027); - struct qssb_policy *policy = qssb_init_policy(); - if(policy == NULL) - { - Logger::error() << "Failed to init sandboxing policy (init)"; - return false; - } - policy->namespace_options = QSSB_UNSHARE_USER; - policy->drop_caps = 0; - qssb_append_syscall_policy(policy, QSSB_SYSCALL_DENY_KILL_PROCESS, QSSB_SYS(execveat)); - qssb_append_syscall_policy(policy, QSSB_SYSCALL_DENY_KILL_PROCESS, QSSB_SYS(execve)); - qssb_append_syscall_default_policy(policy, QSSB_SYSCALL_ALLOW); - - int result = qssb_enable_policy(policy); - if(result != 0) - { - Logger::error() << "Failed to enable sandboxing policy (init): " << result; - qssb_free_policy(policy); - return false; - } - qssb_free_policy(policy); - return true; -} - -bool SandboxLinux::enablePreWorker(std::vector fsPaths) -{ - std::sort(fsPaths.begin(), fsPaths.end(), - [](const std::string &a, const std::string &b) { return a.length() < b.length(); }); - - struct qssb_policy *policy = qssb_init_policy(); - if(policy == NULL) - { - Logger::error() << "Failed to init sandboxing policy (pre)"; - return false; - } - - for(unsigned int i = 0; i < fsPaths.size(); i++) - { - qssb_append_path_policy(policy, QSSB_FS_ALLOW_READ | QSSB_FS_ALLOW_WRITE, fsPaths[i].c_str()); - } - - policy->namespace_options = QSSB_UNSHARE_MOUNT; - policy->drop_caps = 0; - policy->mount_path_policies_to_chroot = 1; - - qssb_append_syscall_policy(policy, QSSB_SYSCALL_DENY_KILL_PROCESS, QSSB_SYS(execveat)); - qssb_append_syscall_policy(policy, QSSB_SYSCALL_DENY_KILL_PROCESS, QSSB_SYS(execve)); - qssb_append_syscall_default_policy(policy, QSSB_SYSCALL_ALLOW); - - int result = qssb_enable_policy(policy); - if(result != 0) - { - Logger::error() << "Failed to install sandboxing policy (preworker): %i" << result; - qssb_free_policy(policy); - return false; - } - qssb_free_policy(policy); - return true; -} - bool SandboxLinux::supported() { std::fstream stream; @@ -102,27 +40,33 @@ bool SandboxLinux::supported() } return true; } -bool SandboxLinux::enableForWorker() +bool SandboxLinux::enable(std::vector fsPaths) { + std::sort(fsPaths.begin(), fsPaths.end(), + [](const std::string &a, const std::string &b) { return a.length() < b.length(); }); + struct qssb_policy *policy = qssb_init_policy(); if(policy == NULL) { Logger::error() << "Failed to init sandboxing policy (worker) "; return false; } + for(unsigned int i = 0; i < fsPaths.size(); i++) + { + qssb_append_path_policy(policy, QSSB_FS_ALLOW_READ | QSSB_FS_ALLOW_WRITE, fsPaths[i].c_str()); + } policy->drop_caps = 1; policy->not_dumpable = 1; policy->no_new_privs = 1; - policy->namespace_options = 0; - + policy->mount_path_policies_to_chroot = 1; /* TODO: as said, a whitelist approach is better. As such, this list is bound to be incomplete in the * sense that more could be listed here and some critical ones are probably missing */ /* TODO: use qssb groups */ - long blacklisted_syscalls[] = {QSSB_SYS(setuid), QSSB_SYS(connect), QSSB_SYS(chroot), QSSB_SYS(pivot_root), - QSSB_SYS(mount), QSSB_SYS(setns), QSSB_SYS(unshare), QSSB_SYS(ptrace), - QSSB_SYS(personality), QSSB_SYS(prctl)}; - + long blacklisted_syscalls[] = {QSSB_SYS(setuid), QSSB_SYS(connect), QSSB_SYS(chroot), QSSB_SYS(pivot_root), + QSSB_SYS(mount), QSSB_SYS(setns), QSSB_SYS(unshare), QSSB_SYS(ptrace), + QSSB_SYS(personality), QSSB_SYS(prctl), QSSB_SYS(execveat), QSSB_SYS(execve), + QSSB_SYS(fork)}; qssb_append_syscalls_policy(policy, QSSB_SYSCALL_DENY_KILL_PROCESS, blacklisted_syscalls, sizeof(blacklisted_syscalls) / sizeof(blacklisted_syscalls[0])); qssb_append_syscall_default_policy(policy, QSSB_SYSCALL_ALLOW); diff --git a/sandbox/sandbox-linux.h b/sandbox/sandbox-linux.h index dbd22b7..2f91e4f 100644 --- a/sandbox/sandbox-linux.h +++ b/sandbox/sandbox-linux.h @@ -8,8 +8,6 @@ class SandboxLinux : public Sandbox public: using Sandbox::Sandbox; bool supported() override; - bool enableForInit() override; - bool enablePreWorker(std::vector fsPaths) override; - bool enableForWorker() override; + bool enable(std::vector fsPaths) override; }; #endif diff --git a/sandbox/sandbox-openbsd.h b/sandbox/sandbox-openbsd.h index f678567..49bc690 100644 --- a/sandbox/sandbox-openbsd.h +++ b/sandbox/sandbox-openbsd.h @@ -6,10 +6,6 @@ class SandboxOpenBSD : public Sandbox { public: bool supported() override; - bool enableForInit() override; - bool enableForWorker() override; - - private: - bool seccomp_blacklist(std::vector syscalls); + bool enable(std::vector fsPaths) override; }; #endif diff --git a/sandbox/sandbox.h b/sandbox/sandbox.h index eb7e00a..81f25fb 100644 --- a/sandbox/sandbox.h +++ b/sandbox/sandbox.h @@ -10,16 +10,7 @@ class Sandbox /* Whether the platform has everything required to active all sandbnox modes */ virtual bool supported() = 0; - /* Activated early. At this point, we need more system calls - * than later on */ - virtual bool enableForInit() = 0; - - /* Activated after config has been read. Now we now which paths we need access to */ - virtual bool enablePreWorker(std::vector fsPaths) = 0; - - /* Activated after we have acquired resources (bound to ports etc.) - * - * This should allow us to further restrcit the process */ - virtual bool enableForWorker() = 0; + /* Activated after we have acquired resources (bound to ports etc.)*/ + virtual bool enable(std::vector fsPaths) = 0; }; #endif