Selva Nair [Tue, 21 Nov 2017 01:43:25 +0000 (20:43 -0500)]
Correct the declaration of handle in 'struct openvpn_plugin_args_open_return'
- This is an opaque pointer so the change should not affect
existing plugins. But it makes the code consistent and clears up
the documentation as the handle pointer is treated as of type
"openvpn_plugin_handle_t" in the rest of the code.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1511228605-23207-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15908.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a2f43c2d6f086e7aa8b6160793f0c462ee9d6aa7)
Simon Rozman [Thu, 19 Apr 2018 11:23:13 +0000 (13:23 +0200)]
Add Interactive Service developer documentation
The OpenVPN Interactive Service documentation from
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService was
upgraded with a description of the client-service communication flow,
service registry configuration, and non-default instance installation.
Steffan Karger [Sun, 3 Jun 2018 10:11:56 +0000 (12:11 +0200)]
man: add security considerations to --compress section
As Ahamed Nafeez reported to the OpenVPN security team, we did not
sufficiently inform our users about the risks of combining encryption
and compression. This patch adds a "Security Considerations" paragraph
to the --compress section of the manpage to point the risks out to our
users.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1528020718-12721-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16919.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a59fd1475089eda4c89942d345070bb942180223)
Gert Doering [Sat, 14 Apr 2018 07:26:17 +0000 (09:26 +0200)]
Fix potential double-free() in Interactive Service (CVE-2018-9336)
Malformed input data on the service pipe towards the OpenVPN interactive
service (normally used by the OpenVPN GUI to request openvpn instances
from the service) can result in a double free() in the error handling code.
This usually only leads to a process crash (DoS by an unprivileged local
account) but since it could possibly lead to memory corruption if
happening while multiple other threads are active at the same time,
CVE-2018-9336 has been assigned to acknowledge this risk.
Fix by ensuring that sud->directory is set to NULL in GetStartUpData()
for all error cases (thus not being free()ed in FreeStartupData()).
Rewrite control flow to use explicit error label for error exit.
Discovered and reported by Jacob Baines <jbaines@tenable.com>.
Gert van Dijk [Sat, 11 Nov 2017 16:11:21 +0000 (17:11 +0100)]
manpage: improve description of --status and --status-version
Signed-off-by: Gert van Dijk <gert@gertvandijk.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171111161122.30087-1-gert@gertvandijk.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15818.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 308c9d7f001a97daebcccf503f255947c0e09183)
Selva Nair [Tue, 6 Mar 2018 06:09:28 +0000 (01:09 -0500)]
Avoid overflow in wakeup time computation
Time interval arithmetic can overflow especially when user
defined intervals are involved. E.g., see Trac #922.
Avoid this by reordering the arithmetic operation in
event_timeout_trigger(). Also avoid unnecessary casting of time
variable to int.
Time until wakeup is now calculated like:
time_t wakeup = (last - now) + delay
Here delay is of type int, but is +ve by construction. Time backtrack
protection in OpenVPN ensures (last - now) <= 0. Then the above
expression cannot overflow (provided time_t is at least as large
as int).
A similar expression in interval.h is also changed.
(This patch grew out of patch 168 by Steffan Karger.)
Steffan Karger [Thu, 4 Jan 2018 12:07:50 +0000 (13:07 +0100)]
Check for more data in control channel
If control channel packets arrive quickly after each other, or out of
order, there might be more data available than we can read in one
tls_process() call. If that happened, and no further control channel
packet arrived (e.g. because the last two packets arrived out-of-order),
we would wait for 16 second ("coarse timer") before we would read the
remaining data. To avoid that, always schedule ourself again if there
was control channel data, to check whether more data is available.
For mbedtls, we could implement a slightly more elegant "is there more
data?" function, instead of blindly rescheduling. But I can't find a way
to implement that for OpenSSL, and the current solution is very simple and
still has quite low overhead.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1515067670-13094-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16151.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit b00d56e1b0cf4d71dc4944ef14ea7eca2fc8c519)
Joost Rijneveld [Wed, 28 Feb 2018 13:52:40 +0000 (14:52 +0100)]
Make return code external tls key match docs
In tls_ctx_use_external_private_key, the return codes were inverted
compared to what is documented in ssl_backend.h (and what can
reasonably be expected). Internally the return code is never checked,
so this did not directly result in any change of behavior. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180228135240.22945-1-joost@joostrijneveld.nl>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16577.html
Simon Matter [Fri, 2 Mar 2018 07:49:31 +0000 (08:49 +0100)]
Add missing #ifdef SSL_OP_NO_TLSv1_1/2
Release/2.4 supports older OpenSSL versions than master, so when
cherrypicking f8a92a4393a -> 2d705accea3e53 these code bits should
have received an #ifdef to ensure compatibility (as done for the
same define in other places in 2.4 already). Add them now.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <52e860ea74ac958309368374049f14bd.squirrel@webmail.bi.invoca.ch>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16588.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Wed, 28 Feb 2018 13:19:18 +0000 (14:19 +0100)]
management: Warn if TCP port is used without password
It is not recommended to use --management on a TCP port without also
adding a password authentication, as this can easily be abused by other
users or processes being able to connect to the managmement interface.
Thus issue a warning that this configuration is strongly discouraged.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180228131918.12954-3-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16574.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 4db7715a3aa62f2e8d8234c1852fb141f62318e2)
Steffan Karger [Wed, 1 Nov 2017 22:03:41 +0000 (23:03 +0100)]
Don't throw fatal errors from create_temp_file()
This function is called in response to connecting clients, and can fail
when I/O fails for some (possibly temporary) reason. In such cases we
should not exit the process, but just reject the connecting client.
This commit changes the function to actually return NULL on errors, and
(where needed) changes the callers to check for and handle errors.
Since the tls-crypt-v2 metadata code also calls create_temp_file() when
clients connect, I consider this a prerequisite for tls-crypt-v2.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20171101220342.14648-4-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15701.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 3e0fd2b0471cf4e53959902ca10d88db7a1ef916)
David Sommerseth [Wed, 28 Feb 2018 13:19:17 +0000 (14:19 +0100)]
man: Reword --management to prefer unix sockets over TCP
It is more secure to use unix sockets instead of TCP ports for the
management interface, so reword it and provide some details why TCP is
not recommended.
Also re-arranged this section to be somewhat easier to read and clearer
on a few related details.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180228131918.12954-2-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16573.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit ec100d7e4ce7aaeb731c22b0d86826bf295df6cd)
Steffan Karger [Wed, 7 Feb 2018 12:22:46 +0000 (13:22 +0100)]
mbedtls: don't use API deprecated in mbed 2.7
The void-returning mbedtls_sha256() was deprecated in mbed TLS 2.7.
Use our own md_full() abstraction instead.
(The new function can theoretically fail, but only in case of highly
unlikely digest function failures. The personalisation on random using
the certificate is a best-effort measure, so we simply log a warning and
skip the personalisation if such highly unlikely errors occur.)
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <1518006166-14285-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16445.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit f22e89bd2311d3cab511e574746c6f82f1fa1a54)
Selva Nair [Thu, 22 Feb 2018 04:33:37 +0000 (23:33 -0500)]
Fix format spec errors in Windows builds
- Correct an instance of %s used for wchar_t * (should be %ls)
and some %d for DWORD or %lu for int.
- Cast socket descriptor to (int) during i/o as its unsigned int
or int64 in Windows but signed int in other platforms.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1519274017-19921-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/search?l=mid&q=1519274017-19921-1-git-send-email-selva.nair@gmail.com Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Tue, 20 Feb 2018 20:25:08 +0000 (21:25 +0100)]
Get rid of ax_check_compile_flag.m4
The macro was too new for some of the platforms we still support. In
particular, centos/rhel 6 and opensolaris 10. To work around that, we
introduce our own simpler and more tailored ACL_CHECK_ADD_COMPILE_FLAGS
macro, that not only checks but also sets the flags in CFLAGS if it is
accepted. Since this doesn't use new-and-shine autoconf features, it
should also work on the legacy platforms.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180220202508.16201-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16515.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 6a5d10e96b9ad2f9a9472aeee8cdb7c02fe4d050)
Selva Nair [Wed, 21 Feb 2018 05:38:30 +0000 (00:38 -0500)]
Adapt to RegGetValue brokenness in Windows 7
- RegGetValue with flags = RRF_RT_REG_SZ|RRF_RT_REG_EXPAND_SZ
fails in Windows 7 with an "invalid parameter" error.
Fix by using RRF_RT_REG_SZ alone.
Note: This is not a regression as in no released version did the
service support expandable strings (ones with embedded %FOO%) in
the registry. However, the GUI does expand such strings. The two
can be made consistent by explicitly expanding the strings -- that
is left for a future patch.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1519191510-3826-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16513.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7de0ee4f6f6f44fab48717e4cc2073ff4e8580f6)
Selva Nair [Wed, 24 Jan 2018 17:31:45 +0000 (12:31 -0500)]
Use lowest metric interface when multiple interfaces match a route
Currently a route addition using IPAPI or service is skipped if the
route gateway is reachable by multiple interfaces. This changes that
to use the interface with lowest metric. Implemented by
(i) Do not over-write the return value with TUN_ADAPTER_INDEX_INVALID in
windows_route_find_if_index() if multiple interfaces match a route.
(ii) Select the interface with lowest metric in adapter_index_of_ip()
instead of the first one found when multiple interfaces match.
Reported by Jan Just Keijser <janjust@nikhef.nl>
Signed-off-by: Selva Nair <selva.nair@gmail.com> Tested-by: Jan Just Keijser <janjust@nikhef.nl> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1516815105-17882-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16347.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 3854d4040e0d6fd2a58292e8bb1c1fbae5c17bb1)
Selva Nair [Sat, 18 Nov 2017 17:40:58 +0000 (12:40 -0500)]
Make most registry values optional
Not all installations need registry values such as log_dir and
config_dir especially if automatic service is not in use.
This patch provides reasonable defaults for registry values.
- Read the default value of HKLM\Software\PACKAGE_NAME to get the
install path and construct defaults for exe_path, config_dir,
log_dir from it. Use "ovpn", "0", NORMAL_PRIORITY as the defaults
for config file extension, log-append flag and process priority.
The only remaining required registry entry is the root key (usually
HKLM\Software\OpenVPN) whose default value should be set to the
installation path.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1511026858-23281-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15892.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit db04bca6729e9fe1ea60f0b3bd0329244a6ed611)
Selva Nair [Sat, 18 Nov 2017 17:40:57 +0000 (12:40 -0500)]
Ensure strings read from registry are null-terminated
- Strings stored in registry are not guaranteed to be null-terminated.
So, use RegGetValue() instead of RegQueryValueEx() as the former
adds null termination to the returned string if missing.
(Needs Windows Vista+)
- While at it also add a default value parameter to GetRegString()
to process optional registry values (such as ovpn_admin_group)
without causing an otherwise confusing error logged to the
eventlog[*].
[*] see Trac: #892
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1511026858-23281-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15893.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b1263b06db40f21a8fd20e0efd0c12e37ce89a2c)
Arne Schwabe [Wed, 31 Jan 2018 09:53:00 +0000 (10:53 +0100)]
show the right string for key-direction
V2: print also a nice string if direction is not set
V3: really include V2 changes Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1517392380-21597-1-git-send-email-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16415.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7f7f00da88eeea847da57f4f34c66c1f4a935a73)
Steffan Karger [Thu, 1 Feb 2018 15:45:21 +0000 (16:45 +0100)]
Enable stricter compiler warnings by default
This by default enables the compiler warnings one could previously
enable using the --enable-strict configure option. I think it is
okay to do so now, because we've taken care of many warnings in the
more standard builds. (Most of those were totally harmless, but they
prevented us from spotting new more serious mistakes.)
The --enable-strict flag now enables two extra warning flags that I
think can be useful:
-Wsign-compare warns when the compiler promotes a signed type to
unsigned before comparing, which can lead to unexpected behaviour.
-Wuninitialized adds extra warnings about usage of uninitialized variables
or struct elements.
Steffan Karger [Sun, 11 Feb 2018 10:19:29 +0000 (11:19 +0100)]
Log pre-handshake packet drops using D_MULTI_DROPPED
We have a debug level packets dropped by the TLS layer - use that for this
packet drop too. This changes this message from 'verb 3' to 'verb 4'
(which should result in less user reports about this almost always
harmless warning).
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180211101929.4535-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16477.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit c215c58f2393e881e16f9805549316a1e257a682)
Selva Nair [Wed, 6 Dec 2017 04:28:41 +0000 (23:28 -0500)]
Refactor get_interface_metric to return metric and auto flag separately
- Instead of returning metric = 0 when automatic metric is in use
return the actual metric and flag automatic metric through a
parameter. This makes the function reusable elsewhere.
- Ensure return value can be correctly cast to int and return -1 on
error.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Simon Rozman <simon@rozman.si>
Message-Id: <1512534521-14760-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16039.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 4229243563bcb22990f71d50e25be9ea6d44f519)
Steffan Karger [Wed, 17 Jan 2018 13:16:24 +0000 (14:16 +0100)]
Plug memory leak if push is interrupted
If a push is interrupted due to a timeout, c->c2.pulled_options_state is
never freed. Fix that by always cleaning up any remaining pulled
options state when we close a connection.
This changes the mbedtls implementation of md_ctx_cleanup to actually
clean up the context, which was not needed earlier.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1516194984-1540-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16265.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 07036fd3c456ed4ebf1809d8d9f34941d42865d0)
Selva Nair [Sat, 20 Jan 2018 04:52:54 +0000 (23:52 -0500)]
TLS v1.2 support for cryptoapicert -- RSA only
- If an NCRYPT handle for the private key can be obtained, use
NCryptSignHash from the Cryptography NG API to sign the hash.
This should work for all keys in the Windows certifiate stores
but may fail for keys in a legacy token, for example. In such
cases, we disable TLS v1.2 and fall back to the current
behaviour. A warning is logged unless TLS version is already
restricted to <= 1.1
Steffan Karger [Sat, 20 Jan 2018 09:42:28 +0000 (10:42 +0100)]
Fix --tls-version-min and --tls-version-max for OpenSSL 1.1+
As described in <80e6b449-c536-dc87-7215-3693872bce5a@birkenwald.de> on
the openvpn-devel mailing list, --tls-version-min no longer works with
OpenSSL 1.1. Kurt Roeckx posted in a debian bug report:
"This is marked as important because if you switch to openssl 1.1.0
the defaults minimum version in Debian is currently TLS 1.2 and
you can't override it with the options that you're currently using
(and are deprecated)."
This patch is loosely based on the original patch by Kurt, but solves the
issue by adding functions to openssl-compat.h, like we also did for all
other openssl 1.1. breakage. This results in not having to add more ifdefs
in ssl_openssl.c and thus cleaner code.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180120094228.13285-1-steffan@karger.me>
URL: https://www.mail-archive.com/search?l=mid&q=20180120094228.13285-1-steffan@karger.me Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked with 2.4 amendments from commit 0e8a30c0b05c1e2b59a1dea0a6eab5daa1d9d9a1)
Emmanuel Deloget [Fri, 12 Jan 2018 16:48:24 +0000 (17:48 +0100)]
OpenSSL: check EVP_PKEY key types before returning the pkey
The internal EVP_PKEY::pkey member is an union thus we need to check for
the real key type before we can return the corresponding RSA, DSA or EC
public key.
Steffan Karger [Fri, 29 Dec 2017 09:47:37 +0000 (10:47 +0100)]
travis: use clang's -fsanitize=address to catch more bugs
The clang address sanitizer is able to catch quite a number of
memory-related bugs, such add memory leaks and buffer under/overruns.
So, enable the address sanitizer for one openssl and one mbedtls build.
This would have caught the buffer list unittest memory leak that
<1512724338-22197-1-git-send-email-steffan@karger.me> wants to fix.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1514540857-19290-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16102.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7b11915ddfe97d8c28f998db54c40384a4eafb93)
Ilya Shipitsin [Thu, 4 Jan 2018 19:37:10 +0000 (00:37 +0500)]
travis-ci: add brew cache, remove ccache
1-2 minutes speedup osx builds by using brew cache.
Also, ccache was removed for a while (builds fail
after travis-ci upgraded clang to version 5.0.0) Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20180104193710.23778-1-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16154.html
Steffan Karger [Thu, 14 Dec 2017 10:21:37 +0000 (11:21 +0100)]
ssl_openssl: fix compiler warning by removing getbio() wrapper
An API change in openssl 1.1 made the BIO_METHOD * returned by BIO_f_ssl()
and BIO_s_mem() const, as well as the BIO_METHOD * argment of BIO_new()
const. This meant that our getbio() function would either have an API
inconsistent with 1.0 or 1.1.
The wrapper was basically an ASSERT, so fix this by replacing the wrapper
with an ASSERT.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1513246897-28171-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16083.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 006d6a57b8835c15222359bfb42c95005723394c)
Steffan Karger [Wed, 10 Jan 2018 08:34:19 +0000 (09:34 +0100)]
Fix types around buffer_list_push(_data)
In C, strings are char pointers, not unsigned char pointers. And
arbitrary data is represented by a void pointer. Change buffer_list_push
and buffer_list_push_data to follow these rules, and remove any now
unneeded casts.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1515573259-20968-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16186.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b395f36e578b2def9da8e9347c0afa79814c0c7d)
As pointed out in finding OVPN-05 of the cryptograpy engineering audit
(funded by Private Internet Access), buffer_list_aggregate_separator()
could perform a 0-byte malloc when called with a list of 0-length buffers
and a "" separator. If other could would later try to access that buffer
memory, this would result in undefined behaviour. To prevent this, always
malloc() 1 byte.
To simplify as we go, use alloc_buf() to allocate the buffer. This has
the additional benefit that the actual buffer data (not the contents) is
zero-terminated, because alloc_buf() calls calloc() and we have 1 extra
byte of data.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1514541240-19536-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16106.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 748902f46260fe11cb25726d2bf93bb06ad338f2)
buffer_list_aggregate_separator() would merge buffer_list entries until it
had exceeded the provided max_len, instead of stopping *before* exceeding
the max value.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1514541191-19471-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16104.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit fb6138dd32cf01922d7ef670d502148596511268)
Allow learning iroutes with network made up of all 0s (only if netbits < 8)
It is plausible for a user to be willing to add a route for a network
made up of all 0s via a VPN client (i.e. 0.0.0.0/1), therefore such
iroute should be supported.
As of now the option parsing code will accept such iroute, but
the learning routine will (silently) reject it after a sanity check.
Such check prevents routes with network made up of all 0s to be
learnt at all..
Change the sanity check so that it will reject iroutes to network
made up of 0s only when netbits is greater than 7.
The reason for choosing 7 is because anything within 0.0.0.0/8 is not
really routable among networks.
While at it, make the sanity check louder so that it can print the
reason why a route is being rejected.
Trac: #726 Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20171206154356.30764-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16044.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit a19c56db9bd42b7b8c4a8f353f7db92781397cec)
reload HTTP proxy credentials when moving to the next connection profile
The HTTP proxy credentials are stored in a static variable that is
possibly initialized before each connection attempt.
However, the variable is never "released" therefore get_user_pass()
refuses to overwrite its content and leaves it as it is.
Consequently, if the user config contains multiple connection profiles
with different http-proxy, each having its own credentials, only the
first user/pass couple is loaded and the others are all ignored.
This leads to connection failures because the proper credentials are
not associated with the right proxy server.
The root of the misbehaviour seems to be located in the fact that,
despite the argument force passed to get_user_pass_http() being true,
no action is taken to release the static object containing the
credentials.
Fix the misbehaviour by releasing the http-proxy credential object
when the reload is "forced".
Trac: #836 Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Steffan Karger <steffan@karger.me> Tested-by: David Sommerseth <davids@openvpn.net> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20171204044907.32261-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16007.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 86b58ceb29cf1cc3acf32e2ff370d9a4af68c051)
Simon Rozman [Sun, 3 Dec 2017 21:16:54 +0000 (22:16 +0100)]
openvpnserv: Add support for multi-instances
While openvpn.exe can run multiple concurrent processes, openvpnserv.exe
is usually only one single globally unique running process.
This patch extends openvpnserv.exe to support multiple service instances
in parallel allowing side-by-side OpenVPN installations.
Alternate instances must be installed as `SERVICE_WIN32_OWN_PROCESS`
(Type 0x10) and must use the newly introduced service command line
parameter:
-instance <name> <id>
<name> can be `automatic` or `interactive`.
- The service settings will be loaded from `HKLM\Software\OpenVPN<id>`
registry key.
- The automatic service will use `openvpn<id>_exit_1` exit event.
- The interactive service will accept requests on
`\\.\pipe\openvpn<id>\service` named pipe, and run IPC with
openvpn.exe on `\\.\pipe\openvpn<id>\service_<pid>`.
This patch preserves backward compatibility, by defaulting to
`SERVICE_WIN32_SHARE_PROCESS` and `<empty string>` as service ID.
Steffan Karger [Fri, 24 Nov 2017 13:58:23 +0000 (14:58 +0100)]
Use P_DATA_V2 for server->client packets too
P_DATA_V2 introduced the peer-id. This allows clients to float, but as a
side-effect 32-bit aligns the encrypted data. That alignment improves
performance particularly on cheaper/older CPUs. So although servers don't
actually have a peer-id, still use the V2 packet format (with a zero-id)
for server->client traffic too.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1511531903-19349-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1511531903-19349-1-git-send-email-steffan.karger@fox-it.com Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 3b9cce657b0ba876c56ee6f14664a8a77f5b82d5)
Steffan Karger [Sun, 12 Nov 2017 16:36:36 +0000 (17:36 +0100)]
Add --tls-cert-profile option.
This allows the user to specify what certificate crypto algorithms to
support. The supported profiles are 'preferred', 'legacy' (default) and
'suiteb', as discussed in <84590a17-1c48-9df2-c48e-4160750b2e33@fox-it.com>
(https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14214.
html).
This fully implements the feature for mbed TLS builds, because for mbed it
is both more easy to implement and the most relevant because mbed TLS 2+
is by default somewhat restrictive by requiring 2048-bit+ for RSA keys.
For OpenSSL, this implements an approximation based on security levels, as
discussed at the hackathon in Karlsruhe.
This patch uses 'legacy' as the default profile following discussion on
the openvpn-devel mailing list. This way this patch can be applied to
both the release/2.4 and master branches. I'll send a follow-up patch for
the master branch to change the default to 'preferred' later.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20171112163636.17434-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15848.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit aba758740d26224b7b3957df221def7ab80c5802)
Gert Doering [Sat, 11 Nov 2017 14:22:30 +0000 (15:22 +0100)]
Remove warning on pushed tun-ipv6 option.
tun-ipv6 is a no-op nowadays, and we print a warning to let users know -
which is not helpful for server-pushed tun-ipv6 (which might be the
result of --server-ipv6 automatically pushing this). So, remove the
warning if parsing pushed options.
Also, remove the VERIFY_PERMISSION() call here which has side effects
on the "which class of options got pushed, do we need to act on them
later on?" flag set.
v2: use existing pull_mode flag
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20171111142230.3288-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20171111142230.3288-1-gert@greenie.muc.de Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7a216d9dba558281d4b6a04124912081a79fcb88)
Simon Rozman [Fri, 13 Oct 2017 09:50:08 +0000 (11:50 +0200)]
Uniform swprintf() across MinGW and MSVC compilers
Legacy _snwprintf() and snwprintf() functions replaced with ISO C
swprintf().
Assigning _snwprintf() return value to unused variable was also removed
at one occasion. Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171013095008.8288-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15633.html
Simon Rozman [Wed, 11 Oct 2017 13:45:30 +0000 (15:45 +0200)]
Document ">PASSWORD:Auth-Token" real-time message
Authentication tokens are security enhancement eliminating client
need to cache passwords, and are indispensable at two factor
authentication methods, such as HOTP or TOTP.
The ">PASSWORD:Auth-Token" message was not mentioned anywhere in
the OpenVPN Management Interface Notes. This patch adds a simple use
case example, while the more detailed feature description remains
explained in the OpenVPN manual. Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20171011134530.6676-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15599.html
Simon Rozman [Thu, 12 Oct 2017 10:34:48 +0000 (12:34 +0200)]
Fix local #include to use quoted form
.h include files from the same folder or addressed relatively to the
same folder should be #included using quoted form in MSVC. The angled
form is reserved for include files from folders specified using /I
path.
Using angled form, MSVC fails to locate local #include file, unless
current folder is added to the include search path: /I . Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171012103448.7632-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15622.html
time_t is only specified as an integer type per POSIX. To reliably
print it, better cast it to "long long", which is at least 64 bits wide
and can represent values beyond 2038.
Printing as a "long" could cause problems on ILP32 systems using a 64
bits time_t (eg OpenBSD/armv7).
James Bottomley [Sun, 29 Oct 2017 15:34:48 +0000 (15:34 +0000)]
autoconf: Fix engine checks for openssl 1.1
In openssl 1.1, ENGINE_cleanup became a #define instead of a function
(because it's no longer needed as engines are self cleaning). Update
the autoconf.ac script to check for ENGINE_cleanup as a declaration to
avoid falsely undefinig HAVE_OPENSSL_ENGINE in openssl 1.1+
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1509291288.3116.14.camel@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15676.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 6b5dbf6c8da0ff82fa1dca4eb4665be0a4fe31d3)
Selva Nair [Fri, 20 Oct 2017 17:25:56 +0000 (13:25 -0400)]
Avoid illegal memory access when malformed data is read from the pipe
- If only 1 byte is read from the interactive service client pipe, that
evaluates to zero wide characters and subsequent check for NUL
termination in the data buffer segfaults.
Fix: reject clients that send less than a complete wide character.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1508520356-18277-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15657.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 6f20808c8f37301c43d822f6a22d30b3587abc57)
Simon Rozman [Thu, 12 Oct 2017 08:07:20 +0000 (10:07 +0200)]
Simplify iphlpapi.dll API calls
Dynamically locating API function addresses at run-time using
GetProcAddress() was a leftover from the early days of the interactive
service development. It was required before `NTDDI_VERSION` was raised
from Windows XP to Windows Vista.
After NTDDI_VERSION API level was raised to NTDDI_VISTA, the direct
calling of Vista introduced API functions is possible and much
simpler.
This patch simplifies the code while in the same time it removes
controversial function type definitions that caused interactive service
not to compile on MSVC. Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171012080720.7764-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15614.html
lz4: Fix broken builds when pkg-config is not present but system library is
In commit f91e4863bc1382 we fixed an issue where LZ4_LIBS could be
overwritten in some situations. But on systems where lz4 is installed on
the system but is lacking pkg-config information, the linker will not know
about the lz4 library when completing the build.
This fixes the issue by explicitly setting LZ4_LIBS to contain -llz4
if pkg-config test was run and failed verifying the installed lz4 version
number. This also ensures that LZ4_LIBS will not be overwritten if it
has been provided on the ./configure command line.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171002190732.12531-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15549.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e5b279f1b62e75569ee8d988b55e6ee0dc93464e)
Older LZ4 library versions used a version number > 100 and not the
current x.y.z versioning scheme. This results in version 122 being
numberically higher than the check we have liblz4 > 1.7.1. And
since that old version (122) does not have the LZ4_compress_default(),
the building explodes later on.
This patch enhances the version check to also ensure the version
number is lower than 100. In addition the function checking we
had was not triggered if system library was found via pkg-config,
so this have now been reworked to really check if we have at least
two of the most important LZ4 functions - as long as a system
library have been found or been accepted via the LZ4_{CFLAGS,LIBS}
variables.
There are more ways to check for functions in autoconf. I opted
for AC_CHECK_LIB() instead of AC_CHECK_FUNC{,S}() as the latter
ones does not test if a function exists in a specific library. This
have the downside of needing to tests instead of AC_CHECK_FUNCS()
which could test for more functions in one go. We also do not
overwrite the LZ4_LIBS variable on success, as that could change
already set library paths (-L)
Finally, a stupid typo got fixed as well.
Trac: 939 Signed-off-by: David Sommerseth <davids@openvpn.net> Tested-by: Richard Bonhomme <fragmentux@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171002161812.9376-1-davids@openvpn.net>
URL: https://www.mail-archive.com/search?l=mid&q=20171002161812.9376-1-davids@openvpn.net Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit f91e4863bc138213a07a2cf53ad71d8a4532abef)
Commit 3d6a4cded2b20fb81 introduced checking for "too many parameters"
at option processing, and neglected to take "ipv6only" as possible
(and optional) argument to "--bind" into account.
Trac: #938
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170928031620.22331-1-hashiz@meridiani.jp>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15522.html
We are using a deprecated function, LZ4_compress_limitedOutput(), which
will be removed with time. The correct function to use is
LZ4_compress_default(). Both function takes the same number of
arguments and data types, so the change is minimal.
This patch will also enforce the system LZ4 library to be at least v1.7.1.
If the system library is not found or it is older, it will be build using
the bundled LZ4 library. The version number requirement is based on the
LZ4 version we ship.
The changes in configure.ac for the version check is modelled around the
same approach we use for OpenSSL. Plus it does a few minor reformats and
improvements to comply with more recommend autoconf coding style.
This patch is a result of the discussions in this mail thread:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14135.html
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170907172004.22534-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15396.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 5f6225c32e41a922069964d9d59c2fcd6589f74c)
Steffan Karger [Tue, 15 Aug 2017 08:04:33 +0000 (10:04 +0200)]
Fix bounds check in read_key()
The bounds check in read_key() was performed after using the value, instead
of before. If 'key-method 1' is used, this allowed an attacker to send a
malformed packet to trigger a stack buffer overflow.
Fix this by moving the input validation to before the writes.
Note that 'key-method 1' has been replaced by 'key method 2' as the default
in OpenVPN 2.0 (released on 2005-04-17), and explicitly deprecated in 2.4
and marked for removal in 2.5. This should limit the amount of users
impacted by this issue.
CVE: 2017-12166 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: <80690690-67ac-3320-1891-9fecedc6a1fa@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=80690690-67ac-3320-1891-9fecedc6a1fa@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 3b1a61e9fb27213c46f76312f4065816bee8ed01)
systemd: Enable systemd's auto-restart feature for server profiles
Systemd supervises services it has started and can act upon unexpected
scenarios. This change will restart OpenVPN after 5 seconds if the OpenVPN
process exits unexpectedly.
The on-failure mode is the recommended mode by upstream systemd.
This change have been tested on a test server for some month, and it
works indeed as intended when provoking the OpenVPN process to stop.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170906235202.26551-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15370.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit a4686e99b047081f0ef6f7945450183088464aa5)
tcp-server: ensure AF family is propagated to child context
Commit 23d61c56 introduced the AF_UNSPEC socket family
to be used when we don't know the actual one until the local
socket binding is performed.
In such case AF_UNSPEC is stored in the `ce.af` member of
the `c->options` object, indicating that the family has to be
determined at runtime.
However, the determined value is never propagated back to the
`options` object, which remains AF_UNSPEC and that is
later used to initialize the TCP children contexts (UDP
children contexts are unaffected).
This unexpected setting can trigger weird behaviours, like
the one reported in ticket #933.
In this case the value AF_UNSPEC in combination with the
changes implemented in 2bed089d are leading to a TCP
server quitting with M_FATAL upon client connection.
Note that the misbehaviour described in #933 can only be
triggered when running a TCP server with mtu-disc set
in the config (no matter the value).
Fix this inconsistency by always propagating the AF
family from the top to the child context when running
in TCP server mode.
As a direct consequence, this patch fixes Trac #933.
Trac: 933 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170907095530.15972-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15380.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 682e7feac3bd57e6ce7e60504cb4da5c894d0e18)
systemd: Ensure systemd shuts down OpenVPN in a proper way
By default, when systemd is stopping OpenVPN it will send the SIGTERM
to all processes within the same process control-group. This can come
as a surprise to plug-ins which may have fork()ed out child processes.
So we tell systemd to only send the SIGTERM signal to the main OpenVPN
process and let OpenVPN take care of the shutdown process on its own.
If the main OpenVPN process does not stop within 90 seconds (unless
changed), it will send SIGKILL to all remaining processes within
the same process control-group.
This issue have been reported in both Debian and Fedora.
Trac: 581
Message-Id: <20170906234705.26202-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15369.html Signed-off-by: David Sommerseth <davids@openvpn.net>
[DS: Applied lazy-ack policy]
(cherry picked from commit 29446a18e1f2b52d20f359253b085e96fe458367)
projects / thirdparty / openvpn.git / log
