From patchwork Fri Dec 8 01:07:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 142 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director5.mail.ord1d.rsapps.net ([172.27.255.59]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id E8QMC+eAKlqaUAAAgoeIoA for ; Fri, 08 Dec 2017 07:09:11 -0500 Received: from proxy2.mail.iad3a.rsapps.net ([172.27.255.59]) by director5.mail.ord1d.rsapps.net (Dovecot) with LMTP id 09jaA+eAKlo8fgAAsdCWiw ; Fri, 08 Dec 2017 07:09:11 -0500 Received: from smtp1.gate.iad3a ([172.27.255.59]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy2.mail.iad3a.rsapps.net (Dovecot) with LMTP id CKZpAeeAKlqgQwAABcWvHw ; Fri, 08 Dec 2017 07:09:11 -0500 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.34.181.88] Authentication-Results: smtp1.gate.iad3a.rsapps.net; iprev=pass policy.iprev="216.34.181.88"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=fox-it.com X-Classification-ID: 96558dc4-dc10-11e7-a697-52540091dea5-1-1 Received: from [216.34.181.88] ([216.34.181.88:29618] helo=lists.sourceforge.net) by smtp1.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 66/7C-04923-2E08A2A5; Fri, 08 Dec 2017 07:09:06 -0500 Received: from localhost ([127.0.0.1] helo=sfs-ml-3.v29.ch3.sourceforge.com) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.89) (envelope-from ) id 1eNHS5-0001Vx-MM; Fri, 08 Dec 2017 12:08:25 +0000 Received: from sfi-mx-4.v28.ch3.sourceforge.com ([172.29.28.194] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1eNHS4-0001Va-LO for openvpn-devel@lists.sourceforge.net; Fri, 08 Dec 2017 12:08:24 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:CC:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=KOZOQma0XTZ9kytnnk6O6Y0bn+bugZLWuHv0wwoXVTA=; b=GodoZ2lSt56VPKD5VJIH3ygV8/ Kkq5ag6pg0j4HO7tjDpyjx/bS/HLLYE65SpVVF/nntric5IlVeYy3EyYF1sOlie9DpubzjkcIEJNR pKZP2/aiuPyeUjbIpkkP+se9rsvte5iWUAxndeTqhYc277dzzXj57JjljV8iauPl1PSY=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject: CC:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=KOZOQma0XTZ9kytnnk6O6Y0bn+bugZLWuHv0wwoXVTA=; b=iKz+D/IuKQo6NYuOV5Uu5/7ZwQ rrb2xNM8T+4HzXiU1do5iLkpyEzoRWHqb/qiUIxqEd5mBbmT/Nd3Hjnr6/trnfBy1HSdsg+BoVN4A oPorGgyqiPKrPOzN156/P3mKUCS+4XaDUbScAUzsaE/GM8PPHUip3Vv+xfxsikIB5h5I=; Received: from ns2.fox-it.com ([178.250.144.131]) by sfi-mx-4.v28.ch3.sourceforge.com with esmtps (TLSv1:ECDHE-RSA-AES256-SHA:256) (Exim 4.89) id 1eNHS1-0000WL-Os for openvpn-devel@lists.sourceforge.net; Fri, 08 Dec 2017 12:08:24 +0000 Received: from FOXDFT52.FOX.local (unknown [10.0.0.129]) by ns2.fox-it.com (Postfix) with ESMTPS id 300EA1C5247; Fri, 8 Dec 2017 13:08:08 +0100 (CET) Received: from steffan-fox.fox.local (172.16.5.166) by FOXDFT52.FOX.local (10.0.0.129) with Microsoft SMTP Server (TLS) id 15.0.1293.2; Fri, 8 Dec 2017 13:08:07 +0100 From: Steffan Karger To: Date: Fri, 8 Dec 2017 13:07:49 +0100 Message-ID: <1512734870-17133-10-git-send-email-steffan.karger@fox-it.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1512734870-17133-1-git-send-email-steffan.karger@fox-it.com> References: <1512734870-17133-1-git-send-email-steffan.karger@fox-it.com> MIME-Version: 1.0 X-ClientProxiedBy: FOXDFT52.FOX.local (10.0.0.129) To FOXDFT52.FOX.local (10.0.0.129) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -0.0 SPF_PASS SPF: sender matches SPF record X-Headers-End: 1eNHS1-0000WL-Os Subject: [Openvpn-devel] [PATCH 09/10] Move execve/run_script helper functions to run_command.c X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Steffan Karger To avoid having to include misc.c - which is a dependency mess - in the tls-crypt unit tests, move the command execution helper functions to a new run_command.c module. While at it, abstract away the script_security global variable. Signed-off-by: Steffan Karger --- src/openvpn/Makefile.am | 1 + src/openvpn/env_set.c | 6 +- src/openvpn/init.c | 5 +- src/openvpn/lladdr.c | 1 + src/openvpn/misc.c | 218 --------------------------- src/openvpn/misc.h | 32 ---- src/openvpn/multi.c | 2 +- src/openvpn/options.c | 3 +- src/openvpn/route.c | 2 +- src/openvpn/run_command.c | 267 +++++++++++++++++++++++++++++++++ src/openvpn/run_command.h | 63 ++++++++ src/openvpn/socket.c | 1 + src/openvpn/ssl_verify.c | 4 +- src/openvpn/tls_crypt.c | 1 + src/openvpn/tun.c | 2 +- src/openvpn/win32.c | 4 +- tests/unit_tests/openvpn/Makefile.am | 3 +- tests/unit_tests/openvpn/test_crypto.c | 2 - 18 files changed, 351 insertions(+), 266 deletions(-) create mode 100644 src/openvpn/run_command.c create mode 100644 src/openvpn/run_command.h diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index 441700a..86bb40a 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -102,6 +102,7 @@ openvpn_SOURCES = \ pushlist.h \ reliable.c reliable.h \ route.c route.h \ + run_command.c run_command.h \ schedule.c schedule.h \ session_id.c session_id.h \ shaper.c shaper.h \ diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c index 2a22e0b..4a9de1a 100644 --- a/src/openvpn/env_set.c +++ b/src/openvpn/env_set.c @@ -32,10 +32,10 @@ #include "syshead.h" -#include "misc.h" - #include "env_set.h" +#include "run_command.h" + /* * Set environmental variable (int or string). * @@ -414,7 +414,7 @@ setenv_str_i(struct env_set *es, const char *name, const char *value, const int bool env_allowed(const char *str) { - return (script_security >= SSEC_PW_ENV || !is_password_env_var(str)); + return (script_security() >= SSEC_PW_ENV || !is_password_env_var(str)); } /* Make arrays of strings */ diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 4055f28..93143b1 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -35,6 +35,7 @@ #include "win32.h" #include "init.h" +#include "run_command.h" #include "sig.h" #include "occ.h" #include "list.h" @@ -3153,11 +3154,11 @@ do_option_warnings(struct context *c) /* If a script is used, print appropiate warnings */ if (o->user_script_used) { - if (script_security >= SSEC_SCRIPTS) + if (script_security() >= SSEC_SCRIPTS) { msg(M_WARN, "NOTE: the current --script-security setting may allow this configuration to call user-defined scripts"); } - else if (script_security >= SSEC_PW_ENV) + else if (script_security() >= SSEC_PW_ENV) { msg(M_WARN, "WARNING: the current --script-security setting may allow passwords to be passed to scripts via environmental variables"); } diff --git a/src/openvpn/lladdr.c b/src/openvpn/lladdr.c index ff71e48..f24596b 100644 --- a/src/openvpn/lladdr.c +++ b/src/openvpn/lladdr.c @@ -11,6 +11,7 @@ #include "syshead.h" #include "error.h" #include "misc.h" +#include "run_command.h" int set_lladdr(const char *ifname, const char *lladdr, diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index b04a9d7..b2c5626 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -51,9 +51,6 @@ const char *iproute_path = IPROUTE_PATH; /* GLOBAL */ #endif -/* contains an SSEC_x value defined in misc.h */ -int script_security = SSEC_BUILT_IN; /* GLOBAL */ - /* * Set standard file descriptors to /dev/null */ @@ -99,221 +96,6 @@ save_inetd_socket_descriptor(void) } /* - * Print an error message based on the status code returned by system(). - */ -const char * -system_error_message(int stat, struct gc_arena *gc) -{ - struct buffer out = alloc_buf_gc(256, gc); -#ifdef _WIN32 - if (stat == -1) - { - buf_printf(&out, "external program did not execute -- "); - } - buf_printf(&out, "returned error code %d", stat); -#else /* ifdef _WIN32 */ - if (stat == -1) - { - buf_printf(&out, "external program fork failed"); - } - else if (!WIFEXITED(stat)) - { - buf_printf(&out, "external program did not exit normally"); - } - else - { - const int cmd_ret = WEXITSTATUS(stat); - if (!cmd_ret) - { - buf_printf(&out, "external program exited normally"); - } - else if (cmd_ret == 127) - { - buf_printf(&out, "could not execute external program"); - } - else - { - buf_printf(&out, "external program exited with error status: %d", cmd_ret); - } - } -#endif /* ifdef _WIN32 */ - return (const char *)out.data; -} - -/* - * Wrapper around openvpn_execve - */ -bool -openvpn_execve_check(const struct argv *a, const struct env_set *es, const unsigned int flags, const char *error_message) -{ - struct gc_arena gc = gc_new(); - const int stat = openvpn_execve(a, es, flags); - int ret = false; - - if (platform_system_ok(stat)) - { - ret = true; - } - else - { - if (error_message) - { - msg(((flags & S_FATAL) ? M_FATAL : M_WARN), "%s: %s", - error_message, - system_error_message(stat, &gc)); - } - } - gc_free(&gc); - return ret; -} - -bool -openvpn_execve_allowed(const unsigned int flags) -{ - if (flags & S_SCRIPT) - { - return script_security >= SSEC_SCRIPTS; - } - else - { - return script_security >= SSEC_BUILT_IN; - } -} - - -#ifndef _WIN32 -/* - * Run execve() inside a fork(). Designed to replicate the semantics of system() but - * in a safer way that doesn't require the invocation of a shell or the risks - * assocated with formatting and parsing a command line. - */ -int -openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned int flags) -{ - struct gc_arena gc = gc_new(); - int ret = -1; - static bool warn_shown = false; - - if (a && a->argv[0]) - { -#if defined(ENABLE_FEATURE_EXECVE) - if (openvpn_execve_allowed(flags)) - { - const char *cmd = a->argv[0]; - char *const *argv = a->argv; - char *const *envp = (char *const *)make_env_array(es, true, &gc); - pid_t pid; - - pid = fork(); - if (pid == (pid_t)0) /* child side */ - { - execve(cmd, argv, envp); - exit(127); - } - else if (pid < (pid_t)0) /* fork failed */ - { - msg(M_ERR, "openvpn_execve: unable to fork"); - } - else /* parent side */ - { - if (waitpid(pid, &ret, 0) != pid) - { - ret = -1; - } - } - } - else if (!warn_shown && (script_security < SSEC_SCRIPTS)) - { - msg(M_WARN, SCRIPT_SECURITY_WARNING); - warn_shown = true; - } -#else /* if defined(ENABLE_FEATURE_EXECVE) */ - msg(M_WARN, "openvpn_execve: execve function not available"); -#endif /* if defined(ENABLE_FEATURE_EXECVE) */ - } - else - { - msg(M_FATAL, "openvpn_execve: called with empty argv"); - } - - gc_free(&gc); - return ret; -} -#endif /* ifndef _WIN32 */ - -/* - * Run execve() inside a fork(), duping stdout. Designed to replicate the semantics of popen() but - * in a safer way that doesn't require the invocation of a shell or the risks - * assocated with formatting and parsing a command line. - */ -int -openvpn_popen(const struct argv *a, const struct env_set *es) -{ - struct gc_arena gc = gc_new(); - int ret = -1; - static bool warn_shown = false; - - if (a && a->argv[0]) - { -#if defined(ENABLE_FEATURE_EXECVE) - if (script_security >= SSEC_BUILT_IN) - { - const char *cmd = a->argv[0]; - char *const *argv = a->argv; - char *const *envp = (char *const *)make_env_array(es, true, &gc); - pid_t pid; - int pipe_stdout[2]; - - if (pipe(pipe_stdout) == 0) - { - pid = fork(); - if (pid == (pid_t)0) /* child side */ - { - close(pipe_stdout[0]); /* Close read end */ - dup2(pipe_stdout[1],1); - execve(cmd, argv, envp); - exit(127); - } - else if (pid > (pid_t)0) /* parent side */ - { - int status = 0; - - close(pipe_stdout[1]); /* Close write end */ - waitpid(pid, &status, 0); - ret = pipe_stdout[0]; - } - else /* fork failed */ - { - close(pipe_stdout[0]); - close(pipe_stdout[1]); - msg(M_ERR, "openvpn_popen: unable to fork %s", cmd); - } - } - else - { - msg(M_WARN, "openvpn_popen: unable to create stdout pipe for %s", cmd); - ret = -1; - } - } - else if (!warn_shown && (script_security < SSEC_SCRIPTS)) - { - msg(M_WARN, SCRIPT_SECURITY_WARNING); - warn_shown = true; - } -#else /* if defined(ENABLE_FEATURE_EXECVE) */ - msg(M_WARN, "openvpn_popen: execve function not available"); -#endif /* if defined(ENABLE_FEATURE_EXECVE) */ - } - else - { - msg(M_FATAL, "openvpn_popen: called with empty argv"); - } - - gc_free(&gc); - return ret; -} - -/* * Prepend a random string to hostname to prevent DNS caching. * For example, foo.bar.gov would be modified to .foo.bar.gov. * Of course, this requires explicit support in the DNS server (wildcard). diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index 82bff9a..491906b 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -38,30 +38,6 @@ /* forward declarations */ struct plugin_list; -/* system flags */ -#define S_SCRIPT (1<<0) -#define S_FATAL (1<<1) - -const char *system_error_message(int, struct gc_arena *gc); - -/* wrapper around the execve() call */ -int openvpn_popen(const struct argv *a, const struct env_set *es); - -int openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned int flags); - -bool openvpn_execve_check(const struct argv *a, const struct env_set *es, const unsigned int flags, const char *error_message); - -bool openvpn_execve_allowed(const unsigned int flags); - -static inline bool -openvpn_run_script(const struct argv *a, const struct env_set *es, const unsigned int flags, const char *hook) -{ - char msg[256]; - - openvpn_snprintf(msg, sizeof(msg), "WARNING: Failed running command (%s)", hook); - return openvpn_execve_check(a, es, flags | S_SCRIPT, msg); -} - /* Set standard file descriptors to /dev/null */ void set_std_files_to_null(bool stdin_only); @@ -198,14 +174,6 @@ void get_user_pass_auto_userid(struct user_pass *up, const char *tag); extern const char *iproute_path; #endif -/* Script security */ -#define SSEC_NONE 0 /* strictly no calling of external programs */ -#define SSEC_BUILT_IN 1 /* only call built-in programs such as ifconfig, route, netsh, etc.*/ -#define SSEC_SCRIPTS 2 /* allow calling of built-in programs and user-defined scripts */ -#define SSEC_PW_ENV 3 /* allow calling of built-in programs and user-defined scripts that may receive a password as an environmental variable */ -extern int script_security; /* GLOBAL */ - - #define COMPAT_FLAG_QUERY 0 /** compat_flags operator: Query for a flag */ #define COMPAT_FLAG_SET (1<<0) /** compat_flags operator: Set a compat flag */ #define COMPAT_NAMES (1<<1) /** compat flag: --compat-names set */ diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index acc6912..2818b9a 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -38,7 +38,7 @@ #include "multi.h" #include "push.h" -#include "misc.h" +#include "run_command.h" #include "otime.h" #include "gremlin.h" #include "mstats.h" diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 89bff1e..2d577f8 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -41,6 +41,7 @@ #include "buffer.h" #include "error.h" #include "common.h" +#include "run_command.h" #include "shaper.h" #include "crypto.h" #include "ssl.h" @@ -6386,7 +6387,7 @@ add_option(struct options *options, else if (streq(p[0], "script-security") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); - script_security = atoi(p[1]); + script_security_set(atoi(p[1])); } else if (streq(p[0], "mssfix") && !p[2]) { diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 8c71e6e..955e15f 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -36,7 +36,7 @@ #include "common.h" #include "error.h" #include "route.h" -#include "misc.h" +#include "run_command.h" #include "socket.h" #include "manage.h" #include "win32.h" diff --git a/src/openvpn/run_command.c b/src/openvpn/run_command.c new file mode 100644 index 0000000..4e19867 --- /dev/null +++ b/src/openvpn/run_command.c @@ -0,0 +1,267 @@ +/* + * OpenVPN -- An application to securely tunnel IP networks + * over a single TCP/UDP port, with support for SSL/TLS-based + * session authentication and key exchange, + * packet encryption, packet authentication, and + * packet compression. + * + * Copyright (C) 2002-2017 OpenVPN Technologies, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#elif defined(_MSC_VER) +#include "config-msvc.h" +#endif + +#include "syshead.h" + +#include "buffer.h" +#include "error.h" +#include "platform.h" +#include "win32.h" + +#include "memdbg.h" + +#include "run_command.h" + +/* contains an SSEC_x value defined in platform.h */ +static int script_security_level = SSEC_BUILT_IN; /* GLOBAL */ + +int script_security(void) +{ + return script_security_level; +} + +void script_security_set(int level) +{ + script_security_level = level; +} + +/* + * Print an error message based on the status code returned by system(). + */ +static const char * +system_error_message(int stat, struct gc_arena *gc) +{ + struct buffer out = alloc_buf_gc(256, gc); +#ifdef _WIN32 + if (stat == -1) + { + buf_printf(&out, "external program did not execute -- "); + } + buf_printf(&out, "returned error code %d", stat); +#else /* ifdef _WIN32 */ + if (stat == -1) + { + buf_printf(&out, "external program fork failed"); + } + else if (!WIFEXITED(stat)) + { + buf_printf(&out, "external program did not exit normally"); + } + else + { + const int cmd_ret = WEXITSTATUS(stat); + if (!cmd_ret) + { + buf_printf(&out, "external program exited normally"); + } + else if (cmd_ret == 127) + { + buf_printf(&out, "could not execute external program"); + } + else + { + buf_printf(&out, "external program exited with error status: %d", cmd_ret); + } + } +#endif /* ifdef _WIN32 */ + return (const char *)out.data; +} + +bool +openvpn_execve_allowed(const unsigned int flags) +{ + if (flags & S_SCRIPT) + { + return script_security() >= SSEC_SCRIPTS; + } + else + { + return script_security() >= SSEC_BUILT_IN; + } +} + + +#ifndef _WIN32 +/* + * Run execve() inside a fork(). Designed to replicate the semantics of system() but + * in a safer way that doesn't require the invocation of a shell or the risks + * assocated with formatting and parsing a command line. + */ +int +openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned int flags) +{ + struct gc_arena gc = gc_new(); + int ret = -1; + static bool warn_shown = false; + + if (a && a->argv[0]) + { +#if defined(ENABLE_FEATURE_EXECVE) + if (openvpn_execve_allowed(flags)) + { + const char *cmd = a->argv[0]; + char *const *argv = a->argv; + char *const *envp = (char *const *)make_env_array(es, true, &gc); + pid_t pid; + + pid = fork(); + if (pid == (pid_t)0) /* child side */ + { + execve(cmd, argv, envp); + exit(127); + } + else if (pid < (pid_t)0) /* fork failed */ + { + msg(M_ERR, "openvpn_execve: unable to fork"); + } + else /* parent side */ + { + if (waitpid(pid, &ret, 0) != pid) + { + ret = -1; + } + } + } + else if (!warn_shown && (script_security() < SSEC_SCRIPTS)) + { + msg(M_WARN, SCRIPT_SECURITY_WARNING); + warn_shown = true; + } +#else /* if defined(ENABLE_FEATURE_EXECVE) */ + msg(M_WARN, "openvpn_execve: execve function not available"); +#endif /* if defined(ENABLE_FEATURE_EXECVE) */ + } + else + { + msg(M_FATAL, "openvpn_execve: called with empty argv"); + } + + gc_free(&gc); + return ret; +} +#endif /* ifndef _WIN32 */ + +/* + * Wrapper around openvpn_execve + */ +bool +openvpn_execve_check(const struct argv *a, const struct env_set *es, const unsigned int flags, const char *error_message) +{ + struct gc_arena gc = gc_new(); + const int stat = openvpn_execve(a, es, flags); + int ret = false; + + if (platform_system_ok(stat)) + { + ret = true; + } + else + { + if (error_message) + { + msg(((flags & S_FATAL) ? M_FATAL : M_WARN), "%s: %s", + error_message, + system_error_message(stat, &gc)); + } + } + gc_free(&gc); + return ret; +} + +/* + * Run execve() inside a fork(), duping stdout. Designed to replicate the semantics of popen() but + * in a safer way that doesn't require the invocation of a shell or the risks + * assocated with formatting and parsing a command line. + */ +int +openvpn_popen(const struct argv *a, const struct env_set *es) +{ + struct gc_arena gc = gc_new(); + int ret = -1; + static bool warn_shown = false; + + if (a && a->argv[0]) + { +#if defined(ENABLE_FEATURE_EXECVE) + if (script_security() >= SSEC_BUILT_IN) + { + const char *cmd = a->argv[0]; + char *const *argv = a->argv; + char *const *envp = (char *const *)make_env_array(es, true, &gc); + pid_t pid; + int pipe_stdout[2]; + + if (pipe(pipe_stdout) == 0) + { + pid = fork(); + if (pid == (pid_t)0) /* child side */ + { + close(pipe_stdout[0]); /* Close read end */ + dup2(pipe_stdout[1],1); + execve(cmd, argv, envp); + exit(127); + } + else if (pid > (pid_t)0) /* parent side */ + { + int status = 0; + + close(pipe_stdout[1]); /* Close write end */ + waitpid(pid, &status, 0); + ret = pipe_stdout[0]; + } + else /* fork failed */ + { + close(pipe_stdout[0]); + close(pipe_stdout[1]); + msg(M_ERR, "openvpn_popen: unable to fork %s", cmd); + } + } + else + { + msg(M_WARN, "openvpn_popen: unable to create stdout pipe for %s", cmd); + ret = -1; + } + } + else if (!warn_shown && (script_security() < SSEC_SCRIPTS)) + { + msg(M_WARN, SCRIPT_SECURITY_WARNING); + warn_shown = true; + } +#else /* if defined(ENABLE_FEATURE_EXECVE) */ + msg(M_WARN, "openvpn_popen: execve function not available"); +#endif /* if defined(ENABLE_FEATURE_EXECVE) */ + } + else + { + msg(M_FATAL, "openvpn_popen: called with empty argv"); + } + + gc_free(&gc); + return ret; +} diff --git a/src/openvpn/run_command.h b/src/openvpn/run_command.h new file mode 100644 index 0000000..4501a5c --- /dev/null +++ b/src/openvpn/run_command.h @@ -0,0 +1,63 @@ +/* + * OpenVPN -- An application to securely tunnel IP networks + * over a single TCP/UDP port, with support for SSL/TLS-based + * session authentication and key exchange, + * packet encryption, packet authentication, and + * packet compression. + * + * Copyright (C) 2002-2017 OpenVPN Technologies, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef RUN_COMMAND_H +#define RUN_COMMAND_H + +#include "basic.h" +#include "env_set.h" + +/* Script security */ +#define SSEC_NONE 0 /* strictly no calling of external programs */ +#define SSEC_BUILT_IN 1 /* only call built-in programs such as ifconfig, route, netsh, etc.*/ +#define SSEC_SCRIPTS 2 /* allow calling of built-in programs and user-defined scripts */ +#define SSEC_PW_ENV 3 /* allow calling of built-in programs and user-defined scripts that may receive a password as an environmental variable */ + +int script_security(void); + +void script_security_set(int level); + +/* openvpn_execve flags */ +#define S_SCRIPT (1<<0) +#define S_FATAL (1<<1) + +/* wrapper around the execve() call */ +int openvpn_popen(const struct argv *a, const struct env_set *es); + +bool openvpn_execve_allowed(const unsigned int flags); + +bool openvpn_execve_check(const struct argv *a, const struct env_set *es, + const unsigned int flags, const char *error_message); + +static inline bool +openvpn_run_script(const struct argv *a, const struct env_set *es, + const unsigned int flags, const char *hook) +{ + char msg[256]; + + openvpn_snprintf(msg, sizeof(msg), + "WARNING: Failed running command (%s)", hook); + return openvpn_execve_check(a, es, flags | S_SCRIPT, msg); +} + +#endif /* ifndef RUN_COMMAND_H */ diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 0fc91f2..e7126da 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -35,6 +35,7 @@ #include "gremlin.h" #include "plugin.h" #include "ps.h" +#include "run_command.h" #include "manage.h" #include "misc.h" #include "manage.h" diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 3568bad..6e7b0b0 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -34,10 +34,10 @@ #include "syshead.h" -#include "misc.h" +#include "base64.h" #include "manage.h" #include "otime.h" -#include "base64.h" +#include "run_command.h" #include "ssl_verify.h" #include "ssl_verify_backend.h" diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 0c94d73..6fbe3e5 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -32,6 +32,7 @@ #include "base64.h" #include "crypto.h" #include "platform.h" +#include "run_command.h" #include "session_id.h" #include "ssl.h" diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 25831ce..6103163 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -40,7 +40,7 @@ #include "tun.h" #include "fdmisc.h" #include "common.h" -#include "misc.h" +#include "run_command.h" #include "socket.h" #include "manage.h" #include "route.h" diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index 95fea5d..1e5d871 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -39,9 +39,9 @@ #include "buffer.h" #include "error.h" #include "mtu.h" +#include "run_command.h" #include "sig.h" #include "win32.h" -#include "misc.h" #include "openvpn-msg.h" #include "memdbg.h" @@ -1137,7 +1137,7 @@ openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned in free(env); gc_free(&gc); } - else if (!exec_warn && (script_security < SSEC_SCRIPTS)) + else if (!exec_warn && (script_security() < SSEC_SCRIPTS)) { msg(M_WARN, SCRIPT_SECURITY_WARNING); exec_warn = true; diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index b69da84..a5d7f01 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -44,7 +44,8 @@ crypto_testdriver_SOURCES = test_crypto.c mock_msg.c \ $(openvpn_srcdir)/env_set.c \ $(openvpn_srcdir)/otime.c \ $(openvpn_srcdir)/packet_id.c \ - $(openvpn_srcdir)/platform.c + $(openvpn_srcdir)/platform.c \ + $(openvpn_srcdir)/run_command.c packet_id_testdriver_CFLAGS = @TEST_CFLAGS@ \ -I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \ diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c index 62d5b3f..b4ef668 100644 --- a/tests/unit_tests/openvpn/test_crypto.c +++ b/tests/unit_tests/openvpn/test_crypto.c @@ -41,8 +41,6 @@ #include "mock_msg.h" -int script_security = 0; /* Avoid including misc.c */ - static const char testtext[] = "Dummy text to test PEM encoding"; /**