From ee518783c05f7c9e65e7c13a051797afca1e55c3 Mon Sep 17 00:00:00 2001
From: Marcus Burghardt <maburgha@redhat.com>
Date: Tue, 29 Mar 2022 16:20:48 -0300
Subject: [PATCH 1/3] Performance improvements for file_owner template

This template used to hit performance issues when directories were
recursively assessed. The OVAL logic was slightly changed to improve the
performance around 50% or more for some rules.
---
 shared/templates/file_owner/oval.template     | 20 ++++++++-----------
 .../file_owner/tests/correct_owner.pass.sh    |  1 -
 .../file_owner/tests/incorrect_owner.fail.sh  |  1 -
 .../tests/missing_file_test.pass.sh           |  1 -
 4 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/shared/templates/file_owner/oval.template b/shared/templates/file_owner/oval.template
index 090ea49863a6..82308fbd4d4a 100644
--- a/shared/templates/file_owner/oval.template
+++ b/shared/templates/file_owner/oval.template
@@ -13,22 +13,12 @@
   {{% endif %}}
     </criteria>
   </definition>
-  {{%- if MISSING_FILE_PASS -%}}
-    {{# Any number of files can exist, from zero to any #}}
-    {{% set FILE_EXISTENCE = "any_exist" %}}
-  {{%- else -%}}
-    {{# All defined files must exist. When using regex, at least one file must match #}}
-    {{% set FILE_EXISTENCE = "all_exist" %}}
-  {{%- endif -%}}
 
   {{% for filepath in FILEPATH %}}
-  <unix:file_test check="all" check_existence="{{{ FILE_EXISTENCE }}}" comment="Testing user ownership of {{{ filepath }}}" id="test_file_owner{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
+  <unix:file_test check="all" check_existence="none_exist" comment="Testing user ownership of {{{ filepath }}}" id="test_file_owner{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
     <unix:object object_ref="object_file_owner{{{ FILEID }}}_{{{ loop.index0 }}}" />
-    <unix:state state_ref="state_file_owner{{{ FILEID }}}_uid_{{{ FILEUID }}}_{{{ loop.index0 }}}" />
   </unix:file_test>
-  <unix:file_state id="state_file_owner{{{ FILEID }}}_uid_{{{ FILEUID }}}_{{{ loop.index0 }}}" version="1">
-    <unix:user_id datatype="int">{{{ FILEUID }}}</unix:user_id>
-  </unix:file_state>
+
   <unix:file_object comment="{{{ filepath }}}" id="object_file_owner{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
     {{%- if IS_DIRECTORY -%}}
       {{%- if RECURSIVE %}}
@@ -45,8 +35,14 @@
       <unix:filepath{{% if FILEPATH_IS_REGEX %}} operation="pattern match"{{% endif %}}>{{{ filepath }}}</unix:filepath>
     {{%- endif %}}
     <filter action="exclude">symlink_file_owner{{{ FILEID }}}_uid_{{{ FILEUID }}}</filter>
+    <filter action="exclude">state_file_owner{{{ FILEID }}}_uid_{{{ FILEUID }}}_{{{ loop.index0 }}}</filter>
   </unix:file_object>
+
+  <unix:file_state id="state_file_owner{{{ FILEID }}}_uid_{{{ FILEUID }}}_{{{ loop.index0 }}}" version="1">
+    <unix:user_id datatype="int">{{{ FILEUID }}}</unix:user_id>
+  </unix:file_state>
   {{% endfor %}}
+
   <unix:file_state id="symlink_file_owner{{{ FILEID }}}_uid_{{{ FILEUID }}}" version="1">
     <unix:type operation="equals">symbolic link</unix:type>
   </unix:file_state>
diff --git a/shared/templates/file_owner/tests/correct_owner.pass.sh b/shared/templates/file_owner/tests/correct_owner.pass.sh
index c22c6a58f1c1..b7dfcb8865cb 100644
--- a/shared/templates/file_owner/tests/correct_owner.pass.sh
+++ b/shared/templates/file_owner/tests/correct_owner.pass.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 {{% for path in FILEPATH %}}
 {{% if IS_DIRECTORY and FILE_REGEX %}}
diff --git a/shared/templates/file_owner/tests/incorrect_owner.fail.sh b/shared/templates/file_owner/tests/incorrect_owner.fail.sh
index 8f402412e15d..04012bfd5fc4 100644
--- a/shared/templates/file_owner/tests/incorrect_owner.fail.sh
+++ b/shared/templates/file_owner/tests/incorrect_owner.fail.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 useradd testuser_123
 
diff --git a/shared/templates/file_owner/tests/missing_file_test.pass.sh b/shared/templates/file_owner/tests/missing_file_test.pass.sh
index 4e3683f9dcf1..45f176226d94 100644
--- a/shared/templates/file_owner/tests/missing_file_test.pass.sh
+++ b/shared/templates/file_owner/tests/missing_file_test.pass.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 {{% for path in FILEPATH %}}
     {{% if MISSING_FILE_PASS %}}

From 54fd22f3182b7a380126510f917752aa4b64199f Mon Sep 17 00:00:00 2001
From: Marcus Burghardt <maburgha@redhat.com>
Date: Tue, 29 Mar 2022 16:30:19 -0300
Subject: [PATCH 2/3] Performance improvements for file_groupowner template

Like file_owner template, file_groupowner was also prone to hit performance
issues when directories were recursively assessed. The OVAL logic was slightly
changed to increase the performance for some rules.
---
 .../templates/file_groupowner/oval.template   | 21 +++++++------------
 .../tests/correct_groupowner.pass.sh          |  1 -
 .../tests/incorrect_groupowner.fail.sh        |  1 -
 .../tests/missing_file_test.pass.sh           |  1 -
 4 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/shared/templates/file_groupowner/oval.template b/shared/templates/file_groupowner/oval.template
index 276965ad77ca..5c1a8bfdf9a1 100644
--- a/shared/templates/file_groupowner/oval.template
+++ b/shared/templates/file_groupowner/oval.template
@@ -13,23 +13,12 @@
    {{% endif %}}
     </criteria>
   </definition>
-  {{%- if MISSING_FILE_PASS -%}}
-    {{# Any number of files can exist, from zero to any #}}
-    {{% set FILE_EXISTENCE = "any_exist" %}}
-  {{%- else -%}}
-    {{# All defined files must exist. When using regex, at least one file must match #}}
-    {{% set FILE_EXISTENCE = "all_exist" %}}
-  {{%- endif -%}}
-
 
   {{% for filepath in FILEPATH %}}
-  <unix:file_test check="all" check_existence="{{{ FILE_EXISTENCE }}}" comment="Testing group ownership of {{{ filepath }}}" id="test_file_groupowner{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
+  <unix:file_test check="all" check_existence="none_exist" comment="Testing group ownership of {{{ filepath }}}" id="test_file_groupowner{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
     <unix:object object_ref="object_file_groupowner{{{ FILEID }}}_{{{ loop.index0 }}}" />
-    <unix:state state_ref="state_file_groupowner{{{ FILEID }}}_gid_{{{ FILEGID }}}_{{{ loop.index0 }}}" />
   </unix:file_test>
-  <unix:file_state id="state_file_groupowner{{{ FILEID }}}_gid_{{{ FILEGID }}}_{{{ loop.index0 }}}" version="1">
-    <unix:group_id datatype="int">{{{ FILEGID }}}</unix:group_id>
-  </unix:file_state>
+
   <unix:file_object comment="{{{ filepath }}}" id="object_file_groupowner{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
     {{%- if IS_DIRECTORY -%}}
       {{%- if RECURSIVE %}}
@@ -46,8 +35,14 @@
       <unix:filepath{{% if FILEPATH_IS_REGEX %}} operation="pattern match"{{% endif %}}>{{{ filepath }}}</unix:filepath>
     {{%- endif %}}
     <filter action="exclude">symlink_file_groupowner{{{ FILEID }}}_uid_{{{ FILEGID }}}</filter>
+    <filter action="exclude">state_file_groupowner{{{ FILEID }}}_gid_{{{ FILEGID }}}_{{{ loop.index0 }}}</filter>
   </unix:file_object>
+
+  <unix:file_state id="state_file_groupowner{{{ FILEID }}}_gid_{{{ FILEGID }}}_{{{ loop.index0 }}}" version="1">
+    <unix:group_id datatype="int">{{{ FILEGID }}}</unix:group_id>
+  </unix:file_state>
   {{% endfor %}}
+
   <unix:file_state id="symlink_file_groupowner{{{ FILEID }}}_uid_{{{ FILEGID }}}" version="1">
     <unix:type operation="equals">symbolic link</unix:type>
   </unix:file_state>
diff --git a/shared/templates/file_groupowner/tests/correct_groupowner.pass.sh b/shared/templates/file_groupowner/tests/correct_groupowner.pass.sh
index f02a02ebcaed..6f209457054a 100644
--- a/shared/templates/file_groupowner/tests/correct_groupowner.pass.sh
+++ b/shared/templates/file_groupowner/tests/correct_groupowner.pass.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 {{% for path in FILEPATH %}}
 {{% if IS_DIRECTORY and FILE_REGEX %}}
diff --git a/shared/templates/file_groupowner/tests/incorrect_groupowner.fail.sh b/shared/templates/file_groupowner/tests/incorrect_groupowner.fail.sh
index 15857443e840..dce6c878376f 100644
--- a/shared/templates/file_groupowner/tests/incorrect_groupowner.fail.sh
+++ b/shared/templates/file_groupowner/tests/incorrect_groupowner.fail.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 groupadd group_test
 
diff --git a/shared/templates/file_groupowner/tests/missing_file_test.pass.sh b/shared/templates/file_groupowner/tests/missing_file_test.pass.sh
index 015ff98c99d5..479678a7fed9 100644
--- a/shared/templates/file_groupowner/tests/missing_file_test.pass.sh
+++ b/shared/templates/file_groupowner/tests/missing_file_test.pass.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 {{% for path in FILEPATH %}}
     {{% if MISSING_FILE_PASS %}}

From 670bae0022ff22fe552675b6052522b29c18dbf7 Mon Sep 17 00:00:00 2001
From: Marcus Burghardt <maburgha@redhat.com>
Date: Tue, 29 Mar 2022 18:53:29 -0300
Subject: [PATCH 3/3] Change the file_permissions template to improve
 performance

OVAL assessment was working with include filter to collect file objects
and in a second step, checking these objects against a file state. This
approach was collecting all file objects regardless of their permissions.
This means that many objects may be evaluated unnecessarily.

The logic was changed to exclude already compliant files from the file
objects list. The permission check itself is expensive, limiting the
performance gain. Even so the performance was slightly improved.
---
 .../templates/file_permissions/oval.template  | 42 ++++---------------
 shared/templates/file_permissions/template.py |  5 +--
 .../tests/correct_permissions.pass.sh         |  1 -
 .../tests/lenient_permissions.fail.sh         |  3 +-
 4 files changed, 11 insertions(+), 40 deletions(-)

diff --git a/shared/templates/file_permissions/oval.template b/shared/templates/file_permissions/oval.template
index a22bb1046877..f314b5757b9e 100644
--- a/shared/templates/file_permissions/oval.template
+++ b/shared/templates/file_permissions/oval.template
@@ -1,19 +1,3 @@
-{{%- if ALLOW_STRICTER_PERMISSIONS %}}
-    {{#
-        When allowing stricter permissions, the check looks for files/directories
-        that have any extra permission bit set. The permissions mode (STATEMODE)
-        is negated (inverted) and becomes a mask for permissions that should not be set.
-    #}}
-  {{% set FILE_EXISTENCE = "at_least_one_exists" %}}
-{{%- else -%}}
-  {{%- if MISSING_FILE_PASS -%}}
-    {{# Any number of files can exist, from zero to any #}}
-    {{% set FILE_EXISTENCE = "any_exist" %}}
-  {{%- else -%}}
-    {{# All defined files must exist. When using regex, at least one file must match #}}
-    {{% set FILE_EXISTENCE = "all_exist" %}}
-  {{%- endif -%}}
-{{%- endif -%}}
 <def-group>
   <definition class="compliance" id="{{{ _RULE_ID }}}" version="1">
   {{% if FILEPATH is not string %}}
@@ -22,28 +6,24 @@
       ") }}}
     <criteria>
   {{% for filepath in FILEPATH %}}
-      <criterion comment="Check file mode of {{{ filepath }}}" test_ref="test_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}"{{{ ' negate="true"' if ALLOW_STRICTER_PERMISSIONS }}}/>
+      <criterion comment="Check file mode of {{{ filepath }}}" test_ref="test_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}"/>
   {{% endfor %}}
   {{% else %}}
     {{{ oval_metadata("This test makes sure that " +  FILEPATH + " has mode " + FILEMODE + ".
       If the target file or directory has an extended ACL, then it will fail the mode check.
       ") }}}
     <criteria>
-      <criterion comment="Check file mode of {{{ FILEPATH }}}" test_ref="test_file_permissions{{{ FILEID }}}"{{{ ' negate="true"' if ALLOW_STRICTER_PERMISSIONS }}}/>
+      <criterion comment="Check file mode of {{{ FILEPATH }}}" test_ref="test_file_permissions{{{ FILEID }}}"/>
   {{% endif %}}
     </criteria>
   </definition>
 
   {{% for filepath in FILEPATH %}}
-  <unix:file_test check="all" check_existence="{{{ FILE_EXISTENCE }}}" comment="Testing mode of {{{ filepath }}}" id="test_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}" version="2">
+  <unix:file_test check="all" check_existence="none_exist" comment="Testing mode of {{{ filepath }}}" id="test_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}" version="3">
     <unix:object object_ref="object_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}" />
-    <unix:state state_ref="state_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}_mode_{{{ 'not_' if ALLOW_STRICTER_PERMISSIONS }}}{{{ FILEMODE }}}" />
   </unix:file_test>
-  <unix:file_state id="state_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}_mode_{{{ 'not_' if ALLOW_STRICTER_PERMISSIONS }}}{{{ FILEMODE }}}"{{{ ' operator="OR"' if ALLOW_STRICTER_PERMISSIONS }}} version="2">
-      {{{ STATEMODE | indent(6) }}}
-  </unix:file_state>
-  <unix:file_object comment="{{{ filepath }}}" id="object_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
 
+  <unix:file_object comment="{{{ filepath }}}" id="object_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}" version="1">
     {{%- if IS_DIRECTORY %}}
       {{%- if RECURSIVE %}}
       <unix:path operation="pattern match">^{{{ filepath[:-1] }}}</unix:path>
@@ -58,17 +38,13 @@
     {{%- else %}}
       <unix:filepath{{% if FILEPATH_IS_REGEX %}} operation="pattern match"{{% endif %}}>{{{ filepath }}}</unix:filepath>
     {{%- endif %}}
-
-    {{%- if ALLOW_STRICTER_PERMISSIONS %}}
-      {{#
-          This filter is required because of an issue in OpenSCAP. The issue has been fixed in:
-          https://github.com/OpenSCAP/openscap/pull/1709 but this line should be kept until the
-          fix is widely available. The fix is expected to be part of OpenSCAP >= 1.3.5.
-      #}}
-      <filter action="include">state_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}_mode_not_{{{ FILEMODE }}}</filter>
-    {{%- endif %}}
       <filter action="exclude">exclude_symlinks_{{{ FILEID }}}</filter>
+      <filter action="exclude">state_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}_mode_{{{ FILEMODE }}}{{{ 'or_stricter_' if ALLOW_STRICTER_PERMISSIONS }}}</filter>
   </unix:file_object>
+
+  <unix:file_state id="state_file_permissions{{{ FILEID }}}_{{{ loop.index0 }}}_mode_{{{ FILEMODE }}}{{{ 'or_stricter_' if ALLOW_STRICTER_PERMISSIONS }}}" operator="AND" version="3">
+      {{{ STATEMODE | indent(6) }}}
+  </unix:file_state>
   {{% endfor %}}
 
   <unix:file_state id="exclude_symlinks_{{{ FILEID }}}" version="1">
diff --git a/shared/templates/file_permissions/template.py b/shared/templates/file_permissions/template.py
index 6e20a625d978..4f46d1b7a1ac 100644
--- a/shared/templates/file_permissions/template.py
+++ b/shared/templates/file_permissions/template.py
@@ -50,11 +50,8 @@ def preprocess(data, lang):
                         "<unix:" + field + " datatype=\"boolean\">true</unix:"
                         + field + ">\n" + mode_str)
             else:
-                value = "false"
-                if data['allow_stricter_permissions']:
-                    value = "true"
                 mode_str = (
-                    "<unix:" + field + " datatype=\"boolean\">{}</unix:".format(value)
+                    "<unix:" + field + " datatype=\"boolean\">false</unix:"
                     + field + ">\n" + mode_str)
             mode_int = mode_int >> 1
         data["statemode"] = mode_str.rstrip("\n")
diff --git a/shared/templates/file_permissions/tests/correct_permissions.pass.sh b/shared/templates/file_permissions/tests/correct_permissions.pass.sh
index 02d480c9984e..db3417dcc760 100644
--- a/shared/templates/file_permissions/tests/correct_permissions.pass.sh
+++ b/shared/templates/file_permissions/tests/correct_permissions.pass.sh
@@ -1,5 +1,4 @@
 #!/bin/bash
-#
 
 {{% for path in FILEPATH %}}
 {{% if IS_DIRECTORY and FILE_REGEX %}}
diff --git a/shared/templates/file_permissions/tests/lenient_permissions.fail.sh b/shared/templates/file_permissions/tests/lenient_permissions.fail.sh
index dc6c6015921a..91413fca4ed7 100644
--- a/shared/templates/file_permissions/tests/lenient_permissions.fail.sh
+++ b/shared/templates/file_permissions/tests/lenient_permissions.fail.sh
@@ -1,11 +1,10 @@
 #!/bin/bash
-#
 
 {{% for path in FILEPATH %}}
 {{% if IS_DIRECTORY and FILE_REGEX %}}
 echo "Create specific tests for this rule because of regex"
 {{% elif IS_DIRECTORY and RECURSIVE %}}
-find -L {{{ path }}} -type d -exec chmod 777 {} \;
+find -L {{{ path }}} -type d -maxdepth 1 -exec chmod 777 {} \;
 {{% else %}}
 if [ ! -f {{{ path }}} ]; then
     mkdir -p "$(dirname '{{{ path }}}')"