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.

Sign up for GitHub

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

Jump to bottom

Fixed the sshd_set_idle_timeout rule #6293

Closed
wants to merge 3 commits into from
Closed

Fixed the sshd_set_idle_timeout rule #6293

wants to merge 3 commits into from

Conversation

matejak
Copy link
Member

@matejak matejak commented Oct 23, 2020 •
edited

The configuration works if and only if ClientAliveCountMax is set to 0.

Therefore, the check and remediations for this rule take both configuration items into account.
The rule description has been clarified to warn from situations when the timeout isn't effective.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1859551

@matejak matejak added this to the 0.1.53 milestone Oct 23, 2020
redhatrises
redhatrises suggested changes Oct 23, 2020
View reviewed changes
Copy link
Contributor

@redhatrises redhatrises left a comment

Choose a reason for hiding this comment

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

ClientAliveCountMax needs to maintain itself in a new rule. One configuration change per rule only. This is a requirement.

@matejak
Copy link
Member Author

matejak commented Oct 23, 2020

Could you please clarify that "one rule = change to one configuration line" requirement?

I see that we can choose between this requirement and a clean, self-contained rule. We know what a self-contained rule is, but we don't know anything about that requirement.

@matejak
Fixed documentation for the sshd_lineinfile template.
a412e84
@matejak
Fixed the sshd_set_idle_timeout rule.
35eb2b6 
The configuration works if and only if ClientAliveCountMax is set to 0.
Therefore, the check for this rule take both configuration items into account.
@matejak
Set the sshd_set_keepalive rule to set the option to zero.
d0a177c 
All other values don't make any sense from the security POV,
so the parametrization has been removed.
Thanks to that, the rule may now use a template.
@openscap-ci
Copy link
Collaborator

openscap-ci commented Oct 27, 2020 •
edited

Changes identified:
Rules:
 sshd_set_idle_timeout
 sshd_set_keepalive
Profiles:
 ospp on ol8
 cui on ol8
 rhelh-stig on rhel7
 cis on rhel8
 ospp on rhel8
 ospp-mls on rhel8
 rhelh-vpp on rhel8
 cui on rhel8
 stig on rhel8
 rhelh-stig on rhel8
 rhvh-stig on rhv4
 cis on sle15
 cis on ubuntu1804

Show details

Rule sshd_set_idle_timeout:
 New node inserted to OVAL check.
 Found change in bash remediation.
Rule sshd_set_keepalive:
 Bash remediation was deleted. No test for it will be selected.
 Ansible remediation for sshd_set_keepalive was deleted.
 OVAL check for sshd_set_keepalive was deleted.
Profile ospp on ol8:
 Variable var_sshd_set_keepalive=0 removed from ospp profile.
Profile cui on ol8:
 CUI profile extends changed OSPP profile.
Profile rhelh-stig on rhel7:
 Variable var_sshd_set_keepalive=3 removed from rhelh-stig profile.
Profile cis on rhel8:
 Variable var_sshd_set_keepalive=0 removed from cis profile.
Profile ospp on rhel8:
 Variable var_sshd_set_keepalive=0 removed from ospp profile.
Profile ospp-mls on rhel8:
 OSPP-MLS profile extends changed OSPP profile.
Profile rhelh-vpp on rhel8:
 RHELH-VPP profile extends changed OSPP profile.
Profile cui on rhel8:
 CUI profile extends changed OSPP profile.
Profile stig on rhel8:
 STIG profile extends changed OSPP profile.
Profile rhelh-stig on rhel8:
 RHELH-STIG profile extends changed STIG profile.
Profile rhvh-stig on rhv4:
 Variable var_sshd_set_keepalive=3 removed from rhvh-stig profile.
Profile cis on sle15:
 Variable var_sshd_set_keepalive=0 removed from cis profile.
Profile cis on ubuntu1804:
 Variable var_sshd_set_keepalive=0 removed from cis profile.

