openvpn: T5487: Remove eprecated option --cipher for server and client mode

4 files changed, 95 insertions, 5 deletions

diff --git a/interface-definitions/include/version/openvpn-version.xml.i b/interface-definitions/include/version/openvpn-version.xml.i
index b4dd742a3..e4eb13b7c 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='1'></syntaxVersion>
+<syntaxVersion component='openvpn' version='2'></syntaxVersion>
 <!-- include end -->
diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py
index e1e9a4ec7..9ca661e87 100755
--- a/smoketest/scripts/cli/test_interfaces_openvpn.py
+++ b/smoketest/scripts/cli/test_interfaces_openvpn.py
@@ -164,6 +164,12 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             self.cli_commit()
         self.cli_delete(path + ['shared-secret-key', 'ovpn_test'])
 
+        # check validate() - cannot specify "encryption cipher" in  client mode
+        self.cli_set(path + ['encryption', 'cipher', 'aes192gcm'])
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_delete(path + ['encryption', 'cipher'])
+
         self.cli_set(path + ['tls', 'ca-certificate', 'ovpn_test'])
         self.cli_set(path + ['tls', 'certificate', 'ovpn_test'])
 
@@ -191,7 +197,7 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             auth_hash = 'sha1'
 
             self.cli_set(path + ['device-type', 'tun'])
-            self.cli_set(path + ['encryption', 'cipher', 'aes256'])
+            self.cli_set(path + ['encryption', 'ncp-ciphers', 'aes256'])
             self.cli_set(path + ['hash', auth_hash])
             self.cli_set(path + ['mode', 'client'])
             self.cli_set(path + ['persistent-tunnel'])
@@ -221,7 +227,7 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             self.assertIn(f'remote {remote_host}', config)
             self.assertIn(f'persist-tun', config)
             self.assertIn(f'auth {auth_hash}', config)
-            self.assertIn(f'cipher AES-256-CBC', config)
+            self.assertIn(f'data-ciphers AES-256-CBC', config)
 
             # TLS options
             self.assertIn(f'ca /run/openvpn/{interface}_ca.pem', config)
@@ -328,6 +334,12 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             self.cli_commit()
         self.cli_delete(path + ['tls', 'dh-params'])
 
+        # check validate() - cannot specify "encryption cipher" in server mode
+        self.cli_set(path + ['encryption', 'cipher', 'aes256'])
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_delete(path + ['encryption', 'cipher'])
+
         # Now test the other path with tls role passive
         self.cli_set(path + ['tls', 'role', 'passive'])
         # check validate() - cannot specify "tcp-active" when "tls role" is "passive"
@@ -359,7 +371,7 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             port = str(2000 + ii)
 
             self.cli_set(path + ['device-type', 'tun'])
-            self.cli_set(path + ['encryption', 'cipher', 'aes192'])
+            self.cli_set(path + ['encryption', 'ncp-ciphers', 'aes192'])
             self.cli_set(path + ['hash', auth_hash])
             self.cli_set(path + ['mode', 'server'])
             self.cli_set(path + ['local-port', port])
@@ -404,7 +416,7 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             self.assertIn(f'persist-key', config)
             self.assertIn(f'proto udp', config) # default protocol
             self.assertIn(f'auth {auth_hash}', config)
-            self.assertIn(f'cipher AES-192-CBC', config)
+            self.assertIn(f'data-ciphers AES-192-CBC', config)
             self.assertIn(f'topology subnet', config)
             self.assertIn(f'lport {port}', config)
             self.assertIn(f'push "redirect-gateway def1"', config)
diff --git a/src/conf_mode/interfaces_openvpn.py b/src/conf_mode/interfaces_openvpn.py
index 627cc90ba..017010a61 100755
--- a/src/conf_mode/interfaces_openvpn.py
+++ b/src/conf_mode/interfaces_openvpn.py
@@ -515,6 +515,10 @@ def verify(openvpn):
                 print('Warning: using dh-params and EC keys simultaneously will ' \
                       'lead to DH ciphers being used instead of ECDH')
 
+        if dict_search('encryption.cipher', openvpn):
+            raise ConfigError('"encryption cipher" option is deprecated for TLS mode. '
+                              'Use "encryption ncp-ciphers" instead')
+
     if dict_search('encryption.cipher', openvpn) == 'none':
         print('Warning: "encryption none" was specified!')
         print('No encryption will be performed and data is transmitted in ' \
diff --git a/src/migration-scripts/openvpn/1-to-2 b/src/migration-scripts/openvpn/1-to-2
new file mode 100644
index 000000000..1f82a2128
--- /dev/null
+++ b/src/migration-scripts/openvpn/1-to-2
@@ -0,0 +1,74 @@
+#!/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/>.
+#
+# Removes --cipher option (deprecated) from OpenVPN configs
+# and moves it to --data-ciphers for server and client modes
+
+import sys
+
+from vyos.configtree import ConfigTree
+
+if len(sys.argv) < 2:
+    print("Must specify file name!")
+    sys.exit(1)
+
+file_name = sys.argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+config = ConfigTree(config_file)
+
+if not config.exists(['interfaces', 'openvpn']):
+    # Nothing to do
+    sys.exit(0)
+else:
+    ovpn_intfs = config.list_nodes(['interfaces', 'openvpn'])
+    for	i in ovpn_intfs:
+        # Remove 'encryption cipher' and add this value to 'encryption ncp-ciphers'
+        # for server and client mode.
+        # Site-to-site mode still can use --cipher option
+        cipher_path = ['interfaces', 'openvpn', i, 'encryption', 'cipher']
+        ncp_cipher_path = ['interfaces', 'openvpn', i, 'encryption', 'ncp-ciphers']
+        if config.exists(cipher_path):
+            if config.exists(['interfaces', 'openvpn', i, 'shared-secret-key']):
+                continue
+            cipher = config.return_value(cipher_path)
+            config.delete(cipher_path)
+            if cipher == 'none':
+                if not config.exists(ncp_cipher_path):
+                    config.delete(['interfaces', 'openvpn', i, 'encryption'])
+                continue
+
+            ncp_ciphers = []
+            if config.exists(ncp_cipher_path):
+                ncp_ciphers = config.return_values(ncp_cipher_path)
+                config.delete(ncp_cipher_path)
+
+            # need to add the deleted cipher at the first place in the list
+            if cipher in ncp_ciphers:
+                ncp_ciphers.remove(cipher)
+            ncp_ciphers.insert(0, cipher)
+
+            for c in ncp_ciphers:
+                config.set(ncp_cipher_path, value=c, replace=False)
+
+    try:
+        with open(file_name, 'w') as f:
+            f.write(config.to_string())
+    except OSError as e:
+        print("Failed to save the modified config: {}".format(e))
+        sys.exit(1)