Arne Schwabe [Fri, 5 Jun 2020 11:25:18 +0000 (13:25 +0200)]
Make cipher_kt_get also accept OpenVPN config cipher name
Basically calls to cipher_kt_get were calling
translate_cipher_name_from_openvpn. The only two exception were the
(broken) unit test and tls-crypt that uses cipher_kt_get("AES-256-CTR")
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20200605112519.22714-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19969.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 5 Jun 2020 11:25:17 +0000 (13:25 +0200)]
Make cipher_kt_name always return normalised cipher name
The mbed TLS variant of the call already returned the normalised
name while the OpenSSL variant did not. On top of that, all calls but
one to cipher_kt_name were translate_cipher_name_to_openvpn. This commit
moves the call of translate_cipher_name_to_openvpn into cipher_kt_name
or avoids calling it twice in the case of mbed TLS.
The one case that did not translate_cipher_name_to_openvpn is an
internal ssl_openssl.c method that should call EVP_CIPHER_name anyway.
Also simplify cipher_name_cmp function that is only used by
openvpn --show-ciphers with the modified cipher_kt_name
function.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20200605112519.22714-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19970.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 4 Jun 2020 23:53:38 +0000 (01:53 +0200)]
Add .git-blame-ignore-revs with reformat commits
This allows git blame to ignore reformatting changes and instead
to show the previous commit that changed the line.
To avoid manually building the list of commits this commit
adds a file with a list of reformatting commits. I might have
missed a few but this should be a good start. To use the file
use:
multi: skip IPv4 logic in multi_select_virtual_addr() if no pool is configured
When no IPv4 pool is configured (but we have an IPv6 pool
only), the multi_select_virtual_addr() function will spit
a warning when allocating an address for a new client.
This happens because the code will check for some IPv4
bits and will see that they are missing.
However, these bits are not really important, because in
this use case we don't want to configure any IPv4 address
at all.
For this reason it is safe to wrap this entire logic in
an if-block that just does not execute when no IPv4 pool
is configured.
This avoids the warning and will also avoid any other
hidden side effect.
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: <20200610084549.4028-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20012.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit 6a8cd033 ("pool: add support for ifconfig-pool-persist with IPv6
only") has accidentally introduced an include for 'options.h', which
revealed to not be useful at all. Remove it.
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: <20200610090100.29738-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20011.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 9 Jun 2020 08:02:29 +0000 (10:02 +0200)]
Simplify pool size handling, fix possible array overrun on pool reading.
Remove separate ipv4.size and ipv6.size in the pool structure, return
to a single pool_size, which is also the allocated array size.
All calls to ifconfig_pool_size() change to "pool->size" now.
pool->size is set to the size of the active pool, or if both IPv4 and IPv6
are in use, to the smaller size (same underlying logic as in 452113155e7,
but really put it into the size field).
This fixes a SIGSEGV crash if an ifconfig-pool-persist file is loaded
that has IPv6 and no IPv4 (= ipv6 handle is used) and that has more
entries than the IPv4 pool size (comparison was done with ipv6.size,
not with actual pool size), introduced by commit 6a8cd033b18.
While at it, fix pool size calculation for IPv6 pools >= /112
(too many -1), introduced by commit 452113155e7.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200609080229.2564-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20006.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
route: warn on IPv4 routes installation when no IPv4 is configured
Same as already happens for IPv6, it is useful for the user to throw a
warning when an IPv4 route is about to be installed and the tun interface
has no IPv4 configured.
The twin message for IPv4 is adapted to have the same format.
The warning is not fatal, becuase the route might actually be external
to the tun interface and therefore it may still work.
At the same time, modify the error message used for a route
installation failure to explicitly mention "IPv4" since this it is
used in the IPv4 code path only.
Trac: #208 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200530000600.1680-6-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19946.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
James Bottomley [Thu, 28 May 2020 22:59:19 +0000 (15:59 -0700)]
crypto_openssl: add initialization to pick up local configuration
The test programme for the new openssl engine code requires overriding
the system default configuration file to point to the location of the
test engine. Add an initialization stanza that makes this behaviour
universal, so now anyone running openvpn configured with openssl can
specify their own configuration file with the OPENSSL_CONF environment
variable.
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200528225920.6983-3-James.Bottomley@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19936.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
James Bottomley [Thu, 28 May 2020 22:59:18 +0000 (15:59 -0700)]
openssl: add engine method for loading the key
As well as doing crypto acceleration, engines can also be used to load
key files. If the engine is set, and the private key loading fails
for bio methods, this patch makes openvpn try to get the engine to
load the key. If that succeeds, we end up using an engine based key.
This can be used with the openssl tpm engines to make openvpn use a
TPM wrapped key file.
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200528225920.6983-2-James.Bottomley@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19937.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
allow usage of --server-ipv6 even when no --server is specified
Until now OpenVPN has not allowed to specify --server-ipv6
if no --server was also set. This constraint comes from the
fact that most of the IPv6 logic (i.e. ifconfig-pool handling)
relied on IPv4 components to be activated and configured as
well.
Now that the IPv6 code path has been made independent from
IPv4, it is finally possible to to relax the constraint
mentioned above and make it possible for the user to have a
configurations with --server-ipv6 only.
Trac: #208 Signed-off-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200530000600.1680-4-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19949.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Jeremy Evans [Wed, 20 May 2020 18:34:04 +0000 (11:34 -0700)]
Switch assertion failure to returning false
This assertion failure can be hit in production, which causes the
openvpn server process to stop and all clients to be disconnected.
Bug #1270 has been filed for this issue on Trac by another user
who has experienced the issue, and this patch attempts to address it.
Tracing callers, it appears that some callers check ks->authenticated
before calling, but others do not. It may be possible to add the check
for the callers that do not check, but this seems to be a simpler
solution.
To give some background, we hit this assertion failure, with the
following log output:
```
Tue May 19 15:57:05 2020 username/73.135.141.11:1194 PUSH: Received
control message: 'PUSH_REQUEST'
Tue May 19 15:57:05 2020 username/73.135.141.11:1194 SENT CONTROL
[username]: 'PUSH_REPLY,redirect-gateway
def1,comp-lzo,persist-key,persist-tun,route-gateway 10.28.47.1,topology
subnet,ping 10,ping-restart 120,ifconfig 10.28.47.38 255.255.255.0,peer-id
89' (status=1)
Tue May 19 15:57:05 2020 username/73.135.141.11:1194 Assertion failed at
/path/to/openvpn-2.4.7/src/openvpn/ssl.c:1944 (ks->authenticated)
Tue May 19 15:57:05 2020 username/73.135.141.11:1194 Exiting due to fatal
error
Tue May 19 15:57:05 2020 username/73.135.141.11:1194 Closing TUN/TAP
interface
```
using the following OpenVPN server configuration:
```
port 1194
proto udp
dev-type tun
ca ca.crt
cert server.crt
key server.key
dh dh.pem
topology subnet
push "redirect-gateway def1"
push "comp-lzo"
push "persist-key"
push "persist-tun"
keepalive 10 120
comp-lzo
user nobody
group nobody
persist-key
persist-tun
cd /home/openvpn/server
chroot /var/empty
daemon
verb 3
crl-verify crl.pem
tls-auth ta.key 0
cipher AES-256-CBC
tls-version-min 1.2
tls-cipher ECDHE-RSA-AES256-GCM-SHA384
ncp-disable
mute-replay-warnings
script-security 3
auth-user-pass-verify "ldap-auth/ldap-auth" via-env
auth-user-pass-optional
```
The failed assertion is inside the function
`tls_session_generate_data_channel_keys`, which is called 3 other places
in `ssl.c.`:
* `key_method_2_write`: checks for `ks->authenticated` before calling
* `key_method_2_read`: appears to run in client mode but not in server
mode
* `tls_session_update_crypto_params`: runs in server mode and does not
check before calling
That leads me to believe the problem caller is
`tls_session_update_crypto_params`. There.s three callers of
`tls_session_update_crypto_params`:.
* `incoming_push_message` (`push.c`): Probably this caller, since the
server pushes configuration to clients, and the log shows the
assertion failure right after the push reply.
* `multi_process_file_closed` (`multi.c`): Not this caller. NCP is
disabled in config, and async push was not enabled when compiling.
* `do_deferred_options` (`init.c`): Not this caller. The server
configuration doesn't pull.
Changing the assertion to returning false appears to be the simplest
fix. Another approach would be changing callers to check
`ks->authenticated` before calling, either
`tls_session_update_crypto_params` or `incoming_push_message`.
Signed-off-by: Jeremy Evans <code@jeremyevans.net> Acked-by: Steffan Karger <steffan.karger@foxcrypto.com>
Message-Id: <20200520183404.54822-1-code@jeremyevans.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19914.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 18 May 2020 15:54:27 +0000 (17:54 +0200)]
Refuse server mode on Android
After the commit 042429d3 "build: Remove --disable-server from ./configure"
Android needs another way to ensure that OpenVPN is not run in server mode.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200518155427.17283-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19904.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 7 May 2020 13:25:34 +0000 (15:25 +0200)]
Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2
Change crypto_pem_encode to not put a nul-terminated terminated
string into the buffer. This was useful for printf but should
not be written into the file.
Instead do not assume that the buffer is null terminated and
print only the number of bytes in the buffer. Also fix a
similar case in printing static key where the 0 byte was
never added to the buffer
Patch V2: make pem_encode behave more like other similar functions in
OpenVPN
and do not null terminate.
Patch V3: also make the mbed TLS variant of pem_decode behave like other
similar functions in OpeNVPN and accept a not null-terminated
buffer.
Patch V4: The newly introduced unit test
test_tls_crypt_v2_write_client_key_file_metadata
was added after the V3 version of the patch and now misses the
strlen with memcmp replacment that were added to
test_tls_crypt_v2_write_client_key_file. Also add the
modifictions to this function.
Unconditionally allocate buffer in mbed TLS path as
requested by Steffan.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan.karger@foxcrypto.com>
Message-Id: <20200507132534.6380-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19852.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 13 May 2020 14:11:47 +0000 (16:11 +0200)]
Change client side of t_lpback.sh configs to use inline material.
We have no real test rig for "inline" key material (key, cert, ca,
tls-auth, tls-crypt*) yet. This change adds the "sample" key set
as inline config to the "loopback-client" config, while keeping
file-based configs for "loopback-server" - that way, testing both
methods of loading keys etc. in one go.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200513141147.17171-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19883.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Fri, 21 Feb 2020 03:00:28 +0000 (22:00 -0500)]
Persist management-query-remote and proxy prompts
Currently this prompt is only output once, not re-written to the
management interface when the management client connects. It is thus
not seen by a client that connects after the prompt is output or one that
disconnects and reconnects. This leads to a deadlock: the daemon waiting
for the "remote" command from the client, the latter not aware of it.
Resolve by adding the ">REMOTE" and ">PROXY" prompt to
man.persist.special_state_msg as done for other persisted prompts such
as ">PASSWORD"
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1582254028-7763-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19497.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 12 May 2020 12:43:44 +0000 (14:43 +0200)]
Fix session id and initial timestamp not being preserved
In the initial state of checking whether an auth-token has been
validated, the check check if multi->auth_token is already set and
only then sets the value. This defeats the purpose and lead to always
a new auth-token with new session id and lifetime being generated when
the server restarts or the client reconnect to another server.
Patch V2: Only set multi->auth_token when NULL to avoid leaking
memory. Improve comments and documentation of auth-token.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200512124344.15929-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19878.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 26 Mar 2020 17:23:31 +0000 (18:23 +0100)]
Fix session id in env missing first byte
sizeof for a constant string return the size including the null byte.
For copying the session id this meant that we do not copy the first
byte. This made the session id reported to the external authenticator
one byte shorter than it was intended to be.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200326172332.2356-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19622.html
Now that the whole inline logic has been converted to using bool flags,
the INLINE_FILE_TAG constant is not useful anymore.
Get rid of the constant as it's now unused and to prevent any future
developer from mistakenly use it again.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200508212356.18522-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19863.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
The inline logic was recently changed by commit
("convert *_inline attributes to bool"), however the code testing a
newly created tls-crypt-v2 client key was not adapted.
Adapt tls-crypt-v2 test routine by properly signaling when the passed
key is inlined or not.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200510140017.16837-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19870.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
With commit ("convert *_inline attributes to bool") the logic for
signaling when a certain option is inline has been changed.
Due to an overlook, the auth-gen-token-secret was not converted, thus
making it impossible to be inlined.
Fix parsing logic and allow auth-gen-token-secret to be inlined as well.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200508211434.27545-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19862.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit cb2e9218f2bc73f re-factored the internal file handling, but
somehow overlooked the --tls-crypt-v2 option processing. It was no
longer possible to load a configuration file with this key file inlined.
There where two issues here. First was that the OPT_P_INLINE flag was
not set, so the option parser rejected --tls-crypt-v2 as inline capable.
Second issue was that the 'streq(p[1], INLINE_FILE_TAG)' check makes no
longer sense, as at this point p[1] contains the file contents. Instead
use the is_inline flag.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200508114411.15762-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19859.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
options: Fix failing inline tls-auth/crypt with persist-key
A configuration file using --persist-key and with inlined --tls-auth or
--tls-crypt files was failing in check_file_access(). The file argument
to check_file_access() contained the key file and not the file name.
This was because check_file_access_inline() which calls
check_file_access() if the file is not inlined was told the file was not
an inline file.
The reason the check_file_access_inline() was misled was due to a prior
option_postprocess_mutate() call puts these key files into a connection
block entry in option_postprocess_mutate_ce(). OpenVPN was modified a
long while ago to always use connection blocks in the option structure
for simplicity. So the "root" key files would be transferred into a
connection entry in this method.
When --persist-key is used, option_postprocess_mutate_ce() will load the
key file and "convert" the option into an inline option. But in
commit cb2e9218f2bc73fa2 this logic had lost the "inline indicator". The
result was that the connection entry had the key file content stored in
the object but was "tagged" as a normal file (name) not an inline file.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200508114243.15532-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19858.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Thu, 27 Feb 2020 20:54:43 +0000 (21:54 +0100)]
build: Remove --disable-server from ./configure
After some discussion among the core community developers [1,2], it was
decided to remove the possibility to build openvpn as a pure client.
This was alterted on the mailing list [3] that it was scheduled for
removal unless anyone had strong arguments why it was needed.
The general consensus was that we had not received any strong arguments
to keep this possibility after approximately 5 months, so it was fine to
remove this ./configure option.
By removing this, we remove quite some entangled sections of #ifdef
scattered all over the code base, making it more readable.
One note:
Inside the options_postprocess_mutate_invariant() function,
the #ifdef P2MP_SERVER and #ifdef _WIN32 blocks where slightly
reworked to make the _WIN32 block more continous and avoiding having an
empty if(options->mode == MODE_SERVER) block.
Carrying around the INLINE_TAG is not really efficient,
because it requires a strcmp() to be performed every
time we want to understand if the data is stored inline
or not.
Convert all the *_inline attributes to bool to make the
logic easier and checks more efficient.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200507135909.21227-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19854.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 21 Apr 2020 10:11:22 +0000 (12:11 +0200)]
Fix tls_ctx_client/server_new leaving error on OpenSSL error stack
In the corner case that the global OpenSSL has an invalid command like
MinProtocol = TLSv1.0
(due to OpenSSL's idiosyncrasies MinProtocol = TLSv1 would be correct)
the SSL_ctx_new function leaves the errors for parsing the config file
on the stack.
OpenSSL: error:14187180:SSL routines:ssl_do_config:bad value
Since the later functions, especially the one of loading the
certificates expected a clean error this error got reported at the
wrong place.
Print the warnings with crypto_msg when we detect that we are in this
situation (this also clears the stack).
When invoking openvpn as standalone with the --genkey
argument, options_postprocess() is not called at all
because do_genkey() takes over the execution earlier.
For this reason, checking the --genkey argument in
options_postprocess_filechecks() is a no-op.
Geti rid of the bogus check altogether.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200420102102.20981-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19795.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 16 Apr 2020 11:39:30 +0000 (13:39 +0200)]
Another round of uncrustify code cleanup.
After the last big formatting patch a number of changes have been
commited that do not conform with our style/uncrustify config. This
has lead to the problem that running uncrustify on before sending
PR some of the changes made by uncrustify need to be backed out again.
To bring everything back to the agreed upon style, run uncrustify once
more. Uncrustify version used:
Uncrustify-0.70.1_f
I double checked the result by running uncrustify (Uncrustify-0.69.0_f)
from Ubuntu focal/20.04 which does not do any further changes and
uncrustify 0.66.1_f from Ubuntu bionic/18.04
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200416113930.15192-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19750.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 16 Apr 2020 11:39:29 +0000 (13:39 +0200)]
Minor style change to improve code style
These are small manual changes that are done to improve the code
style and also make the result of uncrustify better without mixing
manual changes/automatic changes into a single commit.
- Make prototype and function identical for gc_addspecial. Also fixes
uncrustify misparsing the embedded function pointer decleration
- Disallow uncrustify to reformat link_socket_init_phase1, which it
messes up
- Format the the parameters of a call of mbedtls_ssl_tls_prf to
be more inline with the rest of our function calls with multiple
arguments
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200416113930.15192-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19748.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
The tun interface has proved to be a bit fragile for basic netlink tests
as it may introduce delays in switching state, depending on the system
the test is ran on.
For this reason, switch to dummy interface type and at the same type
set its oper-state to up right after creation to avoid hitting the
no-carrier state later. No-carrier has been problematic in pasts tests
as it sometimes persists long enough to create a discrepancy between the
various tests snapshots thus causing a test failure.
Setting a static MAC addressis also re-enabled to avoid it being
different and thus causing a test failure when comparing snapshots.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200416134925.8848-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19751.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
sitnl: fix ignoring EEXIST when sending a netlink command
The logic is to treat EEXIST as non-error because it means that the
address/soute we wanted to install already exists, therefore we can
move on and not fail.
However, this logic is currently based on checking errno == EEXIST.
This is wrong, because sitnl_send() does not set errno, but returns the
error directly as negative value.
Fix this issue by directly comparing the the return value with -EEXIST.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200418094350.26349-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19777.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
The is_tun_p2p() function can return false for both TAP or TUN
interfaces (under certain conditions), therefore we should not
assume any TUN/TAP type when printing related messages.
Remove reference to TUN/TAP when printing messages under conditions
based on is_tun_p2p().
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200418013123.22551-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19775.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
sitnl: fix failure reporting by keeping error negative
The err->errno value reported by netlink is already negative.
Prepending ierr->errno with '-' when forwarding it to
the caller results in a positive value and thus not
detected as error.
Fix error handling in sitnl by not negating the sign of
the value returned by sitnl_send() in case of generic error.
While at it, print the errno in decimal form along with
its string represenation.
Reported-by: Richard Bonhomme <tincanteksup@gmail.com> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200418011849.382-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19773.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Wed, 15 Apr 2020 07:30:17 +0000 (10:30 +0300)]
Fix illegal client float (CVE-2020-11810)
There is a time frame between allocating peer-id and initializing data
channel key (which is performed on receiving push request or on async
push-reply) in which the existing peer-id float checks do not work right.
If a "rogue" data channel packet arrives during that time frame from
another address and with same peer-id, this would cause client to float
to that new address. This is because:
- tls_pre_decrypt() sets packet length to zero if
data channel key has not been initialized, which leads to
- openvpn_decrypt() returns true if packet length is zero,
which leads to
- process_incoming_link_part1() returns true, which
calls multi_process_float(), which commits float
Note that problem doesn't happen when data channel key is initialized,
since in this case openvpn_decrypt() returns false.
The net effect of this behaviour is that the VPN session for the
"victim client" is broken. Since the "attacker client" does not have
suitable keys, it can not inject or steal VPN traffic from the other
session. The time window is small and it can not be used to attack
a specific client's session, unless some other way is found to make it
disconnect and reconnect first.
CVE-2020-11810 has been assigned to acknowledge this risk.
Fix illegal float by adding buffer length check ("is this packet still
considered valid") before calling multi_process_float().
Trac: #1272
CVE: 2020-11810
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200415073017.22839-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19720.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Fri, 13 Mar 2020 16:59:13 +0000 (18:59 +0200)]
Fix broken async push with NCP is used
With NCP and deferred auth, we perform cipher negotiation and generate
data channel keys on incoming push request, assuming that auth succeeded.
With async push, when auth succeeds in between push requests, we send
push reply immediately.
The code which generates data channel keys is only called on handling
incoming push requests (incoming_push_message). It might not be called
with NCP, deferred auth and async push, because on incoming push request,
auth might not be complete yet. When auth is complete in between push
requests, push reply is sent and it is assumed that connection is
established. However, since data channel keys are not generated on the
server side, connection doesn't work.
Fix by adding a call to generate data channel keys when async push is
triggered.
Also, all the "session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized"
checks have been moved into tls_session_update_crypto_params(), which
is just reducing duplicate code, no actual code change (*all* callers
had this pre-check).
Trac: #1259
Reported-by: smaxfield@duosecurity.com Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200313165913.12682-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19553.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 28 Mar 2020 04:08:58 +0000 (05:08 +0100)]
Fix OpenSSL 1.1.1 not using auto elliptic curve selection
Commit 8a01147ff attempted to avoid calling the deprecated/noop
operation SSL_CTX_set_ecdh_auto by surrounding it with #ifdef.
Unfortunately, that change also made the return; that would exit
the function no longer being compiled when using OpenSSL 1.1.0+.
As consequence OpenVPN with OpenSSL 1.1.0+ would always set
secp384r1 as ecdh curve unless otherwise specified by ecdh
This patch restores the correct/previous behaviour. Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200328040858.16505-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19630.html
OpenSSL: Fix --crl-verify not loading multiple CRLs in one file
Lack of this led people accepting multiple CAs to use capath,
which already supports multiple CRLs. But capath mode itself
is somewhat ugly: you have to create new file/symlink every time
CRL is updated, and there's no good way to clean them up without
restarting OpenVPN, since any gap in the sequence would cause it
to lose sync (see trac 623).
mbedtls crypto backend already loads multiple CRLs as is, so
it doesn't need this fix.
The patch also includes some logging changes which I think are useful.
Trac: #623
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200407174436.238933-1-wgh@torlan.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19710.html
Arne Schwabe [Fri, 3 Apr 2020 09:09:44 +0000 (11:09 +0200)]
Fix off-by-one in tls-crypt-v2 client wrapping with custom metadata
Instead of writing at the end of the metadata buffer, the decoded
base64 data overwrites the opcode as BPTR points to the beginning
of the buffer and not the current position. Replace with BEND to
fix this off-by-one
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20200403090944.17726-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19695.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Mon, 30 Mar 2020 18:05:27 +0000 (14:05 -0400)]
When auth-user-pass file has no password query the management interface
(if available).
When only username is found in the file, redirect the auth-user-pass
query to the management interface if management-query-passwords is
enabled. Otherwise the user is prompted on console, if available,
as before.
This changes the behaviour for those who run from the command line,
with --management-query-passwords, but still expect the prompt
on the console.
Note that the management interface will prompt for both username and
password ignoring the username read from the file. As most GUIs can
save the the username, this is a one-time inconvenience.
Currently, the password is queried on the console (or systemd)
in such cases. This is not sensible when console is not available
(windows GUI, tunnelblick etc.) or when the log is redirected
to a file on Windows (for some reason prompt goes to the log file).
Trac # 757
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1585591527-23734-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19655.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 2 Apr 2020 10:38:21 +0000 (12:38 +0200)]
Fix OpenSSL error stack handling of tls_ctx_add_extra_certs
Commit f67efa94 exposed that tls_ctx_add_extra_certs will always leave
an error of PEM_R_NO_START_LINE on the stack that will printed the next
time that the error is printed.
Fix this by discarding this error. Also clean up the logic to report
real error on other errors and also the no start line error if no
certificate can be found at all and it is required (--extra-certs
config option)
Patch V2: fix optional flag was flipped betwen --cert and --extra-certs
Patch V3: Make logic more easy to follow, no functional changes
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200402103821.10347-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19685.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Sat, 14 Mar 2020 12:58:01 +0000 (13:58 +0100)]
tun.c: revise the IPv4 ifconfig flow on Windows
When provisioning IP configuration, we shall not ask what kind of
adapter this is. Rather, we should ask what method of provisioning we
are configured to use.
It is options.c's job to rule out invalid combinations.
- do_ifconfig_ipv4(): unify the workflow with its IPv6 counterpart
No need to distinguish Wintun and TAP-Windows6 here. This also fixes
an issue with --windows-driver wintun overriding --ip-win32 manual,
the later being perfectly fine choice for Wintun too.
- open_tun(), tuntap_post_open(), tuntap_set_ip_addr(): unify Wintun and
TAP-Windows6 workflow. This allows allows --ip-win32 ipapi now.
- close_tun() the cleanup has been revised to match the ifconfig
workflow in reverse.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200314125801.1031-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19560.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 1 Apr 2020 12:40:19 +0000 (14:40 +0200)]
Fetch OpenSSL versions via source/old links
New versions are already available as source/old but old version at
some point disappear from the normal download path. Use the source/old
path for all OpenSSL versions to avoid this problem.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200401124019.10529-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20200401124019.10529-1-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Tom van Leeuwen [Tue, 31 Mar 2020 07:14:37 +0000 (09:14 +0200)]
mbedTLS: Make sure TLS session survives move
When a client disconnects from a server compiled with mbedTLS, the server
cannot process the PUSH_REQUEST from a new connection with the same client
IP and port number. This is the case when the client binds to a static
port.
This behavior is initiated by move_session(), which copies the content of
the
tls_session to a new session and re-initializes the old session once the
new
session is authenticated.
This tls_session contains, among other things, an mbedtls_ssl_config and
bio_ctx structure. However, the mbedtls context has internal pointers to
the
mbedtls_ssl_config and bio_ctx. When the session is moved, these internal
pointers point to the reinitialized session and as a result all received
packets that are stored in the bio_ctx of the moved session can never be
read
by the mbedtls session. The PUSH_REQUEST is therefore never seen by the
server.
Since there is no public method to update these internal pointers, this
patch dynamically allocates the mbedtls_ssl_config and bio_ctx and stores
the pointers to those structures in the tls_session instead.
Trac #880
Signed-off-by: Tom van Leeuwen <tom.van.leeuwen@technolution.eu> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200331071437.12708-1-tom.van.leeuwen@technolution.nl>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19661.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
WGH [Wed, 25 Mar 2020 12:26:24 +0000 (15:26 +0300)]
docs: Add reference to X509_LOOKUP_hash_dir(3)
This is probably the best description of the rather confusing
capath directory structure OpenSSL manual has to offer. Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200325122624.3142017-1-wgh@torlan.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19615.html
Simon Rozman [Tue, 10 Mar 2020 10:40:22 +0000 (11:40 +0100)]
tapctl: Support multiple hardware IDs
TAP-Windows6 adapters created with tapinstall/devcon.exe have hardware
ID "tap0901", where TAP-Windows6 adapters created with tapctl.exe have
hardware ID "root\\tap0901".
The enumeration of the network adapters have been extended to detect
adapters using a list of acceptable hardware IDs.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200310104022.431-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19542.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Mon, 9 Mar 2020 13:17:27 +0000 (14:17 +0100)]
openvpnmsica: Merge FindTUNTAPAdapters into FindSystemInfo
1. We don't need two custom actions to evaluate the system state, do we?
2. FindTUNTAPAdapters was actually broken. It enumerated all existing
network adapters, rather than just the ones we are interested in:
TAP-Windows6 and Wintun.
3. TUNTAPADAPTER and ACTIVETUNTAPADAPTERS were split into
TAPWINDOWS6ADAPTERS, ACTIVETAPWINDOWS6ADAPTERS, WINTUNADAPTERS and
ACTIVEWINTUNADAPTERS to allow finer control.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200309131728.380-11-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19531.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 12 Mar 2020 11:36:54 +0000 (12:36 +0100)]
Normalise ncp-ciphers option and restrict it to 127 bytes
In scenarios of mbed TLS vs OpenSSL we already normalise the ciphers
that are send via the wire protocol via OCC to not have a mismatch
warning between server and client. This is done by
translate_cipher_name_from_openvpn. The same applies also to the
ncp-ciphers list. Specifying non normalised names in ncp-ciphers will
cause negotation not to succeed if ciphers are not in the same form.
Therefore we will normalise the ciphers in options_postmutate.
The alternative and a lot less user friendly alternative would be to
bail if on of the ciphers in ncp-ciphers is not in its normalised form.
Also restrict the ncp-ciphers list to 127. This is somewhat arbitrary
but should prevent too large IV_CIPHER messages and problems sending
those. The server will accept also large IV_CIPHER values from clients.
Patch V2: Correct comment about normalising ciphers
Patch V3: Correct #ifdef statement
Patch V5: Fix tests with OpenSSL 1.0.2 and libraries missing Chacha
Patch V6: Fix unit tests for mbed tls, which recognises ChaCha20-Poly1305
only when used with all uppercase, fix missing space in message
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200312113654.16184-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19546.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Santtu Lakkala [Mon, 21 Oct 2019 11:35:06 +0000 (14:35 +0300)]
Fix OpenSSL private key passphrase notices
Clear error stack on successful certificate loading in
tls_ctx_load_cert_file_and_copy() and handle errors also for
PEM_read_bio_PrivateKey() call in tls_ctx_load_priv_file().
Due to certificate loading possibly leaking non-fatal errors on OpenSSL
error stack, and some slight oversights in error handling, the
>PASSWORD:Verification Failed: 'Private Key'
line was never produced on the management channel for PEM formatted keys.
Ilya Shipitsin [Sun, 22 Mar 2020 12:35:21 +0000 (17:35 +0500)]
travis-ci: add arm64, s390x builds.
as described on https://docs.travis-ci.com/user/multi-cpu-architectures
travis-ci
now supports amd64, ppcle, arm64, s390 architectures. Add arm64 and s390x.
travis-ci images were upgraded to bionic.
"sudo" is deprecated, let us remove it, also "matrix" is deprecated in
favour of "jobs".
LD_LIBRARY_PATH was replaced by using "rpath" in LDFLAGS, which is more
elegant way of linking.
also, dependencies were upgraded to the latest versions.
travis_wait was added for long openssl builds.
cmocka was added to linux and osx builds. Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200322123521.17710-1-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19574.html
Simon Rozman [Mon, 9 Mar 2020 13:17:26 +0000 (14:17 +0100)]
openvpnmsica, tapctl: Revise default hardware ID management
tap_create_adapter() and tap_list_adapter() no longer default to
"root\tap0901". Defining a default hardware ID value is at the
responsibility of upper layers that process user desires.
Since the tap_list_adapter() no longer defaults the hardware ID to
anything, its behavior was simplified to return all existing
adapters when a NULL hardware ID is specified.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200309131728.380-10-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19524.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Mon, 9 Mar 2020 13:17:24 +0000 (14:17 +0100)]
openvpnmsica: "TAP" => "TUN/TAP"
The function and property names that are common to TAP and TUN from
TAP-Windows6 and TUN from Wintun were renamed not to make the now
mainstream TUN sad.
I would have go with just the "adapter". But, wouldn't that cause
confusion when user sees "Deleting adapters" when uninstalling the
OpenVPN?
Internal variable names were simplified thou to omit the TUN/TAP
referencing.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200309131728.380-8-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19526.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Mon, 9 Mar 2020 13:17:23 +0000 (14:17 +0100)]
openvpnmsica, tapctl: "interface" => "adapter"
Interface is not equal to adapter. A quote from Microsoft documentation:
> There is a one-to-one correspondence between the interfaces and
> adapters on a given computer. An interface is an IP-level abstraction,
> whereas an adapter is a datalink-level abstraction.
As tapctl and openvpnmsica are all about managing network adapters on
Windows computers, the terminology has been updated.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200309131728.380-7-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19529.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Tue, 10 Mar 2020 09:48:21 +0000 (10:48 +0100)]
tun.c: reorder IPv6 ifconfig on Windows
The IPv6 interface network route should be setup as soon as possible
after the interface address is set. Actually, all routes should be added
before DNS servers are configured. This would allow Windows to validate
DNS servers properly instead of shutting the validation off.
The cleanup order has been changed to match reverse order of ifconfig.
An additional check was added to skip the cleanup when --ip-win32 is set
to manual.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200310094822.588-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19541.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Thu, 12 Mar 2020 06:08:29 +0000 (08:08 +0200)]
tun.c: fix 'use after free' error
Commit 509c45f has factored out code blocks of open_tun()
into separate functions and introduced "use after free" bug:
Variable "device_guid" is allocated inside tun_open_device()
function and used outside of it. Allocation happens with
local gc_arena, which is freed at the end of tun_open_device(),
making futher access to "device_guid" invalid.
Fix by ensuring that gc_arena scope covers all access to "device_guid".
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Simon Rozman <simon@rozman.si>
Message-Id: <20200312060829.19468-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19547.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Domagoj Pensa [Wed, 5 Feb 2020 12:46:14 +0000 (13:46 +0100)]
Skip DNS address validation
When adding IPv4 DNS servers without interactive service use
"validate=no", on Windows 7 and higher, to skip time consuming automatic
address validation, that is on by default.
Simon Rozman [Wed, 5 Feb 2020 18:38:41 +0000 (19:38 +0100)]
wintun: upgrade error message in case of ring registration failure
Rather than have the Interactive Service return a custom 0x20000004
(ERROR_REGISTER_RING_BUFFERS) error, return the true GetLastError() code
that the TUN_IOCTL_REGISTER_RINGS provides.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200205183841.1118-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19367.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Simon Rozman [Mon, 9 Mar 2020 13:17:17 +0000 (14:17 +0100)]
openvpnmsica: Remove required Windows driver certification detection
The MSI packages are switching to TAP-Windows6 and Wintun MSM modules to
install the TAP/TUN driver. The MSM modules have built-in Windows
version detection already.
This commit is now-dead-code clean up with uncrustification.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200309131728.380-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19530.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 21 Feb 2020 10:07:46 +0000 (11:07 +0100)]
Move NCP related function into a seperate file and add unit tests
This allows unit test the NCP functions. The ssl.c file has too
many dependencies to make unit testing of it viable.
Patch V2: Removing the include "ssl_ncp.h" from options.c for V2 of
implement dynamic NCP forces a new version of this patch to
add the #include in this patch. Merge VS studio file changes
for ssl_ncp.[ch] into this patch
Patch V3: Regenerate for changes in earlier patches, apply Lev's changes
to Visual Studio project file
Patch V4: Regenerate to also have the changes of earlier patches.
Patch V5: Fix unit tests for crypto library missing chacha20-poly1305
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200221100746.7065-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19499.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Thu, 20 Feb 2020 01:56:43 +0000 (20:56 -0500)]
Fix possible access of uninitialized pipe handles
Compile time warning for openvpnserv.exe
interactive.c: In function ‘RunOpenvpn’:
interactive.c:160:27: warning: ‘svc_pipe’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
When RunOpenvpn exits early due to errors, uninitialized svc_pipe and
ovpn_pipe vars could get passed to CloseHandleEx(). Fix by initializing
to NULL.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1582163803-3342-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19480.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Arne Schwabe [Wed, 19 Feb 2020 11:21:53 +0000 (12:21 +0100)]
Warn about insecure ciphers also in init_key_type
With modern Clients and server initialising the crypto cipher later
and not when reading in the config, most users never the warning when
having selected BF-CBC in the configuration.
This patch adds the logic to print out warning to init_key_type.
Main reason for this patch is a personal experience with someone who was
strictly against putting 'cipher' into a config file because he did not
like hardcoding a cipher and "OpenVPN will do AES-GCM anyway" and thinks
that it is better to not have it in configuration even after told by me
that 15 year defaults might not be good anymore.
Patch V2: rebase on master, fix minor style issues
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan.karger@foxcrypto.com>
Message-Id: <20200219112153.13013-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19476.html Signed-off-by: David Sommerseth <davids@openvpn.net>
Documented all the argv related code with minor refactoring
Added doxygen comments for all the functions in argv.c.
There are some slight refactoring, renaming a few variables to make
their use case more obvious and ensure lines do not break our 80-chars
per line coding style limit.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-5-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19377.html
Heiko Hund [Thu, 6 Feb 2020 13:21:02 +0000 (14:21 +0100)]
Add gc_arena to struct argv to save allocations
With the private gc_arena we do not have to allocate the strings
found during parsing again, since we know the arena they are
allocated in is valid as long as the argv vector is.
Signed-off-by: Heiko Hund <heiko.hund@sophos.com> Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-4-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19376.html
Heiko Hund [Thu, 6 Feb 2020 13:21:01 +0000 (14:21 +0100)]
argv: do fewer memory re-allocations
Prevent the re-allocations of memory when the internal argv grows
beyond 2 and 4 arguments by initially allocating argv to hold up to
7 (+ trailing NULL) pointers.
While at it rename argv_reset to argv_free to actually express
what's going on. Redo the argv_reset functionality so that it can
be used to actually reset the argv without re-allocation.
Signed-off-by: Heiko Hund <heiko.hund@sophos.com> Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-3-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19378.html
Heiko Hund [Thu, 6 Feb 2020 13:21:00 +0000 (14:21 +0100)]
re-implement argv_printf_*()
The previous implementation had the problem that it was not fully
compatible with printf() and could only detect % format directives
following a space character (0x20).
It modifies the format string and inserts marks to separate groups
before passing it to the regular printf in libc. The marks are
later used to separate the output string into individual command
line arguments.
The choice of 0x1D as the argument delimiter is based on the
assumption that no "regular" string passed to argv_printf_*() will
ever have to contain that byte (and the fact that it actually is
the ASCII "group separator" control character, which fits its
purpose).
This commit has been updated by David Sommerseth based on Arne
Schwabe and his own feedback on the mailing list.
Signed-off-by: Heiko Hund <heiko.hund@sophos.com> Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-2-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19380.html
Arne Schwabe [Mon, 17 Feb 2020 14:43:37 +0000 (15:43 +0100)]
Implement dynamic NCP negotiation
Our current NCP version is flawed in the way that it can only indicate
support for AES-256-GCM and AES-128-GCM. While configuring client and
server with different ncp-cipher configuration directive works, the
server will blindly push the first cipher of that list to the client
if the client sends IV_NCP=2.
This patches introduces IV_CIPHER sent from the client to the server that
contains the full list of ciphers that the client is willing to support (*).
The server will then pick the first cipher of its own ncp-cipher list that
the client indicates support for.
We choose a textual representation of the ciphers instead of a binary since
a binary would mean that we would need to have a central place to maintain
a mapping between binary and the actual cipher name. Also the normal
ncp-cipher list is quite short, so this should not be problem. It also
provides the freedom to negioate new ciphers from SSL libraries without
the need to upgrade OpenVPN/its binary cipher table.
* the client/server will also accpt the cipher specified in --cipher
but eventually we want to get rid of --ciper. So this patch keeps a
reasonable backwards compatbility (especially poor man's NCP) but does
not encourage to use --cipher for negotiation in documentation or
warning messages.
Patch V2: Remove #include "ssl_ncp.h" Note to compile on windows the patch
"Add strsep compat function" should be applied first
Patch V3: Use string_alloc with gc instead strdup()
Patch V4: Integrate using a short lived gc from patch 006 directly
into this patch
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200217144339.3273-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20200217144339.3273-4-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 17 Feb 2020 14:43:35 +0000 (15:43 +0100)]
Only announce IV_NCP=2 when we are willing to support these ciphers
We currently always announce IV_NCP=2 when we support these ciphers even
when we do not accept them. This lead to a server pushing a AES-GCM-128
cipher to clients and the client then rejecting it.
Patch V2: Remove unecessary restoring of ncp_ciphers
Patch V3: Do not add ncp_ciphers in context
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200217144339.3273-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20200217144339.3273-2-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Wed, 12 Feb 2020 15:06:07 +0000 (10:06 -0500)]
Allow unicode search string in --cryptoapicert option
Currently when the certificate is specified as "SUBJ:foo", the
string foo is assumed to be ascii. Change that and interpret
it as utf-8, convert to a wide string, and flag it as unicode
in CertFindCertifcateInStore().
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1581519967-16950-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19405.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Wed, 12 Feb 2020 15:06:06 +0000 (10:06 -0500)]
Skip expired certificates in Windows certificate store
Have the cryptoapicert option find the first matching certificate
in store that is valid at the present time. Currently the first
found item, even if expired, is returned.
This makes it possible to update certifiates in store without having
to delete old ones. As a side effect, if only expired certificates are
found, the connection fails.
Also remove some unnecessary casts.
Tested on Windows 10.
Trac #966
v4: Handle the case when an unknown certificate specification is passed
to find_certificate_in_store().
Note: Warnings printed from find_certificate_in_store() could show up
multiple times as its called for each certificate store. This could
be improved in a future patch.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1581519967-16950-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19404.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Tue, 21 Jan 2020 08:08:28 +0000 (10:08 +0200)]
configure.ac: simplify AC_CHECK_FUNCS statements
AC_CHECK_FUNCS checks availability of each function
in argument list and defines HAVE_function macro.
AC_CHECK_FUNC takes single function as an argument and
doesn't automatically define any macros.
When we check for availability of a single function and
define own macro, it is enough to use AC_CHECK_FUNC.
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200121080828.1310-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19333.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Mon, 10 Feb 2020 04:33:20 +0000 (23:33 -0500)]
Swap the order of checks for validating interactive service user
Check the config file location and command line options first
and membership in OpenVPNAdministrators group after that as
the latter could be a slow process for active directory users.
When connection to domain controllers is poor or unavailable, checking
the group membership is slow and causes timeouts in the GUI (Trac
1051). However, in cases where the config is in the global directory,
no group membership check should be required. The re-ordering here
avoids the redundant check in such cases.
In addition to this, its also proposed to improve the timeout handling
in the GUI, but this change is still useful as it should completely
eliminate the timeout issue for many users.
v3: Do not send error message to the client pipe from ValidateOptions().
Instead save the error and send it on only if user authorization also
fails. The error buffer size is increased to 512 wide chars as these
messages could get long in some cases and may get truncated otherwise.
Also see: https://github.com/OpenVPN/openvpn-gui/issues/332
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1581309200-27870-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19388.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Domagoj Pensa [Wed, 5 Feb 2020 12:46:15 +0000 (13:46 +0100)]
Fix linking issues on MinGW
MinGW linking fails for several files if compiled without "-O2" due to
a missing "static" declaration for inline functions tuntap_is_wintun()
and tuntap_ring_empty().
Signed-off-by: Domagoj Pensa <domagoj@pensa.hr> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200205124615.15758-3-domagoj@pensa.hr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19356.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Mon, 20 Jan 2020 11:55:18 +0000 (12:55 +0100)]
Move keying material exporter check from syshead.h to configure.ac
Commit ab27c9f7 added a compile-time check for availablitity of
keying-material-export functionality to syshead.h. It turns out that
openvpnserv also includes syshead.h, and has ENABLE_CRYPTO_* defined in
it's config.h, but doesn't have the necessary CFLAGS / LIBS to actually
compile and link against the crypto libraries. That of course breaks
openvpnserv builds.
To fix this, change the compile-time check in syshead.h into a
configure-time check in configure.ac. That's more consistent with how we
do other feature checks anyway.
Signed-off-by: Steffan Karger <steffan.karger@foxcrypto.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <E1itVts-0007ZG-NO@sfs-ml-2.v29.lw.sourceforge.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19328.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
projects / thirdparty / openvpn.git / log
