-
Notifications
You must be signed in to change notification settings - Fork 685
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
Import SRG content for RHEL9 #9574
Conversation
linux_os/guide/services/base/service_kdump_disabled/policy/stig/shared.yml
Outdated
Show resolved
Hide resolved
vuldiscussion: |- | ||
Service configuration files enable or disable features of their respective services that if configured incorrectly | ||
can lead to insecure and vulnerable configurations. Therefore, service configuration files should be owned by the | ||
correct group to prevent unauthorized changes. |
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.
For some reason the VulDiscussion column is empty in the generated HTML. Can you investigate this problem?
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.
This should be fixed now.
@@ -0,0 +1,45 @@ | |||
srg_requirement: |- |
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.
Why does the content of the policy file appear in the resolved rule yaml when I build a different product than RHEL 9, for example RHEL 8, for example I can see this text in policy_specific_content
key build/rhel8/rules/service_kdump_disabled.yml
. Is that expected behavior or not?
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.
It is. Since the content is in the shared.yml
file all products will get the content. Products can override this by adding their own files. For example, rhel8.yml
. Currently, family files (e.g., rhel.yml
) are not supported.
Hi @Mab879 , we should make sure that the imported data are the same data as in the DISA spreadsheet. We need to do it in an automated way because it's a large amount of data that isn't possible to be reviewed. So I think we need to write some simple test script. We can leverage our script that can create the SRG export spreadsheet. We can then compare the spreadsheet created from the project data with the original spreadsheet. For example, we can convert both of them to CSVs and run some sore of diff on these CSVs. What do you think? Is it feasible to write a test like that? |
It is! See the new |
There is still some more tweaking to be done on the script. I will get that pushed tomorrow. |
In some cases env_yaml can be `None`, don't assume it always be defined. This commit adds some guarding to prevent errors around working with `None`. Also adds a test case for this as well.
This occurs when loading from rendered YAML files.
This also cleaned up import_srg_spreadsheet script as well.
10c7152
to
a8d31ce
Compare
This is ready for review, the last Code Climate issue is existing code, and I don't feel the need to change it. |
Attached is an example of the output, diff.html.txt |
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 utils/srg_diff.py tool is amazing and very helpful. I generated the HTML diff. Are all the differences that are shown in my HTML diff expected? Some of them seem that there was some change meanwhile. See comments below.
The "tmux" package allows for a session lock to be implemented and configured. | ||
|
||
checktext: |- | ||
Verify that {{{ full_name }}} has the tmux package installed with the following command:$ sudo dnf list --installed tmuxtmux.x86_64 3.2a-4.el9If the tmux package is not installed, this is a finding. |
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.
Missing newlines
The DoD has mandated the use of the Common Access Card (CAC) to support identity management and personal authentication for systems covered under Homeland Security Presidential Directive (HSPD) 12, as well as making the CAC a primary component of layered protection for national security systems. | ||
|
||
checktext: |- | ||
Verify that {{{ full_name }}} has the opensc package installed with the following command:$ sudo dnf list --installed openscopensc.x86_64 0.22.0-2.el9If the opensc package is not installed, this is a finding. |
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.
missing newlines
Add or edit the following line in a system configuration file in the "/etc/sysctl.d/" directory: | ||
|
||
kernel.randomize_va_space = 2 | ||
|
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.
Why is the The system configuration files need to be reloaded for the changes to take effect.
missing here?
|
||
fs.protected_symlinks = 1 | ||
|
||
Load settings from all system configuration files with the following command: |
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.
Why is the The system configuration files need to be reloaded for the changes to take effect.
removed from hrer?
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.
It appears to be a style change from the external reviewer.
for the rsyslog daemon, which enables secure remote logging. | ||
|
||
checktext: |- | ||
Verify that {{{ full_name }}} has the rsyslog-gnutls package installed with the following command:$ sudo dnf list --installed rsyslog-gnutlsrsyslog-gnutls.x86_64 8.2102.0-101.el9_0.1If the rsyslog-gnutls package is not installed, this is a finding. |
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.
missing newlines inside
|
||
Example: | ||
|
||
```basj |
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.
bash
There are going some deltas, the plan is to fix those up in a follow-up PR. |
Code Climate has analyzed commit b1d853f and detected 3 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 83.3% (50% is the threshold). This pull request will bring the total coverage in the repository to 40.9% (0.1% change). View more on Code Climate. |
Hi @Mab879, I have discussed this with @vojtapolasek and I have some questions. Can you explain what is the meaning of the "Missing in DISA" and "Missing in CaC sections in the HTML report? I think that "Missing in DISA" is a list of rues in our STIG profile that don't have their counterpart in the DISA spreadsheet. But "Missing in CaC" contains a list of rules that exist in our project, so I'm confused. What is actually listed there? And how should I react as a reviewer when I see these lists are non-empty?
What would be the reason of fixing the differences in follow up PRs? Is it because the fixes will be complex? In general, I would prefer having the delta as minimal as possible before merging and fix everything in this PR. Also, we noticed that there are basically 3 categories of diffs in the HTML diff:
What would be the action plan for addressing diffs of each of these 3 categories? Side note: It seems that the HTML output isn't accessible. Do you know whether it's possible to generate a text diff instead? Or configure the library to generate some different HTML tags? This is not a blocker issue, but if the script would be used more frequently, it would be nice to make it accessible later. |
Accessibility note: would it be possible to use and |
Rules that are in the spreadsheet and not in the control file will be Missing in CaC.
So we have another round of feedback that is happening right now.
Accessibility is not going to be easy. I will have to change what library I use to create the diffs. Currently, I'm just using the Python standard library. |
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.
We have discussed this with @ggbecker .
The main thing that I was missing was that in future there will be another import of the DISA spreadsheet data and after that import we can easily git diff
to analyse differences between data that we have in our repo and new version of the spreadsheet. That will greatly help us with our STIG alignement process.
The diff created by the srg_diff.py serves to verify that the data is imported correctly. In theory, there should be no diff between the imported data and exported data. However, the spreadsheet changes and I don't have the exact version that was used for the import available at hand. Therefore, there is some diff.
However, this diff is small. As we will do another import of the data in future we will slowly converge to no diff produced by srg_diff.py.
The expected next import(s) of data also means why we don't insist on making the diff empty - insinsiting on it would make sense only if we are given final version of the spreadsheet that will never change.
Description:
Import SRG content for RHEL9 STIG.
Rationale:
Sync content to the repo.