Skip to contentSkip to conversation

Fix for OpenVPN MTU Tunnel computation bug for TLS reconnects

Openvpn-devel In review Dirkjan Bussink 6 participants 8 messages May 5, 2017 – May 9, 2017

lore
Share & tools
lore
#0 lore raw

When testing upgrading OpenVPN from 2.3 to 2.4, we ran into an issue where every time the same client reconnects the MTU for the tunnel used shrinks, eventually leading to packets being dropped by OpenVPN.

Reproduced the problem with a 2 node setup, one as the server and the other one as a client. Each time the client is restarted the packet sizes shrink until it breaks.

Configuration for the server:

/etc/openvpn/server.conf

port 1194
proto udp
dev tun

ca /etc/openvpn/easy-rsa/keys/ca.crt
cert /etc/openvpn/easy-rsa/keys/server.crt
key /etc/openvpn/easy-rsa/keys/server.key
dh none

server 192.168.1.0 255.255.255.0
topology subnet
client-config-dir /etc/openvpn/ccd

client-to-client
keepalive 10 120

user nobody
group nogroup
persist-key
persist-tun
status /var/run/openvpn-status.log
log /var/log/openvpn/openvpn.log
verb 5

Configuration for the client:

/etc/openvpn/client.conf

client
dev tun
proto udp
float
remote 172.28.128.4 1194
user nobody
group nogroup

persist-key
persist-tun

ca /etc/openvpn/easy-rsa/keys/172.28.128.5-ca.crt
cert /etc/openvpn/easy-rsa/keys/172.28.128.5.crt
key /etc/openvpn/easy-rsa/keys/172.28.128.5.key
log /var/log/openvpn/openvpn.log

ns-cert-type server
verb 5

What happens during the first time the client connects from the perspective of the logs is the following (server log):

May 05 10:50:59 test-primary systemd[1]: Started OpenVPN connection to server.
May 05 10:50:59 test-primary ovpn-server[17464]: Failed to extract curve from certificate (UNDEF), using secp384r1 instead.
May 05 10:50:59 test-primary ovpn-server[17464]: ECDH curve secp384r1 added
May 05 10:50:59 test-primary ovpn-server[17464]: TLS-Auth MTU parms [ L:1621 D:1212 EF:38 EB:0 ET:0 EL:3 ]
May 05 10:50:59 test-primary ovpn-server[17464]: TUN/TAP device tun0 opened
May 05 10:50:59 test-primary ovpn-server[17464]: TUN/TAP TX queue length set to 100
May 05 10:50:59 test-primary ovpn-server[17464]: do_ifconfig, tt->did_ifconfig_ipv6_setup=0
May 05 10:50:59 test-primary ovpn-server[17464]: /sbin/ip link set dev tun0 up mtu 1500
May 05 10:50:59 test-primary ovpn-server[17464]: /sbin/ip addr add dev tun0 192.168.1.1/24 broadcast 192.168.1.255
May 05 10:50:59 test-primary ovpn-server[17464]: Data Channel MTU parms [ L:1621 D:1450 EF:121 EB:406 ET:0 EL:3 ]
May 05 10:50:59 test-primary ovpn-server[17464]: Could not determine IPv4/IPv6 protocol. Using AF_INET
May 05 10:50:59 test-primary ovpn-server[17464]: Socket Buffers: R=[212992->212992] S=[212992->212992]
May 05 10:50:59 test-primary ovpn-server[17464]: UDPv4 link local (bound): [AF_INET][undef]:1194
May 05 10:50:59 test-primary ovpn-server[17464]: UDPv4 link remote: [AF_UNSPEC]
May 05 10:50:59 test-primary ovpn-server[17464]: GID set to nogroup
May 05 10:50:59 test-primary ovpn-server[17464]: UID set to nobody
May 05 10:50:59 test-primary ovpn-server[17464]: MULTI: multi_init called, r=256 v=256
May 05 10:50:59 test-primary ovpn-server[17464]: IFCONFIG POOL: base=192.168.1.2 size=252, ipv6=0
May 05 10:50:59 test-primary ovpn-server[17464]: Initialization Sequence Completed
May 05 10:52:27 test-primary ovpn-server[17464]: MULTI: multi_create_instance called
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 Re-using SSL/TLS context
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 Control Channel MTU parms [ L:1621 D:1212 EF:38 EB:0 ET:0 EL:3 ]
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 Data Channel MTU parms [ L:1621 D:1450 EF:121 EB:406 ET:0 EL:3 ]
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 Local Options String (VER=V4): 'V4,dev-type tun,link-mtu 1541,tun-mtu 1500,proto UDPv4,cipher BF-CBC,auth SHA1,keysize 128,key-method 2,tls-server'
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 Expected Remote Options String (VER=V4): 'V4,dev-type tun,link-mtu 1541,tun-mtu 1500,proto UDPv4,cipher BF-CBC,auth SHA1,keysize 128,key-method 2,tls-client'
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 TLS: Initial packet from [AF_INET]172.28.128.5:1194, sid=59ea927a 09799019
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 VERIFY OK: depth=1, C=US, ST=CA, L=San Francisco, O=GitHub, OU=Enterprise, CN=GitHub Enterprise VPN, name=GitHub Enterprise VPN, [email protected]...
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 VERIFY OK: depth=0, C=US, ST=CA, L=San Francisco, O=GitHub, OU=Enterprise, CN=172.28.128.5, name=GitHub Enterprise VPN, [email protected]...
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_VER=2.4.0
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_PLAT=linux
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_PROTO=2
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_NCP=2
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_LZ4=1
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_LZ4v2=1
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_LZO=1
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_COMP_STUB=1
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_COMP_STUBv2=1
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 peer info: IV_TCPNL=1
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 Control Channel: TLSv1.2, cipher TLSv1/SSLv3 ECDHE-RSA-AES256-GCM-SHA384, 2048 bit RSA
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5:1194 [172.28.128.5] Peer Connection Initiated with [AF_INET]172.28.128.5:1194
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 OPTIONS IMPORT: reading client specific options from: /etc/openvpn/ccd/172.28.128.5
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 MULTI: Learn: 192.168.1.2 -> 172.28.128.5/172.28.128.5:1194
May 05 10:52:27 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 MULTI: primary virtual IP for 172.28.128.5/172.28.128.5:1194: 192.168.1.2
May 05 10:52:28 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 PUSH: Received control message: 'PUSH_REQUEST'
May 05 10:52:28 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 SENT CONTROL [172.28.128.5]: 'PUSH_REPLY,route-gateway 192.168.1.1,topology subnet,ping 10,ping-restart 120,ifconfig 192.168.1.2 255.255.255.0,peer-id 0,cipher AES-256-GCM' (status=1)
May 05 10:52:28 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel MTU parms [ L:1549 D:1450 EF:49 EB:406 ET:0 EL:3 ]
May 05 10:52:28 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel Encrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
May 05 10:52:28 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel Decrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
May 05 10:52:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 MULTI: bad source address from client [172.28.128.5], packet dropped

What can be seen here is that the client initially connects and sets up the TLS control channel. The interesting MTU related bits are the first occurrence of "Data Channel MTU parms [ L:1621 D:1450 EF:121 EB:406 ET:0 EL:3 ]" from the initial setup which comes from
https://github.com/OpenVPN/openvpn/blob/7a1b6a0dd706a81897457b0456a951c0b30bbcfb/src/openvpn/init.c#L4055.

There's another entry for it with
"Data Channel MTU parms [ L:1549 D:1450 EF:49 EB:406 ET:0 EL:3 ]" that comes from
https://github.com/OpenVPN/openvpn/blob/f3705dd1e711ee9f8546b841e4b18e9e9a224975/src/openvpn/ssl.c#L1986.
So there's an adjustment that happens twice, once for for the client initialization and once for the TLS connection setup.

When a client reconnects, the following is shown in the logs:

May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 TLS: new session incoming connection from [AF_INET]172.28.128.5:1194
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 VERIFY OK: depth=1, C=US, ST=CA, L=San Francisco, O=GitHub, OU=Enterprise, CN=GitHub Enterprise VPN, name=GitHub Enterprise VPN, [email protected]...
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 VERIFY OK: depth=0, C=US, ST=CA, L=San Francisco, O=GitHub, OU=Enterprise, CN=172.28.128.5, name=GitHub Enterprise VPN, [email protected]...
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_VER=2.4.0
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_PLAT=linux
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_PROTO=2
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_NCP=2
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_LZ4=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_LZ4v2=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_LZO=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_COMP_STUB=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_COMP_STUBv2=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 peer info: IV_TCPNL=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 TLS: move_session: dest=TM_ACTIVE src=TM_UNTRUSTED reinit_src=1
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 TLS: tls_multi_process: untrusted session promoted to semi-trusted
May 05 10:57:29 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Control Channel: TLSv1.2, cipher TLSv1/SSLv3 ECDHE-RSA-AES256-GCM-SHA384, 2048 bit RSA
May 05 10:57:30 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 PUSH: Received control message: 'PUSH_REQUEST'
May 05 10:57:30 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 SENT CONTROL [172.28.128.5]: 'PUSH_REPLY,route-gateway 192.168.1.1,topology subnet,ping 10,ping-restart 120,ifconfig 192.168.1.2 255.255.255.0,peer-id 0,cipher AES-256-GCM' (status=1)
May 05 10:57:30 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel MTU parms [ L:1477 D:1450 EF:-23 EB:406 ET:0 EL:3 ]
May 05 10:57:30 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel Encrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
May 05 10:57:30 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel Decrypt: Cipher 'AES-256-GCM' initialized with 256 bit key

What can be seen here is that there's no client initialization since the connection setup is reused. This can also be seen from the
"TLS: tls_multi_process: untrusted session promoted to semi-trusted" bit in the logs. This shows that we're reusing the TLS connection. What still happens though is another MTU data channel change in
"Data Channel MTU parms [ L:1477 D:1450 EF:-23 EB:406 ET:0 EL:3 ]" happens. This means the channel is shrinking. This happens on each restart of a client:

May 05 11:00:06 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel MTU parms [ L:1405 D:1405 EF:-95 EB:406 ET:0 EL:3 ]

And after another restart:

May 05 11:00:39 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 Data Channel MTU parms [ L:1333 D:1333 EF:-167 EB:406 ET:0 EL:3 ]

After a few restarts, OpenVPN packets start getting dropped with the following error message:

May 05 11:00:41 test-primary ovpn-server[17464]: 172.28.128.5/172.28.128.5:1194 TCP/UDP packet too large on write to [AF_INET]172.28.128.5:1194 (tried=1419,max=1333)

This means not all traffic through the tunnel works anymore. This then leads to strange hard to debug issues where only bigger packets get dropped but smaller ones work. This issue is also reported in https://community.openvpn.net/openvpn/ticket/879.

I've attached a patch and also opened a pull request on GitHub at
https://github.com/OpenVPN/openvpn/pull/87

with the patch also attached to this email. I have no idea if this is the right approach or if there's another way to know if the connection is reused or not and if the frame calculation should be skipped or not.

Signature · 3 lines
Cheers,

Dirkjan

From 8142cb430c3a740ab1ef822532cee70019a2239d Mon Sep 17 00:00:00 2001
From: Dirkjan Bussink <[email protected]...>
Date: Fri, 5 May 2017 13:17:40 +0200
Subject: [PATCH] Don't resize the MTU windows on a reused connection

This was causing MTU packet sizes to be shrunk each time a TLS connection to a client was reused. Reuse happens easily when a client cleanly restarts.

---
 src/openvpn/crypto.h |  5 +++++
 src/openvpn/ssl.c    | 18 +++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)
src/openvpn/crypto.h +5 -0
index afd6fe51..410259f8 100644
@@ -257,6 +257,11 @@ struct crypto_options
 #define CO_MUTE_REPLAY_WARNINGS (1<<2)
     /**< Bit-flag indicating not to display
      *   replay warnings. */
+#define CO_TLS_CONNECTION_REUSE (1<<4)
+    /**< Bit-flag indicating if the TLS connection
+     *   is reused. This is needed so that framing
+     *   is not done multiple times on a reused
+     *   connection. */
     unsigned int flags;         /**< Bit-flags determining behavior of
                                  *   security operation functions. */
 };
src/openvpn/ssl.c +11 -7
index fca1e7c5..7ed928f3 100644
@@ -1976,13 +1976,17 @@ tls_session_update_crypto_params(struct tls_session *session,
         session->opt->crypto_flags |= CO_PACKET_ID_LONG_FORM;
     }
 
