Hi Steffan,
Patch attached.
-Lev
2014-10-23 10:52 GMT+03:00 Steffan Karger <steffan.kar...@fox-it.com>:
> Hi Lev,
>
> On 10/21/2014 09:33 AM, Lev Stipakov wrote:
>>
>> Thanks for your comments. I have fixed (1) and (2) - well, reusing
>> existing code in (2) has fixed also (1).
>
> Thanks! Do you have the patch somewhere for us to look at?
>
>> Regarding (3) - I don't have much experience in crypto thing, so it
>> would be nice if someone suggests if we should use another alignment.
>
> >From later discussions I get the feeling the alignment was not crypto
> related.
>
> Also, alignment for crypto is very platform-specific. e.g. AES-NI
> instructions require 128-bit alignment for maximum speed. Though it
> depends on the crypto mode and implementation whether the data we supply
> it is directly used or not.
>
> The main accelerated crypto mode currently is AES-GCM, and that does not
> run the data through the crypto (but rather encrypts counters, and then
> XORs the result with the data). So I wouldn't worry too much about that
> for now.
>
> -Steffan
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
--
-Lev
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c4c356d..2c827e2 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -220,6 +220,30 @@ err:
return;
}
+int verify_hmac(struct buffer *buf, struct key_ctx *ctx, int offset)
+{
+ uint8_t local_hmac[MAX_HMAC_KEY_LENGTH]; /* HMAC of ciphertext computed locally */
+ int hmac_len = 0;
+
+ 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)
+ return 0;
+
+ hmac_ctx_update (ctx->hmac, BPTR (buf) + hmac_len + offset,
+ BLEN (buf) - hmac_len - offset);
+ hmac_ctx_final (ctx->hmac, local_hmac);
+
+ /* Compare locally computed HMAC with packet HMAC */
+ if (memcmp_constant_time (local_hmac, BPTR (buf) + offset, hmac_len) == 0)
+ return hmac_len;
+
+ return 0;
+}
+
/*
* If (opt->flags & CO_USE_IV) is not NULL, we will read an IV from the packet.
*
@@ -246,30 +270,13 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
/* 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 < hmac_len)
- CRYPT_ERROR ("missing authentication info");
-
- hmac_ctx_update (ctx->hmac, BPTR (buf) + hmac_len, BLEN (buf) - hmac_len);
- hmac_ctx_final (ctx->hmac, local_hmac);
-
- /* Compare locally computed HMAC with packet HMAC */
- if (memcmp_constant_time (local_hmac, BPTR (buf), hmac_len))
+ int hmac_len = verify_hmac(buf, ctx, 0);
+ if (hmac_len == 0)
CRYPT_ERROR ("packet HMAC authentication failed");
-
ASSERT (buf_advance (buf, hmac_len));
}
/* Decrypt packet ID + payload */
-
if (ctx->cipher)
{
const unsigned int mode = cipher_ctx_mode (ctx->cipher);
@@ -389,6 +396,28 @@ 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)
+{
+ if (buf->len > 0 && opt->key_ctx_bi)
+ {
+ struct key_ctx *ctx = &opt->key_ctx_bi->decrypt;
+
+ /* Verify the HMAC */
+ if (ctx->hmac)
+ {
+ /* sizeof(uint32_t) comes from session_id (3 bytes) and opcode (1 byte) */
+ return verify_hmac(buf, ctx, sizeof(uint32_t)) != 0;
+ }
+ }
+ 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/init.c b/src/openvpn/init.c
index 3b72b96..fef7c1d 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1748,7 +1748,8 @@ pull_permission_mask (const struct context *c)
| OPT_P_MESSAGES
| OPT_P_EXPLICIT_NOTIFY
| OPT_P_ECHO
- | OPT_P_PULL_MODE;
+ | OPT_P_PULL_MODE
+ | OPT_P_SESSION_ID;
if (!c->options.route_nopull)
flags |= (OPT_P_ROUTE | OPT_P_IPWIN32);
@@ -1825,6 +1826,13 @@ do_deferred_options (struct context *c, const unsigned int found)
msg (D_PUSH, "OPTIONS IMPORT: --ip-win32 and/or --dhcp-option options modified");
if (found & OPT_P_SETENV)
msg (D_PUSH, "OPTIONS IMPORT: environment modified");
+
+ if (found & OPT_P_SESSION_ID)
+ {
+ msg (D_PUSH, "OPTIONS IMPORT: session-id set");
+ c->c2.tls_multi->use_session_id = true;
+ c->c2.tls_multi->vpn_session_id = c->options.vpn_session_id;
+ }
}
/*
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 3468dab..7a6911c 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 = ntohl((*(uint32_t*)ptr)) & 0xFFFFFF;
+ 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..ef64a1e 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,9 +3923,9 @@ 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);
+ add_option (options, p, file, line_num, 0, msglevel, permission_mask, option_types_found, es);
+ }
}
- }
return true;
}
@@ -6948,6 +6949,12 @@ add_option (struct options *options,
options->persist_mode = 1;
}
#endif
+ else if (streq (p[0], "session-id"))
+ {
+ VERIFY_PERMISSION (OPT_P_SESSION_ID);
+ options->use_session_id = true;
+ options->vpn_session_id = atoi(p[1]);
+ }
else
{
int i;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 27bbc14..6907dad 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -597,6 +597,9 @@ struct options
bool show_net_up;
int route_method;
#endif
+
+ bool use_session_id;
+ uint32_t vpn_session_id;
};
#define streq(x, y) (!strcmp((x), (y)))
@@ -632,6 +635,7 @@ struct options
#define OPT_P_SOCKBUF (1<<25)
#define OPT_P_SOCKFLAGS (1<<26)
#define OPT_P_CONNECTION (1<<27)
+#define OPT_P_SESSION_ID (1<<28)
#define OPT_P_DEFAULT (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
@@ -716,11 +720,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..15f5172 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -304,6 +304,19 @@ send_push_reply (struct context *c)
if (multi_push)
buf_printf (&buf, ",push-continuation 1");
+ /* Send session-id if client supports it */
+ if (c->c2.tls_multi->peer_info)
+ {
+ const char* proto_str = strstr(c->c2.tls_multi->peer_info, "IV_PROTO=");
+ if (proto_str)
+ {
+ int proto = 0;
+ int r = sscanf(proto_str, "IV_PROTO=%d", &proto);
+ if ((r == 1) && (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 +476,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..7b357de 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,12 @@ 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));
+ if (op == P_DATA_V2) {
+ buf_advance (buf, 3);
+ }
+
++ks->n_packets;
ks->n_bytes += buf->len;
dmsg (D_TLS_KEYSELECT,
@@ -3296,6 +3310,7 @@ tls_pre_decrypt_lite (const struct tls_auth_standalone *tas,
return ret;
error:
+
tls_clear_error();
gc_free (&gc);
return ret;
@@ -3364,14 +3379,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 = htonl(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24 | (multi->vpn_session_id & 0xFFFFFF));
+ 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 +3469,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 +3528,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..10c21fe 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..3288adf 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;
+ bool use_session_id;
+
/*
* Our session objects.
*/
