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
