Skip to content

Commit

Permalink
Fix illegal client float (CVE-2020-11810)
Browse files Browse the repository at this point in the history
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 37bc691)
  • Loading branch information
lstipakov authored and cron2 committed Apr 16, 2020
1 parent 9bb285e commit f7b318f
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/openvpn/multi.c
Expand Up @@ -2562,7 +2562,8 @@ multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst
orig_buf = c->c2.buf.data;
if (process_incoming_link_part1(c, lsi, floated))
{
if (floated)
/* nonzero length means that we have a valid, decrypted packed */
if (floated && c->c2.buf.len > 0)
{
multi_process_float(m, m->pending);
}
Expand Down

0 comments on commit f7b318f

Please sign in to comment.