David Sommerseth [Mon, 14 Aug 2017 13:19:37 +0000 (15:19 +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.
[DS: On-the-fly merge conflict fix:
There was a conflict against the OpenSSL 0.9.6b workaround in v2.4.
Since we no longer support anything older than OpenSSL 0.9.8 in
release/2.4, whack that workaround and be more consistent with git
master those two places]
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>
(cherry picked from commit c43045ca0590364552fbd060cc65ee1c50a4866a)
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>
(cherry picked from commit cd5a74d0d7c6347b31e261e98ca8984819e594df)
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>
(cherry picked from commit 956bb1c32fa40ee184919b3ce569c90643a01b5b)
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>
(cherry picked from commit 72bcdfdc19243c1ed6cb8568f62f0c35e8b70f5f)
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].
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>
(cherry picked from commit 5b004f99d069fe0238aacbb0b3288872a4d7ae17)
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>
(cherry picked from commit e74e3a4db891b3ace0a96461c597d86e87be06f0)
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>
(cherry picked from commit 59e7e9fce8de6ea90d13baeaede83adc0b594e22)
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>
(cherry picked from commit bb23eca847c8edac9c3979b7f35468b74db00459)
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>
(cherry picked from commit e7e4070cb7b90f4836b65c53360166e11fc3f383)
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>
(cherry picked from commit 3322c558fa742cb823fa919f682486973abc4f8e)
- 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>
(cherry picked from commit 20d98427ef37e3b748dbcca2174cd243dcc963dc)
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>
(cherry picked from commit 45b2af9c7719d9a40c6c2b9d0693e4db0d917a04)
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>
(cherry picked from commit fd2a29ab2668fea9c0ac972d5ec69f00232c88b6)
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>
(cherry picked from commit 1cdfc9302aad8570360d278aded5fb9f110ca2b6)
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>
(cherry picked from commit e441d861881669c97906652c3278cc9a6c69a417)
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>
(cherry picked from commit f9ebfe1b5a011e55fb87a5026b1897c8ffb8f75e)
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>
(cherry picked from commit 3be9a1c1cd75627c30dca05bed28c84ad4dc1d37)
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.
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.
Ilya Shipitsin [Sat, 25 Feb 2017 18:00:04 +0000 (23:00 +0500)]
travis-ci: add 3 missing patches from master to release/2.4
travis-ci: add 'make distcheck' to test scenario, V2
in rare cases openvpn is built from tarball, it happens during "installer
build" process. "make distcheck" helps to prevent problems during such builds.
V2: limit "make distcheck" to one build configuration
Inspired by
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13032.html
build options are taken from regular windows installer builds
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>
(cherry picked from commit 9fc0e963c757ffec3cc9fbf797fb7609f409c370)
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.
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.
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>
(cherry picked from commit c3f47077a7756de5929094569421a95aa66f2022)
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>
(cherry picked from commit 7718c8984f04b507c1885f363970e2124e3c6c77)
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>
(cherry picked from commit cb4e35ece4a5b70b10ef9013be3bff263d82f32b)
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>
(cherry picked from commit d2a19185fd78030ce4a1bba6c9f83e0dac9e15a6)
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>
(cherry picked from commit 2d032c7fcdfd692c851ea2fa858b4c2d9ea7d52d)
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>
(cherry picked from commit 0007b2dbd12a83be3e4aeabc20550a5e16faf214)
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>
(cherry picked from commit 426392940c7060300a10077c389f5156c790c2f6)
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>
(cherry picked from commit 5bde5b6d1875fd87b116c943084df0d2f6aee6d0)
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>
(cherry picked from commit 22046a88342878cf43a9a553c83470eeaf97f000)
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.
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>
(cherry picked from commit 21a540f92bf65f39eb92967476eba0bcd2a34ef6)
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.
Emmanuel Deloget [Mon, 12 Jun 2017 13:43:23 +0000 (15:43 +0200)]
OpenSSL: don't use direct access to the internal of X509
OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including X509. We have to use the defined
functions to do so.
In x509_verify_ns_cert_type() in particular, this means that we
cannot directly check for the extended flags to find whether the
certificate should be used as a client or as a server certificate.
We need to leverage the X509_check_purpose() API yet this API is
far stricter than the currently implemented check. So far, I have
not been able to find a situation where this stricter test fails
(although I must admit that I haven't tested that very well).
We double-check the certificate purpose using "direct access" to the
internal of the certificate object (of course, this is not a real
direct access, but we still fetch ASN1 strings within the X509 object
and we check the internal value of these strings). This allow us to
warn the user if there is a discrepancy between the X509_check_purpose()
return value and our internal, less strict check.
We use these changes to make peer_cert a non-const parameter to
x509_verify_ns_cert_type(). The underlying library waits for a
non-const pointer, and forcing it to be a const pointer does not make
much sense (please note that this has an effect on the mbedtls part
too).
Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.
Gert Doering [Sun, 18 Jun 2017 09:22:44 +0000 (11:22 +0200)]
Fix edge case with clients failing to set up cipher on empty PUSH_REPLY.
The NCP (data channel crypto negotiation) code on the client side waits
for an incoming PUSH_REPLY before setting up the data channel crypto
parameters, because the PUSH_REPLY could contain a "cipher xxx" setting.
In the particular case of a empty PUSH_REPLY message, the relevant code
bits was not called because "we have not received any options, do not
bother to look into it in more detail" - so, ciphers were not set up,
resulting in an error message like this:
Key [AF_INET]... [0] not initialized (yet), dropping packet.
Remove that check, always init the crypto layer on PUSH_REPLY.
Trac: #903
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170618092244.8801-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14856.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit bd230079d98bfe6aec70b7aedefdffcdbd0e56da)
David Sommerseth [Wed, 29 Mar 2017 09:36:48 +0000 (11:36 +0200)]
copyright: Update GPLv2 license texts
The COPYRIGHT.GPL file was slightly out-of-sync with the last GPLv2
license from Free Software Foundation, Inc.
The changes are primarily a new address, which required touching almost
all the project files.
Except of that, it is just minor adjustments to formatting, removal of
form-feed characters and referencing "GNU Lesser General Public License"
instead of "GNU Library General Public License".
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170329093648.10156-1-davids@openvpn.net>
URL: https://www.mail-archive.com/search?l=mid&q=20170329093648.10156-1-davids@openvpn.net Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit caa54ac398db25b72d7d1d633d2ee330b5b8a3e9)
Guido Vranken [Fri, 16 Jun 2017 00:58:56 +0000 (02:58 +0200)]
Fix a null-pointer dereference in establish_http_proxy_passthru()
Prevents that the client crashes if the peer does not specify
the 'realm' and/or 'nonce' values. These pointers are
dereferenced in DigestCalcHA1() and DigestCalcResponse();
hence, if not set, a null-pointer dereference would occur.
Signed-off-by: Guido Vranken <guidovranken@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1497574736-2092-1-git-send-email-gv@guidovranken.nl>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14844.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 14865773ad64d861128bc80ad44c37bdc307c996)
Steffan Karger [Mon, 22 May 2017 13:54:13 +0000 (15:54 +0200)]
openssl: fix overflow check for long --tls-cipher option
The length check in tls_ctx_restrict_ciphers() did not check for overflow,
which could lead to a stack buffer overflow.
This has no real-world impact, because --tls-cipher can only be specified
by entities that are allowed to supply config settings. Since those
entities can also change --script-security and call scripts and/or
plugins, these users already have code execution at the level of the
openvpn process. In other words: the attacker would not gain any
capabilities. Nevertheless, a nasty bug that we should fix.
This bug was discovered and reported to the OpenVPN security team by
Guido Vranken.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1495461253-20111-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14716.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e6bf7e033d063535a4414a4cf49c8f367ecdbb4f)
Guido Vranken [Wed, 7 Jun 2017 23:02:38 +0000 (01:02 +0200)]
Ensure option array p[] is always NULL-terminated
Add one element (a terminating NULL pointer) to the array into
which parse_line() stores the arguments. This prevents that options
that traverse this array until a terminator is seen (for instance
options that call no_more_than_n_args) will peek beyond buffer bounds.
In the worst case this might lead to a crash (stack overflow, not
likely in practice).
Signed-off-by: Guido Vranken <guidovranken@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <CAO5O-EKCLjPpdKUH6cCoqoZDAfekSafpc7Ga55H2_5Hs4rBopg@mail.gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14757.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 8b03d3d9307b407b0da98ebefb052b1fa87aefe7)
The pointer to the packet content doesn't seem to be word-aligned,
resulting in a SIGBUS when accessing it as a pointer to struct ip that
contains bit fields.
Replace with struct openvpn_iphdr and OPENVPN_IPH_GET_VER, which only
does a one byte access and thus isn't affected by alignement. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <87ink3vpcs.fsf@ritchie.wxcvbn.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14769.html
Guido Vranken [Thu, 8 Jun 2017 22:04:36 +0000 (00:04 +0200)]
Fix memory leak in add_option() for option 'connection'
This patch ensures that if an error occurs while processing
the 'connection' directive of an options specification,
the variable 'struct options sub', which is initialized
with init_options(), is properly freed with uninit_options().
Signed-off-by: Guido Vranken <guidovranken@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <CAO5O-E+HdfHNAK=--55054AB374U-Ky4q_-3M114makfPdSnBw@mail.gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14764.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit d89e14d92623731d2fa6343a11072caab32e13cd)
Gert Doering [Thu, 18 May 2017 19:13:11 +0000 (21:13 +0200)]
Remove erroneous limitation on max number of args for --plugin
Commit 3d6a4cded2 introduced a maximum limit of possible arguments
for most options, to error out on config lines with too-many args.
Commit 82acf21634 extended the limit for "--plugin" to accept "one
mandatory, one optional" argument.
Both are wrong - "--plugin" can accept an arbitrary number of arguments,
but this is not directly obvious from options.c, one needs to check
plugins.c / plugin_option_list_add() and misc.c / make_extended_arg_array()
to see that.
Thus, remove the max limit for "--plugin" completely.
(Reported by SviMik on IRC)
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170518191311.4769-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14678.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 3f181eaa324892845e0857d80c154512d9e8c59c)
Steven McDonald [Thu, 13 Apr 2017 17:31:29 +0000 (03:31 +1000)]
Fix gateway detection with OpenBSD routing domains
When OpenVPN is started using a non-default routing table on OpenBSD
(e.g., with 'route -T10 exec openvpn ...'), it hangs forever trying to
read its default gateway from a PF_ROUTE socket. This is because
rtm_tableid is not being initialised after bzeroing the rt_msghdr we
write to the socket, so we end up asking the kernel for the default
route in routing table 0.
By default, the OpenBSD kernel will not respond to requests for routing
table 0 from a process running in a different routing table, and even
if it did, it would give us the wrong default gateway.
The solution here is to set rtm_tableid to the value returned by
getrtable(2), which always succeeds and returns the calling process's
current routing table.
This patch makes the test suite (without a t_client.rc) pass when run
in a non-default routing table, where it would fail previously. It has
also been successfully tested in client mode against both git master
and OpenVPN 2.4.1 from ports on an OpenBSD -current system.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170413173129.87367-1-steven@steven-mcdonald.id.au>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14461.html
Gert Doering [Thu, 18 May 2017 10:22:46 +0000 (12:22 +0200)]
Fix NCP behaviour on TLS reconnect.
If a client reconnects on a hard-restart from the same port (due to --bind
in use on the client), both sides will handle this as a "reconnect" and
not a "full new connect" internally, re-using existing crypto context.
The client will still ask the server for pushed options, and the server
code to handle this refuses to do NCP if a key has already been negotiated
(because there is no way to *change* the cipher after that) - which ends
up in "the client uses the non-negotiated cipher from the config file,
while the server uses the previously-negotiated NCP cipher", and nothing
works.
The easy workaround: if we find us in the situation that we think NCP
has already been done, just re-push "cipher o->ciphername" with the
current cipher for this client context.
All credits for this go to Stefan Behrens <sbehrens@giantdisaster.de>
who found and diagnosed the issue in trac #887, came up with a first
patch to solve the issue quite similar to this (simplified) one, and
helped testing.
crypto: Enable SHA256 fingerprint checking in --verify-hash
This enhances --verify-hash with an optional algorithm flag. If not
provided, it defaults to SHA1 to preserve backwards compatbilitity with
existing configurations. The only valid flags are SHA1 and SHA256.
In addition enhance the layout of the --verify-hash section in the man
page.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170504204201.1257-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14538.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 2193d7c08484d56ed07ba2e649abc2d08adcb245)
ValdikSS [Wed, 10 May 2017 18:47:53 +0000 (21:47 +0300)]
Set a low interface metric for tap adapter when block-outside-dns is in use
Windows 10 before Creators Update used to resolve DNS using all
available adapters and IP addresses in parallel. Now it still
resolves addresses using all available adapters but in a round-robin
way, beginning with random adapter.
This behaviour introduces significant delay when block-outside-dns is
in use. Fortunately, setting low metric for the TAP interface solves
this issue, making Windows always pick TAP adapter first and disable
round-robin.
Signed-off-by: ValdikSS <iam@valdikss.org.ru> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20170510184753.27145-1-valdikss@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14624.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 27aa87283f6e766507287649aa5a63f1f5172645)
Steffan Karger [Tue, 9 May 2017 19:30:09 +0000 (21:30 +0200)]
Drop packets instead of assert out if packet id rolls over (CVE-2017-7479)
Previously, if a mode was selected where packet ids are not allowed to roll
over, but renegotiation does not succeed for some reason (e.g. no password
entered in time, certificate expired or a malicious peer that refuses the
renegotiaion on purpose) we would continue to use the old keys. Until the
packet ID would roll over and we would ASSERT() out.
Given that this can be triggered on purpose by an authenticated peer, this
is a fix for an authenticated remote DoS vulnerability. An attack is
rather inefficient though; a peer would need to get us to send 2^32
packets (min-size packet is IP+UDP+OPCODE+PID+TAG (no payload), results in
(20+8+1+4+16)*2^32 bytes, or approx. 196 GB).
This is a fix for finding 5.2 from the OSTIF / Quarkslab audit.
CVE: 2017-7479 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: <1494358209-4568-3-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494358209-4568-3-git-send-email-steffan.karger@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit e498cb0ea8d3a451b39eaf6f9b6a7488f18250b8)
Steffan Karger [Tue, 9 May 2017 19:30:08 +0000 (21:30 +0200)]
Don't assert out on receiving too-large control packets (CVE-2017-7478)
Commit 3c1b19e0 changed the maximum size of accepted control channel
packets. This was needed for crypto negotiation (which is needed for a
nice transition to a new default cipher), but exposed a DoS
vulnerability. The vulnerability was found during the OpenVPN 2.4 code
audit by Quarkslab (commisioned by OSTIF).
To fix the issue, we should not ASSERT() on external input (in this case
the received packet size), but instead gracefully error out and drop the
invalid packet.
CVE: 2017-7478 Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494358209-4568-2-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494358209-4568-2-git-send-email-steffan.karger@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 5774cf4c25e1d8bf4e544702db8f157f111c9d93)
Steffan Karger [Tue, 9 May 2017 18:42:48 +0000 (20:42 +0200)]
Document tls-crypt security considerations in man page
The tls-crypt commit message contained an elaborate discussion on the
function's security properties. This commit adds the gist of that
discussion, "rotate keys periodically" to the man page.
(The 'real' solution will follow later: add support for per-client
tls-crypt keys. That will make tls-crypt useful for VPN providers too.)
Note to non-crypto-geek reviewers: please verify that this text is clear
enough to explain you when you need to replace tls-crypt keys.
Note to crypto-geek reviewers: please check the numbers - see the
--tls-crypt commit message (c6e24fa3) for details.
Selva Nair [Tue, 9 May 2017 19:44:29 +0000 (15:44 -0400)]
In auth-pam plugin clear the password after use
v2: Change the plugin open to use v3 API so that secure_memzero()
exported from OpenVPN can be used.
v3: Relaxe API compatibility check: struct version 4 or higher
will have secure_memzero exported.
Note: context is cast as (openvpn_plugin_handle_t *) for consistency
with the current plugin header. If/when the header is fixed, change
this cast as well.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494359069-13824-1-git-send-email-selva.nair@gmail.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494359069-13824-1-git-send-email-selva.nair@gmail.com Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit f403b9a2bf93f0fa35ee8316c2d219f48638a3e5)
Steffan Karger [Tue, 9 May 2017 18:32:44 +0000 (20:32 +0200)]
Always clear username/password from memory on error
This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.4), we are with their analysis:
"There’s a special case where the client username and password are not
erased when the server is launched without an external script or
authentication plugin. While being invalid, this configuration does not
raise any error. If the client transmits its credentials and the session
is not established (for instance if the certificates chain has not been
verified), these credentials are not erased from memory by the server.
The likelihood of an occurrence of this issue in real life is
exceptionally low since an attacker needs elevated privileges on the
server to exploit this kind of information leak. The severity of this
issue is rated as very low."
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494354764-19354-1-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494354764-19354-1-git-send-email-steffan.karger@fox-it.com Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 2b60198e08a9d7e8de9beeb65a587ee34107efe8)
The provides plug-ins with a safe and secure way to santize sensitive
information such as passwords, by re-using the secure_memzero()
implementation in OpenVPN.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20170505184622.24520-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14546.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit f018dfcc3631f165232afa3d13dc2a608bdb6ce7)
Steffan Karger [Mon, 8 May 2017 21:44:38 +0000 (23:44 +0200)]
Restore pre-NCP frame parameters for new sessions
As reported in trac #879, as of the introduction of NCP we always adjust
the frame parameters on session negotiations, but do not reset the frame
state for a new session on an existing state instance. That caused the
frame parameters to be reduced for each reconnect, resulting in smaller
and smaller packet size limits until no traffic could go through the
tunnel at all. This patch resolves that omission.
Trac: #879 Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1494279878-24601-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14589.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 9900e023bcc49964d33e6f22c2b6223f8932acf8)
Steffan Karger [Tue, 9 May 2017 08:12:43 +0000 (10:12 +0200)]
mbedtls: correctly check return value in pkcs11_certificate_dn()
mbedtls_x509_dn_gets() would not always return -1 error, which could cause
us to incorrectly continue after the function call failed. To fix this,
just call our own x509_get_subject(), which does all the neccesary error
checking correctly.
pkcs11_certificate_dn() is only called by show_pkcs11_ids(), to list the
certificates on the pkcs11 token. Therefor, this mistake did not have a
security impact.
This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.3).
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1494317563-6303-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14591.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 423bb16e8a8fe22a907f469074a25533208fa0bc)
Steffan Karger [Sun, 7 May 2017 11:01:18 +0000 (13:01 +0200)]
Fix memory leak in x509_verify_cert_ku()
If keyUsage was only required to be present, but no specific value was
required, we would omit to free the extracted string. This happens as of
2.4.1, if --remote-cert-tls is used. In that case we leak a bit of
memory on each TLS (re)negotiation.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494154878-18403-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14563.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 7b94d3bbbea46efcea12e1df24da52fe508d0173)
Steffan Karger [Fri, 5 May 2017 17:44:51 +0000 (19:44 +0200)]
cleanup: merge packet_id_alloc_outgoing() into packet_id_write()
The functions packet_id_alloc_outgoing() and packet_id_write() were
always called in tandem. Instead of forcing the caller to allocate a
packet_id_net to do so, merge the two functions. This simplifies the API
and reduces the chance on mistakes in the future.
This patch adds unit tests to verify the behaviour of packet_id_write().
Verifying that we assert out correctly required the change to mock_msg.c.
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: <1494006291-3522-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14541.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a87e1431baccd49a9344cfc63ab7446c4317fa2f)
David Sommerseth [Mon, 10 Apr 2017 22:28:28 +0000 (00:28 +0200)]
Make --cipher/--auth none more explicit on the risks
The warning provided to --cipher and --auth using the 'none' setting may
not have been too clearly understandable to non-developers or people not
fully understanding encryption and cryptography. This tries to improve
that.
While at it, also break up the long source lines.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170410222828.23612-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14436.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 7a1b6a0dd706a81897457b0456a951c0b30bbcfb)
David Sommerseth [Wed, 29 Mar 2017 09:49:25 +0000 (11:49 +0200)]
docs: Fixed man-page warnings discoverd by rpmlint
Running rpmlint against Fedora RPM packages revealed these warnings:
W: manual-page-warning /usr/share/man/man8/openvpn.8.gz 2738:
a special character is not allowed in a name
W: manual-page-warning /usr/share/man/man8/openvpn.8.gz 2740:
a special character is not allowed in a name
This is just a typo mistake in the .B formatting, missing a trailing
space.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170329094925.25644-1-davids@openvpn.net>
URL: http://www.mail-archive.com/search?l=mid&q=20170329094925.25644-1-davids@openvpn.net Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 9636196d5efb719cf1011397a360d46bccb3fe29)
David Sommerseth [Tue, 28 Mar 2017 20:53:46 +0000 (22:53 +0200)]
auth-token: Ensure tokens are always wiped on de-auth
If tls_deauthenticate() was called, it could in some scenarios leave the
authentication token for a session in memory. This change just ensures
auth-tokens are always wiped as soon as a TLS session is considered
broken.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170328205346.18844-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14344.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit daab0a9fa8ff4f40e8a34707db0ac156d49fbfcb)
Gert Doering [Sun, 19 Mar 2017 18:41:35 +0000 (19:41 +0100)]
Make ENABLE_OCC no longer depend on !ENABLE_SMALL
OCC is useful functionality which (according to LEDE devs) adds only
about 3k to the binary size - and if the embedded router folks can
afford this trade-off, everyone else can :-)
projects / thirdparty / openvpn.git / log
