From 55212e16429df7e0c482298eb59ce9e452db8b75 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Thu, 25 Apr 2024 17:16:32 +0200 Subject: [PATCH] OpenSSHConfigScanner: Include directive is supported since RHEL 8.6 This issue could cause false positive reports when the user has the configuration options such as "Subsystem sftp" defined in included file only. Resolves: RHEL-33902 Signed-off-by: Jakub Jelen --- .../libraries/readopensshconfig.py | 46 +++++++++++--- ..._readopensshconfig_opensshconfigscanner.py | 62 ++++++++++++++++++- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/repos/system_upgrade/common/actors/opensshconfigscanner/libraries/readopensshconfig.py b/repos/system_upgrade/common/actors/opensshconfigscanner/libraries/readopensshconfig.py index e6cb9fcc1e..8d1df31518 100644 --- a/repos/system_upgrade/common/actors/opensshconfigscanner/libraries/readopensshconfig.py +++ b/repos/system_upgrade/common/actors/opensshconfigscanner/libraries/readopensshconfig.py @@ -1,5 +1,8 @@ import errno +import glob +import os +from leapp.exceptions import StopActorExecutionError from leapp.libraries.common.rpms import check_file_modification from leapp.libraries.stdlib import api from leapp.models import OpenSshConfig, OpenSshPermitRootLogin @@ -12,14 +15,23 @@ def line_empty(line): return len(line) == 0 or line.startswith('\n') or line.startswith('#') -def parse_config(config): +def parse_config(config, ret=None, depth=0): """Parse OpenSSH server configuration or the output of sshd test option.""" - # RHEL7 defaults - ret = OpenSshConfig( - permit_root_login=[], - deprecated_directives=[] - ) + if depth > 16: + # This should really never happen as it would mean the SSH server won't + # start anyway on the old system. + error = 'Too many recursive includes while parsing sshd_config' + api.current_logger().error(error) + return None + + if ret is None: + # RHEL7 defaults + ret = OpenSshConfig( + permit_root_login=[], + deprecated_directives=[] + ) + # TODO(Jakuje): Do we need different defaults for RHEL8? in_match = None for line in config: @@ -68,6 +80,22 @@ def parse_config(config): # here we need to record all remaining items as command and arguments ret.subsystem_sftp = ' '.join(el[2:]) + elif el[0].lower() == 'include': + # recursively parse the given file or files referenced by this option + pattern = el[1] + if pattern[0] != '/' and pattern[0] != '~': + pattern = os.path.join('/etc/ssh/', pattern) + # NOTE that OpenSSH sorts the files lexicographically + files = glob.glob(pattern) + files.sort() + for f in files: + output = read_sshd_config(f) + if parse_config(output, ret, depth + 1) is None: + raise StopActorExecutionError( + 'Failed to parse sshd configuration file: ', + details={'details': 'Too many recursive includes while parsing {}.'.format(f)} + ) + elif el[0].lower() in DEPRECATED_DIRECTIVES: # Filter out duplicit occurrences of the same deprecated directive if el[0].lower() not in ret.deprecated_directives: @@ -82,10 +110,10 @@ def produce_config(producer, config): producer(config) -def read_sshd_config(): +def read_sshd_config(config): """Read the actual sshd configuration file.""" try: - with open(CONFIG, 'r') as fd: + with open(config, 'r') as fd: return fd.readlines() except IOError as err: if err.errno != errno.ENOENT: @@ -98,7 +126,7 @@ def scan_sshd(producer): """Parse sshd_config configuration file to create OpenSshConfig message.""" # direct access to configuration file - output = read_sshd_config() + output = read_sshd_config(CONFIG) config = parse_config(output) # find out whether the file was modified from the one shipped in rpm diff --git a/repos/system_upgrade/common/actors/opensshconfigscanner/tests/test_readopensshconfig_opensshconfigscanner.py b/repos/system_upgrade/common/actors/opensshconfigscanner/tests/test_readopensshconfig_opensshconfigscanner.py index 68a9ec47ec..019b022017 100644 --- a/repos/system_upgrade/common/actors/opensshconfigscanner/tests/test_readopensshconfig_opensshconfigscanner.py +++ b/repos/system_upgrade/common/actors/opensshconfigscanner/tests/test_readopensshconfig_opensshconfigscanner.py @@ -1,3 +1,9 @@ +import tempfile +import os +import pytest +import shutil + +from leapp.exceptions import StopActorExecutionError from leapp.libraries.actor.readopensshconfig import line_empty, parse_config, produce_config from leapp.models import OpenSshConfig, OpenSshPermitRootLogin @@ -143,12 +149,66 @@ def test_parse_config_deprecated(): def test_parse_config_empty(): output = parse_config([]) assert isinstance(output, OpenSshConfig) - assert isinstance(output, OpenSshConfig) assert not output.permit_root_login assert output.use_privilege_separation is None assert output.protocol is None +def test_parse_config_include(): + """ This already require some files to touch """ + + # python2 compatibility :/ + dirpath = tempfile.mkdtemp() + + config = [ + "Include {}/*.conf".format(dirpath) + ] + + try: + # Prepare tmp directory with some included configuration snippets + my_path = os.path.join(dirpath, "my.conf") + with open(my_path, "w") as f: + f.write('Subsystem sftp internal-sftp') + other_path = os.path.join(dirpath, "another.conf") + with open(other_path, "w") as f: + f.write('permitrootlogin no') + + output = parse_config(config) + finally: + shutil.rmtree(dirpath) + + assert isinstance(output, OpenSshConfig) + assert len(output.permit_root_login) == 1 + assert output.permit_root_login[0].value == 'no' + assert output.permit_root_login[0].in_match is None + assert output.use_privilege_separation is None + assert output.protocol is None + assert output.subsystem_sftp == 'internal-sftp' + + +def test_parse_config_include_recursive(): + """ The recursive include should gracefully fail """ + + # python2 compatibility :/ + dirpath = tempfile.mkdtemp() + + config = [ + "Include {}/*.conf".format(dirpath) + ] + + try: + # this includes recursively the same file + my_path = os.path.join(dirpath, "recursive.conf") + with open(my_path, "w") as f: + f.write(config[0]) + + with pytest.raises(StopActorExecutionError) as recursive_error: + parse_config(config) + assert 'Failed to parse sshd configuration file' in str(recursive_error) + finally: + shutil.rmtree(dirpath) + + def test_produce_config(): output = []