Merge pull request #3829 from vyos/mergify/bp/circinus/pr-3825

openvpn: T6591: deprecate OpenVPN server net30 topology (backport #3825)

4 files changed, 57 insertions, 7 deletions

diff --git a/interface-definitions/include/version/openvpn-version.xml.i b/interface-definitions/include/version/openvpn-version.xml.i
index e4eb13b7c..e03ad55c0 100644
--- a/interface-definitions/include/version/openvpn-version.xml.i
+++ b/interface-definitions/include/version/openvpn-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/openvpn-version.xml.i -->
-<syntaxVersion component='openvpn' version='2'></syntaxVersion>
+<syntaxVersion component='openvpn' version='3'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/interfaces_openvpn.xml.in b/interface-definitions/interfaces_openvpn.xml.in
index 23cc83e9a..1860523c2 100644
--- a/interface-definitions/interfaces_openvpn.xml.in
+++ b/interface-definitions/interfaces_openvpn.xml.in
@@ -589,25 +589,25 @@
                 <properties>
                   <help>Topology for clients</help>
                   <completionHelp>
-                    <list>net30 point-to-point subnet</list>
+                    <list>subnet point-to-point net30</list>
                   </completionHelp>
                   <valueHelp>
-                    <format>net30</format>
-                    <description>net30 topology</description>
+                    <format>subnet</format>
+                    <description>Subnet topology (recommended)</description>
                   </valueHelp>
                   <valueHelp>
                     <format>point-to-point</format>
                     <description>Point-to-point topology</description>
                   </valueHelp>
                   <valueHelp>
-                    <format>subnet</format>
-                    <description>Subnet topology</description>
+                    <format>net30</format>
+                    <description>net30 topology (deprecated)</description>
                   </valueHelp>
                   <constraint>
                     <regex>(subnet|point-to-point|net30)</regex>
                   </constraint>
                 </properties>
-                <defaultValue>net30</defaultValue>
+                <defaultValue>subnet</defaultValue>
               </leafNode>
               <node name="mfa">
                 <properties>
diff --git a/src/conf_mode/interfaces_openvpn.py b/src/conf_mode/interfaces_openvpn.py
index 017010a61..0dc76b39a 100755
--- a/src/conf_mode/interfaces_openvpn.py
+++ b/src/conf_mode/interfaces_openvpn.py
@@ -432,6 +432,13 @@ def verify(openvpn):
                                 if IPv6Address(client['ipv6_ip'][0]) in v6PoolNet:
                                     print(f'Warning: Client "{client["name"]}" IP {client["ipv6_ip"][0]} is in server IP pool, it is not reserved for this client.')
 
+        if 'topology' in openvpn['server']:
+            if openvpn['server']['topology'] == 'net30':
+                DeprecationWarning('Topology net30 is deprecated '\
+                                   'and will be removed in future VyOS versions. '\
+                                   'Switch to "subnet" or "p2p"'
+                )
+
         # add mfa users to the file the mfa plugin uses
         if dict_search('server.mfa.totp', openvpn):
             user_data = ''
diff --git a/src/migration-scripts/openvpn/2-to-3 b/src/migration-scripts/openvpn/2-to-3
new file mode 100644
index 000000000..0b9073ae6
--- /dev/null
+++ b/src/migration-scripts/openvpn/2-to-3
@@ -0,0 +1,43 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2024 VyOS maintainers and contributors
+#
+# 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/>.
+#
+# Adds an explicit old default for 'server topology'
+# to keep old configs working as before even though the default has changed.
+
+from vyos.configtree import ConfigTree
+
+def migrate(config: ConfigTree) -> None:
+    if not config.exists(['interfaces', 'openvpn']):
+        # Nothing to do
+        return
+
+    ovpn_intfs = config.list_nodes(['interfaces', 'openvpn'])
+    for	i in ovpn_intfs:
+        mode = config.return_value(['interfaces', 'openvpn', i, 'mode'])
+        if mode != 'server':
+            # If it's a client or a site-to-site OpenVPN interface,
+            # the topology setting is not applicable
+            # and will cause commit errors on load,
+            # so we must not change such interfaces.
+            continue
+        else:
+            # The default OpenVPN server topology was changed from net30 to subnet
+            # because net30 is deprecated and causes problems with Windows clients.
+            # We add 'net30' to old configs if topology is not set there
+            # to ensure that if anyone relies on net30, their configs work as before.
+            topology_path = ['interfaces', 'openvpn', i, 'server', 'topology']
+            if not config.exists(topology_path):
+                config.set(topology_path, value='net30', replace=False)