Re: [Openvpn-devel] session-id implementation

[André Valentin]

Hi!

Great stuff, can't wait to use it!

André

On 27.03.2014 09:57, Lev Stipakov wrote:
> Hi,
>
> Same patch with added NULL check in push.c:308. Turns out that
> peer_info might be NULL.
>
> -Lev
>
> On Wed, Mar 26, 2014 at 10:52 AM, Lev Stipakov  wrote:
>> 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
>
>
>
>
> --
>
>
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] session-id implementation

[Lev Stipakov]

Hi,

Same patch with added NULL check in push.c:308. Turns out that
peer_info might be NULL.

-Lev

On Wed, Mar 26, 2014 at 10:52 AM, Lev Stipakov  wrote:
> 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



-- 
-Lev
From 0f330408e4b013cd505ad9492888557daa47e905 Mon Sep 17 00:00:00 2001
From: Lev Stipakov 
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 ();
+  int offset = 4; /* 1 byte opcode + 3 bytes session-id */
+
+  if (buf->len > 0 && opt->key_ctx_bi)
+{
+  struct key_ctx *ctx = >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 ();
+	  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 ();
+	  return false;
+	}
+
+	  gc_free ();
+	  return true;
+	}
+}
+
+  gc_free ();
+  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