openvpn-devel Mailing List for OpenVPN

The OpenVPN community project team is proud to release OpenVPN 2.3_rc1. It can be downloaded from here: <http://openvpn.net/index.php/open-source/downloads.html> This release fixes a number of small issues. It also removes support for the "system" method for the "script-security" option, which requires changes to OpenVPN configuration in some cases; for details, look here: <https://community.openvpn.net/openvpn/ticket/228> In addition, an updated GUI is included in the Windows installer. <https://community.openvpn.net/openvpn/wiki/ChangesInOpenvpn23> A full list of new features and the changelog are available here: <https://community.openvpn.net/openvpn/wiki/ChangesInOpenvpn23> The changelog is also attached to this email. For generic help use these support channels: - Official documentation: <http://openvpn.net/index.php/open-source/documentation/howto.html> - Wiki: <https://community.openvpn.net> - Forums: <https://forums.openvpn.net> - User mailing list: <http://sourceforge.net/mail/?group_id=48978> - User IRC channel: #openvpn at irc.freenode.net Please report bugs and ask development questions here: - Bug tracker and Wiki: <https://community.openvpn.net> - Developer mailing list: <http://sourceforge.net/mail/?group_id=48978> - Developer IRC channel: #openvpn-devel at irc.freenode.net (requires Freenode registration) -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock 

On 29/10/12 20:35, David Sommerseth wrote: > From: David Sommerseth <da...@re...> > > This patch removes the support for the system() call, and enforces the usage of execve() > on the *nix platform and CreateProcessW() on Windows. This is to enhance the overall > security when calling external scripts. Using system() is prone to shell expansions, > which may lead to security breaches. Which is also why the execve() approach has > been the default since commit a82813527551f0e79c6d6ed5a9c1162e3c171bcf which > re-introduced the system() in Nov. 2008. > > After having asked on the mailing list and checked around on the IRC channels, the > genereal consensus is that very few uses system() these days. > > The only annoyance I've been made aware of is that this will now require adding a full > path to the script interpreter together with the script, and not just put in the > script name alone. But to just use the script name in Windows, you had to configure > --script-security with the 'system' flag earlier too. So my conclusion is that it's > better to add a full path to the script interpreter in Windows and raise the overal > security with OpenVPN, than to continue to have a possible potentially risky > OpenVPN configuration just to make life "easier" for Windows script users. > > Removal of the system() call, also solves a nasty bug related to the usage of putenv() > on the *nix platforms. > > For more information please see: > http://thread.gmane.org/gmane.network.openvpn.devel/7090 > https://community.openvpn.net/openvpn/ticket/228 > > Trac-ticket: 228 > Signed-off-by: David Sommerseth <da...@re...> > --- > doc/openvpn.8 | 48 ++++++++++++------ > src/openvpn/init.c | 3 -- > src/openvpn/misc.c | 98 ++++++++----------------------------- > src/openvpn/misc.h | 5 -- > src/openvpn/options.c | 16 +----- > src/openvpn/platform.c | 27 +--------- > src/openvpn/platform.h | 4 +- > src/openvpn/win32.c | 127 +++++++++++++----------------------------------- > 8 files changed, 89 insertions(+), 239 deletions(-) > Applied to master and beta/2.3 commit 0563473601abfbf2142bfa0ca5b863c5aa7953a2 (master) commit 3cb9f1a62b4a84dbf4acd1957c900a5b06fd6ac2 (beta/2.3) Author: David Sommerseth <da...@re...> Date: Thu Oct 25 14:22:30 2012 +0200 Remove the support for using system() when executing external programs or scripts Trac-ticket: 228 Signed-off-by: David Sommerseth <da...@re...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <135...@us...> URL: http://article.gmane.org/gmane.network.openvpn.devel/7114 (cherry picked from commit 0563473601abfbf2142bfa0ca5b863c5aa7953a2) kind regards, David Sommerseth 

Hi, On Mon, Oct 29, 2012 at 08:35:52PM +0100, David Sommerseth wrote: > From: David Sommerseth <da...@re...> > > This patch removes the support for the system() call, and enforces the usage of execve() ACK. (Did not test it, but reviewed the code. Relying on Samuli's test results for "how to run scripts with full cmd.exe invocation on windows") gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

I did some tests yesterday and this syntax works with arbitrary interpreters and scripts: script-security 2 up 'c:\\Windows\\System32\\wscript.exe c:\\Program\ Files\\OpenVPN\\config\\test.vbs' For test details, look here: <https://community.openvpn.net/openvpn/ticket/228#comment:4> -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock > From: David Sommerseth <da...@re...> > > This patch removes the support for the system() call, and enforces the usage of execve() > on the *nix platform and CreateProcessW() on Windows. This is to enhance the overall > security when calling external scripts. Using system() is prone to shell expansions, > which may lead to security breaches. Which is also why the execve() approach has > been the default since commit a82813527551f0e79c6d6ed5a9c1162e3c171bcf which > re-introduced the system() in Nov. 2008. > > After having asked on the mailing list and checked around on the IRC channels, the > genereal consensus is that very few uses system() these days. > > The only annoyance I've been made aware of is that this will now require adding a full > path to the script interpreter together with the script, and not just put in the > script name alone. But to just use the script name in Windows, you had to configure > --script-security with the 'system' flag earlier too. So my conclusion is that it's > better to add a full path to the script interpreter in Windows and raise the overal > security with OpenVPN, than to continue to have a possible potentially risky > OpenVPN configuration just to make life "easier" for Windows script users. > > Removal of the system() call, also solves a nasty bug related to the usage of putenv() > on the *nix platforms. > > For more information please see: > http://thread.gmane.org/gmane.network.openvpn.devel/7090 > https://community.openvpn.net/openvpn/ticket/228 > > Trac-ticket: 228 > Signed-off-by: David Sommerseth <da...@re...> > --- > doc/openvpn.8 | 48 ++++++++++++------ > src/openvpn/init.c | 3 -- > src/openvpn/misc.c | 98 ++++++++----------------------------- > src/openvpn/misc.h | 5 -- > src/openvpn/options.c | 16 +----- > src/openvpn/platform.c | 27 +--------- > src/openvpn/platform.h | 4 +- > src/openvpn/win32.c | 127 +++++++++++++----------------------------------- > 8 files changed, 89 insertions(+), 239 deletions(-) > > diff --git a/doc/openvpn.8 b/doc/openvpn.8 > index aa653ec..2ed5201 100644 > --- a/doc/openvpn.8 > +++ b/doc/openvpn.8 > @@ -1886,7 +1886,7 @@ is a safety precaution to prevent a LD_PRELOAD style attack > from a malicious or compromised server. > .\"********************************************************* > .TP > -.B \-\-script-security level [method] > +.B \-\-script-security level > This directive offers policy-level control over OpenVPN's usage of external programs > and scripts. Lower > .B level > @@ -1905,24 +1905,40 @@ Allow calling of built-in executables and user-defined scripts. > .B 3 \-\- > Allow passwords to be passed to scripts via environmental variables (potentially unsafe). > > -The > +OpenVPN releases before v2.3 also supported a > .B method > -parameter indicates how OpenVPN should call external commands and scripts. > -Settings for > -.B method: > +flag which indicated how OpenVPN should call external commands and scripts. This > +could be either > +.B execve > +or > +.B system. > +As of OpenVPN v2.3, this flag is no longer accepted. In most *nix environments the execve() > +approach has been used without any issues. > + > +To run scripts in Windows in earlier OpenVPN > +versions you needed to either add a full path to the script interpreter which can parse the > +script or use the > +.B system > +flag to run these scripts. As of OpenVPN v2.3 it is now a strict requirement to have > +full path to the script interpreter when running non-executables files. > +This is not needed for executable files, such as .exe, .com, .bat or .cmd files. For > +example, if you have a Visual Basic script, you must use this syntax now: > > -.B execve \-\- > -(default) Use execve() function on Unix family OSes and CreateProcess() on Windows. > -.br > -.B system \-\- > -Use system() function (deprecated and less safe since the external program command > -line is subject to shell expansion). > +.nf > +.ft 3 > +.in +4 > +\-\-up 'C:\\\\Windows\\\\System32\\\\wscript.exe C:\\\\Program\\ Files\\\\OpenVPN\\\\config\\\\my-up-script.vbs' > +.in -4 > +.ft > +.fi > > -The > -.B \-\-script-security > -option was introduced in OpenVPN 2.1_rc9. For configuration file compatibility > -with previous OpenVPN versions, use: > -.B \-\-script-security 3 system > +Please note the single quote marks and the escaping of the backslashes (\\) and > +the space character. > + > +The reason the support for the > +.B system > +flag was removed is due to the security implications with shell expansions > +when executing scripts via the system() call. > .\"********************************************************* > .TP > .B \-\-disable-occ > diff --git a/src/openvpn/init.c b/src/openvpn/init.c > index 270ee6a..25d8225 100644 > --- a/src/openvpn/init.c > +++ b/src/openvpn/init.c > @@ -2487,9 +2487,6 @@ do_option_warnings (struct context *c) > msg (M_WARN, "WARNING: the current --script-security setting may allow passwords to be passed to scripts via environmental variables"); > else > msg (M_WARN, "NOTE: " PACKAGE_NAME " 2.1 requires '--script-security 2' or higher to call user-defined scripts or executables"); > - > - if (script_method == SM_SYSTEM) > - msg (M_WARN, "NOTE: --script-security method='system' is deprecated due to the fact that passed parameters will be subject to shell expansion"); > } > > static void > diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c > index d2882d8..53f6d03 100644 > --- a/src/openvpn/misc.c > +++ b/src/openvpn/misc.c > @@ -53,9 +53,6 @@ const char *iproute_path = IPROUTE_PATH; /* GLOBAL */ > /* contains an SSEC_x value defined in misc.h */ > int script_security = SSEC_BUILT_IN; /* GLOBAL */ > > -/* contains SM_x value defined in misc.h */ > -int script_method = SM_EXECVE; /* GLOBAL */ > - > /* > * Pass tunnel endpoint and MTU parms to a user-supplied script. > * Used to execute the up/down script/plugins. > @@ -303,36 +300,25 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i > #if defined(ENABLE_FEATURE_EXECVE) > if (openvpn_execve_allowed (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; > - > - 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 (script_method == SM_SYSTEM) > - { > - ret = openvpn_system (argv_system_str (a), es, flags); > - } > - else > - { > - ASSERT (0); > - } > - } > + 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); > @@ -353,52 +339,6 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i > #endif > > /* > - * Wrapper around the system() call. > - */ > -int > -openvpn_system (const char *command, const struct env_set *es, unsigned int flags) > -{ > -#ifdef HAVE_SYSTEM > - int ret; > - > - perf_push (PERF_SCRIPT); > - > - /* > - * add env_set to environment. > - */ > - if (flags & S_SCRIPT) > - env_set_add_to_environment (es); > - > - > - /* debugging */ > - dmsg (D_SCRIPT, "SYSTEM[%u] '%s'", flags, command); > - if (flags & S_SCRIPT) > - env_set_print (D_SCRIPT, es); > - > - /* > - * execute the command > - */ > - ret = platform_system(command); > - > - /* debugging */ > - dmsg (D_SCRIPT, "SYSTEM return=%u", ret); > - > - /* > - * remove env_set from environment > - */ > - if (flags & S_SCRIPT) > - env_set_remove_from_environment (es); > - > - perf_pop (); > - return ret; > - > -#else > - msg (M_FATAL, "Sorry but I can't execute the shell command '%s' because this operating system doesn't appear to support the system() call", command); > - return -1; /* NOTREACHED */ > -#endif > -} > - > -/* > * 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. > diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h > index b6da3f4..183898e 100644 > --- a/src/openvpn/misc.h > +++ b/src/openvpn/misc.h > @@ -96,7 +96,6 @@ 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); > -int openvpn_system (const char *command, const struct env_set *es, 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) > @@ -322,10 +321,6 @@ extern const char *iproute_path; > #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 SM_EXECVE 0 /* call external programs with execve() or CreateProcess() */ > -#define SM_SYSTEM 1 /* call external programs with system() */ > -extern int script_method; /* GLOBAL */ > - > /* return the next largest power of 2 */ > size_t adjust_power_of_2 (size_t u); > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index edc9195..9baa4ff 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -248,7 +248,7 @@ static const char usage_message[] = > "--setenv name value : Set a custom environmental variable to pass to script.\n" > "--setenv FORWARD_COMPATIBLE 1 : Relax config file syntax checking to allow\n" > " directives for future OpenVPN versions to be ignored.\n" > - "--script-security level mode : mode='execve' (default) or 'system', level=\n" > + "--script-security level: Where level can be:\n" > " 0 -- strictly no calling of external programs\n" > " 1 -- (default) only call built-ins such as ifconfig\n" > " 2 -- allow calling of built-ins and scripts\n" > @@ -5293,20 +5293,6 @@ add_option (struct options *options, > { > VERIFY_PERMISSION (OPT_P_GENERAL); > script_security = atoi (p[1]); > - if (p[2]) > - { > - if (streq (p[2], "execve")) > - script_method = SM_EXECVE; > - else if (streq (p[2], "system")) > - script_method = SM_SYSTEM; > - else > - { > - msg (msglevel, "unknown --script-security method: %s", p[2]); > - goto err; > - } > - } > - else > - script_method = SM_EXECVE; > } > else if (streq (p[0], "mssfix")) > { > diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c > index c79f680..e79de7a 100644 > --- a/src/openvpn/platform.c > +++ b/src/openvpn/platform.c > @@ -205,7 +205,7 @@ platform_chdir (const char* dir) > } > > /* > - * convert system() return into a success/failure value > + * convert execve() return into a success/failure value > */ > bool > platform_system_ok (int stat) > @@ -217,19 +217,6 @@ platform_system_ok (int stat) > #endif > } > > -/* > - * did system() call execute the given command? > - */ > -bool > -platform_system_executed (int stat) > -{ > -#ifdef WIN32 > - return stat != -1; > -#else > - return stat != -1 && WEXITSTATUS (stat) != 127; > -#endif > -} > - > int > platform_access (const char *path, int mode) > { > @@ -288,18 +275,6 @@ platform_unlink (const char *filename) > #endif > } > > -int platform_system(const char *command) { > - int ret; > -#ifdef WIN32 > - struct gc_arena gc = gc_new (); > - ret = _wsystem (wide_string (command, &gc)); > - gc_free (&gc); > -#else > - ret = system (command); > -#endif > - return ret; > -} > - > int platform_putenv(char *string) > { > int status; > diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h > index 7bd2067..7c0a4d7 100644 > --- a/src/openvpn/platform.h > +++ b/src/openvpn/platform.h > @@ -113,10 +113,8 @@ void platform_mlockall (bool print_msg); /* Disable paging */ > > int platform_chdir (const char* dir); > > -/* interpret the status code returned by system()/execve() */ > +/* interpret the status code returned by execve() */ > bool platform_system_ok (int stat); > -bool platform_system_executed (int stat); > -int platform_system(const char *command); > > int platform_access (const char *path, int mode); > > diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c > index d00088e..2db96a8 100644 > --- a/src/openvpn/win32.c > +++ b/src/openvpn/win32.c > @@ -82,51 +82,6 @@ struct semaphore netcmd_semaphore; /* GLOBAL */ > */ > static char *win_sys_path = NULL; /* GLOBAL */ > > -/* > - * Configure PATH. On Windows, sometimes PATH is not set correctly > - * by default. > - */ > -static void > -configure_win_path (void) > -{ > - static bool done = false; /* GLOBAL */ > - if (!done) > - { > - FILE *fp; > - fp = fopen ("c:\\windows\\system32\\route.exe", "rb"); > - if (fp) > - { > - const int bufsiz = 4096; > - struct gc_arena gc = gc_new (); > - struct buffer oldpath = alloc_buf_gc (bufsiz, &gc); > - struct buffer newpath = alloc_buf_gc (bufsiz, &gc); > - const char* delim = ";"; > - DWORD status; > - fclose (fp); > - status = GetEnvironmentVariable ("PATH", BPTR(&oldpath), (DWORD)BCAP(&oldpath)); > -#if 0 > - status = 0; > -#endif > - if (!status) > - { > - *BPTR(&oldpath) = '\0'; > - delim = ""; > - } > - buf_printf (&newpath, "C:\\WINDOWS\\System32;C:\\WINDOWS;C:\\WINDOWS\\System32\\Wbem%s%s", > - delim, > - BSTR(&oldpath)); > - SetEnvironmentVariable ("PATH", BSTR(&newpath)); > -#if 0 > - status = GetEnvironmentVariable ("PATH", BPTR(&oldpath), (DWORD)BCAP(&oldpath)); > - if (status > 0) > - printf ("PATH: %s\n", BSTR(&oldpath)); > -#endif > - gc_free (&gc); > - done = true; > - } > - } > -} > - > void > init_win32 (void) > { > @@ -907,53 +862,41 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i > { > if (openvpn_execve_allowed (flags)) > { > - if (script_method == SM_EXECVE) > - { > - struct gc_arena gc = gc_new (); > - STARTUPINFOW start_info; > - PROCESS_INFORMATION proc_info; > - > - char *env = env_block (es); > - WCHAR *cl = wide_cmd_line (a, &gc); > - WCHAR *cmd = wide_string (a->argv[0], &gc); > - > - CLEAR (start_info); > - CLEAR (proc_info); > - > - /* fill in STARTUPINFO struct */ > - GetStartupInfoW(&start_info); > - start_info.cb = sizeof(start_info); > - start_info.dwFlags = STARTF_USESHOWWINDOW; > - start_info.wShowWindow = SW_HIDE; > - > - if (CreateProcessW (cmd, cl, NULL, NULL, FALSE, 0, env, NULL, &start_info, &proc_info)) > - { > - DWORD exit_status = 0; > - CloseHandle (proc_info.hThread); > - WaitForSingleObject (proc_info.hProcess, INFINITE); > - if (GetExitCodeProcess (proc_info.hProcess, &exit_status)) > - ret = (int)exit_status; > - else > - msg (M_WARN|M_ERRNO, "openvpn_execve: GetExitCodeProcess %S failed", cmd); > - CloseHandle (proc_info.hProcess); > - } > - else > - { > - msg (M_WARN|M_ERRNO, "openvpn_execve: CreateProcess %S failed", cmd); > - } > - free (env); > - gc_free (&gc); > - } > - else if (script_method == SM_SYSTEM) > - { > - configure_win_path (); > - ret = openvpn_system (argv_system_str (a), es, flags); > - } > - else > - { > - ASSERT (0); > - } > - } > + struct gc_arena gc = gc_new (); > + STARTUPINFOW start_info; > + PROCESS_INFORMATION proc_info; > + > + char *env = env_block (es); > + WCHAR *cl = wide_cmd_line (a, &gc); > + WCHAR *cmd = wide_string (a->argv[0], &gc); > + > + CLEAR (start_info); > + CLEAR (proc_info); > + > + /* fill in STARTUPINFO struct */ > + GetStartupInfoW(&start_info); > + start_info.cb = sizeof(start_info); > + start_info.dwFlags = STARTF_USESHOWWINDOW; > + start_info.wShowWindow = SW_HIDE; > + > + if (CreateProcessW (cmd, cl, NULL, NULL, FALSE, 0, env, NULL, &start_info, &proc_info)) > + { > + DWORD exit_status = 0; > + CloseHandle (proc_info.hThread); > + WaitForSingleObject (proc_info.hProcess, INFINITE); > + if (GetExitCodeProcess (proc_info.hProcess, &exit_status)) > + ret = (int)exit_status; > + else > + msg (M_WARN|M_ERRNO, "openvpn_execve: GetExitCodeProcess %S failed", cmd); > + CloseHandle (proc_info.hProcess); > + } > + else > + { > + msg (M_WARN|M_ERRNO, "openvpn_execve: CreateProcess %S failed", cmd); > + } > + free (env); > + gc_free (&gc); > + } > else if (!exec_warn && (script_security < SSEC_SCRIPTS)) > { > msg (M_WARN, SCRIPT_SECURITY_WARNING); > -- > 1.7.10.2 > > > ------------------------------------------------------------------------------ > The Windows 8 Center - In partnership with Sourceforge > Your idea - your app - 30 days. > Get started! > http://windows8center.sourceforge.net/ > what-html-developers-need-to-know-about-coding-windows-8-metro-style-apps/ > _______________________________________________ > Openvpn-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/openvpn-devel 

On 17/10/12 11:19, David Sommerseth wrote: > > Hi all, > > I've been reviewing a bug reported to the v2.3 code base. We're in the > beta phase currently, and this is a bug I'd like to get fixed before > we're moving on further. The bug is related to the use of the 'system' > flag in --script-security. > > <https://community.openvpn.net/openvpn/ticket/228> > > The use of the 'system' flag has been deprecated for a long time. And > it is really a potential security issue to use it, due to shell > expansions which might happen. The preferred (and default way) is to > use execve(), which is far safer and does not do the shell expansions > while executing the script or binary. > > The fix I'm currently considering is to remove support for the system() > call completely. This support was introduced in 2.1_rc9 (Nov 17, 2008). > > Does anyone depend on --script-security where the 'system' flag is > required? If you need this feature, can you please elaborate why this > support is needed and why you cannot use the preferred default with > execve? [...snip...] Based on the input I've received both on the OpenVPN mailing lists and on IRC discussions, I have not been convinced that we should still have the system() call support. Yes, there is an issue on Windows where you need to provide full path to the script interpreter for non-executable files (such as .vbs scripts). However, I consider that more a security enhancement, as you earlier had to provide either the full path (which is now a strict requirement) or use the 'system' flag with the --script-security option. In both cases, a configuration file change would be needed. The patch, which hopefully also updates the man page properly, is sent for review to the developers mailing list: <http://thread.gmane.org/gmane.network.openvpn.devel/7114> Thank you to all who did provide some feedback. kind regards, David Sommerseth 

From: David Sommerseth <da...@re...> This patch removes the support for the system() call, and enforces the usage of execve() on the *nix platform and CreateProcessW() on Windows. This is to enhance the overall security when calling external scripts. Using system() is prone to shell expansions, which may lead to security breaches. Which is also why the execve() approach has been the default since commit a82813527551f0e79c6d6ed5a9c1162e3c171bcf which re-introduced the system() in Nov. 2008. After having asked on the mailing list and checked around on the IRC channels, the genereal consensus is that very few uses system() these days. The only annoyance I've been made aware of is that this will now require adding a full path to the script interpreter together with the script, and not just put in the script name alone. But to just use the script name in Windows, you had to configure --script-security with the 'system' flag earlier too. So my conclusion is that it's better to add a full path to the script interpreter in Windows and raise the overal security with OpenVPN, than to continue to have a possible potentially risky OpenVPN configuration just to make life "easier" for Windows script users. Removal of the system() call, also solves a nasty bug related to the usage of putenv() on the *nix platforms. For more information please see: http://thread.gmane.org/gmane.network.openvpn.devel/7090 https://community.openvpn.net/openvpn/ticket/228 Trac-ticket: 228 Signed-off-by: David Sommerseth <da...@re...> --- doc/openvpn.8 | 48 ++++++++++++------ src/openvpn/init.c | 3 -- src/openvpn/misc.c | 98 ++++++++----------------------------- src/openvpn/misc.h | 5 -- src/openvpn/options.c | 16 +----- src/openvpn/platform.c | 27 +--------- src/openvpn/platform.h | 4 +- src/openvpn/win32.c | 127 +++++++++++++----------------------------------- 8 files changed, 89 insertions(+), 239 deletions(-) diff --git a/doc/openvpn.8 b/doc/openvpn.8 index aa653ec..2ed5201 100644 --- a/doc/openvpn.8 +++ b/doc/openvpn.8 @@ -1886,7 +1886,7 @@ is a safety precaution to prevent a LD_PRELOAD style attack from a malicious or compromised server. .\"********************************************************* .TP -.B \-\-script-security level [method] +.B \-\-script-security level This directive offers policy-level control over OpenVPN's usage of external programs and scripts. Lower .B level @@ -1905,24 +1905,40 @@ Allow calling of built-in executables and user-defined scripts. .B 3 \-\- Allow passwords to be passed to scripts via environmental variables (potentially unsafe). -The +OpenVPN releases before v2.3 also supported a .B method -parameter indicates how OpenVPN should call external commands and scripts. -Settings for -.B method: +flag which indicated how OpenVPN should call external commands and scripts. This +could be either +.B execve +or +.B system. +As of OpenVPN v2.3, this flag is no longer accepted. In most *nix environments the execve() +approach has been used without any issues. + +To run scripts in Windows in earlier OpenVPN +versions you needed to either add a full path to the script interpreter which can parse the +script or use the +.B system +flag to run these scripts. As of OpenVPN v2.3 it is now a strict requirement to have +full path to the script interpreter when running non-executables files. +This is not needed for executable files, such as .exe, .com, .bat or .cmd files. For +example, if you have a Visual Basic script, you must use this syntax now: -.B execve \-\- -(default) Use execve() function on Unix family OSes and CreateProcess() on Windows. -.br -.B system \-\- -Use system() function (deprecated and less safe since the external program command -line is subject to shell expansion). +.nf +.ft 3 +.in +4 +\-\-up 'C:\\\\Windows\\\\System32\\\\wscript.exe C:\\\\Program\\ Files\\\\OpenVPN\\\\config\\\\my-up-script.vbs' +.in -4 +.ft +.fi -The -.B \-\-script-security -option was introduced in OpenVPN 2.1_rc9. For configuration file compatibility -with previous OpenVPN versions, use: -.B \-\-script-security 3 system +Please note the single quote marks and the escaping of the backslashes (\\) and +the space character. + +The reason the support for the +.B system +flag was removed is due to the security implications with shell expansions +when executing scripts via the system() call. .\"********************************************************* .TP .B \-\-disable-occ diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 270ee6a..25d8225 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2487,9 +2487,6 @@ do_option_warnings (struct context *c) msg (M_WARN, "WARNING: the current --script-security setting may allow passwords to be passed to scripts via environmental variables"); else msg (M_WARN, "NOTE: " PACKAGE_NAME " 2.1 requires '--script-security 2' or higher to call user-defined scripts or executables"); - - if (script_method == SM_SYSTEM) - msg (M_WARN, "NOTE: --script-security method='system' is deprecated due to the fact that passed parameters will be subject to shell expansion"); } static void diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index d2882d8..53f6d03 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -53,9 +53,6 @@ const char *iproute_path = IPROUTE_PATH; /* GLOBAL */ /* contains an SSEC_x value defined in misc.h */ int script_security = SSEC_BUILT_IN; /* GLOBAL */ -/* contains SM_x value defined in misc.h */ -int script_method = SM_EXECVE; /* GLOBAL */ - /* * Pass tunnel endpoint and MTU parms to a user-supplied script. * Used to execute the up/down script/plugins. @@ -303,36 +300,25 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i #if defined(ENABLE_FEATURE_EXECVE) if (openvpn_execve_allowed (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; - - 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 (script_method == SM_SYSTEM) - { - ret = openvpn_system (argv_system_str (a), es, flags); - } - else - { - ASSERT (0); - } -	} + 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); @@ -353,52 +339,6 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i #endif /* - * Wrapper around the system() call. - */ -int -openvpn_system (const char *command, const struct env_set *es, unsigned int flags) -{ -#ifdef HAVE_SYSTEM - int ret; - - perf_push (PERF_SCRIPT); - - /* - * add env_set to environment. - */ - if (flags & S_SCRIPT) - env_set_add_to_environment (es); - - - /* debugging */ - dmsg (D_SCRIPT, "SYSTEM[%u] '%s'", flags, command); - if (flags & S_SCRIPT) - env_set_print (D_SCRIPT, es); - - /* - * execute the command - */ - ret = platform_system(command); - - /* debugging */ - dmsg (D_SCRIPT, "SYSTEM return=%u", ret); - - /* - * remove env_set from environment - */ - if (flags & S_SCRIPT) - env_set_remove_from_environment (es); - - perf_pop (); - return ret; - -#else - msg (M_FATAL, "Sorry but I can't execute the shell command '%s' because this operating system doesn't appear to support the system() call", command); - return -1; /* NOTREACHED */ -#endif -} - -/* * 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. diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index b6da3f4..183898e 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -96,7 +96,6 @@ 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); -int openvpn_system (const char *command, const struct env_set *es, 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) @@ -322,10 +321,6 @@ extern const char *iproute_path; #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 SM_EXECVE 0 /* call external programs with execve() or CreateProcess() */ -#define SM_SYSTEM 1 /* call external programs with system() */ -extern int script_method; /* GLOBAL */ - /* return the next largest power of 2 */ size_t adjust_power_of_2 (size_t u); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index edc9195..9baa4ff 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -248,7 +248,7 @@ static const char usage_message[] = "--setenv name value : Set a custom environmental variable to pass to script.\n" "--setenv FORWARD_COMPATIBLE 1 : Relax config file syntax checking to allow\n" " directives for future OpenVPN versions to be ignored.\n" - "--script-security level mode : mode='execve' (default) or 'system', level=\n" + "--script-security level: Where level can be:\n" " 0 -- strictly no calling of external programs\n" " 1 -- (default) only call built-ins such as ifconfig\n" " 2 -- allow calling of built-ins and scripts\n" @@ -5293,20 +5293,6 @@ add_option (struct options *options, { VERIFY_PERMISSION (OPT_P_GENERAL); script_security = atoi (p[1]); - if (p[2]) -	{ - if (streq (p[2], "execve")) - script_method = SM_EXECVE; - else if (streq (p[2], "system")) - script_method = SM_SYSTEM; - else - { - msg (msglevel, "unknown --script-security method: %s", p[2]); - goto err; - } -	} - else -	script_method = SM_EXECVE; } else if (streq (p[0], "mssfix")) { diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index c79f680..e79de7a 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -205,7 +205,7 @@ platform_chdir (const char* dir) } /* - * convert system() return into a success/failure value + * convert execve() return into a success/failure value */ bool platform_system_ok (int stat) @@ -217,19 +217,6 @@ platform_system_ok (int stat) #endif } -/* - * did system() call execute the given command? - */ -bool -platform_system_executed (int stat) -{ -#ifdef WIN32 - return stat != -1; -#else - return stat != -1 && WEXITSTATUS (stat) != 127; -#endif -} - int platform_access (const char *path, int mode) { @@ -288,18 +275,6 @@ platform_unlink (const char *filename) #endif } -int platform_system(const char *command) { - int ret; -#ifdef WIN32 - struct gc_arena gc = gc_new (); - ret = _wsystem (wide_string (command, &gc)); - gc_free (&gc); -#else - ret = system (command); -#endif - return ret; -} - int platform_putenv(char *string) { int status; diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h index 7bd2067..7c0a4d7 100644 --- a/src/openvpn/platform.h +++ b/src/openvpn/platform.h @@ -113,10 +113,8 @@ void platform_mlockall (bool print_msg); /* Disable paging */ int platform_chdir (const char* dir); -/* interpret the status code returned by system()/execve() */ +/* interpret the status code returned by execve() */ bool platform_system_ok (int stat); -bool platform_system_executed (int stat); -int platform_system(const char *command); int platform_access (const char *path, int mode); diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index d00088e..2db96a8 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -82,51 +82,6 @@ struct semaphore netcmd_semaphore; /* GLOBAL */ */ static char *win_sys_path = NULL; /* GLOBAL */ -/* - * Configure PATH. On Windows, sometimes PATH is not set correctly - * by default. - */ -static void -configure_win_path (void) -{ - static bool done = false; /* GLOBAL */ - if (!done) - { - FILE *fp; - fp = fopen ("c:\\windows\\system32\\route.exe", "rb"); - if (fp) -	{ - const int bufsiz = 4096; - struct gc_arena gc = gc_new (); - struct buffer oldpath = alloc_buf_gc (bufsiz, &gc); - struct buffer newpath = alloc_buf_gc (bufsiz, &gc); - const char* delim = ";"; - DWORD status; - fclose (fp); - status = GetEnvironmentVariable ("PATH", BPTR(&oldpath), (DWORD)BCAP(&oldpath)); -#if 0 - status = 0; -#endif - if (!status) - { - *BPTR(&oldpath) = '\0'; - delim = ""; - } - buf_printf (&newpath, "C:\\WINDOWS\\System32;C:\\WINDOWS;C:\\WINDOWS\\System32\\Wbem%s%s", - delim, - BSTR(&oldpath)); - SetEnvironmentVariable ("PATH", BSTR(&newpath)); -#if 0 - status = GetEnvironmentVariable ("PATH", BPTR(&oldpath), (DWORD)BCAP(&oldpath)); - if (status > 0) - printf ("PATH: %s\n", BSTR(&oldpath)); -#endif - gc_free (&gc); - done = true; -	} - } -} - void init_win32 (void) { @@ -907,53 +862,41 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i { if (openvpn_execve_allowed (flags))	{ - if (script_method == SM_EXECVE) - { - struct gc_arena gc = gc_new (); - STARTUPINFOW start_info; - PROCESS_INFORMATION proc_info; - - char *env = env_block (es); - WCHAR *cl = wide_cmd_line (a, &gc); - WCHAR *cmd = wide_string (a->argv[0], &gc); - - CLEAR (start_info); - CLEAR (proc_info); - - /* fill in STARTUPINFO struct */ - GetStartupInfoW(&start_info); - start_info.cb = sizeof(start_info); - start_info.dwFlags = STARTF_USESHOWWINDOW; - start_info.wShowWindow = SW_HIDE; - - if (CreateProcessW (cmd, cl, NULL, NULL, FALSE, 0, env, NULL, &start_info, &proc_info)) -	{ - DWORD exit_status = 0; - CloseHandle (proc_info.hThread); - WaitForSingleObject (proc_info.hProcess, INFINITE); - if (GetExitCodeProcess (proc_info.hProcess, &exit_status)) - ret = (int)exit_status; - else - msg (M_WARN|M_ERRNO, "openvpn_execve: GetExitCodeProcess %S failed", cmd); - CloseHandle (proc_info.hProcess); -	} - else -	{ - msg (M_WARN|M_ERRNO, "openvpn_execve: CreateProcess %S failed", cmd); -	} - free (env); - gc_free (&gc); - } - else if (script_method == SM_SYSTEM) - { - configure_win_path (); - ret = openvpn_system (argv_system_str (a), es, flags); - } - else - { - ASSERT (0); - } -	} + struct gc_arena gc = gc_new (); + STARTUPINFOW start_info; + PROCESS_INFORMATION proc_info; + + char *env = env_block (es); + WCHAR *cl = wide_cmd_line (a, &gc); + WCHAR *cmd = wide_string (a->argv[0], &gc); + + CLEAR (start_info); + CLEAR (proc_info); + + /* fill in STARTUPINFO struct */ + GetStartupInfoW(&start_info); + start_info.cb = sizeof(start_info); + start_info.dwFlags = STARTF_USESHOWWINDOW; + start_info.wShowWindow = SW_HIDE; + + if (CreateProcessW (cmd, cl, NULL, NULL, FALSE, 0, env, NULL, &start_info, &proc_info)) + { + DWORD exit_status = 0; + CloseHandle (proc_info.hThread); + WaitForSingleObject (proc_info.hProcess, INFINITE); + if (GetExitCodeProcess (proc_info.hProcess, &exit_status)) + ret = (int)exit_status; + else + msg (M_WARN|M_ERRNO, "openvpn_execve: GetExitCodeProcess %S failed", cmd); + CloseHandle (proc_info.hProcess); + } + else + { + msg (M_WARN|M_ERRNO, "openvpn_execve: CreateProcess %S failed", cmd); + } + free (env); + gc_free (&gc); + } else if (!exec_warn && (script_security < SSEC_SCRIPTS))	{ msg (M_WARN, SCRIPT_SECURITY_WARNING); -- 1.7.10.2 

On 29/10/12 14:16, Heiko Hund wrote: > If a common name (or user name, when used in conjunction with > --username-as-common-name) contains UTF-8 encoded characters their > octets get replaced by underscores. This becomes problematic when > user "Müller" and "Möller" need to have a CCD files and both would > be receive options from the file "M__ller". The situation is even > worse for non-latin alphabets, where CCD file names consist of > underscores entirely. > > This patch removes that limitation and also allows the file names > to contain any punctuation characters besided the resevered ones. > > Signed-off-by: Heiko Hund <hei...@so...> > --- > src/openvpn/buffer.c | 10 ++++++++++ > src/openvpn/buffer.h | 5 +++++ > src/openvpn/misc.c | 8 +++++++- > 3 files changed, 22 insertions(+), 1 deletion(-) > ACK. Applied to master and beta/2.3. commit 9885f57e3ac8d2e32ba20ca84f6bdd0a1a995eac (master) commit d442b8dbc4230e4252a63fbd57f149ef3fa090c8 (beta/2.3) Author: Heiko Hund <hei...@so...> Date: Mon Oct 29 14:16:37 2012 +0100 Support UTF-8 --client-config-dir Signed-off-by: Heiko Hund <hei...@so...> Acked-by: David Sommerseth <da...@re...> Message-Id: 135...@so... URL: http://article.gmane.org/gmane.network.openvpn.devel/7110 Signed-off-by: David Sommerseth <da...@re...> kind regards, David Sommerseth 

On 29/10/12 14:38, Heiko Hund wrote: > The OPENVPN_PLUGIN_ROUTE_PREDOWN hook was missing and displayed as > "PLUGIN_???" in the log. > > OPENVPN_PLUGIN_ENABLE_PF was the only one that displayed the > OPENVPN_ prefix. > > Signed-off-by: Heiko Hund <hei...@so...> > --- > src/openvpn/plugin.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > ACK. Applied to master and beta/2.3 commit ae303d444c11984b87e0046c4138982d7a41fd8b (master) commit 3b8d116d3d9dbe2fac786311db8b2bec53d88c77 (beta/2.3) Author: Heiko Hund <hei...@so...> Date: Mon Oct 29 14:38:30 2012 +0100 Fix display of plugin hook types Signed-off-by: Heiko Hund <hei...@so...> Acked-by: David Sommerseth <da...@re...> Message-Id: 135...@so... URL: http://article.gmane.org/gmane.network.openvpn.devel/7111 Signed-off-by: David Sommerseth <da...@re...> (cherry picked from commit ae303d444c11984b87e0046c4138982d7a41fd8b) kind regards, David Sommerseth 

The OPENVPN_PLUGIN_ROUTE_PREDOWN hook was missing and displayed as "PLUGIN_???" in the log. OPENVPN_PLUGIN_ENABLE_PF was the only one that displayed the OPENVPN_ prefix. Signed-off-by: Heiko Hund <hei...@so...> --- src/openvpn/plugin.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c index 944d833..83f79e4 100644 --- a/src/openvpn/plugin.c +++ b/src/openvpn/plugin.c @@ -98,7 +98,9 @@ plugin_type_name (const int type) case OPENVPN_PLUGIN_TLS_FINAL: return "PLUGIN_TLS_FINAL"; case OPENVPN_PLUGIN_ENABLE_PF: - return "OPENVPN_PLUGIN_ENABLE_PF"; + return "PLUGIN_ENABLE_PF"; + case OPENVPN_PLUGIN_ROUTE_PREDOWN: + return "PLUGIN_ROUTE_PREDOWN"; default: return "PLUGIN_???"; } -- 1.7.9.5 

If a common name (or user name, when used in conjunction with --username-as-common-name) contains UTF-8 encoded characters their octets get replaced by underscores. This becomes problematic when user "Müller" and "Möller" need to have a CCD files and both would be receive options from the file "M__ller". The situation is even worse for non-latin alphabets, where CCD file names consist of underscores entirely. This patch removes that limitation and also allows the file names to contain any punctuation characters besided the resevered ones. Signed-off-by: Heiko Hund <hei...@so...> --- src/openvpn/buffer.c | 10 ++++++++++ src/openvpn/buffer.h | 5 +++++ src/openvpn/misc.c | 8 +++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 5eee3ee..56d14b1 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -782,6 +782,16 @@ char_class (const unsigned char c, const unsigned int flags) return true; if ((flags & CC_EQUAL) && c == '=') return true; + if ((flags & CC_LESS_THAN) && c == '<') + return true; + if ((flags & CC_GREATER_THAN) && c == '>') + return true; + if ((flags & CC_PIPE) && c == '|') + return true; + if ((flags & CC_QUESTION_MARK) && c == '?') + return true; + if ((flags & CC_ASTERISK) && c == '*') + return true; return false; } diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index 9bc33db..5e11de0 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -736,6 +736,11 @@ const char *np (const char *str); #define CC_REVERSE_QUOTE (1<<23) #define CC_AT (1<<24) #define CC_EQUAL (1<<25) +#define CC_LESS_THAN (1<<26) +#define CC_GREATER_THAN (1<<27) +#define CC_PIPE (1<<28) +#define CC_QUESTION_MARK (1<<29) +#define CC_ASTERISK (1<<30) /* macro classes */ #define CC_NAME (CC_ALNUM|CC_UNDERBAR) diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index d2882d8..d33db20 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -1056,7 +1056,13 @@ hostname_randomize(const char *hostname, struct gc_arena *gc) const char * gen_path (const char *directory, const char *filename, struct gc_arena *gc) { - const char *safe_filename = string_mod_const (filename, CC_ALNUM|CC_UNDERBAR|CC_DASH|CC_DOT|CC_AT, 0, '_', gc); +#if WIN32 + const int CC_PATH_RESERVED = CC_LESS_THAN|CC_GREATER_THAN|CC_COLON| + CC_DOUBLE_QUOTE|CC_SLASH|CC_BACKSLASH|CC_PIPE|CC_QUESTION_MARK|CC_ASTERISK; +#else + const int CC_PATH_RESERVED = CC_SLASH; +#endif + const char *safe_filename = string_mod_const (filename, CC_PRINT, CC_PATH_RESERVED, '_', gc); if (safe_filename && strcmp (safe_filename, ".") -- 1.7.9.5 

Hi, On Thu, Oct 25, 2012 at 07:39:55PM +0200, David Sommerseth wrote: > This will all help the review process. I believe Gert who already > replied have some hands-on experience with Solaris as well, I still carry the scars. But I'm not admitting anything. > so I hope he can have some time to help review your patch(es) as well. I can even *test* it... :-) (though I refuse to boot the Sparc64/Solaris box for that). gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

On 25/10/12 18:24, Magnus Lindholm wrote: > Hi all, > > with openvpn-2.2.2 I get "unable to redirect default gateway -- Cannot > read current default gateway from system" on my solaris sparc box. > Maybe I missed something but I solved this by putting together some > code into a patch to route.c that lets openvpn read the default > gateway correctly on solaris boxes (.SunOS 5.11 snv_151a sun4u) Let me > know if anyone is interested in incorporating this functionality into > openvpn Hey Magnus! Thanks a lot for digging into this, and I hope you can dare to share a patch with us here on the mailing list. First of all, it would be good if we could get this patch based upon the latest openvpn code base, and if you even can manage it as a git patch that would be really great! We have some info on how to contribute here: <https://community.openvpn.net/openvpn/wiki/DeveloperDocumentation> This will all help the review process. I believe Gert who already replied have some hands-on experience with Solaris as well, so I hope he can have some time to help review your patch(es) as well. If we get a patch rather soonish, we can probably manage to squeeze such a fix into the very soonish 2.3_RC1 release. kind regards, David Sommerseth 

Hi, On Thu, Oct 25, 2012 at 06:24:20PM +0200, Magnus Lindholm wrote: > with openvpn-2.2.2 I get "unable to redirect default gateway -- Cannot > read current default gateway from system" on my solaris sparc box. > Maybe I missed something but I solved this by putting together some > code into a patch to route.c that lets openvpn read the default > gateway correctly on solaris boxes (.SunOS 5.11 snv_151a sun4u) Let me > know if anyone is interested in incorporating this functionality into > openvpn Please send the patch, then we'll see :-) gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Hi all, with openvpn-2.2.2 I get "unable to redirect default gateway -- Cannot read current default gateway from system" on my solaris sparc box. Maybe I missed something but I solved this by putting together some code into a patch to route.c that lets openvpn read the default gateway correctly on solaris boxes (.SunOS 5.11 snv_151a sun4u) Let me know if anyone is interested in incorporating this functionality into openvpn Regards Magnus 

On 17/10/12 12:46, Arne Schwabe wrote: > In the old patch the if incorrectly closed the outer if condition. (closes ticket #231) > --- > src/openvpn/options.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > ACK. Applied to master and beta/2.3 branches. commit 70a07339f8d323d69cdcf8d59da1f331d39e4d0a (master) commit ad0cc02234e17ab1f43488c4393059ea1c9d8f95 (beta/2.3) Author: Arne Schwabe <ar...@rf...> Date: Wed Oct 17 12:46:14 2012 +0200 Options parsing demands unnecessary configuration if PKCS11 is used In the old patch the if incorrectly closed the outer if condition. (closes ticket #231) Trac-ticket: 231 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: David Sommerseth <da...@re...> Message-Id: 135...@rf... URL: http://article.gmane.org/gmane.network.openvpn.devel/7095 Signed-off-by: David Sommerseth <da...@re...> kind regards, David Sommerseth 

hi everybody,I have some problems about entry point of packets into openvpn. I know that there is a multi_instance structure which is created per packet and there is a scheduler which detects a packet with earliest wakeup but suppose two clients are connecting to openvpn we know that there is a queue for holding their packets and process these packets, respectively. I have two questions, where is this queue exatly (i mean which structure in code is responsible for holding packets)? and how it's get updated (where does queue and dequeue operation happen?) Thanks Ehsan 

Thanks to all of you for your replies. The consensus seems to be that the documentation is correct, and that if OpenVPN behaves differently then that is a bug. I'll try to get the user's configs/logs, or try to generate some test cases that show this myself (and include a 2.1... test case to see if this behavior was introduced in 2.2) and post them on this thread. On Mon, Oct 22, 2012 at 6:11 AM, David Sommerseth < ope...@to...> wrote: > On 22/10/12 10:48, Gert Doering wrote: > > Hi Jonathan, > > > > On Sun, Oct 21, 2012 at 06:40:08PM -0400, Jonathan K. Bullard wrote: > >> A Tunnelblick user has reported odd behavior with name resolution > failures. > >> I can't tell if it is a bug in OpenVPN, a bug in the documentation, or > >> something else. The behavior is apparently the same in OpenVPN 2.2.1 and > >> 2.3alpha1. > > [..] > >> Any ideas if this is a program bug, a documentation bug, or something > else, > >> for example, a problem that only occurs on OS X? > > > > "One of those" :-) - I don't know yet, and haven't looked into the code > yet > > (will do ASAP). > > > > The resolver code has been changed for 2.2 (getting rid of the old > > randomization stuff), so it might be a bug introduced there, or a much > > older bug that nobody ever noticed. > > > > Do you have an OpenVPN log that shows this behaviour? Might help in > > digging down to it... > > A client config file would help in addition. I'm also wondering if > connection profiles are used ... that is, more --remote options or > <connection> blocks. > > Otherwise, I would not be surprised if there are some corner case here > appearing, which surfaced more easily when the resolving code was > re-written (as Gert mentioned). > > This resolver_retry_second option is only used in socket.c AFAICT, which > most of us try to avoid looking too hard on, to avoid eye cancer and/or > loosing our sanity ;-) But there's work in progress on cleaning up that > one, unfortunately that will take some time. But we'll try to hunt down > this which looks like an openvpn bug to me. > > > kind regards, > > David Sommerseth > > 

Am 22.10.12 00:40, schrieb Jonathan K. Bullard: > A Tunnelblick user has reported odd behavior with name resolution > failures. I can't tell if it is a bug in OpenVPN, a bug in the > documentation, or something else. The behavior is apparently the same > in OpenVPN 2.2.1 and 2.3alpha1. > > The 2.3 man page says: > > --resolv-retry n > If hostname resolve fails for --remote, retry resolve for n > seconds before failing. > Set n to "infinite" to retry indefinitely. > By default, --resolv-retry infinite is enabled. You can > disable by setting n=0. > > > But the behavior seems very different: > > If a name resolution failure (caused, for example, by losing the > network connection) occurs > and --resolv-retry is: > > 1, the first resolution failure terminates the connection; > 5, resolution attempts are made dozens of times per second, > seemingly forever; or > 10, resolution attempts are made every five seconds, seemingly > forever. > > > Any ideas if this is a program bug, a documentation bug, or something > else, for example, a problem that only occurs on OS X? I guess a programming bug. The number of retries for a resolv is defined as (fail_wait_interval is fixed to 5): int resolve_retries = (flags & GETADDR_TRY_ONCE) ? 1 : (resolve_retry_seconds / fail_wait_interval); so if resolv-retry < 5 resolve_retries is set to 0 giving you a direct failure. Without trying you should then see a USR1 and openvpn will directly do the next connection attempt without waiting the 5s delay. On a quick glance I cannot see a way to tell openvpn to have a fixed number of connection retries after it quits. Arne 

On 22/10/12 10:48, Gert Doering wrote: > Hi Jonathan, > > On Sun, Oct 21, 2012 at 06:40:08PM -0400, Jonathan K. Bullard wrote: >> A Tunnelblick user has reported odd behavior with name resolution failures. >> I can't tell if it is a bug in OpenVPN, a bug in the documentation, or >> something else. The behavior is apparently the same in OpenVPN 2.2.1 and >> 2.3alpha1. > [..] >> Any ideas if this is a program bug, a documentation bug, or something else, >> for example, a problem that only occurs on OS X? > > "One of those" :-) - I don't know yet, and haven't looked into the code yet > (will do ASAP). > > The resolver code has been changed for 2.2 (getting rid of the old > randomization stuff), so it might be a bug introduced there, or a much > older bug that nobody ever noticed. > > Do you have an OpenVPN log that shows this behaviour? Might help in > digging down to it... A client config file would help in addition. I'm also wondering if connection profiles are used ... that is, more --remote options or <connection> blocks. Otherwise, I would not be surprised if there are some corner case here appearing, which surfaced more easily when the resolving code was re-written (as Gert mentioned). This resolver_retry_second option is only used in socket.c AFAICT, which most of us try to avoid looking too hard on, to avoid eye cancer and/or loosing our sanity ;-) But there's work in progress on cleaning up that one, unfortunately that will take some time. But we'll try to hunt down this which looks like an openvpn bug to me. kind regards, David Sommerseth 

Hi Jonathan, On Sun, Oct 21, 2012 at 06:40:08PM -0400, Jonathan K. Bullard wrote: > A Tunnelblick user has reported odd behavior with name resolution failures. > I can't tell if it is a bug in OpenVPN, a bug in the documentation, or > something else. The behavior is apparently the same in OpenVPN 2.2.1 and > 2.3alpha1. [..] > Any ideas if this is a program bug, a documentation bug, or something else, > for example, a problem that only occurs on OS X? "One of those" :-) - I don't know yet, and haven't looked into the code yet (will do ASAP). The resolver code has been changed for 2.2 (getting rid of the old randomization stuff), so it might be a bug introduced there, or a much older bug that nobody ever noticed. Do you have an OpenVPN log that shows this behaviour? Might help in digging down to it... gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Well, I guess that puts a stopper in my attempt to pass the buck! ----- Eric F Crist On Oct 21, 2012, at 18:21:51, Jonathan K. Bullard <jkb...@gm...> wrote: > On Sun, Oct 21, 2012 at 7:03 PM, Eric Crist wrote: > This sounds like a Tunnelblick failure. I'd suggest checking with them first, they do all sorts of things with scripts and such. > > Thanks, but I'm the current Tunnelblick developer! > > You're correct that Tunnelblick does a lot in its scripts, but as far as I can tell, this behavior has nothing to do with the scripts or Tunnelblick. > > It has to do with what OpenVPN does -- the Tunnelblick scripts are not involved. The behavior takes place completely within OpenVPN. > 

On Sun, Oct 21, 2012 at 7:03 PM, Eric Crist wrote: > This sounds like a Tunnelblick failure. I'd suggest checking with them > first, they do all sorts of things with scripts and such. > Thanks, but *I'm* the current Tunnelblick developer! You're correct that Tunnelblick does a lot in its scripts, but as far as I can tell, this behavior has nothing to do with the scripts or Tunnelblick. It has to do with what OpenVPN does -- the Tunnelblick scripts are not involved. The behavior takes place completely within OpenVPN. 

This sounds like a Tunnelblick failure. I'd suggest checking with them first, they do all sorts of things with scripts and such. Cheers ----- Eric F Crist On Oct 21, 2012, at 17:40:08, Jonathan K. Bullard <jkb...@gm...> wrote: > A Tunnelblick user has reported odd behavior with name resolution failures. I can't tell if it is a bug in OpenVPN, a bug in the documentation, or something else. The behavior is apparently the same in OpenVPN 2.2.1 and 2.3alpha1. > > The 2.3 man page says: > --resolv-retry n > If hostname resolve fails for --remote, retry resolve for n seconds before failing. > Set n to "infinite" to retry indefinitely. > By default, --resolv-retry infinite is enabled. You can disable by setting n=0. > > But the behavior seems very different: > > If a name resolution failure (caused, for example, by losing the network connection) occurs > and --resolv-retry is: > 1, the first resolution failure terminates the connection; > 5, resolution attempts are made dozens of times per second, seemingly forever; or > 10, resolution attempts are made every five seconds, seemingly forever. > > Any ideas if this is a program bug, a documentation bug, or something else, for example, a problem that only occurs on OS X? > ------------------------------------------------------------------------------ > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics > Download AppDynamics Lite for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct_______________________________________________ > Openvpn-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/openvpn-devel 

A Tunnelblick user has reported odd behavior with name resolution failures. I can't tell if it is a bug in OpenVPN, a bug in the documentation, or something else. The behavior is apparently the same in OpenVPN 2.2.1 and 2.3alpha1. The 2.3 man page says: > --resolv-retry n > If hostname resolve fails for --remote, retry resolve for n seconds > before failing. > Set n to "infinite" to retry indefinitely. > By default, --resolv-retry infinite is enabled. You can disable by > setting n=0. But the behavior seems very different: If a name resolution failure (caused, for example, by losing the network connection) occurs and --resolv-retry is: 1, the first resolution failure terminates the connection; 5, resolution attempts are made dozens of times per second, seemingly forever; or 10, resolution attempts are made every five seconds, seemingly forever. Any ideas if this is a program bug, a documentation bug, or something else, for example, a problem that only occurs on OS X? 

In the old patch the if incorrectly closed the outer if condition. (closes ticket #231) --- src/openvpn/options.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 05a0f54..8717b89 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -2192,13 +2192,15 @@ options_postprocess_verify_ce (const struct options *options, const struct conne } else #endif -#ifdef ENABLE_CRYPTOAPI #ifdef MANAGMENT_EXTERNAL_KEY if((options->management_flags & MF_EXTERNAL_KEY) && options->priv_key_file) -	msg (M_USAGE, "--key and --management-external-key are mutually exclusive"); + { + msg (M_USAGE, "--key and --management-external-key are mutually exclusive"); + } + else #endif - - if (options->cryptoapi_cert) +#ifdef ENABLE_CRYPTOAPI + if (options->cryptoapi_cert)	{ if ((!(options->ca_file)) && (!(options->ca_path))) msg(M_USAGE, "You must define CA file (--ca) or CA path (--capath)"); -- 1.7.9.5