1 files changed, 27 insertions, 43 deletions

diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py
index 28a2cc22e..974aeea69 100755
--- a/src/conf_mode/interfaces-openvpn.py
+++ b/src/conf_mode/interfaces-openvpn.py
@@ -20,7 +20,6 @@ import re
 from jinja2 import FileSystemLoader, Environment
 from copy import deepcopy
 from sys import exit
-from stat import S_IRUSR
 from ipaddress import ip_address,ip_network,IPv4Interface
 from netifaces import interfaces
 from time import sleep
@@ -29,7 +28,7 @@ from shutil import rmtree
 from vyos.config import Config
 from vyos.defaults import directories as vyos_data_dir
 from vyos.ifconfig import VTunIf
-from vyos.util import call, is_bridge_member, chown, chmod_x
+from vyos.util import call, is_bridge_member, chown, chmod_600, chmod_755
 from vyos.validate import is_addr_assigned
 from vyos import ConfigError
 
@@ -98,23 +97,6 @@ def get_config_name(intf):
     cfg_file = f'/run/openvpn/{intf}.conf'
     return cfg_file
 
-def openvpn_mkdir(directory):
-    # create directory on demand
-    if not os.path.exists(directory):
-        os.mkdir(directory)
-
-    # fix permissions - corresponds to mode 755
-    chmod_x(directory)
-    chown(directory, user, group)
-
-def fixup_permission(filename, permission=S_IRUSR):
-    """
-    Check if the given file exists and change ownershit to root/vyattacfg
-    and appripriate file access permissions - default is user and group readable
-    """
-    if os.path.isfile(filename):
-        os.chmod(filename, permission)
-        chown(filename, 'root', 'vyattacfg')
 
 def checkCertHeader(header, filename):
     """
@@ -679,39 +661,42 @@ def generate(openvpn):
     interface = openvpn['intf']
     directory = os.path.dirname(get_config_name(interface))
 
-    # we can't know which clients were deleted, remove all client configs
-    if os.path.isdir(os.path.join(directory, 'ccd', interface)):
-        rmtree(os.path.join(directory, 'ccd', interface), ignore_errors=True)
+    # we can't know in advance which clients have been,
+    # remove all client configs
+    ccd_dir = os.path.join(directory, 'ccd', interface)
+    if os.path.isdir(ccd_dir):
+        rmtree(ccd_dir, ignore_errors=True)
 
     # create config directory on demand
-    openvpn_mkdir(directory)
-    # create status directory on demand
-    openvpn_mkdir(directory + '/status')
-    # create client config dir on demand
-    openvpn_mkdir(directory + '/ccd')
-    # crete client config dir per interface on demand
-    openvpn_mkdir(directory + '/ccd/' + interface)
+    directories = []
+    directories.append(f'{directory}/status')
+    directories.append(f'{directory}/ccd/{interface}')
+    for directory in directories:
+        if not os.path.exists(directory):
+            os.makedirs(directory, 0o755)
+        chown(directory, user, group)
 
     # Fix file permissons for keys
-    fixup_permission(openvpn['shared_secret_file'])
-    fixup_permission(openvpn['tls_key'])
+    fix_permissions = []
+    fix_permissions.append(openvpn['shared_secret_file'])
+    fix_permissions.append(openvpn['tls_key'])
 
     # Generate User/Password authentication file
+    user_auth_file = f'/tmp/openvpn-{interface}-pw'
     if openvpn['auth']:
-        auth_file = '/tmp/openvpn-{}-pw'.format(interface)
-        with open(auth_file, 'w') as f:
+        with open(user_auth_file, 'w') as f:
             f.write('{}\n{}'.format(openvpn['auth_user'], openvpn['auth_pass']))
-
-        fixup_permission(auth_file)
+        # also change permission on auth file
+        fix_permissions.append(user_auth_file)
 
     else:
         # delete old auth file if present
-        if os.path.isfile('/tmp/openvpn-{}-pw'.format(interface)):
-            os.remove('/tmp/openvpn-{}-pw'.format(interface))
+        if os.path.isfile(user_auth_file):
+            os.remove(user_auth_file)
 
     # Generate client specific configuration
     for client in openvpn['client']:
-        client_file = directory + '/ccd/' + interface + '/' + client['name']
+        client_file = os.path.join(ccd_dir, client['name'])
         tmpl = env.get_template('client.conf.tmpl')
         client_text = tmpl.render(client)
         with open(client_file, 'w') as f:
@@ -727,6 +712,10 @@ def generate(openvpn):
         f.write(config_text)
     chown(get_config_name(interface), user, group)
 
+    # Fixup file permissions
+    for file in fix_permissions:
+        chmod_600(file)
+
     return None
 
 def apply(openvpn):
@@ -745,11 +734,6 @@ def apply(openvpn):
         if os.path.isdir(ccd_dir):
             rmtree(ccd_dir, ignore_errors=True)
 
-        # cleanup auth file
-        user_auth_file = f'/tmp/openvpn-{interface}-pw'
-        if os.path.isfile(user_auth_file):
-            os.remove(user_auth_file)
-
         return None
 
     # On configuration change we need to wait for the 'old' interface to