]> git.ipfire.org Git - thirdparty/openvpn.git/log
thirdparty/openvpn.git
7 years agoPrint time_t as long long and suseconds_t as long
Jeremie Courreges-Anglas [Thu, 26 Oct 2017 09:40:05 +0000 (11:40 +0200)] 
Print time_t as long long and suseconds_t as long

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>
7 years agoCast time_t to long long in order to print it.
Jeremie Courreges-Anglas [Wed, 4 Oct 2017 23:43:33 +0000 (01:43 +0200)] 
Cast time_t to long long in order to print it.

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).

Signed-off-by: Jeremie Courreges-Anglas <jca@wxcvbn.org>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <87efq4havl.fsf@ritchie.wxcvbn.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15640.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoautoconf: Fix engine checks for openssl 1.1
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>
7 years agolz4: Rebase compat-lz4 against upstream v1.7.5
David Sommerseth [Tue, 21 Feb 2017 19:27:36 +0000 (20:27 +0100)] 
lz4: Rebase compat-lz4 against upstream v1.7.5

This rebase is done using the new lz4-rebaser.sh tool

The lz4 v1.7.5 is based on commit 7bb64ff2b69a9f8367 in
git://github.com/lz4/lz4

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170221192737.24166-3-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14137.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoRemove references to keychain-mcd in Changes.rst
David Sommerseth [Thu, 5 Oct 2017 21:22:23 +0000 (23:22 +0200)] 
Remove references to keychain-mcd in Changes.rst

In commit 59e7e9fce8de6ea90d13baeaede83adc0b594e22 the contrib/keychain-mcd
code was completely removed.  But it was not removed from Changes.rst.

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20171005212223.24694-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15561.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoFix missing check for return value of malloc'd buffer
Selva Nair [Sun, 15 Oct 2017 19:05:15 +0000 (15:05 -0400)] 
Fix missing check for return value of malloc'd buffer

- Use utf8to16 from common.c for utf8 to wide conversion and
  check its return value

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1508094315-466-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15641.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoAvoid illegal memory access when malformed data is read from the pipe
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>
7 years agoSimplify iphlpapi.dll API calls
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

Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoRtlIpv6AddressToStringW() and RtlIpv4AddressToStringW() require mstcpip.h
Simon Rozman [Tue, 10 Oct 2017 23:11:20 +0000 (01:11 +0200)] 
RtlIpv6AddressToStringW() and RtlIpv4AddressToStringW() require mstcpip.h

Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171010231130.6832-3-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15582.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoMixing wide and regular strings in concatenations is not allowed in MSVC.
Simon Rozman [Tue, 10 Oct 2017 23:11:19 +0000 (01:11 +0200)] 
Mixing wide and regular strings in concatenations is not allowed in MSVC.

Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171010231130.6832-2-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15579.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoLocal functions are not supported in MSVC. Bummer.
Simon Rozman [Tue, 10 Oct 2017 23:11:21 +0000 (01:11 +0200)] 
Local functions are not supported in MSVC. Bummer.

Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20171010231130.6832-4-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15587.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agolz4: Fix broken builds when pkg-config is not present but system library is
David Sommerseth [Mon, 2 Oct 2017 19:07:32 +0000 (21:07 +0200)] 
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>
7 years agolz4: Fix confused version check
David Sommerseth [Mon, 2 Oct 2017 16:18:12 +0000 (18:18 +0200)] 
lz4: Fix confused version check

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>
7 years agoCheck whether in pull_mode before warning about previous connection blocks
Selva Nair [Fri, 15 Sep 2017 18:46:15 +0000 (14:46 -0400)] 
Check whether in pull_mode before warning about previous connection blocks

In particular, this eliminates the message that says "explicit-exit-notify
is ignored by previous <connection> blocks" when the option is pushed.

Note: pull_mode is identified as "allowed & OPT_P_PULL_MODE" matching
with the definition in add_options().

Reported by: Eike Lohmann e.lohmann@ic3s.de
https://www.mail-archive.com/openvpn-users@lists.sourceforge.net/msg04052.h
tml

v2: move the check to verify_permissions() as suggested by
    Gert <gert@greenie.muc.de>

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1505501175-9016-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15452.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agoFix '--bind ipv6only'
hashiz [Thu, 28 Sep 2017 03:16:20 +0000 (12:16 +0900)] 
Fix '--bind ipv6only'

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

Signed-off-by: Gert Doering <gert@greenie.muc.de>
7 years agolz4: Move towards a newer LZ4 API
David Sommerseth [Thu, 7 Sep 2017 17:20:04 +0000 (19:20 +0200)] 
lz4: Move towards a newer LZ4 API

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>
7 years agoFix bounds check in read_key()
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>
7 years agosystemd: Enable systemd's auto-restart feature for server profiles
David Sommerseth [Wed, 6 Sep 2017 23:52:02 +0000 (01:52 +0200)] 
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>
7 years agotcp-server: ensure AF family is propagated to child context
Antonio Quartulli [Thu, 7 Sep 2017 09:55:30 +0000 (17:55 +0800)] 
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>
7 years agosystemd: Ensure systemd shuts down OpenVPN in a proper way
David Sommerseth [Wed, 6 Sep 2017 23:47:05 +0000 (01:47 +0200)] 
systemd: Ensure systemd shuts down OpenVPN in a proper way

