pki: T9149: fix broken OpenVPN shared-secret generated by op-mode

Since T9008, "generate pki openvpn shared-secret install" kept the comment
lines of the openvpn --genkey output as empty lines. The BEGIN marker of the
key was therefore stored as part of the key data, and the key file written
on commit carried it twice. OpenVPN refused to start every tunnel using it.

Comment-only lines are dropped again, as the previous grep -o pipeline did,
so only the hex key material is installed. Printed and file output lose the
leading blank lines as well.

The OpenVPN smoketest now installs a key through the op-mode command and
brings up a site-to-site tunnel with it, instead of only using a hardcoded
key.

2 files changed, 36 insertions, 3 deletions

diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py
index 0f4a2a2a3..e40d6e4c1 100755
--- a/smoketest/scripts/cli/test_interfaces_openvpn.py
+++ b/smoketest/scripts/cli/test_interfaces_openvpn.py
@@ -434,6 +434,38 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             sleep(1)
         self.assertTrue(is_systemd_service_running(f'openvpn@{interface}.service'))
 
+    def test_openvpn_site2site_generated_shared_secret(self):
+        # A key from "generate pki openvpn shared-secret install" must be stored
+        # without its BEGIN/END wrapper, as that is added again on commit
+        interface = 'vtun5000'
+        path = base_path + [interface]
+        key_name = 'ovpn_generated'
+        key_path = ['pki', 'openvpn', 'shared-secret', key_name]
+
+        # op-mode inherits the session environment and installs into it
+        out = self.op_mode(
+            ['generate', 'pki', 'openvpn', 'shared-secret', 'install', key_name]
+        )
+        self.assertNotIn('not in configure mode', out)
+
+        self.cli_set(path + ['mode', 'site-to-site'])
+        self.cli_set(path + ['local-address', '10.0.0.1'])
+        self.cli_set(path + ['remote-address', '192.168.0.1'])
+        self.cli_set(path + ['shared-secret-key', key_name])
+        self.cli_set(path + ['encryption', 'cipher', 'aes256'])
+        self.cli_commit()
+
+        key_file = read_file(f'/run/openvpn/{interface}_shared.key')
+        self.assertEqual(key_file.count('BEGIN OpenVPN Static key'), 1)
+
+        for _ in range(10):
+            if is_systemd_service_running(f'openvpn@{interface}.service'):
+                break
+            sleep(1)
+        self.assertTrue(is_systemd_service_running(f'openvpn@{interface}.service'))
+
+        self.cli_delete(key_path)
+
     # OVPN_CMD_PEER_GET carries GENL_ADMIN_PERM, hence the detour through
     # sudo. The probe swallows its own errors so a missing interface or a
     # refused dump reads as "no peer" with the reason attached, instead of
diff --git a/src/op_mode/pki.py b/src/op_mode/pki.py
index d53be1f92..55f307894 100755
--- a/src/op_mode/pki.py
+++ b/src/op_mode/pki.py
@@ -939,9 +939,10 @@ def generate_keypair(name, install=False, file=False):
 
 def generate_openvpn_key(name, install=False, file=False):
     raw_result = cmdl(['openvpn', '--genkey', 'secret', '/dev/stdout'])
-    # equivalent to the old `grep -o "^[^#]*"`, applied per line: keep
-    # everything before the first '#' on each line (comment lines become empty)
-    result = '\n'.join(line.split('#', 1)[0] for line in raw_result.splitlines())
+    # equivalent to the old `grep -o "^[^#]*"`: keep everything before the first
+    # '#' on each line and drop lines left empty (grep -o prints nothing for them)
+    stripped = (line.split('#', 1)[0] for line in raw_result.splitlines())
+    result = '\n'.join(line for line in stripped if line)
 
     if not result:
         print('Failed to generate OpenVPN key')