Recommended tests to execute:
 build_product ubuntu1804
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-ubuntu1804-ds.xml cis
 build_product rhel8
 tests/test_suite.py rule --libvirt qemu:///system test-suite-vm --remediate-using bash --datastream build/ssg-rhel8-ds.xml sshd_set_idle_timeout
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml cis
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml ospp
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml stig
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml rhelh-stig
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml rhelh-vpp
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml ospp-mls
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml cui
 build_product rhel7
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel7-ds.xml rhelh-stig
 build_product rhv4
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhv4-ds.xml rhvh-stig
 build_product ol8
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-ol8-ds.xml ospp
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-ol8-ds.xml cui
 build_product sle15
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-sle15-ds.xml cis

redhatrises
redhatrises reviewed Oct 29, 2020
View reviewed changes
linux_os/guide/services/ssh/ssh_server/sshd_set_idle_timeout/rule.yml
@@ -8,7 +8,7 @@ description: |-
<br /><br />
To set an idle timeout interval, edit the following line in <tt>/etc/ssh/sshd_config</tt> as
follows:
<pre>ClientAliveInterval <b>{{{ xccdf_value("sshd_idle_timeout_value") }}}</b></pre>
<pre>ClientAliveInterval <b>{{{ xccdf_value("sshd_idle_timeout_value") }}}</b></pre>.
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick... this period . should be removed as it doesn't read right.

@matejak matejak mentioned this pull request Oct 30, 2020
Merged
@ggbecker ggbecker mentioned this pull request Nov 2, 2020
Merged
6 tasks
@redhatrises
Copy link
Contributor

Fixed and merged

@redhatrises redhatrises closed this Nov 3, 2020
@matejak
Copy link
Member Author

matejak commented Nov 4, 2020 •
edited

I assume that closing of the PR without merge was not intentional, so I suggest to repoen it, as the period has been removed.

@matejak
Copy link
Member Author

matejak commented Nov 5, 2020

So this is a really weird glitch - the branch has been merged, but this PR doesn't show it. See e.g. https://github.com/ComplianceAsCode/content/tree/master/linux_os/guide/services/ssh/ssh_server/sshd_set_idle_timeout for the proof that this PR has been really merged.

@matejak matejak added the bugfix Fixes to reported bugs. label Nov 5, 2020
@ggbecker
Copy link
Member

ggbecker commented Nov 6, 2020

So this is a really weird glitch - the branch has been merged, but this PR doesn't show it. See e.g. https://github.com/ComplianceAsCode/content/tree/master/linux_os/guide/services/ssh/ssh_server/sshd_set_idle_timeout for the proof that this PR has been really merged.

The reason is because @redhatrises has fixed the request by himself and I'm guessing he pushed straight to master since you can see that he has co-authored the commits, for example: 5ab82b4

@matejak
Copy link
Member Author

matejak commented Nov 6, 2020

OK, that makes sense.
I would prefer to push to the branch, or to suggest a typo-fixing commit like that and merge it, as that would make the history cleaner.

ggbecker added a commit to ggbecker/content that referenced this pull request Nov 6, 2020
@ggbecker
Backport developer guide changes from ComplianceAsCode#6293.
0905004
ggbecker added a commit to ggbecker/content that referenced this pull request Nov 6, 2020
@ggbecker
Backport developer guide changes from ComplianceAsCode#6293.
7f4b468
ggbecker added a commit to ggbecker/content that referenced this pull request Nov 9, 2020
@ggbecker
Backport developer guide changes from ComplianceAsCode#6293.
dc59737
@vojtapolasek vojtapolasek added the backported-into-stabilization PRs which were cherry-picked during stabilization process. label Nov 12, 2020
@vojtapolasek vojtapolasek modified the milestones: 0.1.53, 0.1.54 Nov 12, 2020
@vojtapolasek vojtapolasek added this to the 0.1.53 milestone Nov 12, 2020
@ggbecker ggbecker mentioned this pull request Nov 23, 2020
Closed
@yuumasato yuumasato mentioned this pull request Apr 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@redhatrises redhatrises redhatrises requested changes

Assignees
No one assigned
Labels
backported-into-stabilization PRs which were cherry-picked during stabilization process. bugfix Fixes to reported bugs.
Projects
None yet
Milestone
0.1.53
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@matejak @openscap-ci @redhatrises @ggbecker @vojtapolasek