By default, when systemd is stopping OpenVPN it will send the SIGTERM
to all processes within the same process control-group.  This can come
as a surprise to plug-ins which may have fork()ed out child processes.

So we tell systemd to only send the SIGTERM signal to the main OpenVPN
process and let OpenVPN take care of the shutdown process on its own.

If the main OpenVPN process does not stop within 90 seconds (unless
changed), it will send SIGKILL to all remaining processes within
the same process control-group.

This issue have been reported in both Debian and Fedora.

Trac: 581
Message-Id: <20170906234705.26202-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15369.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
[DS: Applied lazy-ack policy]

7 years agoOpenSSL: Always set SSL_OP_CIPHER_SERVER_PREFERENCE flag
Szilárd Pfeiffer [Mon, 4 Sep 2017 08:10:12 +0000 (10:10 +0200)] 
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>
7 years agoWarn that DH config option is only meaningful in a tls-server context
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>
7 years agofragment.c: simplify boolean expression
Antonio Quartulli [Thu, 24 Aug 2017 07:55:47 +0000 (15:55 +0800)] 
fragment.c: simplify boolean expression

!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>
7 years agodocs: Replace all PolarSSL references to mbed TLS
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>
7 years agotravis: reorder matrix to speed up build
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>
7 years agoman: Corrections to doc/openvpn.8
Richard Bonhomme [Sat, 19 Aug 2017 20:37:35 +0000 (21:37 +0100)] 
man: Corrections to doc/openvpn.8

Correct usage example: --verify-x509-name name-stub- name-prefix

  This was to correct "--verfiy-x509-name Server -name-prexif"
                   to "--verify-x509-name Server- name-prefix"

Escape all dashes (with some exceptions)

[DS: On-the-fly change - Updated copyright year from 2010 to 2017]

Signed-off-by: Richard Bonhomme <fragmentux@gmail.com>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170819203735.8681-1-fragmentux@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15297.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agofix a couple of typ0s in comments and strings
Antonio Quartulli [Sat, 19 Aug 2017 07:52:09 +0000 (15:52 +0800)] 
fix a couple of typ0s in comments and strings

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170819075209.28520-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15293.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agotls-crypt: don't leak memory for incorrect tls-crypt messages
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>
7 years agoroute: avoid definition of unused variables in certain configurations
Antonio Quartulli [Wed, 16 Aug 2017 12:55:04 +0000 (20:55 +0800)] 
route: avoid definition of unused variables in certain configurations

Although this patch adds more ifdefs, this is an easy
fix towards a no-warning-build process.

A proper cleanup should be carried out later on route.c.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Reviewed-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170816125504.21181-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15272.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoAdd coverity static analysis to Travis CI config
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>
7 years agocrypto: create function to initialize encrypt and decrypt key
Steffan Karger [Fri, 7 Jul 2017 04:47:04 +0000 (12:47 +0800)] 
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>
7 years agorename mroute_extract_addr_ipv4 to mroute_extract_addr_ip
Antonio Quartulli [Mon, 24 Jul 2017 14:35:59 +0000 (22:35 +0800)] 
rename mroute_extract_addr_ipv4 to mroute_extract_addr_ip

mroute_extract_addr_ipv4() is able to extract an IPv4 as well as an
IPv6. Remove the "v4" suffix from its name to make this behaviour more
explicit.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170724143559.11503-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15129.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoUse consistent version references
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>
7 years agoHighlight deprecated features
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.

[1] https://community.openvpn.net/openvpn/wiki/DeprecatedOptions

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170815215451.21662-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15261.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agotls-crypt: introduce tls_crypt_kt()
Steffan Karger [Sat, 12 Aug 2017 09:53:52 +0000 (11:53 +0200)] 
tls-crypt: introduce tls_crypt_kt()

Reduces code duplication (and prepares for tls-crypt-v2, which needs the
same functionality at more places).

Because tls_crypt_kt() is a static function we now need to include
tls_crypt.c from the tests, rather than tls_crypt.h.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <1502531632-16833-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15229.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoMove run_up_down() to init.c
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>
7 years agoremove the --disable-multi config switch
Antonio Quartulli [Wed, 16 Aug 2017 13:24:54 +0000 (21:24 +0800)] 
remove the --disable-multi config switch

This switch is broken and unmaintained.
However there wasn't any ticket about it so far,
which means that it is practically unused.

Get rid of it and simplify P2MP logic.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170816132454.13046-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15275.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agontlm: avoid breaking anti-aliasing rules
Antonio Quartulli [Wed, 16 Aug 2017 12:18:06 +0000 (20:18 +0800)] 
ntlm: avoid breaking anti-aliasing rules

The problem is visible when compiling with -O2:

ntlm.c: In function ‘ntlm_phase_3’:
ntlm.c:305:9: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing]
         if ((*((long *)&buf2[0x14]) & 0x00800000) == 0x00800000)

The spec suggests to interpret those 4 bytes as a long, but
this needs to be done carefully.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170816121806.26471-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15268.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoFix socks_proxy_port pointing to invalid data
Thomas Veerman via Openvpn-devel [Fri, 7 Jul 2017 19:59:41 +0000 (21:59 +0200)] 
Fix socks_proxy_port pointing to invalid data

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>
7 years agoDeprecate --keysize
Steffan Karger [Sat, 1 Jul 2017 11:29:51 +0000 (13:29 +0200)] 
Deprecate --keysize

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>
7 years agoDocument down-root plugin usage in client.down
Conrad Hoffmann [Wed, 2 Aug 2017 18:14:35 +0000 (20:14 +0200)] 
Document down-root plugin usage in client.down

