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

SLE AIDE periodic check and remediation via systemd timer #10589

Conversation

teacup-on-rockingchair
Copy link
Contributor

Description:

  • For SLE15 consider the possibility to enable AIDE periodic checks via systemd timer

Rationale:

  • Modify oval shared check to include also option via systemd service and timer
  • Add bash and ansible remediations for aide periodic checking via systemd timer

@teacup-on-rockingchair teacup-on-rockingchair added Ansible Ansible remediation update. Bash Bash remediation update. SLES SUSE Linux Enterprise Server product related. labels May 18, 2023
@github-actions
Copy link

github-actions bot commented May 18, 2023

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

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

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@marcusburghardt
Copy link
Member

Nice. I wonder if other distros already use or tend to use systemd timer with AIDE. If so, it would be interesting to extend the existing remediation so other distros can also benefit on this. I will take a look in Fedora and RHEL. @dodys @freddieRv @Xeicker FYI

@marcusburghardt marcusburghardt added this to the 0.1.69 milestone Jun 2, 2023
@dodys
Copy link
Contributor

dodys commented Jun 7, 2023

Nice. I wonder if other distros already use or tend to use systemd timer with AIDE. If so, it would be interesting to extend the existing remediation so other distros can also benefit on this. I will take a look in Fedora and RHEL. @dodys @freddieRv @Xeicker FYI

I wonder if this should be a new rule instead.
We aren't offering a way for people to choose one of the two solutions: cron or systemd timer. Instead in SUSE's bash they are choosing it for their users.
Also the name of the rule implies cron.

@teacup-on-rockingchair
Copy link
Contributor Author

Nice. I wonder if other distros already use or tend to use systemd timer with AIDE. If so, it would be interesting to extend the existing remediation so other distros can also benefit on this. I will take a look in Fedora and RHEL. @dodys @freddieRv @Xeicker FYI

I wonder if this should be a new rule instead. We aren't offering a way for people to choose one of the two solutions: cron or systemd timer. Instead in SUSE's bash they are choosing it for their users. Also the name of the rule implies cron.

Well my motivation with the sle15 specific files was exactly this not to imply SLE policy and not make too many conditions in the shared files.

On the other hand, putting is as totally different rule in my opinion would duplicate too much of the logic. I agree the name is confusing, but if you say cron these days, I personally do not understand it as exact package or service, rather than type of functionality for scheduled execution of tasks.

@marcusburghardt
Copy link
Member

Nice. I wonder if other distros already use or tend to use systemd timer with AIDE. If so, it would be interesting to extend the existing remediation so other distros can also benefit on this. I will take a look in Fedora and RHEL. @dodys @freddieRv @Xeicker FYI

I wonder if this should be a new rule instead. We aren't offering a way for people to choose one of the two solutions: cron or systemd timer. Instead in SUSE's bash they are choosing it for their users. Also the name of the rule implies cron.

Well my motivation with the sle15 specific files was exactly this not to imply SLE policy and not make too many conditions in the shared files.

On the other hand, putting is as totally different rule in my opinion would duplicate too much of the logic. I agree the name is confusing, but if you say cron these days, I personally do not understand it as exact package or service, rather than type of functionality for scheduled execution of tasks.

Thanks for the more context you both provided. I just tried to investigate this case and didn't find any package defining the aidecheck.service and aidecheck.timer by default, which is kind of expected since there is nothing similar for cron in the context of aide. For me, I feel that creating new systemd units is something a little bigger than including cron rules. Anyway, they are valid files but optionally defined by the user.

After thinking more about it, I am more inclined to agree with @dodys in creating a new rule for this.
This case you brought is pretty valid @teacup-on-rockingchair and I believe it deserves its own rule.
The hard work is basically done as I see in this PR. A new rule should not be difficult.
It could be called similarly, something like aide_periodic_checking_systemd_timer, for example. But the name I keep up to you. : )

As the benefits of creating this new rule I see:

  • Both rules much simpler, which makes them easier to understand and maintain
  • More granularity for profiles. Profiles can select both or only one of these two rules depending on each case

If in the future, for example, CIS drops cron approach in favor of systemd timer, we can simply update the profile by removing the cron rule. Otherwise, the rule would demand changes again and likely be split in any case.

@marcusburghardt marcusburghardt removed this from the 0.1.69 milestone Jul 11, 2023
@teacup-on-rockingchair teacup-on-rockingchair requested a review from a team as a code owner July 21, 2023 04:45
Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

Nice rule. I believe it might be useful for other products in the future. Besides some minor suggestion to descriptions, I only have some comments to make its adoption easier and some considerations in the Ansible remediation.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label Jul 25, 2023
teacup-on-rockingchair and others added 14 commits July 26, 2023 07:32
Define new rule aide_periodic_checking_systemd_timer
…de/aide_periodic_checking_systemd_timer/ansible/sle15.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/ansible/sle15.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/bash/sle15.sh

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/oval/sle15.xml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/rule.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/rule.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/rule.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/tests/aide_not_installed.fail.sh

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/rule.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
…de/aide_periodic_checking_systemd_timer/rule.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
Thanks to @marcusburghardt for the feedback on those

- Dropped paide package installation on remediation no need to duplicate that
- Improved service definition procs by setting ownership, mode and forcing daemon_reload on systemd unit definition
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Used by openshift-ci bot. label Jul 26, 2023
@marcusburghardt
Copy link
Member

@teacup-on-rockingchair , this PR is almost ready to be merged, but cause of failures in CI tests should be fixed:
ValueError: The rule 'aide_periodic_checking_systemd_timer' isn't mapped to any component! Insert the rule ID to at least one file in '/__w/content/content/components'.

You should include this new rule in the components mapping, in the aide.yml file.

@codeclimate
Copy link

codeclimate bot commented Jul 27, 2023

Code Climate has analyzed commit c9da482 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 53.2% (0.0% change).

View more on Code Climate.

Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@marcusburghardt marcusburghardt added this to the 0.1.70 milestone Jul 27, 2023
@marcusburghardt
Copy link
Member

Overriding CODEOWNERS as @teacup-on-rockingchair can't approve his own PR.

@marcusburghardt
Copy link
Member

I double-checked the failed CI tests and confirmed they can be waived. Tests successfully passed in my local VMs.

@marcusburghardt marcusburghardt merged commit b484c46 into ComplianceAsCode:master Jul 27, 2023
27 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update. Bash Bash remediation update. SLES SUSE Linux Enterprise Server product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants