security/openvpn: cherry-pick fixes from git repo

* 098edbb1 2020-05-20 | Switch assertion failure to returning false [Jeremy Evans]
* fc029714 2020-05-30 | pool: prevent IPv6 pools to be larger than 2^16 addresses [Antonio Quartulli]
* 38b46e6b 2020-02-20 | Persist management-query-remote and proxy prompts [Selva Nair]

MFH:		2020Q2 (blanket approval for stability fixes)
This commit is contained in:
Matthias Andree 2020-05-31 08:40:02 +00:00
parent 698d1150d5
commit 20ed7ce24d
Notes: svn2git 2021-03-31 03:12:20 +00:00
svn path=/head/; revision=537129
4 changed files with 226 additions and 1 deletions

View File

@ -3,7 +3,7 @@
PORTNAME= openvpn
DISTVERSION= 2.4.9
PORTREVISION?= 1
PORTREVISION?= 2
CATEGORIES= security net net-vpn
MASTER_SITES= https://swupdate.openvpn.org/community/releases/ \
https://build.openvpn.net/downloads/releases/ \

View File

@ -0,0 +1,136 @@
From 098edbb1f5a2e1360fd6a4ae0642b63bec12e992 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Wed, 20 May 2020 11:34:04 -0700
Subject: [PATCH] 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
```
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)
---
src/openvpn/ssl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index cf6689982..215147f37 100644
--- ./src/openvpn/ssl.c
+++ ./src/openvpn/ssl.c
@@ -1941,7 +1941,10 @@ tls_session_generate_data_channel_keys(struct tls_session *session)
const struct session_id *server_sid = !session->opt->server ?
&ks->session_id_remote : &session->session_id;
- ASSERT(ks->authenticated);
+ if (!ks->authenticated) {
+ msg(D_TLS_ERRORS, "TLS Error: key_state not authenticated");
+ goto cleanup;
+ }
ks->crypto_options.flags = session->opt->crypto_flags;
if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi,

View File

@ -0,0 +1,61 @@
From 38b46e6bf65489c2c5d75da1c02a3a1c33e6da88 Mon Sep 17 00:00:00 2001
From: Selva Nair <selva.nair@gmail.com>
Date: Thu, 20 Feb 2020 22:00:28 -0500
Subject: [PATCH] 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>
(cherry picked from commit 93ba6ccddafcc87f336f50dadde144ea4f6178ad)
---
src/openvpn/init.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8bac74f97..e153682ed 100644
--- ./src/openvpn/init.c
+++ ./src/openvpn/init.c
@@ -269,6 +269,7 @@ ce_management_query_proxy(struct context *c)
buf_printf(&out, ">PROXY:%u,%s,%s", (l ? l->current : 0) + 1,
(proto_is_udp(ce->proto) ? "UDP" : "TCP"), np(ce->remote));
management_notify_generic(management, BSTR(&out));
+ management->persist.special_state_msg = BSTR(&out);
}
ce->flags |= CE_MAN_QUERY_PROXY;
while (ce->flags & CE_MAN_QUERY_PROXY)
@@ -280,6 +281,7 @@ ce_management_query_proxy(struct context *c)
break;
}
}
+ management->persist.special_state_msg = NULL;
gc_free(&gc);
}
@@ -349,6 +351,7 @@ ce_management_query_remote(struct context *c)
buf_printf(&out, ">REMOTE:%s,%s,%s", np(ce->remote), ce->remote_port,
proto2ascii(ce->proto, ce->af, false));
management_notify_generic(management, BSTR(&out));
+ management->persist.special_state_msg = BSTR(&out);
ce->flags &= ~(CE_MAN_QUERY_REMOTE_MASK << CE_MAN_QUERY_REMOTE_SHIFT);
ce->flags |= (CE_MAN_QUERY_REMOTE_QUERY << CE_MAN_QUERY_REMOTE_SHIFT);
@@ -362,6 +365,7 @@ ce_management_query_remote(struct context *c)
break;
}
}
+ management->persist.special_state_msg = NULL;
}
gc_free(&gc);

View File

@ -0,0 +1,28 @@
From fc0297143494e0a0f08564d90dbb210669d0abf5 Mon Sep 17 00:00:00 2001
From: Antonio Quartulli <a@unstable.cc>
Date: Sat, 30 May 2020 02:05:54 +0200
Subject: [PATCH] pool: prevent IPv6 pools to be larger than 2^16 addresses
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200530000600.1680-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19945.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 81d66a1f14d4be3282dd648ecc2049658e3a65ed)
---
src/openvpn/pool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index da28bc06b..e45bf88a2 100644
--- ./src/openvpn/pool.c
+++ ./src/openvpn/pool.c
@@ -183,7 +183,7 @@ ifconfig_pool_init(int type, in_addr_t start, in_addr_t end,
if (pool->ipv6)
{
pool->base_ipv6 = ipv6_base;
- pool->size_ipv6 = ipv6_netbits>96 ? ( 1<<(128-ipv6_netbits) )
+ pool->size_ipv6 = ipv6_netbits > 112 ? (1 << (128 - ipv6_netbits))
: IFCONFIG_POOL_MAX;
msg( D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: (IPv4) size=%d, size_ipv6=%d, netbits=%d, base_ipv6=%s",