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>
Selva Nair [Thu, 25 Jan 2018 19:45:13 +0000 (14:45 -0500)]
Allow external EC key through --management-external-key
- This automatically supports EC certificates through
--management-external-cert
- EC signature request from management is prompted by
>PK_SIGN if the client supports it (or >RSA_SIGN)
Response should be of the form 'pk-sig' (or rsa-sig
by older clients) followed by DER encoded signature
as base64 terminated by 'END' on a new line.
v3: This is v2 adapted to the client_version capability
Requires pacthes 1 and 2 of the series 147:
https://patchwork.openvpn.net/project/openvpn2/list/?series=147
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1516909513-31683-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16365.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
Selva Nair [Thu, 25 Jan 2018 19:41:01 +0000 (14:41 -0500)]
Prompt for signature using '>PK_SIGN' if the client supports it
- Increase the management version from 1 to 2
- If the client announces support for management version > 1
prompt for signature using >PK_SIGN to which the client
responds using 'pk-sig'
Older (current) clients will be continued to be prompted
by '>RSA_SIGN' and can respond using 'rsa-sig'
- Remove an unused rsa_sig buffer-list variable
This facilitates a transparent transition to PK_SIG and future deprecation
of RSA_SIGN
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1516909261-31623-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16364.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Thu, 25 Jan 2018 19:41:00 +0000 (14:41 -0500)]
Add management client version
- "version" command from client to management can now set
the version of management interface supported by the client
by specifying an optional integer parameter.
If no parameter is specified the version of OpenVPN
and its management interface is returned (current behaviour).
The client version defaults to 1 which is the current version of
the Management Interface.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1516909261-31623-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16363.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
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 [Sun, 26 Nov 2017 14:15:55 +0000 (15:15 +0100)]
tls_ctx_set_tls_versions: move verify_flags to where it is used
Minor cleanup of this function now that we are allowed to write C99: move
(and rename) flags to the code where it's actually used to improve
readability.
(I originally did this as part of the tls-version-{min,max} patch for
openssl 1.1, but that made the diff hard to read.)
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171126141555.25930-3-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15931.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Fri, 19 Jan 2018 21:27:21 +0000 (22:27 +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.
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 [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>
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>
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>
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>
Steffan Karger [Tue, 2 Jan 2018 22:52:51 +0000 (23:52 +0100)]
Don't throw fatal errors from verify_cert_export_cert()
As with create_temp_file(), this function is called on client connects
and should not cause fatal errors when I/O (possibly temporarily) fails.
Fix this and the openssl backend implementation of x509_write_pem() to
no longer throw fatal errors.
The callers of this function are already fixed in the commit that does
the same for create_temp_file().
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>
projects / thirdparty / openvpn.git / log
