RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1857179 - Some ansible remediation tasks are removing files even when not necessary
Summary: Some ansible remediation tasks are removing files even when not necessary
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: scap-security-guide
Version: 8.2
Hardware: x86_64
OS: Linux
high
low
Target Milestone: rc
: 8.0
Assignee: Marcus Burghardt
QA Contact: Matus Marhefka
Khushbu Borole
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-07-15 10:52 UTC by jtougne
Modified: 2021-11-10 00:56 UTC (History)
8 users (show)

Fixed In Version: scap-security-guide-0.1.57-2.el8
Doc Type: Bug Fix
Doc Text:
.Ansible updates banner files only when needed Previously, the playbook used for banner remediation always removed the file and recreated it. As a consequence, the banner file inodes were always modified regardless of need. With this update, the Ansible remediation playbook has been improved to use the `copy` module, which first compares existing content with the intended content and only updates the file when needed. As a result, banner files are only updated when the existing content differs from the intended content.
Clone Of:
Environment:
Last Closed: 2021-11-09 18:43:58 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2021:4265 0 None None None 2021-11-09 18:44:09 UTC

Description jtougne 2020-07-15 10:52:34 UTC
Description of problem:

Hi, 
One customer pointed out that in the ansible remediation file, there are tasks which are removing files unconditionally when not necessary (as in they are already in the intended state)-

For example:

 - name: Modify the System Login Banner - remove incorrect banner
      file:
        state: absent
        path: /etc/issue
      tags:
        - banner_etc_issue
        - medium_severity
        - unknown_strategy
        - low_complexity
        - medium_disruption
        - no_reboot_needed
        - CCE-80763-6
        - NIST-800-53-AC-8(a)
        - NIST-800-53-AC-8(c)
        - NIST-800-171-3.1.9


One could argue that the next task recreates the file in the intended state.
For example:
    - name: Modify the System Login Banner - add correct banner
      lineinfile:
        dest: /etc/issue
        line: '{{ login_banner_text | regex_replace("^\^(.*)\$$", "\1") | regex_replace("^\((.*)\|.*\)$",
          "\1") | regex_replace("\[\\s\\n\]\+"," ") | regex_replace("\(\?:\[\\n\]\+\|\(\?:\\\\n\)\+\)",
          "\n") | regex_replace("\\", "") | wordwrap() }}'
        create: true
      tags:
        - banner_etc_issue
        - medium_severity
        - unknown_strategy
        - low_complexity
        - medium_disruption
        - no_reboot_needed
        - CCE-80763-6
        - NIST-800-53-AC-8(a)
        - NIST-800-53-AC-8(c)
        - NIST-800-171-3.1.9



The customer explained that deleting/recreating is not the same because if using a file integrity manager, it will pick up a problem because technically the file is being modified (and might get a new inode id for example).
That is also not idempotent.

The situation is the same for /etc/motd for example, maybe there are other cases in the file.


Version-Release number of selected component (if applicable):

scap-security-guide.noarch                                                  0.1.50-2.fc32

How reproducible:
Run the remediation using ansible and/or look the playbook.



Steps to Reproduce:
N/A

Actual results:
The file is removed first (even if complying with the rule)

Expected results:
The file, if complying with the rule, should not be removed,

Additional info:

Could the tasks modified to be idempotent?

Comment 1 Vojtech Polasek 2020-07-15 12:32:11 UTC
Hello, thank you for the report. Are you aware of more rules which are behaving in this way?

Comment 2 jtougne 2020-07-15 13:41:23 UTC
Hi,
I am aware of this one:

  - name: Modify the System Message of the Day Banner - remove incorrect banner
      file:
        state: absent
        path: /etc/motd
      tags:
        - banner_etc_motd
        - medium_severity
        - unknown_strategy
        - low_complexity
        - medium_disruption
        - no_reboot_needed
        - CCE-83496-0

    - name: Modify the System Message of the Day Banner - add correct banner
      lineinfile:
        dest: /etc/motd
        line: '{{ login_banner_text | regex_replace("^\^(.*)\$$", "\1") | regex_replace("^\((.*)\|.*\)$",
          "\1") | regex_replace("\[\\s\\n\]\+"," ") | regex_replace("\(\?:\[\\n\]\+\|\(\?:\\\\n\)\+\)",
          "\n") | regex_replace("\\", "") | wordwrap() }}'
        create: true
      tags:
        - banner_etc_motd
        - medium_severity
        - unknown_strategy
        - low_complexity
        - medium_disruption
        - no_reboot_needed
        - CCE-83496-0


I am not sure if the situation is similar with other rules. I briefly looked with other rules within a customized file for CIS, I did not see.
Having said that CIS has by default around 201 rules, and I saw over 1000+ rules in scap-workbench for all profiles.

So it is possible that it is similar on other rules.

Cheers,

Comment 3 Watson Yuuma Sato 2020-07-15 14:01:28 UTC
For banner_etc_issue this, this commit https://github.com/ComplianceAsCode/content/commit/556018017f provides context why the Ansible remediation is written as it is.

Comment 4 Matěj Týč 2020-08-10 14:22:19 UTC
Hello Jean, could you please react to Yuuma's comment?

Generally, Ansible is not powerful enough to enable us to write an idiomatic idempotent snippet. In this case, trailing blank lines could cause problems.

I guess that we could attempt to complicate the Ansible code to first perform a more thorough Ansible "scan", and erase the file iff this "scan" tells us that the file needs to be replaced. Or we can generate the correct file, and replace the original one iff the two differ.
In any case, could you please create a customer case for that if it is not a cosmetic issue?

Comment 5 jtougne 2020-08-11 08:05:51 UTC
Hi there,
Yes the issue was observed by the customer expert at SIX Group, and it's not a cosmetic issue.
I think to some extend that would impact every customers who are using a file integrity manager because it will trigger a notification that the file is no longer the same.
It looks that complicating the code by making a new file would result in the same situation?

Cheers,

Comment 6 Watson Yuuma Sato 2020-08-11 13:54:57 UTC
Maybe the "slurp" module could be used to improve the Ansible remediation.
https://docs.ansible.com/ansible/latest/modules/slurp_module.html

Before removing the file, it could read the contents of the file with "slurp" and compare to the desired banner, there may be issues with newlines though. Testing needed.

Comment 7 Vojtech Polasek 2020-08-12 07:18:15 UTC
If I understand it correctly, if the file is changed in place, then the customer's problem is solved, right? In this case, we can firstly clear the file and then put there the right content:
- name: clear the file
  lineinfile:
    path: /etc/issue
    regexp: '^.*$'
    state: absent
Is the customer willing to test it? Or can we test it so that we know that it won't trigger alarm on their file integrity manager?

Comment 8 bernhard.duebi 2020-08-12 08:09:19 UTC
(In reply to Matěj Týč from comment #4)
> Hello Jean, could you please react to Yuuma's comment?
> 
> Generally, Ansible is not powerful enough to enable us to write an idiomatic
> idempotent snippet. In this case, trailing blank lines could cause problems.
> 
> I guess that we could attempt to complicate the Ansible code to first
> perform a more thorough Ansible "scan", and erase the file iff this "scan"
> tells us that the file needs to be replaced. Or we can generate the correct
> file, and replace the original one iff the two differ.
> In any case, could you please create a customer case for that if it is not a
> cosmetic issue?

I like your statement: Ansible is not powerful

Seriously, no file should be touched when it already complies with the requirements.
I would go for your second proposal, create the expected content in a temporary file and update the original file only when it differs

Comment 9 bernhard.duebi 2020-08-12 08:23:48 UTC
(In reply to Vojtech Polasek from comment #7)
> If I understand it correctly, if the file is changed in place, then the
> customer's problem is solved, right? In this case, we can firstly clear the
> file and then put there the right content:
> - name: clear the file
>   lineinfile:
>     path: /etc/issue
>     regexp: '^.*$'
>     state: absent
> Is the customer willing to test it? Or can we test it so that we know that
> it won't trigger alarm on their file integrity manager?

Hi,
I'm the customer
I can test it and report if the current configuration of the File Integrity Monitor will tolerate a change of the Last Modified Date.

As stated above, the claim must be: no file is touched if not necessary

Comment 13 Marcus Burghardt 2021-08-05 12:15:31 UTC
Patch for this bug sent to upstream:
https://github.com/ComplianceAsCode/content/pull/7228

The logic was changed to use the "copy" module instead of the combination of "file" and "lineinfile".
The copy module[1], with the "content" parameter, check the existing content first and only update the file if the existing content doesn't match the desired content.

By the way, I checked all the project and didn't find more similar occurrences besides the /etc/motd and /etc/issue.


[1] https://docs.ansible.com/ansible/latest/collections/ansible/builtin/copy_module.html

Comment 16 Matus Marhefka 2021-08-16 07:04:29 UTC
Upstream PR with the test: https://github.com/ComplianceAsCode/content/pull/7376

Comment 26 errata-xmlrpc 2021-11-09 18:43:58 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (scap-security-guide bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2021:4265


Note You need to log in before you can comment on or make changes to this bug.