Skip to content

Commit

Permalink
Merge pull request #3639 from natali-rs1985/T5487-current
Browse files Browse the repository at this point in the history
openvpn: T5487:  Remove deprecated option --cipher for server and client mode
  • Loading branch information
dmbaturin authored Jun 13, 2024
2 parents e1916a1 + 0f669a2 commit 1abf323
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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 -->
20 changes: 16 additions & 4 deletions smoketest/scripts/cli/test_interfaces_openvpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ def test_openvpn_client_verify(self):
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'])

Expand Down Expand Up @@ -191,7 +197,7 @@ def test_openvpn_client_interfaces(self):
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'])
Expand Down Expand Up @@ -221,7 +227,7 @@ def test_openvpn_client_interfaces(self):
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)
Expand Down Expand Up @@ -328,6 +334,12 @@ def test_openvpn_server_verify(self):
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"
Expand Down Expand Up @@ -359,7 +371,7 @@ def test_openvpn_server_subnet_topology(self):
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])
Expand Down Expand Up @@ -404,7 +416,7 @@ def test_openvpn_server_subnet_topology(self):
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)
Expand Down
4 changes: 4 additions & 0 deletions src/conf_mode/interfaces_openvpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ' \
Expand Down
74 changes: 74 additions & 0 deletions src/migration-scripts/openvpn/1-to-2
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 1abf323

Please sign in to comment.