[Openvpn-devel] [PATCH v5 1/2] Allow tun-mtu to be pushed

[Openvpn-devel] [PATCH v5 1/2] Allow tun-mtu to be pushed

[Arne Schwabe]

This allows tun-mtu to pushed but only up to the size of the preallocated
buffers. This is not a perfect solution but should allow most of the use
cases where the mtu is close enough to 1500 (or smaller).

Signed-off-by: Arne Schwabe <arne@...1227...>

Patch v4: rebase for check_session_cipher name change
Patch v5: remove mention of change of default mtu, remove leftover code
          from an earlier approach.

Signed-off-by: Arne Schwabe <arne@...1227...>
---
 Changes.rst                              |  5 +++
 doc/man-sections/client-options.rst      |  4 +++
 doc/man-sections/vpn-network-options.rst |  5 +++
 src/openvpn/init.c                       | 44 ++++++++++++++++++++----
 src/openvpn/mtu.c                        |  1 +
 src/openvpn/mtu.h                        |  3 ++
 src/openvpn/options.c                    | 15 +++++++-
 src/openvpn/options.h                    |  3 ++
 src/openvpn/ssl.c                        |  3 ++
 9 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index 657227c0f..889689877 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -105,6 +105,11 @@ Secure renegotiation
     previously authenticated peer can do trigger renegotiation and complete
     renegotiations. This also closes CVE-2021-3568.
 
+Tun MTU can be pushed
+    The  client can now also dynamically configure its MTU and the server
+    will try to push the client MTU when the client supports it. The
+    directive ``--tun-mtu-max`` has been introduced to specify the maximum
+    pushable MTU size (defaults to 1600).
 
 Improved control channel packet size control (``max-packet-size``)
     The size of control channel is no longer tied to
diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst
index 5a906895b..07651479f 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -363,6 +363,10 @@ configuration.
         The client announces the list of supported ciphers configured with the
         ``--data-ciphers`` option to the server.
 
+  :code:`IV_MTU=<max_mtu>`
+        The client announces the support of pushable MTU and the maximum MTU
+        it is willing to accept.
+
   :code:`IV_GUI_VER=<gui_id> <version>`
         The UI version of a UI if one is running, for example
         :code:`de.blinkt.openvpn 0.5.47` for the Android app.
diff --git a/doc/man-sections/vpn-network-options.rst b/doc/man-sections/vpn-network-options.rst
index 5b2f84707..2d0e662e4 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -516,6 +516,11 @@ routing.
   It's best to use the ``--fragment`` and/or ``--mssfix`` options to deal
   with MTU sizing issues.
 
+--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
+  allocation for larger packet sizes.
+
 --tun-mtu-extra n
   Assume that the TUN/TAP device might return as many as ``n`` bytes more
   than the ``--tun-mtu`` size on read. This parameter defaults to 0, which
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c6f6865a8..02714b43d 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2312,7 +2312,8 @@ pull_permission_mask(const struct context *c)
         | OPT_P_ECHO
         | OPT_P_PULL_MODE
         | OPT_P_PEER_ID
-        | OPT_P_NCP;
+        | OPT_P_NCP
+        | OPT_P_PUSH_MTU;
 
     if (!c->options.route_nopull)
     {
@@ -2475,6 +2476,25 @@ do_deferred_options(struct context *c, const unsigned int found)
         }
     }
 
+    /* Cipher is considered safe, so we can use it to calculate the max
+     * MTU size */
+    if (found & OPT_P_PUSH_MTU)
+    {
+        /* MTU has changed, check that the pushed MTU is small enough to
+         * be able to change it */
+        msg(D_PUSH, "OPTIONS IMPORT: tun-mtu set to %d", c->options.ce.tun_mtu);
+
+        struct frame *frame = &c->c2.frame;
+
+        if (c->options.ce.tun_mtu > frame->tun_max_mtu)
+        {
+            msg(D_PUSH_ERRORS, "Server pushed a large mtu, please add "
+                "tun-mtu-max %d in the client configuration",
+                c->options.ce.tun_mtu);
+        }
+        frame->tun_mtu = min_int(frame->tun_max_mtu, c->options.ce.tun_mtu);
+    }
+
     return true;
 }
 
@@ -2635,10 +2655,16 @@ frame_finalize_options(struct context *c, const struct options *o)
     struct frame *frame = &c->c2.frame;
 
     frame->tun_mtu = get_frame_mtu(c, o);
+    frame->tun_max_mtu = o->ce.tun_mtu_max;
+
+    /* max mtu needs to be at least as large as the tun mtu */
+    frame->tun_max_mtu = max_int(frame->tun_mtu, frame->tun_max_mtu);
 
-    /* We always allow at least 1500 MTU packets to be received in our buffer
-     * space */
-    size_t payload_size = max_int(1500, frame->tun_mtu);
+    /* We always allow at least 1600 MTU packets to be received in our buffer
+     * space to allow server to push "baby giant" MTU sizes */
+    frame->tun_max_mtu = max_int(1600, frame->tun_max_mtu);
+
+    size_t payload_size = frame->tun_max_mtu;
 
     /* we need to be also large enough to hold larger control channel packets
      * if configured */
@@ -2650,9 +2676,9 @@ frame_finalize_options(struct context *c, const struct options *o)
         payload_size += o->ce.tun_mtu_extra;
     }
 
-    /* Add 100 byte of extra space in the buffer to account for slightly
-     * mismatched MUTs between peers */
-    payload_size += 100;
+    /* Add 32 byte of extra space in the buffer to account for small errors
+     * in the calculation */
+    payload_size += 32;
 
 
     /* the space that is reserved before the payload to add extra headers to it
@@ -3215,6 +3241,10 @@ do_init_frame_tls(struct context *c)
                c->c2.frame.buf.payload_size);
         frame_print(&c->c2.tls_multi->opt.frame, D_MTU_INFO,
                     "Control Channel MTU parms");
+
+        /* Keep the max mtu also in the frame of tls multi so it can access
+         * it in push_peer_info */
+        c->c2.tls_multi->opt.frame.tun_max_mtu = c->c2.frame.tun_max_mtu;
     }
     if (c->c2.tls_auth_standalone)
     {
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index f60f48534..a74a08153 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -222,6 +222,7 @@ frame_print(const struct frame *frame,
     buf_printf(&out, " max_frag:%d", frame->max_fragment_size);
 #endif
     buf_printf(&out, " tun_mtu:%d", frame->tun_mtu);
+    buf_printf(&out, " tun_max_mtu:%d", frame->tun_max_mtu);
     buf_printf(&out, " headroom:%d", frame->buf.headroom);
     buf_printf(&out, " payload:%d", frame->buf.payload_size);
     buf_printf(&out, " tailroom:%d", frame->buf.tailroom);
diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h
index 370806fb5..badb3a6a8 100644
--- a/src/openvpn/mtu.h
+++ b/src/openvpn/mtu.h
@@ -138,6 +138,9 @@ struct frame {
                                  *   control frame payload (although most of
                                  *   code ignores it)
                                  */
+    int tun_max_mtu;            /**< the maximum tun-mtu size the buffers are
+                                 *   are sized for. This is the upper bound that
+                                 *   a server can push as MTU */
 
     int extra_tun;              /**< Maximum number of bytes in excess of
                                  *   the tun/tap MTU that might be read
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8027c572f..235d1f6cd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6449,10 +6449,23 @@ add_option(struct options *options,
     }
     else if (streq(p[0], "tun-mtu") && p[1] && !p[2])
     {
-        VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION);
+        VERIFY_PERMISSION(OPT_P_PUSH_MTU|OPT_P_CONNECTION);
         options->ce.tun_mtu = positive_atoi(p[1]);
         options->ce.tun_mtu_defined = true;
     }
+    else if (streq(p[0], "tun-mtu-max") && p[1] && !p[3])
+    {
+        VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION);
+        int max_mtu = positive_atoi(p[1]);
+        if (max_mtu < 68 || max_mtu > 65536)
+        {
+            msg(msglevel, "--tun-mtu-max value '%s' is invalid", p[1]);
+        }
+        else
+        {
+            options->ce.tun_mtu_max = max_mtu;
+        }
+    }
     else if (streq(p[0], "tun-mtu-extra") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b165ee5b7..a2bc13a1c 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -118,6 +118,8 @@ struct connection_entry
     const char *socks_proxy_authfile;
 
     int tun_mtu;         /* MTU of tun device */
+    int tun_mtu_max;     /* maximum MTU that can be pushed */
+
     bool tun_mtu_defined; /* true if user overriding parm with command line option */
     int tun_mtu_extra;
     bool tun_mtu_extra_defined;
@@ -730,6 +732,7 @@ struct options
 #define OPT_P_CONNECTION      (1<<27)
 #define OPT_P_PEER_ID         (1<<28)
 #define OPT_P_INLINE          (1<<29)
+#define OPT_P_PUSH_MTU        (1<<30)
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 31bea2b23..443613096 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2061,6 +2061,9 @@ push_peer_info(struct buffer *buf, struct tls_session *session)
 
             /* support for AUTH_FAIL,TEMP control message */
             iv_proto |= IV_PROTO_AUTH_FAIL_TEMP;
+
+            /* support for tun-mtu as part of the push message */
+            buf_printf(&out, "IV_MTU=%d\n", session->opt->frame.tun_max_mtu);
         }
 
         /* support for Negotiable Crypto Parameters */
-- 
2.37.0 (Apple Git-136)

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

[Arne Schwabe]

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@...1227...>
---
 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(-)

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;
 }
 
-- 
2.37.0 (Apple Git-136)

[Openvpn-devel] [PATCH applied] Re: Allow tun-mtu to be pushed

[Gert Doering]

Acked-by: Gert Doering <gert@...1296...>

This is a step towards being able to adjust MTU in a server-controlled
way, and to either enable large-MTU setups (9000), or to reduce MTU
to "outside packets never need to be fragmented".  This does not change
defaults, so no behavioural changes are to be expected.

Thanks to all the previous reviewers - since v5 is a bit different again,
I've only recorded my own ACK, but the previous reviews helped.


Stare-at-code I was wondering why we introduce a new OPT_P_PUSH_MTU
now, but this is needed for "changed option handling".

Tested on the generic testbed, and then with a server instance pushing
"tun-mtu 1300" and t_client on the other end (leading to a semi-intentional
MTU mismatch).  Works, as the buffers are always sized appropriately
(and, interstingly enough, the OS tun interfaces accept incoming 1500 byte
packets even if set to mtu 1300).

Modifications, as discussed on IRC:

 - removed the "Cipher is considered safe" comment, this code does not
   exist anymore
 - reworded the error message to "Server-pushed tun-mtu is too large, ..."
 - reworded the manpage slightly, to document that reducing the max-mtu
   (below the 1600 default) is not possible

Your patch has been applied to the master branch.

commit 01aed6a5df193a09d8d3c28c5a1580241e0a5e05
Author: Arne Schwabe
Date:   Wed Nov 9 16:48:09 2022 +0100

     Allow tun-mtu to be pushed

     Signed-off-by: Arne Schwabe <arne@...1227...>
     Signed-off-by: Arne Schwabe <arne@...1227...>
     Acked-by: Gert Doering <gert@...1296...>
     Message-Id: <20221109154810.1268403-1-arne@...1227...>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25498.html
     Signed-off-by: Gert Doering <gert@...1296...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH applied] Re: Push server mtu to client when supported and support occ mtu

[Gert Doering]

Acked-by: Gert Doering <gert@...1296...>

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@...1227...>
     Acked-by: Gert Doering <gert@...1296...>
     Message-Id: <20221109154810.1268403-2-arne@...1227...>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25499.html
     Signed-off-by: Gert Doering <gert@...1296...>

--
kind regards,

Gert Doering