-    /* Update frame parameters: undo worst-case overhead, add actual overhead */
-    frame_add_to_extra_frame(frame, -(crypto_max_overhead()));
-    crypto_adjust_frame_parameters(frame, &session->opt->key_type,
-                                   options->replay, packet_id_long_form);
-    frame_finalize(frame, options->ce.link_mtu_defined, options->ce.link_mtu,
-                   options->ce.tun_mtu_defined, options->ce.tun_mtu);
-    frame_init_mssfix(frame, options);
+    if (!BOOL_CAST(session->opt->crypto_flags & CO_TLS_CONNECTION_REUSE))
+    {
+        /* Update frame parameters: undo worst-case overhead, add actual overhead */
+        frame_add_to_extra_frame(frame, -(crypto_max_overhead()));
+        crypto_adjust_frame_parameters(frame, &session->opt->key_type,
+                                       options->replay, packet_id_long_form);
+        frame_finalize(frame, options->ce.link_mtu_defined, options->ce.link_mtu,
+                       options->ce.tun_mtu_defined, options->ce.tun_mtu);
+        frame_init_mssfix(frame, options);
+        session->opt->crypto_flags |= CO_TLS_CONNECTION_REUSE;
+    }
     frame_print(frame, D_MTU_INFO, "Data Channel MTU parms");
 
     return tls_session_generate_data_channel_keys(session);
Signature · 1 line
2.12.2
#1 lore raw

Am 05.05.17 um 13:38 schrieb Dirkjan Bussink:

Quoted 9 lines

When testing upgrading OpenVPN from 2.3 to 2.4, we ran into an issue where every time the same client reconnects the MTU for the tunnel used shrinks, eventually leading to packets being dropped by OpenVPN.

Reproduced the problem with a 2 node setup, one as the server and the other one as a client. Each time the client is restarted the packet sizes shrink until it breaks.

I can reproduce this problem. Actually I have experienced that problem longer but never looked into enough. On first glance it looks okay and the patch actually also works for me but I don't know the reuse code path good enough to figure what is really going on.

I am actually suprised that this is "TLS resuse" as in my case the client is completely restarted.

Arne

#2 lore raw
Quoted 11 lines · Arne Schwabe

Am 05.05.17 um 13:38 schrieb Dirkjan Bussink:

Quoted 9 lines

When testing upgrading OpenVPN from 2.3 to 2.4, we ran into an issue where every time the same client reconnects the MTU for the tunnel used shrinks, eventually leading to packets being dropped by OpenVPN.

Reproduced the problem with a 2 node setup, one as the server and the other one as a client. Each time the client is restarted the packet sizes shrink until it breaks.

Quoted 4 lines

I can reproduce this problem. Actually I have experienced that problem longer but never looked into enough. On first glance it looks okay and the patch actually also works for me but I don't know the reuse code path good enough to figure what is really going on.

I'm following up on this on trac:
https://community.openvpn.net/openvpn/ticket/879

It seems to me that we need a different fix - see the patch on trac. I'll turn is onto a proper patch and send it to the list.

Quoted 2 lines

I am actually suprised that this is "TLS resuse" as in my case the client is completely restarted.

This is because "nobind" is not used in these client configs, which results in the client using port 1194 for it's outgoing connection. The server then sees a connection from a ip:port it already has an instance state for, and re-uses the instance instead of creating a new one. That is not very common (most people use "nobind" in their client configs), but should of course still work.

-Steffan

#3 lore raw

PATCH 1/1 Restore pre-NCP frame parameters for new sessions

As reported in trac #879, as of the introduction of NCP we always adjust the frame parameters on session negotiations, but do not reset the frame state for a new session on an existing state instance. That caused the frame parameters to be reduced for each reconnect, resulting in smaller and smaller packet size limits until no traffic could go through the tunnel at all. This patch resolves that omission.

Trac: #879

---
 src/openvpn/forward.c | 7 +++++++
 src/openvpn/init.c    | 2 ++
 src/openvpn/openvpn.h | 3 ++-
 src/openvpn/ssl.c     | 9 +--------
 src/openvpn/ssl.h     | 8 ++++++++
 5 files changed, 20 insertions(+), 9 deletions(-)
src/openvpn/forward.c +7 -0
index 8102e94..2f3f3c5 100644
@@ -866,9 +866,16 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
              * will load crypto_options with the correct encryption key
              * and return false.
              */
+            uint8_t opcode = *BPTR(&c->c2.buf) >> P_OPCODE_SHIFT;
             if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf, &co,
                                 floated, &ad_start))
             {
+                /* Restore pre-NCP frame parameters */
+                if (is_hard_reset(opcode, c->options.key_method))
+                {
+                    c->c2.frame = c->c2.frame_initial;
+                }
+
                 interval_action(&c->c2.tmp_int);
 
                 /* reset packet received timer if TLS packet */
src/openvpn/init.c +2 -0
index ee14f67..3c22d67 100644
@@ -4051,6 +4051,8 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f
         c->c2.did_open_tun = do_open_tun(c);
     }
 
