David Sommerseth [Fri, 16 Apr 2010 20:09:48 +0000 (22:09 +0200)]
Renamed all calls to create_temp_filename()
All places where create_temp_filename() was called are now calling
create_temp_file(). Extra checks on the result of create_temp_file()
is added in addition.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Fri, 16 Apr 2010 20:02:36 +0000 (22:02 +0200)]
Harden create_temp_filename() (version 2)
By hardening the create_temp_filename() function to check if the generated
filename exists and to create the temp file with only S_IRUSR|S_IWUSR bit
files set before calling the script, it should become even more difficult to
exploit such a scenario.
After a discussion on the mailing list, Fabian Knittel provided an enhanced
version of the inital patch which is added to this patch.
This patch also renames create_temp_filename() to create_temp_file(), as this
patch also creates the temporary file. The function returns the filename of the
created file, or NULL on error.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Signed-off-by: Fabian Knittel <fabian.knittel@avona.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Daniel Johnson [Tue, 30 Mar 2010 13:54:44 +0000 (15:54 +0200)]
When I began testing OpenVPN v2.1_rc9 I was having trouble authenticating to the MS Active Directory through auth-pam and Samba. I used the following line in my configs (without the linebreak of course):
Finally I turned on more verbose logging and found that the plugin did
not recognize "USERNAME" as something to replace, because it expected
the string to be surrounded by whitespace. I wrote the following patch
to correct this. I hope you find it useful,
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
After having disucssed this patch on IRC (#openvpn-discussions)
March 4, 2010, it was decided to accept this patch when not modifying
TARGET_* defines through out the code. Further, in a mail comment
Alon Bar-Lev had some other comments of what would be needed to be done.
Mail reference:
<http://thread.gmane.org/gmane.network.openvpn.devel/3176>
This patch has been tested by bootstrapping the code on a RHEL4.6 box.
with the following autotools packages installed:
autoconf-2.59-5
automake-1.9.2-3
libtool-1.5.6-4.EL4.2
It builds cleanly and 'make check' passes.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: James Yonan <james@openvpn.net> Acked-by: Alon Bar-Lev <alon.barlev@gmail.com>
Jan Brinkmann [Sun, 28 Feb 2010 22:29:29 +0000 (23:29 +0100)]
The man page needs dash escaping in UTF-8 environments
There was a debian bugreport which was filed in 2005 . It was patched but
it seems that nobody forwarded the patch to the openvpn project itself.
The problem is quite simple:
The dashes for options (the double dashes) are not escaped. This causes
trouble in relationship with utf-8 .
Since the bugreport was closed it was patched within the debian/ubuntu
packages itself. I've attached the patch to get it atleast reviewed by the
openvpn project itself.
See <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=296133> for details.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Tested-by: Jan Just Keijser <janjust@nikhef.nl> Tested-by: Pavel Shramov <shramov@mexmat.net> Tested-by: Samuli Seppänen <samuli@openvpn.net>
Dan Nelson [Sun, 28 Feb 2010 21:09:18 +0000 (22:09 +0100)]
bash->bourne script cleanup
Many of the scripts in the openvpn source have their shell set to
/bin/bash, but only two use bash features. The attached patch (against
openvpn-2.1_rc9) sets the shell on the rest of the scripts to /bin/sh for
better portability. The only scripts that actually require bash are
contrib/pull-resolv-conf/client.{up,down} ; they use the ${!var} variable
indirection feature.
Enrico Scholz [Sun, 28 Feb 2010 13:40:57 +0000 (14:40 +0100)]
Allow 'lport 0' setup for random port binding
I am running a multihomed host where 'local <extip>' must be specified
for proper operation. Unfortunately, this implies 'lport 1194' or
another static port.
This causes problems with stateful firewalls which register the host/port
pairs in the internal connection tracking table. On ungraceful reconnects,
the new TCP connection will have same the host/port pairs but unexpected
sequence numbers. The new connection will be assumed as invalid hence and
be dropped.
It would be nice when local port can be configured to be bound to a
random port number. After reading code,
| else if (streq (p[0], "lport") && p[1])
| ...
| port = atoi (p[1]);
|- if (!legal_ipv4_port (port))
|+ if (port != 0 && !legal_ipv4_port (port))
| {
in options.c seems to be the only required change.
This has been discussed here:
<http://thread.gmane.org/gmane.network.openvpn.user/28622>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Fri, 19 Feb 2010 16:32:56 +0000 (17:32 +0100)]
verb 5 logging wrongly reports received bytes
With --verb 5, openvpn logs a single letter (rwRW) for each package
received or sent. I recently ran into a problem with the tun device on
Linux where the read from that device returned 0. Unfortunately this was
also logged as "r", which made me assume that openvpn had received
something, while it actually hadn't.
(See https://dev.openwrt.org/ticket/6650 for the bug that made me find out
about this problem with openvpn.)
I'm attaching a patch which prevents openvpn from logging "r" or "R" when
it didn't actually read anything. This is against openvpn 2.1-rc20, but
probably still applies to the most recent version.
This patch was received anonymously via the sf.net bug tracker:
<http://sourceforge.net/tracker/?func=detail&atid=454719&aid=2951003&group_id=48978>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Karl O. Pinc [Thu, 18 Feb 2010 20:30:48 +0000 (21:30 +0100)]
[PATCH] Change verify-cn so cn is no longer hardcoded in openvpn's config file
This patch should be easy to process.
A resubmission of the patch sent to this list on 04/23/2009.
The patch changes the verify-cn script sample
to be used with --tls-verify so that instead of having
to hardcode a cn to verify in the OpenVPN configuration file
the allowed cns may be written into a separate file.
This makes the process of verifying cns a whole
lot more dynamic, to the point where it is useful
in the real world.
One problem with this patch is that it is backwards
incompatible. I did not bother keeping the original
calling interface as A) it's a sample script, and B) the
original's functionality seems useless
and equalivant functionality is easily available
with the new script.
The problem with the original is that there seems
little point in verifying a client's cn when all
the clients share one cn, as would have to be
the case when the cn is hardcoded into the openvpn
config file.
This patch applies against the testing allmiscs branch,
and should apply against any of the other testing
branches as well.
It works for me. I've tested it throughly but not
used it extensively in production.
Regards,
Karl <kop@meme.com>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Eric F Crist <ecrist@secure-computing.net>
David Sommerseth [Thu, 18 Feb 2010 20:20:14 +0000 (21:20 +0100)]
Do not randomize resolving of IP addresses in getaddr()
Based on a discussion on the mailing list and in the IRC meeting Feb 18,
it was decided to remove get_random() from the getaddr() function as that
can conflict with round-robin/randomization done by DNS servers.
This change must be documented in the release notes.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
Daniel Johnson [Tue, 30 Mar 2010 13:54:44 +0000 (15:54 +0200)]
When I began testing OpenVPN v2.1_rc9 I was having trouble authenticating to the MS Active Directory through auth-pam and Samba. I used the following line in my configs (without the linebreak of course):
Finally I turned on more verbose logging and found that the plugin did
not recognize "USERNAME" as something to replace, because it expected
the string to be surrounded by whitespace. I wrote the following patch
to correct this. I hope you find it useful,
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
After having disucssed this patch on IRC (#openvpn-discussions)
March 4, 2010, it was decided to accept this patch when not modifying
TARGET_* defines through out the code. Further, in a mail comment
Alon Bar-Lev had some other comments of what would be needed to be done.
Mail reference:
<http://thread.gmane.org/gmane.network.openvpn.devel/3176>
This patch has been tested by bootstrapping the code on a RHEL4.6 box.
with the following autotools packages installed:
autoconf-2.59-5
automake-1.9.2-3
libtool-1.5.6-4.EL4.2
It builds cleanly and 'make check' passes.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: James Yonan <james@openvpn.net> Acked-by: Alon Bar-Lev <alon.barlev@gmail.com>
Jan Brinkmann [Sun, 28 Feb 2010 22:29:29 +0000 (23:29 +0100)]
The man page needs dash escaping in UTF-8 environments
There was a debian bugreport which was filed in 2005 . It was patched but
it seems that nobody forwarded the patch to the openvpn project itself.
The problem is quite simple:
The dashes for options (the double dashes) are not escaped. This causes
trouble in relationship with utf-8 .
Since the bugreport was closed it was patched within the debian/ubuntu
packages itself. I've attached the patch to get it atleast reviewed by the
openvpn project itself.
See <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=296133> for details.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Tested-by: Jan Just Keijser <janjust@nikhef.nl> Tested-by: Pavel Shramov <shramov@mexmat.net> Tested-by: Samuli Seppänen <samuli@openvpn.net>
Dan Nelson [Sun, 28 Feb 2010 21:09:18 +0000 (22:09 +0100)]
bash->bourne script cleanup
Many of the scripts in the openvpn source have their shell set to
/bin/bash, but only two use bash features. The attached patch (against
openvpn-2.1_rc9) sets the shell on the rest of the scripts to /bin/sh for
better portability. The only scripts that actually require bash are
contrib/pull-resolv-conf/client.{up,down} ; they use the ${!var} variable
indirection feature.
Enrico Scholz [Sun, 28 Feb 2010 13:40:57 +0000 (14:40 +0100)]
Allow 'lport 0' setup for random port binding
I am running a multihomed host where 'local <extip>' must be specified
for proper operation. Unfortunately, this implies 'lport 1194' or
another static port.
This causes problems with stateful firewalls which register the host/port
pairs in the internal connection tracking table. On ungraceful reconnects,
the new TCP connection will have same the host/port pairs but unexpected
sequence numbers. The new connection will be assumed as invalid hence and
be dropped.
It would be nice when local port can be configured to be bound to a
random port number. After reading code,
| else if (streq (p[0], "lport") && p[1])
| ...
| port = atoi (p[1]);
|- if (!legal_ipv4_port (port))
|+ if (port != 0 && !legal_ipv4_port (port))
| {
in options.c seems to be the only required change.
This has been discussed here:
<http://thread.gmane.org/gmane.network.openvpn.user/28622>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Fri, 19 Feb 2010 16:32:56 +0000 (17:32 +0100)]
verb 5 logging wrongly reports received bytes
With --verb 5, openvpn logs a single letter (rwRW) for each package
received or sent. I recently ran into a problem with the tun device on
Linux where the read from that device returned 0. Unfortunately this was
also logged as "r", which made me assume that openvpn had received
something, while it actually hadn't.
(See https://dev.openwrt.org/ticket/6650 for the bug that made me find out
about this problem with openvpn.)
I'm attaching a patch which prevents openvpn from logging "r" or "R" when
it didn't actually read anything. This is against openvpn 2.1-rc20, but
probably still applies to the most recent version.
This patch was received anonymously via the sf.net bug tracker:
<http://sourceforge.net/tracker/?func=detail&atid=454719&aid=2951003&group_id=48978>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Karl O. Pinc [Thu, 18 Feb 2010 20:30:48 +0000 (21:30 +0100)]
[PATCH] Change verify-cn so cn is no longer hardcoded in openvpn's config file
This patch should be easy to process.
A resubmission of the patch sent to this list on 04/23/2009.
The patch changes the verify-cn script sample
to be used with --tls-verify so that instead of having
to hardcode a cn to verify in the OpenVPN configuration file
the allowed cns may be written into a separate file.
This makes the process of verifying cns a whole
lot more dynamic, to the point where it is useful
in the real world.
One problem with this patch is that it is backwards
incompatible. I did not bother keeping the original
calling interface as A) it's a sample script, and B) the
original's functionality seems useless
and equalivant functionality is easily available
with the new script.
The problem with the original is that there seems
little point in verifying a client's cn when all
the clients share one cn, as would have to be
the case when the cn is hardcoded into the openvpn
config file.
This patch applies against the testing allmiscs branch,
and should apply against any of the other testing
branches as well.
It works for me. I've tested it throughly but not
used it extensively in production.
Regards,
Karl <kop@meme.com>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Eric F Crist <ecrist@secure-computing.net>
David Sommerseth [Thu, 18 Feb 2010 20:20:14 +0000 (21:20 +0100)]
Do not randomize resolving of IP addresses in getaddr()
Based on a discussion on the mailing list and in the IRC meeting Feb 18,
it was decided to remove get_random() from the getaddr() function as that
can conflict with round-robin/randomization done by DNS servers.
This change must be documented in the release notes.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
Dan Nelson [Sun, 28 Feb 2010 21:09:18 +0000 (22:09 +0100)]
bash->bourne script cleanup
Many of the scripts in the openvpn source have their shell set to
/bin/bash, but only two use bash features. The attached patch (against
openvpn-2.1_rc9) sets the shell on the rest of the scripts to /bin/sh for
better portability. The only scripts that actually require bash are
contrib/pull-resolv-conf/client.{up,down} ; they use the ${!var} variable
indirection feature.
James Yonan [Sat, 6 Mar 2010 15:38:23 +0000 (15:38 +0000)]
Fixed an issue where if reneg-sec was set to 0 on the client,
so that the server-side value would take precedence,
the auth_deferred_expire_window function would incorrectly
return a window period of 0 seconds. In this case, the
correct window period should be the handshake window
period.
james [Sat, 6 Mar 2010 15:38:23 +0000 (15:38 +0000)]
Fixed an issue where if reneg-sec was set to 0 on the client,
so that the server-side value would take precedence,
the auth_deferred_expire_window function would incorrectly
return a window period of 0 seconds. In this case, the
correct window period should be the handshake window
period.
Enrico Scholz [Sun, 28 Feb 2010 13:40:57 +0000 (14:40 +0100)]
Allow 'lport 0' setup for random port binding
I am running a multihomed host where 'local <extip>' must be specified
for proper operation. Unfortunately, this implies 'lport 1194' or
another static port.
This causes problems with stateful firewalls which register the host/port
pairs in the internal connection tracking table. On ungraceful reconnects,
the new TCP connection will have same the host/port pairs but unexpected
sequence numbers. The new connection will be assumed as invalid hence and
be dropped.
It would be nice when local port can be configured to be bound to a
random port number. After reading code,
| else if (streq (p[0], "lport") && p[1])
| ...
| port = atoi (p[1]);
|- if (!legal_ipv4_port (port))
|+ if (port != 0 && !legal_ipv4_port (port))
| {
in options.c seems to be the only required change.
This has been discussed here:
<http://thread.gmane.org/gmane.network.openvpn.user/28622>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
James Yonan [Fri, 26 Feb 2010 10:26:45 +0000 (10:26 +0000)]
Fixed an issue in the Management Interface that could cause
a process hang with 100% CPU utilization in --management-client
mode if the management interface client disconnected at the
point where credentials are queried.
James Yonan [Fri, 26 Feb 2010 10:26:45 +0000 (10:26 +0000)]
Fixed an issue in the Management Interface that could cause
a process hang with 100% CPU utilization in --management-client
mode if the management interface client disconnected at the
point where credentials are queried.
james [Fri, 26 Feb 2010 10:26:45 +0000 (10:26 +0000)]
Fixed an issue in the Management Interface that could cause
a process hang with 100% CPU utilization in --management-client
mode if the management interface client disconnected at the
point where credentials are queried.
David Sommerseth [Fri, 19 Feb 2010 16:32:56 +0000 (17:32 +0100)]
verb 5 logging wrongly reports received bytes
With --verb 5, openvpn logs a single letter (rwRW) for each package
received or sent. I recently ran into a problem with the tun device on
Linux where the read from that device returned 0. Unfortunately this was
also logged as "r", which made me assume that openvpn had received
something, while it actually hadn't.
(See https://dev.openwrt.org/ticket/6650 for the bug that made me find out
about this problem with openvpn.)
I'm attaching a patch which prevents openvpn from logging "r" or "R" when
it didn't actually read anything. This is against openvpn 2.1-rc20, but
probably still applies to the most recent version.
This patch was received anonymously via the sf.net bug tracker:
<http://sourceforge.net/tracker/?func=detail&atid=454719&aid=2951003&group_id=48978>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Karl O. Pinc [Thu, 18 Feb 2010 20:30:48 +0000 (21:30 +0100)]
[PATCH] Change verify-cn so cn is no longer hardcoded in openvpn's config file
This patch should be easy to process.
A resubmission of the patch sent to this list on 04/23/2009.
The patch changes the verify-cn script sample
to be used with --tls-verify so that instead of having
to hardcode a cn to verify in the OpenVPN configuration file
the allowed cns may be written into a separate file.
This makes the process of verifying cns a whole
lot more dynamic, to the point where it is useful
in the real world.
One problem with this patch is that it is backwards
incompatible. I did not bother keeping the original
calling interface as A) it's a sample script, and B) the
original's functionality seems useless
and equalivant functionality is easily available
with the new script.
The problem with the original is that there seems
little point in verifying a client's cn when all
the clients share one cn, as would have to be
the case when the cn is hardcoded into the openvpn
config file.
This patch applies against the testing allmiscs branch,
and should apply against any of the other testing
branches as well.
It works for me. I've tested it throughly but not
used it extensively in production.
Regards,
Karl <kop@meme.com>
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Eric F Crist <ecrist@secure-computing.net>
David Sommerseth [Thu, 18 Feb 2010 20:20:14 +0000 (21:20 +0100)]
Do not randomize resolving of IP addresses in getaddr()
Based on a discussion on the mailing list and in the IRC meeting Feb 18,
it was decided to remove get_random() from the getaddr() function as that
can conflict with round-robin/randomization done by DNS servers.
This change must be documented in the release notes.
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
james [Tue, 12 Jan 2010 18:26:22 +0000 (18:26 +0000)]
When aborting in a non-graceful way, try to execute do_close_tun in
init.c prior to daemon exit to ensure that the tun/tap interface is
closed and any added routes are deleted.
james [Fri, 11 Dec 2009 23:44:34 +0000 (23:44 +0000)]
Fixed some breakage in openvpn.spec (which is required to build an
RPM distribution) where it was referencing a non-existent
subdirectory in the tarball, causing it to fail (patch from
David Sommerseth).
james [Thu, 10 Dec 2009 23:50:03 +0000 (23:50 +0000)]
Fixed a couple issues in sample plugins auth-pam.c and down-root.c:
1. Fail gracefully rather than segfault if calloc returns NULL.
2. The openvpn_plugin_abort_v1 function can potentially be called
with handle == NULL. Add code to detect this case, and if
so, avoid dereferencing pointers derived from handle.
(Thanks to David Sommerseth for finding this bug).
james [Thu, 19 Nov 2009 16:42:51 +0000 (16:42 +0000)]
Fixed a client-side bug that occurred when the "dhcp-pre-release"
or "dhcp-renew" options were combined with "route-gateway dhcp".
The problem is that the IP Helper functions for DHCP release and
renew are blocking, and so calling them from a single-threaded
client stops tunnel traffic forwarding, and hence breaks
"route-gateway dhcp" which requires an active tunnel. The fix is
to call the IP Helper functions for DHCP release and renew from
another process.
james [Fri, 16 Oct 2009 16:31:01 +0000 (16:31 +0000)]
Added "setenv GENERIC_CONFIG" directive, for generic configs
that cannot directly be used as a config file. The directive
will simply cause OpenVPN to exit with an error if a generic
config file is used.
james [Thu, 1 Oct 2009 21:08:40 +0000 (21:08 +0000)]
client-kill management interface command, when issued on server, will
now send a RESTART message to client.
This feature is intended to make UDP clients respond the same as TCP
clients in the case where the server issues a RESTART message in
order to force the client to reconnect and pull a new options/route
list.
james [Tue, 29 Sep 2009 23:10:14 +0000 (23:10 +0000)]
Added the ability for the server to provide a custom reason string
when an AUTH_FAILED message is returned to the client. This
string can be set by the server-side managment interface and read
by the client-side management interface.
For more info, see management/management-notes.txt, and look for
references to "client-reason-text".
james [Mon, 28 Sep 2009 07:50:30 +0000 (07:50 +0000)]
Fixed a bug introduced in r4436 (2.1_rc17) where using the
redirect-gateway option by itself, without any extra parameters,
would cause the option to be ignored.
james [Mon, 28 Sep 2009 07:27:22 +0000 (07:27 +0000)]
Added --server-poll-timeout option : when polling possible remote
servers to connect to in a round-robin fashion, spend no more than
n seconds waiting for a response before trying the next server.
james [Sun, 27 Sep 2009 02:12:15 +0000 (02:12 +0000)]
Eliminated the limitation on the number of options that can be pushed
to clients, including routes. Previously, all pushed options needed
to fit within a 1024 byte options string.
Remember that to make use of this feature to allow many routes to
be pushed to clients, the client config file must specify the
max-routes option, and the number of pushed routes cannot exceed
this limit. Also, both server and client must include this commit.
james [Thu, 17 Sep 2009 23:43:37 +0000 (23:43 +0000)]
The maximum number of "route" directives (specified in the config
file or pulled from a server) can now be configured via the new
"max-routes" directive.
Previously, the limit was set to 100 and fixed by a compile-time
constant. Now the limit is dynamic and can be modified by the
"max-routes" directive. If max-routes is not specified, the default
limit is 100.
Note that this change does not address the maximum size of the
pushed options string sent from server to client, which is still
controlled by the TLS_CHANNEL_BUF_SIZE compile-time constant.
james [Wed, 16 Sep 2009 18:58:49 +0000 (18:58 +0000)]
Modified client to send a PUSH_REQUEST message to server 1 second
after connection initiation rather than 0 seconds after.
Successive PUSH_REQUEST messages after the first will continue to be
sent at 5 second intervals until a response is received. This tends
to speed up the client connection sequence by 4 seconds because the
first PUSH_REQUEST message is usually sent too soon and is dropped,
causing a wait of 5 seconds until the next PUSH_REQUEST message is
sent.