Steven McDonald [Thu, 13 Apr 2017 17:31:29 +0000 (03:31 +1000)]
Fix gateway detection with OpenBSD routing domains
When OpenVPN is started using a non-default routing table on OpenBSD
(e.g., with 'route -T10 exec openvpn ...'), it hangs forever trying to
read its default gateway from a PF_ROUTE socket. This is because
rtm_tableid is not being initialised after bzeroing the rt_msghdr we
write to the socket, so we end up asking the kernel for the default
route in routing table 0.
By default, the OpenBSD kernel will not respond to requests for routing
table 0 from a process running in a different routing table, and even
if it did, it would give us the wrong default gateway.
The solution here is to set rtm_tableid to the value returned by
getrtable(2), which always succeeds and returns the calling process's
current routing table.
This patch makes the test suite (without a t_client.rc) pass when run
in a non-default routing table, where it would fail previously. It has
also been successfully tested in client mode against both git master
and OpenVPN 2.4.1 from ports on an OpenBSD -current system.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170413173129.87367-1-steven@steven-mcdonald.id.au>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14461.html
Gert Doering [Thu, 18 May 2017 10:22:46 +0000 (12:22 +0200)]
Fix NCP behaviour on TLS reconnect.
If a client reconnects on a hard-restart from the same port (due to --bind
in use on the client), both sides will handle this as a "reconnect" and
not a "full new connect" internally, re-using existing crypto context.
The client will still ask the server for pushed options, and the server
code to handle this refuses to do NCP if a key has already been negotiated
(because there is no way to *change* the cipher after that) - which ends
up in "the client uses the non-negotiated cipher from the config file,
while the server uses the previously-negotiated NCP cipher", and nothing
works.
The easy workaround: if we find us in the situation that we think NCP
has already been done, just re-push "cipher o->ciphername" with the
current cipher for this client context.
All credits for this go to Stefan Behrens <sbehrens@giantdisaster.de>
who found and diagnosed the issue in trac #887, came up with a first
patch to solve the issue quite similar to this (simplified) one, and
helped testing.
crypto: Enable SHA256 fingerprint checking in --verify-hash
This enhances --verify-hash with an optional algorithm flag. If not
provided, it defaults to SHA1 to preserve backwards compatbilitity with
existing configurations. The only valid flags are SHA1 and SHA256.
In addition enhance the layout of the --verify-hash section in the man
page.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170504204201.1257-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14538.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
ValdikSS [Wed, 10 May 2017 18:47:53 +0000 (21:47 +0300)]
Set a low interface metric for tap adapter when block-outside-dns is in use
Windows 10 before Creators Update used to resolve DNS using all
available adapters and IP addresses in parallel. Now it still
resolves addresses using all available adapters but in a round-robin
way, beginning with random adapter.
This behaviour introduces significant delay when block-outside-dns is
in use. Fortunately, setting low metric for the TAP interface solves
this issue, making Windows always pick TAP adapter first and disable
round-robin.
Steffan Karger [Tue, 9 May 2017 19:30:09 +0000 (21:30 +0200)]
Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)
Previously, if a mode was selected where packet ids are not allowed to roll
over, but renegotiation does not succeed for some reason (e.g. no password
entered in time, certificate expired or a malicious peer that refuses the
renegotiaion on purpose) we would continue to use the old keys. Until the
packet ID would roll over and we would ASSERT() out.
Given that this can be triggered on purpose by an authenticated peer, this
is a fix for an authenticated remote DoS vulnerability. An attack is
rather inefficient though; a peer would need to get us to send 2^32
packets (min-size packet is IP+UDP+OPCODE+PID+TAG (no payload), results in
(20+8+1+4+16)*2^32 bytes, or approx. 196 GB).
This is a fix for finding 5.2 from the OSTIF / Quarkslab audit.
CVE: 2017-7479 Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494358209-4568-3-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494358209-4568-3-git-send-email-steffan.karger@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Tue, 9 May 2017 19:30:08 +0000 (21:30 +0200)]
Don't assert out on receiving too-large control packets (CVE-2017-7478)
Commit 3c1b19e0 changed the maximum size of accepted control channel
packets. This was needed for crypto negotiation (which is needed for a
nice transition to a new default cipher), but exposed a DoS
vulnerability. The vulnerability was found during the OpenVPN 2.4 code
audit by Quarkslab (commisioned by OSTIF).
To fix the issue, we should not ASSERT() on external input (in this case
the received packet size), but instead gracefully error out and drop the
invalid packet.
CVE: 2017-7478 Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494358209-4568-2-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494358209-4568-2-git-send-email-steffan.karger@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Tue, 9 May 2017 18:42:48 +0000 (20:42 +0200)]
Document tls-crypt security considerations in man page
The tls-crypt commit message contained an elaborate discussion on the
function's security properties. This commit adds the gist of that
discussion, "rotate keys periodically" to the man page.
(The 'real' solution will follow later: add support for per-client
tls-crypt keys. That will make tls-crypt useful for VPN providers too.)
Note to non-crypto-geek reviewers: please verify that this text is clear
enough to explain you when you need to replace tls-crypt keys.
Note to crypto-geek reviewers: please check the numbers - see the
--tls-crypt commit message (c6e24fa3) for details.
Selva Nair [Tue, 9 May 2017 19:44:29 +0000 (15:44 -0400)]
In auth-pam plugin clear the password after use
v2: Change the plugin open to use v3 API so that secure_memzero()
exported from OpenVPN can be used.
v3: Relaxe API compatibility check: struct version 4 or higher
will have secure_memzero exported.
Note: context is cast as (openvpn_plugin_handle_t *) for consistency
with the current plugin header. If/when the header is fixed, change
this cast as well.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494359069-13824-1-git-send-email-selva.nair@gmail.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494359069-13824-1-git-send-email-selva.nair@gmail.com Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Tue, 9 May 2017 18:32:44 +0000 (20:32 +0200)]
Always clear username/password from memory on error
This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.4), we are with their analysis:
"There’s a special case where the client username and password are not
erased when the server is launched without an external script or
authentication plugin. While being invalid, this configuration does not
raise any error. If the client transmits its credentials and the session
is not established (for instance if the certificates chain has not been
verified), these credentials are not erased from memory by the server.
The likelihood of an occurrence of this issue in real life is
exceptionally low since an attacker needs elevated privileges on the
server to exploit this kind of information leak. The severity of this
issue is rated as very low."
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494354764-19354-1-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494354764-19354-1-git-send-email-steffan.karger@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
The provides plug-ins with a safe and secure way to santize sensitive
information such as passwords, by re-using the secure_memzero()
implementation in OpenVPN.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20170505184622.24520-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14546.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Mon, 8 May 2017 21:44:38 +0000 (23:44 +0200)]
Restore pre-NCP frame parameters for new sessions
As reported in trac #879, as of the introduction of NCP we always adjust
the frame parameters on session negotiations, but do not reset the frame
state for a new session on an existing state instance. That caused the
frame parameters to be reduced for each reconnect, resulting in smaller
and smaller packet size limits until no traffic could go through the
tunnel at all. This patch resolves that omission.
Trac: #879 Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1494279878-24601-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14589.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Tue, 9 May 2017 08:12:43 +0000 (10:12 +0200)]
mbedtls: correctly check return value in pkcs11_certificate_dn()
mbedtls_x509_dn_gets() would not always return -1 error, which could cause
us to incorrectly continue after the function call failed. To fix this,
just call our own x509_get_subject(), which does all the neccesary error
checking correctly.
pkcs11_certificate_dn() is only called by show_pkcs11_ids(), to list the
certificates on the pkcs11 token. Therefor, this mistake did not have a
security impact.
This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.3).
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1494317563-6303-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14591.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Inspired by
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13032.h
tml
build options are taken from regular windows installer builds
Steffan Karger [Sun, 7 May 2017 11:01:18 +0000 (13:01 +0200)]
Fix memory leak in x509_verify_cert_ku()
If keyUsage was only required to be present, but no specific value was
required, we would omit to free the extracted string. This happens as of
2.4.1, if --remote-cert-tls is used. In that case we leak a bit of
memory on each TLS (re)negotiation.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494154878-18403-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14563.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Fri, 5 May 2017 17:44:51 +0000 (19:44 +0200)]
cleanup: merge packet_id_alloc_outgoing() into packet_id_write()
The functions packet_id_alloc_outgoing() and packet_id_write() were
always called in tandem. Instead of forcing the caller to allocate a
packet_id_net to do so, merge the two functions. This simplifies the API
and reduces the chance on mistakes in the future.
This patch adds unit tests to verify the behaviour of packet_id_write().
Verifying that we assert out correctly required the change to mock_msg.c.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494006291-3522-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14541.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Mon, 24 Apr 2017 14:39:10 +0000 (16:39 +0200)]
Fix broken ./configure on systems without openssl.pc
Commit 039a89c331e9b799 changed the OpenSSL check slightly, but that
broke ./configure on systems which do not install the openssl.pc
pkg-config support file. This is typically an issue on most of the BSD
platforms, where the OpenSSL package from the base repository does not
provide that file.
We should anyway in this case have a better check of OpenSSL version
available. So in the case pkg-config fails, it will run an additional
test looking for the OpenSSL version number in the opensslv.h header
file and check against that version number.
I did consider to rip out the pkg-config test all together, but decided
to let it stay. If pkg-config works, it provides much more details to
the ./configure script than just the version number check - such as
include and library paths if those are outside the default system paths.
If the user adds OPENSSL_CFLAGS or OPENSSL_LIBS to the ./configure
script, the pkg-config will not be run. But this patch ensures that the
OpenSSL version is also checked in this situation.
This patch have been tested on Scientic Linux 7.3 (RHEL clone) and
FreeBSD 10.3-RELEASE-p11.
v5 - Remove the right OPENSSL_LIBS and preserve the old one
- In PKG_CHECK_MODULES(), check for openssl instead of libssl
+ libcrypto
- Fix tab/space issues once again
v4 - Move the CFLAGS/LDFLAGS declarations before the manual
version test; otherwise we're still testing the system install
version
v3 - Remove not needed and duplicated OPENSSL_LIBS assignment
- Fix tab/space issues in modified lines
v2 - Don't use try to simplify the version matching, use the full
OPENSSL_VERSION_NUMBER
- Fixed typo (OpneSSL -> OpenSSL)
- Improve a few comments
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170424143910.20118-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14503.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Tue, 11 Apr 2017 17:31:33 +0000 (19:31 +0200)]
Require minimum OpenSSL 1.0.1
As RHEL 5 has reached EOL, we no longer need to support OpenSSL v0.9.8.
This also makes it possible to remove a few workaronds which was
needed earlier, as well as some left overs from v0.9.6.
This also makes ./configure really stop running unless a new enough
OpenSSL library is found.
Compile tested on RHEL7.3 and RHEL6.7 (mock chroot build), both shipping
openssl-1.0.1e.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170411173133.18060-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14441.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Mon, 10 Apr 2017 22:28:28 +0000 (00:28 +0200)]
Make --cipher/--auth none more explicit on the risks
The warning provided to --cipher and --auth using the 'none' setting may
not have been too clearly understandable to non-developers or people not
fully understanding encryption and cryptography. This tries to improve
that.
While at it, also break up the long source lines.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170410222828.23612-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14436.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Wed, 29 Mar 2017 09:49:25 +0000 (11:49 +0200)]
docs: Fixed man-page warnings discoverd by rpmlint
Running rpmlint against Fedora RPM packages revealed these warnings:
W: manual-page-warning /usr/share/man/man8/openvpn.8.gz 2738:
a special character is not allowed in a name
W: manual-page-warning /usr/share/man/man8/openvpn.8.gz 2740:
a special character is not allowed in a name
This is just a typo mistake in the .B formatting, missing a trailing
space.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170329094925.25644-1-davids@openvpn.net>
URL: http://www.mail-archive.com/search?l=mid&q=20170329094925.25644-1-davids@openvpn.net Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Tue, 28 Mar 2017 20:53:46 +0000 (22:53 +0200)]
auth-token: Ensure tokens are always wiped on de-auth
If tls_deauthenticate() was called, it could in some scenarios leave the
authentication token for a session in memory. This change just ensures
auth-tokens are always wiped as soon as a TLS session is considered
broken.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170328205346.18844-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14344.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Gert Doering [Sun, 19 Mar 2017 18:41:35 +0000 (19:41 +0100)]
Make ENABLE_OCC no longer depend on !ENABLE_SMALL
OCC is useful functionality which (according to LEDE devs) adds only
about 3k to the binary size - and if the embedded router folks can
afford this trade-off, everyone else can :-)
Gert Doering [Sun, 19 Mar 2017 19:10:49 +0000 (20:10 +0100)]
Fix installation of IPv6 host route to VPN server when using iservice.
The "prepare IPv6 route message to interactive service" was properly
handing the correct interface index (r->adapter_index) for this case,
but then always overwrote the gateway address with our magic tun/tap
fe80::8 value. Only do this for "on tap adapter" routes.
Pinpointed by Selva Nair.
Trac #850
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20170319191049.23970-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14282.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Sat, 25 Feb 2017 02:02:29 +0000 (03:02 +0100)]
cleanup: Remove faulty env processing functions
The env_set_add_to_environmenti() and env_set_remove_from_environment()
functions where not used in the code at all and they would cause an
ASSERT() in setenv_str_ex() later on, as it would not allow the
struct env_set *es pointer to be NULL (misc.c:807).
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170225020229.17287-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14195.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
ignore remote-random-hostname if a numeric host is provided
Although it does not make sense to specify remote-random-hostname
when a numeric hostname is provided (being it the remote, the http
proxy or the socks server), this is still a valid configuration.
For this reason, this combination should still work as expected,
which means ignoring the randomization and directly using the
numeric IP.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170130152658.15786-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13993.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Fri, 27 Jan 2017 14:21:20 +0000 (15:21 +0100)]
plugin: Improve the handling of default plug-in directory
OpenVPN uses a default plug-in directore, set using PLUGINDIR when
running ./configure. If this is set, it will use $LIBDIR/openvpn/plugin.
When using --plugin, OpenVPN will load plug-ins from this directory with
the only exception if the plug-in filename is based on an absolute path.
Any other relative paths are relative to the PLUGINDIR.
This patch adds a third variant, using plug-in paths starting with '.'
In this case, OpenVPN will use the relative directory of where OpenVPN
was started, or the directory OpenVPN have changed into due to --cd
being used before the actual --plugin option.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170127142120.10492-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13970.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Wed, 15 Mar 2017 21:20:20 +0000 (22:20 +0100)]
Be less picky about keyUsage extensions
We long recommended users to use --ns-cert-type to distinguish between
client and server certificates, but that extension is long deprecated and
now can even no longer be accurately checked in OpenSSL 1.1+. We support
a more modern alternative, --remote-cert-tls (which expands to
--remote-cert-ku + --remote-cert-eku), but are overly strict in checking
the keyUsage. This patch makes our implementation less picky, so that
correct-but-slightly-weird certicates will not immediately be rejected.
We currently allow users to specify a list of allowed keyUsage values, and
require that the remote certificate matches one of these values exactly.
This is for more strict than keyUsage usually requires; which is that a
certificate is okay to use if it can *at least* be used for our intended
purpose. This patch changes the behaviour to match that, by using the
library-provided mbedtls_x509_crt_check_key_usage() function in mbed TLS
builds, and performing the 'at least bits xyz' check for OpenSSL builds
(OpenSSL unfortunately does not expose a similar function).
Furthermore, this patch adds better error messages when the checking fails;
it now explains that is expects to match either of the supplied values,
and only does so if the check actually failed.
This patch also changes --remote-cert-tls to still require a specific EKU,
but only *some* keyUsage value. Both our supported crypto libraries will
check the keyUsage value for correctness during the handshake, but only if
it is present. So this still enforces a correct keyUsage, but is a bit
less picky about certificates that do not exactly match expectations.
This patch should be applied together with the 'deprecate --ns-cert-type'
patch I sent earlier.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1489612820-15284-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14265.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Sat, 4 Mar 2017 18:49:57 +0000 (19:49 +0100)]
Deprecate --ns-cert-type
The nsCertType x509 extension is very old, and barely used. We already
have had an alternative for a long time: --remote-cert-tls uses the far
more common keyUsage and extendedKeyUsage extensions instead.
OpenSSL 1.1 longer exposes an API to (separately) check the nsCertType x509
extension. Since we want be able to migrate to OpenSSL 1.1, we should
deprecate this option immediately.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1488653397-2309-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14222.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
CRL: use time_t instead of struct timespec to store last mtime
As of now, we store the last mtime for the CRL file in a timespec
object. However we store seconds only and we ignore the subsecond
field (this came into being because not all platforms have nanoseconds
precision in timespec).
Given the above, we can safely replace the timespec object with a
simple time_t.
Reported-by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170316082117.21020-1-a@unstable.cc>
URL: http://www.mail-archive.com/search?l=mid&q=20170316082117.21020-1-a@unstable.cc Signed-off-by: Gert Doering <gert@greenie.muc.de>
Eric Thorpe [Wed, 15 Mar 2017 22:40:31 +0000 (09:40 +1100)]
Fix Building Using MSVC
This patch enables the building of OpenVPN for the 2.4 and master
branches using MSVC (Visual Studio 2013 / MSVC v120), which currently
doesn't work with 2.4 or a clone of master. 2013 is being used as it
reduces the complexity of the redistributable requirements and has
mostly complete C99 support. Further changes will be necessary for 2015
support when the switch is made.
Note the changes to config-msvc-version.h.in are more of a work around.
It was a simpler approach when compared to modifying msvc-generate.js to
handle m4 syntax, and so it may be dropped if there is an intention to
update the javascript generator.
Signed-off by: Eric Thorpe <eric@sparklabs.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <df273b9b-6ca4-a539-cdf5-d4f9f991896b@sparklabs.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14268.html
Steffan Karger [Thu, 9 Mar 2017 08:13:32 +0000 (09:13 +0100)]
Remove duplicate X509 env variables
Commit 13b585e8 added support for multiple X509 env variables with the
same name, but as a side effect caused these variables to pile up for
each renegotiation. The old code would simply overwrite the old variables
(as long as an equally-long chain was used for the new session).
To stop the variables from piling up, this commit removes any old X509
env variables if we start negotiating a new TLS session.
Trac: #854
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1489047212-31994-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14237.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Thu, 9 Mar 2017 10:47:35 +0000 (11:47 +0100)]
Fix types in WIN32 socket_listen_accept()
SOCKET_UNDEFINED is of type socket_descriptor_t (or SOCKET, in MS types),
so new_sd should be too. Also, the return value of this function is
always stored in a socket_descriptor_t variable, so it should return that
type (which makes sense now, because it returns new_sd) instead of an int.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1489056455-6004-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14239.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ilya Shipitsin [Sun, 5 Mar 2017 17:21:32 +0000 (20:21 +0300)]
travis-ci: remove unused files
Those files were commited by mistake. I implemented building
dependencies in 4 separate scripts, later Steffan Karger combined
all 4 scripts into "build-deps.sh".
Ilya Shipitsin [Sat, 25 Feb 2017 18:00:04 +0000 (23:00 +0500)]
travis-ci: add 'make distcheck' to test scenario, V2
in rare cases openvpn is built from tarball, it happens during "installer
build"
process. "make distcheck" helps to prevent problems during such builds.
V2: limit "make distcheck" to one build configuration Signed-off-by: Ilya Shipitsin <chipitsine@gmail.com> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1488045604-25460-1-git-send-email-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14208.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Emmanuel Deloget [Mon, 20 Feb 2017 14:32:34 +0000 (15:32 +0100)]
OpenSSL: use EVP_CipherInit_ex() instead of EVP_CipherInit()
The behavior of EVP_CipherInit() changed in OpenSSL 1.1 -- instead
of clearing the context when the cipher parameter was !NULL, it now
clears the context unconditionnaly. As a result, subsequent calls
to the function with additional information now fails.
The bulk work is done by EVP_CipherInit_ex() which has been part of the
OpenSSL interface since the dawn of time (0.9.8 already has it). Thus,
the change allows us to get the old behavior back instead of relying
on dirty tricks.
Gert Doering [Fri, 24 Feb 2017 13:52:22 +0000 (14:52 +0100)]
Fix '--dev null'
To test whether a server is reachable and all the key handling is
right, openvpn can connect with "--dev null --ifconfig-noexec" to
avoid needing to the client with elevated privileges.
This was erroring out for no good reason (because the "set environment
variables appropriately" code didn't know if this is a tun or tap
device...) - treat --dev null as "tap", done.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170224135222.44640-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14186.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Thu, 23 Feb 2017 08:49:54 +0000 (09:49 +0100)]
Add openssl_compat.h to openvpn_SOURCES
Commit b936ddfb63 introduced a new header file but forgot to include
it in the list of openvpn_SOURCES, so it did not get bundled in the
generated tarballs.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170223084954.58464-1-gert@greenie.muc.de>
URL: http://www.mail-archive.com/search?l=mid&q=20170223084954.58464-1-gert@greenie.muc.de Signed-off-by: Gert Doering <gert@greenie.muc.de>
Emmanuel Deloget [Fri, 17 Feb 2017 22:00:48 +0000 (23:00 +0100)]
OpenSSL: don't use direct access to the internal of X509_STORE_CTX
OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including X509_STORE_CTX. We have to use the defined
functions to do so.
Fortunately, these functions have existed since the dawn of time so
we don't have any compatibility issue here.
fix redirect-gateway behaviour when an IPv4 default route does not exist
When no IPv4 default route exists, the "redirect-gateway" routine
aborts even if the sub-option "local" was specified or if we are
connecting to the remote host using IPv6.
This is not expected because in either case OpenVPN should not
bother checking the existence of the default route as it is not
required at all.
Therefore, skip the IPv4 default route check when "local" is
specified or we are connecting to an IPv6 remote host.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170119162518.31752-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13905.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
attempt to add IPv6 route even when no IPv6 address was configured
Even if no IPv6 address is configured, OpenVPN still supports
transporting IPv6 segments, therefore adding an IPv6 route
should always be allowed.
However, the route might fail to be installed or may just not work
as expected, therefore, a proper warning should be printed to inform
the user of the possible pitfall.
Always allow adding an IPv6 route and print a WARNING when
no IPv6 address is configured for the interface.
Trac: #832 Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170131112131.13570-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13994.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ilya Shipitsin [Mon, 6 Feb 2017 06:55:15 +0000 (11:55 +0500)]
github: Add PR template with contributor related information
Add PR template in order to simplify new developers cooperate properly if
they open PR. discussed here:
https://sourceforge.net/p/openvpn/mailman/message/35601310/
v2: Changed links and provided an example of "git send-email"
[DS: Cleaned up commit message, changed order of URLs at commit time]
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1486364115-9801-1-git-send-email-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14010.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Ilya Shipitsin [Sun, 29 Jan 2017 06:58:11 +0000 (11:58 +0500)]
Resolve several travis-ci issues
MBEDTLS_VERSION, OPENSSL_VERSION were defined twice - in both
.travis.yml and .travis/build-deps.sh files, the last one
defined OPENSSL_VERSION via nonexistent OPENSSL_VERION
variable, which lead us to use openssl-1.0.1 instead of
openssl-1.0.2, I removed variable definition from build-deps.sh
"cache: [ apt: true ]" is not a travis supported option, it was
introduced by mistake, I removed it
LD_LIBRARY_PATH was defined for the entire test run, it includes
custom openssl build, which was picked by "wget", so "wget"
could not verify SSL cert at https://www.openssl.org sometimes.
We do not want wget to pick our custom LD_LIBRARY_PATH, so I moved
that variable to "script" section
LD_LIBRARY_PATH was defined for both linux and osx environments,
for the second DYLD_LIBRARY_PATH must be defined instead
v2: Upgrade openssl, mbedtls to the most recent versions
v3: DYLD_LIBRARY_PATH was defined via LD_LIBRARY_PATH by mistake
Christian Hesse [Tue, 24 Jan 2017 14:39:47 +0000 (15:39 +0100)]
systemd: Do not race on RuntimeDirectory
Different unit instances create and destroy the same RuntimeDirectory.
This leads to running instances where the status file (and possibly
more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d
configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content.
As /run is volatile we do not have to care about cleanup.
Signed-off-by: Christian Hesse <mail@eworm.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170124143947.27385-2-list@eworm.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13939.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Christian Hesse [Tue, 24 Jan 2017 14:39:46 +0000 (15:39 +0100)]
systemd: Use automake tools to install unit files
If systemd is enabled we install unit files to $libdir/systemd/system
(or the path specified by SYSTEMD_UNIT_DIR).
The unit files are generated on the fly with matching $sbindir.
Signed-off-by: Christian Hesse <mail@eworm.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170124143947.27385-1-list@eworm.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13940.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Tue, 24 Jan 2017 23:23:44 +0000 (00:23 +0100)]
systemd: Move the READY=1 signalling to an earlier point
Currently, OpenVPN will first tell systemd it is ready once the
log will be appended with "Initialization Sequence Completed".
This turns out to cause some issues several places.
First, it adds challenges if --chroot is used in the configuration;
this is already fixed. Secondly, it will cause havoc on static key
p2p mode configurations where the log line above will not happen
before either sides have completed establishing a connection. And
thirdly, if a client configuration fails to establish a connection
within 90 seconds, it will also fail. For the third case this may
not be a critical issue itself, as the host just needs to get
an Internet access established first - which in some scenarios may
take much longer than those 90 seconds systemd grants after the
OpenVPN client configuration is started.
The approach this patch takes is to consider OpenVPN ready when
all the initial preparations and configurations have completed - but
before a connection to a remote side have been attempted. This
also removes the need for specially handling the --chroot scenario.
The final "Initialization Sequence Completed" message update is
kept (though slightly simplified) to indicate we're in a good
state - even though this update will not be visible if --chroot
is used (which was the situation also before this patch).
Trac: #827, #801 Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Christian Hesse <mail@eworm.de>
Message-Id: <20170124232344.7825-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13945.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Sun, 22 Jan 2017 16:04:41 +0000 (17:04 +0100)]
Use SHA256 for the internal digest, instead of MD5
Our internal options digest uses MD5 hashes to store the state, instead of
storing the full options string. There's nothing wrong with that, but it
would still be better to use SHA256 because:
* That makes it easier to make OpenVPN "FIPS-compliant" (forbids MD5)
* We don't have to explain anymore that MD5 is fine too
The slightly less bytes for the digest (16 instead of 32) and operations
per connection setup are not worth sticking to MD5.
Note that might SHA256 not be available in de crypto lib, OpenVPN will
refuse to start and shout "Message hash algorithm 'SHA256' not found".
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1485101081-9784-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13926.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Fri, 20 Jan 2017 21:04:57 +0000 (22:04 +0100)]
git: Merge .gitignore files into a single file
We already track a lot of files over the whole directory structure
in the main .gitignore file. But a few additional ones had been
added into some of the subdirectories.
This unifies all these files into a master file for the whole project,
making it easier to know where to look at and edit if changes needs
to be done.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170120210457.3383-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13916.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Sat, 14 Jan 2017 14:10:20 +0000 (15:10 +0100)]
More broadly enforce Allman style and braces-around-conditionals
We want { and } aligned, which means also adding a newline between each
for() and {, while() and {, etc.
Also, we agreed to always use braces with conditionals. The previous
uncrustify config added these for if()s, now also add these for while()
and for().
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1484403020-6857-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13875.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Tue, 10 Jan 2017 20:34:32 +0000 (21:34 +0100)]
management: >REMOTE operation would overwrite ce change indicator
If the management interface on a client received a signal while waiting
for input on the management channel, the "connection entry changed" status
would be overwritten even though nothing was changed. Which could lead
into connecting to the wrong server.
This patch improves this by adding a check if a bool value was changed to
false. This change happens only on signals.
Further, the former 'ret' value have been renamed to 'ce_changed', to
clarify what the expected return value contains. Plus adding some comments
related to this.
And finally do some code style cleanup, breaking up too long lines, adding
some air here and there to improve the readability.
Signed-off-by: David Sommerseth <davids@openvpn.net> Cc: Selva Nair <selva.nair@gmail.com> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1484080473-10415-1-git-send-email-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13851.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Selva Nair [Tue, 3 Jan 2017 20:38:03 +0000 (15:38 -0500)]
Always release dhcp address in close_tun() on Windows.
Also make sure --dhcp-pre-release results in not just dhcp_release()
in open_tun() but a subsequent dhcp_renew() as well. Else dhcp transaction
gets aborted as this call to release() happens after the adapter status
is changed to connected.
Fixes Trac #807 (but can't say the same for Trac #665 without knowing
how to reproduce it)
v2: Mark --dhcp-release as obsolete in manpage and option parser, and
remove the unused dhcp_release variable.
Enforce dhcp-renew with dhcp-pre-release while parsing the option
instead of in open_tun().
Trac: #807
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1483475883-17450-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13814.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gisle Vanem [Mon, 2 Jan 2017 16:17:51 +0000 (17:17 +0100)]
Crash in options.c
When compiling with --disable-crypto, openvpn would crash on --help as
commit 5d429efd97 introduce and extra %d into the "usage_message" string
but forgot to add it to the #ifndef ENABLE_CRYPTO fprintf() call.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <9d41f9dd-a587-5c1e-2e0d-ebb6c921f4ae@yahoo.no>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13808.html
Steffan Karger [Mon, 26 Dec 2016 19:15:43 +0000 (20:15 +0100)]
Textual fixes for Changes.rst
We will likely refer many people to the Changes.rst file once we've
released 2.4. This commits tries to polish the language a bit, and
adds two real changes:
- Remove duplicate mention of the changes --tls-cipher defaults
- Move the 'redirect-gateway' behavioural change from 'features' to
'behavioural changes'.
v2 - On the fly commit changes, based on comments from Selva Nair.
DS also added a few minor corrections on top of that.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1482779743-9548-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13732.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Sun, 25 Dec 2016 22:02:14 +0000 (23:02 +0100)]
man: encourage user to read on about --tls-crypt
As suggested by krzee in trac #790, refer to the --tls-crypt option
form the --tls-auth section of the man page, to encourage users to
check out the --tls-crypt feature.
Trac: #790 Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1482703334-18949-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13713.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Sun, 25 Dec 2016 22:38:25 +0000 (23:38 +0100)]
Document that RSA_SIGN can also request TLS 1.2 signatures
Ever since we support TLS 1.2 (OpenVPN 2.3.3+), the RSA_SIGN might not
only request MD5-SHA1 'TLS signatures', but also other variants.
Document this by updating the implementation hints, and explicitly
stating that we expect a PKCS#1 1.5 signature.
Trac: #764 Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1482705505-20302-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13714.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Mon, 19 Dec 2016 18:52:12 +0000 (19:52 +0100)]
dev-tools: Added script for updating copyright years in files
Very simple tool which modifies the Copyright lines in all git checked-in
files with an updated year. Lines only listing a single year (2016) will
be modified to list a range instead.
Only the Copyright lines owners of specific owners will be modified. The
script will need to be slightly updated to cover more owners. See the
UPDATE_COPYRIGHT_LINES line in the script for the currently set owners.
v2 - On-the-fly-commit-update: use vendor/ instead of cmocka and
add @sophos.com to the list of copyright holders to update
Steffan Karger [Thu, 15 Dec 2016 21:46:06 +0000 (22:46 +0100)]
Don't reopen tun if cipher changes
When the pulled options change, OpenVPN will attempt to reopen the tun
device. That might fail if the process has already dropper privileges,
and is not needed unless the tun MTU is changed. This patch therefore
ignores the cipher value for the digest if a fixed tun-mtu is used.
Additionally, this patch changes the md_ctx_update() call to include the
trailing zero byte of each option, to make sure that parsing "foo,bar"
results in a different hash than "foobar". (Sorry for not catching that
during the review...)
The unit tests are a bit lame, but it secretly serves as a way to lower
the bar for adding more buffer.c unit tests.
Trac: #761 Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1481838366-32335-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13579.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Wed, 14 Dec 2016 21:05:00 +0000 (22:05 +0100)]
dev-tools: Add reformat-all.sh for code style unification
This script will run all files related to the currently checked out
git branch through uncrustify using a standardized style configuration.
Due to a bug in uncrustify 0.64, it is needed to add a special treatment
to one of the files at the moment. So this both pre- and post-patched
before/after uncrustify is run. This is to simply to assure that all
file processing will happen consistently each time.
Also added doc/doxygen/doc_key_generation.h to an ignore list, as
it carries some specific Doxygen formatting we should be careful with.
This file is anyhow not so critical and can be managed manually.
The src/compat/compat-lz4.[ch] files are also not touched, as they
are based on upstream formatting. This makes it easier to update
to a newer LZ4 version later on and even see what the differences
are.
v2 - Include updated config from CodeStyle wiki page
Remove line lenght restriction for The Great Reformatting
Update the script with improvements by krzee
v3 - Update with a fixed config from the CodeStyle wiki page
Corrected a typo in the commit message (0.63->0.64)
Minor changes to the reformat script (no pushd/popd,
some new lines moved around, bash->sh)
David Sommerseth [Tue, 13 Dec 2016 12:16:56 +0000 (13:16 +0100)]
Changes.rst: Mainatiner update on C99
Mention for maintainers that we've moved to build with -std=c99 by
default. Also document that 32-bit RHEL5 builds will need -std=gnu99
to be buildable.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1481631416-15377-1-git-send-email-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13518.html Signed-off-by: David Sommerseth <davids@openvpn.net>