+    c->c2.frame_initial = c->c2.frame;
+
     /* print MTU info */
     do_print_data_channel_mtu_parms(c);
 
src/openvpn/openvpn.h +2 -1
index 893296e..f8682d1 100644
@@ -263,7 +263,8 @@ struct context_2
     struct link_socket_actual from;             /* address of incoming datagram */
 
     /* MTU frame parameters */
-    struct frame frame;
+    struct frame frame;                         /* Active frame parameters */
+    struct frame frame_initial;                 /* Restored on new session */
 
 #ifdef ENABLE_FRAGMENT
     /* Object to handle advanced MTU negotiation and datagram fragmentation */
src/openvpn/ssl.c +1 -8
index fca1e7c..b1f0f6b 100644
@@ -832,14 +832,7 @@ print_key_id(struct tls_multi *multi, struct gc_arena *gc)
     return BSTR(&out);
 }
 
-/*
- * Given a key_method, return true if op
- * represents the required form of hard_reset.
- *
- * If key_method = 0, return true if any
- * form of hard reset is used.
- */
-static bool
+bool
 is_hard_reset(int op, int key_method)
 {
     if (!key_method || key_method == 1)
src/openvpn/ssl.h +8 -0
index ed1344e..03688ca 100644
@@ -591,6 +591,14 @@ void show_tls_performance_stats(void);
 /*#define EXTRACT_X509_FIELD_TEST*/
 void extract_x509_field_test(void);
 
+/**
+ * Given a key_method, return true if opcode represents the required form of
+ * hard_reset.
+ *
+ * If key_method == 0, return true if any form of hard reset is used.
+ */
+bool is_hard_reset(int op, int key_method);
+
 #endif /* ENABLE_CRYPTO */
 
 #endif /* ifndef OPENVPN_SSL_H */
Signature · 1 line
2.7.4
#4 lore raw

Re: [Openvpn-devel] [PATCH] Restore pre-NCP frame parameters for new sessions

Quoted 6 lines · Steffan Karger

As reported in trac #879, as of the introduction of NCP we always adjust the frame parameters on session negotiations, but do not reset the frame state for a new session on an existing state instance. That caused the frame parameters to be reduced for each reconnect, resulting in smaller and smaller packet size limits until no traffic could go through the tunnel at all. This patch resolves that omission.

Only glared at the code changes, and it makes sense to me. I'll try to run a few tests to see if I can reproduce this issue. If others can help test this patch, that will also help tremendously - and I'll try to remember to add a Tested-by: tag in the commit log too for all those confirming this fix.

A little nit-pick below though.

Quoted 15 lines

Trac: #879

---
 src/openvpn/forward.c | 7 +++++++
 src/openvpn/init.c    | 2 ++
 src/openvpn/openvpn.h | 3 ++-
 src/openvpn/ssl.c     | 9 +--------
 src/openvpn/ssl.h     | 8 ++++++++
 5 files changed, 20 insertions(+), 9 deletions(-)
src/openvpn/forward.c +0 -0
index 8102e94..2f3f3c5 100644

src/openvpn/forward.c process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo · line 872

Quoted 7 lines
src/openvpn/forward.c +1 -0
@@ -866,9 +866,16 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
              * will load crypto_options with the correct encryption key
              * and return false.
              */
+            uint8_t opcode = *BPTR(&c->c2.buf) >> P_OPCODE_SHIFT;
             if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf, &co,
                                 floated, &ad_start))
             {

Wouldn't it make more sense to move the opcode declaration to inside the if (tls_pre_decrypt(...)) block? I can do that on-the-fly though.

Signature · 4 lines
kind regards,

David Sommerseth
OpenVPN Technologies, Inc
#5 lore raw

Re: [Openvpn-devel] [PATCH] Restore pre-NCP frame parameters for new sessions

Hi,

Quoted 9 lines · David Sommerseth
Quoted 6 lines · Steffan Karger

As reported in trac #879, as of the introduction of NCP we always adjust the frame parameters on session negotiations, but do not reset the frame state for a new session on an existing state instance. That caused the frame parameters to be reduced for each reconnect, resulting in smaller and smaller packet size limits until no traffic could go through the tunnel at all. This patch resolves that omission.

Quoted 5 lines

Only glared at the code changes, and it makes sense to me. I'll try to run a few tests to see if I can reproduce this issue. If others can help test this patch, that will also help tremendously - and I'll try to remember to add a Tested-by: tag in the commit log too for all those confirming this fix.

The two reporters on trac confirm this fix works for them.

Quoted 13 lines

A little nit-pick below though.

Quoted 10 lines

Trac: #879

---
 src/openvpn/forward.c | 7 +++++++
 src/openvpn/init.c    | 2 ++
 src/openvpn/openvpn.h | 3 ++-
 src/openvpn/ssl.c     | 9 +--------
 src/openvpn/ssl.h     | 8 ++++++++
 5 files changed, 20 insertions(+), 9 deletions(-)
Quoted 16 lines

src/openvpn/forward.c process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo · line 872

Quoted 12 lines
src/openvpn/forward.c +1 -0
index 8102e94..2f3f3c5 100644
@@ -866,9 +866,16 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
              * will load crypto_options with the correct encryption key
              * and return false.
              */
+            uint8_t opcode = *BPTR(&c->c2.buf) >> P_OPCODE_SHIFT;
             if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf, &co,
                                 floated, &ad_start))
             {

Wouldn't it make more sense to move the opcode declaration to inside the if (tls_pre_decrypt(...)) block? I can do that on-the-fly though.

That won't work, because tls_pre_decrypt() alters buf.

-Steffan

#6 lore raw

Re: [Openvpn-devel] [PATCH] Restore pre-NCP frame parameters for new sessions

In reply to David Sommerseth

Am 09.05.17 um 12:46 schrieb David Sommerseth:

Quoted 9 lines · Steffan Karger
Quoted 6 lines

As reported in trac #879, as of the introduction of NCP we always adjust the frame parameters on session negotiations, but do not reset the frame state for a new session on an existing state instance. That caused the frame parameters to be reduced for each reconnect, resulting in smaller and smaller packet size limits until no traffic could go through the tunnel at all. This patch resolves that omission.

Quoted 7 lines

Only glared at the code changes, and it makes sense to me. I'll try to run a few tests to see if I can reproduce this issue. If others can help test this patch, that will also help tremendously - and I'll try to remember to add a Tested-by: tag in the commit log too for all those confirming this fix.

For me this patch also fixes the issue. And the patch looks correct, so ACK from me.

Arne

#7 lore raw

Re: [Openvpn-devel] [PATCH applied] Restore pre-NCP frame parameters for new sessions

In reply to Steffan Karger

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

As mentioned, code looks sane + Arne and some users on the Trac ticket have tested this as well. So ACK from me as well.

Your patch has been applied to the following branches

commit 9900e023bcc49964d33e6f22c2b6223f8932acf8 (master) commit 03d01f4f69cfc6768343b9f0f2dde2049e4882d2 (release/2.4)

Author: Steffan Karger
Date:   Mon May 8 23:44:38 2017 +0200
     Restore pre-NCP frame parameters for new sessions

     Trac: #879
     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14589.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJZEcHCAAoJEIbPlEyWcf3yrZoP/05MaZIrD1NCzhfUkYmL8t18 eUGCBJUkDjr9tg+OhPhf5e/axf4GKzwBcDFZTiqdtXj+bntz/WXJp+ad4HAlwXmE zfOKrwtIh8oqjXeyf9J9cRQJlLACGnzf/mfJet1uMssoTi4yoK8MspSbk8kdA3Nn hEMINxKb+h/byZHjifNT25ZLB1x3gqzVrcvnN2afYQTGgt4BAEQBiVDK8fATh5Si tqW7utXBYTOUepdm/uN9/2bpWLmG/HGzZR8AebsLRGb/xZhZKIaVP7+a3RUzPWw+ ljFvoVJlo8rcj91jWyQY/4nS2ULlFJ4kbKlwPaey0eNlPqtz+XO5uynBWi6AhNEN +e5Xurs8D93pasfNzFAmi0pa7n92D42VLq1NR1qSfhf958/9rEaEnTd4W456C8Kh iWd6NpbosQI0KhJLjnLCymMdD6WcH3yqvIxhJazMdmJICzCeIvPH7/rgtoUdmwPD Gp5t6GtkCI9kKus9soWY0W2hQhIRXpO6IeKncRzjvjIhOSoThxMWelVbchtShOhd 0MQkxOZmYkjj5dXvVm6KZT+Y3CQxxOgqLvtAwfxn/35Uw9oayu1qfiUGJqZpt6wF 24FxyllopXQXK2qe35rZUDN1mNNkusO6q7iv78z2FbD7n9r/9LJBsuxDngvN/3ok 9fOG7apJJLSD0w+8J55v
=dhej
-----END PGP SIGNATURE-----