Fix permission reading/applying (mga#35275, mga#20847)

* config:
  - fix regex read of 3 or 4 characters for permisson settings
  - avoid failures when force/acl are undefined
  - fix s.decode() call ?
* libmsec: drop utf8 decoding, log and guard against invalid entries
* msecgui: apply the permissions when saving the configuration
* msecperms: read in the acl entries

4 files changed, 58 insertions, 15 deletions

diff --git a/src/msec/config.py b/src/msec/config.py
index ef2946c..e77e9fd 100755
--- a/src/msec/config.py
+++ b/src/msec/config.py
@@ -172,12 +172,10 @@ def merge_with_baselevel(log, config, base_level, load_func, root=''):
 
 
 def to_utf8(s):
-    """ Returs string after decoding if needed """
-    try:
-        s.decode()
-        return s
-    except:
-        return str(s).decode("utf-8")
+    """ Returns string, decodes bytes if necessary """
+    if isinstance(s, bytes):
+        return s.decode('utf-8')
+    return str(s)
 
 # {{{ MsecConfig
 class MsecConfig:
@@ -406,7 +404,7 @@ class PermConfig(MsecConfig):
         self.options_order = []
         self.comments = []
         self.log = log
-        self.regexp = re.compile(r"^([^\s]*)\s*([a-z]*)\.([a-z]*)\s*([\d]?\d\d\d|current)\s*(force)?\s?([^\s]*)$")
+        self.regexp = re.compile(r"^([^\s]*)\s*([a-z]*)\.([a-z]*)\s*(\d{3,4}|current)\s*(force)?\s?([^\s]*)$")
 
     def merge(self, newconfig, overwrite=False):
         """Merges parameters from newconfig to current config"""
@@ -435,6 +433,8 @@ class PermConfig(MsecConfig):
         except:
             self.log.error(_("Unable to load configuration file %s: %s") % (self.config, sys.exc_info()[1]))
             return False
+        # Look up for pattern: user1:acl,user2:acl
+        acl_re = re.compile(r'^([A-Za-z][A-Za-z0-9._-]+:[^:,]+)(,([A-Za-z][A-Za-z0-9._-]*)+:[^:,]+)*$')
         for line in fd.readlines():
             line = line.strip()
             if not line:
@@ -448,8 +448,27 @@ class PermConfig(MsecConfig):
                 if res:
                     if len(res[0]) == 6:
                         file, user, group, perm, force, acl = res[0]
-                    self.options[file] = (user, group, perm, force, acl)
-                    self.options_order.append(file)
+
+                        # validate force field
+                        if force not in ('force', ''):
+                            self.log.warn(_("Invalid force value '%s' for '%s', ignoring") % (force, file))
+                            force = ''
+                        # validate acl field
+                        if acl and not acl_re.match(acl):
+                            self.log.warn(_("Invalid acl value '%s' for '%s', ignoring") % (acl, file))
+                            acl = ''
+                        # discard fully no-op entries: all current/empty and no force/acl
+                        user_noop = user in ('current', '')
+                        group_noop = group in ('current', '')
+                        perm_noop = perm in ('current', '')
+                        if user_noop and group_noop and perm_noop and not acl:
+                            self.log.debug(_("Skipping no-op entry for '%s'") % file)
+                            continue
+
+                        self.options[file] = (user, group, perm, force, acl)
+                        self.options_order.append(file)
+                    else:
+                        self.log.warn(_("Unexpected format in line: %s") % line)
             except:
                 traceback.print_exc()
                 self.log.warn(_("Bad config option: %s") % line)
@@ -462,10 +481,15 @@ class PermConfig(MsecConfig):
         return self.options_order
 
     def get(self, option, default=None):
-        """Gets a configuration option, or defines it if not defined"""
+        """Gets a configuration option, or defines it if not defined.
+        Always returns a 5-tuple (user, group, perm, force, acl) or the default."""
         if option not in self.options:
             self.set(option, default)
-        return self.options[option]
+        value = self.options[option]
+        # guard against None or removed entries — return a safe no-op tuple
+        if value is None:
+            return ('current', 'current', 'current', '', '')
+        return value
 
     def set(self, option, value):
         """Sets a configuration option"""
diff --git a/src/msec/libmsec.py b/src/msec/libmsec.py
index d4fa75b..a683558 100755
--- a/src/msec/libmsec.py
+++ b/src/msec/libmsec.py
@@ -708,8 +708,7 @@ class MSEC:
             if plugin_ in self.plugins:
                 plugin = self.plugins[plugin_]
             else:
-                self.log.info(_("Plugin %s not found") % to_utf8(plugin_))
-                return self.log.info
+                self.log.info(_("Plugin %s not found") % plugin_)
                 return None
         try:
             func = getattr(plugin, callback)
@@ -923,7 +922,19 @@ class PERMS:
         If files_to_check is specified, only the specified files are checked.'''
 
         for file in perms.list_options():
-            user_s, group_s, perm_s, force, acl = perms.get(file)
+            entry = perms.get(file)
+            if not entry or len(entry) != 5:
+                self.log.warn(_("Skipping malformed entry for '%s'") % file)
+                continue
+            user_s, group_s, perm_s, force, acl = entry
+
+            # skip entries where nothing would actually change
+            user_noop = user_s in ('current', '')
+            group_noop = group_s in ('current', '')
+            perm_noop = perm_s in ('current', '')
+            if user_noop and group_noop and perm_noop and not acl:
+                self.log.debug("Skipping no-op entry for '%s'" % file)
+                continue
 
             # permission
             if perm_s == 'current':
diff --git a/src/msec/msecgui.py b/src/msec/msecgui.py
index 9f4afe4..eeadabb 100755
--- a/src/msec/msecgui.py
+++ b/src/msec/msecgui.py
@@ -446,7 +446,9 @@ class MsecGui(Gtk.Window):
         self.msec.commit(True)
 
         # saving permissions
+        self.perms.check_perms(self.permconfig)
         self.permconfig.save(standard_permconf)
+        self.perms.commit(really_commit=True, enforce=False)
 
         self.reload_config()
 
@@ -462,6 +464,12 @@ class MsecGui(Gtk.Window):
         self.permconfig.reset()
         self.permconfig.load()
         config.merge_with_baselevel(log, self.permconfig, self.msecconfig.get_base_level(), config.load_default_perms, root='')
+        # merge legacy perm.local if present
+        perm_local_path = os.path.join(config.MSEC_DIR, "perm.local")
+        if os.access(perm_local_path, os.R_OK):
+            perm_local = config.PermConfig(self.log, config=perm_local_path)
+            perm_local.load()
+            self.permconfig.merge(perm_local, overwrite=True)
         # exceptions
         self.exceptions.reset()
         self.exceptions.load()
diff --git a/src/msec/msecperms.py b/src/msec/msecperms.py
index 2d0887a..a22cb88 100755
--- a/src/msec/msecperms.py
+++ b/src/msec/msecperms.py
@@ -96,7 +96,7 @@ if __name__ == "__main__":
                 print(_("Invalid security level '%s'.") % level, file=sys.stderr)
                 sys.exit(1)
             for file in params:
-                user, group, perm, force = permconf.get(file)
+                user, group, perm, force, acl = permconf.get(file)
                 if force:
                     print("!! forcing permissions on %s" % file)
                 print("%s: %s.%s perm %s" % (file, user, group, perm))