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

Try 4110 for file_permissions_sudo #11805

Merged
merged 7 commits into from
Apr 26, 2024
Merged

Try 4110 for file_permissions_sudo #11805

merged 7 commits into from
Apr 26, 2024

Conversation

Mab879
Copy link
Member

@Mab879 Mab879 commented Apr 8, 2024 •
edited

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 OpenShift CI
Copy link

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 GitHub Actions
Copy link

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 GitHub Actions
Copy link

github-actions bot commented Apr 8, 2024 •
edited

🤖 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
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
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
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
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
Try 4110 for file_permissions_sudo
ecfb605
@Mab879
Don't allow stricter permissions on file_permissions_sudo
4c00053 
The permissions 4110 need to be exact to ensure that sudo
operates correctly.
@Mab879
Allow sudo_dedicated_group in RHEL9 and RHEL10 ANSSI profiles
9c3fab7 
This is needed for R38.
@Mab879
Move R38 to automated status
3bb0b47
@Mab879
Add RHEL9 CCE for sudo_dedicated_group
82c4b09
@Mab879
Fix stricter permissions test for file_permission
f8eabb1 
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
marcusburghardt
marcusburghardt requested changes Apr 26, 2024
View reviewed changes
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.

Just a minor point, to remove an outdated note.

controls/anssi.yml Show resolved Hide resolved
@Mab879
Update R38 notes
7c2780a 
Since the status is now automated
@codeclimate CodeClimate
Copy link

codeclimate bot commented Apr 26, 2024

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.

marcusburghardt
marcusburghardt approved these changes Apr 26, 2024
View reviewed changes
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.

Thanks

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

@marcusburghardt marcusburghardt marcusburghardt approved these changes

Assignees

@marcusburghardt marcusburghardt

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
Milestone
0.1.73
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Mab879 @vojtapolasek @marcusburghardt