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

SLE15 add implementation of nftables_rules_permanent rule #10201

Conversation

teacup-on-rockingchair
Copy link
Contributor

Description:

  • Add SLE15 based implementation of nftables_rules_permanent rule

Rationale:

  • Allocate CCE for the nftables_rules_permanent rule and add rule to the CIS profile
  • _The nftables_rules_permanent rule is based on the CIS requirement 3.5.2.10
    There are some internal decisions made for the rule based on the facts that:
    • Requirement assumes the system firewall is managed explicitely by nftables package and relevant kernel modules, i.e. no firewalld, no iptables installed
    • SLE 15 nftables package comes with following number of configuration files:
      • /etc/nftables/arp-filter
      • /etc/nftables/bridge-filter
      • /etc/nftables/inet-filter
      • /etc/nftables/ipv4-filter
      • /etc/nftables/ipv4-mangle
      • /etc/nftables/ipv4-nat
      • /etc/nftables/ipv4-raw
      • /etc/nftables/ipv6-filter
      • /etc/nftables/ipv6-mangle
      • /etc/nftables/ipv6-nat
      • /etc/nftables/ipv6-raw
        , where bridge, arp and inet(ipv4+ipv6), should cover L2+L3 layers of the IP stack, which are mainly addressed in other nftables requirements

Review Hints:

The design of the OVAL checks and Ansible and Bash remediation was made in a way that the rule will check that there is top-level configuration file,
as described by the requirement /etc/sysconfig/nftables, which will include per protocol family configuration files that come with the native package.

The contents of the actual configuration as far as I understand the requirement should be left to the customer site policy.

The main idea of the rule as I tried to follow it was to make sure that there exists a configuration which to be applied on boot.

All of the changes made were SLE15 specific, since I based my logic on how the native nftables package for SUSE SLE15 looks like.

The nftables_rules_permanent rule is based on the CIS requirement 3.5.2.10
There are some internal decisions made for the rule based on the facts that:
- Requirement assumes the system firewall is managed explicitely by nftables package and relevant kernel modules, i.e. no firewalld, no iptables installed
- SLE 15 nftables package comes with following number of configuration files:
  - /etc/nftables/arp-filter
  - /etc/nftables/bridge-filter
  - /etc/nftables/inet-filter
  - /etc/nftables/ipv4-filter
  - /etc/nftables/ipv4-mangle
  - /etc/nftables/ipv4-nat
  - /etc/nftables/ipv4-raw
  - /etc/nftables/ipv6-filter
  - /etc/nftables/ipv6-mangle
  - /etc/nftables/ipv6-nat
  - /etc/nftables/ipv6-raw
, where bridge, arp and inet(ipv4+ipv6), should cover L2+L3 layers of the IP stack, which are mainly addressed in other nftables requirements

So the design of the OVAL checks and Ansible and Bash remediation was made in a way that the rule will check that there is top-level configuration file,
as described by the requirement /etc/sysconfig/nftables, which will include per protocol family configuration files that come with the native package

The contents of the actual configuration as far as I uderstand the requirement should be left to the customer site policy.
The main idea of the rule as I tried to follow it was to make sure that there exists a configuration which to be applied on boot.

All of the changes made were SLE15 specific, since I based my logic on how the native nftables package for SUSE SLE15 looks like.
@teacup-on-rockingchair teacup-on-rockingchair requested a review from a team as a code owner February 13, 2023 08:24
@openshift-ci
Copy link

openshift-ci bot commented Feb 13, 2023

Hi @teacup-on-rockingchair. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Feb 13, 2023
@github-actions
Copy link

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

sle15 (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 marcusburghardt added SLES SUSE Linux Enterprise Server product related. New Rule Issues or pull requests related to new Rules. labels Feb 13, 2023
@jan-cerny jan-cerny self-assigned this Feb 14, 2023
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.

LGTM

Thanks

@jan-cerny
Copy link
Collaborator

@ComplianceAsCode/suse-maintainers PTAL

@jan-cerny
Copy link
Collaborator

@marcusburghardt Can you take a look if it can be merged instead of SUSE?

@jan-cerny
Copy link
Collaborator

or also @Mab879

@jan-cerny jan-cerny removed their assignment Mar 8, 2023
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.

The rule is complete, but there are some opportunities to improve the readability.

…permanent/rule.yml

Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
Use cleaner approach when looping throug bridge arp and inet protocol families for file definitions
Thanks to @marcusburghardt for the hint 🙇
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.

Only a small detail is pending. The jinja2 variable declaration can be removed in Ansible now.

@marcusburghardt marcusburghardt self-assigned this Apr 5, 2023
…permanent/ansible/sle15.yml


Unnecessary variable
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 now. Thanks

@codeclimate
Copy link

codeclimate bot commented Apr 5, 2023

Code Climate has analyzed commit 1467145 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 52.4%.

View more on Code Climate.

@marcusburghardt
Copy link
Member

Overriding CODEOWNERS since a SUSE approver is not currently available.

@marcusburghardt marcusburghardt merged commit 9a62e1e into ComplianceAsCode:master Apr 6, 2023
24 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ok-to-test Used by openshift-ci bot. New Rule Issues or pull requests related to new Rules. SLES SUSE Linux Enterprise Server product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants