From 2ea54eeb2d1bd29ee5c6203ae07f474745536841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Mon, 31 May 2021 11:25:48 +0200 Subject: [PATCH 01/16] Slightly refactor handling of downloaded content. --- org_fedora_oscap/content_handling.py | 69 +++++++++++++----- testing_files/cpe-dict.xml | 9 +++ tests/test_content_handling.py | 101 +++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 17 deletions(-) create mode 100644 testing_files/cpe-dict.xml create mode 100644 tests/test_content_handling.py diff --git a/org_fedora_oscap/content_handling.py b/org_fedora_oscap/content_handling.py index fddcf580..f2af22f3 100644 --- a/org_fedora_oscap/content_handling.py +++ b/org_fedora_oscap/content_handling.py @@ -27,6 +27,8 @@ import os.path from collections import namedtuple +import multiprocessing + from pyanaconda.core.util import execReadlines try: from html.parser import HTMLParser @@ -37,6 +39,29 @@ log = logging.getLogger("anaconda") +CONTENT_TYPES = dict( + DATASTREAM="Source Data Stream", + XCCDF_CHECKLIST="XCCDF Checklist", + OVAL="OVAL Definitions", + CPE_DICT="CPE Dictionary", + TAILORING="XCCDF Tailoring", +) + + +class ContentHandlingError(Exception): + """Exception class for errors related to SCAP content handling.""" + + pass + + +class ContentCheckError(ContentHandlingError): + """ + Exception class for errors related to content (integrity,...) checking. + """ + + pass + + class ParseHTMLContent(HTMLParser): """Parser class for HTML tags within content""" @@ -85,6 +110,33 @@ def parse_HTML_from_content(content): ContentFiles = namedtuple("ContentFiles", ["xccdf", "cpe", "tailoring"]) +def identify_files(fpaths): + with multiprocessing.Pool(os.cpu_count()) as p: + labels = p.map(get_doc_type, fpaths) + return {path: label for (path, label) in zip(fpaths, labels)} + + +def get_doc_type(file_path): + content_type = "unknown" + try: + for line in execReadlines("oscap", ["info", file_path]): + if line.startswith("Document type:"): + _prefix, _sep, type_info = line.partition(":") + content_type = type_info.strip() + break + except OSError: + # 'oscap info' exitted with a non-zero exit code -> unknown doc + # type + pass + except UnicodeDecodeError: + # 'oscap info' supplied weird output, which happens when it tries + # to explain why it can't examine e.g. a JPG. + return None + log.info("OSCAP addon: Identified {file_path} as {content_type}" + .format(file_path=file_path, content_type=content_type)) + return content_type + + def explore_content_files(fpaths): """ Function for finding content files in a list of file paths. SIMPLY PICKS @@ -99,23 +151,6 @@ def explore_content_files(fpaths): :rtype: ContentFiles """ - - def get_doc_type(file_path): - content_type = "unknown" - try: - for line in execReadlines("oscap", ["info", file_path]): - if line.startswith("Document type:"): - _prefix, _sep, type_info = line.partition(":") - content_type = type_info.strip() - break - except OSError: - # 'oscap info' exitted with a non-zero exit code -> unknown doc - # type - pass - log.info("OSCAP addon: Identified {file_path} as {content_type}" - .format(file_path=file_path, content_type=content_type)) - return content_type - xccdf_file = "" cpe_file = "" tailoring_file = "" diff --git a/testing_files/cpe-dict.xml b/testing_files/cpe-dict.xml new file mode 100644 index 00000000..394cae8a --- /dev/null +++ b/testing_files/cpe-dict.xml @@ -0,0 +1,9 @@ + + + + Applicable example platform + oval:x:def:1 + + diff --git a/tests/test_content_handling.py b/tests/test_content_handling.py new file mode 100644 index 00000000..b21b8e91 --- /dev/null +++ b/tests/test_content_handling.py @@ -0,0 +1,101 @@ +import os +import glob + +import pytest + +from org_fedora_oscap import content_handling as ch + + +TESTING_FILES_PATH = os.path.join( + os.path.dirname(__file__), os.path.pardir, "testing_files") +DS_FILEPATH = os.path.join( + TESTING_FILES_PATH, "testing_ds.xml") + +DS_IDS = "scap_org.open-scap_datastream_tst" +CHK_FIRST_ID = "scap_org.open-scap_cref_first-xccdf.xml" +CHK_SECOND_ID = "scap_org.open-scap_cref_second-xccdf.xml" + +PROFILE1_ID = "xccdf_com.example_profile_my_profile" +PROFILE2_ID = "xccdf_com.example_profile_my_profile2" +PROFILE3_ID = "xccdf_com.example_profile_my_profile3" + + +@pytest.fixture() +def ds_handler(): + return ch.DataStreamHandler(DS_FILEPATH) + + +def test_init_invalid_file_path(): + with pytest.raises(ch.DataStreamHandlingError) as excinfo: + ch.DataStreamHandler("testing_ds.xmlll") + assert "Invalid file path" in str(excinfo.value) + + +def test_init_not_scap_content(): + with pytest.raises(ch.DataStreamHandlingError) as excinfo: + ch.DataStreamHandler(os.path.join(TESTING_FILES_PATH, "testing_ks.cfg")) + assert "not a valid SCAP content file" in str(excinfo.value) + + +def test_init_xccdf_content(): + with pytest.raises(ch.DataStreamHandlingError) as excinfo: + ch.DataStreamHandler(os.path.join(TESTING_FILES_PATH, "xccdf.xml")) + assert "not a data stream collection" in str(excinfo.value) + + +def test_get_data_streams(ds_handler): + assert DS_IDS in ds_handler.get_data_streams() + + +def test_get_data_streams_checklists(ds_handler): + expected_ids = {DS_IDS: [CHK_FIRST_ID, CHK_SECOND_ID]} + + ds_ids = ds_handler.get_data_streams_checklists() + assert expected_ids == ds_ids + + +def test_get_checklists(ds_handler): + expected_checklists = [CHK_FIRST_ID, CHK_SECOND_ID] + + chk_ids = ds_handler.get_checklists(DS_IDS) + assert expected_checklists == chk_ids + + +def test_get_checklists_invalid(ds_handler): + with pytest.raises(ch.DataStreamHandlingError) as excinfo: + ds_handler.get_checklists("invalid.id") + assert "Invalid data stream id given" in str(excinfo.value) + + +def test_get_profiles(ds_handler): + profile_ids = ds_handler.get_profiles(DS_IDS, CHK_FIRST_ID) + + # When Benchmark doesn't contain Rules selected by default + # the default Profile should not be present + assert 2 == len(profile_ids) + assert PROFILE1_ID == profile_ids[0].id + assert PROFILE2_ID == profile_ids[1].id + + +def test_get_profiles_with_default(ds_handler): + profile_ids = ds_handler.get_profiles(DS_IDS, CHK_SECOND_ID) + + # When Benchmark contains Rules selected by default + # the default Profile should be present + assert 2 == len(profile_ids) + assert "default" == profile_ids[0].id + assert PROFILE3_ID == profile_ids[1].id + + +def test_identify_files(): + filenames = glob.glob(TESTING_FILES_PATH + "/*") + identified = ch.identify_files(filenames) + assert identified[DS_FILEPATH] == ch.CONTENT_TYPES["DATASTREAM"] + assert identified[ + os.path.join(TESTING_FILES_PATH, "scap-mycheck-oval.xml")] == ch.CONTENT_TYPES["OVAL"] + assert identified[ + os.path.join(TESTING_FILES_PATH, "tailoring.xml")] == ch.CONTENT_TYPES["TAILORING"] + assert identified[ + os.path.join(TESTING_FILES_PATH, "testing_xccdf.xml")] == ch.CONTENT_TYPES["XCCDF_CHECKLIST"] + assert identified[ + os.path.join(TESTING_FILES_PATH, "cpe-dict.xml")] == ch.CONTENT_TYPES["CPE_DICT"] From b0b7f003d370f9a60fbe18edbb70389c773aad0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Mon, 31 May 2021 11:26:38 +0200 Subject: [PATCH 02/16] Added a function for local content fetching. That function has the same interface as the remote fetching one. --- org_fedora_oscap/data_fetch.py | 20 ++++++++++++++++++++ org_fedora_oscap/ks/oscap.py | 2 +- tests/test_data_fetch.py | 8 ++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/org_fedora_oscap/data_fetch.py b/org_fedora_oscap/data_fetch.py index eefaed51..08ce2007 100644 --- a/org_fedora_oscap/data_fetch.py +++ b/org_fedora_oscap/data_fetch.py @@ -75,6 +75,26 @@ class FetchError(DataFetchError): pass +def fetch_local_data(url, out_file): + """ + Function that fetches data locally. + + :see: org_fedora_oscap.data_fetch.fetch_data + :return: the name of the thread running fetch_data + :rtype: str + + """ + fetch_data_thread = AnacondaThread(name=common.THREAD_FETCH_DATA, + target=fetch_data, + args=(url, out_file, None), + fatal=False) + + # register and run the thread + threadMgr.add(fetch_data_thread) + + return common.THREAD_FETCH_DATA + + def wait_and_fetch_net_data(url, out_file, ca_certs=None): """ Function that waits for network connection and starts a thread that fetches diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index ff805fc9..34f47635 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -47,7 +47,7 @@ "scap-security-guide", ) -SUPPORTED_URL_PREFIXES = ("http://", "https://", "ftp://" +SUPPORTED_URL_PREFIXES = ("http://", "https://", "ftp://", "file://" # LABEL:?, hdaX:?, ) diff --git a/tests/test_data_fetch.py b/tests/test_data_fetch.py index e2bb573d..cfc56b9a 100644 --- a/tests/test_data_fetch.py +++ b/tests/test_data_fetch.py @@ -59,3 +59,11 @@ def test_supported_url(): def test_unsupported_url(): assert not data_fetch.can_fetch_from("aaaaa") + + +def test_fetch_local(tmp_path): + source_path = pathlib.Path(__file__).absolute() + dest_path = tmp_path / "dest" + data_fetch.fetch_data("file://" + str(source_path), dest_path) + with open(dest_path, "r") as copied_file: + assert "This line is here and in the copied file as well" in copied_file.read() From e3ff5b4b3d187b273bfccd3fa68734ded95f03fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Fri, 4 Jun 2021 16:11:04 +0200 Subject: [PATCH 03/16] Enabled building images without sudo when possible. sudo is only needed when RPMs are installed. --- create_update_image.sh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/create_update_image.sh b/create_update_image.sh index fe5a5891..64eb7705 100755 --- a/create_update_image.sh +++ b/create_update_image.sh @@ -204,8 +204,8 @@ install_addon_from_repo() { else install_po_files="DEFAULT_INSTALL_OF_PO_FILES=yes" fi - # "copy files" to new root, sudo needed because we may overwrite files installed by rpm - sudo make install "$install_po_files" DESTDIR="${tmp_root}" >&2 || die "Failed to install the addon to $tmp_root." + # "copy files" to new root, sudo may be needed because we may overwrite files installed by rpm + $SUDO make install "$install_po_files" DESTDIR="${tmp_root}" >&2 || die "Failed to install the addon to $tmp_root." } @@ -216,12 +216,16 @@ create_image() { cleanup() { - # cleanup, sudo needed because former RPM installs - sudo rm -rf "$tmp_root" + # cleanup, sudo may be needed because former RPM installs + $SUDO rm -rf "$tmp_root" } - -sudo true || die "Unable to get sudo working, bailing out." +if test $_arg_start_with_index -gt 1; then + SUDO= +else + SUDO=sudo + $SUDO true || die "Unable to get sudo working, bailing out." +fi for (( action_index=_arg_start_with_index; action_index < ${#actions[*]}; action_index++ )) do "${actions[$action_index]}" From 6632e53a972d113756472b0db4421ccadf660031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Fri, 4 Jun 2021 16:28:33 +0200 Subject: [PATCH 04/16] Extracted rule data gathering. --- org_fedora_oscap/gui/spokes/oscap.py | 12 +++--------- org_fedora_oscap/ks/oscap.py | 11 +++-------- org_fedora_oscap/rule_handling.py | 11 +++++++++++ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index 56de4b9e..6873b89d 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -726,10 +726,9 @@ def _select_profile(self, profile_id): # get pre-install fix rules from the content try: - rules = common.get_fix_rules_pre(profile_id, - self._addon_data.preinst_content_path, - ds, xccdf, - self._addon_data.preinst_tailoring_path) + self._rule_data = rule_handling.get_rule_data_from_content( + profile_id, self._addon_data.preinst_content_path, + ds, xccdf, self._addon_data.preinst_tailoring_path) except common.OSCAPaddonError as exc: log.error( "Failed to get rules for the profile '{}': {}" @@ -745,11 +744,6 @@ def _select_profile(self, profile_id): self._profiles_store.set_value(itr, 2, True) itr = self._profiles_store.iter_next(itr) - # parse and store rules with a clean RuleData instance - self._rule_data = rule_handling.RuleData() - for rule in rules.splitlines(): - self._rule_data.new_rule(rule) - # remember the active profile self._active_profile = profile_id diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 34f47635..ca9bbb94 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -379,15 +379,7 @@ def _fetch_content_and_initialize(self): common.INSTALLATION_CONTENT_DIR, [self.content_path]) - rules = common.get_fix_rules_pre(self.profile_id, - self.preinst_content_path, - self.datastream_id, self.xccdf_id, - self.preinst_tailoring_path) - # parse and store rules with a clean RuleData instance - self.rule_data = rule_handling.RuleData() - for rule in rules.splitlines(): - self.rule_data.new_rule(rule) def setup(self, storage, ksdata, payload): """ @@ -463,6 +455,9 @@ def setup(self, storage, ksdata, payload): progressQ.send_quit(1) while True: time.sleep(100000) + self.rule_data = rule_handling.get_rule_data_from_content( + self.profile_id, self.preinst_content_path, + self.datastream_id, self.xccdf_id, self.preinst_tailoring_path) # evaluate rules, do automatic fixes and stop if something that cannot # be fixed automatically is wrong diff --git a/org_fedora_oscap/rule_handling.py b/org_fedora_oscap/rule_handling.py index 80d86c7c..d7729f71 100644 --- a/org_fedora_oscap/rule_handling.py +++ b/org_fedora_oscap/rule_handling.py @@ -68,6 +68,17 @@ _ = common._ +def get_rule_data_from_content(profile_id, content_path, ds_id="", xccdf_id="", tailoring_path=""): + rules = common.get_fix_rules_pre( + profile_id, content_path, ds_id, xccdf_id, tailoring_path) + + # parse and store rules with a clean RuleData instance + rule_data = RuleData() + for rule in rules.splitlines(): + rule_data.new_rule(rule) + return rule_data + + # TODO: use set instead of list for mount options? def parse_csv(option, opt_str, value, parser): for item in value.split(","): From dfdc41c3e5a682788e5ea6865301fcaeff7488e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Fri, 4 Jun 2021 16:49:59 +0200 Subject: [PATCH 05/16] Various improvements. Notably introduced the _terminate function that extracts all the installation termination-related boilerplate. --- org_fedora_oscap/common.py | 5 +++- org_fedora_oscap/data_fetch.py | 1 + org_fedora_oscap/gui/spokes/oscap.py | 6 ++-- org_fedora_oscap/ks/oscap.py | 43 ++++++++++++++++------------ 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/org_fedora_oscap/common.py b/org_fedora_oscap/common.py index 27415714..884bbc88 100644 --- a/org_fedora_oscap/common.py +++ b/org_fedora_oscap/common.py @@ -307,7 +307,10 @@ def extract_data(archive, out_dir, ensure_has_files=None): ensure_has_files = [] # get rid of empty file paths - ensure_has_files = [fpath for fpath in ensure_has_files if fpath] + if not ensure_has_files: + ensure_has_files = [] + else: + ensure_has_files = [fpath for fpath in ensure_has_files if fpath] msg = "OSCAP addon: Extracting {archive}".format(archive=archive) if ensure_has_files: diff --git a/org_fedora_oscap/data_fetch.py b/org_fedora_oscap/data_fetch.py index 08ce2007..e8d2b7d4 100644 --- a/org_fedora_oscap/data_fetch.py +++ b/org_fedora_oscap/data_fetch.py @@ -172,6 +172,7 @@ def fetch_data(url, out_file, ca_certs=None): else: msg = "Cannot fetch data from '%s': unknown URL format" % url raise UnknownURLformatError(msg) + log.info(f"Data fetch from {url} completed") def _curl_fetch(url, out_file, ca_certs=None): diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index 6873b89d..8adf3e91 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -713,6 +713,8 @@ def _select_profile(self, profile_id): # no profile specified, nothing to do return False + ds = None + xccdf = None if self._using_ds: ds = self._current_ds_id xccdf = self._current_xccdf_id @@ -720,9 +722,6 @@ def _select_profile(self, profile_id): if not all((ds, xccdf, profile_id)): # something is not set -> do nothing return False - else: - ds = None - xccdf = None # get pre-install fix rules from the content try: @@ -1127,6 +1126,7 @@ def on_fetch_button_clicked(self, *args): with self._fetch_flag_lock: if self._fetching: # some other fetching/pre-processing running, give up + log.warn("Clicked the fetch button, although the GUI is in the fetching mode.") return # prevent user from changing the URL in the meantime diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index ca9bbb94..06ada8a5 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -379,6 +379,25 @@ def _fetch_content_and_initialize(self): common.INSTALLATION_CONTENT_DIR, [self.content_path]) + def _terminate(self, message): + if flags.flags.automatedInstall and not flags.flags.ksprompt: + # cannot have ask in a non-interactive kickstart + # installation + raise errors.CmdlineError(message) + + answ = errors.errorHandler.ui.showYesNoQuestion(msg) + if answ == errors.ERROR_CONTINUE: + # prevent any futher actions here by switching to the dry + # run mode and let things go on + self.dry_run = True + return + else: + # Let's sleep forever to prevent any further actions and + # wait for the main thread to quit the process. + progressQ.send_quit(1) + while True: + time.sleep(100000) + def setup(self, storage, ksdata, payload): @@ -464,25 +483,11 @@ def setup(self, storage, ksdata, payload): fatal_messages = [message for message in self.rule_data.eval_rules(ksdata, storage) if message.type == common.MESSAGE_TYPE_FATAL] if any(fatal_messages): - msg = "Wrong configuration detected!\n" - msg += "\n".join(message.text for message in fatal_messages) - msg += "\nThe installation should be aborted. Do you wish to continue anyway?" - if flags.flags.automatedInstall and not flags.flags.ksprompt: - # cannot have ask in a non-interactive kickstart installation - raise errors.CmdlineError(msg) - - answ = errors.errorHandler.ui.showYesNoQuestion(msg) - if answ == errors.ERROR_CONTINUE: - # prevent any futher actions here by switching to the dry - # run mode and let things go on - self.dry_run = True - return - else: - # Let's sleep forever to prevent any further actions and wait - # for the main thread to quit the process. - progressQ.send_quit(1) - while True: - time.sleep(100000) + msg = ["Wrong configuration detected!"] + msg.extend(fatal_messages) + msg.append("The installation should be aborted. Do you wish to continue anyway?") + self._terminate("\n".join(msg)) + return # add packages needed on the target system to the list of packages # that are requested to be installed From 3164e025a3308ae0ff295cdd833619ed68f19d1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Fri, 4 Jun 2021 16:55:06 +0200 Subject: [PATCH 06/16] Introduced the Model class. The class unifies the content workflow between kickstart and GUI installations. It provides methods to work with the content in non-interactive and interactive modes using a system of callbacks. --- org_fedora_oscap/gui/spokes/oscap.py | 105 +++++------ org_fedora_oscap/ks/oscap.py | 117 ++++++------ org_fedora_oscap/model.py | 266 +++++++++++++++++++++++++++ 3 files changed, 361 insertions(+), 127 deletions(-) create mode 100644 org_fedora_oscap/model.py diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index 8adf3e91..e23dface 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -21,6 +21,7 @@ import threading import logging from functools import wraps +import pathlib # the path to addons is in sys.path so we can import things # from org_fedora_oscap @@ -31,6 +32,7 @@ from org_fedora_oscap import scap_content_handler from org_fedora_oscap import utils from org_fedora_oscap.common import dry_run_skip +from org_fedora_oscap.model import Model from pyanaconda.threading import threadMgr, AnacondaThread from pyanaconda.ui.gui.spokes import NormalSpoke from pyanaconda.ui.communication import hubQ @@ -250,6 +252,8 @@ def __init__(self, data, storage, payload): self._anaconda_spokes_initialized = threading.Event() self.initialization_controller.init_done.connect(self._all_anaconda_spokes_initialized) + self.model = Model(None) + def _all_anaconda_spokes_initialized(self): log.debug("OSCAP addon: Anaconda init_done signal triggered") self._anaconda_spokes_initialized.set() @@ -333,6 +337,27 @@ def initialize(self): # else fetch data self._fetch_data_and_initialize() + def _handle_error(self, exception): + log.error(str(exception)) + if isinstance(exception, KickstartValueError): + self._invalid_url() + elif isinstance(exception, common.OSCAPaddonNetworkError): + self._network_problem() + elif isinstance(exception, data_fetch.DataFetchError): + self._data_fetch_failed() + elif isinstance(exception, common.ExtractionError): + self._extraction_failed(str(exception)) + elif isinstance(exception, content_handling.ContentHandlingError): + self._invalid_content() + elif isinstance(exception, content_handling.ContentCheckError): + self._integrity_check_failed() + else: + raise exception + + def _end_fetching(self, fetched_content): + # stop the spinner in any case + fire_gtk_action(self._progress_spinner.stop) + def _render_selected(self, column, renderer, model, itr, user_data=None): if model[itr][2]: renderer.set_property("stock-id", "gtk-apply") @@ -349,24 +374,10 @@ def _fetch_data_and_initialize(self): self._fetching = True thread_name = None - if any(self._addon_data.content_url.startswith(net_prefix) - for net_prefix in data_fetch.NET_URL_PREFIXES): - # need to fetch data over network - try: - thread_name = data_fetch.wait_and_fetch_net_data( - self._addon_data.content_url, - self._addon_data.raw_preinst_content_path, - self._addon_data.certificates) - except common.OSCAPaddonNetworkError: - self._network_problem() - with self._fetch_flag_lock: - self._fetching = False - return - except KickstartValueError: - self._invalid_url() - with self._fetch_flag_lock: - self._fetching = False - return + if self._addon_data.content_url and self._addon_data.content_type != "scap-security-guide": + self.model.CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) + self.model.content_uri = self._addon_data.content_url + thread_name = self.model.fetch_content(self._addon_data.certificates, self._handle_error) # pylint: disable-msg=E1101 hubQ.send_message(self.__class__.__name__, @@ -388,61 +399,27 @@ def _init_after_data_fetch(self, wait_for): :type wait_for: str or None """ + content_path = None + actually_fetched_content = wait_for is not None - try: - threadMgr.wait(wait_for) - except data_fetch.DataFetchError: - self._data_fetch_failed() + if actually_fetched_content: + content_path = self._addon_data.raw_preinst_content_path + + content = self.model.finish_content_fetch(wait_for, self._addon_data.fingerprint, lambda msg: log.info(msg), content_path, self._end_fetching, self._handle_error) + if not content: with self._fetch_flag_lock: self._fetching = False - return - finally: - # stop the spinner in any case - fire_gtk_action(self._progress_spinner.stop) - - if self._addon_data.fingerprint: - hash_obj = utils.get_hashing_algorithm(self._addon_data.fingerprint) - digest = utils.get_file_fingerprint(self._addon_data.raw_preinst_content_path, - hash_obj) - if digest != self._addon_data.fingerprint: - self._integrity_check_failed() - # fetching done - with self._fetch_flag_lock: - self._fetching = False - return - # RPM is an archive at this phase - if self._addon_data.content_type in ("archive", "rpm"): - # extract the content - try: - fpaths = common.extract_data(self._addon_data.raw_preinst_content_path, - common.INSTALLATION_CONTENT_DIR, - [self._addon_data.content_path]) - except common.ExtractionError as err: - self._extraction_failed(str(err)) - # fetching done - with self._fetch_flag_lock: - self._fetching = False - return - - # and populate missing fields - files = content_handling.explore_content_files(fpaths) - files = common.strip_content_dir(files) - - # pylint: disable-msg=E1103 - self._addon_data.content_path = self._addon_data.content_path or files.xccdf - self._addon_data.cpe_path = self._addon_data.cpe_path or files.cpe - self._addon_data.tailoring_path = (self._addon_data.tailoring_path or - files.tailoring) - elif self._addon_data.content_type not in ( - "datastream", "scap-security-guide"): - raise common.OSCAPaddonError("Unsupported content type") + return try: + preferred_content = self._addon_data.get_preferred_content(content) + if actually_fetched_content: + self._addon_data.update_with_content(content) self._content_handler = scap_content_handler.SCAPContentHandler( self._addon_data.preinst_content_path, self._addon_data.preinst_tailoring_path) - except scap_content_handler.SCAPContentHandlerError as e: + except Exception as e: log.warning(str(e)) self._invalid_content() # fetching done diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 06ada8a5..223a8ab0 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -25,6 +25,7 @@ import os import time import logging +import pathlib from pyanaconda.addons import AddonData from pyanaconda.core.configuration.anaconda import conf @@ -35,6 +36,8 @@ from pykickstart.errors import KickstartParseError, KickstartValueError from org_fedora_oscap import utils, common, rule_handling, data_fetch from org_fedora_oscap.common import SUPPORTED_ARCHIVES, _ +from org_fedora_oscap.content_handling import ContentCheckError, ContentHandlingError +from org_fedora_oscap import model log = logging.getLogger("anaconda") @@ -101,6 +104,9 @@ def __init__(self, name, just_clear=False): self.rule_data = rule_handling.RuleData() self.dry_run = False + self.model = model.Model(self) + self.model.CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) + def __str__(self): """ What should end up in the resulting kickstart file, i.e. string @@ -367,17 +373,24 @@ def postinst_tailoring_path(self): return utils.join_paths(common.TARGET_CONTENT_DIR, self.tailoring_path) - def _fetch_content_and_initialize(self): - """Fetch content and initialize from it""" + def get_preferred_content(self, content): + if self.content_path: + preferred_content = content.find_expected_usable_content(self.content_path) + else: + preferred_content = content.select_main_usable_content() + + if self.tailoring_path: + if self.tailoring_path != str(content.tailoring.relative_to(content.root)): + msg = f"Expected a tailoring {self.tailoring_path}, but it couldn't be found" + raise content_handling.ContentHandlingError(msg) + return preferred_content + + def update_with_content(self, content): + preferred_content = self.get_preferred_content(content) - data_fetch.fetch_data(self.content_url, self.raw_preinst_content_path, - self.certificates) - # RPM is an archive at this phase - if self.content_type in ("archive", "rpm"): - # extract the content - common.extract_data(self.raw_preinst_content_path, - common.INSTALLATION_CONTENT_DIR, - [self.content_path]) + self.content_path = str(preferred_content.relative_to(content.root)) + if content.tailoring: + self.tailoring_path = str(content.tailoring.relative_to(content.root)) def _terminate(self, message): if flags.flags.automatedInstall and not flags.flags.ksprompt: @@ -398,7 +411,22 @@ def _terminate(self, message): while True: time.sleep(100000) + def _handle_error(self, exception): + log.error("Failed to fetch and initialize SCAP content!") + + if isinstance(exception, ContentCheckError): + msg = _("The integrity check of the security content failed.\n" + + "The installation should be aborted. Do you wish to continue anyway?") + self._terminate(msg) + elif (isinstance(exception, common.OSCAPaddonError) + or isinstance(exception, data_fetch.DataFetchError)): + msg = _("There was an error fetching and loading the security content:\n" + + "%s\n" + + "The installation should be aborted. Do you wish to continue anyway?") % str(exception) + self._terminate(msg) + else: + raise exception def setup(self, storage, ksdata, payload): """ @@ -419,61 +447,24 @@ def setup(self, storage, ksdata, payload): # selected return + thread_name = None if not os.path.exists(self.preinst_content_path) and not os.path.exists(self.raw_preinst_content_path): # content not available/fetched yet - try: - self._fetch_content_and_initialize() - except (common.OSCAPaddonError, data_fetch.DataFetchError) as e: - log.error("Failed to fetch and initialize SCAP content!") - msg = _("There was an error fetching and loading the security content:\n" + - "%s\n" + - "The installation should be aborted. Do you wish to continue anyway?") % e - - if flags.flags.automatedInstall and not flags.flags.ksprompt: - # cannot have ask in a non-interactive kickstart - # installation - raise errors.CmdlineError(msg) - - answ = errors.errorHandler.ui.showYesNoQuestion(msg) - if answ == errors.ERROR_CONTINUE: - # prevent any futher actions here by switching to the dry - # run mode and let things go on - self.dry_run = True - return - else: - # Let's sleep forever to prevent any further actions and - # wait for the main thread to quit the process. - progressQ.send_quit(1) - while True: - time.sleep(100000) - - # check fingerprint if given - if self.fingerprint: - hash_obj = utils.get_hashing_algorithm(self.fingerprint) - digest = utils.get_file_fingerprint(self.raw_preinst_content_path, - hash_obj) - if digest != self.fingerprint: - log.error("Failed to fetch and initialize SCAP content!") - msg = _("The integrity check of the security content failed.\n" + - "The installation should be aborted. Do you wish to continue anyway?") - - if flags.flags.automatedInstall and not flags.flags.ksprompt: - # cannot have ask in a non-interactive kickstart - # installation - raise errors.CmdlineError(msg) - - answ = errors.errorHandler.ui.showYesNoQuestion(msg) - if answ == errors.ERROR_CONTINUE: - # prevent any futher actions here by switching to the dry - # run mode and let things go on - self.dry_run = True - return - else: - # Let's sleep forever to prevent any further actions and - # wait for the main thread to quit the process. - progressQ.send_quit(1) - while True: - time.sleep(100000) + self.model.content_uri = self.content_url + thread_name = self.model.fetch_content(self.certificates, self._handle_error) + + content = self.model.finish_content_fetch(thread_name, self.fingerprint, lambda msg: log.info(msg), self.raw_preinst_content_path, lambda content: content, self._handle_error) + + if not content: + return + + try: + # just check that preferred content exists + _ = self.get_preferred_content(content) + except Exception as exc: + self._terminate(str(exc)) + return + self.rule_data = rule_handling.get_rule_data_from_content( self.profile_id, self.preinst_content_path, self.datastream_id, self.xccdf_id, self.preinst_tailoring_path) diff --git a/org_fedora_oscap/model.py b/org_fedora_oscap/model.py new file mode 100644 index 00000000..1ae69b23 --- /dev/null +++ b/org_fedora_oscap/model.py @@ -0,0 +1,266 @@ +import threading +import logging +import pathlib +import shutil +from glob import glob + +from pyanaconda.core import constants +from pyanaconda.threading import threadMgr +from pykickstart.errors import KickstartValueError + +from org_fedora_oscap import data_fetch, utils +from org_fedora_oscap import common +from org_fedora_oscap import content_handling + +log = logging.getLogger("anaconda") + + +def is_network(scheme): + return any( + scheme.startswith(net_prefix) + for net_prefix in data_fetch.NET_URL_PREFIXES) + + +class Model: + CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) / "content-download" + + def __init__(self, policy_data): + self.content_uri_scheme = "" + self.content_uri_path = "" + self.fetched_content = "" + + self.activity_lock = threading.Lock() + self.now_fetching_or_processing = False + + self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True) + + def get_content_type(self, url): + if url.endswith(".rpm"): + return "rpm" + elif any(url.endswith(arch_type) for arch_type in common.SUPPORTED_ARCHIVES): + return "archive" + else: + return "file" + + @property + def content_uri(self): + return self.content_uri_scheme + "://" + self.content_uri_path + + @content_uri.setter + def content_uri(self, uri): + scheme, path = uri.split("://", 1) + self.content_uri_path = path + self.content_uri_scheme = scheme + + def fetch_content(self, cert, what_if_fail): + shutil.rmtree(self.CONTENT_DOWNLOAD_LOCATION, ignore_errors=True) + self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True) + return self.fetch_files(self.content_uri_scheme, self.content_uri_path, self.CONTENT_DOWNLOAD_LOCATION, cert, what_if_fail) + + def fetch_files(self, scheme, path, destdir, cert, what_if_fail): + with self.activity_lock: + if self.now_fetching_or_processing: + msg = "Strange, it seems that we are already fetching something." + log.warn(msg) + return + self.now_fetching_or_processing = True + + thread_name = None + try: + thread_name = self._start_actual_fetch(scheme, path, destdir, cert) + except Exception as exc: + with self.activity_lock: + self.now_fetching_or_processing = False + what_if_fail(exc) + + # We are not finished yet with the fetch + return thread_name + + def _start_actual_fetch(self, scheme, path, destdir, cert): + thread_name = None + url = scheme + "://" + path + + if "/" not in path: + msg = f"Missing the path component of the '{url}' URL" + raise KickstartValueError(msg) + basename = path.rsplit("/", 1)[1] + if not basename: + msg = f"Unable to deduce basename from the '{url}' URL" + raise KickstartValueError(msg) + + dest = destdir / basename + + if is_network(scheme): + thread_name = data_fetch.wait_and_fetch_net_data( + url, + dest, + cert + ) + else: # invalid schemes are handled down the road + thread_name = data_fetch.fetch_local_data( + url, + dest, + ) + return thread_name + + def finish_content_fetch(self, thread_name, fingerprint, report_callback, dest_filename, after_fetch, what_if_fail): + """ + Args: + what_if_fail: Callback accepting exception. + after_fetch: Callback accepting the content class. + """ + try: + content = self._finish_actual_fetch(thread_name, fingerprint, report_callback, dest_filename) + except Exception as exc: + what_if_fail(exc) + content = None + finally: + with self.activity_lock: + self.now_fetching_or_processing = False + + after_fetch(content) + + return content + + def _verify_fingerprint(self, dest_filename, fingerprint=""): + if not fingerprint: + return + + hash_obj = utils.get_hashing_algorithm(fingerprint) + digest = utils.get_file_fingerprint(dest_filename, + hash_obj) + if digest != fingerprint: + log.error( + "File {dest_filename} failed integrity check - assumed a " + "{hash_obj.name} hash and '{fingerprint}', got '{digest}'" + ) + msg = f"Integrity check of the content failed - {hash_obj.name} hash didn't match" + raise content_handling.ContentCheckError(msg) + + def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_filename): + threadMgr.wait(wait_for) + actually_fetched_content = wait_for is not None + + self._verify_fingerprint(dest_filename, fingerprint) + + content = ObtainedContent(self.CONTENT_DOWNLOAD_LOCATION) + + report_callback("Analyzing content.") + if not actually_fetched_content: + fpaths = [f"{common.SSG_DIR}/{common.SSG_CONTENT}"] + labelled_files = content_handling.identify_files(fpaths) + else: + dest_filename = pathlib.Path(dest_filename) + # RPM is an archive at this phase + content_type = self.get_content_type(str(dest_filename)) + if content_type in ("archive", "rpm"): + # extract the content + content.add_content_archive(dest_filename) + try: + fpaths = common.extract_data( + str(dest_filename), + str(dest_filename.parent) + ) + except common.ExtractionError as err: + msg = f"Failed to extract the '{dest_filename}' archive: {str(err)}" + log.error(msg) + raise err + + # and populate missing fields + labelled_files = content_handling.identify_files(fpaths) + + elif content_type == "file": + labelled_files = content_handling.identify_files([str(dest_filename)]) + else: + raise common.OSCAPaddonError("Unsupported content type") + + for f, l in labelled_files.items(): + content.add_file(f, l) + + if fingerprint: + content.record_verification(dest_filename) + + return content + + +class ObtainedContent: + def __init__(self, root): + self.labelled_files = dict() + self.datastream = "" + self.xccdf = "" + self.oval = "" + self.tailoring = "" + self.archive = "" + self.verified = "" + self.root = pathlib.Path(root) + + def record_verification(self, path): + assert path in self.labelled_files + self.verified = path + + def add_content_archive(self, fname): + path = pathlib.Path(fname) + self.labelled_files[path] = None + self.archive = path + + def _assign_content_type(self, attribute_name, new_value): + old_value = getattr(self, attribute_name) + if old_value: + msg = ( + f"When dealing with {attribute_name}, " + f"there was already the {old_value.name} when setting the new {new_value.name}") + raise RuntimeError(msg) + setattr(self, attribute_name, new_value) + + def add_file(self, fname, label): + path = pathlib.Path(fname) + if label == content_handling.CONTENT_TYPES["TAILORING"]: + self._assign_content_type("tailoring", path) + elif label == content_handling.CONTENT_TYPES["DATASTREAM"]: + self._assign_content_type("datastream", path) + elif label == content_handling.CONTENT_TYPES["OVAL"]: + self._assign_content_type("oval", path) + elif label == content_handling.CONTENT_TYPES["XCCDF_CHECKLIST"]: + self._assign_content_type("xccdf", path) + self.labelled_files[path] = label + + def _datastream_content(self): + if not self.datastream: + return None + if not self.datastream.exists(): + return None + return self.datastream + + def _xccdf_content(self): + if not self.xccdf or not self.oval: + return None + if not (self.xccdf.exists() and self.oval.exists()): + return None + return self.xccdf + + def find_expected_usable_content(self, relative_expected_content_path): + content_path = self.root / relative_expected_content_path + elligible_main_content = (self._datastream_content(), self._xccdf_content()) + + if content_path in elligible_main_content: + return content_path + else: + if not content_path.exists(): + msg = f"Couldn't find '{content_path}' among the available content" + else: + msg = ( + "File '{content_path}' is not a valid datastream " + "or a valid XCCDF of a XCCDF-OVAL file tuple") + raise content_handling.ContentHandlingError(msg) + + def select_main_usable_content(self): + elligible_main_content = (self._datastream_content(), self._xccdf_content()) + if not any(elligible_main_content): + msg = ( + "Couldn't find a valid datastream or a valid XCCDF-OVAL file tuple " + "among the available content") + raise content_handling.ContentHandlingError(msg) + if elligible_main_content[0]: + return elligible_main_content[0] + else: + return elligible_main_content[1] From 4a859189bc4feef8b5584dadb7cc3ce72836e028 Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Mon, 7 Jun 2021 15:25:47 +0200 Subject: [PATCH 07/16] Fied GUI installs. Anaconda will want to perform fetch, but if we install via GUI, a fetch has already been performed, and the addon should be able to acknowledge the work that has been done on the GUI side. --- org_fedora_oscap/ks/oscap.py | 6 +++++- org_fedora_oscap/model.py | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 223a8ab0..7146bbed 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -453,7 +453,11 @@ def setup(self, storage, ksdata, payload): self.model.content_uri = self.content_url thread_name = self.model.fetch_content(self.certificates, self._handle_error) - content = self.model.finish_content_fetch(thread_name, self.fingerprint, lambda msg: log.info(msg), self.raw_preinst_content_path, lambda content: content, self._handle_error) + content_dest = None + if self.content_type != "scap-security-guide": + content_dest = self.raw_preinst_content_path + + content = self.model.finish_content_fetch(thread_name, self.fingerprint, lambda msg: log.info(msg), content_dest, lambda content: content, self._handle_error) if not content: return diff --git a/org_fedora_oscap/model.py b/org_fedora_oscap/model.py index 1ae69b23..2ece5d0e 100644 --- a/org_fedora_oscap/model.py +++ b/org_fedora_oscap/model.py @@ -147,8 +147,12 @@ def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_file report_callback("Analyzing content.") if not actually_fetched_content: - fpaths = [f"{common.SSG_DIR}/{common.SSG_CONTENT}"] - labelled_files = content_handling.identify_files(fpaths) + if not dest_filename: # using scap-security-guide + fpaths = [f"{common.SSG_DIR}/{common.SSG_CONTENT}"] + labelled_files = content_handling.identify_files(fpaths) + else: # Using downloaded XCCDF/OVAL/DS/tailoring + fpaths = glob(str(self.CONTENT_DOWNLOAD_LOCATION / "*.xml")) + labelled_files = content_handling.identify_files(fpaths) else: dest_filename = pathlib.Path(dest_filename) # RPM is an archive at this phase From 3764f402aa403fe9238b1e778260094f620221fa Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Wed, 9 Jun 2021 16:36:24 +0200 Subject: [PATCH 08/16] Improved code based on review feedback. - Added docstringsu - Improved naming. - Removed redundant arguments. - Split large method into two smaller. --- org_fedora_oscap/gui/spokes/oscap.py | 9 +- org_fedora_oscap/ks/oscap.py | 5 +- org_fedora_oscap/model.py | 135 +++++++++++++++++---------- 3 files changed, 98 insertions(+), 51 deletions(-) diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index e23dface..32c28ef8 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -377,7 +377,7 @@ def _fetch_data_and_initialize(self): if self._addon_data.content_url and self._addon_data.content_type != "scap-security-guide": self.model.CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) self.model.content_uri = self._addon_data.content_url - thread_name = self.model.fetch_content(self._addon_data.certificates, self._handle_error) + thread_name = self.model.fetch_content(self._handle_error, self._addon_data.certificates) # pylint: disable-msg=E1101 hubQ.send_message(self.__class__.__name__, @@ -399,13 +399,18 @@ def _init_after_data_fetch(self, wait_for): :type wait_for: str or None """ + def update_progress_label(msg): + fire_gtk_action(self._progress_label.set_text(msg)) + content_path = None actually_fetched_content = wait_for is not None if actually_fetched_content: content_path = self._addon_data.raw_preinst_content_path - content = self.model.finish_content_fetch(wait_for, self._addon_data.fingerprint, lambda msg: log.info(msg), content_path, self._end_fetching, self._handle_error) + content = self.model.finish_content_fetch( + wait_for, self._addon_data.fingerprint, update_progress_label, + content_path, self._handle_error) if not content: with self._fetch_flag_lock: self._fetching = False diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 7146bbed..04174638 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -451,13 +451,14 @@ def setup(self, storage, ksdata, payload): if not os.path.exists(self.preinst_content_path) and not os.path.exists(self.raw_preinst_content_path): # content not available/fetched yet self.model.content_uri = self.content_url - thread_name = self.model.fetch_content(self.certificates, self._handle_error) + thread_name = self.model.fetch_content(self._handle_error, self.certificates) content_dest = None if self.content_type != "scap-security-guide": content_dest = self.raw_preinst_content_path - content = self.model.finish_content_fetch(thread_name, self.fingerprint, lambda msg: log.info(msg), content_dest, lambda content: content, self._handle_error) + content = self.model.finish_content_fetch( + thread_name, self.fingerprint, lambda msg: log.info(msg), content_dest, self._handle_error) if not content: return diff --git a/org_fedora_oscap/model.py b/org_fedora_oscap/model.py index 2ece5d0e..e71ba1fb 100644 --- a/org_fedora_oscap/model.py +++ b/org_fedora_oscap/model.py @@ -12,6 +12,8 @@ from org_fedora_oscap import common from org_fedora_oscap import content_handling +from org_fedora_oscap.common import _ + log = logging.getLogger("anaconda") @@ -23,6 +25,7 @@ def is_network(scheme): class Model: CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) / "content-download" + DEFAULT_CONTENT = f"{common.SSG_DIR}/{common.SSG_CONTENT}" def __init__(self, policy_data): self.content_uri_scheme = "" @@ -52,12 +55,23 @@ def content_uri(self, uri): self.content_uri_path = path self.content_uri_scheme = scheme - def fetch_content(self, cert, what_if_fail): + def fetch_content(self, what_if_fail, cert=""): + """ + Initiate fetch of the content into an appropriate directory + + Args: + what_if_fail: Callback accepting exception as an argument that + should handle them in the calling layer. + cert: HTTPS certificates + """ shutil.rmtree(self.CONTENT_DOWNLOAD_LOCATION, ignore_errors=True) self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True) - return self.fetch_files(self.content_uri_scheme, self.content_uri_path, self.CONTENT_DOWNLOAD_LOCATION, cert, what_if_fail) + fetching_thread_name = self._fetch_files( + self.content_uri_scheme, self.content_uri_path, + self.CONTENT_DOWNLOAD_LOCATION, cert, what_if_fail) + return fetching_thread_name - def fetch_files(self, scheme, path, destdir, cert, what_if_fail): + def _fetch_files(self, scheme, path, destdir, cert, what_if_fail): with self.activity_lock: if self.now_fetching_or_processing: msg = "Strange, it seems that we are already fetching something." @@ -65,19 +79,19 @@ def fetch_files(self, scheme, path, destdir, cert, what_if_fail): return self.now_fetching_or_processing = True - thread_name = None + fetching_thread_name = None try: - thread_name = self._start_actual_fetch(scheme, path, destdir, cert) + fetching_thread_name = self._start_actual_fetch(scheme, path, destdir, cert) except Exception as exc: with self.activity_lock: self.now_fetching_or_processing = False what_if_fail(exc) # We are not finished yet with the fetch - return thread_name + return fetching_thread_name def _start_actual_fetch(self, scheme, path, destdir, cert): - thread_name = None + fetching_thread_name = None url = scheme + "://" + path if "/" not in path: @@ -91,26 +105,41 @@ def _start_actual_fetch(self, scheme, path, destdir, cert): dest = destdir / basename if is_network(scheme): - thread_name = data_fetch.wait_and_fetch_net_data( + fetching_thread_name = data_fetch.wait_and_fetch_net_data( url, dest, cert ) else: # invalid schemes are handled down the road - thread_name = data_fetch.fetch_local_data( + fetching_thread_name = data_fetch.fetch_local_data( url, dest, ) - return thread_name + return fetching_thread_name - def finish_content_fetch(self, thread_name, fingerprint, report_callback, dest_filename, after_fetch, what_if_fail): + def finish_content_fetch(self, fetching_thread_name, fingerprint, report_callback, dest_filename, + what_if_fail): """ + Finish any ongoing fetch and analyze what has been fetched. + + After the fetch is completed, it analyzes verifies fetched content if applicable, + analyzes it and compiles into an instance of ObtainedContent. + Args: - what_if_fail: Callback accepting exception. - after_fetch: Callback accepting the content class. + fetching_thread_name: Name of the fetching thread + or None if we are only after the analysis + fingerprint: A checksum for downloaded file verification + report_callback: Means for the method to send user-relevant messages outside + dest_filename: The target of the fetch operation. Can be falsy - + in this case there is no content filename defined + what_if_fail: Callback accepting exception as an argument + that should handle them in the calling layer. + + Returns: + Instance of ObtainedContent if everything went well, or None. """ try: - content = self._finish_actual_fetch(thread_name, fingerprint, report_callback, dest_filename) + content = self._finish_actual_fetch(fetching_thread_name, fingerprint, report_callback, dest_filename) except Exception as exc: what_if_fail(exc) content = None @@ -118,8 +147,6 @@ def finish_content_fetch(self, thread_name, fingerprint, report_callback, dest_f with self.activity_lock: self.now_fetching_or_processing = False - after_fetch(content) - return content def _verify_fingerprint(self, dest_filename, fingerprint=""): @@ -131,35 +158,47 @@ def _verify_fingerprint(self, dest_filename, fingerprint=""): hash_obj) if digest != fingerprint: log.error( - "File {dest_filename} failed integrity check - assumed a " - "{hash_obj.name} hash and '{fingerprint}', got '{digest}'" + f"File {dest_filename} failed integrity check - assumed a " + f"{hash_obj.name} hash and '{fingerprint}', got '{digest}'" ) - msg = f"Integrity check of the content failed - {hash_obj.name} hash didn't match" + msg = _(f"Integrity check of the content failed - {hash_obj.name} hash didn't match") raise content_handling.ContentCheckError(msg) def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_filename): threadMgr.wait(wait_for) actually_fetched_content = wait_for is not None - self._verify_fingerprint(dest_filename, fingerprint) + if fingerprint and dest_filename: + self._verify_fingerprint(dest_filename, fingerprint) + + fpaths = self._gather_available_files(actually_fetched_content, dest_filename) + + structured_content = ObtainedContent(self.CONTENT_DOWNLOAD_LOCATION) + content_type = self.get_content_type(str(dest_filename)) + if content_type in ("archive", "rpm"): + structured_content.add_content_archive(dest_filename) + + labelled_files = content_handling.identify_files(fpaths) + for fname, label in labelled_files.items(): + structured_content.add_file(fname, label) + + if fingerprint and dest_filename: + structured_content.record_verification(dest_filename) - content = ObtainedContent(self.CONTENT_DOWNLOAD_LOCATION) + return structured_content - report_callback("Analyzing content.") + def _gather_available_files(self, actually_fetched_content, dest_filename): + fpaths = [] if not actually_fetched_content: if not dest_filename: # using scap-security-guide - fpaths = [f"{common.SSG_DIR}/{common.SSG_CONTENT}"] - labelled_files = content_handling.identify_files(fpaths) + fpaths = [self.DEFAULT_CONTENT] else: # Using downloaded XCCDF/OVAL/DS/tailoring fpaths = glob(str(self.CONTENT_DOWNLOAD_LOCATION / "*.xml")) - labelled_files = content_handling.identify_files(fpaths) else: dest_filename = pathlib.Path(dest_filename) # RPM is an archive at this phase content_type = self.get_content_type(str(dest_filename)) if content_type in ("archive", "rpm"): - # extract the content - content.add_content_archive(dest_filename) try: fpaths = common.extract_data( str(dest_filename), @@ -170,24 +209,20 @@ def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_file log.error(msg) raise err - # and populate missing fields - labelled_files = content_handling.identify_files(fpaths) - elif content_type == "file": - labelled_files = content_handling.identify_files([str(dest_filename)]) + fpaths = [str(dest_filename)] else: raise common.OSCAPaddonError("Unsupported content type") - - for f, l in labelled_files.items(): - content.add_file(f, l) - - if fingerprint: - content.record_verification(dest_filename) - - return content + return fpaths class ObtainedContent: + """ + This class aims to assist the gathered files discovery - + the addon can downloaded files directly, or they can be extracted for an archive. + The class enables user to quickly understand what is available, + and whether the current set of contents is usable for further processing. + """ def __init__(self, root): self.labelled_files = dict() self.datastream = "" @@ -199,10 +234,17 @@ def __init__(self, root): self.root = pathlib.Path(root) def record_verification(self, path): + """ + Declare a file as verified (typically by means of a checksum) + """ + path = pathlib.Path(path) assert path in self.labelled_files self.verified = path def add_content_archive(self, fname): + """ + If files come from an archive, record this information using this function. + """ path = pathlib.Path(fname) self.labelled_files[path] = None self.archive = path @@ -244,9 +286,9 @@ def _xccdf_content(self): def find_expected_usable_content(self, relative_expected_content_path): content_path = self.root / relative_expected_content_path - elligible_main_content = (self._datastream_content(), self._xccdf_content()) + eligible_main_content = (self._datastream_content(), self._xccdf_content()) - if content_path in elligible_main_content: + if content_path in eligible_main_content: return content_path else: if not content_path.exists(): @@ -258,13 +300,12 @@ def find_expected_usable_content(self, relative_expected_content_path): raise content_handling.ContentHandlingError(msg) def select_main_usable_content(self): - elligible_main_content = (self._datastream_content(), self._xccdf_content()) - if not any(elligible_main_content): + if self._datastream_content(): + return self._datastream_content() + elif self._xccdf_content(): + return self._xccdf_content() + else: msg = ( "Couldn't find a valid datastream or a valid XCCDF-OVAL file tuple " "among the available content") raise content_handling.ContentHandlingError(msg) - if elligible_main_content[0]: - return elligible_main_content[0] - else: - return elligible_main_content[1] From ecdfff1cf159eb1d4a78ab4957c8d0aa3e1c7bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Thu, 10 Jun 2021 14:37:20 +0200 Subject: [PATCH 09/16] Pulled more functionality to the Model class. --- org_fedora_oscap/gui/spokes/oscap.py | 12 ++++------ org_fedora_oscap/ks/oscap.py | 21 +---------------- org_fedora_oscap/model.py | 35 +++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index 32c28ef8..ba18bc6e 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -252,7 +252,7 @@ def __init__(self, data, storage, payload): self._anaconda_spokes_initialized = threading.Event() self.initialization_controller.init_done.connect(self._all_anaconda_spokes_initialized) - self.model = Model(None) + self.model = Model(self._addon_data) def _all_anaconda_spokes_initialized(self): log.debug("OSCAP addon: Anaconda init_done signal triggered") @@ -418,14 +418,14 @@ def update_progress_label(msg): return try: - preferred_content = self._addon_data.get_preferred_content(content) if actually_fetched_content: - self._addon_data.update_with_content(content) + self.model.use_downloaded_content(content) + log.info(f"{self._addon_data.preinst_content_path}, {self._addon_data.preinst_tailoring_path}") self._content_handler = scap_content_handler.SCAPContentHandler( self._addon_data.preinst_content_path, self._addon_data.preinst_tailoring_path) except Exception as e: - log.warning(str(e)) + log.error(str(e)) self._invalid_content() # fetching done with self._fetch_flag_lock: @@ -1148,7 +1148,5 @@ def on_change_content_clicked(self, *args): self.refresh() def on_use_ssg_clicked(self, *args): - self._addon_data.clear_all() - self._addon_data.content_type = "scap-security-guide" - self._addon_data.content_path = common.SSG_DIR + common.SSG_CONTENT + self.model.use_system_content() self._fetch_data_and_initialize() diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 04174638..d1dc77c7 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -373,25 +373,6 @@ def postinst_tailoring_path(self): return utils.join_paths(common.TARGET_CONTENT_DIR, self.tailoring_path) - def get_preferred_content(self, content): - if self.content_path: - preferred_content = content.find_expected_usable_content(self.content_path) - else: - preferred_content = content.select_main_usable_content() - - if self.tailoring_path: - if self.tailoring_path != str(content.tailoring.relative_to(content.root)): - msg = f"Expected a tailoring {self.tailoring_path}, but it couldn't be found" - raise content_handling.ContentHandlingError(msg) - return preferred_content - - def update_with_content(self, content): - preferred_content = self.get_preferred_content(content) - - self.content_path = str(preferred_content.relative_to(content.root)) - if content.tailoring: - self.tailoring_path = str(content.tailoring.relative_to(content.root)) - def _terminate(self, message): if flags.flags.automatedInstall and not flags.flags.ksprompt: # cannot have ask in a non-interactive kickstart @@ -465,7 +446,7 @@ def setup(self, storage, ksdata, payload): try: # just check that preferred content exists - _ = self.get_preferred_content(content) + _ = self.model.get_preferred_content(content) except Exception as exc: self._terminate(str(exc)) return diff --git a/org_fedora_oscap/model.py b/org_fedora_oscap/model.py index e71ba1fb..8a4d7192 100644 --- a/org_fedora_oscap/model.py +++ b/org_fedora_oscap/model.py @@ -27,7 +27,7 @@ class Model: CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) / "content-download" DEFAULT_CONTENT = f"{common.SSG_DIR}/{common.SSG_CONTENT}" - def __init__(self, policy_data): + def __init__(self, addon_data): self.content_uri_scheme = "" self.content_uri_path = "" self.fetched_content = "" @@ -37,6 +37,8 @@ def __init__(self, policy_data): self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True) + self._addon_data = addon_data + def get_content_type(self, url): if url.endswith(".rpm"): return "rpm" @@ -215,6 +217,37 @@ def _gather_available_files(self, actually_fetched_content, dest_filename): raise common.OSCAPaddonError("Unsupported content type") return fpaths + def use_downloaded_content(self, content): + preferred_content = self.get_preferred_content(content) + + # We know that we have ended up with a datastream-like content, + # but if we can't convert an archive to a datastream. + # self._addon_data.content_type = "datastream" + self._addon_data.content_path = str(preferred_content.relative_to(content.root)) + + preferred_tailoring = self.get_preferred_tailoring(content) + if content.tailoring: + self._addon_data.tailoring_path = str(preferred_tailoring.relative_to(content.root)) + + def use_system_content(self, content=None): + self._addon_data.clear_all() + self._addon_data.content_type = "scap-security-guide" + self._addon_data.content_path = common.get_ssg_path() + + def get_preferred_content(self, content): + if self._addon_data.content_path: + preferred_content = content.find_expected_usable_content(self._addon_data.content_path) + else: + preferred_content = content.select_main_usable_content() + return preferred_content + + def get_preferred_tailoring(self, content): + if self._addon_data.tailoring_path: + if self._addon_data.tailoring_path != str(content.tailoring.relative_to(content.root)): + msg = f"Expected a tailoring {self.tailoring_path}, but it couldn't be found" + raise content_handling.ContentHandlingError(msg) + return content.tailoring + class ObtainedContent: """ From 4574d94dd3a6aa8ef0d24afd3fb9a57851dfbae3 Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Thu, 10 Jun 2021 16:39:20 +0200 Subject: [PATCH 10/16] Refactored handling of kickstart install abort. --- org_fedora_oscap/ks/oscap.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index d1dc77c7..37b472d3 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -374,12 +374,14 @@ def postinst_tailoring_path(self): self.tailoring_path) def _terminate(self, message): + message += "\n" + _("The installation should be aborted.") + message += " " + _("Do you wish to continue anyway?") if flags.flags.automatedInstall and not flags.flags.ksprompt: # cannot have ask in a non-interactive kickstart # installation raise errors.CmdlineError(message) - answ = errors.errorHandler.ui.showYesNoQuestion(msg) + answ = errors.errorHandler.ui.showYesNoQuestion(message) if answ == errors.ERROR_CONTINUE: # prevent any futher actions here by switching to the dry # run mode and let things go on @@ -396,14 +398,12 @@ def _handle_error(self, exception): log.error("Failed to fetch and initialize SCAP content!") if isinstance(exception, ContentCheckError): - msg = _("The integrity check of the security content failed.\n" + - "The installation should be aborted. Do you wish to continue anyway?") + msg = _("The integrity check of the security content failed.") self._terminate(msg) elif (isinstance(exception, common.OSCAPaddonError) or isinstance(exception, data_fetch.DataFetchError)): msg = _("There was an error fetching and loading the security content:\n" + - "%s\n" + - "The installation should be aborted. Do you wish to continue anyway?") % str(exception) + f"{str(exception)}") self._terminate(msg) else: @@ -462,7 +462,6 @@ def setup(self, storage, ksdata, payload): if any(fatal_messages): msg = ["Wrong configuration detected!"] msg.extend(fatal_messages) - msg.append("The installation should be aborted. Do you wish to continue anyway?") self._terminate("\n".join(msg)) return From 5c116dc4735a40b7d00779d35aabcffba8ce7c86 Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Thu, 10 Jun 2021 17:05:53 +0200 Subject: [PATCH 11/16] Rebranded the model class. --- .../{model.py => content_discovery.py} | 5 +++-- org_fedora_oscap/gui/spokes/oscap.py | 15 +++++++-------- org_fedora_oscap/ks/oscap.py | 12 +++++------- 3 files changed, 15 insertions(+), 17 deletions(-) rename org_fedora_oscap/{model.py => content_discovery.py} (99%) diff --git a/org_fedora_oscap/model.py b/org_fedora_oscap/content_discovery.py similarity index 99% rename from org_fedora_oscap/model.py rename to org_fedora_oscap/content_discovery.py index 8a4d7192..8feabd4f 100644 --- a/org_fedora_oscap/model.py +++ b/org_fedora_oscap/content_discovery.py @@ -23,8 +23,8 @@ def is_network(scheme): for net_prefix in data_fetch.NET_URL_PREFIXES) -class Model: - CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) / "content-download" +class ContentBringer: + CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) DEFAULT_CONTENT = f"{common.SSG_DIR}/{common.SSG_CONTENT}" def __init__(self, addon_data): @@ -66,6 +66,7 @@ def fetch_content(self, what_if_fail, cert=""): should handle them in the calling layer. cert: HTTPS certificates """ + self.content_uri = self._addon_data.content_url shutil.rmtree(self.CONTENT_DOWNLOAD_LOCATION, ignore_errors=True) self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True) fetching_thread_name = self._fetch_files( diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index ba18bc6e..98000fcf 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -32,7 +32,7 @@ from org_fedora_oscap import scap_content_handler from org_fedora_oscap import utils from org_fedora_oscap.common import dry_run_skip -from org_fedora_oscap.model import Model +from org_fedora_oscap.content_discovery import ContentBringer from pyanaconda.threading import threadMgr, AnacondaThread from pyanaconda.ui.gui.spokes import NormalSpoke from pyanaconda.ui.communication import hubQ @@ -252,7 +252,7 @@ def __init__(self, data, storage, payload): self._anaconda_spokes_initialized = threading.Event() self.initialization_controller.init_done.connect(self._all_anaconda_spokes_initialized) - self.model = Model(self._addon_data) + self.content_bringer = ContentBringer(self._addon_data) def _all_anaconda_spokes_initialized(self): log.debug("OSCAP addon: Anaconda init_done signal triggered") @@ -375,9 +375,8 @@ def _fetch_data_and_initialize(self): thread_name = None if self._addon_data.content_url and self._addon_data.content_type != "scap-security-guide": - self.model.CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) - self.model.content_uri = self._addon_data.content_url - thread_name = self.model.fetch_content(self._handle_error, self._addon_data.certificates) + thread_name = self.content_bringer.fetch_content( + self._handle_error, self._addon_data.certificates) # pylint: disable-msg=E1101 hubQ.send_message(self.__class__.__name__, @@ -408,7 +407,7 @@ def update_progress_label(msg): if actually_fetched_content: content_path = self._addon_data.raw_preinst_content_path - content = self.model.finish_content_fetch( + content = self.content_bringer.finish_content_fetch( wait_for, self._addon_data.fingerprint, update_progress_label, content_path, self._handle_error) if not content: @@ -419,7 +418,7 @@ def update_progress_label(msg): try: if actually_fetched_content: - self.model.use_downloaded_content(content) + self.content_bringer.use_downloaded_content(content) log.info(f"{self._addon_data.preinst_content_path}, {self._addon_data.preinst_tailoring_path}") self._content_handler = scap_content_handler.SCAPContentHandler( self._addon_data.preinst_content_path, @@ -1148,5 +1147,5 @@ def on_change_content_clicked(self, *args): self.refresh() def on_use_ssg_clicked(self, *args): - self.model.use_system_content() + self.content_bringer.use_system_content() self._fetch_data_and_initialize() diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 37b472d3..eaa733bc 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -37,7 +37,7 @@ from org_fedora_oscap import utils, common, rule_handling, data_fetch from org_fedora_oscap.common import SUPPORTED_ARCHIVES, _ from org_fedora_oscap.content_handling import ContentCheckError, ContentHandlingError -from org_fedora_oscap import model +from org_fedora_oscap import content_discovery log = logging.getLogger("anaconda") @@ -104,8 +104,7 @@ def __init__(self, name, just_clear=False): self.rule_data = rule_handling.RuleData() self.dry_run = False - self.model = model.Model(self) - self.model.CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) + self.content_bringer = content_discovery.ContentBringer(self) def __str__(self): """ @@ -431,14 +430,13 @@ def setup(self, storage, ksdata, payload): thread_name = None if not os.path.exists(self.preinst_content_path) and not os.path.exists(self.raw_preinst_content_path): # content not available/fetched yet - self.model.content_uri = self.content_url - thread_name = self.model.fetch_content(self._handle_error, self.certificates) + thread_name = self.content_bringer.fetch_content(self._handle_error, self.certificates) content_dest = None if self.content_type != "scap-security-guide": content_dest = self.raw_preinst_content_path - content = self.model.finish_content_fetch( + content = self.content_bringer.finish_content_fetch( thread_name, self.fingerprint, lambda msg: log.info(msg), content_dest, self._handle_error) if not content: @@ -446,7 +444,7 @@ def setup(self, storage, ksdata, payload): try: # just check that preferred content exists - _ = self.model.get_preferred_content(content) + _ = self.content_bringer.get_preferred_content(content) except Exception as exc: self._terminate(str(exc)) return From ff1cc8335b973d6db38471818ff1bd19f3ba011e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Wed, 16 Jun 2021 17:44:27 +0200 Subject: [PATCH 12/16] Improve code style. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Černý --- org_fedora_oscap/content_discovery.py | 6 +++--- org_fedora_oscap/gui/spokes/oscap.py | 9 ++++++++- org_fedora_oscap/ks/oscap.py | 6 +++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/org_fedora_oscap/content_discovery.py b/org_fedora_oscap/content_discovery.py index 8feabd4f..2af78999 100644 --- a/org_fedora_oscap/content_discovery.py +++ b/org_fedora_oscap/content_discovery.py @@ -25,7 +25,7 @@ def is_network(scheme): class ContentBringer: CONTENT_DOWNLOAD_LOCATION = pathlib.Path(common.INSTALLATION_CONTENT_DIR) - DEFAULT_CONTENT = f"{common.SSG_DIR}/{common.SSG_CONTENT}" + DEFAULT_SSG_DATA_STREAM_PATH = f"{common.SSG_DIR}/{common.SSG_CONTENT}" def __init__(self, addon_data): self.content_uri_scheme = "" @@ -194,7 +194,7 @@ def _gather_available_files(self, actually_fetched_content, dest_filename): fpaths = [] if not actually_fetched_content: if not dest_filename: # using scap-security-guide - fpaths = [self.DEFAULT_CONTENT] + fpaths = [self.DEFAULT_SSG_DATA_STREAM_PATH] else: # Using downloaded XCCDF/OVAL/DS/tailoring fpaths = glob(str(self.CONTENT_DOWNLOAD_LOCATION / "*.xml")) else: @@ -329,7 +329,7 @@ def find_expected_usable_content(self, relative_expected_content_path): msg = f"Couldn't find '{content_path}' among the available content" else: msg = ( - "File '{content_path}' is not a valid datastream " + f"File '{content_path}' is not a valid datastream " "or a valid XCCDF of a XCCDF-OVAL file tuple") raise content_handling.ContentHandlingError(msg) diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index 98000fcf..b9b3704d 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -419,7 +419,14 @@ def update_progress_label(msg): try: if actually_fetched_content: self.content_bringer.use_downloaded_content(content) - log.info(f"{self._addon_data.preinst_content_path}, {self._addon_data.preinst_tailoring_path}") + + msg = f"Opening SCAP content at {self._addon_data.preinst_content_path}" + if self._addon_data.tailoring_path: + msg += f" with tailoring {self._addon_data.preinst_tailoring_path}" + else: + msg += " without considering tailoring" + log.info(msg) + self._content_handler = scap_content_handler.SCAPContentHandler( self._addon_data.preinst_content_path, self._addon_data.preinst_tailoring_path) diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index eaa733bc..5b5ba762 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -458,9 +458,9 @@ def setup(self, storage, ksdata, payload): fatal_messages = [message for message in self.rule_data.eval_rules(ksdata, storage) if message.type == common.MESSAGE_TYPE_FATAL] if any(fatal_messages): - msg = ["Wrong configuration detected!"] - msg.extend(fatal_messages) - self._terminate("\n".join(msg)) + msg_lines = [_("Wrong configuration detected!")] + msg_lines.extend(fatal_messages) + self._terminate("\n".join(msg_lines)) return # add packages needed on the target system to the list of packages From 76cf4046638fa5c927cc0120e5b5c406aaef1996 Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Thu, 17 Jun 2021 13:34:25 +0200 Subject: [PATCH 13/16] Cleaned up tests. Tests became obsolete because of earlier refactoring. --- tests/test_content_handling.py | 67 ---------------------------------- tests/test_ks_oscap.py | 5 +-- 2 files changed, 2 insertions(+), 70 deletions(-) diff --git a/tests/test_content_handling.py b/tests/test_content_handling.py index b21b8e91..56dfe686 100644 --- a/tests/test_content_handling.py +++ b/tests/test_content_handling.py @@ -20,73 +20,6 @@ PROFILE3_ID = "xccdf_com.example_profile_my_profile3" -@pytest.fixture() -def ds_handler(): - return ch.DataStreamHandler(DS_FILEPATH) - - -def test_init_invalid_file_path(): - with pytest.raises(ch.DataStreamHandlingError) as excinfo: - ch.DataStreamHandler("testing_ds.xmlll") - assert "Invalid file path" in str(excinfo.value) - - -def test_init_not_scap_content(): - with pytest.raises(ch.DataStreamHandlingError) as excinfo: - ch.DataStreamHandler(os.path.join(TESTING_FILES_PATH, "testing_ks.cfg")) - assert "not a valid SCAP content file" in str(excinfo.value) - - -def test_init_xccdf_content(): - with pytest.raises(ch.DataStreamHandlingError) as excinfo: - ch.DataStreamHandler(os.path.join(TESTING_FILES_PATH, "xccdf.xml")) - assert "not a data stream collection" in str(excinfo.value) - - -def test_get_data_streams(ds_handler): - assert DS_IDS in ds_handler.get_data_streams() - - -def test_get_data_streams_checklists(ds_handler): - expected_ids = {DS_IDS: [CHK_FIRST_ID, CHK_SECOND_ID]} - - ds_ids = ds_handler.get_data_streams_checklists() - assert expected_ids == ds_ids - - -def test_get_checklists(ds_handler): - expected_checklists = [CHK_FIRST_ID, CHK_SECOND_ID] - - chk_ids = ds_handler.get_checklists(DS_IDS) - assert expected_checklists == chk_ids - - -def test_get_checklists_invalid(ds_handler): - with pytest.raises(ch.DataStreamHandlingError) as excinfo: - ds_handler.get_checklists("invalid.id") - assert "Invalid data stream id given" in str(excinfo.value) - - -def test_get_profiles(ds_handler): - profile_ids = ds_handler.get_profiles(DS_IDS, CHK_FIRST_ID) - - # When Benchmark doesn't contain Rules selected by default - # the default Profile should not be present - assert 2 == len(profile_ids) - assert PROFILE1_ID == profile_ids[0].id - assert PROFILE2_ID == profile_ids[1].id - - -def test_get_profiles_with_default(ds_handler): - profile_ids = ds_handler.get_profiles(DS_IDS, CHK_SECOND_ID) - - # When Benchmark contains Rules selected by default - # the default Profile should be present - assert 2 == len(profile_ids) - assert "default" == profile_ids[0].id - assert PROFILE3_ID == profile_ids[1].id - - def test_identify_files(): filenames = glob.glob(TESTING_FILES_PATH + "/*") identified = ch.identify_files(filenames) diff --git a/tests/test_ks_oscap.py b/tests/test_ks_oscap.py index 4baafec0..ac9bc2d2 100644 --- a/tests/test_ks_oscap.py +++ b/tests/test_ks_oscap.py @@ -311,12 +311,11 @@ def test_valid_fingerprints(blank_oscap_data): def test_invalid_fingerprints(blank_oscap_data): # invalid character - with pytest.raises( - KickstartValueError, message="Unsupported or invalid fingerprint"): + with pytest.raises(KickstartValueError, match="Unsupported or invalid fingerprint"): blank_oscap_data.handle_line("fingerprint = %s?" % ("a" * 31)) # invalid lengths (odd and even) for repetitions in (31, 41, 54, 66, 98, 124): with pytest.raises( - KickstartValueError, message="Unsupported fingerprint"): + KickstartValueError, match="Unsupported fingerprint"): blank_oscap_data.handle_line("fingerprint = %s" % ("a" * repetitions)) From 670ba00cd0ea939172f6f8f706969de7ff58be9e Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Thu, 17 Jun 2021 13:35:09 +0200 Subject: [PATCH 14/16] Increased robustness of content handling. - Multiple OVAL files in an archive cause no longer an error. - Multiple instances of other filetypes result in a content-related error. - Unexpected errors don't crash the addon. --- org_fedora_oscap/content_discovery.py | 11 ++++++----- org_fedora_oscap/gui/spokes/oscap.py | 8 +++++++- org_fedora_oscap/ks/oscap.py | 3 ++- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/org_fedora_oscap/content_discovery.py b/org_fedora_oscap/content_discovery.py index 2af78999..056f11af 100644 --- a/org_fedora_oscap/content_discovery.py +++ b/org_fedora_oscap/content_discovery.py @@ -261,7 +261,7 @@ def __init__(self, root): self.labelled_files = dict() self.datastream = "" self.xccdf = "" - self.oval = "" + self.ovals = [] self.tailoring = "" self.archive = "" self.verified = "" @@ -289,7 +289,7 @@ def _assign_content_type(self, attribute_name, new_value): msg = ( f"When dealing with {attribute_name}, " f"there was already the {old_value.name} when setting the new {new_value.name}") - raise RuntimeError(msg) + raise content_handling.ContentHandlingError(msg) setattr(self, attribute_name, new_value) def add_file(self, fname, label): @@ -299,7 +299,7 @@ def add_file(self, fname, label): elif label == content_handling.CONTENT_TYPES["DATASTREAM"]: self._assign_content_type("datastream", path) elif label == content_handling.CONTENT_TYPES["OVAL"]: - self._assign_content_type("oval", path) + self.ovals.append(path) elif label == content_handling.CONTENT_TYPES["XCCDF_CHECKLIST"]: self._assign_content_type("xccdf", path) self.labelled_files[path] = label @@ -312,9 +312,10 @@ def _datastream_content(self): return self.datastream def _xccdf_content(self): - if not self.xccdf or not self.oval: + if not self.xccdf or not self.ovals: return None - if not (self.xccdf.exists() and self.oval.exists()): + some_ovals_exist = any([path.exists() for path in self.ovals]) + if not (self.xccdf.exists() and some_ovals_exist): return None return self.xccdf diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py index b9b3704d..c57b1cdb 100644 --- a/org_fedora_oscap/gui/spokes/oscap.py +++ b/org_fedora_oscap/gui/spokes/oscap.py @@ -352,7 +352,7 @@ def _handle_error(self, exception): elif isinstance(exception, content_handling.ContentCheckError): self._integrity_check_failed() else: - raise exception + self._general_content_problem() def _end_fetching(self, fetched_content): # stop the spinner in any case @@ -769,6 +769,12 @@ def _set_error(self, msg): self._error = None self.clear_info() + @async_action_wait + def _general_content_problem(self): + msg = _("There was an unexpected problem with the supplied content.") + self._progress_label.set_markup("%s" % msg) + self._wrong_content(msg) + @async_action_wait def _invalid_content(self): """Callback for informing user about provided content invalidity.""" diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 5b5ba762..2172293e 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -406,7 +406,8 @@ def _handle_error(self, exception): self._terminate(msg) else: - raise exception + msg = _("There was an unexpected problem with the supplied content.") + self._terminate(msg) def setup(self, storage, ksdata, payload): """ From 9e0dd0958a4d4d6f105fd7054c63dec688e8130f Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Thu, 17 Jun 2021 13:55:23 +0200 Subject: [PATCH 15/16] Update gating test configuration. - mock is part of the Python standard library. - scanner is used in unit tests. --- .github/workflows/build.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index e8ec26c0..e9eaedb4 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -13,7 +13,7 @@ jobs: image: fedora:32 steps: - name: Install Deps - run: dnf install -y make anaconda openscap-python3 python3-cpio python3-mock python3-pytest python3-pycurl + run: dnf install -y make anaconda openscap-scanner openscap-python3 python3-cpio python3-pytest python3-pycurl - name: Checkout uses: actions/checkout@v2 - name: Test From 1732af68a325591c1727892eb2958dff1c64433a Mon Sep 17 00:00:00 2001 From: Matej Tyc Date: Thu, 17 Jun 2021 14:19:40 +0200 Subject: [PATCH 16/16] Come up with a better name for the certificate path. --- org_fedora_oscap/content_discovery.py | 14 ++++++------- org_fedora_oscap/data_fetch.py | 30 +++++++++++++-------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/org_fedora_oscap/content_discovery.py b/org_fedora_oscap/content_discovery.py index 056f11af..63930865 100644 --- a/org_fedora_oscap/content_discovery.py +++ b/org_fedora_oscap/content_discovery.py @@ -57,24 +57,24 @@ def content_uri(self, uri): self.content_uri_path = path self.content_uri_scheme = scheme - def fetch_content(self, what_if_fail, cert=""): + def fetch_content(self, what_if_fail, ca_certs_path=""): """ Initiate fetch of the content into an appropriate directory Args: what_if_fail: Callback accepting exception as an argument that should handle them in the calling layer. - cert: HTTPS certificates + ca_certs_path: Path to the HTTPS certificate file """ self.content_uri = self._addon_data.content_url shutil.rmtree(self.CONTENT_DOWNLOAD_LOCATION, ignore_errors=True) self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True) fetching_thread_name = self._fetch_files( self.content_uri_scheme, self.content_uri_path, - self.CONTENT_DOWNLOAD_LOCATION, cert, what_if_fail) + self.CONTENT_DOWNLOAD_LOCATION, ca_certs_path, what_if_fail) return fetching_thread_name - def _fetch_files(self, scheme, path, destdir, cert, what_if_fail): + def _fetch_files(self, scheme, path, destdir, ca_certs_path, what_if_fail): with self.activity_lock: if self.now_fetching_or_processing: msg = "Strange, it seems that we are already fetching something." @@ -84,7 +84,7 @@ def _fetch_files(self, scheme, path, destdir, cert, what_if_fail): fetching_thread_name = None try: - fetching_thread_name = self._start_actual_fetch(scheme, path, destdir, cert) + fetching_thread_name = self._start_actual_fetch(scheme, path, destdir, ca_certs_path) except Exception as exc: with self.activity_lock: self.now_fetching_or_processing = False @@ -93,7 +93,7 @@ def _fetch_files(self, scheme, path, destdir, cert, what_if_fail): # We are not finished yet with the fetch return fetching_thread_name - def _start_actual_fetch(self, scheme, path, destdir, cert): + def _start_actual_fetch(self, scheme, path, destdir, ca_certs_path): fetching_thread_name = None url = scheme + "://" + path @@ -111,7 +111,7 @@ def _start_actual_fetch(self, scheme, path, destdir, cert): fetching_thread_name = data_fetch.wait_and_fetch_net_data( url, dest, - cert + ca_certs_path ) else: # invalid schemes are handled down the road fetching_thread_name = data_fetch.fetch_local_data( diff --git a/org_fedora_oscap/data_fetch.py b/org_fedora_oscap/data_fetch.py index e8d2b7d4..1ce7a260 100644 --- a/org_fedora_oscap/data_fetch.py +++ b/org_fedora_oscap/data_fetch.py @@ -95,7 +95,7 @@ def fetch_local_data(url, out_file): return common.THREAD_FETCH_DATA -def wait_and_fetch_net_data(url, out_file, ca_certs=None): +def wait_and_fetch_net_data(url, out_file, ca_certs_path=None): """ Function that waits for network connection and starts a thread that fetches data over network. @@ -119,7 +119,7 @@ def wait_and_fetch_net_data(url, out_file, ca_certs=None): log.info(f"Fetching data from {url}") fetch_data_thread = AnacondaThread(name=common.THREAD_FETCH_DATA, target=fetch_data, - args=(url, out_file, ca_certs), + args=(url, out_file, ca_certs_path), fatal=False) # register and run the thread @@ -143,9 +143,9 @@ def can_fetch_from(url): return any(url.startswith(prefix) for prefix in resources) -def fetch_data(url, out_file, ca_certs=None): +def fetch_data(url, out_file, ca_certs_path=None): """ - Fetch data from a given URL. If the URL starts with https://, ca_certs can + Fetch data from a given URL. If the URL starts with https://, ca_certs_path can be a path to PEM file with CA certificate chain to validate server certificate. @@ -153,10 +153,10 @@ def fetch_data(url, out_file, ca_certs=None): :type url: str :param out_file: path to the output file :type out_file: str - :param ca_certs: path to a PEM file with CA certificate chain - :type ca_certs: str + :param ca_certs_path: path to a PEM file with CA certificate chain + :type ca_certs_path: str :raise WrongRequestError: if a wrong combination of arguments is passed - (ca_certs file path given and url starting with + (ca_certs_path file path given and url starting with http://) or arguments don't have required format :raise CertificateValidationError: if server certificate validation fails :raise FetchError: if data fetching fails (usually due to I/O errors) @@ -168,14 +168,14 @@ def fetch_data(url, out_file, ca_certs=None): utils.ensure_dir_exists(out_dir) if can_fetch_from(url): - _curl_fetch(url, out_file, ca_certs) + _curl_fetch(url, out_file, ca_certs_path) else: msg = "Cannot fetch data from '%s': unknown URL format" % url raise UnknownURLformatError(msg) log.info(f"Data fetch from {url} completed") -def _curl_fetch(url, out_file, ca_certs=None): +def _curl_fetch(url, out_file, ca_certs_path=None): """ Function that fetches data and writes it out to the given file path. If a path to the file with CA certificates is given and the url starts with @@ -185,11 +185,11 @@ def _curl_fetch(url, out_file, ca_certs=None): :type url: str :param out_file: path to the output file :type out_file: str - :param ca_certs: path to the file with CA certificates for server + :param ca_certs_path: path to the file with CA certificates for server certificate validation - :type ca_certs: str + :type ca_certs_path: str :raise WrongRequestError: if a wrong combination of arguments is passed - (ca_certs file path given and url starting with + (ca_certs_path file path given and url starting with http://) or arguments don't have required format :raise CertificateValidationError: if server certificate validation fails :raise FetchError: if data fetching fails (usually due to I/O errors) @@ -223,18 +223,18 @@ def _curl_fetch(url, out_file, ca_certs=None): if not out_file: raise WrongRequestError("out_file cannot be an empty string") - if ca_certs and protocol != "https": + if ca_certs_path and protocol != "https": msg = "Cannot verify server certificate when using plain HTTP" raise WrongRequestError(msg) curl = pycurl.Curl() curl.setopt(pycurl.URL, url) - if ca_certs and protocol == "https": + if ca_certs_path and protocol == "https": # the strictest verification curl.setopt(pycurl.SSL_VERIFYHOST, 2) curl.setopt(pycurl.SSL_VERIFYPEER, 1) - curl.setopt(pycurl.CAINFO, ca_certs) + curl.setopt(pycurl.CAINFO, ca_certs_path) # may be turned off by flags (specified on command line, take precedence) if not conf.payload.verify_ssl: