After first round of mailing people with more than 10 commits we have
almost all committers have agreed. This put this license in the realm
of having a realistic change to work. Had any of these contributers
disagreed, rewriting all their code might have been not feasible.
The rationale of adding this exception now is to avoid having to
have a second round of agreement for new contributers and ensure
that all new code will include the exemption.
patch v2: add explaination and use exception rather than excemption
patch v3: actually send v3
Change-Id: Ide83f914f383b53ef37ddf628e4da5a78e241bf0
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20230426094931.1168078-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26610.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7b21c69dbe1e1ecfb5bed564417387892b42108a)
This fixes two places were we do not have enough space in the array
of parameters given to parse_line for the final NULL parameter that
signal the end of the parsed argument errors.
Both these cases can lead to a buffer overflow. But both of these
cases require root/admin access to OpenVPN:
- parse_argv, only able to trigger if starting openvpn from the command
line, at this point you cannot gain more privileges than you already
have.
Way to reproduce, compile with ASAN and run:
openvpn --tls-verify a a a a a a a a a a a a a a a
- remove_iroutes_from_push_route_list
This operates on the list of pushed entries that is generated
by the server itself. So trigger this, you need to have control
over config, management interface, a plugin or cdd files.
The parse_argv problem was found by Trial of Bits. I found the
remove_iroutes_from_push_route_list problem by looking for similar
problems.
Reported-By: Trial of Bits (TOB-OVPN-4)
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221215190143.2107896-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25734.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 749beb6d0cb9f8628997bb656ba2f64e31cac377)
The plug-in API in OpenVPN 2.x is not designed for running multiple
deferred authentication processes in parallel. The authentication
results of such configurations are not to be trusted. For now we bail
out when this discovered with an error in the log.
This is a backport of commit 282ddbac54f8d4923844f699 (master), taking
the different man-page format into account. The code change is the same.
CVE: 2022-0547
Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220315155344.37787-3-openvpn@sf.lists.topphemmelig.net>
URL: https://www.mail-archive.com/search?l=mid&q=20220315155344.37787-3-openvpn@sf.lists.topphemmelig.net
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--mtu-disc (on Linux) needs two components to work:
- setsockopt() with IP_MTU_DISCOVER or IPV6_MTU_DISCOVER
- "extended error reporting" (setsockopt(IP_RECVERR) and
then via mtu.c/format_extended_socket_error()) to react on
"packet too big" errors on sendto() / sendmsg()
Some configure.ac reorganization broke detection of <linux/errqueue.h>
and "struct sock_extended_err". Re-add <linux/errqueue.h> to configure.ac,
remove all the other conditionals in syshead.h, and remove the
"struct sock_extended_err" check completely (assumption: if errqueue.h
exists, it contains what we need).
Thus, the "non-helpful" socket error message turns into:
2022-02-22 12:31:42 write UDPv4 [EMSGSIZE Path-MTU=800]: Message too long (fd=3,code=90)
2022-02-22 12:31:42 Note adjusting 'mssfix 1400 mtu' to 'mssfix 800 mtu' according to path MTU discovery
2022-02-22 12:31:42 Note adjusting 'fragment 1400 mtu' to 'fragment 800 mtu' according to path MTU discovery
... while at it, fix extra space in first part of these messages, and
print o->ce.fragment for the "fragment" message...
v2: assume that "if it's linux, and has these two headers, everything
else will be there as well" and get rid of most of the #ifdef checks
Trac: #1452
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220222113832.13383-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23863.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 4225114b96723bdecd68398f7a89765879b31b5d)
(cherry picked from commit 3e0c506e5d9135ef4b08547db8679cc5bd2a7582)
When username-as-common-name is in effect, the common_name
is "CN" from the certificate for auth-user-pass-verify. It gets
changed to "username" after successful authentication. This
changed value gets into the env when client-connect script is
called.
However, "common_name" goes through the cycle of being
"CN", then "username" during every reauth (renegotiation).
As the client-connect script is not called during reneg, the changed
value never gets back into the env. The end result is that the
disconnect script gets "common_name=<CN>" instead of the username.
Unless no reneg steps have happened before disconnect.
(For a more detailed analysis see
https://community.openvpn.net/openvpn/ticket/1434#comment:12)
Fix by adding common_name to env whenever it changes.
Trac: #1434
Very likely applies to #160 as well, but that's too old and
some of the relevant code path has evolved since then.
Same as commit fa5ab2438a in master, except for the context change
due to PF.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20211023000706.25016-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23050.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a2412bf4a6bb6ac7a6f26128d00fe81b0fa4a18e)
If a route structure is passed to add_route() or add_route_ipv6()
without the RT_DEFINED flag set, both functions leak an "argv"
structure allocation.
Add appropriate argv_free() calls.
Backport to 2.4: argv_free() was called argv_reset() back then.
Signed-off-by: David Korczynski <david@adalogics.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210714162533.10098-1-david@adalogics.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22637.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a11bea18b1c93b260352ec505db15be0ec9431ee)
p2p connections with both ends backing off seldom succeed
as their connection attempt durations becomes increasingly
unlikely to overlap when the retry wait time is long.
Avoid this by applying the backoff logic only on TCP clients
or the tls_client side for UDP.
Regression warning: shared secret setups are left out of the
backoff logic.
Trac: #1010, #1384
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210602194739.29488-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22485.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 063d55afeea723fc6df0af29a19df257a8ab6920)
This reverts commit 1bdd09e7e019ac6d8fdc1b009bdec818b9183e76 - it was
not indended to be ever merged to release/2.4, but only as a refactoring
commit intended to go to master.
The master commit is 3a16a8678ded8df8e.
Also change the types to use C99 uint64_t and its printf u64 define.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210421134348.1950392-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22171.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
In the unlikely case that we are compiled with -DDMALLOC *and*
malloc() returns NULL, there is an uncaught memset() which would
crash then. Remove the memset(), as the right the next operation
after check_malloc_return() is a mempcy() which will overwrite
the whole memory block anyway.
Trac: #586
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210402173414.14216-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21981.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e2acfad40c0d79ce7fd431c380d7466d383bcefa)
sample-plugins/defer/simple.c uses OPENVPN_PLUGINv3_STRUCTVER settings
that may not be obvious to a new author. Add a comment to reduce
possible confusion.
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1612163389-16421-1-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21540.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit fdfbd4441c2225dc69431c57d18291e103c466cf)
While not required, adding openvpn_plugin_min_version_required_v1 helps
by making an example for others to copy, and helps to explicitly call
attention to the difference between the API version number and the
struct version number in v3 calls.
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1611778909-20630-2-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21508.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a385a3e8a28f2ce96c7ee0be8940b257765add5a)
This isn't strictly required, but it modernizes the functions used.
This change makes _open the same parameter form as _func (for better
parallelism in function writing) and includes a check for the correct
struct version, as recommended by openvpn-plugin.h
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1611778909-20630-1-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21507.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7d1361c18f38d6301b4d558578c73e74f6597927)
The comments refered to parameters found in openvpn_plugin_func_v2 but not
in v3
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1611531973-443-1-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21481.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 595be121b60f8cee9d4816172a7f9a4987560641)
If we ship something that we consider a form of documentation
"this is how to write an OpenVPN plugin" it should meet our standards
for secure and modern code. This plugin did neither.
- get rid of system() calls, especially those that enabled a
remote-root exploit if this code was used "as is"
- change logging from printf() to OpenVPN's plugin_log()
- this requires changing to openvpn_plugin_open_v3() to get
to the function pointers
- change wacky "background and sleep in the shell call" to the
double-fork/waitpid model we use in plugins/auth-pam
(copy-paste code reuse)
- OpenVPN 2.5 and later react badly to OPENVPN_PLUGIN_FUNC_ERROR
returns to OPENVPN_PLUGIN_ENABLE_PF calls (SIGSEGV crash), so
always return SUCCESS. Only hook ENABLE_PF if that functionality
is actually requested ("setenv test_packet_filter NN").
- change deeply-nested functions auth_user_pass_verify() and
tls_final() to use early-return style
- actually make defered PF setup *work* with recent OpenVPNs
(pre-creating temp files broke this, so unlink() the pre-created
file in the ENABLE_PF hook, and re-create asyncronously later)
- add lots of comments explaining why we do things this way
Security issue reported by "oxr463" on HackerOne.
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210121172536.32500-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21466.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 452e016cba977cb1c109e74977029b9c0de33de2)
<inline> segments neglected to increment the "current line number
in config file" variable (line_num), so after the first <inline>,
errors reported have the wrong line number.
Fix by introducing an extra argument to read_inline_file() function:
"so many lines in the inline block", and changing the return values of
the "check_inline*()" functions to "int", changing this from "false/true"
to "0 = no inline, 1...N = inline with <N> lines".
On calling add_options() this is implicitly converted back to bool.
v2: use int return value, not extra call-by-reference parameter
Trac: #1325
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20201206125711.12071-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21334.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a686f7e29af012783371f401f394ac1e62e5b75f)
This variable was first introduce in earlier attempt to fix the
auth-token problems with auth-nocache before user_password and
auth_token were split into two variables. The idea of the variable it
is being set if --pull is in use. However the variable was not always
set correctly, especially if username/password are queried after an
expired auth-token. Instead using that variable use session->opt->pull
directly.
Patch V2: rename delayed_auth_pass_purge to ssl_clean_user_pass to give
a more fitting name since this function is not only used in
the delayed code path and also the new name aligns with
ssl_clean_auth_token. Also fix a leftover wait_for_push
in that function
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201202115928.16615-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21297.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit dfd624b52bce7ddd0eeaab516df9848e432f3242)
This fixes the auth-token not being updated if auth-nocache is set. Our
set_auth_token method ensures that the auth-token always has a username
but is a little bit too strict in the check.
Also add doxygen documentation and remove null checks. We use this function
only with non-null pointers and it makes it a bit nicer to read.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201130123928.21837-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21291.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit fb789947ab1eba3e68fb8e4b3551d095a53962bd)
This improves compatbility to a OpenVPN 2.5 server and
allows to negotiate a different cipher than AES-128/256-GCM
without abusing the poor man's NCP support with --cipher.
We keep the IV_NCP=2 flag logic as broken as it is since 2.5 server
ignore the flag if IV_CIPHERS is set and this might break existing
2.4 setups.
Server support for IV_CIPHERS is not added since it would be quite
intrusive and users should rather upgrade to 2.5 on the server
if they want the full benefits.
This commit cherry picks a few parts of
868b200c3aef6ee5acfdf679770832018ebc7b70
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200830140736.16571-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20844.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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.
Cherry picked from be4531564e2be7c8a0222e6923e3f7580b358cab and adjusted
for 2.4 (methods added to ssl.h/ssl.c instead ssl_ncp.c/.h
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200830140736.16571-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20846.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Reported by "jub0bs" on hackerone.com (#1039504)
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201124161313.18831-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21264.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 0d4069e41d3ba7178be30f78f1174f689dbdfa59)
Commit aa34684972eb0 fixed a long-standing bug in setting the
"route-list" flag RTSA_REMOTE_HOST for IPv4 ("we have a well-defined
remote_host == VPN server IP address") even if connecting over IPv6.
Unfortunately the logic in redirect_default_route_to_vpn() was also
wrong, and refused cooperation if that flag is not set, triggering
the message
"NOTE: unable to redirect IPv4 default gateway -- Cannot
obtain current remote host address"
Correct operation: if RTSA_REMOTE_HOST is not set, or remote_host
is IPV4_INVALID_ADDR (= 255.255.255.255), do not try to install a
host route for continued connectivity to the VPN server - which is
not needed when connecting over IPv6. But the actual *routes*
(/0 or 2 x /1) can be installed just fine.
There is a second bug here, which hits if there is no IPv4 gateway
at all. In that case, the same function triggers the message
"NOTE: unable to redirect IPv4 default gateway -- Cannot
read current default gateway from system"
This is caused by using "IPV4_INVALID_ADDR" as a flag for "do we
know the remote_host?" - which worked before, but after the commit
referenced above, the "remote_host" field is not well-defined unless
RTSA_REMOTE_HOST is set. So, change the condition to check that.
Reported-By: François Kooman <fkooman@tuxed.net>
Reported-By: Thomas Schäfer <tschaefer@t-online.de>
Trac: #1332
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20201002175736.82609-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21152.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 23e11e591347080efa3b933beca7f620dd059d5c)
(cherry picked from commit 7b4f53095c761bde8c6b39cf645cade4c1c0c5d4)
It's a long while since the bundled lz4 library has received an update.
It pulls in a lot of various fixes and enhancements, some of the changes
fixes compiler warnings and hardens the code a bit too.
Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201001154658.9798-1-davids@openvpn.net>
URL: https://www.mail-archive.com/search?l=mid&q=20201001154658.9798-1-davids@openvpn.net
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 0f44a9080530df70410106c244e9efc7f2d8a802)
If remote server has been resolved to multiple addresses, at
least one connection attempt has been made and connection to
the last address was skipped by management - resolved earlier
link socket addrinfo objects will not be cleared neither on
instance close nor in the next connection entry loop.
This causes fatal error assert:
>REMOTE:openvpn.net,1194,udp
remote ACCEPT
SUCCESS: remote command succeeded
>REMOTE:openvpn.net,1194,udp
remote SKIP
SUCCESS: remote command succeeded
>FATAL:Assertion failed at init.c:504
(c->c1.link_socket_addr.current_remote == NULL)
Fix this behaviour by cleaning stale addrinfo objects.
v2: better comment placement and too long length fix
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200916141755.1923-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21019.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 3ad86c2534a92af137809b6d446d570193e6d01f)
When a SOCKS5 server sends back a reply, it encodes an "address",
which can be IPv4 (4 bytes), IPv6 (16 bytes) or "a domain name",
which has a lenght (1 byte) and "a string of length <length>" - so
when copying bytes, we need to hande "length +1" bytes.
Our code totally doesn't use this variant of addresses on reception,
but since this has been pointed out by "tpw_rules" in Trac, fix it,
so if/when someone works on this again, the foundation is correct.
While at it, increase buffer size used for sending to handle domain
names longer than 122 characters (length was already checked, so a
longer name would not overflow but just "not work").
v2: increase buf[] len in recv_socks_reply() from 22 to 270 so it
is large enough to actually copy a domain name
v3: increase buf[] len in establish_socks_proxy_passthru() from 128 to
270, to handle long domain names in queries
Reported-By: tpw_rules in Trac
Trac: #848
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200909122223.9222-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20928.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit eebeaa02367d247fc2549df3edf8e598c58c3572)
Our ROUNDUP() macro to achieve the required system-specific alignment
for data structures sent to the routing socket was wrong for NetBSD -
unlike OpenBSD/FreeBSD, NetBSD is not using "long" (32/64 bit depending
on OS architecture), and not "uint32_t" either (32/32) like MacOS, but
uint64_t.
So our use of "long" always worked on NetBSD/amd64 and stopped working
on NetBSD/i386 when this was changed on the OS side...
NetBSD conveniently exports a RT_ROUNDUP() macro from <net/route.h> - use
that, and avoid trying to second-guess OS requirements.
While at it, add M_ERRNO to ominous "GDG6: problem writing to routing
socket"
error message to differenciate between "EINVAL" and other errors.
Trac: #734
Signed-off-by: Gert Doering <gert@greenie.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200913145621.12125-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20983.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 37aab49b083a9e385970e3ab2dd727ea1a95ff35)
This is basic housekeeping, adding NULL checks to context initialization
of the sample plugin collection which are missing it. Realistically,
this can never happen, but since these are supposed to be "good examples",
not checking calloc() return isn't one.
Trac: #587
Reported-By: Dogbert (in Trac)
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200909104837.6123-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20922.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a61c08a2c80d95dcc2bc30ddcb9a54a462e565ed)
The man page claimed that --client-disconnect "is passed the same
pathname as the corresponding --client-connect command", which is
not what the code does. Fix.
Reported-By: hvenev in Trac
Trac: #884
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200909122926.9523-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20929.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 50c7700da09a1f83474e18f8709d59dbc4b509e2)
If we connect to a VPN server over IPv6, and the config has a
route like this:
route remote_host default net_gateway
OpenVPN would try to install a route to "255.255.255.255", which
is obviously bogus.
The bug is twofold: init_route_list() should not set RTSA_REMOTE_HOST
for an "IPV4_INVALID_ADDR" remote_host (wrong condition, this is not
a pointer but an integer, and "invalid" is "-1" numerically here),
and init_route() must not ignore "status = false" returns from
get_special_addr().
I have just added the "if (!status)" check, not done refactoring for
init_route() to see whether I could make it "more pretty".
Trac: #1247
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200911085907.26004-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20958.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit aa34684972eb01bfa5c355d1c8a8a9d384bf0175)
Our code works on "very old Linux" (Fedora-1), but needs a #define
for TUNSETGROUP to compile. Everything else is there.
While at it, fix TUNSETGROUP error message.
Reported-By: noloader on Trac
Trac: #1152
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200909153725.1158-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20932.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a4e0ac0604460ea2431acb7481d6ffb7a3fc6298)
Calling "openvpn --inetd" from the CLI (= no socket on stdin) will
lead to endless looping in the accept(4) loop.
Instead of cluttering that function further, detect failure to call
getsockame() in phase2_inetd() already, and trigger a M_FATAL abort
on "errno == ENOTSOCK" ("The argument s is a file, not a socket").
While at it, uncrustify the --bind-dev code (whitespace only).
Trac: #350
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200908105130.24171-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20897.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit a09a2fadbadb5dc435f6fccc581163e1f637f43f)
It's a long-standing and well-known problem that --push-reset removes
"critical" options from the push list (like "topology subnet") which
will then lead to non-working client configs. This can not be
reasonably fixed, because the list of "critical" options depends on
overall server config.
So just document the fact, and point people towards --push-remove as
a more selective tool.
Trac: #29
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200908111511.9271-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20899.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 5fd66510dfdef628fa95f156c5f9d80af9ae1531)
Commit 5fde831c5807 fixed NEXTADDR() for all *BSDs and MacOS.
OpenSolaris has to use a slightly different macro due to lack of
sockaddr->sa_len - but it has the same problem, first rounding up,
then memmove()'ing. Switch order.
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200813101301.12720-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20731.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7e65483d1227adfb855844467e4d30894ffc355d)
As it appears commit 767e4c56becbfeea525e4695a810593f373883cd "Log
serial number of revoked certificate" hasn't survive refactoring
of CRL handling.
In most of situations admin of OpenVPN server needs to know which
particular certificate is used by client.
In the case when certificate is valid, environment variable can be
used for that but once it is revoked, no user scripts are invoked
so there is no way to get serial number, only subject is logged.
Let's log certificate serial in case it is revoked and additionally
log certificate depth & subject in crl-verify "dir" mode for better
consistency with crl file (non-dir) mode.
v2: log if serial is not availble, require it in crl-verify dir mode
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200805102333.3109-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20642.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 992e9cec40539a155afa9eae10502aa62f617965)
copy first, then round up the length when adding padding
to the advance.
Found by: GCC 9.3.0 (FreeBSD)
Signed-off-by: Matthias Andree <matthias.andree@gmx.de>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200717171818.230371-1-matthias.andree@gmx.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20461.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 5fde831c580775aa5c1fe3539b06260d994eee10)
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
```
and the following command line options:
```
--config openvpn.conf --dev tun1 --local 206.131.72.52 \
--log-append openvpn.log --status openvpn-status.log \
--server 10.28.47.0 255.255.255.0
```
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>
(cherry picked from commit 984bd1e1601e4b9562dbc88b02a8db60b884286f)
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>
(cherry picked from commit 93ba6ccddafcc87f336f50dadde144ea4f6178ad)
In the auth-pam plugin correctly parse the static challenge string
even when password or challenge response is empty.
Whether an empty user input is an error is determined by the PAM
conversation function depending on whether the PAM module queries
for it or not.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1533696271-21799-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17382.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7a8109023f4c345fe12f23421c5fa7e88e1ea85b)
If static challenge is in use, the password passed to the plugin by openvpn
is of the form "SCRV1:base64-pass:base64-response". Parse this string to
separate it into password and response and use them to respond to queries
in the pam conversation function.
On the plugin parameters line the substitution keyword for the static
challenge response is "OTP". For example, for pam config named "test" that
prompts for "user", "password" and "pin", use
plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1532486093-24793-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17307.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7369d01bf360bcfa02f26c05b86dde5496d120f6)
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).
Debian Bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=958296
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200421101122.24284-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19802.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 75aa88af774abaa168bf72e43e1dbb57be14c044)
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>
(cherry picked from commit 37bc691e7d26ea4eb61a8a434ebd7a9ae76225ab)
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>
(cherry picked from commit 3b06b57d9f1d972ec16f0893d06697439c1bb1fe)