Signed-off-by: Conrad Hoffmann <ch@bitfehler.net>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170802181435.14549-3-ch@bitfehler.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15164.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoUse provided env vars in up/down script.
Conrad Hoffmann [Wed, 2 Aug 2017 18:14:34 +0000 (20:14 +0200)] 
Use provided env vars in up/down script.

This makes the down script work both as regular down script as well as
with the
down-root plugin. The up script is just changed for consistency.

Signed-off-by: Conrad Hoffmann <ch@bitfehler.net>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170802181435.14549-2-ch@bitfehler.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15165.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agosample-plugins: fix ASN1_STRING_to_UTF8 return value checks
Steffan Karger [Fri, 28 Jul 2017 10:38:22 +0000 (12:38 +0200)] 
sample-plugins: fix ASN1_STRING_to_UTF8 return value checks

As we did in 2d032c7f for the ASN1_STRING_to_UTF8() calls in the core code,
we should also free(buf) if the function returns 0.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1501238302-16714-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15161.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoMove create_temp_file() out of #ifdef ENABLE_CRYPTO
Steffan Karger [Tue, 25 Jul 2017 21:02:34 +0000 (23:02 +0200)] 
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>
7 years agoAlways use default keysize for NCP'd ciphers
Steffan Karger [Thu, 20 Jul 2017 17:55:57 +0000 (19:55 +0200)] 
Always use default keysize for NCP'd ciphers

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>
7 years agoadd missing static attribute to functions
Antonio Quartulli [Fri, 11 Aug 2017 09:07:42 +0000 (17:07 +0800)] 
add missing static attribute to functions

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>
7 years agouse NULL instead of 0 when assigning pointers
Antonio Quartulli [Fri, 11 Aug 2017 09:07:44 +0000 (17:07 +0800)] 
use NULL instead of 0 when assigning pointers

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170811090744.31750-6-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15204.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoremove unused functions
Antonio Quartulli [Fri, 11 Aug 2017 09:07:43 +0000 (17:07 +0800)] 
remove unused functions

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170811090744.31750-5-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15205.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agomake function declarations C99 compliant
Antonio Quartulli [Fri, 11 Aug 2017 09:07:40 +0000 (17:07 +0800)] 
make function declarations C99 compliant

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].

Defects identified with sparse[2].

[1] ISO/IEC 9899:1999 spec, TC3 - section 6.7.5.3
[1] https://sparse.wiki.kernel.org/index.php/Main_Page

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170811090744.31750-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15203.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoOpenSSL: remove unreachable call to SSL_CTX_get0_privatekey()
Antonio Quartulli [Wed, 9 Aug 2017 07:42:37 +0000 (15:42 +0800)] 
OpenSSL: remove unreachable call to SSL_CTX_get0_privatekey()

In tls_ctx_load_ecdh_params() the SSL_CTX_get0_privatekey() function
is invoked only when "OPENSSL_VERSION_NUMBER >= 0x10002000L" and
curve_name is NULL.

However, under the very same conditions the code flow will
lead to an earlier return, thus never reaching the invocation of
SSL_CTX_get0_privatekey().

Restructure the surrounding code in order to make the if/else
block a bit easier to read and get rid of the unreachable
invocation.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170809074237.31291-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15186.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agotravis-ci: update pkcs11-helper to 1.22
Ilya Shipitsin [Wed, 9 Aug 2017 08:12:19 +0000 (13:12 +0500)] 
travis-ci: update pkcs11-helper to 1.22

use pkcs11-helper from https://github.com/OpenSC/pkcs11-helper/
to match build process used in windows installer build

Signed-off-by: Ilya Shipitsin <chipitsine@gmail.com>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170809081219.10367-1-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15187.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agotravis-ci: update openssl to 1.0.2l, update mbedtls to 2.5.1
Ilya Shipitsin [Mon, 7 Aug 2017 13:23:00 +0000 (18:23 +0500)] 
travis-ci: update openssl to 1.0.2l, update mbedtls to 2.5.1

Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170807132301.22759-2-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15171.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agocleanup: Move init_random_seed() to where it is being used
David Sommerseth [Tue, 25 Jul 2017 15:07:23 +0000 (17:07 +0200)] 
cleanup: Move init_random_seed() to where it is being used

The init_random_seed() function is only used by the init_static() in
init.c.  As this function was pretty basic and it is only being called
once, it was merged into init_static() instead of keeping it as a separate
function.

(I agree that calling functions often makes the code more readable, but
I would rather see that as a part of cleaning up the whole init_static()
function - in fact when moving all "unit tests" in init_static() to cmocka,
it will not be too bad in the end.)

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170725150723.14919-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15136.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agocontrib: Remove keychain-mcd code
David Sommerseth [Tue, 25 Jul 2017 13:03:14 +0000 (15:03 +0200)] 
contrib: Remove keychain-mcd code

