[Openvpn-devel] Set the correct mtu on windows based systems

Message ID 20200421154612.14140-1-cschenk@mail.uni-paderborn.de
State Accepted
Headers show
Series
  • [Openvpn-devel] Set the correct mtu on windows based systems
Related show

Commit Message

Christopher Schenk April 21, 2020, 3:46 p.m.
Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
---
 include/openvpn-msg.h         | 10 +++-
 src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
 src/openvpnserv/interactive.c | 31 ++++++++++++
 3 files changed, 129 insertions(+), 1 deletion(-)

Comments

Gert Doering June 10, 2020, 4:01 p.m. | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Selva has ACKed the version sent in April 2019, and it should have been
merged then and there - we (I) let it rot, tun.c got changed quite a bit
for wintun support, now we have a new patch.  Thanks!  (And thanks for
the repeated reminders, this is necessary at times)

I have compared both patches.  They differ in context, of course (*),
but the actual changes introduced by this patch are the same - so, I
think I can take the ACK from Selva for the new patch as well.


(*) one of the context differences is the iservice message types - we
have grown "msg_register_ring_buffers" in the meantime.  So to get this
patch into 2.4 would be somewhat complicated, to ensure no iservice
breakage.  But since 2.5 is really close now, I find that acceptable.


I have also reviewed this patch (so, two ACKs, one from Selva and one
from me) - code looks good.

The "SitePrefixLength = 0" call confused me a bit, but this is correct
according to (long URL):
  https://docs.microsoft.com/en-us/windows/win32/api/netioapi/nf-netioapi-setipinterfaceentry

I have tested this on Win10:

 - config with no "tun-mtu" and iservice -> interface MTU = 1500, good
 - config with "tun-mtu 1330" and iservice -> interface MTU = 1330, good
 - config with no "tun-mtu" and priv. gui -> interface MTU = 1500, good
 - config with "tun-mtu 1330" and priv. gui -> interface MTU = 1330, good

because I sometimes test things before I question myself whether this is
a useful thing to test:
 - config with "tun-mtu 1200" and priv. gui -> 
     *succeeds* for IPv4
     *fails* for IPv6
     "TUN: setting IPv6 mtu using service failed: Falscher Parameter.
      [status=87 if_index=8]

   (which is Windows telling me that "the IETF says that the mininum MTU
    for IPv6 is 1280, so go away and complain to them!" - technically
    correct, though slightly confusing)


Just for reference: "ipconfig /all" does not show the MTU, you need to
call "netsh interface ipv4/ipv6 show subinterfaces".


Remarks:

  - if using the iservice, openvpn will log
      "IPv<x> MTU set to 1330 on interface 8 using service"
    if using the gui in privileged mode, it will log
      "Successfully set IPv<x> mtu on interface 8"

    --> I think it would be good to use the same message here, and 
    include the numeric value in both cases, like

      "IPv<x> MTU set to 1330 on interface 8 using SetIpInterfaceEntry()"

    or so.  Can you send a followup patch?  It will be expedited!

  - shall we do "if MTU < 1280, log a notice and do not attempt to set
    IPv6 MTU"?  Or "just log the notice, in case windows will accept
    it nonetheless"?

    if (mtu<1280)
    {
      msg(M_INFO, "NOTE: IPv6 interface MTU < 1280 conflicts with IETF standards and might not work");
    }


Your patch has been applied to the master branch.

Thanks for your patience.


commit 0213f80ed72ad8b6bb43db3bbd72a66ec2e12fcd
Author: Christopher Schenk
Date:   Tue Apr 21 17:46:12 2020 +0200

     Set the correct mtu on windows based systems

     Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20200421154612.14140-1-cschenk@mail.uni-paderborn.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19803.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 3ed62069..a4789e34 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -39,7 +39,8 @@  typedef enum {
     msg_del_block_dns,
     msg_register_dns,
     msg_enable_dhcp,
-    msg_register_ring_buffers
+    msg_register_ring_buffers,
+    msg_set_mtu
 } message_type_t;
 
 typedef struct {
@@ -127,4 +128,11 @@  typedef struct {
     HANDLE receive_tail_moved;
 } register_ring_buffers_message_t;
 
+typedef struct {
+    message_header_t header;
+    interface_t iface;
+    short family;
+    int mtu;
+} set_mtu_message_t;
+
 #endif /* ifndef OPENVPN_MSG_H_ */
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 8e692977..0e6dfe72 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -73,6 +73,10 @@  static void netsh_ifconfig(const struct tuntap_options *to,
                            const in_addr_t netmask,
                            const unsigned int flags);
 
+static void windows_set_mtu(const int iface_index,
+                            const short family,
+                            const int mtu);
+
 static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
                                    const int addr_len,
                                    const char *flex_name);
@@ -214,6 +218,47 @@  out:
     return ret;
 }
 
+static bool
+do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
+{
+    DWORD len;
+    bool ret = false;
+    ack_message_t ack;
+    struct gc_arena gc = gc_new();
+    HANDLE pipe = tt->options.msg_channel;
+    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
+    set_mtu_message_t mtu_msg = {
+        .header = {
+            msg_set_mtu,
+            sizeof(set_mtu_message_t),
+            0
+        },
+        .iface = {.index = tt->adapter_index,.name = tt->actual_name },
+        .mtu = mtu,
+        .family = family
+    };
+
+    if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack, "Set_mtu"))
+    {
+        goto out;
+    }
+
+    if (ack.error_number != NO_ERROR)
+    {
+        msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s [status=%u if_index=%d]",
+            family_name, strerror_win32(ack.error_number, &gc), ack.error_number, mtu_msg.iface.index);
+    }
+    else
+    {
+        msg(M_INFO, "%s MTU set to %d on interface %d using service", family_name, mtu, mtu_msg.iface.index);
+        ret = true;
+    }
+
+out:
+    gc_free(&gc);
+    return ret;
+}
+
 #endif /* ifdef _WIN32 */
 
 #ifdef TARGET_SOLARIS
@@ -1018,6 +1063,7 @@  do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,
         do_address_service(true, AF_INET6, tt);
         add_route_connected_v6_net(tt, es);
         do_dns_service(true, AF_INET6, tt);
+        do_set_mtu_service(tt, AF_INET6, tun_mtu);
     }
     else
     {
@@ -1035,6 +1081,7 @@  do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,
         add_route_connected_v6_net(tt, es);
         /* set ipv6 dns servers if any are specified */
         netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len, ifname);
+        windows_set_mtu(tt->adapter_index, AF_INET6, tun_mtu);
     }
 #else /* platforms we have no IPv6 code for */
     msg(M_FATAL, "Sorry, but I don't know how to do IPv6 'ifconfig' commands on this operating system.  You should ifconfig your TUN/TAP device manually or use an --up script.");
@@ -1404,6 +1451,14 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
         netsh_ifconfig(&tt->options, ifname, tt->local,
                        tt->adapter_netmask, NI_IP_NETMASK|NI_OPTIONS);
     }
+    if (tt->options.msg_channel)
+    {
+        do_set_mtu_service(tt, AF_INET, tun_mtu);
+    }
+    else
+    {
+        windows_set_mtu(tt->adapter_index, AF_INET, tun_mtu);
+    }
 #else  /* if defined(TARGET_LINUX) */
     msg(M_FATAL, "Sorry, but I don't know how to do 'ifconfig' commands on this operating system.  You should ifconfig your TUN/TAP device manually or use an --up script.");
 #endif /* if defined(TARGET_LINUX) */
@@ -5432,6 +5487,40 @@  out:
     return ret;
 }
 
+static void
+windows_set_mtu(const int iface_index, const short family,
+                const int mtu)
+{
+    DWORD err = 0;
+    struct gc_arena gc = gc_new();
+    MIB_IPINTERFACE_ROW ipiface;
+    InitializeIpInterfaceEntry(&ipiface);
+    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
+    ipiface.Family = family;
+    ipiface.InterfaceIndex = iface_index;
+    err = GetIpInterfaceEntry(&ipiface);
+    if (err == NO_ERROR)
+    {
+        if (family == AF_INET)
+        {
+            ipiface.SitePrefixLength = 0;
+        }
+        ipiface.NlMtu = mtu;
+        err = SetIpInterfaceEntry(&ipiface);
+    }
+
+    if (err != NO_ERROR)
+    {
+        msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]",
+            family_name, strerror_win32(err, &gc), err, iface_index);
+    }
+    else
+    {
+        msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index);
+    }
+}
+
+
 /*
  * Return a TAP name for netsh commands.
  */
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 04d64b97..207cc4ae 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1286,6 +1286,29 @@  HandleRegisterRingBuffers(const register_ring_buffers_message_t *rrb, HANDLE ovp
     return err;
 }
 
+static DWORD
+HandleMTUMessage(const set_mtu_message_t *mtu)
+{
+    DWORD err = 0;
+    MIB_IPINTERFACE_ROW ipiface;
+    InitializeIpInterfaceEntry(&ipiface);
+    ipiface.Family = mtu->family;
+    ipiface.InterfaceIndex = mtu->iface.index;
+    err = GetIpInterfaceEntry(&ipiface);
+    if (err != NO_ERROR)
+    {
+        return err;
+    }
+    if (mtu->family == AF_INET)
+    {
+        ipiface.SitePrefixLength = 0;
+    }
+    ipiface.NlMtu = mtu->mtu;
+
+    err = SetIpInterfaceEntry(&ipiface);
+    return err;
+}
+
 static VOID
 HandleMessage(HANDLE pipe, HANDLE ovpn_proc, ring_buffer_handles_t *ring_buffer_handles,
               DWORD bytes, DWORD count, LPHANDLE events, undo_lists_t *lists)
@@ -1300,6 +1323,7 @@  HandleMessage(HANDLE pipe, HANDLE ovpn_proc, ring_buffer_handles_t *ring_buffer_
         dns_cfg_message_t dns;
         enable_dhcp_message_t dhcp;
         register_ring_buffers_message_t rrb;
+        set_mtu_message_t mtu;
     } msg;
     ack_message_t ack = {
         .header = {
@@ -1374,6 +1398,13 @@  HandleMessage(HANDLE pipe, HANDLE ovpn_proc, ring_buffer_handles_t *ring_buffer_
             }
             break;
 
+        case msg_set_mtu:
+            if (msg.header.size == sizeof(msg.mtu))
+            {
+                ack.error_number = HandleMTUMessage(&msg.mtu);
+            }
+            break;
+
         default:
             ack.error_number = ERROR_MESSAGE_TYPE;
             MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type %d"), msg.header.type);