From 8bd3d7e5b466eb78160ca92bd36ccaa04357f28b Mon Sep 17 00:00:00 2001 From: Gabriel Becker Date: Tue, 3 Sep 2019 17:57:35 +0200 Subject: [PATCH 1/5] SSG Test Suite: Fix (all) profile when running rule mode. When overriding the profile metadata from test scenarios with the (all) profile, dependending the version of OpenSCAP it may fail to run because of some bash syntax error. --- ssg/constants.py | 1 + tests/ssg_test_suite/oscap.py | 38 ++++++++++++++++++++++++++++++++++- tests/ssg_test_suite/rule.py | 15 +++++++------- tests/test_suite.py | 1 - 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/ssg/constants.py b/ssg/constants.py index 09808480a71f..10f1f75b8e6f 100644 --- a/ssg/constants.py +++ b/ssg/constants.py @@ -51,6 +51,7 @@ OSCAP_GROUP_VAL = "xccdf_%s.content_group_values" % OSCAP_VENDOR OSCAP_GROUP_NON_PCI = "xccdf_%s.content_group_non-pci-dss" % OSCAP_VENDOR OSCAP_PATH = "oscap" +OSCAP_PROFILE_ALL_ID = "(all)" XCCDF11_NS = "http://checklists.nist.gov/xccdf/1.1" XCCDF12_NS = "http://checklists.nist.gov/xccdf/1.2" min_ansible_version = "2.5" diff --git a/tests/ssg_test_suite/oscap.py b/tests/ssg_test_suite/oscap.py index fbec5878e30f..8a7ccc50fdc7 100644 --- a/tests/ssg_test_suite/oscap.py +++ b/tests/ssg_test_suite/oscap.py @@ -9,6 +9,9 @@ import json import datetime + +from ssg.constants import OSCAP_PROFILE_ALL_ID + from ssg_test_suite.log import LogHelper from ssg_test_suite import test_env from ssg_test_suite import common @@ -27,6 +30,10 @@ _BASH_TEMPLATE = 'urn:xccdf:fix:script:sh' _XCCDF_NS = 'http://checklists.nist.gov/xccdf/1.2' +# TODO: set this variable to false when the oscap-ssh bug fix is +# released and available, see: https://github.com/OpenSCAP/openscap/pull/1366 +PROFILE_ALL_ID_SINGLE_QUOTED = True + def analysis_to_serializable(analysis): result = dict(analysis) @@ -117,7 +124,12 @@ def generate_fixes_remotely(formatting, verbose_path): ] command_operands = ['/{arf_file}'.format(** formatting)] if 'result_id' in formatting: - command_options.extend(['--result-id', formatting['result_id']]) + result_id = formatting['result_id'] + for char in "()": + # sanitize id because when using "(all)" profile the parenthesis + # can cause some bash trouble + result_id = result_id.replace(char, "") + command_options.extend(['--result-id', result_id]) command_string = ' '.join(command_base + command_options + command_operands) rc, stdout = common.run_cmd_remote( @@ -208,6 +220,30 @@ def send_arf_to_remote_machine_and_generate_remediations_there( return False +def is_virtual_oscap_profile(profile): + """ Test if the profile belongs to the so called category virtual + from OpenSCAP available profiles. It can be (all) or other id we + might come up in the future, it just needs to be encapsulated + with parenthesis for example "(custom_profile)". + """ + if profile is not None: + if profile == OSCAP_PROFILE_ALL_ID: + return True + else: + if "(" == profile[:1] and ")" == profile[-1:]: + return True + return False + + +def process_profile_id(profile): + # Detect if the profile is virtual and include single quotes if needed. + if is_virtual_oscap_profile(profile): + if PROFILE_ALL_ID_SINGLE_QUOTED: + return "'{}'".format(profile) + else: + return profile + + class GenericRunner(object): def __init__(self, environment, profile, datastream, benchmark_id): self.environment = environment diff --git a/tests/ssg_test_suite/rule.py b/tests/ssg_test_suite/rule.py index f38bb5e22369..b25329372c57 100644 --- a/tests/ssg_test_suite/rule.py +++ b/tests/ssg_test_suite/rule.py @@ -9,7 +9,7 @@ import collections import json -from ssg.constants import OSCAP_PROFILE +from ssg.constants import OSCAP_PROFILE, OSCAP_PROFILE_ALL_ID from ssg_test_suite import oscap from ssg_test_suite import xml_operations from ssg_test_suite import test_env @@ -18,8 +18,6 @@ import data -ALL_PROFILE_ID = "(all)" - logging.getLogger(__name__).addHandler(logging.NullHandler()) @@ -36,7 +34,7 @@ def get_viable_profiles(selected_profiles, datastream, benchmark): all_profiles_elements = xml_operations.get_all_profiles_in_benchmark( datastream, benchmark, logging) all_profiles = [el.attrib["id"] for el in all_profiles_elements] - all_profiles.append(ALL_PROFILE_ID) + all_profiles.append(OSCAP_PROFILE_ALL_ID) for ds_profile in all_profiles: if 'ALL' in selected_profiles: @@ -122,7 +120,7 @@ def _run_test(self, profile, test_data): runner_cls = oscap.REMEDIATION_RULE_RUNNERS[self.remediate_using] runner = runner_cls( - self.test_env, profile, self.datastream, self.benchmark_id, + self.test_env, oscap.process_profile_id(profile), self.datastream, self.benchmark_id, rule_id, scenario.script, self.dont_clean, self.manual_debug) if not self._initial_scan_went_ok(runner, rule_id, scenario.context): return False @@ -229,10 +227,10 @@ def _modify_parameters(self, script, params): params['profiles'] = [self.scenarios_profile] if not params["profiles"]: - params["profiles"].append(ALL_PROFILE_ID) + params["profiles"].append(OSCAP_PROFILE_ALL_ID) logging.debug( "Added the {0} profile to the list of available profiles for {1}" - .format(ALL_PROFILE_ID, script)) + .format(OSCAP_PROFILE_ALL_ID, script)) return params def _parse_parameters(self, script): @@ -343,7 +341,8 @@ def perform_rule_check(options): checker.scenarios_profile = options.scenarios_profile # check if target is a complete profile ID, if not prepend profile prefix if (checker.scenarios_profile is not None and - not checker.scenarios_profile.startswith(OSCAP_PROFILE)): + not checker.scenarios_profile.startswith(OSCAP_PROFILE) and + not oscap.is_virtual_oscap_profile(checker.scenarios_profile)): checker.scenarios_profile = OSCAP_PROFILE+options.scenarios_profile checker.test_target(options.target) diff --git a/tests/test_suite.py b/tests/test_suite.py index 2cf877fcfc12..e53bc0dd4642 100755 --- a/tests/test_suite.py +++ b/tests/test_suite.py @@ -144,7 +144,6 @@ def parse_args(): " Variable selections will be done according " "to this profile.") - parser_combined = subparsers.add_parser("combined", help=("Tests all rules in a profile evaluating them " "against their test scenarios."), From 1b9a743751ca24990d85ff0b3920d422adce6b75 Mon Sep 17 00:00:00 2001 From: Gabriel Becker Date: Wed, 4 Sep 2019 15:32:11 +0200 Subject: [PATCH 2/5] SSG Test Suite: Switch off adding single quotes to (all) profile. The fix for oscap-ssh is already in Fedora 30 and is going to be backported to 29 soon. --- tests/ssg_test_suite/oscap.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/ssg_test_suite/oscap.py b/tests/ssg_test_suite/oscap.py index 8a7ccc50fdc7..872c30a2ee10 100644 --- a/tests/ssg_test_suite/oscap.py +++ b/tests/ssg_test_suite/oscap.py @@ -30,9 +30,8 @@ _BASH_TEMPLATE = 'urn:xccdf:fix:script:sh' _XCCDF_NS = 'http://checklists.nist.gov/xccdf/1.2' -# TODO: set this variable to false when the oscap-ssh bug fix is -# released and available, see: https://github.com/OpenSCAP/openscap/pull/1366 -PROFILE_ALL_ID_SINGLE_QUOTED = True + +PROFILE_ALL_ID_SINGLE_QUOTED = False def analysis_to_serializable(analysis): From 7de3fe162c1ffd41b7eab7bdda9ba28024f3947d Mon Sep 17 00:00:00 2001 From: Gabriel Becker Date: Wed, 4 Sep 2019 16:00:10 +0200 Subject: [PATCH 3/5] SSG Test Suite: Set result-id empty string. When generating remediation from arf and the arf file has only on TestResult, it doesn't matter if you provide a value for result-id and for (all) profile it was breaking the bash command with bad syntax. --- tests/ssg_test_suite/oscap.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ssg_test_suite/oscap.py b/tests/ssg_test_suite/oscap.py index 872c30a2ee10..66766abebc51 100644 --- a/tests/ssg_test_suite/oscap.py +++ b/tests/ssg_test_suite/oscap.py @@ -123,12 +123,12 @@ def generate_fixes_remotely(formatting, verbose_path): ] command_operands = ['/{arf_file}'.format(** formatting)] if 'result_id' in formatting: - result_id = formatting['result_id'] - for char in "()": - # sanitize id because when using "(all)" profile the parenthesis - # can cause some bash trouble - result_id = result_id.replace(char, "") - command_options.extend(['--result-id', result_id]) + """ result_id has no effect when the arf file has only TestResult, + which is the case here, but we need at least to provide an empty string + so the functionality doesn't break. This also covers the case where using (all) + profile causes bash syntax error because of parenthesis. + """ + command_options.extend(['--result-id', '""']) command_string = ' '.join(command_base + command_options + command_operands) rc, stdout = common.run_cmd_remote( From c3131b9200dac998378b2ac88fa50708e71a4580 Mon Sep 17 00:00:00 2001 From: Gabriel Becker Date: Wed, 4 Sep 2019 16:01:54 +0200 Subject: [PATCH 4/5] SSG Test Suite: Fix an omission when processin profile id. --- tests/ssg_test_suite/oscap.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssg_test_suite/oscap.py b/tests/ssg_test_suite/oscap.py index 66766abebc51..4dd8617325ff 100644 --- a/tests/ssg_test_suite/oscap.py +++ b/tests/ssg_test_suite/oscap.py @@ -241,6 +241,8 @@ def process_profile_id(profile): return "'{}'".format(profile) else: return profile + else: + return profile class GenericRunner(object): From 429af7df7efe0cdd3b379e6cea4ec25a9948db95 Mon Sep 17 00:00:00 2001 From: Gabriel Becker Date: Thu, 5 Sep 2019 11:30:23 +0200 Subject: [PATCH 5/5] SSG Test Suite: Make sure that strings are properly quoted for oscap. --- tests/ssg_test_suite/oscap.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/ssg_test_suite/oscap.py b/tests/ssg_test_suite/oscap.py index 4dd8617325ff..954e36140c36 100644 --- a/tests/ssg_test_suite/oscap.py +++ b/tests/ssg_test_suite/oscap.py @@ -113,6 +113,13 @@ def get_result_id_from_arf(arf_path, verbose_path): return res_id +def single_quote_string(input): + result = input + for char in "\"'": + result = result.replace(char, "") + return "'{}'".format(result) + + def generate_fixes_remotely(formatting, verbose_path): command_base = ['oscap', 'xccdf', 'generate', 'fix'] command_options = [ @@ -123,14 +130,10 @@ def generate_fixes_remotely(formatting, verbose_path): ] command_operands = ['/{arf_file}'.format(** formatting)] if 'result_id' in formatting: - """ result_id has no effect when the arf file has only TestResult, - which is the case here, but we need at least to provide an empty string - so the functionality doesn't break. This also covers the case where using (all) - profile causes bash syntax error because of parenthesis. - """ - command_options.extend(['--result-id', '""']) - - command_string = ' '.join(command_base + command_options + command_operands) + command_options.extend(['--result-id', formatting['result_id']]) + + command_components = command_base + command_options + command_operands + command_string = ' '.join([single_quote_string(c) for c in command_components]) rc, stdout = common.run_cmd_remote( command_string, formatting['domain_ip'], verbose_path) if rc != 0: