openvpn: T9364: default the MTU to 1420 when DCO is enabled

The ovpn Kernel module does no MSS clamping, so with a 1500 bytes MTU
full sized packets get fragmented on the underlay.

Add an "mtu" node, rendered as "tun-mtu". With "offload dco" it
defaults to 1420, unless a raw tun-mtu/link-mtu/udp-mtu option is
present. Reject "mtu" together with such an option, below 100, and
below 1280 when IPv6 runs inside the tunnel.

2 files changed, 185 insertions, 2 deletions

diff --git a/src/conf_mode/interfaces_openvpn.py b/src/conf_mode/interfaces_openvpn.py
index 130c027a4..109e0f401 100755
--- a/src/conf_mode/interfaces_openvpn.py
+++ b/src/conf_mode/interfaces_openvpn.py
@@ -107,6 +107,18 @@ dco_cipher_options = ['data-ciphers', 'data-ciphers-fallback', 'ncp-ciphers']
 # dco_ciphers again, spelled the way OpenVPN reports them in
 # dco_get_supported_ciphers()
 dco_raw_ciphers = ['AES-128-GCM', 'AES-192-GCM', 'AES-256-GCM', 'CHACHA20-POLY1305']
+# The "ovpn" Kernel module does not clamp the TCP MSS the way the userspace data
+# path does with "mssfix", so a 1500 bytes tunnel MTU fragments every full sized
+# packet on the underlay. Leave room for the worst case encapsulation: IPv6 (40)
+# plus UDP (8) plus the DCO data channel header (24: opcode and peer-id 4,
+# packet-id 4, AEAD tag 16) is 72 bytes, rounded down to the WireGuard default.
+dco_default_mtu = '1420'
+# Raw options that size the tunnel themselves. OpenVPN refuses "tun-mtu" next to
+# "link-mtu" (or its alias "udp-mtu"), so any of them rules out the one rendered
+# from "mtu".
+raw_mtu_options = ['tun-mtu', 'link-mtu', 'udp-mtu']
+# TUN_MTU_MIN in OpenVPN's mtu.h
+openvpn_min_mtu = 100
 otp_path = '/config/auth/openvpn'
 otp_file = '/config/auth/openvpn/{ifname}-otp-secrets'
 secret_chars = list('ABCDEFGHIJKLMNOPQRSTUVWXYZ234567')
@@ -183,8 +195,9 @@ def get_config(config=None):
     if is_node_changed(conf, base + [ifname, 'openvpn-option']):
         openvpn.update({'restart_required': {}})
     # the offload, the operating mode and the device type all decide what kind
-    # of interface the data path needs, which can only change on a restart
-    for node in [['offload', 'dco'], ['mode'], ['device-type']]:
+    # of interface the data path needs, which can only change on a restart.
+    # OpenVPN also applies "tun-mtu" only when it brings the interface up.
+    for node in [['offload', 'dco'], ['mode'], ['device-type'], ['mtu']]:
         if is_node_changed(conf, base + [ifname] + node):
             openvpn.update({'restart_required': {}})
             break
@@ -219,12 +232,38 @@ def get_config(config=None):
     else:
         openvpn['protocol_modifier'] = ''
 
+    # The XML default only documents what OpenVPN uses on its own, so only an
+    # MTU set on the CLI counts. Anything else is left to OpenVPN to apply: VyOS
+    # sets an explicit "mtu" on the link only, so neither a raw option nor a
+    # tun-mtu pushed by the server is overridden.
+    if conf.exists(base + [ifname, 'mtu']):
+        openvpn['tun_mtu'] = openvpn['mtu']
+    else:
+        openvpn.pop('mtu', None)
+        # DCO can not fix the MSS, so pick an MTU that does not fragment on
+        # the underlay - unless a raw option sizes the tunnel
+        if (
+            dict_search('offload.dco', openvpn) is not None
+            and raw_mtu_option(openvpn) is None
+        ):
+            openvpn['tun_mtu'] = dco_default_mtu
+
     # Check vrf membership, to ensure firewall is updated
     if is_vrf_changed(conf, ifname):
         set_dependents('firewall', conf)
 
     return openvpn
 
+
+def raw_mtu_option(openvpn):
+    """The raw "openvpn-option" keyword that sizes the tunnel, if any."""
+    for option in dict_search('openvpn_option', openvpn) or []:
+        tmp = option.split()
+        if tmp and tmp[0].lstrip('-') in raw_mtu_options:
+            return tmp[0].lstrip('-')
+    return None
+
+
 def is_ec_private_key(pki, cert_name):
     if not pki or 'certificate' not in pki:
         return False
@@ -331,6 +370,32 @@ def verify_shared_secret(pki: dict, interface: str, path: list, name: str):
             f'Invalid "{path_str}" value "{name}" on OpenVPN interface {interface}'
         )
 
+
+def verify_openvpn_mtu(openvpn):
+    if 'mtu' not in openvpn:
+        return
+
+    # The raw option lands on the command line: "tun-mtu" silently overrides the
+    # rendered one while VyOS still applies "mtu" to the link, and "link-mtu"
+    # keeps the daemon from starting at all
+    keyword = raw_mtu_option(openvpn)
+    if keyword is not None:
+        raise ConfigError(f'Cannot use "mtu" together with "openvpn-option {keyword}"')
+
+    if int(openvpn['mtu']) < openvpn_min_mtu:
+        raise ConfigError(f'OpenVPN requires an MTU of at least {openvpn_min_mtu}')
+
+    # The tunnel addresses tell whether IPv6 runs inside the tunnel, which then
+    # needs the IPv6 minimum link MTU. "ip-version" is about the underlay only.
+    addresses = list(dict_search('local_address', openvpn) or [])
+    addresses += dict_search('server.subnet', openvpn) or []
+    min_mtu = 1280
+    if int(openvpn['mtu']) < min_mtu and any(is_ipv6(addr) for addr in addresses):
+        raise ConfigError(
+            f'IPv6 is used on interface "{openvpn["ifname"]}", '
+            f'the required minimum MTU is "{min_mtu}"!'
+        )
+
 def verify_pki(openvpn):
     pki = openvpn['pki']
     interface = openvpn['ifname']
@@ -816,6 +881,7 @@ def verify(openvpn):
 
     verify_data_ciphers_fallback(openvpn)
     verify_dco(openvpn)
+    verify_openvpn_mtu(openvpn)
 
     return None
 
diff --git a/src/tests/test_interfaces_openvpn.py b/src/tests/test_interfaces_openvpn.py
new file mode 100644
index 000000000..886bf5b05
--- /dev/null
+++ b/src/tests/test_interfaces_openvpn.py
@@ -0,0 +1,117 @@
+# Copyright VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import importlib
+import os
+import unittest
+
+from vyos import ConfigError
+
+try:
+    openvpn = importlib.import_module('src.conf_mode.interfaces_openvpn')
+except ModuleNotFoundError:  # for unittest.main()
+    import sys
+
+    sys.path.append(os.path.join(os.path.dirname(__file__), '../..'))
+    openvpn = importlib.import_module('src.conf_mode.interfaces_openvpn')
+
+
+class TestRawMtuOption(unittest.TestCase):
+    def test_sizing_options(self):
+        # with or without the dashes, and wherever it is in the list
+        for options, keyword in [
+            (['--tun-mtu 1400'], 'tun-mtu'),
+            (['tun-mtu 1400'], 'tun-mtu'),
+            (['--verb 3', '--tun-mtu 1400'], 'tun-mtu'),
+            (['--link-mtu 1450'], 'link-mtu'),
+            (['link-mtu 1450'], 'link-mtu'),
+            (['--udp-mtu 1450'], 'udp-mtu'),
+        ]:
+            with self.subTest(options=options):
+                config = {'openvpn_option': options}
+                self.assertEqual(openvpn.raw_mtu_option(config), keyword)
+
+    def test_lookalikes(self):
+        # options merely named alike, or carrying tun-mtu as an argument, do
+        # not size the tunnel
+        for options in [
+            ['--tun-mtu-max 1600'],
+            ['tun-mtu-extra 32'],
+            ['push "tun-mtu 1400"'],
+            ['--verb 3'],
+            [''],
+            [],
+        ]:
+            with self.subTest(options=options):
+                config = {'openvpn_option': options}
+                self.assertIsNone(openvpn.raw_mtu_option(config))
+
+        self.assertIsNone(openvpn.raw_mtu_option({}))
+
+
+class TestVerifyOpenvpnMtu(unittest.TestCase):
+    def config(self, **kwargs):
+        return {'ifname': 'vtun0', 'mode': 'server', **kwargs}
+
+    def test_no_mtu(self):
+        # nothing to check - not even a raw option on its own
+        openvpn.verify_openvpn_mtu(self.config())
+        openvpn.verify_openvpn_mtu(self.config(openvpn_option=['--tun-mtu 1400']))
+
+    def test_raw_option_conflict(self):
+        for option, keyword in [
+            ('--tun-mtu 1400', 'tun-mtu'),
+            ('--link-mtu 1450', 'link-mtu'),
+            ('--udp-mtu 1450', 'udp-mtu'),
+        ]:
+            with self.subTest(option=option):
+                config = self.config(mtu='1420', openvpn_option=[option])
+                with self.assertRaisesRegex(
+                    ConfigError, rf'openvpn-option\s+{keyword}'
+                ):
+                    openvpn.verify_openvpn_mtu(config)
+
+        # a lookalike is no conflict
+        config = self.config(mtu='1420', openvpn_option=['--tun-mtu-max 1600'])
+        openvpn.verify_openvpn_mtu(config)
+
+    def test_minimum_mtu(self):
+        with self.assertRaisesRegex(ConfigError, r'at\s+least\s+100'):
+            openvpn.verify_openvpn_mtu(self.config(mtu='99'))
+        openvpn.verify_openvpn_mtu(self.config(mtu='100'))
+
+    def test_ipv6_minimum_mtu(self):
+        server = {'subnet': ['192.0.2.0/24', '2001:db8::/64']}
+        local_address = {'2001:db8::1': {}}
+        for kwargs in [
+            {'server': server},
+            {'mode': 'site-to-site', 'local_address': local_address},
+        ]:
+            with self.subTest(**kwargs):
+                config = self.config(mtu='1279', **kwargs)
+                with self.assertRaisesRegex(ConfigError, r'minimum\s+MTU'):
+                    openvpn.verify_openvpn_mtu(config)
+                openvpn.verify_openvpn_mtu(self.config(mtu='1280', **kwargs))
+
+        # IPv4 alone may go below the IPv6 minimum
+        for kwargs in [
+            {'server': {'subnet': ['192.0.2.0/24']}},
+            {'mode': 'site-to-site', 'local_address': {'10.0.0.1': {}}},
+        ]:
+            with self.subTest(**kwargs):
+                openvpn.verify_openvpn_mtu(self.config(mtu='1200', **kwargs))
+
+
+if __name__ == '__main__':
+    unittest.main()