Hi, On 18 September 2016 at 08:51, Lev Stipakov <lstipa...@gmail.com> wrote: > From: Lev Stipakov <lev.stipa...@f-secure.com> > > v5: > * Few more nickpicks > > v4: > * replace magic number with define > * show user a decimal value instead of hex > > v3: > * move assert outside of loop > * add max-clients value check to options > > v2: > * Add round brackets for clarity > * Rephrase comment > > Support for disabled peer-id > > When peer-id value is 0xFFFFFF, server should ignore it and treat packet > in a same way as P_DATA_V1. > --- > src/openvpn/mudp.c | 13 ++++++++++--- > src/openvpn/multi.c | 3 ++- > src/openvpn/openvpn.h | 3 +++ > src/openvpn/options.c | 5 +++++ > 4 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c > index 21a7e97..fec5e8d 100644 > --- a/src/openvpn/mudp.c > +++ b/src/openvpn/mudp.c > @@ -64,12 +64,16 @@ multi_get_create_instance_udp (struct multi_context *m, > bool *floated) > struct hash_bucket *bucket = hash_bucket (hash, hv); > uint8_t* ptr = BPTR(&m->top.c2.buf); > uint8_t op = ptr[0] >> P_OPCODE_SHIFT; > + bool v2 = (op == P_DATA_V2) && (m->top.c2.buf.len >= (1 + 3)); > + bool peer_id_disabled = false; > > /* make sure buffer has enough length to read opcode (1 byte) and > peer-id (3 bytes) */ > - if (op == P_DATA_V2 && m->top.c2.buf.len >= (1 + 3)) > + if (v2) > { > uint32_t peer_id = ntohl(*(uint32_t*)ptr) & 0xFFFFFF; > - if ((peer_id < m->max_clients) && (m->instances[peer_id])) > + peer_id_disabled = (peer_id == MAX_PEER_ID); > + > + if (!peer_id_disabled && (peer_id < m->max_clients) && > (m->instances[peer_id])) > { > mi = m->instances[peer_id]; > > @@ -84,7 +88,7 @@ multi_get_create_instance_udp (struct multi_context *m, > bool *floated) > } > } > } > - else > + if (!v2 || peer_id_disabled) > { > he = hash_lookup_fast (hash, bucket, &real, hv); > if (he) > @@ -107,6 +111,9 @@ multi_get_create_instance_udp (struct multi_context *m, > bool *floated) > hash_add_fast (hash, bucket, &mi->real, hv, mi); > mi->did_real_hash = true; > > + /* max_clients must be less then max peer-id value */ > + ASSERT(m->max_clients < MAX_PEER_ID); > + > for (i = 0; i < m->max_clients; ++i) > { > if (!m->instances[i]) > diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c > index ba7f2c0..3bc6ee9 100644 > --- a/src/openvpn/multi.c > +++ b/src/openvpn/multi.c > @@ -605,7 +605,8 @@ multi_close_instance (struct multi_context *m, > } > #endif > > - m->instances[mi->context.c2.tls_multi->peer_id] = NULL; > + if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID) > + m->instances[mi->context.c2.tls_multi->peer_id] = NULL; > > schedule_remove_entry (m->schedule, (struct schedule_entry *) mi); > > diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h > index 1a458f1..65a183a 100644 > --- a/src/openvpn/openvpn.h > +++ b/src/openvpn/openvpn.h > @@ -595,4 +595,7 @@ struct context > #define CIPHER_ENABLED(c) (false) > #endif > > +/* this represents "disabled peer-id" */ > +#define MAX_PEER_ID 0xFFFFFF > + > #endif > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index c9688c3..4b7203d 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -5893,6 +5893,11 @@ add_option (struct options *options, > msg (msglevel, "--max-clients must be at least 1"); > goto err; > } > + if (max_clients >= MAX_PEER_ID) /* max peer-id value */ > + { > + msg (msglevel, "--max-clients must be less than %d", MAX_PEER_ID); > + goto err; > + } > options->max_clients = max_clients; > } > else if (streq (p[0], "max-routes-per-client") && p[1] && !p[2]) > -- > 1.9.1
Thanks. No more nitpicking from me ;) ACK. -Steffan ------------------------------------------------------------------------------ _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel