diff mbox series

[Openvpn-devel] Implement fixed MSS value for mssfix and use it for non default MTUs

Message ID 20220224144245.878056-1-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel] Implement fixed MSS value for mssfix and use it for non default MTUs | expand

Commit Message

Arne Schwabe Feb. 24, 2022, 3:42 a.m. UTC 
This allows to set the MSS value inside the tunnel to a user specified
value instead of calculating it form (somewhat) dynamic encapsoluation
overhead.

Also default to the MTU when tun-mtu does not have the default value
to ensure that packets are not larger than the tun-mtu. This only affects
packets that are routed via the VPN and none of the peers is an endpoint
since otherwise the peer would already set a lower MTU.
---
 doc/man-sections/link-options.rst |  9 +++++++-
 src/openvpn/mss.c                 |  8 +++++++
 src/openvpn/options.c             | 37 ++++++++++++++++++++-----------
 src/openvpn/options.h             |  1 +
 4 files changed, 41 insertions(+), 14 deletions(-)

Comments

Lev Stipakov March 4, 2022, 12:57 a.m. UTC | #1 
Hi,

encapsoluation
>

Built and tested on Windows with dco, works as expected - "mssfix 1000
fixed" results in MSS 960 and 1000 bytes in-tunnel TCP packets with 1024
bytes transport UDP packets. Same behavior with "tun-mtu 1000".

Acked-by: Lev Stipakov <lstipakov@gmail.com>
<div dir="ltr"><div dir="ltr">Hi,<div><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">encapsoluation<br></blockquote><div><br></div><div>Built and tested on Windows with dco, works as expected - &quot;mssfix 1000 fixed&quot; results in MSS 960 and 1000 bytes in-tunnel TCP packets with 1024 bytes transport UDP packets. Same behavior with &quot;tun-mtu 1000&quot;. </div><div><br></div><div>Acked-by: Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>&gt;</div></div></div>
Gert Doering March 25, 2022, 7:57 a.m. UTC | #2 
I have tested this on the server and client test rigs ("nothing broke").

Also, explicitly tested this using "--mssfix 500 fixed" and tcpdumped
on the tunnel interface - not caring about outside packet size, this time.

TUN mode:
  IPv4 MSS: 460
    Family: IP (2)
    Total Length: 500
    Transmission Control Protocol, Src Port: 22, Dst Port: 36351, Seq: 5005, Ack: 37, Len: 448
    TCP payload (448 bytes)

  IPv6 MSS: 440
    Family: IPv6 (28)
    Payload Length: 460
    Transmission Control Protocol, Src Port: 22, Dst Port: 27345, Seq: 8518, Ack: 3034, Len: 428
    [TCP Segment Len: 428]
    TCP payload (428 bytes)

  (tshark / tcpdump claim "504 bytes on the wire" in both cases, but
   I assume this is some FreeBSD/tun overhead artefact... maybe the
   "is this ipv4 or or ipv6?" extra header)

TAP mode:

  IPv4 MSS: 460
    514 bytes on wire (4112 bits), 514 bytes captured (4112 bits) on interface tap0, id 0
    ...
    TCP payload (448 bytes)

  IPv6 MSS: 440
    514 bytes on wire (4112 bits), 514 bytes captured (4112 bits) on interface tap0, id 0
    ...
    TCP payload (428 bytes)


So, in tun mode, this does what it says on the lid ("MSS is lowered so
the resulting IPv4/IPv6 packet is not exceeding this value").  For TAP,
there is overhead, which gets added.

Arguably this is *correct* behaviour - this goes hand in hand with
"--tun-mtu $small --mssfix (default)", and if you set a "--tun-mtu 500"
on a *TAP* interface, this is what you get: IP packets "up to 500",
plus ethernet header.  The system-set MSS value will be exactly the
same number that OpenVPN uses in this case (tested!), so -> perfect.


Your patch has been applied to the master branch.

commit 47671d6d6814eadb3dd5e742ebc40c6f21038224
Author: Arne Schwabe
Date:   Thu Feb 24 15:42:45 2022 +0100

     Implement fixed MSS value for mssfix and use it for non default MTUs

     Acked-by: Lev Stipakov <lstipakov@gmail.com>
     Message-Id: <20220224144245.878056-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23886.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst
index 782aa7381..6473ad423 100644
--- a/doc/man-sections/link-options.rst
+++ b/doc/man-sections/link-options.rst
@@ -132,12 +132,14 @@  the local and the remote host.
 
      mssfix max [mtu]
 
+     mssfix max [fixed]
+
      mssfix
 
   Announce to TCP sessions running over the tunnel that they should limit
   their send packet sizes such that after OpenVPN has encapsulated them,
   the resulting UDP packet size that OpenVPN sends to its peer will not
-  exceed ``max`` bytes. The default value is :code:`1450`. Use :code:`0`
+  exceed ``max`` bytes. The default value is :code:`1492 mtu`. Use :code:`0`
   as max to disable mssfix.
 
   If the :code:`mtu` parameter is specified the ``max`` value is interpreted
@@ -153,6 +155,11 @@  the local and the remote host.
   transmitted over IPv4 on a link with MTU 1478 or higher without IP level
   fragmentation (and 1498 for IPv6).
 
+  If the :code:`fixed` parameter is specified, OpenVPN will make no attempt
+  to calculate the VPN encapsulation overhead but instead will set the MSS to
+  limit the size of the payload IP packets to the specified number. IPv4 packets
+  will have the MSS value lowered to mssfix - 40 and IPv6 packets to mssfix - 60.
+
   if ``--mssfix`` is specified is specified without any parameter it
   inherits the parameters of ``--fragment`` if specified or uses the
   default for ``--mssfix`` otherwise.
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 25b264059..22f9fcf2f 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -289,6 +289,14 @@  frame_calculate_mssfix(struct frame *frame, struct key_type *kt,
                        const struct options *options,
                        struct link_socket_info *lsi)
 {
+    if (options->ce.mssfix_fixed)
+    {
+        /* we subtract IPv4 and TCP overhead here, mssfix method will add the
+         * extra 20 for IPv6 */
+        frame->mss_fix = options->ce.mssfix - (20 + 20);
+        return;
+    }
+
     unsigned int overhead, payload_overhead;
 
     overhead = frame_calculate_protocol_header_size(kt, options, false);
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7ce0ba613..2bf711fd0 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1515,6 +1515,7 @@  show_connection_entry(const struct connection_entry *o)
 #endif
     SHOW_INT(mssfix);
     SHOW_BOOL(mssfix_encap);
+    SHOW_BOOL(mssfix_fixed);
 
     SHOW_INT(explicit_exit_notification);
 
@@ -2937,19 +2938,24 @@  options_postprocess_mutate_ce(struct options *o, struct connection_entry *ce)
         }
         else
 #endif
-        if (ce->tun_mtu_defined && o->ce.tun_mtu == TUN_MTU_DEFAULT)
+        if (ce->tun_mtu_defined)
         {
-            /* We want to only set mssfix default value if we use a default
-             * MTU Size, otherwise the different size of tun should either
-             * already solve the problem or mssfix might artifically make the
-             * payload packets smaller without mssfix 0 */
-            ce->mssfix = MSSFIX_DEFAULT;
-            ce->mssfix_encap = true;
-        }
-        else
-        {
-            msg(D_MTU_INFO, "Note: not enabling mssfix for non-default value "
-                            "of --tun-mtu");
+            if (o->ce.tun_mtu == TUN_MTU_DEFAULT)
+            {
+                /* We want to only set mssfix default value if we use a default
+                 * MTU Size, otherwise the different size of tun should either
+                 * already solve the problem or mssfix might artifically make the
+                 * payload packets smaller without mssfix 0 */
+                ce->mssfix = MSSFIX_DEFAULT;
+                ce->mssfix_encap = true;
+            }
+            else
+            {
+                /* We still apply the mssfix value but only adjust it to the
+                 * size of the tun interface. */
+                ce->mssfix = ce->tun_mtu;
+                ce->mssfix_fixed = true;
+            }
         }
     }
 
@@ -6844,7 +6850,7 @@  add_option(struct options *options,
         if (p[1])
         {
             /* value specified, assume encapsulation is not
-             * included unles "mtu" follows later */
+             * included unless "mtu" follows later */
             options->ce.mssfix = positive_atoi(p[1]);
             options->ce.mssfix_encap = false;
             options->ce.mssfix_default = false;
@@ -6854,12 +6860,17 @@  add_option(struct options *options,
             /* Set MTU to default values */
             options->ce.mssfix_default = true;
             options->ce.mssfix_encap = true;
+            options->ce.mssfix_fixed = false;
         }
 
         if (p[2] && streq(p[2], "mtu"))
         {
             options->ce.mssfix_encap = true;
         }
+        else if (p[2] && streq(p[2], "fixed"))
+        {
+            options->ce.mssfix_fixed = true;
+        }
         else if (p[2])
         {
             msg(msglevel, "Unknown parameter to --mssfix: %s", p[2]);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 9c25fbafd..75f3bb264 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -131,6 +131,7 @@  struct connection_entry
     bool mssfix_default; /* true if --mssfix should use the default parameters */
     bool mssfix_encap;   /* true if --mssfix had the "mtu" parameter to include
                           * overhead from IP and TCP/UDP encapsulation */
+    bool mssfix_fixed;   /* use the mssfix value without any encapsulation adjustments */
 
     int explicit_exit_notification; /* Explicitly tell peer when we are exiting via OCC_EXIT or [RESTART] message */