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

Groupw bugfixes #13

Closed

Conversation

PoundsOfFlesh
Copy link

Overall Review of Changes:
These changes result from applying the Ansible role to a fresh install of RHEL 9.3 and fixing Ansible-produced errors. Then, the DISA SCAP tool was executed on the system. Further changes were made to the role to address the findings.

Issue Fixes:
No issues were fixed

Enhancements:
No enhancements were made. Only bug fixes.

How has this been tested?:
The changes were tested by running an Ansible playbook that activated the role. The DISA SCAP tool was used to discover findings.

The missing variable caused an error message. Adding it to the role's defaults with a true
value eliminates the message.

Signed-off-by: PoundsOfFlesh <[email protected]>
Removed the hyphen on the loop statement. The rhel9stig_rsyslog_conf.files variable is
already an array. The hyphen creates an array containing this array as an element.

Signed-off-by: PoundsOfFlesh <[email protected]>
The lineinfile module produces a error if the file does not exist. Adding the directive allows the
module to create the file.

Signed-off-by: PoundsOfFlesh <[email protected]>
The value stored in rhel9stig_sysctl_file.kernel was not the correct file to modify. Changed the
sysctl module to modify /usr/lib/sysctl.d/10-default-yama-scope.conf.

Signed-off-by: PoundsOfFlesh <[email protected]>
The SCAP tool checks the openssh.config file for the correct cyphers. Changed the lineinfile
module to update openssh.conf instead of opensshserver.config.

Signed-off-by: PoundsOfFlesh <[email protected]>
The X11forwarding option should be in the file /etc/ssh/sshd_config.d/50-redhat.conf.

Signed-off-by: PoundsOfFlesh <[email protected]>
The full dconf path is /org/gnome/login-screen/banner-message-enable.

Signed-off-by: PoundsOfFlesh <[email protected]>
The SCAP tool does not handle extra spaces when checking this rule.

Signed-off-by: PoundsOfFlesh <[email protected]>
The picture-uri cannot be blank. It must be an empty string ("").

Signed-off-by: PoundsOfFlesh <[email protected]>
The RHEL-09-271095 rule modified security settings. This appeared to be copy/paste error.
Corrected the rule to update the 02-login-screen file.

Signed-off-by: PoundsOfFlesh <[email protected]>
The stdout_lines var is already an array. The hyphen in the loop clause creates an array
containing an array.

Signed-off-by: PoundsOfFlesh <[email protected]>
If the /etc/sudoers.d folder is empty, grep produces an error when the file glob '*' is used.
The -r option was added to recurse into the empty folder without an error.

Signed-off-by: PoundsOfFlesh <[email protected]>
The grep command separates the filename from the matched text with a colon. Split the
output on the colon character and use the first element as the filename.

Signed-off-by: PoundsOfFlesh <[email protected]>
The regular expression had an erroneous "\".

Signed-off-by: PoundsOfFlesh <[email protected]>
Several names had the prefix RHEL-08 instead of RHEL-09.

Signed-off-by: PoundsOfFlesh <[email protected]>
Correct conditional check so task runs if inactive user setting *is* set to -1.

Signed-off-by: PoundsOfFlesh <[email protected]>
The SCAP tool checks the permissions on the root user home folder.

Signed-off-by: PoundsOfFlesh <[email protected]>
The variable, rhel9stig_library_directory_perms.stdout_lines, is already an array. Remove the hyphen in the loop statement.

Signed-off-by: PoundsOfFlesh <[email protected]>
The SCAP tool does not consider spaces when scanning NetworkManager.conf.

Signed-off-by: PoundsOfFlesh <[email protected]>
The SCAP tool looks for 'true' (with quotes) when scanning the 02-login-screen file.

Signed-off-by: PoundsOfFlesh <[email protected]>
The file objects returned by the find module have a path attribute.

Signed-off-by: PoundsOfFlesh <[email protected]>
The opensc package installs the opensc.conf file. Move RHEL_09_611185 before
RHEL_09_611160 so that opensc is installed before trying to modify opensc.conf.

Signed-off-by: PoundsOfFlesh <[email protected]>
Add a task to RHEL_09_412075 to remove the silent option from pam_lastlog in the postlogin
file.

Signed-off-by: PoundsOfFlesh <[email protected]>
The SCAP tool always checks for umount auditing.

Signed-off-by: PoundsOfFlesh <[email protected]>
The rhel9stig_users_passwd_max.stdout_lines variable is already an array. Adding the hyphen
creates an array within an array.

Signed-off-by: PoundsOfFlesh <[email protected]>
Correct grep to find mounts without the nodev option.

Signed-off-by: PoundsOfFlesh <[email protected]>
The grep to search for FIPS contained  -V, which prints the version. The intent was a lowercase
v to find lines not containing FIPS.

Signed-off-by: PoundsOfFlesh <[email protected]>
The lineinfile module does not have a modification_time parameter.

Signed-off-by: PoundsOfFlesh <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

Trying to see why this would be changed? The original allows those to add where they wish to configure or have already configured? the requirements is
Fix Text: Configure RHEL 9 to restrict usage of ptrace to descendant processes by adding the following line to a file, in the "/etc/sysctl.d" directory:

Copy link
Author

Choose a reason for hiding this comment

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

I discovered that the SCAP tool checks in the file /usr/lib/sysctl.d/10-default-yama-scope.conf. The var rhel9stig_sysctl_file.kernel is used in several places, so I was reluctant to change it. I considered adding another attribute to rhel9stig_sysctl_file called yama, or yama_scope. I agree that hard-coding the file name removed flexibility. If you have a suggestion in mind, I will implement it.

Here are the test results that failed in the SCAP tool for V-257811:

Test ID: oval:mil.disa.stig.unix:tst:23054600 (sysctl_test)
Result: true
Title: kernel.yama.ptrace_scope setting in kernel is set to 1
Check Existence: All collected items must exist.
Check: All collected items must match the given state(s).
Object ID: oval:mil.disa.stig.unix:obj:23054600 (sysctl_object)
Object Requirements:
name must be equal to 'kernel.yama.ptrace_scope'
State ID: oval:mil.disa.stig.unix:ste:20000010 (sysctl_state)
State Requirements:
check_existence = 'at_least_one_exists', value must be equal to '1'
Test ID: oval:mil.disa.stig.ind:tst:23054601 (textfilecontent54_test)
Result: false
Title: kernel.yama.ptrace_scope in sysctl configuration files is set to 1, and nothing else, and there are no conflicting settings in other files
Check Existence: One or more collected items must exist.
Check: All collected items must match the given state(s).
Object ID: oval:mil.disa.stig.ind:obj:23054603 (textfilecontent54_object)
Object Requirements:
Collect any available items.
State ID: oval:mil.disa.stig.ind:ste:20000003 (textfilecontent54_state)
State Requirements:
check_existence = 'at_least_one_exists', subexpression must be equal to '1'
Collected Item/State Result:
[ false ]
filepath equals '/usr/lib/sysctl.d/10-default-yama-scope.conf'
path equals '/usr/lib/sysctl.d'
filename equals '10-default-yama-scope.conf'
pattern equals '(?:^|.\n)\skernel.yama.ptrace_scope\s*=\s*(\d+)\s*$'
instance equals '1'
text equals '
kernel.yama.ptrace_scope = 0

'
subexpression equals '0'
Collected Item/State Result:
[ false ]
filepath equals '/lib/sysctl.d/10-default-yama-scope.conf'
path equals '/lib/sysctl.d'
filename equals '10-default-yama-scope.conf'
pattern equals '(?:^|.\n)\skernel.yama.ptrace_scope\s*=\s*(\d+)\s*$'
instance equals '1'
text equals '
kernel.yama.ptrace_scope = 0

'
subexpression equals '0'
Additional Information: Check requirement not met.
subexpression
subexpression

Copy link
Member

Choose a reason for hiding this comment

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

hi @PoundsOfFlesh

This is exactly the issue we see with many scanners, they do not align with the actual documentation, but implement their own rules.
In this case the documentation says it needs to be set in the extra conf directory and gives an example, it doesn't state that is what the file must be called.
This is a false positive which we see with many scanners, it gets worse when they change the name of the files in the examples and then everyone assumes they also need to make that change on all their hosts, potentially leading to changes that are not necessary being carried out.

For this reason we do not align with scanners we align with the documentation and the requirements.

So while you can add another variable if you wish and default back to the original (thereby not affect all the other users) it is a change that is not necessary.

Thanks

uk-bolly

Copy link
Member

Choose a reason for hiding this comment

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

for 232020 - why are we removing special bits? These maybe set for a reason doing more than asked or documented may cause confusion on why this doesn't match documented values

Copy link
Author

Choose a reason for hiding this comment

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

The SCAP tool checks for SUID and SGID special permissions. I assumed it was an oversight. Here is the definition from the SCAP tool:

Definition ID: oval:mil.disa.stig.rhel9os:def:257884
Result: false
Title: RHEL-09-232020 - RHEL 9 library files must have mode 755 or less permissive.
Description: If RHEL 9 allowed any user to make changes to software libraries, then those changes might be implemented without undergoing the appropriate testing and approvals that are part of a robust change management process.

This requirement applies to RHEL 9 with software libraries that are accessible and configurable, as in the case of interpreted languages. Software libraries also include privileged programs that execute with escalated privileges.
Class: compliance
Tests:
false (All child checks must be true.)
false (All child checks must be true.)
false (no operating system library regular files have the SUID special permission set)
false (no operating system library regular files have the SGID special permission set)
true (no operating system library regular files have the sticky bit set)
true (no operating system library regular files are group-writable)
true (no operating system library regular files are world-writable)

Copy link
Member

Choose a reason for hiding this comment

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

See comment above between documentation and what the scanner is actually looking for.

thanks

@uk-bolly
Copy link
Member

hi @PoundsOfFlesh

Awesome PR, thank you so much for taking the time on this.
Just to confirm, we are trying to align with specific releases of STIG, this is currently with Jan2024. With April2024 to be released approx end of this month.
For specific rules, we are trying to align as much as possible with documentation, so adding some items that are not in the documentation will likely cause confusion for others (although as often happens this does occur in later releases).
I believe you are also on the discord, happy to chat on there if that is quicker?

Many thanks

uk-bolly

uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
uk-bolly added a commit that referenced this pull request Jul 9, 2024
Signed-off-by: Mark Bolwell <[email protected]>
@uk-bolly
Copy link
Member

hi @PoundsOfFlesh

I have incorporated a lot of the changes you have provided in the latest PR with credits. Thank you once again for the time and effort, we truly understand how much work is involved.
I hope that he discussions clear up any questions that you may have.
Please let us know if there is anything outstanding.

many thanks

uk-bolly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants