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

RHEL 10 STIG Update #12348

Merged

Conversation

Mab879
Copy link
Member

@Mab879 Mab879 commented Aug 28, 2024

Description:

  • Update RHEL 10 STIG
  • Update to V3R1 SRG GPOS

Rationale:

  • Prep for RHEL 10

Review Hints:

./build_product rhel10
cd tests
./ds_unselect_rules.sh ../build/ssg-rhel10-ds.xml unselect_rules_list
./automatus.py profile --libvirt qemu:///system automatus_rhel10 --datastream /tmp/ssg-rhel10-ds.xml stig

@Mab879 Mab879 added Update Profile Issues or pull requests related to Profiles updates. STIG STIG Benchmark related. RHEL10 Red Hat Enterprise Linux 10 product related. labels Aug 28, 2024
@Mab879 Mab879 added this to the 0.1.75 milestone Aug 28, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 28, 2024
Copy link

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

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link

github-actions bot commented Aug 28, 2024

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:12348
This image was built from commit: 560991f

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:12348

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

@Mab879 Mab879 marked this pull request as ready for review August 30, 2024 15:35
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 30, 2024
@marcusburghardt marcusburghardt self-assigned this Sep 3, 2024
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.

The updates looks good to me. I only have some minor considerations regarding consistence, such as empty lines and quotes. I also saw that some rules are using vuldiscussion in their stig policy declaration while others use vuln_discussion. Are both accepted? It would be good to keep a consistence on this too.

controls/srg_gpos/SRG-OS-000276-GPOS-00106.yml Outdated Show resolved Hide resolved
controls/srg_gpos/SRG-OS-000024-GPOS-00007.yml Outdated Show resolved Hide resolved
controls/srg_gpos/SRG-OS-000590-GPOS-00110.yml Outdated Show resolved Hide resolved
controls/srg_gpos/SRG-OS-000730-GPOS-00190.yml Outdated Show resolved Hide resolved
controls/srg_gpos/SRG-OS-000745-GPOS-00210.yml Outdated Show resolved Hide resolved
controls/srg_gpos/SRG-OS-000775-GPOS-00230.yml Outdated Show resolved Hide resolved
controls/srg_gpos/SRG-OS-000780-GPOS-00240.yml Outdated Show resolved Hide resolved
@marcusburghardt
Copy link
Member

./automatus.py profile --libvirt qemu:///session rhel10 --datastream /tmp/ssg-rhel10-ds.xml stig
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Evaluation of the profile has passed: xccdf_org.ssgproject.content_profile_stig (initial stage).
INFO - Evaluation of the profile has passed: xccdf_org.ssgproject.content_profile_stig (remediation stage).
INFO - Rebooting domain 'rhel10' before final scan.
INFO - Waiting for 30 seconds to let the system finish startup.
INFO - Evaluation of the profile has passed: xccdf_org.ssgproject.content_profile_stig (final stage).

@Mab879
Copy link
Member Author

Mab879 commented Sep 3, 2024

The updates looks good to me. I only have some minor considerations regarding consistence, such as empty lines and quotes. I also saw that some rules are using vuldiscussion in their stig policy declaration while others use vuln_discussion. Are both accepted? It would be good to keep a consistence on this too.

Thanks for the review, I have cleaned up titles and empty lines.

I cleaned up the vuln_discussion and fixed the bug the import script.

I also remove the extraneous text in the GitHub Actions file.

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.

LGTM. There is a lot of changes related to text. They were automated and shouldn't have any technical impact, but it would be good other eyes to also take a look if I missed any typo. @ggbecker , could you also take a look, please?

Copy link
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

Revert back the {{{ full_name }}} in various file. I think you do a search through all the stig files with Red Hat Enterprise Linux 9 to find where we need to revert the change.

Regarding the vuldiscussion vs vuln_discussion topic, I still see places with the vuln_discussion field. It's worth to double check that because there seems to have duplicated data there.

I see that many of the changes are only formatting options, if we can't avoid that, I suggest we do that in a different pull request... maybe having the script to detect if the text is actually different and only perform the change if there is any. Then as a follow up we can open another pull request that will just perform this type of formatting change.

@Mab879
Copy link
Member Author

Mab879 commented Sep 4, 2024

Revert back the {{{ full_name }}} in various file. I think you do a search through all the stig files with Red Hat Enterprise Linux 9 to find where we need to revert the change.

Regarding the vuldiscussion vs vuln_discussion topic, I still see places with the vuln_discussion field. It's worth to double check that because there seems to have duplicated data there.

I see that many of the changes are only formatting options, if we can't avoid that, I suggest we do that in a different pull request... maybe having the script to detect if the text is actually different and only perform the change if there is any. Then as a follow up we can open another pull request that will just perform this type of formatting change.

For how I have decided that addressing the vuldiscussion vs vuln_discussion should be reserved for another PR. So I have dropped those commits from the PR.

Copy link

codeclimate bot commented Sep 4, 2024

Code Climate has analyzed commit 560991f 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.5% (0.0% change).

View more on Code Climate.

@ggbecker
Copy link
Member

ggbecker commented Sep 4, 2024

Revert back the {{{ full_name }}} in various file. I think you do a search through all the stig files with Red Hat Enterprise Linux 9 to find where we need to revert the change.
Regarding the vuldiscussion vs vuln_discussion topic, I still see places with the vuln_discussion field. It's worth to double check that because there seems to have duplicated data there.
I see that many of the changes are only formatting options, if we can't avoid that, I suggest we do that in a different pull request... maybe having the script to detect if the text is actually different and only perform the change if there is any. Then as a follow up we can open another pull request that will just perform this type of formatting change.

I don't see this as something that blocks the PR to get merged. Specially because the changes are automatically populated and we are only interested if the content is accurate, the git history is not that valuable in this context.

For how I have decided that addressing the vuldiscussion vs vuln_discussion should be reserved for another PR. So I have dropped those commits from the PR.

Okay, sounds good to me.

Copy link
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

If you don't plan to do any further changes in this PR considering all the previous comments, then I'm good with it. Considering there will be follow ups.

@Mab879
Copy link
Member Author

Mab879 commented Sep 4, 2024

I moved policy specific content updates to #12365.

@marcusburghardt
Copy link
Member

Thanks @Mab879 and @ggbecker

@marcusburghardt marcusburghardt merged commit 5196e75 into ComplianceAsCode:master Sep 5, 2024
100 checks passed
@Mab879 Mab879 deleted the rhel10_stig_beta_update branch September 5, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RHEL10 Red Hat Enterprise Linux 10 product related. STIG STIG Benchmark related. Update Profile Issues or pull requests related to Profiles updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants