Opened 8 years ago

Closed 8 years ago

#228 closed Bug / Defect (fixed)

"Assertion failed at buffer.c:331" with up/down options and "script-security 2 system"

Reported by: andreax Owned by: David Sommerseth
Priority: minor Milestone: beta 2.3
Component: plug-ins / plug-in API Version: OpenVPN 2.3-beta / 2.3-RC (Community Ed)
Severity: Patch Queue: Merged Keywords:
Cc:

Description

Using the up or down option with "script-security 2 system",
the OpenVPN server reports the following error:

Sep 24 11:15:36 efw openvpn[6233]: Assertion failed at buffer.c:331
Sep 24 11:15:36 efw openvpn[6233]: Exiting due to fatal error

Note that with "script-security 2 execve" it works without problems.

OpenVPN configuration:

daemon
mode server
tls-server
proto udp
port 10443
user openvpn
group openvpn
dev tap0
server 192.168.100.0 255.255.255.0
verb 5
dh /var/efw/openvpn/dh1024.pem
pkcs12 /var/efw/openvpn/pkcs12.p12

script-security 2 system
up "/usr/local/bin/dir.d-exec /etc/openvpn/ifup.server.d/"

Configure options:
configure --prefix=/usr --sysconfdir=/etc/openvpn/ --enable-password-save --enable-pthread --enable-iproute2

# openvpn --version
OpenVPN 2.3_beta1 i686-redhat-linux-gnu [SSL (OpenSSL)] [LZO] [EPOLL] [eurephia] [MH] [IPv6] built on Sep 24 2012
Originally developed by James Yonan
Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <sales@…>
Compile time defines: enable_crypto=yes enable_debug=yes enable_def_auth=yes enable_dlopen=unknown enable_dlopen_self=unknown enable_dlopen_self_static=unknown enable_eurephia=yes enable_fast_install=yes enable_fragment=yes enable_http_proxy=yes enable_iproute2=yes enable_libtool_lock=yes enable_lzo=yes enable_lzo_stub=no enable_management=yes enable_multi=yes enable_multihome=yes enable_pam_dlopen=no enable_password_save=yes enable_pedantic=no enable_pf=yes enable_pkcs11=no enable_plugin_auth_pam=yes enable_plugin_down_root=yes enable_plugins=yes enable_port_share=yes enable_pthread=yes enable_selinux=no enable_server=yes enable_shared=yes enable_shared_with_static_runtimes=no enable_small=no enable_socks=yes enable_ssl=yes enable_static=yes enable_strict=no enable_strict_options=no enable_systemd=no enable_win32_dll=yes enable_x509_alt_username=no with_crypto_library=openssl with_gnu_ld=yes with_mem_check=no with_plugindir='$(libdir)/openvpn/plugins'

# uname -a
Linux efw.localdomain 2.6.32.43-57.e48.i586 #1 SMP Wed Jun 27 12:16:03 EDT 2012 i686 athlon i386 GNU/Linux

Change History (5)

comment:1 Changed 8 years ago by David Sommerseth

Component: Generic / unclassifiedplug-ins / plug-in API
Owner: set to David Sommerseth
Priority: majorminor
Status: newaccepted

Is the use of 'system' a feature you really need? The reason I'm wondering is that fixing this isn't going to be trivial at all. The use of putenv() internally in OpenVPN is adding some nasty challenges, which are triggered when configured to use system() instead of execve()

The use of system() is also deprecated. So I'm considering the fix to be to remove 'system' support.

comment:2 Changed 8 years ago by David Sommerseth

A patch which removes the system() call support has been sent to the developers mailing list:

http://thread.gmane.org/gmane.network.openvpn.devel/7114

comment:3 Changed 8 years ago by David Sommerseth

Severity: Not set (if unsure, select this one)Patch Queue: New / awaiting ACK

comment:4 Changed 8 years ago by Samuli Seppänen

Here's list of tests I ran yesterday to verify that removing "system" method does not cause any regressions. All tests were done on openvpn-2.3_beta1 on Windows 7 64-bit.

This one tries to connect, but fails with "assertion failed at buffer.c:331":

script-security 2 system
up "c:
test.vbs"

This fails with "%1 is not a valid Win32 application":

script-security 2 execve
up "c:
test.vbs"

This Fails with "options error. Invalid argument":

script-security 2 execve
up "c:
Windows
System32
wscript.exe \"c:
test.vbs\""

This tries to execute "c:
Windows
System32
wscript.exe Local Area Connection 4" (not good):

script-security 2 execve
up "c:
Windows
System32
wscript.exe"

This fails with "options error. Invalid argument":

script-security 2 execve
up "c:
Windows
System32
wscript.exe c:
test.vbs"

This fails with "options error. Invalid argument":

script-security 2 execve
up "c:
Windows
System32
wscript.exe \"c:
test.vbs\""

This works:

script-security 2 execve
up 'c:
Windows
System32
wscript.exe c:
test.vbs'

This fails with "There's no file c:\Program":

script-security 2 execve
up 'c:
Windows
System32
wscript.exe c:
Program Files
OpenVPN
config
test.vbs'

This works:

script-security 2 execve
up 'c:
Windows
System32
wscript.exe c:
Program\ Files
OpenVPN
config
test.vbs'

Summary: removing system() may require modifying openvpn configurations, but does not cause any permanent regressions.

comment:5 Changed 8 years ago by David Sommerseth

Resolution: fixed
Severity: Patch Queue: New / awaiting ACKPatch Queue: Merged
Status: acceptedclosed

Patch mentioned comment 2 has been applied.

commit 3cb9f1a62b4a84dbf4acd1957c900a5b06fd6ac2 (beta/2.3)
Author: David Sommerseth <davids@redhat.com>
Date:   Thu Oct 25 14:22:30 2012 +0200

    Remove the support for using system() when executing external programs or scripts
    
    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 <davids@redhat.com>
    Acked-by: Gert Doering <gert@greenie.muc.de>
    Message-Id: <1351539352-17371-1-git-send-email-dazo@users.sourceforge.net>
    URL: http://article.gmane.org/gmane.network.openvpn.devel/7114
    (cherry picked from commit 0563473601abfbf2142bfa0ca5b863c5aa7953a2) (master)
Note: See TracTickets for help on using tickets.