After the security audits performed by Cryptography Engineering the
spring of 2017 [1], there were several concerns about the contrib code
for the macOS keychain support.  After more careful review of this
code base, it was considered to be in such a bad shape that it will
need a massive overhaul.  There were more issues than what the security
audit revealed.

It was attempted several times to get in touch with the contributor
of this code; with no response at all [2].  There has however
been some discussions with the Tunnelblick project [3]. There is one
person there willing to go through this and improve the situation.
The main Tunnelblick maintainer is also willing to include the improved
code to their project instead of having this as a contrib code in
the upstream OpenVPN project.

So this patch just removes the code which we will no longer
ship as part of OpenVPN - and the Tunnelblick project will take
over the responsibility for this code base on their own.  And since
this code base is purely macOS specific, this seems to be a far
better place for this code to reside.

Signed-off-by: David Sommerseth <davids@openvpn.net>
[1]
<http://community.openvpn.net/openvpn/wiki/QuarkslabAndCryptographyEngineer
Audits#OVPN-04-1:PossibleNULLpointerderefenceincontribkeychain-mcdcert_data
.c>
[2]
<https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14559.
html>
[3] <https://github.com/Tunnelblick/Tunnelblick/pull/369>
Acked-by: Jonathan K. Bullard <jkbullard@gmail.com>
Message-Id: <20170725130314.12919-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15130.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agoPrint ec bit details, refuse management-external-key if key is not RSA
Arne Schwabe [Sun, 23 Jul 2017 16:45:36 +0000 (18:45 +0200)] 
Print ec bit details, refuse management-external-key if key is not RSA

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use
    EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add
    compat headers for 1.0.2
V4: Formatting changes and change M_ERR to M_WARN

Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <1500828336-30314-1-git-send-email-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15124.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agontlm: improve code style and readability
Antonio Quartulli [Mon, 10 Jul 2017 04:34:41 +0000 (12:34 +0800)] 
ntlm: improve code style and readability

This patch does not introduce any functional or behavioural change.

The code in ntlm.c has been restyled to better to obey to the
new coding style and its readability has been a improved a bit.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170710043441.24770-5-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15028.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agontlm: restyle compressed multiple function calls
Antonio Quartulli [Mon, 10 Jul 2017 04:34:40 +0000 (12:34 +0800)] 
ntlm: restyle compressed multiple function calls

The gen_md4_hash() function is receiving as first argument a buffer
that is filled by a function invoked when evaluating the second
argument.

Although this is proper C, it makes the call invocation a bit obscure
because it is not immediately easy to grasp how the 'pwbuf' buffer is
filled.

Unroll the multiple function call onto lines and make the core more
readable.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170710043441.24770-4-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15030.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
7 years agontlm: convert binary buffers to uint8_t *
Antonio Quartulli [Mon, 10 Jul 2017 04:34:38 +0000 (12:34 +0800)] 
ntlm: convert binary buffers to uint8_t *

Several binary buffers in the ntlm component are stored
as char *, however this generates a lot of warnings, because
hashing functions expect something unsigned.

Convert binary buffers to uint8_t *, while use explicit cast
for buffers that are really carrying a string inside.

This commit removes several warnings from ntlm.c that you can
catch with "-Wall -std=c99".

[DS: Done minor typo-fixes in commit message at commit time]

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170710043441.24770-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15032.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
8 years agocleanup: Move write_pid() to where it is being used
David Sommerseth [Tue, 25 Jul 2017 14:57:18 +0000 (16:57 +0200)] 
cleanup: Move write_pid() to where it is being used

The write_pid() function is only used in openvpn.c, so no
need to have that in the misc.[ch] mixed bag.

[on-the-fly change: Added #include "platform.h"]

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170725145718.13175-1-davids@openvpn.net>
URL: https://www.mail-archive.com/search?l=mid&q=20170725145718.13175-1-davids@openvpn.net
Signed-off-by: David Sommerseth <davids@openvpn.net>
8 years agotls-crypt: avoid warnings when --disable-crypto is used
Antonio Quartulli [Fri, 7 Jul 2017 10:22:38 +0000 (18:22 +0800)] 
tls-crypt: avoid warnings when --disable-crypto is used

Avoid including the content of tls_crypt.h when --disable-crypto
is used, as it will trigger some warnings due to missing
structures declarations.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Steffan Karger <steffan@karger.me>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170707102238.8781-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15014.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
8 years agomanagement: preserve wait_for_push field when asking for user/pass
Antonio Quartulli [Fri, 7 Jul 2017 14:01:08 +0000 (22:01 +0800)] 
management: preserve wait_for_push field when asking for user/pass

With the introduction of the wait_for_push field in the auth_user_pass
structure, we have to make sure that such field is not accidentally
erased when the management asks the user for user/pass.

Erasing such field would mess up the logic introduced by
("Ignore auth-nocache for auth-user-pass if auth-token is pushed").

Thanks to David Sommerseth for the preliminary analysis and debugging.

Reported-by: Steven Haigh <netwiz@crc.id.au>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Tested-by: Steven Haigh <netwiz@crc.id.au>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170707140108.31612-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15015.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
8 years agoroute: improve error message
Antonio Quartulli [Thu, 20 Jul 2017 08:23:38 +0000 (16:23 +0800)] 
route: improve error message

- fix typ0 in message: NLSMG -> NLMSG
- use strerror() to print a human readable message
- don't print error message if error is ENETUNREACH: it means no route
  found

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170720082338.1302-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15101.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agofixup: also change missed openvpn_sleep() occurrences
Steffan Karger [Thu, 20 Jul 2017 19:17:02 +0000 (21:17 +0200)] 
fixup: also change missed openvpn_sleep() occurrences

45b2af9c missed some openvpn_sleep() occurrences in platform-specific code
in tun.c - fix that.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1500578222-21689-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15111.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoMove openvpn_sleep() to manage.c
Steffan Karger [Thu, 20 Jul 2017 16:00:35 +0000 (18:00 +0200)] 
Move openvpn_sleep() to manage.c

openvpn_sleep() is basically "service the management interface for x
seconds, then return".  Therefore, manage.c is a more suitable location
than the random collection of unrelated stuff called misc.c.

(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: Gert Doering <gert@greenie.muc.de>
Message-Id: <1500566435-29920-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15109.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoRemove strerror_ts()
Steffan Karger [Thu, 20 Jul 2017 11:39:00 +0000 (13:39 +0200)] 
Remove strerror_ts()

This function was only called in string format functions, which already
copy the contents, so all this ever did was adding redundant malloc() and
free() calls.

Also, this wasn't as thread-safe as it claims: another thread could still
change the string value between the strerror() and buf_printf() calls. So,
instead of a not needed false sense of thread-safeness, just be honest and
use strerror() directly.

(I think we should find a better place for everything currently in misc.c,
and get rid of it all together.  In this case, the better place is
/dev/null.  This patch is part of that effort.)

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1500550740-24773-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15105.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agontlm: unwrap multiple function calls
Antonio Quartulli [Wed, 12 Jul 2017 04:30:02 +0000 (12:30 +0800)] 
ntlm: unwrap multiple function calls

In order to improve code readability it is better to unwrap
multiple function calls onto multiple lines.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170712043002.11083-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15041.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agontlm: avoid useless cast
Antonio Quartulli [Mon, 10 Jul 2017 04:34:39 +0000 (12:34 +0800)] 
ntlm: avoid useless cast

The argument passed to my_strupr() is converted to an upper case
string by means of toupper(). The latter expects a single signed int
as argument, therefore it makes sense to have my_strupr() take a
signed argument too and avoid an explicit and an implicit cast.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170710043441.24770-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15031.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agodon't print errno twice
Antonio Quartulli [Thu, 13 Jul 2017 08:05:27 +0000 (16:05 +0800)] 
don't print errno twice

when passing the M_ERRNO flag to msg(), the latter will already
print the errno message (in a form of a string and number) for us,
hence there is no need to explicitly print it a second time.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170713080527.13299-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15057.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agouse M_ERRNO instead of explicitly printing errno
Antonio Quartulli [Thu, 13 Jul 2017 08:05:26 +0000 (16:05 +0800)] 
use M_ERRNO instead of explicitly printing errno

the msg() function will print the errno for us when
provided with the M_ERRNO flag.

Therefore, don't bother printing errno explicitly and always
pass M_ERRNO to msg().

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170713080527.13299-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15056.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agodoc: The CRL processing is not a deprecated feature
David Sommerseth [Wed, 28 Jun 2017 19:15:38 +0000 (21:15 +0200)] 
doc: The CRL processing is not a deprecated feature

The note related to the CRL processing was somehow put into
the deprecated section.  This is quite confusing.

Since this is a fairly important change, and there have been
a noticable amount of supports questions related to OpenVPN
not starting due to CRL errors, I put this into the
"New features" section labelled as an improvement.  Otherwise
I fear this would drown in the list of "User-visible Changes"
later on.

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170628191538.9135-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14985.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoUndo cipher push in client options state if cipher is rejected
Steffan Karger [Tue, 27 Jun 2017 22:20:29 +0000 (00:20 +0200)] 
Undo cipher push in client options state if cipher is rejected

Because of the way we re-use the options parser for both config files and
pushed options, we always update the local options state when we accept an
option.  This resulted in a pushed cipher being rejected the first time it
was pushed, but being accepted the second time.

This patch is a minimal way to resolve this issue in the master and
release/2.4 branches.  I'll send a more invasive patch for master, to
reset the entire options state on reconnects, later.

Trac: #906

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170627222029.26623-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14984.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer
Emmanuel Deloget [Thu, 29 Jun 2017 14:21:19 +0000 (16:21 +0200)] 
OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer

For unknown reason, the writer of the compat layer seemed to think that
this function was only present in OpenSSL 1.1. This is not the case at
all, since it has been introduced in OpenSSL before version 0.9.8.

Thus, there is no need to add this function to the compat layer, and it
can be safely removed.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170629142119.29502-2-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14988.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer
Emmanuel Deloget [Thu, 29 Jun 2017 14:21:18 +0000 (16:21 +0200)] 
OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer

For unknown reason, the writer of the compat layer seemed to think that
this function was only present in OpenSSL 1.1. This is not the case at
all, since it has been introduced in OpenSSL before version 0.9.8.

Thus, there is no need to add this function to the compat layer, and it
can be safely removed.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170629142119.29502-1-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14989.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoMove adjust_power_of_2() to integer.h
Steffan Karger [Wed, 21 Jun 2017 21:10:43 +0000 (23:10 +0200)] 
Move adjust_power_of_2() to integer.h

misc.c is a mess of incoherent functions, and is therefore included by
virtually all our source files.  That makes testing harder than it should
be.  As a first step of cleaning up misc.c, move adjust_power_of_2() to
integer.h, which is a more suitable place for a function like this.

This allows us to remove the duplicate implementation from test_argv.c.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20170621211043.6490-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14940.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoinit_key_ctx: key and iv arguments can (now) be const
Steffan Karger [Mon, 19 Jun 2017 11:51:05 +0000 (13:51 +0200)] 
init_key_ctx: key and iv arguments can (now) be const

In older OpenSSL, the key and iv arguments of EVP_CipherInit_ex() were not
const, which meant that our API could not be const either.  Since we
dropped support for OpenSSL 0.9.8, we can now fix our internal API.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1497873065-2229-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14881.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoSet tls-cipher restriction before loading certificates
Arne Schwabe [Mon, 26 Jun 2017 11:13:26 +0000 (13:13 +0200)] 
Set tls-cipher restriction before loading certificates

OpenSSL 1.1 does not allow MD5 signed certificates by default anymore.
This can be enabled again by settings tls-cipher "DEFAULT:@SECLEVEL=0" but
only if the cipher list is set before loading the certificates. This patch
changes the order of loading.

Acked-by: Christian Hesse <list@eworm.de>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1498475606-8337-1-git-send-email-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14961.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agocrypto: correct typ0 in error message
Antonio Quartulli [Tue, 27 Jun 2017 12:00:47 +0000 (20:00 +0800)] 
crypto: correct typ0 in error message

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170627120047.12304-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14975.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoFix typo in extract_x509_extension() debug message
Steffan Karger [Wed, 21 Jun 2017 21:21:31 +0000 (23:21 +0200)] 
Fix typo in extract_x509_extension() debug message

This message should use the external name, not the internal one.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170621212131.6776-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14939.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: remove pre-1.1 function from the OpenSSL compat interface
Emmanuel Deloget [Mon, 19 Jun 2017 15:35:13 +0000 (17:35 +0200)] 
OpenSSL: remove pre-1.1 function from the OpenSSL compat interface

HMAC_CTX_init() has been removed from OpenSSL 1.1. Both this function
and function HMAC_CTX_cleanup() has been replaced by HMAC_CTX_reset().

Commit aba98e9050eb54d72d921e70bcd422cb892b9c6c introduced support for
HMAC_CTX_init() for OpenSSL 1.1+ while other functions were mimicking
the OpenSSL 1.1 interface for earlier version. This is clearly not a
good idea -- a better approach would be to provide the new interface for
pre-1.1 versions in order to have the dependant code use only one
interface version. To implement that, we remove HMAC_CTX_init() from our
compatibility layer and implement HMAC_CTX_reset() in terms of a cleanup
followed by an init (as the regular HMAC_CTX_reset() function does in
OpenSSL 1.1. This change has a consequence on HMAC_CTX_free() which now
need to cleanup() the HMAC context before freeing it.

Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170619153513.5420-1-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14889.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agotravis-ci: added gcc and clang openssl-1.1.0 builds
Ilya Shipitsin [Mon, 19 Jun 2017 18:38:08 +0000 (23:38 +0500)] 
travis-ci: added gcc and clang openssl-1.1.0 builds

openssl build script was modified according to official openssl manual:
https://wiki.openssl.org/index.php/Compilation_and_Installation
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1497897488-15999-1-git-send-email-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14890.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoUpdate Changes.rst with relevant info for 2.4.3 release.
Gert Doering [Mon, 19 Jun 2017 17:44:00 +0000 (19:44 +0200)] 
Update Changes.rst with relevant info for 2.4.3 release.

(Since Changes.rst is mostly synchronized between master and 2.4.3,
and patches flow master->release/2.4, the change is done here even
if this particular commit is not directly part of 2.4.3 release)

General cleanup, whitespace fixes, quoting fixes by David Sommerseth
<davids@openvpn.net>

Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoFix remotely-triggerable ASSERT() on malformed IPv6 packet.
Gert Doering [Tue, 13 Jun 2017 20:08:32 +0000 (22:08 +0200)] 
Fix remotely-triggerable ASSERT() on malformed IPv6 packet.

Correct sanity checks on IPv6 packet length in mss_fixup_ipv6(),
and change the ASSERT() check in mss_fixup_dowork() into a simple
"return" (= the TCP header will simply not be inspected further).

CVE-2017-7508 has been assigned due to the serious nature of the
bug: it can be used to remotely shutdown an openvpn server or
client, if IPv6 and --mssfix are enabled and the IPv6 networks used
inside the VPN are known.

Found by Guido Vranken <guidovranken@gmail.com>.

v2: style changes

CVE: 2017-7508
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170613200832.15027-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20170613200832.15027-1-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoPrevent two kinds of stack buffer OOB reads and a crash for invalid input data
Guido Vranken [Fri, 19 May 2017 12:04:25 +0000 (14:04 +0200)] 
Prevent two kinds of stack buffer OOB reads and a crash for invalid input data

Pre-authentication remote crash/information disclosure for clients

If clients use a HTTP proxy with NTLM authentication (i.e.
"--http-proxy <server> <port> [<authfile>|'auto'|'auto-nct'] ntlm2"),
a man-in-the-middle attacker between the client and the proxy can
cause the client to crash or disclose at most 96 bytes of stack
memory. The disclosed stack memory is likely to contain the proxy
password.

If the proxy password is not reused, this is unlikely to compromise
the security of the OpenVPN tunnel itself.  Clients who do not use
the --http-proxy option with ntlm2 authentication are not affected.

CVE: 2017-7520
Signed-off-by: Guido Vranken <guidovranken@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <CAO5O-EJvHKid-zTj+hmFG_3Gv78ixqCayE9=C62DZaxN32WNtQ@mail.gmail.com>
URL: https://www.mail-archive.com/search?l=mid&q=CAO5O-EJvHKid-zTj+hmFG_3Gv78ixqCayE9=C62DZaxN32WNtQ@mail.gmail.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoFix potential double-free in --x509-alt-username (CVE-2017-7521)
Steffan Karger [Mon, 19 Jun 2017 09:28:40 +0000 (11:28 +0200)] 
Fix potential double-free in --x509-alt-username (CVE-2017-7521)

We didn't check the return value of ASN1_STRING_to_UTF8() in
extract_x509_extension().  Ignoring such a failure could result in buf
being free'd twice.  An error in ASN1_STRING_to_UTF8() can be caused
remotely if the peer can make the local process run out of memory.

The problem can only be triggered for configurations that use the
--x509-alt-username option with an x509 extension (i.e. the option
parameter starts with "ext:").

This issue was discovered, analysed and reported to the OpenVPN team by
Guido Vranken.

Extensive testing by Guido Vranken gives confidence that this function
is very unlikely to fail in real-world usage (using subjectAltName or
issuerAltName extensions) for other reasons than memory exhaustion.

CVE: 2017-7521
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>
Acked-by: Guido Vranken <guidovranken@gmail.com>
Message-Id: <1497864520-12219-6-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1497864520-12219-6-git-send-email-steffan.karger@fox-it.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoRestrict --x509-alt-username extension types
Steffan Karger [Mon, 19 Jun 2017 09:28:39 +0000 (11:28 +0200)] 
Restrict --x509-alt-username extension types

The code never supported all extension types.  Make this explicit by only
allowing subjectAltName and issuerAltName (for which the current code does
work).

Using unsupported extension fields would most likely cause OpenVPN to crash
as soon as a client connects.  This does not have a real-world security
impact, as such a configuration would not be possible to use in practice.

This bug was discovered, analysed and reported to the OpenVPN team by
Guido Vranken.

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>
Acked-by: Guido Vranken <guidovranken@gmail.com>
Message-Id: <1497864520-12219-5-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1497864520-12219-5-git-send-email-steffan.karger@fox-it.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoFix remote-triggerable memory leaks (CVE-2017-7521)
Steffan Karger [Mon, 19 Jun 2017 09:28:38 +0000 (11:28 +0200)] 
Fix remote-triggerable memory leaks (CVE-2017-7521)

Several of our OpenSSL-specific certificate-parsing code paths did not
always clear all allocated memory.  Since a client can cause a few bytes
of memory to be leaked for each connection attempt, a client can cause a
server to run out of memory and thereby kill the server.  That makes this
a (quite inefficient) DoS attack.

When using the --x509-alt-username option on openssl builds with an
extension (argument prefixed with "ext:", e.g. "ext:subjectAltName"), the
code would not free all allocated memory.  Fix this by using the proper
free function.

If ASN1_STRING_to_UTF8() returns 0, it didn't fail and *did* allocate
memory.  So also free the returned buffer if it returns 0.

These issues were found, analysed and reported to the OpenVPN team by Guido
Vranken.

CVE: 2017-7521
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>
Acked-by: Guido Vranken <guidovranken@gmail.com>
Message-Id: <1497864520-12219-4-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1497864520-12219-4-git-send-email-steffan.karger@fox-it.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agombedtls: require C-string compatible types for --x509-username-field
Steffan Karger [Mon, 19 Jun 2017 09:28:37 +0000 (11:28 +0200)] 
mbedtls: require C-string compatible types for --x509-username-field

In the --x509-username-field extenstion, we handle the subject string as
if it is a C string.  Make this assumption explicit and reject incomatible
ASN.1 string types.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1497864520-12219-3-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1497864520-12219-3-git-send-email-steffan.karger@fox-it.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agombedtls: fix --x509-track post-authentication remote DoS (CVE-2017-7522)
Steffan Karger [Mon, 19 Jun 2017 09:28:36 +0000 (11:28 +0200)] 
mbedtls: fix --x509-track post-authentication remote DoS (CVE-2017-7522)

asn1_buf_to_c_string() returned a literal string if the input ASN.1 string
contained a NUL character, while the caller expects a mutable string.
The caller will attempt to change this string, which allows a client to
crash a server by sending a certificate with an embedded NUL character.

(The other way around is not interesting, as servers are allowed to stop
a client by design.)

Impact analysis:
 * applies to mbedtls builds only
 * introduced in 2.4 (so 2.3 is not affected)
 * can only be exploited if the --x509-track option is used
 * requires the CA to sign a certificate with an embedded NUL in the
   certificate subject

This bug was discovered and reported to the OpenVPN security team by
Guido Vranken.

CVE: 2017-7522
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1497864520-12219-2-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1497864520-12219-2-git-send-email-steffan.karger@fox-it.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoauth-token with auth-nocache fix broke --disable-crypto builds
David Sommerseth [Mon, 19 Jun 2017 13:05:07 +0000 (15:05 +0200)] 
auth-token with auth-nocache fix broke --disable-crypto builds

After adding commit 571165360db0392fa83e, it broke builds where
the --disable-crypto was used with ./configure.  This was due to
the delayed_auth_pass_purge() which requires the crypto code paths
being called from init.c without the proper #ifdef encapsulation.

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170619130507.13892-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14883.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
8 years agoIgnore auth-nocache for auth-user-pass if auth-token is pushed
Antonio Quartulli [Sat, 25 Feb 2017 00:40:14 +0000 (08:40 +0800)] 
Ignore auth-nocache for auth-user-pass if auth-token is pushed

When the auth-token option is pushed from the server to the client,
the latter has to ignore the auth-nocache directive (if specified).

The password will now be substituted by the unique token, therefore
it can't be wiped out, otherwise the next renegotiation will fail.

Trac: #840
Cc: David Sommerseth <openvpn@sf.lists.topphemmelig.net>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170225004014.28638-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14194.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
8 years agoFix potential 1-byte overread in TCP option parsing.
Gert Doering [Sun, 18 Jun 2017 19:41:04 +0000 (21:41 +0200)] 
Fix potential 1-byte overread in TCP option parsing.

A malformed TCP header could lead to a one-byte overread when
searching for the MSS option (but as far as we know, with no
adverse consequences).

Change outer loop to always ensure there's one extra byte available
in the buffer examined.

Technically, this would cause OpenVPN to ignore the only single-byte
TCP option available, 'NOP', if it ends up being the very last
option in the buffer - so what, it's a NOP anyway, and all we
are interested is MSS, which needs 4 bytes.
(https://www.iana.org/assignments/tcp-parameters/tcp-parameters.xhtml)

Found and reported by Guido Vranken <guidovranken@gmail.com>.

Trac: #745

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170618194104.25179-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14874.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: don't use direct access to the internal of HMAC_CTX
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:29 +0000 (15:43 +0200)] 
OpenSSL: don't use direct access to the internal of HMAC_CTX

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including HMAC_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-8-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14797.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: don't use direct access to the internal of EVP_CIPHER_CTX
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:28 +0000 (15:43 +0200)] 
OpenSSL: don't use direct access to the internal of EVP_CIPHER_CTX

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_CIPHER_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-7-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14796.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: don't use direct access to the internal of EVP_MD_CTX
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:27 +0000 (15:43 +0200)] 
OpenSSL: don't use direct access to the internal of EVP_MD_CTX

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_MD_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-6-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14793.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoFix mbedtls fingerprint calculation
Steffan Karger [Sat, 20 May 2017 12:57:55 +0000 (14:57 +0200)] 
Fix mbedtls fingerprint calculation

Commit 'Migrate to mbed TLS 2.x' (86d8cd68) introduced a bug in mbedtls
builds where we would calculate the certificate fingerprint over the
(too-short) 'to-be-signed' length of the certificate, rather than over the
certificate including the signature.  Fix that.

The security impact of the incorrect calculation is very minimal; the last
few bytes (max 4, typically 4) are not verified by the fingerprint.  We
expect no real-world impact, because users that used this feature before
will notice that it has suddenly stopped working, and users that didn't
will notice that connection setup fails.

Even if the user managed to somehow extract the incorrect hash (e.g. by
reading out the tls_digest_* env vars using a --tls-verify script), the
impact is miminal: the last 4 bytes must still be properly signed by the
CA, and typically contain extension fields, or the last bytes of the
public key (which are hard to choose).  The most important bits of the
certificate were always checked: the version, serial, signature algorithm,
issuer, validity and subject.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1495285075-4957-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14711.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoAdd a DSA test key/cert pair to sample-keys
Steffan Karger [Sun, 18 Jun 2017 10:57:40 +0000 (12:57 +0200)] 
Add a DSA test key/cert pair to sample-keys

Makes it easier to test changes to DSA-related code.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170618105740.10090-1-steffan@karger.me>
URL: https://www.mail-archive.com/search?l=mid&q=20170618105740.10090-1-steffan@karger.me
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: force meth->name as non-const when we free() it
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:30 +0000 (15:43 +0200)] 
OpenSSL: force meth->name as non-const when we free() it

We are in control of meth->name (we string_alloc() it in RSA_meth_new())
so we know that we can free() it when it's no longer needed. Yet we have
to force the value to be non-const to avoid a compiler warning -- due to
the fact that OpenSSL defines the value as a const char*, regardless of
its origin.

Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-9-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14798.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: don't use direct access to the internal of DSA
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:26 +0000 (15:43 +0200)] 
OpenSSL: don't use direct access to the internal of DSA

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including DSA. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-5-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14791.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: don't use direct access to the internal of RSA
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:25 +0000 (15:43 +0200)] 
OpenSSL: don't use direct access to the internal of RSA

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including RSA. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-4-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14790.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
8 years agoOpenSSL: don't use direct access to the internal of EVP_PKEY
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:24 +0000 (15:43 +0200)] 
OpenSSL: don't use direct access to the internal of EVP_PKEY

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_PKEY. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-3-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14795.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>