Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Bash remediation of firewalld-based rules for offline mode #11868

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

evgenyz
Copy link
Member

@evgenyz evgenyz commented Apr 22, 2024

Description:

  • Remediation scripts now use firewall-offline-cmd (where possible) or nicely bail out when running in chroot.

  • We also now check the offline mode marker (chroot) instead of the service status as the service may be active but not belonging to the target system (host service).

Rationale:

Review Hints:

Remediation scripts now use firewall-offline-cmd (where possible) or
nicely bail out when running in chroot.
@evgenyz evgenyz added Bash Bash remediation update. offline Issues or features of the content related to the OpenSCAP's 'offline' mode osbuild Related in some way to Image Builder. labels Apr 22, 2024
@evgenyz evgenyz added this to the 0.1.73 milestone Apr 22, 2024
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_restricted' differs.
--- xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_restricted
+++ xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_restricted
@@ -5,15 +5,16 @@
     yum install -y "firewalld"
 fi
 
-if systemctl is-active firewalld; then
-    firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv4 source address="127.0.0.1" destination not address="127.0.0.1" drop'
-    firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv6 source address="::1" destination not address="::1" drop'
+ipv4_rule='rule family=ipv4 source address="127.0.0.1" destination not address="127.0.0.1" drop'
+ipv6_rule='rule family=ipv6 source address="::1" destination not address="::1" drop'
+
+if test "$(stat -c %d:%i /)" != "$(stat -c %d:%i /proc/1/root/.)"; then
+    firewall-offline-cmd --zone=trusted --add-rich-rule="${ipv4_rule}"
+    firewall-offline-cmd --zone=trusted --add-rich-rule="${ipv6_rule}"
+else
+    firewall-cmd --permanent --zone=trusted --add-rich-rule="${ipv4_rule}"
+    firewall-cmd --permanent --zone=trusted --add-rich-rule="${ipv6_rule}"
     firewall-cmd --reload
-else
-    echo "
-    firewalld service is not active. Remediation aborted!
-    This remediation could not be applied because it depends on firewalld service running.
-    The service is not started by this remediation in order to prevent connection issues."
 fi
 
 else

bash remediation for rule 'xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_trusted' differs.
--- xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_trusted
+++ xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_trusted
@@ -5,14 +5,11 @@
     yum install -y "firewalld"
 fi
 
-if systemctl is-active firewalld; then
+if test "$(stat -c %d:%i /)" != "$(stat -c %d:%i /proc/1/root/.)"; then
+    firewall-offline-cmd --zone=trusted --add-interface=lo
+else
     firewall-cmd --permanent --zone=trusted --add-interface=lo
     firewall-cmd --reload
-else
-    echo "
-    firewalld service is not active. Remediation aborted!
-    This remediation could not be applied because it depends on firewalld service running.
-    The service is not started by this remediation in order to prevent connection issues."
 fi
 
 else

bash remediation for rule 'xccdf_org.ssgproject.content_rule_firewalld_sshd_port_enabled' differs.
--- xccdf_org.ssgproject.content_rule_firewalld_sshd_port_enabled
+++ xccdf_org.ssgproject.content_rule_firewalld_sshd_port_enabled
@@ -10,36 +10,40 @@
 firewalld_sshd_zone=''
 
 
-if systemctl is-active NetworkManager && systemctl is-active firewalld; then
-    # First make sure the SSH service is enabled in run-time for the proper zone.
-    # This is to avoid connection issues when new interfaces are addeded to this zone.
-    firewall-cmd --zone="$firewalld_sshd_zone" --add-service=ssh
+if test "$(stat -c %d:%i /)" != "$(stat -c %d:%i /proc/1/root/.)"; then
+    # TODO: NM (nmcli) now has --offline mode support, and it could operate without NM service.
+    # See: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1183
+    # The feature is not quite straighforward (and probably incomplete), though.
+    echo "Not applicable in offline mode. Remediation aborted!"
+else
+    if systemctl is-active NetworkManager && systemctl is-active firewalld; then
+        # First make sure the SSH service is enabled in run-time for the proper zone.
+        # This is to avoid connection issues when new interfaces are addeded to this zone.
+        firewall-cmd --zone="$firewalld_sshd_zone" --add-service=ssh
 
