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

sshd_limit_user_access: Improve rule description, add oval and tests #10463

Merged
merged 7 commits into from
Apr 18, 2023

Conversation

dodys
Copy link
Contributor

@dodys dodys commented Apr 14, 2023

Description:

Rationale:

  • This is needed for CIS on most distros

@dodys dodys requested review from a team April 14, 2023 15:55
@dodys dodys requested a review from a team as a code owner April 14, 2023 15:55
@github-actions
Copy link

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

@github-actions
Copy link

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_sshd_limit_user_access'.
--- xccdf_org.ssgproject.content_rule_sshd_limit_user_access
+++ xccdf_org.ssgproject.content_rule_sshd_limit_user_access
@@ -4,11 +4,29 @@
 
 [description]:
 By default, the SSH configuration allows any user with an account
-to access the system. In order to specify the users that are allowed to login
-via SSH and deny all other users, add or correct the following line in the
-/etc/ssh/sshd_config file:
-AllowUsers USER1 USER2
-Where USER1 and USER2 are valid user names.
+to access the system. There are several options available to limit
+which users and group can access the system via SSH. It is
+recommended that at least one of the following options be leveraged:
+- AllowUsers variable gives the system administrator the option of
+ allowing specific users to ssh into the system. The list consists of
+ space separated user names. Numeric user IDs are not recognized with
+ this variable. If a system administrator wants to restrict user
+ access further by specifically allowing a user's access only from a
+ particular host, the entry can be specified in the form of user@host.
+- AllowGroups variable gives the system administrator the option of
+ allowing specific groups of users to ssh into the system. The list
+ consists of space separated group names. Numeric group IDs are not
+ recognized with this variable.
+- DenyUsers variable gives the system administrator the option of
+ denying specific users to ssh into the system. The list consists of
+ space separated user names. Numeric user IDs are not recognized with
+ this variable. If a system administrator wants to restrict user
+ access further by specifically denying a user's access from a
+ particular host, the entry can be specified in the form of user@host.
+- DenyGroups variable gives the system administrator the option of
+ denying specific groups of users to ssh into the system. The list
+ consists of space separated group names. Numeric group IDs are not
+ recognized with this variable.
 
 [reference]:
 11

New datastream adds OVAL for rule 'xccdf_org.ssgproject.content_rule_sshd_limit_user_access'.
OCIL for rule 'xccdf_org.ssgproject.content_rule_sshd_limit_user_access' differs.
--- ocil:ssg-sshd_limit_user_access_ocil:questionnaire:1
+++ ocil:ssg-sshd_limit_user_access_ocil:questionnaire:1
@@ -1,6 +1,6 @@
 To ensure sshd limits the users who can log in, run the following:
-$ sudo grep AllowUsers /etc/ssh/sshd_config
-If properly configured, the output should be a list of usernames allowed to log in
-to this system.
+pre>$ sudo grep -rPi '^\h*(allow|deny)(users|groups)\h+\H+(\h+.*)?$' /etc/ssh/sshd_config*
+If properly configured, the output should be a list of usernames and/or
+groups allowed to log in to this system.
 Is it the case that sshd does not limit the users who can log in?

@marcusburghardt marcusburghardt added this to the 0.1.68 milestone Apr 17, 2023
@marcusburghardt marcusburghardt self-assigned this Apr 17, 2023
@marcusburghardt marcusburghardt added Update Rule Issues or pull requests related to Rules updates. CIS CIS Benchmark related. labels Apr 17, 2023
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.

Would it be valid to have a test with an option defined but empty? Like AllowUsers without any user specified.

Copy link
Contributor

@freddieRv freddieRv left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@marcusburghardt
Copy link
Member

/packit test

@marcusburghardt
Copy link
Member

/packit build

@codeclimate
Copy link

codeclimate bot commented Apr 18, 2023

Code Climate has analyzed commit ef90ed0 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.

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.

Thanks for this rule update!

@marcusburghardt
Copy link
Member

Overriding CODEOWNERS since a SUSE approver is not currently available and @dodys can't approve his own PR.

@marcusburghardt marcusburghardt merged commit df637de into ComplianceAsCode:master Apr 18, 2023
31 of 33 checks passed
@jan-cerny
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CIS CIS Benchmark related. Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants