Update system_user_dirs datasource (#3586)

* Optimize the code Signed-off-by: Jitka Obselkova <[email protected]> * Update code logic Signed-off-by: Jitka Obselkova <[email protected]> * Update code as recommended Signed-off-by: Jitka Obselkova <[email protected]>
RedHatInsights · Nov 10, 2022 · 90a9a41 · 90a9a41
1 parent 00fcbe3
commit 90a9a41
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 33 deletions.
diff --git a/insights/parsers/system_user_dirs.py b/insights/parsers/system_user_dirs.py
@@ -17,14 +17,14 @@ class SystemUserDirs(Parser):
 
     Sample output of this datasource is::
 
-        ["ca-certificates", "kmod", "sssd-ldap"]
+        ["httpd-core"]
 
     Examples:
 
         >>> type(system_user_dirs)
         <class 'insights.parsers.system_user_dirs.SystemUserDirs'>
         >>> system_user_dirs.packages
-        ['ca-certificates', 'kmod', 'sssd-ldap']
+        ['httpd-core']
     """
 
     def parse_content(self, content):

diff --git a/insights/specs/datasources/system_user_dirs.py b/insights/specs/datasources/system_user_dirs.py
@@ -15,10 +15,10 @@
 
 class LocalSpecs(Specs):
     """
-    Local spec used only by the system_user_dirs datasource
+    Local spec used only by the system_user_dirs datasource.
     """
     rpm_args = simple_command(
-        'rpm -qa --qf="[%{=NAME}; %{FILEMODES:perms}; %{FILEUSERNAME}; %{FILEGROUPNAME}\n]"',
+        'rpm -qa --nosignature --qf="[%{=NAME}; %{FILENAMES}; %{FILEMODES:perms}; %{FILEUSERNAME}; %{FILEGROUPNAME}\n]"',
         signum=signal.SIGTERM
     )
 
@@ -28,23 +28,23 @@ def get_shells():
     Returns all full pathnames of valid login shells without nologins.
     """
     with open("/etc/shells") as file:
-        return [line.strip() for line in file if "nologin" not in line]
+        return set(line.strip() for line in file if "nologin" not in line)
 
 
 def get_users():
     """
     Returns all users with shell specified in get_shells() except for root.
     """
     shells = get_shells()
-    users = []
+    users = set()
 
     for user in pwd.getpwall():
         name = user[0]
         shell = user[6]
 
         if name == "root" or (shell not in shells):
             continue
-        users.append(name)
+        users.add(name)
     return users
 
 
@@ -53,17 +53,19 @@ def get_groups(users):
     Returns all groups for users specified in get_users().
     Every user has at least one group with its name.
     """
-    groups = []
+    groups = set()
 
     for group in grp.getgrall():
         group_name = group[0]
         user_list = group[3]
 
         if group_name in users:
-            groups.append(group_name)  # group for an user of interest
-        for user in user_list:
-            if user.strip() in users:
-                groups.append(group_name)  # add groups containing a user
+            groups.add(group_name)
+        else:
+            for user in user_list:
+                if user.strip() in users:
+                    groups.add(group_name)
+                    break
     return groups
 
 
@@ -72,33 +74,44 @@ def system_user_dirs(broker):
     r"""
     Custom datasource for CVE-2021-35937, CVE-2021-35938, and CVE-2021-35939.
 
-    It collects package names from the ``rpm -qa --qf="[%{=NAME}; %{FILEMODES:perms};
-    %{FILEUSERNAME}; %{FILEGROUPNAME}\n]" command``, if the package has
-    at least one directory which is writable by a specific user/group or the others.
+    It collects package names from the ``rpm -qa --nosignature --qf="[%{=NAME}; %{FILENAMES};
+    %{FILEMODES:perms}; %{FILEUSERNAME}; %{FILEGROUPNAME}\n]" command``.
+
+    The output is a sorted list of all packages, which have at least one directory with files
+    inside, and this directory is writable by a specific user/group or the others.
 
     Raises:
         SkipComponent: Raised if no data is available
 
     Returns:
         List[str]: Sorted list of package names
     """
-    users = get_users()
-    groups = get_groups(users)
-    packages = set()
     content = broker[LocalSpecs.rpm_args].content
 
     if not content or "command not found" in content[0]:
         raise SkipComponent
 
+    users = get_users()
+    groups = get_groups(users)
+
+    dir_package = {}
+    dirs = set()
+
     for line in content:
-        name, perms, user, group = line.split("; ")
+        pkg_name, path_name, perms, user, group = line.split("; ")
         if perms[0] == "d":
             user_w = user in users and perms[2] == "w"
             group_w = group in groups and perms[5] == "w"
             others_w = perms[8] == "w"
-
             if user_w or group_w or others_w:
-                packages.add(name)
+                # Stores a writeable directory with its package
+                dir_package[path_name] = pkg_name
+        else:
+            # Stores a file directory for all files
+            dirs.add(path_name.rsplit('/', 1)[0])
+
+    # Stores a package if its associated file is in a writable directory
+    packages = set(dir_package[dir] for dir in dirs if dir in dir_package)
 
     if packages:
         return DatasourceProvider(

diff --git a/insights/tests/datasources/test_system_user_dirs.py b/insights/tests/datasources/test_system_user_dirs.py
@@ -8,17 +8,14 @@
 from insights.specs.datasources.system_user_dirs import LocalSpecs, system_user_dirs
 
 RPM_CMD = """
-sssd-ldap; drwxr-xr-x; apache; root
-ca-certificates; drwxrwxr-x; root; apache
-kmod; drwxr-xrwx; root; root
-libbsd; dr-xr-xr-x; apache; root
-libbsd; drwxr-xr-x; root; apache
-libbsd; drwxrwxr-x; root; root
-libbsd; -rwxr-xr-x; apache; root
-libbsd; lrwxrwxrwx; apache; apache
+httpd-core; /usr/share/doc/httpd-core; drwxr-xr-x; apache; root
+httpd-core; /usr/share/doc/httpd-core/CHANGES; -rw-r--r--; root; root
+postgresql-server; /var/lib/pgsql; drwx------; postgres; postgres
+polkit; /usr/lib/polkit-1; drwxrwxr-x; polkitd; polkitd
+polkit; /usr/lib/polkit-1/polkitd; -rwxr-xr-x; root; root
 """.strip()
 
-RPM_EXPECTED = ["ca-certificates", "kmod", "sssd-ldap"]
+RPM_EXPECTED = ["httpd-core"]
 
 RPM_BAD_CMD = "bash: rpm: command not found..."
 
@@ -28,11 +25,11 @@
 
 
 def get_users():
-    return ["apache"]
+    return ["apache", "postgres"]
 
 
 def get_groups(users):
-    return ["apache"]
+    return ["apache", "postgres"]
 
 
 @mock.patch("insights.specs.datasources.system_user_dirs.get_users", get_users)

diff --git a/insights/tests/parsers/test_system_user_dirs.py b/insights/tests/parsers/test_system_user_dirs.py
@@ -4,7 +4,7 @@
 from insights.parsers.system_user_dirs import SystemUserDirs
 from insights.tests import context_wrap
 
-PACKAGES = ["ca-certificates", "kmod", "sssd-ldap"]
+PACKAGES = ["httpd-core"]
 
 
 def test_system_user_dirs():