-    # This will collect all NetworkManager connections names
-    readarray -t nm_connections < <(nmcli -f UUID,TYPE con | grep ethernet | awk '{ print $1 }')
-    # If the connection is not yet assigned to a firewalld zone, assign it to the proper zone.
-    # This will not change connections which are already assigned to any firewalld zone.
-    for connection in "${nm_connections[@]}"; do
-        current_zone=$(nmcli -f connection.zone connection show "$connection" | awk '{ print $2}')
-        if [ $current_zone = "--" ]; then
-            nmcli connection modify "$connection" connection.zone $firewalld_sshd_zone
-        fi
-    done
-    systemctl restart NetworkManager
+        # This will collect all NetworkManager connections names
+        readarray -t nm_connections < <(nmcli -f UUID,TYPE con | grep ethernet | awk '{ print $1 }')
+        # If the connection is not yet assigned to a firewalld zone, assign it to the proper zone.
+        # This will not change connections which are already assigned to any firewalld zone.
+        for connection in "${nm_connections[@]}"; do
+            current_zone=$(nmcli -f connection.zone connection show "$connection" | awk '{ print $2}')
+            if [ $current_zone = "--" ]; then
+                nmcli connection modify "$connection" connection.zone $firewalld_sshd_zone
+            fi
+        done
+        systemctl restart NetworkManager
 
-    # Active zones are zones with at least one interface assigned to it.
-    # It is possible that traffic is comming by any active interface and consequently any
-    # active zone. So, this make sure all active zones are permanently allowing SSH service.
-    readarray -t firewalld_active_zones < <(firewall-cmd --get-active-zones | grep -v interfaces)
-    for zone in "${firewalld_active_zones[@]}"; do
-        firewall-cmd --permanent --zone="$zone" --add-service=ssh
-    done
-    firewall-cmd --reload
-else
-    echo "
-    firewalld and NetworkManager services are not active. Remediation aborted!
-    This remediation could not be applied because it depends on firewalld and NetworkManager services running.
-    The service is not started by this remediation in order to prevent connection issues."
+        # Active zones are zones with at least one interface assigned to it.
+        # It is possible that traffic is comming by any active interface and consequently any
+        # active zone. So, this make sure all active zones are permanently allowing SSH service.
+        readarray -t firewalld_active_zones < <(firewall-cmd --get-active-zones | grep -v interfaces)
+        for zone in "${firewalld_active_zones[@]}"; do
+            firewall-cmd --permanent --zone="$zone" --add-service=ssh
+        done
+        firewall-cmd --reload
+    else
+        echo "The firewalld or NetworkManager service is not active. Remediation aborted!"
+    fi
 fi
 
 else

Copy link

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11868
This image was built from commit: f3618c2

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11868

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11868 make deploy-local

Copy link

codeclimate bot commented Apr 22, 2024

Code Climate has analyzed commit f3618c2 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 59.2% (0.0% change).

View more on Code Climate.

@jan-cerny jan-cerny self-assigned this Apr 24, 2024
Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fail of Automatus is caused by using containers as a test backend because they don't provide network/firewalld. If executed locally with a VM back end, the test pass.

jcerny@fedora:~/work/git/scap-security-guide (pr/11868)$ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel8 firewalld_sshd_port_enabled firewalld_loopback_traffic_restricted firewalld_loopback_traffic_trusted
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/jcerny/work/git/scap-security-guide/logs/rule-custom-2024-04-24-0857/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_firewalld_sshd_port_enabled
INFO - Script customized_zone_configured.pass.sh using profile (all) OK
INFO - Script customized_zone_without_ssh.fail.sh using profile (all) OK
INFO - Script new_zone_configured.pass.sh using profile (all) OK
INFO - Script new_zone_without_ssh.fail.sh using profile (all) OK
INFO - Script only_nics_configured.fail.sh using profile (all) OK
INFO - Script only_zones_configured.fail.sh using profile (all) OK
INFO - Script zones_and_nics_configured.pass.sh using profile (all) OK
INFO - Script zones_and_nics_ok_no_custom_files.pass.sh using profile (all) OK
INFO - Script zones_and_nics_ok_port_changed.pass.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_restricted
INFO - Script customized_zones_configured.pass.sh using profile (all) OK
INFO - Script customized_zones_without_richrules.fail.sh using profile (all) OK
INFO - Script default_zones_configured.pass.sh using profile (all) OK
INFO - Script default_zones_without_richrules.fail.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_trusted
INFO - Script customized_zone_configured.pass.sh using profile (all) OK
INFO - Script customized_zone_without_lo.fail.sh using profile (all) OK
INFO - Script default_zone_configured.pass.sh using profile (all) OK
INFO - Script default_zone_without_lo.fail.sh using profile (all) OK
jcerny@fedora:~/work/git/scap-security-guide (pr/11868)$ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel9 firewalld_sshd_port_enabled firewalld_loopback_traffic_restricted firewalld_loopback_traffic_trusted
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/jcerny/work/git/scap-security-guide/logs/rule-custom-2024-04-24-0911/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_firewalld_sshd_port_enabled
INFO - Script customized_zone_configured.pass.sh using profile (all) OK
INFO - Script customized_zone_without_ssh.fail.sh using profile (all) OK
INFO - Script new_zone_configured.pass.sh using profile (all) OK
INFO - Script new_zone_without_ssh.fail.sh using profile (all) OK
INFO - Script only_nics_configured.fail.sh using profile (all) OK
INFO - Script only_zones_configured.fail.sh using profile (all) OK
INFO - Script zones_and_nics_configured.pass.sh using profile (all) OK
INFO - Script zones_and_nics_ok_no_custom_files.pass.sh using profile (all) OK
INFO - Script zones_and_nics_ok_port_changed.pass.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_restricted
INFO - Script customized_zones_configured.pass.sh using profile (all) OK
INFO - Script customized_zones_without_richrules.fail.sh using profile (all) OK
INFO - Script default_zones_configured.pass.sh using profile (all) OK
INFO - Script default_zones_without_richrules.fail.sh using profile (all) OK
INFO - xccdf_org.ssgproject.content_rule_firewalld_loopback_traffic_trusted
INFO - Script customized_zone_configured.pass.sh using profile (all) OK
INFO - Script customized_zone_without_lo.fail.sh using profile (all) OK
INFO - Script default_zone_configured.pass.sh using profile (all) OK
INFO - Script default_zone_without_lo.fail.sh using profile (all) OK

@jan-cerny jan-cerny merged commit 01337bd into ComplianceAsCode:master Apr 24, 2024
40 of 44 checks passed
marcusburghardt added a commit to marcusburghardt/contest that referenced this pull request Apr 25, 2024
Remove the waivers for firewalld_loopback_traffic_trusted rules in
Anaconda tests after the
ComplianceAsCode/content#11868
comps pushed a commit to RHSecurityCompliance/contest that referenced this pull request Apr 25, 2024
Remove the waivers for firewalld_loopback_traffic_trusted rules in
Anaconda tests after the
ComplianceAsCode/content#11868
marcusburghardt added a commit to marcusburghardt/content that referenced this pull request Apr 26, 2024
After the ComplianceAsCode#11868 the condition to test the firewalld service state was
removed, causing the remediation to report error when the service is stopped.
This change also created a missaligment with Ansible remediation.
This commit returns the condition to keep the alignment and better
report the case to users.
marcusburghardt added a commit to marcusburghardt/content that referenced this pull request Apr 26, 2024
After the ComplianceAsCode#11868 the condition to test the firewalld service state was
removed, causing the remediation to report error when the service is stopped.
This change also created a misalignment with Ansible remediation.
This commit returns the condition to keep the alignment and better
report the case to users.
@evgenyz evgenyz deleted the fix-firewalld-bash branch April 29, 2024 09:52
vojtapolasek added a commit to vojtapolasek/contest that referenced this pull request Apr 29, 2024
vojtapolasek added a commit to vojtapolasek/contest that referenced this pull request Apr 29, 2024
comps pushed a commit to vojtapolasek/contest that referenced this pull request Apr 29, 2024
comps pushed a commit to RHSecurityCompliance/contest that referenced this pull request Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bash Bash remediation update. offline Issues or features of the content related to the OpenSCAP's 'offline' mode osbuild Related in some way to Image Builder.
Projects
None yet
2 participants