-
Notifications
You must be signed in to change notification settings - Fork 685
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
New SLE 15 rule set_nftables_base_chain #10180
New SLE 15 rule set_nftables_base_chain #10180
Conversation
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 Once the patch is verified, the new status will be reflected by the 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. |
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
Hello @marcusburghardt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requirement is intentionally manual
in CIS benchmark because both the check and remediation is complex since the requirement can be accomplished by different ways.
Including a very specific approach in remediation is too risky for the system stability. In cases of intentionally manual
requirements for CIS, it should be fine to create an OVAL assessment intended help admins and auditors, but I noticed there is no OVAL for this rule.
Regarding the remediation, it should be indeed manual to avoid system disruptions. I can't see a feasible approach robust enough to safely avoid disruptions or undesired changes in intentional settings present in the system.
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/rule.yml
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
Hello @marcusburghardt |
89dc961
to
4784ac1
Compare
Please, take a look in the failed CI tests. It seems they are legit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a script, executing it and finally removing it in Ansible is definitely weird. It is feasible to decompose everything in this remediation in order to use individual Ansible tasks. Please, avoid using this approach in Ansible when individual tasks are doable.
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
Hello @marcusburghardt |
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/bash/shared.sh
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_priorities.var
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_priorities.var
Show resolved
Hide resolved
Much better now @rumch-se . I left some other comments after the changes. Thanks |
…se_chain_priorities.var Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
Hello @marcusburghardt |
Hello @marcusburghardt, I have updated bash and ansible remediation in the following way Have a nice day |
/packit retest-failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rumch-se this rule is almost ready. Thanks for the updates. I have some few comments in the Ansible. Also, it would be great if you include an SCE check for it. We can't currently check this requirement using OVAL, but using SCE it is possible and would be very useful.
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
Thanks for the updates in the Ansible remediation @rumch-se . |
Hello @marcusburghardt Would it be possible to give me an example for SCE check, and I will adapt to this case? |
Here are some examples in the Project: |
Hello @marcusburghardt I have added a SCE check Have a nice day |
Thanks! |
Hello @marcusburghardt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the remediation is failing with some errors. Please, see my comments.
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/bash/shared.sh
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/bash/shared.sh
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/bash/shared.sh
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/sce/shared.sh
Outdated
Show resolved
Hide resolved
Hello @marcusburghardt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @rumch-se . I could test the remediations and check successfully. However, I had to update the default values in the variables. Please, see my comments about it. Also, there are some minor issues about the Ansible tasks names.
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/bash/shared.sh
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_hooks.var
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_names.var
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_policies.var
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_priorities.var
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/var_nftables_base_chain_types.var
Outdated
Show resolved
Hide resolved
linux_os/guide/system/network/network-nftables/set_nftables_base_chain/ansible/shared.yml
Outdated
Show resolved
Hide resolved
Hello @marcusburghardt |
/packit build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now. Thanks @rumch-se
@teacup-on-rockingchair , could you take a look too, please? |
…se_chain/sce/shared.sh Co-authored-by: Marcus Burghardt <2074099+marcusburghardt@users.noreply.github.com>
Code Climate has analyzed commit fb379e6 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great contribution 👍
Description:
Rationale: