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.

Sign up for GitHub

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

Jump to bottom

New SLE 15 rule set_nftables_table #10128

Merged
merged 3 commits into from
Feb 15, 2023
Merged

New SLE 15 rule set_nftables_table #10128

merged 3 commits into from
Feb 15, 2023

Conversation

rumch-se
Copy link
Contributor

Description:

  • New SLE 15 rule which covers CIS requirement 3.5.2.4 Ensure a table exists (Automated)

Rationale:

  • At the moment CIS profile for SLE 15 does not include this rule.

@rumch-se rumch-se requested a review from a team as a code owner January 26, 2023 11:30
@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Jan 26, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jan 26, 2023

Hi @rumch-se. 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.

@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

@Mab879 Mab879 added this to the 0.1.67 milestone Jan 26, 2023
@Mab879 Mab879 added SLES SUSE Linux Enterprise Server product related. New Rule Issues or pull requests related to new Rules. CIS CIS Benchmark related. labels Jan 26, 2023
@Mab879 Mab879 self-assigned this Jan 26, 2023
Mab879
Mab879 requested changes Jan 26, 2023
View reviewed changes
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I have a few suggestions. Please also add Automatus tests for this rule as well.

linux_os/guide/system/network/network-nftables/set_nftables_table/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_table/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_family.var Outdated Show resolved Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_table.var Outdated Show resolved Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_table/ansible/shared.yml Outdated Show resolved Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_table/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_table/ansible/shared.yml Outdated
{{{ ansible_instantiate_variables("var_nftables_table") }}}

- name: Collect existing nftables
ansible.builtin.shell: nft list tables
Copy link
Member

Choose a reason for hiding this comment

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

Do you need ansible.builtin.shell or will ansible.builtin.command work? ansible.builtin.command is preferred.

linux_os/guide/system/network/network-nftables/set_nftables_table/bash/shared.sh Outdated

#Set nftables family name
{{{ bash_instantiate_variables("var_nftables_family") }}}
NETWORK_LEVEL=$var_nftables_family
Copy link
Member

Choose a reason for hiding this comment

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

These can be removed, and just the variables directly in your command.

linux_os/guide/system/network/network-nftables/set_nftables_table/bash/shared.sh Outdated

#Set nftables table name
{{{ bash_instantiate_variables("var_nftables_table") }}}
TABLE_NAME=$var_nftables_table
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

controls/cis_sle15.yml
status: manual # rule is missing
status: automated
notes:
The audit (OVAL check) cannot be automated,
Copy link
Member

Choose a reason for hiding this comment

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

Just a side note, if you are willing to not be SCAP compliant SCE (script check engine) content could be used on this rule to automate the check.

@rumch-se
Copy link
Contributor Author

Hello @Mab879
Thank you for your feedback.
I have done the corrections proposed.
Have a nice day
Rumen

@codeclimate
Copy link

codeclimate bot commented Jan 27, 2023

Code Climate has analyzed commit 1fb9537 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 49.5% (0.0% change).

View more on Code Climate.

@rumch-se rumch-se mentioned this pull request Jan 30, 2023
Closed
Mab879
Mab879 approved these changes Feb 2, 2023
View reviewed changes
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@Mab879
Copy link
Member

Mab879 commented Feb 2, 2023

@teacup-on-rockingchair can you please take a look at this?

@teacup-on-rockingchair
Copy link
Contributor

@teacup-on-rockingchair can you please take a look at this?

LGTM, not sure what I can add, besides maybe a clause on nftables package in the remediation for bash:

{{{ bash_package_install("nftables") }}}

and ansible:

- name: "Ensure nftables is installed"
  package:
    name: "{{ item }}"
    state: present
  with_items:
    - nftables

otherwise on my broken SLE15 setup the remediation passes, assuming nftables commands work, when there is no nft command at all.

@Mab879
Copy link
Member

Mab879 commented Feb 15, 2023

Overriding CODEOWNERS since an SUSE approver is not available currently.

@Mab879 Mab879 merged commit 525eb53 into ComplianceAsCode:master Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcusburghardt marcusburghardt marcusburghardt left review comments

@Mab879 Mab879 Mab879 approved these changes

Assignees

@Mab879 Mab879

Labels
CIS CIS Benchmark related. 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
Milestone
0.1.67
Development

Successfully merging this pull request may close these issues.
4 participants
@rumch-se @Mab879 @teacup-on-rockingchair @marcusburghardt