Message ID | 20200413155650.114-1-lstipakov@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [Openvpn-devel] Fix illegal client float | expand |
Am 13.04.20 um 17:56 schrieb Lev Stipakov: > From: Lev Stipakov <lev@openvpn.net> > > There is a time frame between allocating peer-id and initializing data > channel key, which is performed on receiving push request. > > 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 buffer length to zero if > data channel key has not been initialized, which leads to > > - openvpn_decrypt() returns true if buffer 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. > > Fix illegal float by adding buffer length check before calling > multi_process_float(). > > This should fix Trac #1272. > > Signed-off-by: Lev Stipakov <lev@openvpn.net> > --- > src/openvpn/multi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c > index da0aeeba..58892a87 100644 > --- a/src/openvpn/multi.c > +++ b/src/openvpn/multi.c > @@ -2555,7 +2555,7 @@ 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) > + if (floated && c->c2.buf.len > 0) > { > multi_process_float(m, m->pending); > } > I think the commit is right but should we add a comment here saying that len >0 is required to have valid decrypted packet? Arne
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index da0aeeba..58892a87 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2555,7 +2555,7 @@ 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) + if (floated && c->c2.buf.len > 0) { multi_process_float(m, m->pending); }