Re: [Openvpn-devel] rc9 and external commands

2008-09-06 Thread James Yonan

James Yonan wrote:

Matthias Andree wrote:

On Thu, 21 Aug 2008, Alberto Gonzalez Iniesta wrote:


It seems that tightening the security on OpenVPN brought some surprises
[1] to users and broke some features [2].

As for [1], I included a note in the Debian NEWS file on the new
--script-security option. But those updating a VPN using the very same
VPN (and without previous knowledge of this option) may find themselves
without access to the remote system (if the VPN/system is restarted, and
a script is to be executed). Also, those using NetworkManager [3] aren't
able to specify the '--script-security' option, and since NetworkManager
may/will call external scripts, this new security feature will break
their VPNs. The option is really useful, but 2 would be a more sensible
default IMHO.

One might argue that NetworkManager doesn't support all *RC versions of
OpenVPN's. At the end of the day, it will be a NetworkManager issue.
Interfaces change...


Regarding [2] an strace shows that calls to external commands with
arguments include the arguments as part of the command filename:
For: 
--up "/tmp/foo up"


The call is:
[pid  3519] execve("/tmp/foo up", ["/tmp/foo up", "tun0", "1500", "1542",
"10.XXX.XXX.X", "10.XXX.XXX.X", "init"], [/* 30 vars */]) = -1 ENOENT
(No such file or directory)

Apparently argument splitting - as documented in the --up section of the
manpage - no longer works; it was probably formerly done by the implicit
/bin/sh -c that is now gone with the switch to exec*().

(I didn't check, and didn't check the two Debian BTS reports either.)

So either the code needs argument splitting or you need a two-line shell
wrapper similar to:

#! /bin/sh -e
exec /tmp/foo up "$@"

Not my call to make.


I agree that argument splitting in --up "/tmp/foo up" should be
supported, as it doesn't detract from the sought-after security goals in
migrating from system() to execve(), and preserves backward compatibility.


Okay, argument splitting should be working again with this patch.

One of the above posters asked if --script-security could be defaulted 
to 2 to preserve backward compatibility.  I would tend to disagree on 
this as it would defeat, in my view, the primary purpose of 
--script-security.


I think the guiding principle here is that security software should 
refrain from doing potentially dangerous things (and I don't think 
anyone can deny that running user-defined scripts from a VPN daemon is 
potentially dangerous) unless explicit permission is given by the user.



r3311 | james | 2008-09-06 03:42:17 -0600 (Sat, 06 Sep 2008) | 20 lines
Changed paths:
   M /branches/BETA21/openvpn/buffer.c
   M /branches/BETA21/openvpn/buffer.h
   M /branches/BETA21/openvpn/errlevel.h
   M /branches/BETA21/openvpn/init.c
   M /branches/BETA21/openvpn/misc.c
   M /branches/BETA21/openvpn/misc.h
   M /branches/BETA21/openvpn/multi.c
   M /branches/BETA21/openvpn/socket.c
   M /branches/BETA21/openvpn/ssl.c

2.1_rc8 and earlier did implicit shell expansion on script
arguments since all scripts were called by system().
The security hardening changes made to 2.1_rc9 no longer
use system(), but rather use the safer execve or CreateProcess
system calls.  The security hardening also introduced a
backward incompatibility with 2.1_rc8 and earlier in that
script parameters were no longer shell-expanded, so
for example:

  client-connect "docc CLIENT-CONNECT"

would fail to work because execve would try to execute
a script called "docc CLIENT-CONNECT" instead of "docc"
with "CLIENT-CONNECT" as the first argument.

This patch fixes the issue, bringing the script argument
semantics back to pre 2.1_rc9 behavior in order to preserve
backward compatibility while still using execve or CreateProcess
to execute the script/executable.





Re: [Openvpn-devel] rc9 and external commands

2008-08-22 Thread James Yonan

Matthias Andree wrote:

On Thu, 21 Aug 2008, Alberto Gonzalez Iniesta wrote:


It seems that tightening the security on OpenVPN brought some surprises
[1] to users and broke some features [2].

As for [1], I included a note in the Debian NEWS file on the new
--script-security option. But those updating a VPN using the very same
VPN (and without previous knowledge of this option) may find themselves
without access to the remote system (if the VPN/system is restarted, and
a script is to be executed). Also, those using NetworkManager [3] aren't
able to specify the '--script-security' option, and since NetworkManager
may/will call external scripts, this new security feature will break
their VPNs. The option is really useful, but 2 would be a more sensible
default IMHO.


One might argue that NetworkManager doesn't support all *RC versions of
OpenVPN's. At the end of the day, it will be a NetworkManager issue.
Interfaces change...


