From 1052277e4525b139d24065db576f8bd750b8da36 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 21 Mar 2022 20:17:52 +0100 Subject: [PATCH 20/39] PermitRootLogin check: add new use cases for 8to9 Signed-off-by: Jakub Jelen --- .../opensshpermitrootlogincheck/actor.py | 79 +++++++++++++++++-- .../libraries/opensshpermitrootlogincheck.py | 8 ++ ...est_library_opensshpermitrootlogincheck.py | 26 +++++- 3 files changed, 104 insertions(+), 9 deletions(-) diff --git a/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/actor.py b/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/actor.py index f13a7672..f7ee61da 100644 --- a/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/actor.py +++ b/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/actor.py @@ -1,7 +1,8 @@ from leapp import reporting from leapp.actors import Actor from leapp.exceptions import StopActorExecutionError -from leapp.libraries.actor.opensshpermitrootlogincheck import semantics_changes +from leapp.libraries.actor.opensshpermitrootlogincheck import global_value, semantics_changes +from leapp.libraries.common.config.version import get_source_major_version from leapp.libraries.stdlib import api from leapp.models import OpenSshConfig, Report from leapp.reporting import create_report @@ -14,13 +15,21 @@ COMMON_REPORT_TAGS = [ reporting.Tags.SERVICES ] +COMMON_RESOURCES = [ + reporting.RelatedResource('package', 'openssh-server'), + reporting.RelatedResource('file', '/etc/ssh/sshd_config') +] + class OpenSshPermitRootLoginCheck(Actor): """ OpenSSH no longer allows root logins with password. Check the values of PermitRootLogin in OpenSSH server configuration file - and warn about potential issues after update. + and warn about potential issues after upgrade to the next major version of RHEL. + + The RHEL8 still provided default configuration that allowed root logins, + which can lead to possible unwanted changes during the upgrade """ name = 'openssh_permit_root_login' consumes = (OpenSshConfig, ) @@ -37,10 +46,15 @@ class OpenSshPermitRootLoginCheck(Actor): 'Could not check openssh configuration', details={'details': 'No OpenSshConfig facts found.'} ) - resources = [ - reporting.RelatedResource('package', 'openssh-server'), - reporting.RelatedResource('file', '/etc/ssh/sshd_config') - ] + if get_source_major_version() == '7': + self.process7to8(config) + elif get_source_major_version() == '8': + self.process8to9(config) + else: + api.current_logger().warning('Unknown source major version: {} (expecting 7 or 8)' + .format(get_source_major_version())) + + def process7to8(self, config): # When the configuration does not contain the PermitRootLogin directive and # the configuration file was locally modified, it will not get updated by # RPM and the user might be locked away from the server. Warn the user here. @@ -61,7 +75,7 @@ class OpenSshPermitRootLoginCheck(Actor): '"PermitRootLogin yes" to sshd_config.' ), reporting.Flags([reporting.Flags.INHIBITOR]) - ] + resources) + ] + COMMON_RESOURCES) # Check if there is at least one PermitRootLogin other than "no" # in match blocks (other than Match All). @@ -87,4 +101,53 @@ class OpenSshPermitRootLoginCheck(Actor): 'in global context if desired.' ), reporting.Flags([reporting.Flags.INHIBITOR]) - ] + resources) + ] + COMMON_RESOURCES) + + def process8to9(self, config): + # RHEL8 default sshd configuration file is not modified: It will get replaced by rpm and + # root will no longer be able to connect through ssh. This will probably result in many + # false positives so it will have to be waived a lot + if not config.modified: + create_report([ + reporting.Title('Possible problems with remote login using root account'), + reporting.Summary( + 'OpenSSH configuration file will get updated to RHEL9 ' + 'version, no longer allowing root login with password. ' + 'It is a good practice to use non-root administrative ' + 'user and non-password authentications, but if you rely ' + 'on the remote root login, this change can lock you out ' + 'of this system.' + ), + reporting.Severity(reporting.Severity.HIGH), + reporting.Tags(COMMON_REPORT_TAGS), + reporting.Remediation( + hint='If you depend on remote root logins using passwords, ' + 'consider setting up a different user for remote ' + 'administration or adding a comment into the ' + 'sshd_config next to the "PermitRootLogin yes" directive ' + 'to prevent rpm replacing it during the upgrade.' + ), + reporting.Flags([reporting.Flags.INHIBITOR]) + ] + COMMON_RESOURCES) + # If the configuration is modified and contains any directive allowing + # root login (which is in default configuration), we are upgrading to + # RHEL9 keeping the old "security policy", which might keep the root + # login unexpectedly open. This might be just high priority warning + if global_value(config, 'prohibit-password') == 'yes': + create_report([ + reporting.Title('Remote root logins globally allowed using password'), + reporting.Summary( + 'RHEL9 no longer allows remote root logins, but the ' + 'server configuration explicitly overrides this default. ' + 'The configuration file will not be updated and root is ' + 'still going to be allowed to login with password. ' + 'This is not recommended and considered as a security risk.' + ), + reporting.Severity(reporting.Severity.HIGH), + reporting.Tags(COMMON_REPORT_TAGS), + reporting.Remediation( + hint='If you depend on remote root logins using passwords, ' + 'consider setting up a different user for remote ' + 'administration. Otherwise you can ignore this message.' + ) + ] + COMMON_RESOURCES) diff --git a/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/libraries/opensshpermitrootlogincheck.py b/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/libraries/opensshpermitrootlogincheck.py index 0cb90819..d247b220 100644 --- a/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/libraries/opensshpermitrootlogincheck.py +++ b/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/libraries/opensshpermitrootlogincheck.py @@ -1,8 +1,16 @@ +def global_value(config, default): + for opt in config.permit_root_login: + if (opt.in_match is None or opt.in_match[0].lower() == 'all'): + return opt.value + return default def semantics_changes(config): globally_enabled = False in_match_disabled = False + if not config.permit_root_login: + return True + for opt in config.permit_root_login: if opt.value != "yes" and opt.in_match is not None \ and opt.in_match[0].lower() != 'all': diff --git a/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/tests/test_library_opensshpermitrootlogincheck.py b/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/tests/test_library_opensshpermitrootlogincheck.py index 23110839..6ccd5851 100644 --- a/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/tests/test_library_opensshpermitrootlogincheck.py +++ b/repos/system_upgrade/common/actors/opensshpermitrootlogincheck/tests/test_library_opensshpermitrootlogincheck.py @@ -1,7 +1,20 @@ -from leapp.libraries.actor.opensshpermitrootlogincheck import semantics_changes +from leapp.libraries.actor.opensshpermitrootlogincheck import global_value, semantics_changes from leapp.models import OpenSshConfig, OpenSshPermitRootLogin +def test_empty_file(): + """ Empty file + """ + config = OpenSshConfig( + permit_root_login=[ + ], + deprecated_directives=[] + ) + + assert semantics_changes(config) + assert global_value(config, "default") == "default" + + def test_globally_enabled(): """ Configuration file in this format: @@ -17,6 +30,7 @@ def test_globally_enabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "yes" def test_globally_disabled(): @@ -34,6 +48,7 @@ def test_globally_disabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "no" def test_globally_disabled_password(): @@ -51,6 +66,7 @@ def test_globally_disabled_password(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "prohibit-password" def test_in_match_disabled(): @@ -70,6 +86,7 @@ def test_in_match_disabled(): ) assert semantics_changes(config) + assert global_value(config, "default") == "default" def test_in_match_disabled_password(): @@ -89,6 +106,7 @@ def test_in_match_disabled_password(): ) assert semantics_changes(config) + assert global_value(config, "default") == "default" def test_in_match_enabled(): @@ -109,6 +127,7 @@ def test_in_match_enabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "default" def test_in_match_all_disabled(): @@ -128,6 +147,7 @@ def test_in_match_all_disabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "no" def test_in_match_all_disabled_password(): @@ -147,6 +167,7 @@ def test_in_match_all_disabled_password(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "prohibit-password" def test_in_match_all_enabled(): @@ -166,6 +187,7 @@ def test_in_match_all_enabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "yes" def test_in_match_enabled_globally_disabled(): @@ -188,6 +210,7 @@ def test_in_match_enabled_globally_disabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "no" def test_in_match_disabled_globally_enabled(): @@ -210,3 +233,4 @@ def test_in_match_disabled_globally_enabled(): ) assert not semantics_changes(config) + assert global_value(config, "default") == "yes" -- 2.35.3