Hello all,
I am attaching a new patch which takes care of few things discussed yesterday.
Summary of patch:
1) Add new option --script-dir which restricts any user defined script to run
only from specific directory
2) Backward compatible. If script-dir is not specified then it allows script
from any directory.
3) Adds compile time configure option --with-script-dir=/some/path/
If this option is used then script-dir becomes hard-coded and CAN NOT be
changed from config file or command line.
This allows flexibility to anyone (people like me) who wants to compile the
code on their own making sure that script-dir can not be changed at all.
4) If it is not enabled at compile time then first script-dir has preference.
a) if script-dir is specified on command line, it is given the priority
b) if script-dir is specified twice (either in same config file or included
config file) then only 1st occurrence is accepted and warning is logged for the
rest of the occurrences.
This allows, easy binary distribution (by not hard-coding the script-dir) and
then user can decide on their own script-dir.
Root user can then call OpenVPN with script-dir either on command line or
inside parent config file. Parent config file can then call child config
(config child.conf). And then "freely" give access to child.conf to other lower
admins without worrying about them running any random script.
Even if lower admins specify script-dir, it will be ignored.
Hope that this patch now satisfy everyone in devel group and makes much more
sense to be implemented.
Security in my opinion should be prime concern especially when we know that
there is a way to run any random script. And hence atleast for such insecure
options, sanity checks has to be there in program itself instead of trusting
the frontend.
Patch is clean and simple and just about 25 lines of real code addition.
Patch eliminates danger of openvpn running any script blindly.
So please review it and consider to merge in source tree.
Thank you
AMM.
--- openvpn-2.2.2.old/config.h.in 2011-12-14 16:51:49.000000000 +0530
+++ openvpn-2.2.2/config.h.in 2012-08-24 21:32:01.843738238 +0530
@@ -468,6 +468,9 @@
/* Path to route tool */
#undef ROUTE_PATH
+/* Path to script directory */
+#undef SCRIPTDIR_PATH
+
/* The size of `unsigned int', as computed by sizeof. */
#undef SIZEOF_UNSIGNED_INT
--- openvpn-2.2.2.old/configure 2011-12-14 16:51:11.000000000 +0530
+++ openvpn-2.2.2/configure 2012-08-24 21:32:01.846738307 +0530
@@ -734,6 +734,7 @@
with_pkcs11_helper_headers
with_pkcs11_helper_lib
with_ifconfig_path
+with_script_dir
with_iproute_path
with_route_path
with_netstat_path
@@ -1409,6 +1410,7 @@
--with-pkcs11-helper-headers=DIR pkcs11-helper Include files location
--with-pkcs11-helper-lib=DIR pkcs11-helper Library location
--with-ifconfig-path=PATH Path to ifconfig tool
+ --with-script-dir=PATH Path to script dir
--with-iproute-path=PATH Path to iproute tool
--with-route-path=PATH Path to route tool
--with-netstat-path=PATH Path to netstat tool
@@ -3400,6 +3402,18 @@
+# Check whether --with-script-dir was given.
+if test "${with_script_dir+set}" = set; then :
+ withval=$with_script_dir; test -n $withval &&
+cat >>confdefs.h <<_ACEOF
+#define SCRIPTDIR_PATH "$withval"
+_ACEOF
+
+
+fi
+
+
+
# Check whether --with-iproute-path was given.
if test "${with_iproute_path+set}" = set; then :
withval=$with_iproute_path; IPROUTE="$withval"
--- openvpn-2.2.2.old/configure.ac 2011-12-13 22:28:56.000000000 +0530
+++ openvpn-2.2.2/configure.ac 2012-08-24 21:32:02.027742387 +0530
@@ -257,6 +257,11 @@
)
AC_DEFINE_UNQUOTED(IFCONFIG_PATH, "$IFCONFIG", [Path to ifconfig tool])
+AC_ARG_WITH(script-dir,
+ [ --with-script-dir=PATH Path to script dir],
+ test -n $withval && AC_DEFINE_UNQUOTED(SCRIPTDIR_PATH, "$withval", [Path to script directory])
+)
+
AC_ARG_WITH(iproute-path,
[ --with-iproute-path=PATH Path to iproute tool],
[IPROUTE="$withval"],
--- openvpn-2.2.2.old/init.c 2011-12-13 22:28:56.000000000 +0530
+++ openvpn-2.2.2/init.c 2012-08-24 21:32:02.028742410 +0530
@@ -2291,9 +2291,14 @@
#endif
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)
- msg (M_WARN, "WARNING: the current --script-security setting may allow passwords to be passed to scripts via environmental variables");
+ {
+ msg (M_WARN, "NOTE: the current --script-security setting may allow this configuration to call user-defined scripts");
+ 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");
+ if (script_dir == NULL)
+ msg (M_WARN, "WARNING: setting --script-dir is recommended for higher security");
+ else msg (M_WARN, "NOTE: user-defined scripts will be run only from %s", script_dir);
+ }
else
msg (M_WARN, "NOTE: " PACKAGE_NAME " 2.1 requires '--script-security 2' or higher to call user-defined scripts or executables");
--- openvpn-2.2.2.old/misc.c 2011-12-13 22:28:56.000000000 +0530
+++ openvpn-2.2.2/misc.c 2012-08-24 21:32:02.029742433 +0530
@@ -45,6 +45,12 @@
/* contains an SSEC_x value defined in misc.h */
int script_security = SSEC_BUILT_IN; /* GLOBAL */
+#ifdef SCRIPTDIR_PATH
+const char *script_dir = SCRIPTDIR_PATH; /* GLOBAL */
+#else
+#warning use of --with-script-dir=PATH is recommended
+const char *script_dir = NULL; /* GLOBAL */
+#endif
/* contains SM_x value defined in misc.h */
int script_method = SM_EXECVE; /* GLOBAL */
@@ -484,10 +490,14 @@
}
bool
-openvpn_execve_allowed (const unsigned int flags)
+openvpn_execve_allowed (const char *cmd, const unsigned int flags)
{
if (flags & S_SCRIPT)
- return script_security >= SSEC_SCRIPTS;
+ {
+ if (script_dir && script_dir[0] && strncmp(script_dir, cmd, strlen(script_dir)))
+ return false;
+ return script_security >= SSEC_SCRIPTS;
+ }
else
return script_security >= SSEC_BUILT_IN;
}
@@ -509,11 +519,11 @@
if (a && a->argv[0])
{
#if defined(ENABLE_EXECVE)
- if (openvpn_execve_allowed (flags))
+ const char *cmd = a->argv[0];
+ if (openvpn_execve_allowed (cmd, flags))
{
if (script_method == SM_EXECVE)
{
- 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;
--- openvpn-2.2.2.old/misc.h 2011-12-13 22:28:56.000000000 +0530
+++ openvpn-2.2.2/misc.h 2012-08-24 21:32:02.030742455 +0530
@@ -133,7 +133,7 @@
/* wrapper around the execve() call */
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);
+bool openvpn_execve_allowed (const char *cmd, const unsigned int flags);
int openvpn_system (const char *command, const struct env_set *es, unsigned int flags);
static inline bool
@@ -353,6 +353,7 @@
#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 */
+extern const char *script_dir; /* GLOBAL */
#define SM_EXECVE 0 /* call external programs with execve() or CreateProcess() */
#define SM_SYSTEM 1 /* call external programs with system() */
--- openvpn-2.2.2.old/options.c 2011-12-13 22:28:56.000000000 +0530
+++ openvpn-2.2.2/options.c 2012-08-24 21:47:42.068931155 +0530
@@ -218,6 +218,7 @@
" 1 -- (default) only call built-ins such as ifconfig\n"
" 2 -- allow calling of built-ins and scripts\n"
" 3 -- allow password to be passed to scripts via env\n"
+ "--script-dir dir: Only run user-defined script if it resides in this directory\n"
"--shaper n : Restrict output to peer to n bytes per second.\n"
"--keepalive n m : Helper option for setting timeouts in server mode. Send\n"
" ping once every n seconds, restart if ping not received\n"
@@ -4748,6 +4749,14 @@
else
script_method = SM_EXECVE;
}
+ else if (streq (p[0], "script-dir") && p[1])
+ {
+ VERIFY_PERMISSION (OPT_P_GENERAL);
+ if (script_dir)
+ msg (M_WARN, "--script-dir already set, ignoring new value: %s", p[1]);
+ else
+ script_dir = p[1];
+ }
else if (streq (p[0], "mssfix"))
{
VERIFY_PERMISSION (OPT_P_GENERAL);
--- openvpn-2.2.2.old/win32.c 2011-12-13 22:28:56.000000000 +0530
+++ openvpn-2.2.2/win32.c 2012-08-24 21:32:02.034742544 +0530
@@ -956,7 +956,8 @@
if (a && a->argv[0])
{
- if (openvpn_execve_allowed (flags))
+ char *cmd = a->argv[0];
+ if (openvpn_execve_allowed (cmd, flags))
{
if (script_method == SM_EXECVE)
{
@@ -965,7 +966,6 @@
char *env = env_block (es);
char *cl = cmd_line (a);
- char *cmd = a->argv[0];
CLEAR (start_info);
CLEAR (proc_info);