Regarding [2] an strace shows that calls to external commands with
arguments include the arguments as part of the command filename:
For: 
--up "/tmp/foo up"


The call is:
[pid  3519] execve("/tmp/foo up", ["/tmp/foo up", "tun0", "1500", "1542",
"10.XXX.XXX.X", "10.XXX.XXX.X", "init"], [/* 30 vars */]) = -1 ENOENT
(No such file or directory)


Apparently argument splitting - as documented in the --up section of the
manpage - no longer works; it was probably formerly done by the implicit
/bin/sh -c that is now gone with the switch to exec*().

(I didn't check, and didn't check the two Debian BTS reports either.)

So either the code needs argument splitting or you need a two-line shell
wrapper similar to:

#! /bin/sh -e
exec /tmp/foo up "$@"

Not my call to make.


I agree that argument splitting in --up "/tmp/foo up" should be
supported, as it doesn't detract from the sought-after security goals in
migrating from system() to execve(), and preserves backward compatibility.

James





Re: [Openvpn-devel] rc9 and external commands

2008-08-21 Thread Matthias Andree
On Thu, 21 Aug 2008, Alberto Gonzalez Iniesta wrote:

> It seems that tightening the security on OpenVPN brought some surprises
> [1] to users and broke some features [2].
> 
> As for [1], I included a note in the Debian NEWS file on the new
> --script-security option. But those updating a VPN using the very same
> VPN (and without previous knowledge of this option) may find themselves
> without access to the remote system (if the VPN/system is restarted, and
> a script is to be executed). Also, those using NetworkManager [3] aren't
> able to specify the '--script-security' option, and since NetworkManager
> may/will call external scripts, this new security feature will break
> their VPNs. The option is really useful, but 2 would be a more sensible
> default IMHO.

One might argue that NetworkManager doesn't support all *RC versions of
OpenVPN's. At the end of the day, it will be a NetworkManager issue.
Interfaces change...

> Regarding [2] an strace shows that calls to external commands with
> arguments include the arguments as part of the command filename:
> For: 
> --up "/tmp/foo up"
> 
> The call is:
> [pid  3519] execve("/tmp/foo up", ["/tmp/foo up", "tun0", "1500", "1542",
> "10.XXX.XXX.X", "10.XXX.XXX.X", "init"], [/* 30 vars */]) = -1 ENOENT
> (No such file or directory)

Apparently argument splitting - as documented in the --up section of the
manpage - no longer works; it was probably formerly done by the implicit
/bin/sh -c that is now gone with the switch to exec*().

(I didn't check, and didn't check the two Debian BTS reports either.)

So either the code needs argument splitting or you need a two-line shell
wrapper similar to:

#! /bin/sh -e
exec /tmp/foo up "$@"

Not my call to make.

-- 
Matthias Andree



[Openvpn-devel] rc9 and external commands

2008-08-21 Thread Alberto Gonzalez Iniesta
Hi James,

It seems that tightening the security on OpenVPN brought some surprises
[1] to users and broke some features [2].

As for [1], I included a note in the Debian NEWS file on the new
--script-security option. But those updating a VPN using the very same
VPN (and without previous knowledge of this option) may find themselves
without access to the remote system (if the VPN/system is restarted, and
a script is to be executed). Also, those using NetworkManager [3] aren't
able to specify the '--script-security' option, and since NetworkManager
may/will call external scripts, this new security feature will break
their VPNs. The option is really useful, but 2 would be a more sensible
default IMHO.

Regarding [2] an strace shows that calls to external commands with
arguments include the arguments as part of the command filename:
For: 
--up "/tmp/foo up"

The call is:
[pid  3519] execve("/tmp/foo up", ["/tmp/foo up", "tun0", "1500", "1542",
"10.XXX.XXX.X", "10.XXX.XXX.X", "init"], [/* 30 vars */]) = -1 ENOENT
(No such file or directory)

Where in previous versions the call was:
[pid  4074] execve("/tmp/kk", ["/tmp/kk", "up", "tun0", "1500", "1542",
"10.XXX.XXX.X", "10.XXX.XXX.X", "init"], [/* 51 vars */]) = 0

Please let me know what's your opinion regarding [1].

Thanks a lot,

Alberto

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=494998
[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=495964
[3] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=494998#10

-- 
Alberto Gonzalez Iniesta| Formación, consultoría y soporte técnico
agi@(inittab.org|debian.org)| en GNU/Linux y software libre
Encrypted mail preferred| http://inittab.com

Key fingerprint = 9782 04E7 2B75 405C F5E9  0C81 C514 AF8E 4BA4 01C3