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

New SLE 12/15 rule all_apparmor_profiles_in_enforce_complain_mode whi… #10064

Conversation

rumch-se
Copy link
Contributor

…ch covers CIS recommendation

Description:

  • This rule covers CIS recommendation 1.7.1.3 Ensure all AppArmor Profiles are in enforce or complain mode

Rationale:

  • At the moment CIS profiles for SLE 12/15 does not include this recommendation. The recommendation could be extended to support - other Linux distributions

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

openshift-ci bot commented Jan 16, 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:

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

Copy link
Contributor

@dodys dodys left a comment

Choose a reason for hiding this comment

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

a few fixes needed

<linux:state state_ref="no_processes_which_are_unconfined" />
</linux:apparmorstatus_test>

<linux:apparmorstatus_object id="apparmor_exists_on_our_system" version="1">
Copy link
Contributor

Choose a reason for hiding this comment

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

you can consolidate this is one line:
<linux:apparmorstatus_object id="apparmor_exists_on_our_system" version="1" />

@dodys dodys self-assigned this Jan 17, 2023
@dodys
Copy link
Contributor

dodys commented Jan 17, 2023

fyi, since we don't have a SUSE maintainer and this rule also is needed for Ubuntu, I've assigned it to me.

@dodys
Copy link
Contributor

dodys commented Jan 17, 2023

This rule is not buildable in Ubuntu (and Debian), as apparmor schema was only added in OVAL Format 5.11.2 and Ubuntu now only has OVAL Format 5.11.1. A separate OVAL will be sent after this is merged to make it work in Ubuntu.

@rumch-se can you confirm that this is also the case for OpenSUSE, as the logs are showing that it doesn't recognize the apparmor schema.

@rumch-se
Copy link
Contributor Author

Hello @dodys
Yes I can confirm about openSUSE
"
AppArmor is installed and running on any installation of openSUSE® Leap by default, regardless of what patterns are installed. The packages listed below are needed for a fully-functional instance of AppArmor:
source: https://doc.opensuse.org/documentation/leap/security/html/book-security/cha-apparmor-start.html
"

@dodys
Copy link
Contributor

dodys commented Jan 18, 2023

Hello @dodys Yes I can confirm about openSUSE " AppArmor is installed and running on any installation of openSUSE® Leap by default, regardless of what patterns are installed. The packages listed below are needed for a fully-functional instance of AppArmor: source: https://doc.opensuse.org/documentation/leap/security/html/book-security/cha-apparmor-start.html "

Hey @rumch-se, I meant please check the OVAL format version in opensuse, I believe you have OVAL Format < 5.11.2 and therefore that's why the build/test is failing here. For that you must run: oscap -V

@rumch-se
Copy link
Contributor Author

Hello @dodys
I have done expected corrections.
According to question related to OVAL version check - in my case the version is 5.11.1 which is < 5.11.2
Have a nice day
Rumen

Copy link
Contributor

@dodys dodys 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 this rule will fail to build in a few of the containers because of the OVAL version, as apparmor tests were added to OVAL Format 5.11.2 only. Can we merge it as-is?

@marcusburghardt
Copy link
Member

lgtm, thanks! @marcusburghardt this rule will fail to build in a few of the containers because of the OVAL version, as apparmor tests were added to OVAL Format 5.11.2 only. Can we merge it as-is?

Is the package already available but the containers are not updated?
If not, is there any plan to update the OVAL version on these systems?

I am not sure about the impact of CI tests if we merge it. @mildas , maybe you can help us on this?

@marcusburghardt marcusburghardt added SLES SUSE Linux Enterprise Server product related. Ubuntu Ubuntu product related. labels Feb 8, 2023
@rumch-se
Copy link
Contributor Author

Good morning @marcusburghardt and @dodys
Thank you for your feedback.
The proposed corrections are done.
Have a nice day
Rumen

@marcusburghardt
Copy link
Member

lgtm, thanks! @marcusburghardt this rule will fail to build in a few of the containers because of the OVAL version, as apparmor tests were added to OVAL Format 5.11.2 only. Can we merge it as-is?

Is the package already available but the containers are not updated? If not, is there any plan to update the OVAL version on these systems?

I am not sure about the impact of CI tests if we merge it. @mildas , maybe you can help us on this?

I have talked to @mildas offline and he confirmed my concern of merging this PR now. It would impact the CI tests and cause failures in other PRs.

The ideal would be to ensure the OVAL Format 5.11.2 support before merging.

@dodys
Copy link
Contributor

dodys commented Feb 13, 2023

lgtm, thanks! @marcusburghardt this rule will fail to build in a few of the containers because of the OVAL version, as apparmor tests were added to OVAL Format 5.11.2 only. Can we merge it as-is?

Is the package already available but the containers are not updated? If not, is there any plan to update the OVAL version on these systems?
I am not sure about the impact of CI tests if we merge it. @mildas , maybe you can help us on this?

I have talked to @mildas offline and he confirmed my concern of merging this PR now. It would impact the CI tests and cause failures in other PRs.

The ideal would be to ensure the OVAL Format 5.11.2 support before merging.

Should we rename the oval to be sle15.xml so it won't get used in other platforms? Would that fix this issue temporarily?

@marcusburghardt
Copy link
Member

lgtm, thanks! @marcusburghardt this rule will fail to build in a few of the containers because of the OVAL version, as apparmor tests were added to OVAL Format 5.11.2 only. Can we merge it as-is?

Is the package already available but the containers are not updated? If not, is there any plan to update the OVAL version on these systems?
I am not sure about the impact of CI tests if we merge it. @mildas , maybe you can help us on this?

I have talked to @mildas offline and he confirmed my concern of merging this PR now. It would impact the CI tests and cause failures in other PRs.
The ideal would be to ensure the OVAL Format 5.11.2 support before merging.

Should we rename the oval to be sle15.xml so it won't get used in other platforms? Would that fix this issue temporarily?

Interesting idea @dodys . I believe this should work. What do you think @yuumasato ?

@dodys
Copy link
Contributor

dodys commented Feb 21, 2023

@rumch-se could you please rename the oval file from shared.xml to sle.xml ?

@rumch-se
Copy link
Contributor Author

Hello @dodys
The change was done.
Have a nice day
Rumen

@dodys
Copy link
Contributor

dodys commented Feb 22, 2023

Hello @dodys The change was done. Have a nice day Rumen

Thanks!
We will wait now for the test results to be sure it works. Also do test on your side, but I would expect that an OVAL file named sle.xml, should work for you.

@dodys
Copy link
Contributor

dodys commented Feb 22, 2023

@marcusburghardt ok, so the renaming of the oval file will not prevent tests to fail.
Then I think the way to go is to drop that oval altogether. What do you think?

@dodys
Copy link
Contributor

dodys commented Mar 9, 2023

Hi @rumch-se, could you please rebase this PR and remove the commit to add the OVAL. Since your OVAL is dependent of OVAL format 5.11.2 and most of the CI only has OVAL format 5.11.1, we won't be able to merge it as this will break the CI.
I will later add an SCE audit script for ubuntu.
@teacup-on-rockingchair fyi ^
let us know if you have any doubts.

@rumch-se rumch-se force-pushed the rule_all_apparmor_profiles_in_enforce_complain_mode branch from 4e10996 to bb7dd28 Compare March 9, 2023 10:16
@rumch-se
Copy link
Contributor Author

rumch-se commented Mar 9, 2023

Hello @dodys
I have done a rebase and removed the OVAL part.
Have a nice day
Rumen

@codeclimate
Copy link

codeclimate bot commented Mar 9, 2023

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

View more on Code Climate.

Copy link
Contributor

@dodys dodys 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
Copy link
Member

The Automatus tests are failing because the rule is restricted to sle products.

@marcusburghardt
Copy link
Member

Although I don't have a system to properly test apparmor, this new rule is restricted to sle products, was proposed by a known SUSE maintainer and approved by a know Ubuntu maintainer. The files are also sane for me.

Therefore, I am overriding CODEOWNERS since a SUSE approver is not currently available.

@marcusburghardt marcusburghardt merged commit 64a2698 into ComplianceAsCode:master Mar 13, 2023
@marcusburghardt marcusburghardt added this to the 0.1.67 milestone Mar 13, 2023
@marcusburghardt marcusburghardt added the New Rule Issues or pull requests related to new Rules. label Mar 13, 2023
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. Ubuntu Ubuntu product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants