Hello,
Despite that implementation of session-id has already been discussed,
I would like to propose an alternative approach.
I suggest to use an array of multi_instance objects and session-id as
an item's index in that array. Why I think it is a good idea?
1) We could utilize less bytes for session-id, since array's max
length would be equal to max_clients. 3 bytes for a max of 2**24
clients, no need to swap one byte to the end of packet.
[op][s1][s2][s3][data]
2) Lookup in array is faster than lookup by 8-byte session-id in
hashtable and happens with constant speed.
3) Proof of concept is attached!
I have tested it with openvpn master branch (server) and ics-openvpn
(android) - merged without any conflicts and works nicely. Of course
it supports also peers without session-id support.
This patch solves 2 issues for mobile clients:
1) UDP NAT timeout. After, say, 30, seconds router might close UDP
socket and new packet from client to VPN server originates from
different port. Lookup by peer address fails and packet got dropped.
Currently this is solved by small keep-alive interval, which drains
battery. With new implementation it is not an issue anymore since
lookup is done by session-id, therefore keep-alive interval can be
increased.
2) Transition between networks (for example WiFi->3G). Without
session-id client got disconnected. With this patch and
http://code.google.com/p/ics-openvpn/source/detail?r=bc9f3f448b9a4d95c999d3e582987840ba1e0fbf
transition happens seamlessly.
I would love to hear any critics / comments!
--
-Lev
From 284e473548a49012baf6c954a637161eec11c2e8 Mon Sep 17 00:00:00 2001
From: Lev Stipakov <lev.stipa...@f-secure.com>
Date: Tue, 11 Mar 2014 17:58:31 +0200
Subject: [PATCH] Floating implementation. Use array lookup for new opcode
P_DATA_V2 and check HMAC for floated peers.
---
src/openvpn/crypto.c | 54 ++++++++++++++++++++++++
src/openvpn/crypto.h | 3 ++
src/openvpn/mudp.c | 106 ++++++++++++++++++++++++++++++++++++++++++++---
src/openvpn/multi.c | 14 ++++++-
src/openvpn/multi.h | 2 +
src/openvpn/options.c | 15 ++++++-
src/openvpn/options.h | 4 +-
src/openvpn/push.c | 8 +++-
src/openvpn/ssl.c | 59 +++++++++++++++++++++++---
src/openvpn/ssl.h | 9 +++-
src/openvpn/ssl_common.h | 4 ++
11 files changed, 259 insertions(+), 19 deletions(-)
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c4c356d..8c0d33f 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -389,6 +389,60 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
}
/*
+ * This verifies if a packet and its HMAC fit to a crypto context.
+ *
+ * On success true is returned.
+ */
+bool
+crypto_test_hmac (struct buffer *buf, const struct crypto_options *opt)
+{
+ struct gc_arena gc;
+ gc_init (&gc);
+ int offset = 4; /* 1 byte opcode + 3 bytes session-id */
+
+ if (buf->len > 0 && opt->key_ctx_bi)
+ {
+ struct key_ctx *ctx = &opt->key_ctx_bi->decrypt;
+
+ /* Verify the HMAC */
+ if (ctx->hmac)
+ {
+ int hmac_len;
+ uint8_t local_hmac[MAX_HMAC_KEY_LENGTH]; /* HMAC of ciphertext computed locally */
+
+ hmac_ctx_reset(ctx->hmac);
+
+ /* Assume the length of the input HMAC */
+ hmac_len = hmac_ctx_size (ctx->hmac);
+
+ /* Authentication fails if insufficient data in packet for HMAC */
+ if ((buf->len - offset) < hmac_len)
+ {
+ gc_free (&gc);
+ return false;
+ }
+
+ hmac_ctx_update (ctx->hmac, BPTR (buf) + offset + hmac_len,
+ BLEN (buf) - offset - hmac_len);
+ hmac_ctx_final (ctx->hmac, local_hmac);
+
+ /* Compare locally computed HMAC with packet HMAC */
+ if (memcmp (local_hmac, BPTR (buf) + offset, hmac_len))
+ {
+ gc_free (&gc);
+ return false;
+ }
+
+ gc_free (&gc);
+ return true;
+ }
+ }
+
+ gc_free (&gc);
+ return false;
+}
+
+/*
* How many bytes will we add to frame buffer for a given
* set of crypto options?
*/
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 3b4b88e..68cdf16 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -279,6 +279,9 @@ bool openvpn_decrypt (struct buffer *buf, struct buffer work,
const struct crypto_options *opt,
const struct frame* frame);
+
+bool crypto_test_hmac (struct buffer *buf, const struct crypto_options *opt);
+
/** @} name Functions for performing security operations on data channel packets */
void crypto_adjust_frame_parameters(struct frame *frame,
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 3468dab..f7ab625 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -38,6 +38,55 @@
#include "memdbg.h"
/*
+ * Update instance with new peer address
+ */
+void
+update_floated(struct multi_context *m, struct multi_instance *mi,
+ struct mroute_addr real, uint32_t hv)
+{
+ struct mroute_addr real_old;
+
+ real_old = mi->real;
+ generate_prefix (mi);
+
+ /* remove before modifying mi->real, since it also modifies key in hash */
+ hash_remove(m->hash, &real_old);
+ hash_remove(m->iter, &real_old);
+
+ /* update address */
+ memcpy(&mi->real, &real, sizeof(real));
+
+ mi->context.c2.from = m->top.c2.from;
+ mi->context.c2.to_link_addr = &mi->context.c2.from;
+
+ /* switch to new log prefix */
+ generate_prefix (mi);
+ /* inherit buffers */
+ mi->context.c2.buffers = m->top.c2.buffers;
+
+ /* inherit parent link_socket and link_socket_info */
+ mi->context.c2.link_socket = m->top.c2.link_socket;
+ mi->context.c2.link_socket_info->lsa->actual = m->top.c2.from;
+
+ /* fix remote_addr in tls structure */
+ tls_update_remote_addr (mi->context.c2.tls_multi, &mi->context.c2.from);
+ mi->did_open_context = true;
+
+ hash_add(m->hash, &mi->real, mi, false);
+ hash_add(m->iter, &mi->real, mi, false);
+
+ mi->did_real_hash = true;
+#ifdef MANAGEMENT_DEF_AUTH
+ hash_remove (m->cid_hash, &mi->context.c2.mda_context.cid);
+ hash_add (m->cid_hash, &mi->context.c2.mda_context.cid, mi, false);
+#endif
+
+#ifdef MANAGEMENT_DEF_AUTH
+ mi->did_cid_hash = true;
+#endif
+}
+
+/*
* Get a client instance based on real address. If
* the instance doesn't exist, create it while
* maintaining real address hash table atomicity.
@@ -56,15 +105,47 @@ multi_get_create_instance_udp (struct multi_context *m)
struct hash_element *he;
const uint32_t hv = hash_value (hash, &real);
struct hash_bucket *bucket = hash_bucket (hash, hv);
-
- he = hash_lookup_fast (hash, bucket, &real, hv);
+ uint8_t* ptr = BPTR(&m->top.c2.buf);
+ uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
+ uint32_t sess_id;
+ bool session_forged = false;
- if (he)
+ if (op == P_DATA_V2)
{
- mi = (struct multi_instance *) he->value;
+ sess_id = (*(uint32_t*)ptr) >> 8;
+ if ((sess_id < m->max_clients) && (m->instances[sess_id]))
+ {
+ mi = m->instances[sess_id];
+
+ if (!link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from))
+ {
+ msg(D_MULTI_MEDIUM, "floating detected from %s to %s",
+ print_link_socket_actual (&mi->context.c2.from, &gc), print_link_socket_actual (&m->top.c2.from, &gc));
+
+ /* session-id is not trusted, so check hmac */
+ session_forged = !(crypto_test_hmac(&m->top.c2.buf, &mi->context.c2.crypto_options));
+ if (session_forged)
+ {
+ mi = NULL;
+ msg (D_MULTI_MEDIUM, "hmac verification failed, session forge detected!");
+ }
+ else
+ {
+ update_floated(m, mi, real, hv);
+ }
+ }
+ }
}
else
{
+ he = hash_lookup_fast (hash, bucket, &real, hv);
+ if (he)
+ {
+ mi = (struct multi_instance *) he->value;
+ }
+ }
+ if (!mi && !session_forged)
+ {
if (!m->top.c2.tls_auth_standalone
|| tls_pre_decrypt_lite (m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
{
@@ -75,6 +156,17 @@ multi_get_create_instance_udp (struct multi_context *m)
{
hash_add_fast (hash, bucket, &mi->real, hv, mi);
mi->did_real_hash = true;
+
+ int i;
+ for (i = 0; i < m->max_clients; ++ i)
+ {
+ if (!m->instances[i])
+ {
+ mi->context.c2.tls_multi->vpn_session_id = i;
+ m->instances[i] = mi;
+ break;
+ }
+ }
}
}
else
@@ -89,15 +181,17 @@ multi_get_create_instance_udp (struct multi_context *m)
#ifdef ENABLE_DEBUG
if (check_debug_level (D_MULTI_DEBUG))
{
- const char *status;
+ const char *status = mi ? "[ok]" : "[failed]";
+ /*
if (he && mi)
status = "[succeeded]";
else if (!he && mi)
status = "[created]";
else
status = "[failed]";
-
+ */
+
dmsg (D_MULTI_DEBUG, "GET INST BY REAL: %s %s",
mroute_addr_print (&real, &gc),
status);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 2839b30..618ade5 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -302,6 +302,7 @@ multi_init (struct multi_context *m, struct context *t, bool tcp_mode, int threa
cid_compare_function);
#endif
+
/*
* This is our scheduler, for time-based wakeup
* events.
@@ -372,6 +373,13 @@ multi_init (struct multi_context *m, struct context *t, bool tcp_mode, int threa
*/
m->max_clients = t->options.max_clients;
+ int i;
+ m->instances = malloc(sizeof(struct multi_instance*) * m->max_clients);
+ for (i = 0; i < m->max_clients; ++ i)
+ {
+ m->instances[i] = NULL;
+ }
+
/*
* Initialize multi-socket TCP I/O wait object
*/
@@ -552,6 +560,8 @@ multi_close_instance (struct multi_context *m,
}
#endif
+ m->instances[mi->context.c2.tls_multi->vpn_session_id] = NULL;
+
schedule_remove_entry (m->schedule, (struct schedule_entry *) mi);
ifconfig_pool_release (m->ifconfig_pool, mi->vaddr_handle, false);
@@ -628,6 +638,8 @@ multi_uninit (struct multi_context *m)
#endif
m->hash = NULL;
+ free(m->instances);
+
schedule_free (m->schedule);
mbuf_free (m->mbuf);
ifconfig_pool_free (m->ifconfig_pool);
@@ -651,8 +663,6 @@ multi_create_instance (struct multi_context *m, const struct mroute_addr *real)
perf_push (PERF_MULTI_CREATE_INSTANCE);
- msg (D_MULTI_MEDIUM, "MULTI: multi_create_instance called");
-
ALLOC_OBJ_CLEAR (mi, struct multi_instance);
mi->gc = gc_new ();
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index fc2ffb2..0446fbf 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -125,6 +125,8 @@ struct multi_context {
# define MC_WORK_THREAD (MC_MULTI_THREADED_WORKER|MC_MULTI_THREADED_SCHEDULER)
int thread_mode;
+ struct multi_instance** instances;
+
struct hash *hash; /**< VPN tunnel instances indexed by real
* address of the remote peer. */
struct hash *vhash; /**< VPN tunnel instances indexed by
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index ef6170c..8e44e06 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3908,7 +3908,8 @@ apply_push_options (struct options *options,
struct buffer *buf,
unsigned int permission_mask,
unsigned int *option_types_found,
- struct env_set *es)
+ struct env_set *es,
+ struct tls_multi *tls_multi)
{
char line[OPTION_PARM_SIZE];
int line_num = 0;
@@ -3922,7 +3923,17 @@ apply_push_options (struct options *options,
++line_num;
if (parse_line (line, p, SIZE (p), file, line_num, msglevel, &options->gc))
{
- add_option (options, p, file, line_num, 0, msglevel, permission_mask, option_types_found, es);
+ if (streq(p[0], "session_id"))
+ {
+ /* Server supports P_DATA_V2 */
+ tls_multi->vpn_session_id = atoi(p[1]);
+ tls_multi->use_session_id = true;
+ msg(D_PUSH, "session id: %d", tls_multi->vpn_session_id);
+ }
+ else
+ {
+ add_option (options, p, file, line_num, 0, msglevel, permission_mask, option_types_found, es);
+ }
}
}
return true;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 27bbc14..295a0c8 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -716,11 +716,13 @@ void options_postprocess (struct options *options);
void pre_pull_save (struct options *o);
void pre_pull_restore (struct options *o, struct gc_arena *gc);
+struct tls_multi;
bool apply_push_options (struct options *options,
struct buffer *buf,
unsigned int permission_mask,
unsigned int *option_types_found,
- struct env_set *es);
+ struct env_set *es,
+ struct tls_multi* tls_multi);
void options_detach (struct options *o);
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 606bb05..67c36e3 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -304,6 +304,11 @@ send_push_reply (struct context *c)
if (multi_push)
buf_printf (&buf, ",push-continuation 1");
+ /* Send session_id if client supports it */
+ if (strstr(c->c2.tls_multi->peer_info, "IV_PROTO=2")) {
+ buf_printf(&buf, ",session_id %d", c->c2.tls_multi->vpn_session_id);
+ }
+
if (BLEN (&buf) > sizeof(cmd)-1)
{
const bool status = send_control_channel_string (c, BSTR (&buf), D_PUSH);
@@ -463,7 +468,8 @@ process_incoming_push_msg (struct context *c,
&buf,
permission_mask,
option_types_found,
- c->c2.es))
+ c->c2.es,
+ c->c2.tls_multi))
switch (c->options.push_continuation)
{
case 0:
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index c61701a..84f2b16 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -611,6 +611,8 @@ packet_opcode_name (int op)
return "P_ACK_V1";
case P_DATA_V1:
return "P_DATA_V1";
+ case P_DATA_V2:
+ return "P_DATA_V2";
default:
return "P_???";
}
@@ -1037,6 +1039,9 @@ tls_multi_init (struct tls_options *tls_options)
ret->key_scan[1] = &ret->session[TM_ACTIVE].key[KS_LAME_DUCK];
ret->key_scan[2] = &ret->session[TM_LAME_DUCK].key[KS_LAME_DUCK];
+ /* By default not use P_DATA_V2 */
+ ret->use_session_id = false;
+
return ret;
}
@@ -1810,6 +1815,9 @@ push_peer_info(struct buffer *buf, struct tls_session *session)
buf_printf (&out, "IV_PLAT=win\n");
#endif
+ /* support for P_DATA_V2 */
+ buf_printf(&out, "IV_PROTO=2\n");
+
/* push compression status */
#ifdef USE_COMP
comp_generate_peer_info_string(&session->opt->comp_options, &out);
@@ -2766,8 +2774,9 @@ tls_pre_decrypt (struct tls_multi *multi,
key_id = c & P_KEY_ID_MASK;
}
- if (op == P_DATA_V1)
- { /* data channel packet */
+ if ((op == P_DATA_V1) || (op == P_DATA_V2))
+ {
+ /* data channel packet */
for (i = 0; i < KEY_SCAN_SIZE; ++i)
{
struct key_state *ks = multi->key_scan[i];
@@ -2799,7 +2808,9 @@ tls_pre_decrypt (struct tls_multi *multi,
opt->pid_persist = NULL;
opt->flags &= multi->opt.crypto_flags_and;
opt->flags |= multi->opt.crypto_flags_or;
- ASSERT (buf_advance (buf, 1));
+
+ ASSERT (buf_advance (buf, op == P_DATA_V1 ? 1 : 4));
+
++ks->n_packets;
ks->n_bytes += buf->len;
dmsg (D_TLS_KEYSELECT,
@@ -3296,6 +3307,7 @@ tls_pre_decrypt_lite (const struct tls_auth_standalone *tas,
return ret;
error:
+
tls_clear_error();
gc_free (&gc);
return ret;
@@ -3364,14 +3376,24 @@ tls_post_encrypt (struct tls_multi *multi, struct buffer *buf)
{
struct key_state *ks;
uint8_t *op;
+ uint32_t sess;
ks = multi->save_ks;
multi->save_ks = NULL;
if (buf->len > 0)
{
ASSERT (ks);
- ASSERT (op = buf_prepend (buf, 1));
- *op = (P_DATA_V1 << P_OPCODE_SHIFT) | ks->key_id;
+
+ if (!multi->opt.server && multi->use_session_id)
+ {
+ sess = ((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) | (multi->vpn_session_id << 8);
+ ASSERT (buf_write_prepend (buf, &sess, 4));
+ }
+ else
+ {
+ ASSERT (op = buf_prepend (buf, 1));
+ *op = (P_DATA_V1 << P_OPCODE_SHIFT) | ks->key_id;
+ }
++ks->n_packets;
ks->n_bytes += buf->len;
}
@@ -3444,6 +3466,31 @@ tls_rec_payload (struct tls_multi *multi,
return ret;
}
+/* Update the remote_addr, needed if a client floats. */
+void
+tls_update_remote_addr (struct tls_multi *multi,
+const struct link_socket_actual *from)
+{
+ struct gc_arena gc = gc_new ();
+ int i;
+
+ for (i = 0; i < KEY_SCAN_SIZE; ++i)
+ {
+ struct key_state *ks = multi->key_scan[i];
+ if (DECRYPT_KEY_ENABLED (multi, ks) && ks->authenticated && link_socket_actual_defined(&ks->remote_addr))
+ {
+ if (link_socket_actual_match (from, &ks->remote_addr))
+ continue;
+ dmsg (D_TLS_KEYSELECT,
+ "TLS: tls_update_remote_addr from IP=%s to IP=%s",
+ print_link_socket_actual (&ks->remote_addr, &gc),
+ print_link_socket_actual (from, &gc));
+ memcpy(&ks->remote_addr, from, sizeof(*from));
+ }
+ }
+ gc_free (&gc);
+}
+
/*
* Dump a human-readable rendition of an openvpn packet
* into a garbage collectable string which is returned.
@@ -3478,7 +3525,7 @@ protocol_dump (struct buffer *buffer, unsigned int flags, struct gc_arena *gc)
key_id = c & P_KEY_ID_MASK;
buf_printf (&out, "%s kid=%d", packet_opcode_name (op), key_id);
- if (op == P_DATA_V1)
+ if ((op == P_DATA_V1) || (op == P_DATA_V2))
goto print_data;
/*
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index cd7cae2..e634b73 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -61,6 +61,7 @@
#define P_CONTROL_V1 4 /* control channel packet (usually TLS ciphertext) */
#define P_ACK_V1 5 /* acknowledgement for packets received */
#define P_DATA_V1 6 /* data channel packet */
+#define P_DATA_V2 9 /* data channel packet with session_id */
/* indicates key_method >= 2 */
#define P_CONTROL_HARD_RESET_CLIENT_V2 7 /* initial key from client, forget previous state */
@@ -68,7 +69,7 @@
/* define the range of legal opcodes */
#define P_FIRST_OPCODE 1
-#define P_LAST_OPCODE 8
+#define P_LAST_OPCODE 9
/* Should we aggregate TLS
* acknowledgements, and tack them onto
@@ -431,6 +432,12 @@ bool tls_send_payload (struct tls_multi *multi,
bool tls_rec_payload (struct tls_multi *multi,
struct buffer *buf);
+/*
+ * Update remote address of a tls_multi structure
+ */
+void tls_update_remote_addr (struct tls_multi *multi,
+ const struct link_socket_actual *from);
+
#ifdef MANAGEMENT_DEF_AUTH
static inline char *
tls_get_peer_info(const struct tls_multi *multi)
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 04ba789..2fc72aa 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -495,6 +495,10 @@ struct tls_multi
char *peer_info;
#endif
+ /* For P_DATA_V2 */
+ uint32_t vpn_session_id;
+ int use_session_id;
+
/*
* Our session objects.
*/
--
1.8.3.2
