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

Improve rule file_permissions_ungroupowned for use in bootable containers #12584

Merged

Conversation

jan-cerny
Copy link
Collaborator

Description:

  • Exclude /sysroot from scanning
  • Get additional group data from /usr/lib/group
  • Add test scenarios

For more details, please read commit messages of all commits.

Rationale:

In systems based on bootable container images the /sysroot directory contains the filesystem of the image which should be excluded from the scanned files check.

If the nss-altfiles are installed and /etc/nsswitch.conf is configured to use nss-altfiles, the users group can be defined
also in /usr/lib/group next to /etc/group. The /usr/lib/group is a valid source of group definitions and therefore needs to be consulted during the check if nsswitch is configured to use this file. The nss-altfiles is often used in bootable containers base images.

Review Hints:

Build CS9 data stream and apply STIG profile in podman build of an image based on quay.io/centos-bootc/centos-bootc:stream9.

In systems based on bootable container images the `/sysroot`
directory contains the filesystem of the image which should be
excluded from the scanned files check.
If the `nss-altfiles` are installed and `/etc/nsswitch.conf` is
configured to use `nss-altfiles`, the users group can be defined
als in `/usr/lib/group` next to `/etc/group`. The `/usr/lib/group`
is a valid source of group definitions and therefore needs to
be consulted during the check if nsswitch is configured to use
this file. The `nss-altfiles` is often used in bootable containers
base images.
Add new Automatus test scenarios for rule file_permissions_ungroupowned
that cover recent changes - exclusion of /sysroot and addition
of /usr/lib/group as second source of group data.
@jan-cerny jan-cerny added OVAL OVAL update. Related to the systems assessments. Update Rule Issues or pull requests related to Rules updates. Image Mode Bootable containers and Image Mode RHEL labels Nov 7, 2024
@jan-cerny jan-cerny added this to the 0.1.76 milestone Nov 7, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Nov 7, 2024
Copy link

openshift-ci bot commented Nov 7, 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

Copy link

github-actions bot commented Nov 7, 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

Reflects the recent change that the check now considers /usr/lib/group
in addition.
Copy link

github-actions bot commented Nov 7, 2024

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
New content has different text for rule 'xccdf_org.ssgproject.content_rule_file_permissions_ungroupowned'.
--- xccdf_org.ssgproject.content_rule_file_permissions_ungroupowned
+++ xccdf_org.ssgproject.content_rule_file_permissions_ungroupowned
@@ -3,9 +3,11 @@
 Ensure All Files Are Owned by a Group
 
 [description]:
-If any file is not group-owned by a group present in /etc/group, the cause of the lack of
+If any file is not group-owned by a valid defined group, the cause of the lack of
 group-ownership must be investigated. Following this, those files should be deleted or
-assigned to an appropriate group.
+assigned to an appropriate group. The groups need to be defined in /etc/group
+or in /usr/lib/group if nss-altfiles are configured to be used
+in /etc/nsswitch.conf.
 
 Locate the mount points related to local devices by the following command:
 $ findmnt -n -l -k -it $(awk '/nodev/ { print $2 }' /proc/filesystems | paste -sd,)
@@ -16,7 +18,7 @@
 
 [warning]:
 This rule only considers local groups as valid groups.
-If you have your groups defined outside /etc/group, the rule won't consider those.
+If you have your groups defined outside /etc/group or /usr/lib/group, the rule won't consider those.
 
 [warning]:
 This rule can take a long time to perform the check and might consume a considerable

OVAL for rule 'xccdf_org.ssgproject.content_rule_file_permissions_ungroupowned' differs.
--- oval:ssg-file_permissions_ungroupowned:def:1
+++ oval:ssg-file_permissions_ungroupowned:def:1
@@ -1,2 +1,7 @@
+criteria OR
 criteria AND
+criterion oval:ssg-test_file_permissions_ungroupowned_nsswitch_uses_altfiles:tst:1
 criterion oval:ssg-test_file_permissions_ungroupowned:tst:1
+criteria AND
+criterion oval:ssg-test_file_permissions_ungroupowned_nsswitch_uses_altfiles:tst:1
+criterion oval:ssg-test_file_permissions_ungroupowned_with_usrlib:tst:1

@jan-cerny
Copy link
Collaborator Author

jan-cerny commented Nov 13, 2024

I think this won't cover all situations. In bootable containers, for "system" users it's strongly recommended to use systemd DynamicUser=yes where possible. See https://containers.github.io/bootc/building/users-and-groups.html#using-dynamicuseryes-for-systemd-units. This isn't covered by our OVAL.

This will ensure that the test will use the groups from nssaltfiles
only if the nsswitch is configured to use nssaltfiles.
@jan-cerny
Copy link
Collaborator Author

I have changed the OVAL so that it will consider the /usr/lib/group only if the /etc/nssswitch.conf is configured to consider it.

I can't cover the systemd DynamicUser by OVAL.

I will mark the PR as ready.

@jan-cerny jan-cerny marked this pull request as ready for review November 27, 2024 09:45
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Nov 27, 2024
Copy link

codeclimate bot commented Nov 27, 2024

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

View more on Code Climate.

@matusmarhefka matusmarhefka self-assigned this Nov 27, 2024
Copy link
Member

@matusmarhefka matusmarhefka left a comment

Choose a reason for hiding this comment

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

Tested in bootable container and with Automatus, LGTM.

$ ./automatus.py rule --libvirt qemu:///session ssgts_94 --datastream ../build/ssg-rhel9-ds.xml --remediate-using bash file_permissions_ungroupowned
INFO - xccdf_org.ssgproject.content_rule_file_permissions_ungroupowned
INFO - Script unowned_file.fail.sh using profile (all) OK
INFO - Script all_owned.pass.sh using profile (all) OK
INFO - Script group_in_usr_lib.pass.sh using profile (all) OK
INFO - Script unowned_in_sysroot.pass.sh using profile (all) OK

@matusmarhefka matusmarhefka merged commit 8812908 into ComplianceAsCode:master Nov 27, 2024
96 of 103 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Image Mode Bootable containers and Image Mode RHEL OVAL OVAL update. Related to the systems assessments. Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants