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>
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
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>
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>
ENABLE_PUSH_PEER_INFO depended on ENABLE_CRYPTO that now does
not exist anymore.
Get rid of ENABLE_PUSH_PEER_INFO by assuming that it is always
enabled and simplify the code.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20171202134541.7688-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15953.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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.
Simon Rozman [Sun, 3 Dec 2017 20:30:07 +0000 (21:30 +0100)]
openvpnserv: Review MSVC down-casting warnings
Data size arithmetic was reviewed according to 64-bit MSVC complaints.
The warnings were addressed by migrating to size_t, rewriting the code,
or silencing the warnings by an explicit cast where appropriate. Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171203203007.6628-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16001.html
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>
Steffan Karger [Wed, 1 Nov 2017 22:03:42 +0000 (23:03 +0100)]
create_temp_file/gen_path: prevent memory leak if gc == NULL
If gc == NULL, the data allocated in the alloc_gc_buf() call in
create_temp_file or the string_mod_const call in gen_path would never
be free'd.
These functions are currently never called that way, but let's prevent
future problems.
While touching create_temp_file, also remove the counter variable, which is
never read, simplify the do-while to a while loop, and truncate the prefix
(if needed) to preserve the random and extension of the created filename.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20171101220342.14648-5-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15703.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Steffan Karger [Wed, 1 Nov 2017 22:03:40 +0000 (23:03 +0100)]
pf: reject client if PF plugin is configured, but init fails
This changes the behavior for pf plugins: instead of just not initializing
the firewall rules and happily continuing, this now rejects the client in
the case of an (unlikely) failure to initialize the pf.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20171101220342.14648-3-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15704.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Wed, 1 Nov 2017 22:03:39 +0000 (23:03 +0100)]
pf: clean up temporary files if plugin init fails
close_instance() tries to remove the file in c2.pf.filename, but that only
works if we actually set that if we fail. So, set that filename as soon
as we know we've created the file.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20171101220342.14648-2-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15705.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Simon Matter [Thu, 16 Nov 2017 14:09:58 +0000 (15:09 +0100)]
Add per session pseudo-random jitter to --reneg-sec intervals
While we were suffering from the "TLS Renegotiation Slowdown" bug here
https://community.openvpn.net/openvpn/ticket/854 we realized that there is
still room for improvement in our use case.
It appears that TLS renegotiation is getting more and more expensive in
terms of CPU cycles with recent changes for more security. To make things
worse, we realized that most renegotiation procedures took place at almost
the same time and increased the CPU load too much during these periods.
That's especially true on large, multi-instance openvpn setups.
I've created attached patch to add a per session pseudo-random component to
the --reneg-sec intervals so that renegotiation is evenly spread over time.
It is configured by simply adding a second value to --reneg-sec as
described in the --help text:
--reneg-sec max [min] : Renegotiate data chan. key after at most max
(default=3600) and at least min (default 90% of max on
servers and equal to max on clients).
The jitter is only enabled by default on servers, because the actual reneg
time is min(reneg_server, reneg_client). Introducing jitter at both ends
would bias the actual reneg time to the min value.
Note that the patch also slightly changes the log output to show the sec
value in the same way as the bytes/pkts values:
The idea and first versions of this patch are from Simon Matter. Steffan
Karger later incorporated the mailing list comments into this patch. So
credits go to Simon, and all bugs are Steffan's fault ;-)
Signed-off-by: Simon Matter <simon.matter@invoca.ch> Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171116140958.12847-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15888.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Steffan Karger [Wed, 8 Nov 2017 12:12:54 +0000 (13:12 +0100)]
doxygen: add make target and use relative paths
Add a make target, such that 'make doxygen' works (both for in-tree and
out-of-tree builds). This now generates the doxygen in doc/doxygen/,
rather than in doxygen/.
While doing so, instead of genering docs with full path names (e.g.
/home/steffan/dev/openvpn/src/openvpn/crypto.h), use a relative path wrt
the project root (e.g. src/openvpn/crypto.h) in the generated
documentation. This makes the generated doxygen easier to read.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1510143174-15248-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1510143174-15248-1-git-send-email-steffan.karger@fox-it.com Signed-off-by: Gert Doering <gert@greenie.muc.de>
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
As per previous commit, this is a simple solution to cope with the
various sizes of time_t on different archs, including those that use 64
bits time_t on ILP32 archs to cope with y2038.
Also:
- convert the time_type/time_format abstraction that used unsigned long
to inlined long long code
- print suseconds_t as a long, which appears to be the underlying type
on most Unix systems around
Signed-off-by: Jeremie Courreges-Anglas <jca@wxcvbn.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <87k1zi18lt.fsf@ritchie.wxcvbn.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15667.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
Simon Rozman [Thu, 12 Oct 2017 08:07:20 +0000 (10:07 +0200)]
Simplify iphlpapi.dll API calls
Dynamically locating API function addresses at run-time using
GetProcAddress() was a leftover from the early days of the interactive
service development. It was required before `NTDDI_VERSION` was raised
from Windows XP to Windows Vista.
After NTDDI_VERSION API level was raised to NTDDI_VISTA, the direct
calling of Vista introduced API functions is possible and much
simpler.
This patch simplifies the code while in the same time it removes
controversial function type definitions that caused interactive service
not to compile on MSVC. Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171012080720.7764-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15614.html
lz4: Fix broken builds when pkg-config is not present but system library is
In commit f91e4863bc1382 we fixed an issue where LZ4_LIBS could be
overwritten in some situations. But on systems where lz4 is installed on
the system but is lacking pkg-config information, the linker will not know
about the lz4 library when completing the build.
This fixes the issue by explicitly setting LZ4_LIBS to contain -llz4
if pkg-config test was run and failed verifying the installed lz4 version
number. This also ensures that LZ4_LIBS will not be overwritten if it
has been provided on the ./configure command line.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171002190732.12531-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15549.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Older LZ4 library versions used a version number > 100 and not the
current x.y.z versioning scheme. This results in version 122 being
numberically higher than the check we have liblz4 > 1.7.1. And
since that old version (122) does not have the LZ4_compress_default(),
the building explodes later on.
This patch enhances the version check to also ensure the version
number is lower than 100. In addition the function checking we
had was not triggered if system library was found via pkg-config,
so this have now been reworked to really check if we have at least
two of the most important LZ4 functions - as long as a system
library have been found or been accepted via the LZ4_{CFLAGS,LIBS}
variables.
There are more ways to check for functions in autoconf. I opted
for AC_CHECK_LIB() instead of AC_CHECK_FUNC{,S}() as the latter
ones does not test if a function exists in a specific library. This
have the downside of needing to tests instead of AC_CHECK_FUNCS()
which could test for more functions in one go. We also do not
overwrite the LZ4_LIBS variable on success, as that could change
already set library paths (-L)
Finally, a stupid typo got fixed as well.
Trac: 939 Signed-off-by: David Sommerseth <davids@openvpn.net> Tested-by: Richard Bonhomme <fragmentux@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171002161812.9376-1-davids@openvpn.net>
URL: https://www.mail-archive.com/search?l=mid&q=20171002161812.9376-1-davids@openvpn.net Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit 3d6a4cded2b20fb81 introduced checking for "too many parameters"
at option processing, and neglected to take "ipv6only" as possible
(and optional) argument to "--bind" into account.
Trac: #938
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170928031620.22331-1-hashiz@meridiani.jp>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15522.html
We are using a deprecated function, LZ4_compress_limitedOutput(), which
will be removed with time. The correct function to use is
LZ4_compress_default(). Both function takes the same number of
arguments and data types, so the change is minimal.
This patch will also enforce the system LZ4 library to be at least v1.7.1.
If the system library is not found or it is older, it will be build using
the bundled LZ4 library. The version number requirement is based on the
LZ4 version we ship.
The changes in configure.ac for the version check is modelled around the
same approach we use for OpenSSL. Plus it does a few minor reformats and
improvements to comply with more recommend autoconf coding style.
This patch is a result of the discussions in this mail thread:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14135.html
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170907172004.22534-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15396.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Tue, 15 Aug 2017 08:04:33 +0000 (10:04 +0200)]
Fix bounds check in read_key()
The bounds check in read_key() was performed after using the value, instead
of before. If 'key-method 1' is used, this allowed an attacker to send a
malformed packet to trigger a stack buffer overflow.
Fix this by moving the input validation to before the writes.
Note that 'key-method 1' has been replaced by 'key method 2' as the default
in OpenVPN 2.0 (released on 2005-04-17), and explicitly deprecated in 2.4
and marked for removal in 2.5. This should limit the amount of users
impacted by this issue.
CVE: 2017-12166 Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <80690690-67ac-3320-1891-9fecedc6a1fa@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=80690690-67ac-3320-1891-9fecedc6a1fa@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
systemd: Enable systemd's auto-restart feature for server profiles
Systemd supervises services it has started and can act upon unexpected
scenarios. This change will restart OpenVPN after 5 seconds if the OpenVPN
process exits unexpectedly.
The on-failure mode is the recommended mode by upstream systemd.
This change have been tested on a test server for some month, and it
works indeed as intended when provoking the OpenVPN process to stop.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170906235202.26551-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15370.html Signed-off-by: David Sommerseth <davids@openvpn.net>
tcp-server: ensure AF family is propagated to child context
Commit 23d61c56 introduced the AF_UNSPEC socket family
to be used when we don't know the actual one until the local
socket binding is performed.
In such case AF_UNSPEC is stored in the `ce.af` member of
the `c->options` object, indicating that the family has to be
determined at runtime.
However, the determined value is never propagated back to the
`options` object, which remains AF_UNSPEC and that is
later used to initialize the TCP children contexts (UDP
children contexts are unaffected).
This unexpected setting can trigger weird behaviours, like
the one reported in ticket #933.
In this case the value AF_UNSPEC in combination with the
changes implemented in 2bed089d are leading to a TCP
server quitting with M_FATAL upon client connection.
Note that the misbehaviour described in #933 can only be
triggered when running a TCP server with mtu-disc set
in the config (no matter the value).
Fix this inconsistency by always propagating the AF
family from the top to the child context when running
in TCP server mode.
As a direct consequence, this patch fixes Trac #933.
Trac: 933 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170907095530.15972-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15380.html Signed-off-by: David Sommerseth <davids@openvpn.net>
systemd: Ensure systemd shuts down OpenVPN in a proper way
By default, when systemd is stopping OpenVPN it will send the SIGTERM
to all processes within the same process control-group. This can come
as a surprise to plug-ins which may have fork()ed out child processes.
So we tell systemd to only send the SIGTERM signal to the main OpenVPN
process and let OpenVPN take care of the shutdown process on its own.
If the main OpenVPN process does not stop within 90 seconds (unless
changed), it will send SIGKILL to all remaining processes within
the same process control-group.
This issue have been reported in both Debian and Fedora.
OpenSSL: Always set SSL_OP_CIPHER_SERVER_PREFERENCE flag
* safe bet to say that server admins are better at updating their configs
than client users are and if client do want to restrict their ciphers,
they should simply evict the ciphers they don't want from their cipher
suite
* mbed TLS and OpenSSL behave more similar with the
SSL_OP_CIPHER_SERVER_PREFERENCE flag
Signed-off-by: Szilárd Pfeiffer <coroner@pfeifferszilard.hu> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170904081012.1975-1-coroner@pfeifferszilard.hu>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15356.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Gert van Dijk [Sun, 27 Aug 2017 16:15:15 +0000 (18:15 +0200)]
Warn that DH config option is only meaningful in a tls-server context
If specified in a tls-client context, don't try to open the file as it's
not used. Worse even, if 'none' was specified to disable explicitly, it
complained that the file 'none' could not be found.
[DS: On-the-fly update - Prefixed the message with 'WARNING: ']
Signed-off-by: Gert van Dijk <gert@gertvandijk.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170827161515.2424-1-gert@gertvandijk.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15332.html Signed-off-by: David Sommerseth <davids@openvpn.net>
!A || (A && B) is equivalent to the simpler !A || B
therefore it is preferable to use the second version as
it is simpler to parse while reading the code.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170824075547.29844-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15313.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Tue, 22 Aug 2017 11:47:15 +0000 (13:47 +0200)]
docs: Replace all PolarSSL references to mbed TLS
There were references in our documentation to the now deprecated PolarSSL
library, which have changed name upstream to mbed TLS.
In addition, where appropriate, the documentation now considers only
mbed TLS 2.0 and newer. This is in accordance with the requirements
./configure sets.
[DS: On-the-fly change - Updated Makefile.am to use README.mbedtls
instead of README.polarssl. This ensures make dist and buildbots
won't explode]
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170822114715.14225-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15309.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Sun, 20 Aug 2017 09:19:04 +0000 (11:19 +0200)]
travis: reorder matrix to speed up build
The OSX and mingw builds are much slower than the other jobs. Our free
travis account can only use 4 build executors in parallel. Run the slow
builds earlier, so that when one or more of these finish, the free build
executors will start building the configure variants in parallel with the
slow ones. (Instead of doing the slow ones last, which results in using
only 1-2 executors during the end stage.)
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <1503220744-5569-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15302.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Wed, 16 Aug 2017 17:04:50 +0000 (19:04 +0200)]
tls-crypt: don't leak memory for incorrect tls-crypt messages
If tls_crypt_unwrap() failed, we would jump to cleanup and forget to free
the buffer. Instead, allocate the buffer through gc, which is free'd in
the cleanup section.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170816170450.10415-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15282.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Steffan Karger [Tue, 8 Aug 2017 15:55:41 +0000 (17:55 +0200)]
Add coverity static analysis to Travis CI config
Enable coverity analysis for the release/2.4 branch.
We can only do a limited number of coverity scans per week with our FOSS
account, but since we only occasionally push commits, that should work out
fine. But this limit is the reason we don't use the standard travis addon,
because that would cause the coverity script to run on all of our matrix
builds. That would cause us to reach our limit faster, and waste travis'
resources.
Since our FOSS coverity account doesn't handle multiple branches very well,
we have to pick one branch to run coverity on. I think it's best to use
the most recent stable branch for that (i.e. for now, release/2.4).
Though for ease of maintenance, it's probably best to apply the patch to
both master and release/2.4.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <1502207741-31750-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15176.html Signed-off-by: David Sommerseth <davids@openvpn.net>
crypto: create function to initialize encrypt and decrypt key
Instead of always initialize the encrypt and decrypt keys separately,
implement an helper function init_key_ctx_bi() that takes care of
both of them for us.
Reduces code duplication and improves readability.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20170707044704.7239-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15011.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Tue, 15 Aug 2017 20:53:01 +0000 (22:53 +0200)]
Use consistent version references
A simple clean-up where the version references have been unified
all those places I could find now. The versioning scheme used is:
* OpenVPN 2.x
* v2.x
We want to avoid:
* 2.x (2.4 can be just an ordindary decimal number,
OID reference, a version number or anything else)
* OpenVPN v2.x (OpenVPN indicates we're talking about a version)
In addition, several places where it made sense I tried to ensure
the first version reference uses "OpenVPN 2.x" and the following
references in the same section/paragraph uses "v2.x", to set the
context for the version reference.
In Changes.rst modified paragraphs exceeding 80 chars lines where
reformatted as well.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170815205301.14542-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15260.html Signed-off-by: David Sommerseth <davids@openvpn.net>
David Sommerseth [Tue, 15 Aug 2017 21:54:51 +0000 (23:54 +0200)]
Highlight deprecated features
We have quite a list of deprecated options currently. Ensure this
is highlighted both in documentation and code.
This patch builds on the wiki page [1] enlisting all deprecated features
and their status. There are also some options not listed here, as
there exists patches in release/2.4 which awaits an update for git master.
Steffan Karger [Tue, 15 Aug 2017 15:39:46 +0000 (17:39 +0200)]
Move run_up_down() to init.c
This function is only used in init.c, and is not easy to fit into a
specific category because it both runs scripts and plugin hooks. Making
it static in init.c is probably the best place for this function.
(I think we should find a better place for everything currently in misc.c,
and get rid of it all together. This patch is part of that effort.)
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <1502811586-19578-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15256.html Signed-off-by: David Sommerseth <davids@openvpn.net>
When setting the SOCKS proxy through the management interface, the
socks_proxy_port pointer would be set to a value that's no longer valid
by the time it's used by do_preresolve_host.
Signed-off-by: Thomas Veerman <thomas.veerman@wanwire.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170707195941.61773-1-thomas.veerman@wanwire.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15018.html Signed-off-by: David Sommerseth <davids@openvpn.net>
The --keysize option can only be used with already deprecated ciphers,
such as CAST5, RC2 or BF. Deviating from the default keysize is
generally not a good idea (see man page text), and otherwise only
complicates our code.
Since we will also remove the support for weak ciphers (ciphers with
cipher block length less than 128 bits) in OpenVPN 2.6 as well, we
start the deprecation of this option instantly.
[DS: Slightly amended the patch, referencing OpenVPN 2.6 and added
a few more details to Changes.rst and the commit message]
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170701112951.19119-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15004.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Move create_temp_file() out of #ifdef ENABLE_CRYPTO
By using get_random() instead of prng_bytes(), we no longer have to place
create_temp_file() inside #ifdef ENABLE_CRYPTO.
The resulting filename now has 62 bits of entropy (2 * [0-INT_MAX])
instead of the previous 128 bits, but that should be plenty. Assuming an
int is 32 bits, we would need about 2**31 (2147483648) files to have a
(roughly) 0.5 chance of failing in one of the 6 attempts we do.
(This is preparing to move the function out of misc.c, where I'd prefer to
not have to add a #include "crypto.h".)
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170725210234.5673-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15146.html Signed-off-by: David Sommerseth <davids@openvpn.net>
If a peer has set --keysize, and NCP negotiates a cipher with a different
key size (e.g. --keysize 128 + AES-256-GCM), that peer will exit with a
"invalid key size" error. To prevent that, always set keysize=0 for NCP'd
ciphers.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1500573357-20496-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15110.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Functions used only in the file where they are
defined and not exported in any header, should
always defined as static in order to make the scope
clear to the compiler and the developers.
Add the static attribute where missing.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170811090744.31750-4-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15202.html Signed-off-by: David Sommerseth <davids@openvpn.net>
In the attempt of adhering to the C99 standard as much as possible,
ensure that all the function declarations with no parameter contain
the "void" keyword[1].
projects / thirdparty / openvpn.git / log
