Gert Doering [Sun, 19 Aug 2018 20:07:03 +0000 (22:07 +0200)]
Fix combination of --dev tap and --topology subnet across multiple platforms.
--topology should have no effect in tap mode (tap is always "subnet"),
but due to the way options are checked, setting "topology subnet" caught
an improper branch on all non-linux and non-win32 platforms.
Easily tested by adding "--topology subnet" to a "--dev tap" t_client
test.
Tested, verified, and fixed on FreeBSD 10.4, NetBSD 7.0.1, OpenBSD 6.0,
and OpenSolaris 10. Compile-tested on MacOS X.
Trac: #1085
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20180819200703.20362-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17414.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Sat, 6 Oct 2018 00:08:15 +0000 (20:08 -0400)]
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.
- Move setting RSA_sign method from ssl_ctx_set_rsakey() to its
right place in SSL_CTX_use_CryptoAPI_certificate(). The former
function is only in master and appeared when the code was
refactored for EC cert support.
- Remove the stale comment about NULL hash algorithm pointed out by Gert.
Except for the context change of one hunk, the patch is the same as
for master and nothing extra is pulled-in.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1538784495-24988-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17579.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Fri, 5 Oct 2018 13:58:49 +0000 (15:58 +0200)]
Fix --disable-crypto build
Commit d2ff5164 was fine for the master branch, but broke the 2.4 build if
the --disable-crypto configure options was used (which is removed in the
master branch).
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20181005135849.28718-1-steffan@karger.me>
URL: https://www.mail-archive.com/search?l=mid&q=20181005135849.28718-1-steffan@karger.me 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>
(cherry picked from commit b4fc8bbd6b1d0211dd6982c4accedfbe4ae7e3ed)
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
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>
(cherry picked from commit 5fa25eeb7fefdbb17ad639d72fe46f393989159f)
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>
(cherry picked from commit a564781cfd9912d0f755394d1fa610706d93e707)
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>
(cherry picked from commit 7170bef507bfe74ceb4a12f8f10df4d2d6ad39c9)
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>
(cherry picked from commit dd1da0e485a3d161feb5230b6aa57df11ea72705)
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
projects / thirdparty / openvpn.git / log
