Merge pull request #5397 from jeleel-muibi/t8999-openvpn-shared-secret-errors

openvpn: T8999: improve shared-secret reference errors

2 files changed, 55 insertions, 14 deletions

diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py
index 672f13bcb..922695ff2 100755
--- a/smoketest/scripts/cli/test_interfaces_openvpn.py
+++ b/smoketest/scripts/cli/test_interfaces_openvpn.py
@@ -938,6 +938,50 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             interface = f'vtun{ii}'
             self.assertNotIn(interface, interfaces())
 
+    def test_openvpn_missing_shared_secret_references(self):
+        interface = 'vtun5099'
+        path = base_path + [interface]
+        missing_key = 'NONEXISTENT'
+        pki_key_path = ['pki', 'openvpn', 'shared-secret', 'ovpn_test']
+
+        self.cli_delete(pki_key_path)
+
+        self.cli_set(path + ['mode', 'server'])
+        self.cli_set(path + ['server', 'subnet', '10.20.30.0/24'])
+        self.cli_set(path + ['tls', 'ca-certificate', 'ovpn_test'])
+        self.cli_set(path + ['tls', 'certificate', 'ovpn_test'])
+        self.cli_set(path + ['tls', 'dh-params', 'ovpn_test'])
+
+        self.cli_set(path + ['tls', 'auth-key', missing_key])
+        with self.assertRaisesRegex(
+            ConfigSessionError,
+            rf'Invalid "tls auth-key" value "{missing_key}" on OpenVPN interface\s+{interface}',
+        ):
+            self.cli_commit()
+
+        self.cli_delete(path + ['tls', 'auth-key'])
+        self.cli_set(path + ['tls', 'crypt-key', missing_key])
+        with self.assertRaisesRegex(
+            ConfigSessionError,
+            rf'Invalid "tls crypt-key" value "{missing_key}" on OpenVPN interface\s+{interface}',
+        ):
+            self.cli_commit()
+
+        self.cli_delete(path + ['tls'])
+        self.cli_delete(path + ['server'])
+        self.cli_set(path + ['mode', 'site-to-site'])
+        self.cli_set(path + ['local-address', '10.0.0.1'])
+        self.cli_set(path + ['remote-address', '10.0.0.2'])
+        self.cli_set(path + ['encryption', 'cipher', 'aes256'])
+        self.cli_set(path + ['shared-secret-key', missing_key])
+        with self.assertRaisesRegex(
+            ConfigSessionError,
+            rf'Invalid "shared-secret-key" value "{missing_key}" on OpenVPN interface\s+{interface}',
+        ):
+            self.cli_commit()
+
+        self.cli_set(pki_key_path + ['key'], ovpn_key_data.replace('\n', ''))
+
     def test_openvpn_server_ip_version(self):
         # Test the server mode behavior combined with each IP protocol version
 
diff --git a/src/conf_mode/interfaces_openvpn.py b/src/conf_mode/interfaces_openvpn.py
index 3cf416c3c..c8f4ebc17 100755
--- a/src/conf_mode/interfaces_openvpn.py
+++ b/src/conf_mode/interfaces_openvpn.py
@@ -323,6 +323,14 @@ def verify_dco(openvpn):
             if cipher is not None:
                 raise ConfigError(f'DCO does not support cipher "{cipher}"')
 
+
+def verify_shared_secret(pki: dict, interface: str, path: list, name: str):
+    if name not in (dict_search_args(pki, 'openvpn', 'shared_secret') or {}):
+        path_str = ' '.join(path)
+        raise ConfigError(
+            f'Invalid "{path_str}" value "{name}" on OpenVPN interface {interface}'
+        )
+
 def verify_pki(openvpn):
     pki = openvpn['pki']
     interface = openvpn['ifname']
@@ -340,11 +348,7 @@ def verify_pki(openvpn):
         raise ConfigError('PKI is not configured')
 
     if shared_secret_key:
-        if not dict_search_args(pki, 'openvpn', 'shared_secret'):
-            raise ConfigError('There are no openvpn shared-secrets in PKI configuration')
-
-        if shared_secret_key not in pki['openvpn']['shared_secret']:
-            raise ConfigError(f'Invalid shared-secret on openvpn interface {interface}')
+        verify_shared_secret(pki, interface, ['shared-secret-key'], shared_secret_key)
 
         # If PSK settings are correct, warn about its deprecation
         DeprecationWarning('OpenVPN shared-secret support will be removed in future '\
@@ -405,18 +409,11 @@ def verify_pki(openvpn):
             if dh_bits < 2048:
                 raise ConfigError(f'Minimum DH key-size is 2048 bits')
 
-
-        if 'auth_key' in tls or 'crypt_key' in tls:
-            if not dict_search_args(pki, 'openvpn', 'shared_secret'):
-                raise ConfigError('There are no openvpn shared-secrets in PKI configuration')
-
         if 'auth_key' in tls:
-            if tls['auth_key'] not in pki['openvpn']['shared_secret']:
-                raise ConfigError(f'Invalid auth-key on openvpn interface {interface}')
+            verify_shared_secret(pki, interface, ['tls', 'auth-key'], tls['auth_key'])
 
         if 'crypt_key' in tls:
-            if tls['crypt_key'] not in pki['openvpn']['shared_secret']:
-                raise ConfigError(f'Invalid crypt-key on openvpn interface {interface}')
+            verify_shared_secret(pki, interface, ['tls', 'crypt-key'], tls['crypt_key'])
 
 def verify(openvpn):
     if 'deleted' in openvpn: