Skip to content

Try 4110 for file_permissions_sudo#11805

Merged
marcusburghardt merged 7 commits into
ComplianceAsCode:masterfrom
Mab879:file_permissions_sudo_strict
Apr 26, 2024
Merged

Try 4110 for file_permissions_sudo#11805
marcusburghardt merged 7 commits into
ComplianceAsCode:masterfrom
Mab879:file_permissions_sudo_strict

Conversation

@Mab879
Copy link
Copy Markdown
Member

@Mab879 Mab879 commented Apr 8, 2024

Description:

Try 4110 for file_permissions_sudo.

Rationale:

Better align with the benchmark.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 8, 2024
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 8, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2024

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

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

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2024

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11805
This image was built from commit: 7c2780a

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11805

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11805 make deploy-local

@vojtapolasek
Copy link
Copy Markdown
Collaborator

I think it might be worth modifying anssi.yml control and move the rule from related_rules to rules... and let's see what the tests show.

@Mab879 Mab879 force-pushed the file_permissions_sudo_strict branch from d9fc62d to dda8c4f Compare April 9, 2024 20:05
@Mab879
Copy link
Copy Markdown
Member Author

Mab879 commented Apr 9, 2024

I think it might be worth modifying anssi.yml control and move the rule from related_rules to rules... and let's see what the tests show.

And everything works?

@vojtapolasek
Copy link
Copy Markdown
Collaborator

hm... I did a thorough investigation of this requirement and I wonder how to solve it.
I see two things which we should consider.
The first is related to the file_permissions_sudo rule. I think we should set the allow_stricter_permissions to false, because stricter permissions also means for example no suid, that defends purpose of sudo. But in general, I think this rule can be in the profile and by that I mean in the list of active rules, not related rules.
The second thing which I noticed is that we explicitly unselect the sudo_dedicated_group for RHEL 9 version of the profile... I think we should reconsider it. Or do you know the history of the reason why the rule was not added to RHEL 9?
Anyway, the requirement can't be fully automated exactly because of the sudo_dedicated_group rule, this has to be remediated manually.

@Mab879 Mab879 marked this pull request as ready for review April 15, 2024 17:52
@Mab879 Mab879 requested a review from a team as a code owner April 15, 2024 17:52
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 15, 2024
@Mab879 Mab879 force-pushed the file_permissions_sudo_strict branch from a420d1d to 0edd9c5 Compare April 16, 2024 17:28
@marcusburghardt
Copy link
Copy Markdown
Member

The test scenarios should be updated since the rule no longer accepts stricter permissions:
https://github.com/ComplianceAsCode/content/actions/runs/8709948662/job/23890853371?pr=11805

@marcusburghardt marcusburghardt added this to the 0.1.73 milestone Apr 18, 2024
@marcusburghardt marcusburghardt added Update Rule Issues or pull requests related to Rules updates. ANSSI ANSSI Benchmark related. RHEL9 Red Hat Enterprise Linux 9 product related. RHEL10 Red Hat Enterprise Linux 10 product related. labels Apr 18, 2024
@Mab879 Mab879 force-pushed the file_permissions_sudo_strict branch 2 times, most recently from 742bd78 to 57469c9 Compare April 24, 2024 15:08
Mab879 added 6 commits April 25, 2024 14:53
The permissions 4110 need to be exact to ensure that sudo
operates correctly.
Before this test would always fail this commit adds some
conditionals to check if ALLOW_STRICTER_PERMISSIONS is true
and adjust the tests if the value is true.
@Mab879 Mab879 force-pushed the file_permissions_sudo_strict branch from 57469c9 to f8eabb1 Compare April 25, 2024 19:53
@marcusburghardt marcusburghardt self-assigned this Apr 26, 2024
Copy link
Copy Markdown
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.

Just a minor point, to remove an outdated note.

Comment thread controls/anssi.yml
Since the status is now automated
@Mab879 Mab879 requested a review from marcusburghardt April 26, 2024 11:29
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 7c2780a 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 59.4% (0.1% change).

View more on Code Climate.

Copy link
Copy Markdown
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.

Thanks

@marcusburghardt marcusburghardt merged commit ee7e506 into ComplianceAsCode:master Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ANSSI ANSSI Benchmark related. RHEL9 Red Hat Enterprise Linux 9 product related. RHEL10 Red Hat Enterprise Linux 10 product related. Update Rule Issues or pull requests related to Rules updates.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants