platform.c: In function ?platform_create_temp_file?:
platform.c:355:31: warning: implicit declaration of function
?get_random? [-Wimplicit-function-declaration]
prefix, (unsigned long) get_random(),
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20181008165648.27504-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17652.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Samy Mahmoudi [Sun, 7 Oct 2018 22:35:47 +0000 (00:35 +0200)]
man: correct a --redirection-gateway option flag
Replace "servers" with "peers" in the description
of the --redirection-gateway option flag local. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20181007223544.GA2246@t520.my.lan>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17630.html
Simon Rozman [Mon, 8 Oct 2018 09:45:59 +0000 (11:45 +0200)]
msvc: Unify Unicode/MultiByte string setting across all cfg|plat
The openvpnserv.vcxproj source code is Windows API Unicode compliant
with only Debug|x64 set to Unicode, while other cfg|plat pairs were set
to MultiByte. Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20181008094600.10164-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17633.html
Steffan Karger [Sun, 7 Oct 2018 22:30:34 +0000 (00:30 +0200)]
Add support for CHACHA20-POLY1305 in the data channel
We explicitly only supported GCM as a valid AEAD mode, change that to also
allow ChaCha20-Poly1305 as an AEAD cipher. That works nicely with our new
(GCM) data channel format, because is has the same 96-bit IV.
Note that we need some tricks to not treat the cipher as insecure, because
we used to only look at the block size of a cipher to determine if find a
cipher insecure. But ChaCha20-Poly1305 is a stream cipher, which
essentially
has a 'block size' of 1 byte and is reported as such. So, special-case
this
cipher to be in the list of secure ciphers.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20181007223035.21179-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17629.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Mon, 8 Oct 2018 09:46:00 +0000 (11:46 +0200)]
msvc: Move common project settings to reusable property sheets
The Visual Studio 2017 project files were refactored by migrating all
repeating common settings into three property sheets: Debug.props,
Release.props and the existing PropertySheet.props.
This simplifies configuration management while providing uniformity
across projects, configurations and platforms. Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20181008094600.10164-2-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17634.html
Simon Rozman [Mon, 8 Oct 2018 10:03:23 +0000 (12:03 +0200)]
Reference msvc-generate from compat to assure correct build order
Single-process builds start building compat project first and they fail,
since the referenced config-msvc-version.h is not available yet. Multi-
process rebuilds also tends to fail if the compat project is built
faster than msvc-generate is able to produce the required output files.
Adding a reference to msvc-generate project assures correct build order. Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20181008100323.11308-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17635.html
Steffan Karger [Sun, 7 Oct 2018 17:52:15 +0000 (19:52 +0200)]
Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'
Like 'proto', a mismatch in key-method, keydir or tls-auth would fail
before we ever get to the point where we can print this warning.
This prepares for removing these from the occ string later on, but also
prepares for tls-crypt-v2, which allows a server to support tls-auth and
tls-crypt-v2 connections in parallel. Such a server will send 'keydir'
and 'tls-auth' in the occ string. This change removes the spurious
warnings about that in the client log.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20181007175215.25009-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17618.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Fri, 5 Oct 2018 15:00:32 +0000 (17:00 +0200)]
Simplify --genkey option syntax
Instead of requiring users to do "--genkey --secret new.key", allow
them to just do "--genkey new.key". This has hit me often enough that I
decided to write a patch for it. Also, the upcoming tls-crypt-v2-genkey
uses a similar syntax and Antonio suggested we should make them consistent.
The documentation is updated to no longer mention the old syntax, but it is
still supported so people who are used to the old syntax can still use it.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20181005150032.16541-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17574.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Wed, 3 Oct 2018 17:21:21 +0000 (20:21 +0300)]
openvpnserv: clarify return values type
Functions openvpn_vsntprintf and openvpn_sntprintf return
values of type int, but in reality it is always 0 or 1 (and -1 for
snrptinf), which can be represented as boolean.
To make code clearer, change return type to BOOL. Also
use stdbool.h header instead of bool definition macros in automatic.c.
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1538587281-3209-1-git-send-email-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17532.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Sun, 7 Oct 2018 10:00:32 +0000 (12:00 +0200)]
Fix use-after-free in tls_ctx_use_management_external_key
Commit 98bfeeb4 changed our openssl backend implementation of
tls_ctx_use_management_external_key() to no longer use
tls_ctx_load_cert_file_and_copy(), but still free'd 'cert'. Which it no
longer should do. Credits go to Arne for spotting the issue (even though
it was missed during the review).
The offending commit is only recently applied to the master branch, so was
never part of a OpenVPN release. For that reason I did not do full impact
analysis.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20181007100032.17060-1-steffan@karger.me>
URL: https://www.mail-archive.com/search?l=mid&q=20181007100032.17060-1-steffan@karger.me Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Fri, 13 Apr 2018 12:47:56 +0000 (14:47 +0200)]
Signed/unsigned warnings of MSVC resolved
This patch fixes the signed/unsigned comparison warnings discovered when
compiling openvpnserv using MSVC.
Wherever possible, it changes iterator and/or size variables to a more
appropriate type, or uses type-casting when it is safe to do so. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180413124756.5756-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16756.html
ensure function declarations are compiled with their definitions
Function prototypes should be included when compiling their
definitions so that it is clear to compilers and static
analyzers that they are not static.
This means that several declarations have to be moved to the
related header files which in turn have to be included by the
source files implementing them.
Generally speaking this also improves the coding style and
makes this code more consistent with the rest that already
follows this rule.
Cc: Steffan Karger <steffan@karger.me> Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20171111161836.23356-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15820.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Tue, 2 Oct 2018 20:01:13 +0000 (16:01 -0400)]
Enable dhcp on tap adapter using interactive service
Currently, if dhcp on the TAP interface is disabled, OpenVPN
on Windows tries to enable it using netsh but that succeeds only when
run with admin privileges.
When interactive service is available, delegate this task to the
service.
Trac: #1111
Tested on Windows 7
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1538510474-27602-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17517.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 5 Oct 2018 12:23:30 +0000 (14:23 +0200)]
Add OpenSSL compat definition for RSA_meth_set_sign
Commit 6b495dc4c5cfc118091ddc9c19330b3c9e3e3dff introduced
RSA_meth_set_sign, which is OpenSSL 1.1.0 and newer. Add a compatibility
definition. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20181005122330.31431-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20181005122330.31431-1-arne@rfc2549.org
Pass the hash without the DigestInfo header to NCryptSignHash()
In case of TLS 1.2 signatures, the callback rsa_priv_enc() gets
the hash with the DigestInfo prepended. Signing this using
NCryptSignHash() with hash algorithm id set to NULL works in most cases.
But when using some hardware tokens, the data gets interpreted as the pre
TLS 1.2 MD5+SHA1 hash and is silently truncated to 36 bytes.
Avoid this by passing the raw hash to NCryptSignHash() and let it
add the DigestInfo.
To get the raw hash we set the RSA_sign() method in the rsa_method
structure. This callback bypasses rsa_priv_enc() and gets called with
the hash type and the hash.
Lev Stipakov [Thu, 20 Sep 2018 13:12:34 +0000 (16:12 +0300)]
Refactor NCP-negotiable options handling
NCP negotiation can alter options. On reconnect
client sends possibly altered options while server
expects original values. This leads to warnings
in log and, if server uses --opt-verify, breaks
reconnect.
Fix by decouple setting/unsetting NCP options from
the state of TLS context. At startup (and once per sighup)
we load original values to c->c1, which persists over
sigusr1 (restart). When tearing tunnel down we restore
(possibly altered) options back to original values.
Trac: #1105
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1537449154-26879-1-git-send-email-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17477.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Fri, 28 Sep 2018 13:26:49 +0000 (16:26 +0300)]
win: support for Visual Studio 2017
This patch enables building openvpn with Visual Studio 2017.
It is advised to use openvpn-build/msvs/build.bat which
also downloads and build required dependencies.
Changes made:
- updated path to Visual Studio toolchain
- updated platform toolset
- added missing libraries
- added x64 configurations
- enabled AEAD ciphers to make NCP work
- enabled unicode support
- updated source files in project settings
- fix includes
- restored variable which was erroneously removed
- added properties file which sets required env variables
(required to build with IDE)
- etc
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Simon Rozman <simon@rozman.si>
Message-Id: <1538141209-32330-1-git-send-email-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17499.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit 98bfeeb4 introduced a memory leak in SSL_CTX_use_certificate by
removing the "if(x509) { ... }" bit while not changing the
"else if(x) {}" right after to an "if(x) {}".
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20180926192706.29460-1-steffan@karger.me> Signed-off-by: Gert Doering <gert@greenie.muc.de>
mbedtls: remove dependency on mbedtls pkcs11 module
Instead of using mbedtls's pkcs11 module, reuse the code we already have
for management-external-key to also do pkcs11 signatures. As far as mbed
is concerned, we simply provide an external signature.
This has the following advantages:
* We no longer need mbed TLS to be compiled with the pkcs11 modules
enabled (which is not enabled by default). This makes it easier to use
a system/distribution-provided mbed shared library.
* We no longer have a dependency on pkcs11-helper through mbed TLS. So if
we want to migrate to some other pkcs11 lib (see e.g. trac #491, #538
and #549 for reason why), this will be easier.
While touching this code, switch from M_FATAL to M_WARN and proper error
handling. This improves the error reporting, and helps prevent potential
future DoS attacks if someone starts using these functions on peer input.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1536916459-25900-3-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17463.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Do not load certificate from tls_ctx_use_external_private_key()
The cert and key loading logic surrounding management-external-key and
management-external cert was somewhat intertwined. Untangle these to
prepare for making the external key code more reusable.
The best part is that this even reduces the number of lines of code.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1536916459-25900-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17464.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
systemd: extend CapabilityBoundingSet for auth_pam
Auth_pam will require audit writes or the connection will be rejected
as the plugin fails to initialize like:
openvpn[1111]: sudo: unable to send audit message
openvpn[1111]: sudo: pam_open_session: System error
openvpn[1111]: sudo: policy plugin failed session initialization
See links from https://community.openvpn.net/openvpn/ticket/918 for
more.
auth_pam is a common use case and capabilties for it should be allowed
by the .service file.
Fixes: #918 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20180829142715.417-2-christian.ehrhardt@canonical.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17432.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Wed, 29 Aug 2018 13:49:43 +0000 (15:49 +0200)]
Fix memory leak after sighup
The c.es env_set is (re)allocated for each "sighup loop iteration", while
it was free'd only once at process shutdown. Move the env_set_destroy()
call to match the same level as the env_set_create() call to fix that.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1535550583-21825-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17429.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Wed, 29 Aug 2018 12:04:46 +0000 (14:04 +0200)]
mbedtls: print warning if random personalisation fails
... instead of when it doesn't fail. Looks like 'someone' mixed up the
mbedtls return style (0 means success) with the openvpn internal return
style (true means success).
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1535544286-29638-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17428.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Mon, 6 Aug 2018 08:02:33 +0000 (10:02 +0200)]
Introduce buffer_write_file()
Rewrite buf_write_string_file to buffer_write_file, which is simpler to
use and can deal with not-null-terminated strings. Mostly implemented so
this can be easily reused for tls-crypt-v2 (client) key files.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net> Tested-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1533542553-7383-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17371.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Parse static challenge response in auth-pam plugin
If static challenge is in use, the password passed to the plugin by openvpn
is of the form "SCRV1:base64-pass:base64-response". Parse this string to
separate it into password and response and use them to respond to queries
in the pam conversation function.
On the plugin parameters line the substitution keyword for the static
challenge response is "OTP". For example, for pam config named "test" that
prompts for "user", "password" and "pin", use
plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1532486093-24793-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17307.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
make tls-auth and tls-crypt per-connection-block options
Different VPN servers may use different tls-auth/crypt keys.
For this reason it is convenient to make tls-auth/crypt
per-connection-block options so that the user is allowed to
specify one key per remote.
If no tls-auth/crypt option is specified in a given connection
block, the global settings, if any, are used.
In preparation to having tls-auth/crypt keys per connection
block, it is important to ensure that such material is always
reloaded upon SIGUSR1, no matter if `persist-key` was specified
or not.
This is required because when moving from one remote to the
other the key may change and thus the key context needs to
be refreshed.
To ensure that the `persist-key` logic will still work
as expected, the tls-auth/crypt key is pre-loaded so that
the keyfile is not required at runtime.
Needed for tls-crypt-v2, but isolated enough to be reviewed as a separate
patch.
The encode API allocates memory, because it fits our typical gc-oriented
code pattern and the caller does not have to do multiple calls or
calculations to determine the required destination buffer size.
The decode API does not allocate memory, because the required destination
buffer is always smaller than the input buffer (so is easy to manage by
the caller) and does not force the caller to use the heap.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20180722100645.5813-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17284.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
stream_buf_init(), stream_buf_close() and stream_buf_added()
are only used within socket.c, therefore there is noneed to
have them declared in socket.h.
Make them static and remove useless declarations.
This change reuired adding function prototypes in socket.c to
avoid useless code re-ordering.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180712012955.24050-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17246.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Move execve/run_script helper functions to run_command.c
To avoid having to include misc.c - which is a dependency mess - in the
tls-crypt unit tests, move the command execution helper functions to a new
run_command.c module.
While at it, abstract away the script_security global variable.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20180704175404.22371-2-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17212.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Move file-related functions from misc.c to platform.c
To avoid having to include misc.c - which is a dependency mess - in the
tls-crypt unit tests, move file-handing related functions to platform.c
(which is where other file-related functions already reside).
Note that platform_create_temp_file() needs random. To avoid including
misc.c in other tests that use platform.c, add a mock get_random().
(Almost every test includes platform.c, because buffer.c depends on it.
That smells like it needs cleanup too, but not in this patch set.)
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180704175404.22371-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17208.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 3 Jul 2018 16:17:51 +0000 (18:17 +0200)]
Add MTU to Android IFCONFIG6 control command
Since OpenVPN nows supports IPv6 only connections, OpenVPN for Android
cannot longer rely on IFCONFIG to send the MTU. Add sending the MTU to
IFCONFIG6 too. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180703161751.7680-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17186.html
Gert Doering [Sun, 1 Jul 2018 19:59:38 +0000 (21:59 +0200)]
Extend push-remove to also handle 'ifconfig'.
Push-remove (introduced in commit 970312f1850) did not handle "ifconfig"
yet, as both "ifconfig" and "ifconfig-ipv6" are handled differently from
all other pushed options. Since there was no valid use-case to not-push
"ifconfig" (no support on the client side for running IPv6-only) this
was not an issue so far - but with the recent commits to enable ipv6-only
operation it can be a desirable feature.
The implementation is similar to "push-remove ifconfig-ipv6" - namely,
flagging via a new context option (c->options.push_ifconfig_ipv4_blocked)
and then not creating the push statement in "send_push_reply()".
While not truly elegant, it's much less invasive than the alternatives
(storing the list of "push-remove" statements somewhere and then checking
in push_option_ex())
Trac: #1072
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20180701195938.2541-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17169.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Sun, 26 Nov 2017 15:49:12 +0000 (16:49 +0100)]
openssl: add missing #include statements
Compiling our current master against OpenSSL 1.1 with
-DOPENSSL_API_COMPAT=0x10100000L screams bloody murder. This patch fixes
the errors caused by missing includes. Previous openssl versions would
usually include 'the rest of the world', but they're fixing that. So we
should no longer rely on it.
(And sneaking in alphabetic ordering of the includes while touching them.)
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171126154912.13283-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15936.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Sun, 26 Nov 2017 15:04:00 +0000 (16:04 +0100)]
openssl: don't use deprecated SSLEAY/SSLeay symbols
Compiling our current master against OpenSSL 1.1 with
-DOPENSSL_API_COMPAT=0x10100000L screams bloody murder. This patch fixes
the errors about the deprecated SSLEAY/SSLeay symbols and defines.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171126150401.28565-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15934.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
windows: properly configure TAP driver when no IPv4 is configured
This patch ensures that the TAP driver on a windows host is still
configured, even though no IPv4 has been provided.
In this case the TAP driver ioctl will be invoked with a fake
0.0.0.0/0.0.0.0 IPv4 which will simply start the interface and
get it to a working state.
Trac: #208 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180623183108.18684-1-a@unstable.cc>
URL: https://www.mail-archive.com/search?l=mid&q=20180623183108.18684-1-a@unstable.cc Signed-off-by: Gert Doering <gert@greenie.muc.de>
Both "compiler" and "exclude" are redundant, so remove them.
Add openssl-1.0.1u to build matrix. Enable explicit apt update
(it was disabled by default in travis-ci).
Signed-off-by: Ilya Shipitsin <chipitsine@gmail.com>
--
v2: Add openssl-1.0.1u to build matrix (thanks to Steffan Karger),
Add explicit apt-get update (it was disabled by default in travis-ci) Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20180527190057.3488-1-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16898.html
tun: ensure interface can be configured with IPv6 only
This change ensures that an interface is properly brought
up and down even when only IPv6 settings are configured/pushed.
At the same time, some code restyling took place to ensure the new
generic logic is easier to read. Both do_ifconfig() and close_tun()
(Linux only) functions have been rearranged by splitting the logic
into a v4 and a v6 specific part. Each part has then been moved
into an idependent helper that can be invoked as
needed.
This makes the code easier to read and more "symmetric" with
respect to the two address families.
Trac: #208 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180618074733.19773-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17064.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
This patch is a small "logic restyle" which basically moves the
check for "tt != NULL" outside of the various close_tun()
implementations and replaces it with an ASSERT.
This way the check is done only once and the function can rely
on the assumption that "tt" is always valid.
This change is mainly to improve the code style inside close_tun()
implementations by removing one level of indentation.
No functional change is present.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180613122824.4207-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17045.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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.
pool: restyle ipv4/ipv6 members to improve readability
(This is only code refactoring)
IPv4 and IPv6 members are all part of the same flat hierarchy
in the pool data structure, without a proper name convention.
Create 2 sub-structures to properly separate IPv4 from IPv6
related members. This should make the structure more organized
and also slightly improve code readability.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180605090421.9746-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16944.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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:22 +0000 (17:11 +0100)]
Add negotiated cipher to status file format 2 and 3
With NCP turned off, this will still display the cipher used.
Trac: #814
Signed-off-by: Gert van Dijk <gert@gertvandijk.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20171111161122.30087-2-gert@gertvandijk.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15817.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
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
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>
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>
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>
Selva Nair [Fri, 23 Feb 2018 03:03:19 +0000 (22:03 -0500)]
Move setting private key to a function in prep for EC support
- Also add reference counting to CAPI_DATA (application data):
When the application data is assigned to the private key
we free it in the key's finish method. Proper error handling
requires to keep track of whether data is assigned to the
key or not before an error occurs. For this purpose, add a
reference count to CAPI_DATA struct and increment it when it is
assigned to the key or its method.
CAPI_DATA_free now frees the data only if ref_count <= 0
Selva Nair [Thu, 22 Feb 2018 04:54:55 +0000 (23:54 -0500)]
Fix format spec errors in Windows builds
- "%ll" is not supported by Windows run time, so use PRIi64
and cast the variable to (int64_t) in output statements
(as in commit 9ba36639abcac4367c8227d2dd87b18fb56267c4)
- Fix an instance of wchar_t * printed using %s -- should be %ls.
- Cast variables to int or unsigned int to match the output
format spec when necessary.
- In route.c correct format of adapter_index (should be %lu) in a few
places and remove some unnecessary casts to (unsigned int). Not
all such instances are changed, only those related to adapter_index
(for consistency) or close-by contexts are edited.
Most of these errors are seen in current Windows cross-compile,
but a few are triggered only if some DEBUG options are enabled.
Some are not in Windows specific paths. But for consistency, all uses
of %llu/%lld are removed. As these only affect log output, there are
no potential side effects.
Replacing long long by int64_t also has the advantage of avoiding
size ambiguity as long long is not guaranteed to be 64 bytes.
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>
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>
Selva Nair [Wed, 21 Feb 2018 16:46:02 +0000 (11:46 -0500)]
Disable external ec key support when building with libressl
- This codepath uses some openssl-1.1 specific API and is enabled only
for openssl 1.1 and higher versions. But, due to incompatible
version numbering in libressl, it gets wrongly enabled with libressl
versions that do not support the reqired API. As an easy workaround
disable the feature when LIBRESSL_VERSION_NUMBER is defined.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1519231562-5641-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16510.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Wed, 3 Jan 2018 09:40:48 +0000 (10:40 +0100)]
reliable: remove reliable_unique_retry()
This function has been in the code since 2005, and enabled since 2010, but
it's not clear why we'd want this behaviour.
Running some simple tests, where I simulate an server->client link of
1mbit with 30ms delay and 1% packet loss, and a client->server link of
200kbit, 200ms delay, I get the following connection setup times over
100 runs when pushing ~100kb of options (e.g. a lot of routes):
With reliable_unique_retry: Mean: 14.9 s, Std dev: 9.1 s
Without reliable_unique_retry: Mean: 11.8 s, Std dev: 5.8 s
For more standard push sizes (~1kb), the effect is of course smaller:
With reliable_unique_retry: Mean: 2.7 s, Std dev: 0.6 s
Without reliable_unique_retry: Mean: 2.6 s, Std dev: 0.7 s
This shows that this mechanism hurts performance under packet loss
conditions. (Without packet loss, there are no retransmissions and this
has no influence, of course.) Querying the openvpn collective memory
(read: James, David and Gert) did not yield any arguments to keep it.
James even mentioned that OpenVPN 3 doesn't include this mechanism.
So, improve our connection setup performance by removing some code :)
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1514972448-7010-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16146.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
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>