diff mbox series

[Openvpn-devel,v1] Ensure pushed tun-mtu is no lower than TUN_MTU_MIN

Message ID 20260605180931.3547-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] Ensure pushed tun-mtu is no lower than TUN_MTU_MIN | expand

Commit Message

Gert Doering June 5, 2026, 6:09 p.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

The normal path ensure that the minimum tun mtu is set to at least
TUN_MTU_MIN. However, the pushed options path does not have this
restriction.

Check that the tun-mtu is within the limits of min/max mtu
in options.c. This ensure that the check is also correctly done
on the pushed variant.

Also add an extra check to keep the allowed payload for icmp6 packets
to be at least 64 bytes in the the block-ipv6 code path
(ipv6_send_icmp_unreachable) as extra layer of defence.

Pushing a low mtu like 1 and also block-ipv6 could trigger an
assertion in the ipv6_send_icmp_unreachable code path.

Reported-By: Haiyang Huang <huanghaiyang83@gmail.com>
Change-Id: Iff8b336126a5dff9871213664b1e8585fb70d21e
Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Acked-by: MaxF <max@max-fillinger.net>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1707
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1707
This mail reflects revision 1 of this Change.

Signed-off-by line for the author was added as per our policy.

Acked-by according to Gerrit (reflected above):
MaxF <max@max-fillinger.net>
diff mbox series

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 27cfd36..d24f534 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1598,7 +1598,9 @@ 
      * frame should be <= 1280 and have as much as possible of the original
      * packet
      */
-    const int max_payload_size = min_int(MAX_ICMPV6LEN, c->c2.frame.tun_mtu - icmpheader_len);
+    int max_payload_size = min_int(MAX_ICMPV6LEN, c->c2.frame.tun_mtu - icmpheader_len);
+    /* Ensure that minimum payload size is at least 64 bytes as extra safety layer */
+    max_payload_size = max_int(max_payload_size, 64);
     const int payload_len = min_int(max_payload_size, BLEN(&inputipbuf));
     const uint16_t icmp_len = (uint16_t)(sizeof(struct openvpn_icmp6hdr) + payload_len);
 
diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h
index ca8109c..8f07e75 100644
--- a/src/openvpn/mtu.h
+++ b/src/openvpn/mtu.h
@@ -68,6 +68,11 @@ 
  */
 #define TUN_MTU_DEFAULT 1500
 
+/**
+ * Maximum MTU we accept for MTU related options
+ */
+#define TUN_MTU_MAX 65536
+
 /*
  * Minimum maximum MTU
  */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 0c2866c..128d1e5 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6409,26 +6409,28 @@ 
     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], msglevel);
-        options->ce.tun_mtu_defined = true;
-        if (p[2])
+        if (atoi_constrained(p[1], &options->ce.tun_mtu, "tun-mtu", TUN_MTU_MIN, TUN_MTU_MAX, msglevel))
         {
-            options->ce.occ_mtu = positive_atoi(p[2], msglevel);
-        }
-        else
-        {
-            options->ce.occ_mtu = 0;
+            options->ce.tun_mtu_defined = true;
+            if (p[2])
+            {
+                atoi_constrained(p[2], &options->ce.occ_mtu, "tun-mtu occ-mtu", TUN_MTU_MIN, TUN_MTU_MAX, msglevel);
+            }
+            else
+            {
+                options->ce.occ_mtu = 0;
+            }
         }
     }
     else if (streq(p[0], "tun-mtu-max") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
-        atoi_constrained(p[1], &options->ce.tun_mtu_max, p[0], TUN_MTU_MAX_MIN, 65536, msglevel);
+        atoi_constrained(p[1], &options->ce.tun_mtu_max, p[0], TUN_MTU_MAX_MIN, TUN_MTU_MAX, msglevel);
     }
     else if (streq(p[0], "tun-mtu-extra") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
-        if (atoi_constrained(p[1], &options->ce.tun_mtu_extra, p[0], 0, 65536, msglevel))
+        if (atoi_constrained(p[1], &options->ce.tun_mtu_extra, p[0], 0, TUN_MTU_MAX, msglevel))
         {
             options->ce.tun_mtu_extra_defined = true;
         }