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 :-)
Gert Doering [Sun, 19 Mar 2017 19:10:49 +0000 (20:10 +0100)]
Fix installation of IPv6 host route to VPN server when using iservice.
The "prepare IPv6 route message to interactive service" was properly
handing the correct interface index (r->adapter_index) for this case,
but then always overwrote the gateway address with our magic tun/tap
fe80::8 value. Only do this for "on tap adapter" routes.
Pinpointed by Selva Nair.
Trac #850
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20170319191049.23970-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14282.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 27740b376c1ca89a43dcff5c8309f1e1afecc5c9)
David Sommerseth [Sat, 25 Feb 2017 02:02:29 +0000 (03:02 +0100)]
cleanup: Remove faulty env processing functions
The env_set_add_to_environmenti() and env_set_remove_from_environment()
functions where not used in the code at all and they would cause an
ASSERT() in setenv_str_ex() later on, as it would not allow the
struct env_set *es pointer to be NULL (misc.c:807).
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170225020229.17287-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14195.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a87f119afcfcc1c855a6ea2ba3d765966f1f2591)
ignore remote-random-hostname if a numeric host is provided
Although it does not make sense to specify remote-random-hostname
when a numeric hostname is provided (being it the remote, the http
proxy or the socks server), this is still a valid configuration.
For this reason, this combination should still work as expected,
which means ignoring the randomization and directly using the
numeric IP.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170130152658.15786-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13993.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 3c748aeb5e4b82c449e7de28846a3915ab45aeec)
David Sommerseth [Fri, 27 Jan 2017 14:21:20 +0000 (15:21 +0100)]
plugin: Improve the handling of default plug-in directory
OpenVPN uses a default plug-in directore, set using PLUGINDIR when
running ./configure. If this is set, it will use $LIBDIR/openvpn/plugin.
When using --plugin, OpenVPN will load plug-ins from this directory with
the only exception if the plug-in filename is based on an absolute path.
Any other relative paths are relative to the PLUGINDIR.
This patch adds a third variant, using plug-in paths starting with '.'
In this case, OpenVPN will use the relative directory of where OpenVPN
was started, or the directory OpenVPN have changed into due to --cd
being used before the actual --plugin option.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170127142120.10492-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13970.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit f9609f1df9d8c070245b7c008dc54ac9ccdbe231)
Steffan Karger [Wed, 15 Mar 2017 21:20:20 +0000 (22:20 +0100)]
Be less picky about keyUsage extensions
We long recommended users to use --ns-cert-type to distinguish between
client and server certificates, but that extension is long deprecated and
now can even no longer be accurately checked in OpenSSL 1.1+. We support
a more modern alternative, --remote-cert-tls (which expands to
--remote-cert-ku + --remote-cert-eku), but are overly strict in checking
the keyUsage. This patch makes our implementation less picky, so that
correct-but-slightly-weird certicates will not immediately be rejected.
We currently allow users to specify a list of allowed keyUsage values, and
require that the remote certificate matches one of these values exactly.
This is for more strict than keyUsage usually requires; which is that a
certificate is okay to use if it can *at least* be used for our intended
purpose. This patch changes the behaviour to match that, by using the
library-provided mbedtls_x509_crt_check_key_usage() function in mbed TLS
builds, and performing the 'at least bits xyz' check for OpenSSL builds
(OpenSSL unfortunately does not expose a similar function).
Furthermore, this patch adds better error messages when the checking fails;
it now explains that is expects to match either of the supplied values,
and only does so if the check actually failed.
This patch also changes --remote-cert-tls to still require a specific EKU,
but only *some* keyUsage value. Both our supported crypto libraries will
check the keyUsage value for correctness during the handshake, but only if
it is present. So this still enforces a correct keyUsage, but is a bit
less picky about certificates that do not exactly match expectations.
This patch should be applied together with the 'deprecate --ns-cert-type'
patch I sent earlier.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1489612820-15284-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14265.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 92a5b9fb76cbb7f43a6aa86994ff559f06c55c7a)
Steffan Karger [Sat, 4 Mar 2017 18:49:57 +0000 (19:49 +0100)]
Deprecate --ns-cert-type
The nsCertType x509 extension is very old, and barely used. We already
have had an alternative for a long time: --remote-cert-tls uses the far
more common keyUsage and extendedKeyUsage extensions instead.
OpenSSL 1.1 longer exposes an API to (separately) check the nsCertType x509
extension. Since we want be able to migrate to OpenSSL 1.1, we should
deprecate this option immediately.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1488653397-2309-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14222.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 2dc332266449d5378f1fe04f950cbebf128ec9c9)
CRL: use time_t instead of struct timespec to store last mtime
As of now, we store the last mtime for the CRL file in a timespec
object. However we store seconds only and we ignore the subsecond
field (this came into being because not all platforms have nanoseconds
precision in timespec).
Given the above, we can safely replace the timespec object with a
simple time_t.
Reported-by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170316082117.21020-1-a@unstable.cc>
URL: http://www.mail-archive.com/search?l=mid&q=20170316082117.21020-1-a@unstable.cc Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit f3705dd1e711ee9f8546b841e4b18e9e9a224975)
Eric Thorpe [Wed, 15 Mar 2017 22:40:31 +0000 (09:40 +1100)]
Fix Building Using MSVC
This patch enables the building of OpenVPN for the 2.4 and master
branches using MSVC (Visual Studio 2013 / MSVC v120), which currently
doesn't work with 2.4 or a clone of master. 2013 is being used as it
reduces the complexity of the redistributable requirements and has
mostly complete C99 support. Further changes will be necessary for 2015
support when the switch is made.
Note the changes to config-msvc-version.h.in are more of a work around.
It was a simpler approach when compared to modifying msvc-generate.js to
handle m4 syntax, and so it may be dropped if there is an intention to
update the javascript generator.
Signed-off by: Eric Thorpe <eric@sparklabs.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <df273b9b-6ca4-a539-cdf5-d4f9f991896b@sparklabs.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14268.html
Steffan Karger [Thu, 9 Mar 2017 08:13:32 +0000 (09:13 +0100)]
Remove duplicate X509 env variables
Commit 13b585e8 added support for multiple X509 env variables with the
same name, but as a side effect caused these variables to pile up for
each renegotiation. The old code would simply overwrite the old variables
(as long as an equally-long chain was used for the new session).
To stop the variables from piling up, this commit removes any old X509
env variables if we start negotiating a new TLS session.
Trac: #854
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1489047212-31994-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14237.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit fd0361813cd3d5a55f3408a018e2ed776d79fef6)
Steffan Karger [Thu, 9 Mar 2017 10:47:35 +0000 (11:47 +0100)]
Fix types in WIN32 socket_listen_accept()
SOCKET_UNDEFINED is of type socket_descriptor_t (or SOCKET, in MS types),
so new_sd should be too. Also, the return value of this function is
always stored in a socket_descriptor_t variable, so it should return that
type (which makes sense now, because it returns new_sd) instead of an int.
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1489056455-6004-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14239.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 33e1a869fc6edb6bce5816b11dbecfaca57b20d4)
Ilya Shipitsin [Sun, 5 Mar 2017 17:21:32 +0000 (20:21 +0300)]
travis-ci: remove unused files
Those files were commited by mistake. I implemented building
dependencies in 4 separate scripts, later Steffan Karger combined
all 4 scripts into "build-deps.sh".
Emmanuel Deloget [Mon, 20 Feb 2017 14:32:34 +0000 (15:32 +0100)]
OpenSSL: use EVP_CipherInit_ex() instead of EVP_CipherInit()
The behavior of EVP_CipherInit() changed in OpenSSL 1.1 -- instead
of clearing the context when the cipher parameter was !NULL, it now
clears the context unconditionnaly. As a result, subsequent calls
to the function with additional information now fails.
The bulk work is done by EVP_CipherInit_ex() which has been part of the
OpenSSL interface since the dawn of time (0.9.8 already has it). Thus,
the change allows us to get the old behavior back instead of relying
on dirty tricks.
Gert Doering [Fri, 24 Feb 2017 13:52:22 +0000 (14:52 +0100)]
Fix '--dev null'
To test whether a server is reachable and all the key handling is
right, openvpn can connect with "--dev null --ifconfig-noexec" to
avoid needing to the client with elevated privileges.
This was erroring out for no good reason (because the "set environment
variables appropriately" code didn't know if this is a tun or tap
device...) - treat --dev null as "tap", done.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170224135222.44640-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14186.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 22c5381b71710ad0e1dbbccc1d5680fccb602311)
Gert Doering [Thu, 23 Feb 2017 08:49:54 +0000 (09:49 +0100)]
Add openssl_compat.h to openvpn_SOURCES
Commit b936ddfb63 introduced a new header file but forgot to include
it in the list of openvpn_SOURCES, so it did not get bundled in the
generated tarballs.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170223084954.58464-1-gert@greenie.muc.de>
URL: http://www.mail-archive.com/search?l=mid&q=20170223084954.58464-1-gert@greenie.muc.de Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 827c05732b0414dbf3cc05bf4ae6bfda042eadd3)
Emmanuel Deloget [Fri, 17 Feb 2017 22:00:48 +0000 (23:00 +0100)]
OpenSSL: don't use direct access to the internal of X509_STORE_CTX
OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including X509_STORE_CTX. We have to use the defined
functions to do so.
Fortunately, these functions have existed since the dawn of time so
we don't have any compatibility issue here.
fix redirect-gateway behaviour when an IPv4 default route does not exist
When no IPv4 default route exists, the "redirect-gateway" routine
aborts even if the sub-option "local" was specified or if we are
connecting to the remote host using IPv6.
This is not expected because in either case OpenVPN should not
bother checking the existence of the default route as it is not
required at all.
Therefore, skip the IPv4 default route check when "local" is
specified or we are connecting to an IPv6 remote host.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170119162518.31752-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13905.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 14670a9d654be48f92b58ac47e6f74d3dcfe1733)
attempt to add IPv6 route even when no IPv6 address was configured
Even if no IPv6 address is configured, OpenVPN still supports
transporting IPv6 segments, therefore adding an IPv6 route
should always be allowed.
However, the route might fail to be installed or may just not work
as expected, therefore, a proper warning should be printed to inform
the user of the possible pitfall.
Always allow adding an IPv6 route and print a WARNING when
no IPv6 address is configured for the interface.
Trac: #832 Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20170131112131.13570-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13994.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 2b7650e7ec9241745e4f66c932d6cffaece927d7)
Ilya Shipitsin [Sun, 29 Jan 2017 06:58:11 +0000 (11:58 +0500)]
Resolve several travis-ci issues
MBEDTLS_VERSION, OPENSSL_VERSION were defined twice - in both
.travis.yml and .travis/build-deps.sh files, the last one
defined OPENSSL_VERSION via nonexistent OPENSSL_VERION
variable, which lead us to use openssl-1.0.1 instead of
openssl-1.0.2, I removed variable definition from build-deps.sh
"cache: [ apt: true ]" is not a travis supported option, it was
introduced by mistake, I removed it
LD_LIBRARY_PATH was defined for the entire test run, it includes
custom openssl build, which was picked by "wget", so "wget"
could not verify SSL cert at https://www.openssl.org sometimes.
We do not want wget to pick our custom LD_LIBRARY_PATH, so I moved
that variable to "script" section
LD_LIBRARY_PATH was defined for both linux and osx environments,
for the second DYLD_LIBRARY_PATH must be defined instead
v2: Upgrade openssl, mbedtls to the most recent versions
v3: DYLD_LIBRARY_PATH was defined via LD_LIBRARY_PATH by mistake
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1485673091-7600-1-git-send-email-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13983.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 208c03ea145ed89083c43267733487c99a805069)
Christian Hesse [Tue, 24 Jan 2017 14:39:47 +0000 (15:39 +0100)]
systemd: Do not race on RuntimeDirectory
Different unit instances create and destroy the same RuntimeDirectory.
This leads to running instances where the status file (and possibly
more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d
configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content.
As /run is volatile we do not have to care about cleanup.
Signed-off-by: Christian Hesse <mail@eworm.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170124143947.27385-2-list@eworm.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13939.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 3de7be7b17de879a78eea4afe4c918c6104c635d)
Christian Hesse [Tue, 24 Jan 2017 14:39:46 +0000 (15:39 +0100)]
systemd: Use automake tools to install unit files
If systemd is enabled we install unit files to $libdir/systemd/system
(or the path specified by SYSTEMD_UNIT_DIR).
The unit files are generated on the fly with matching $sbindir.
Signed-off-by: Christian Hesse <mail@eworm.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20170124143947.27385-1-list@eworm.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13940.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit ca5b4c2aad2370be7862660d274b7485f2d0af71)
David Sommerseth [Tue, 24 Jan 2017 23:23:44 +0000 (00:23 +0100)]
systemd: Move the READY=1 signalling to an earlier point
Currently, OpenVPN will first tell systemd it is ready once the
log will be appended with "Initialization Sequence Completed".
This turns out to cause some issues several places.
First, it adds challenges if --chroot is used in the configuration;
this is already fixed. Secondly, it will cause havoc on static key
p2p mode configurations where the log line above will not happen
before either sides have completed establishing a connection. And
thirdly, if a client configuration fails to establish a connection
within 90 seconds, it will also fail. For the third case this may
not be a critical issue itself, as the host just needs to get
an Internet access established first - which in some scenarios may
take much longer than those 90 seconds systemd grants after the
OpenVPN client configuration is started.
The approach this patch takes is to consider OpenVPN ready when
all the initial preparations and configurations have completed - but
before a connection to a remote side have been attempted. This
also removes the need for specially handling the --chroot scenario.
The final "Initialization Sequence Completed" message update is
kept (though slightly simplified) to indicate we're in a good
state - even though this update will not be visible if --chroot
is used (which was the situation also before this patch).
Trac: #827, #801 Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Christian Hesse <mail@eworm.de>
Message-Id: <20170124232344.7825-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13945.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit e83a8684f0a0d944e9d53cdad2b543cfd1b6fbae)
Steffan Karger [Sun, 22 Jan 2017 16:04:41 +0000 (17:04 +0100)]
Use SHA256 for the internal digest, instead of MD5
Our internal options digest uses MD5 hashes to store the state, instead of
storing the full options string. There's nothing wrong with that, but it
would still be better to use SHA256 because:
* That makes it easier to make OpenVPN "FIPS-compliant" (forbids MD5)
* We don't have to explain anymore that MD5 is fine too
The slightly less bytes for the digest (16 instead of 32) and operations
per connection setup are not worth sticking to MD5.
Note that might SHA256 not be available in de crypto lib, OpenVPN will
refuse to start and shout "Message hash algorithm 'SHA256' not found".
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1485101081-9784-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13926.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 5b48e8c9f85442936f744c3c550d9d41fe8c7b60)
David Sommerseth [Fri, 20 Jan 2017 21:04:57 +0000 (22:04 +0100)]
git: Merge .gitignore files into a single file
We already track a lot of files over the whole directory structure
in the main .gitignore file. But a few additional ones had been
added into some of the subdirectories.
This unifies all these files into a master file for the whole project,
making it easier to know where to look at and edit if changes needs
to be done.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170120210457.3383-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13916.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit d14b3c60c7796736e07bc3cddb0ab3a58475793e)
Steffan Karger [Sat, 14 Jan 2017 14:10:20 +0000 (15:10 +0100)]
More broadly enforce Allman style and braces-around-conditionals
We want { and } aligned, which means also adding a newline between each
for() and {, while() and {, etc.
Also, we agreed to always use braces with conditionals. The previous
uncrustify config added these for if()s, now also add these for while()
and for().
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1484403020-6857-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13875.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 4cd4899e8e80efae03c584a760fd107251735723)
David Sommerseth [Tue, 10 Jan 2017 20:34:32 +0000 (21:34 +0100)]
management: >REMOTE operation would overwrite ce change indicator
If the management interface on a client received a signal while waiting
for input on the management channel, the "connection entry changed" status
would be overwritten even though nothing was changed. Which could lead
into connecting to the wrong server.
This patch improves this by adding a check if a bool value was changed to
false. This change happens only on signals.
Further, the former 'ret' value have been renamed to 'ce_changed', to
clarify what the expected return value contains. Plus adding some comments
related to this.
And finally do some code style cleanup, breaking up too long lines, adding
some air here and there to improve the readability.
Signed-off-by: David Sommerseth <davids@openvpn.net> Cc: Selva Nair <selva.nair@gmail.com> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1484080473-10415-1-git-send-email-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13851.html Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit e81f313a71e548638d9e9679226ee84b3b614f13)
Selva Nair [Tue, 3 Jan 2017 20:38:03 +0000 (15:38 -0500)]
Always release dhcp address in close_tun() on Windows.
Also make sure --dhcp-pre-release results in not just dhcp_release()
in open_tun() but a subsequent dhcp_renew() as well. Else dhcp transaction
gets aborted as this call to release() happens after the adapter status
is changed to connected.
Fixes Trac #807 (but can't say the same for Trac #665 without knowing
how to reproduce it)
v2: Mark --dhcp-release as obsolete in manpage and option parser, and
remove the unused dhcp_release variable.
Enforce dhcp-renew with dhcp-pre-release while parsing the option
instead of in open_tun().
Trac: #807
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1483475883-17450-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13814.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit db5b9b45508ea8f66ea80565279af3edd9300499)
projects / thirdparty / openvpn.git / log
