[Openvpn-devel,v5,2/2] Push server mtu to client when supported and support occ mtu

Message ID 20221109154810.1268403-2-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v5,1/2] Allow tun-mtu to be pushed | expand

Commit Message

Arne Schwabe Nov. 9, 2022, 3:48 p.m. UTC
To maximise compatibility allow to lie our MTU in the default OCC
message.

Patch v2: improve documentation
Patch v3: split changing default MTU into its own patch
Patch v5: remove leftover mentions to default MTU

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 Changes.rst                              |  6 +++++-
 doc/man-sections/vpn-network-options.rst | 25 ++++++++++++++++++++----
 src/openvpn/options.c                    | 21 ++++++++++++++++++--
 src/openvpn/options.h                    |  1 +
 src/openvpn/push.c                       | 16 +++++++++++++++
 5 files changed, 62 insertions(+), 7 deletions(-)

Comments

Gert Doering Nov. 14, 2022, 12:37 p.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Stared-at-code, discussed on IRC, and dropped "Changes.rst" entry, as
user-visible behaviour is no longer changed (no default-MTU change).

Tested on the server testbed, having one instance do "tun-mtu 1400 1500"
now, and depending on client version, have it push MTU or not.

The OCC lying feature works - tried "max-mtu 1400 1490" and got
  WARNING: 'tun-mtu' is used inconsistently, local='tun-mtu 1490', remote='tun-mtu 1500'

.. we need to do something about OCC and link-mtu, though...
  WARNING: 'link-mtu' is used inconsistently, local='link-mtu 1442', remote='link-mtu 1545'
(outside of this patch, but annoying)


Also tested with "--tun-mtu 2000 1500", to trigger the "client cannot
handle this" message... works.  Well, sort of - the server still sends
a too-big MTU...

194.97.140.21:50362 peer info: IV_MTU=1800
cron2-freebsd-tc-amd64/194.97.140.21:50362 Warning: reported maximum MTU from client (1800) is lower than MTU used on the server (2000). Add tun-max-mtu 2000 to client configuration.
cron2-freebsd-tc-amd64/194.97.140.21:50362 SENT CONTROL [cron2-freebsd-tc-amd64]: 'PUSH_REPLY,... cc-exit tls-ekm,tun-mtu 2000' (status=1)

.. but the client ignores this...

OPTIONS IMPORT: tun-mtu set to 2000
Server-pushed tun-mtu is too large, please add tun-mtu-max 2000 in the client configuration

.. and configures 1800 (configured --max-tun-mtu on the client).

tcpdump confirms that pings *inside* the tunnel go up to 1800 without
fragmentation...

13:29:57.041939 IP 10.204.2.6 > 10.204.0.1: ICMP echo request, id 49729, seq 31, length 1780
13:29:57.042693 IP 10.204.0.1 > 10.204.2.6: ICMP echo reply, id 49729, seq 31, length 1780

.. but if I go larger ("ping -s 2000") the MTU mismatch "server 2000,
client 1800" strikes, and I get

2022-11-14 13:30:13 tun packet too large on write (tried=1996,max=1968)

(--tun-mtu-max 2000 on the client will fix this).  This is expected -
there is some headroom, but not "200 bytes".  So when going large-mtu,
ensure your client configs are matching.


Your patch has been applied to the master branch.

commit 761575cb7b49e94f361c6aaf4bb43d7c7baa5b38
Author: Arne Schwabe
Date:   Wed Nov 9 16:48:10 2022 +0100

     Push server mtu to client when supported and support occ mtu

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20221109154810.1268403-2-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25499.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>

--
kind regards,

Gert Doering

Patch

diff --git a/Changes.rst b/Changes.rst
index 889689877..a158143a7 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -184,7 +184,11 @@  User-visible Changes
 - control channel packet maximum size is no longer influenced by
   ``--link-mtu``/``--tun-mtu`` and must be set by ``--max-packet-size`` now.
   The default is 1250 for the control channel size.
-
+- the default of ``--tun-mtu`` has been changed to ``--tun-mtu 1420 1500`` when
+  running in server mode. This will create an MTU mismatch with older clients
+  (newer clients allow pushable mtu) but the most common server platforms
+  (Linux and FreeBSD) allow receiving 1500 byte packets even when tun-mtu is
+  set to 1420, still allowing larger packets from clients with 1500 byte MTU.
 - In point-to-point OpenVPN setups (no ``--server``), using
   ``--explict-exit-notiy`` on one end would terminate the other side at
   session end.  This is considered a no longer useful default and has
diff --git a/doc/man-sections/vpn-network-options.rst b/doc/man-sections/vpn-network-options.rst
index 2d0e662e4..6bd41bf5f 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -500,10 +500,23 @@  routing.
   arguments of ``--ifconfig`` to mean "address netmask", no longer "local
   remote".
 
---tun-mtu n
-  Take the TUN device MTU to be **n** and derive the link MTU from it
-  (default :code:`1500`). In most cases, you will probably want to leave
-  this parameter set to its default value.
+--tun-mtu args
+
+  Valid syntaxes:
+  ::
+
+      tun-mtu tun-mtu
+      tun-mtu tun-mtu occ-mtu
+
+  Take the TUN device MTU to be ``tun-mtu`` and derive the link MTU from it.
+  In most cases, you will probably want to leave this parameter set to
+  its default value.
+
+  The default for :code:`tun-mtu` is 1500.
+
+  The OCC MTU can be used to avoid warnings about mismatched MTU from
+  clients. If :code:`occ-mtu` is not specified, it will to default to the
+  tun-mtu.
 
   The MTU (Maximum Transmission Units) is the maximum datagram size in
   bytes that can be sent unfragmented over a particular network path.
@@ -516,6 +529,10 @@  routing.
   It's best to use the ``--fragment`` and/or ``--mssfix`` options to deal
   with MTU sizing issues.
 
+  Note: Depending on the platform, the operating system allows to receive
+  packets larger than ``tun-mtu`` (e.g. Linux and FreeBSD) but other platforms
+  (like macOS) limit received packets to the same size as the MTU.
+
 --tun-max-mtu maxmtu
   This configures the maximum MTU size that a server can push to ``maxmtu``.
   The default for ``maxmtu`` is 1600. This will increase internal buffers
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 235d1f6cd..33b7c698d 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -825,6 +825,7 @@  init_options(struct options *o, const bool init_gc)
     o->status_file_version = 1;
     o->ce.bind_local = true;
     o->ce.tun_mtu = TUN_MTU_DEFAULT;
+    o->ce.occ_mtu = 0;
     o->ce.link_mtu = LINK_MTU_DEFAULT;
     o->ce.tls_mtu = TLS_MTU_DEFAULT;
     o->ce.mtu_discover_type = -1;
@@ -4193,7 +4194,15 @@  options_string(const struct options *o,
     buf_printf(&out, ",link-mtu %u",
                (unsigned int) calc_options_string_link_mtu(o, frame));
 
-    buf_printf(&out, ",tun-mtu %d", frame->tun_mtu);
+    if (o->ce.occ_mtu != 0)
+    {
+        buf_printf(&out, ",tun-mtu %d", o->ce.occ_mtu);
+    }
+    else
+    {
+        buf_printf(&out, ",tun-mtu %d", frame->tun_mtu);
+    }
+
     buf_printf(&out, ",proto %s",  proto_remote(o->ce.proto, remote));
 
     bool p2p_nopull = o->mode == MODE_POINT_TO_POINT && !PULL_DEFINED(o);
@@ -6447,11 +6456,19 @@  add_option(struct options *options,
         options->ce.link_mtu = positive_atoi(p[1]);
         options->ce.link_mtu_defined = true;
     }
-    else if (streq(p[0], "tun-mtu") && p[1] && !p[2])
+    else if (streq(p[0], "tun-mtu") && p[1] && !p[3])
     {
         VERIFY_PERMISSION(OPT_P_PUSH_MTU|OPT_P_CONNECTION);
         options->ce.tun_mtu = positive_atoi(p[1]);
         options->ce.tun_mtu_defined = true;
+        if (p[2])
+        {
+            options->ce.occ_mtu = positive_atoi(p[2]);
+        }
+        else
+        {
+            options->ce.occ_mtu = 0;
+        }
     }
     else if (streq(p[0], "tun-mtu-max") && p[1] && !p[3])
     {
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index a2bc13a1c..68ad0cacb 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -118,6 +118,7 @@  struct connection_entry
     const char *socks_proxy_authfile;
 
     int tun_mtu;         /* MTU of tun device */
+    int occ_mtu;         /* if non-null, this is the MTU we announce to peers in OCC */
     int tun_mtu_max;     /* maximum MTU that can be pushed */
 
     bool tun_mtu_defined; /* true if user overriding parm with command line option */
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index b2e46f1ca..3d32669af 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -678,6 +678,22 @@  prepare_push_reply(struct context *c, struct gc_arena *gc,
         push_option_fmt(gc, push_list, M_USAGE, "protocol-flags%s", buf_str(&proto_flags));
     }
 
+    /* Push our mtu to the peer if it supports pushable MTUs */
+    int client_max_mtu = 0;
+    const char *iv_mtu = extract_var_peer_info(tls_multi->peer_info, "IV_MTU=", gc);
+
+    if (iv_mtu && sscanf(iv_mtu, "%d", &client_max_mtu) == 1)
+    {
+        push_option_fmt(gc, push_list, M_USAGE, "tun-mtu %d", o->ce.tun_mtu);
+        if (client_max_mtu < o->ce.tun_mtu)
+        {
+            msg(M_WARN, "Warning: reported maximum MTU from client (%d) is lower "
+                "than MTU used on the server (%d). Add tun-max-mtu %d "
+                "to client configuration.", client_max_mtu,
+                o->ce.tun_mtu, o->ce.tun_mtu);
+        }
+    }
+
     return true;
 }