-
Notifications
You must be signed in to change notification settings - Fork 698
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
Introduce Fedora and Firefox CaC profiles for common workstation users #10506
Introduce Fedora and Firefox CaC profiles for common workstation users #10506
Conversation
Skipping CI for Draft Pull Request. |
/test all |
This datastream diff is auto generated by the check Click here to see the full diffNew datastream adds bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_umask_etc_login_defs'.
New datastream adds bash remediation for rule 'xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll'.
New datastream adds ansible remediation for rule 'xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I think will be good addition to the project. I have a couple of nitpicks on top of the CI findings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two gating items I can already see that will need to be addressed,
- the order in which the
prodtype
fedora
has been put. It needs to be in the alphabetical order - I noticed at least one file without the new line at then end
controls/cusp_firefox.yml
, but there is more. If you used vscode, I suggest enabling the option to add newline at the end whenever a file is saved. You can pressCtrl+,
(Control + Comma), it will open the preferences window, then typeInsert Final Newline
and make sure it is ticked.
@j-ode I remember we have this: https://github.com/ComplianceAsCode/content/blob/master/utils/autoprodtyper.py please use it. |
controls/cusp_fedora.yml
Outdated
- sysctl_net_ipv4_ip_forward | ||
- sysctl_net_ipv6_conf_all_forwarding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the use of VMs a supported use case for the profiles?
Once IP forwarding is disabled in a host the guest VMs will lose access to the external world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out, I suppose this profile should be VM-friendly as well. I looked at the two rules, and the ipv4 one has a jinja macro in the platform: https://github.com/ComplianceAsCode/content/blob/f31f13744364d29489e2a414a6a3fd43957ca3bd/shared/macros/01-general.jinja#LL972C1-L977C17 that disables the rule if rhel8 product uses oVirt. I could create a new macro that would apply the machine platform for fedora product only, but I wonder if that could have adverse effect on other potential fedora profiles that do not consider the VM usecase. What are your thoughts on such a solution @yuumasato?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may be able to use CPE applicability language here. Something like package[libvirtd]
as a platform on Fedora might do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I am finalizing the source code attachment for my thesis, I just removed the rules in question for now, but I like the idea of using the CPE applicability language
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess making the rules for IP forwarding not applicable when an VM hypervisor is installed makes sense.
But could there be usecases that expect VMs to not have access to the external world?
I.e. I have VMS which I don't want to access the outside networks, they only need to talk to their host and other VMs in the same virtual network.
I think this is unlikely, but I don't have any data on this.
In case of oVirt
and RHV4
product, we expect that VMs have access to the external world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that libvirt is installed by default on Fedora 38 Workstation:
The purpose of this profile is to increase security but at the same time maintain usability, so I believe that users should be able to run VMs on their machines without connectivity problems (most users probably want networking inside their VMs). I am not sure there is a way to include these rules in the profile.
17a0028
to
ffde5ac
Compare
In this case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ensure the all descriptions are in sentence case (first letter of sentence is capitalized). The title keys should be title case.
Also, my original comments from my first review still stand, it seems that one of the rebases must have gotten rid of your changes.
controls/cusp_fedora.yml
Outdated
- sysctl_net_ipv4_ip_forward | ||
- sysctl_net_ipv6_conf_all_forwarding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may be able to use CPE applicability language here. Something like package[libvirtd]
as a platform on Fedora might do it.
title: Disk partitioning | ||
description: >- | ||
Users should put the /home, /tmp, /var, /var/tmp and /var/log directories on separate partitions. | ||
status: manual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rules from linux_os/guide/system/software/disk_partitioning
might be useful here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the issue is that the respective Anaconda remediations block the installation when the disk is not correctly partitioned which is not ideal, I talk about this in the thesis and created an issue in OAA OpenSCAP/oscap-anaconda-addon#236
ffde5ac
to
aed07b4
Compare
600ba48
to
9518bf2
Compare
The Automatus CS8/CS9 CI fails are expected, as package_gnome_software_installed is a fedora-only rule and therefore cannot be present in the RHEL8/9 benchmarks as the errors suggest |
Source of the controls file will be my thesis on the Fedora Common User Profile, source will be added later
This reverts commit 7ca5bf4. Fix EOL newline and wrong prodtype
Co-authored-by: Matthew Burket <[email protected]>
Co-authored-by: Matthew Burket <[email protected]>
Make titles title case, descriptions sentence case
Make titles title case, descriptions sentence case
9518bf2
to
4bd2e87
Compare
Code Climate has analyzed commit 4bd2e87 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 52.4% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fail in centos8 testing farm job is related to rule rsyslog_files_permissions and will be fixed by #10540, therefore it isn't related to the contents of this PR.
feedback has been addressed
Description:
Rationale: