openvpn: T8264: migrate away DCO the Kernel module cannot serve

"offload dco" was accepted next to tap, static keys, compression, CBC
ciphers and raw options OpenVPN refuses to offload, because 2.6 ignored the
module altogether. Those combinations now fail verify(), which would break
config load on upgrade, so drop the request instead.

The migration has to mirror verify_dco() exactly: anything it keeps that
verify() turns away leaves a router unable to commit its own configuration.

2 files changed, 309 insertions, 0 deletions

diff --git a/src/migration-scripts/openvpn/5-to-6 b/src/migration-scripts/openvpn/5-to-6
new file mode 100644
index 000000000..46c9aaeb7
--- /dev/null
+++ b/src/migration-scripts/openvpn/5-to-6
@@ -0,0 +1,95 @@
+# Copyright VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+# T8264: "offload dco" used to be accepted next to settings the Kernel module
+# can not serve, because OpenVPN 2.6 silently ignored the module. Now that the
+# offload is real those combinations are rejected, so drop the request from
+# configurations that would otherwise fail to load.
+
+from vyos.configtree import ConfigTree
+
+base = ['interfaces', 'openvpn']
+dco_ciphers = ['aes128gcm', 'aes192gcm', 'aes256gcm']
+dco_incompatible_options = [
+    'comp-lzo',
+    'disable-dco',
+    'fragment',
+    'http-proxy',
+    'management-query-proxy',
+    'socks-proxy',
+]
+dco_conditional_options = {
+    'allow-compression': 'no',
+    'compress': 'migrate',
+}
+
+
+def _offloadable(options: list) -> bool:
+    """Whether every raw option leaves the data path offloadable."""
+    for option in options:
+        tmp = option.split()
+        if not tmp:
+            continue
+        keyword = tmp[0].lstrip('-')
+        if keyword in dco_incompatible_options:
+            return False
+        keep = dco_conditional_options.get(keyword)
+        if keep is not None and tmp[1:] != [keep]:
+            return False
+    return True
+
+
+def _value(config: ConfigTree, path: list):
+    # return_value() raises on a missing path, and nodes carrying a CLI
+    # default are usually absent from a saved configuration
+    return config.return_value(path) if config.exists(path) else None
+
+
+def migrate(config: ConfigTree) -> None:
+    if not config.exists(base):
+        return
+
+    for interface in config.list_nodes(base):
+        path = base + [interface]
+        if not config.exists(path + ['offload', 'dco']):
+            continue
+
+        ciphers = []
+        if config.exists(path + ['encryption', 'data-ciphers']):
+            ciphers += config.return_values(path + ['encryption', 'data-ciphers'])
+        fallback = _value(config, path + ['encryption', 'data-ciphers-fallback'])
+        if fallback:
+            ciphers.append(fallback)
+
+        options = []
+        if config.exists(path + ['openvpn-option']):
+            options = config.return_values(path + ['openvpn-option'])
+
+        topology = None
+        if _value(config, path + ['mode']) == 'server':
+            topology = _value(config, path + ['server', 'topology'])
+
+        if (
+            _value(config, path + ['device-type']) == 'tap'
+            or config.exists(path + ['shared-secret-key'])
+            or config.exists(path + ['use-lzo-compression'])
+            or (topology is not None and topology != 'subnet')
+            or any(cipher not in dco_ciphers for cipher in ciphers)
+            or not _offloadable(options)
+        ):
+            offload = path + ['offload']
+            config.delete(offload + ['dco'])
+            if config.exists(offload) and not config.list_nodes(offload):
+                config.delete(offload)
diff --git a/src/tests/test_openvpn_migration.py b/src/tests/test_openvpn_migration.py
new file mode 100644
index 000000000..bd0bbdbbe
--- /dev/null
+++ b/src/tests/test_openvpn_migration.py
@@ -0,0 +1,214 @@
+# Copyright VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+import importlib.machinery
+import importlib.util
+from unittest import TestCase
+
+from vyos.configtree import ConfigTree
+
+migration_script = 'src/migration-scripts/openvpn/5-to-6'
+
+dco = ['interfaces', 'openvpn', 'vtun10', 'offload', 'dco']
+
+
+def load_migrate():
+    loader = importlib.machinery.SourceFileLoader('openvpn_5_to_6', migration_script)
+    spec = importlib.util.spec_from_loader(loader.name, loader)
+    module = importlib.util.module_from_spec(spec)
+    loader.exec_module(module)
+    return module.migrate
+
+
+def config_tree(body):
+    return ConfigTree(
+        'interfaces {\n'
+        '    openvpn vtun10 {\n'
+        f'{body}'
+        '        offload {\n'
+        '            dco\n'
+        '        }\n'
+        '    }\n'
+        '}\n'
+    )
+
+
+class TestOpenVPNMigration(TestCase):
+    def setUp(self):
+        self.migrate = load_migrate()
+
+    def test_empty_config(self):
+        # must not raise when no OpenVPN interface is configured
+        config = ConfigTree('system {\n    host-name vyos\n}\n')
+        self.migrate(config)
+        self.assertFalse(config.exists(['interfaces', 'openvpn']))
+
+    def test_supported_dco_is_kept(self):
+        config = config_tree(
+            '        mode server\n'
+            '        device-type tun\n'
+            '        encryption {\n'
+            '            data-ciphers aes256gcm\n'
+            '        }\n'
+            '        server {\n'
+            '            topology subnet\n'
+            '        }\n'
+        )
+        self.migrate(config)
+        self.assertTrue(config.exists(dco))
+
+    def test_compress_migrate_keeps_dco(self):
+        # the option OpenVPN suggests for keeping the offload with legacy
+        # clients must not cost the interface its offload
+        config = config_tree(
+            '        mode server\n        openvpn-option "--compress migrate"\n'
+        )
+        self.migrate(config)
+        self.assertTrue(config.exists(dco))
+
+    def test_compress_algorithm_drops_dco(self):
+        config = config_tree(
+            '        mode server\n        openvpn-option "--compress lzo"\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_allow_compression_no_keeps_dco(self):
+        config = config_tree(
+            '        mode server\n        openvpn-option "--allow-compression no"\n'
+        )
+        self.migrate(config)
+        self.assertTrue(config.exists(dco))
+
+    def test_allow_compression_asym_drops_dco(self):
+        config = config_tree(
+            '        mode server\n        openvpn-option "--allow-compression asym"\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_incompatible_option_drops_dco(self):
+        config = config_tree(
+            '        mode server\n        openvpn-option "--fragment 1300"\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_harmless_option_keeps_dco(self):
+        config = config_tree(
+            '        mode server\n        openvpn-option "--persist-tun"\n'
+        )
+        self.migrate(config)
+        self.assertTrue(config.exists(dco))
+
+    def test_interfaces_are_independent(self):
+        # an interface without "offload" must be left alone, and dropping the
+        # offload from one must not disturb another
+        config = ConfigTree(
+            'interfaces {\n'
+            '    openvpn vtun10 {\n'
+            '        mode server\n'
+            '        device-type tap\n'
+            '        offload {\n'
+            '            dco\n'
+            '        }\n'
+            '    }\n'
+            '    openvpn vtun11 {\n'
+            '        mode server\n'
+            '    }\n'
+            '    openvpn vtun12 {\n'
+            '        mode server\n'
+            '        offload {\n'
+            '            dco\n'
+            '        }\n'
+            '    }\n'
+            '}\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(['interfaces', 'openvpn', 'vtun10', 'offload']))
+        self.assertTrue(config.exists(['interfaces', 'openvpn', 'vtun11']))
+        self.assertFalse(config.exists(['interfaces', 'openvpn', 'vtun11', 'offload']))
+        self.assertTrue(
+            config.exists(['interfaces', 'openvpn', 'vtun12', 'offload', 'dco'])
+        )
+
+    def test_minimal_dco_is_kept(self):
+        # a plain tun server spells out neither device-type nor topology,
+        # which is the usual shape of a saved configuration
+        config = config_tree('        mode server\n')
+        self.migrate(config)
+        self.assertTrue(config.exists(dco))
+
+    def test_tap_drops_dco(self):
+        config = config_tree('        mode server\n        device-type tap\n')
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+        # the now empty parent must go too
+        self.assertFalse(config.exists(dco[:-1]))
+
+    def test_shared_secret_drops_dco(self):
+        config = config_tree(
+            '        mode site-to-site\n        shared-secret-key ovpn_test\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_lzo_drops_dco(self):
+        config = config_tree('        mode server\n        use-lzo-compression\n')
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_cbc_cipher_drops_dco(self):
+        config = config_tree(
+            '        mode server\n'
+            '        encryption {\n'
+            '            data-ciphers aes256gcm\n'
+            '            data-ciphers aes256\n'
+            '        }\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_cbc_fallback_drops_dco(self):
+        config = config_tree(
+            '        mode site-to-site\n'
+            '        encryption {\n'
+            '            data-ciphers-fallback aes256\n'
+            '        }\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_non_subnet_topology_drops_dco(self):
+        config = config_tree(
+            '        mode server\n'
+            '        server {\n'
+            '            topology net30\n'
+            '        }\n'
+        )
+        self.migrate(config)
+        self.assertFalse(config.exists(dco))
+
+    def test_topology_ignored_outside_server_mode(self):
+        # "server topology" is meaningless for a client, so it must not
+        # cost that interface its offload
+        config = config_tree(
+            '        mode client\n'
+            '        server {\n'
+            '            topology net30\n'
+            '        }\n'
+        )
+        self.migrate(config)
+        self.assertTrue(config.exists(dco))