Fix multiple small issues

- Fix problems with handling multi-datastream archives
- Fix a crash when compulsively clicking in the GUI

Resolves: rhbz#2129846
Resolves: rhbz#2127502
This commit is contained in:
Matej Tyc 2022-11-11 16:09:48 +01:00
parent 542912f10e
commit c7ab66cabd
3 changed files with 463 additions and 1 deletions

View File

@ -0,0 +1,380 @@
From a1b983b4b5f8e49daa978aec6f9d28ba6dcea20c Mon Sep 17 00:00:00 2001
From: Matej Tyc <matyc@redhat.com>
Date: Wed, 12 Oct 2022 11:37:04 +0200
Subject: [PATCH 1/5] Add capability to preselect content from archives
Users can specify content path and tailoring path in kickstarts,
and the addon should be able to assure that those files are available,
and that they have precedence over other files.
---
org_fedora_oscap/content_discovery.py | 35 +++++++++++++++++++
tests/test_content_discovery.py | 48 +++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
create mode 100644 tests/test_content_discovery.py
diff --git a/org_fedora_oscap/content_discovery.py b/org_fedora_oscap/content_discovery.py
index ccfe6c8..9ef144e 100644
--- a/org_fedora_oscap/content_discovery.py
+++ b/org_fedora_oscap/content_discovery.py
@@ -12,6 +12,7 @@
from org_fedora_oscap import data_fetch, utils
from org_fedora_oscap import common
from org_fedora_oscap import content_handling
+from org_fedora_oscap.content_handling import CONTENT_TYPES
from org_fedora_oscap import rule_handling
from org_fedora_oscap.common import _
@@ -191,6 +192,38 @@ def _verify_fingerprint(self, dest_filename, fingerprint=""):
raise content_handling.ContentCheckError(msg)
log.info(f"Integrity check passed using {hash_obj.name} hash")
+ def filter_discovered_content(self, labelled_files):
+ expected_path = self._addon_data.content_path
+ categories = (CONTENT_TYPES["DATASTREAM"], CONTENT_TYPES["XCCDF_CHECKLIST"])
+ if expected_path:
+ labelled_files = self.reduce_files(labelled_files, expected_path, categories)
+
+ expected_path = self._addon_data.tailoring_path
+ categories = (CONTENT_TYPES["TAILORING"], )
+ if expected_path:
+ labelled_files = self.reduce_files(labelled_files, expected_path, categories)
+
+ expected_path = self._addon_data.cpe_path
+ categories = (CONTENT_TYPES["CPE_DICT"], )
+ if expected_path:
+ labelled_files = self.reduce_files(labelled_files, expected_path, categories)
+
+ return labelled_files
+
+ def reduce_files(self, labelled_files, expected_path, categories):
+ reduced_files = dict()
+ if expected_path not in labelled_files:
+ msg = (
+ f"Expected a file {expected_path} to be part of the supplied content, "
+ f"but it was not the case, got only {list(labelled_files.keys())}"
+ )
+ raise RuntimeError(msg)
+ for path, label in labelled_files.items():
+ if label in categories and path != expected_path:
+ continue
+ reduced_files[path] = label
+ return reduced_files
+
def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_filename):
if wait_for:
log.info(f"OSCAP Addon: Waiting for thread {wait_for}")
@@ -210,6 +243,8 @@ def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_file
structured_content.add_content_archive(dest_filename)
labelled_files = content_handling.identify_files(fpaths)
+ labelled_files = self.filter_discovered_content(labelled_files)
+
for fname, label in labelled_files.items():
structured_content.add_file(fname, label)
diff --git a/tests/test_content_discovery.py b/tests/test_content_discovery.py
new file mode 100644
index 0000000..5463c9a
--- /dev/null
+++ b/tests/test_content_discovery.py
@@ -0,0 +1,48 @@
+import pytest
+
+import org_fedora_oscap.content_discovery as tested_module
+
+
+@pytest.fixture
+def labelled_files():
+ return {
+ "dir/datastream": "D",
+ "dir/datastream2": "D",
+ "dir/dir/datastream3": "D",
+ "dir/dir/datastream3": "D",
+ "dir/XCCDF": "X",
+ "XCCDF2": "X",
+ "cpe": "C",
+ "t1": "T",
+ "dir3/t2": "T",
+ }
+
+
+def test_reduce(labelled_files):
+ bringer = tested_module.ContentBringer(None)
+
+ d_count = 0
+ x_count = 0
+ for l in labelled_files.values():
+ if l == "D":
+ d_count += 1
+ elif l == "X":
+ x_count += 1
+
+ reduced = bringer.reduce_files(labelled_files, "dir/datastream", ["D"])
+ assert len(reduced) == len(labelled_files) - d_count + 1
+ assert "dir/datastream" in reduced
+
+ reduced = bringer.reduce_files(labelled_files, "dir/datastream", ["D", "X"])
+ assert len(reduced) == len(labelled_files) - d_count - x_count + 1
+ assert "dir/datastream" in reduced
+
+ reduced = bringer.reduce_files(labelled_files, "dir/XCCDF", ["D", "X"])
+ assert len(reduced) == len(labelled_files) - d_count - x_count + 1
+ assert "dir/XCCDF" in reduced
+
+ with pytest.raises(RuntimeError, match="dir/datastream4"):
+ bringer.reduce_files(labelled_files, "dir/datastream4", ["D"])
+
+ reduced = bringer.reduce_files(labelled_files, "cpe", ["C"])
+ assert reduced == labelled_files
From 2a536a8ec4cdf20e4f19e8175898b7ace3fc7ca4 Mon Sep 17 00:00:00 2001
From: Matej Tyc <matyc@redhat.com>
Date: Wed, 12 Oct 2022 11:40:11 +0200
Subject: [PATCH 2/5] Handle changes in content identification
The code is able to handle changes in the way how oscap identifies
content much more gracefully.
---
org_fedora_oscap/content_discovery.py | 13 +++++++++----
org_fedora_oscap/content_handling.py | 5 +++++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/org_fedora_oscap/content_discovery.py b/org_fedora_oscap/content_discovery.py
index 9ef144e..9ed643b 100644
--- a/org_fedora_oscap/content_discovery.py
+++ b/org_fedora_oscap/content_discovery.py
@@ -2,6 +2,7 @@
import logging
import pathlib
import shutil
+import os
from glob import glob
from typing import List
@@ -242,11 +243,15 @@ def _finish_actual_fetch(self, wait_for, fingerprint, report_callback, dest_file
if content_type in ("archive", "rpm"):
structured_content.add_content_archive(dest_filename)
- labelled_files = content_handling.identify_files(fpaths)
- labelled_files = self.filter_discovered_content(labelled_files)
+ labelled_filenames = content_handling.identify_files(fpaths)
+ labelled_relative_filenames = {
+ os.path.relpath(path, self.CONTENT_DOWNLOAD_LOCATION): label
+ for path, label in labelled_filenames.items()}
+ labelled_relative_filenames = self.filter_discovered_content(labelled_relative_filenames)
- for fname, label in labelled_files.items():
- structured_content.add_file(fname, label)
+ for rel_fname, label in labelled_relative_filenames.items():
+ fname = self.CONTENT_DOWNLOAD_LOCATION / rel_fname
+ structured_content.add_file(str(fname), label)
if fingerprint and dest_filename:
structured_content.record_verification(dest_filename)
diff --git a/org_fedora_oscap/content_handling.py b/org_fedora_oscap/content_handling.py
index 65d5a28..3e2ecae 100644
--- a/org_fedora_oscap/content_handling.py
+++ b/org_fedora_oscap/content_handling.py
@@ -122,6 +122,11 @@ def get_doc_type(file_path):
if line.startswith("Document type:"):
_prefix, _sep, type_info = line.partition(":")
content_type = type_info.strip()
+ if content_type not in CONTENT_TYPES.values():
+ log.info(
+ f"File {file_path} labelled by oscap as {content_type}, "
+ "which is an unexpected type.")
+ content_type = f"unknown - {content_type}"
break
except OSError:
# 'oscap info' exitted with a non-zero exit code -> unknown doc
From 17f80b71d17ce5a2bdbed87730133cdabec2e22b Mon Sep 17 00:00:00 2001
From: Matej Tyc <matyc@redhat.com>
Date: Wed, 12 Oct 2022 11:38:51 +0200
Subject: [PATCH 3/5] Remove unused code
The function is not referenced anywhere in the project
---
org_fedora_oscap/content_handling.py | 40 ----------------------------
1 file changed, 40 deletions(-)
diff --git a/org_fedora_oscap/content_handling.py b/org_fedora_oscap/content_handling.py
index 3e2ecae..5096bab 100644
--- a/org_fedora_oscap/content_handling.py
+++ b/org_fedora_oscap/content_handling.py
@@ -141,43 +141,3 @@ def get_doc_type(file_path):
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
- THE FIRST USABLE CONTENT FILE OF A PARTICULAR TYPE AND JUST PREFERS DATA
- STREAMS OVER STANDALONE BENCHMARKS.
-
- :param fpaths: a list of file paths to search for content files in
- :type fpaths: [str]
- :return: ContentFiles instance containing the file names of the XCCDF file,
- CPE dictionary and tailoring file or "" in place of those items
- if not found
- :rtype: ContentFiles
-
- """
- xccdf_file = ""
- cpe_file = ""
- tailoring_file = ""
- found_ds = False
-
- for fpath in fpaths:
- doc_type = get_doc_type(fpath)
- if not doc_type:
- continue
-
- # prefer DS over standalone XCCDF
- if doc_type == "Source Data Stream" and (not xccdf_file or not found_ds):
- xccdf_file = fpath
- found_ds = True
- elif doc_type == "XCCDF Checklist" and not xccdf_file:
- xccdf_file = fpath
- elif doc_type == "CPE Dictionary" and not cpe_file:
- cpe_file = fpath
- elif doc_type == "XCCDF Tailoring" and not tailoring_file:
- tailoring_file = fpath
-
- # TODO: raise exception if no xccdf_file is found?
- files = ContentFiles(xccdf_file, cpe_file, tailoring_file)
- return files
From 3aff547e2689a1ede4236c9166b11c99f272e3f7 Mon Sep 17 00:00:00 2001
From: Matej Tyc <matyc@redhat.com>
Date: Thu, 13 Oct 2022 14:11:25 +0200
Subject: [PATCH 4/5] Dont use tailoring if it is not expected
Take tailorings into account only if it is specified in the kickstart.
Compulsive usage of tailoring may be unwanted.
---
org_fedora_oscap/content_discovery.py | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/org_fedora_oscap/content_discovery.py b/org_fedora_oscap/content_discovery.py
index 9ed643b..4235af7 100644
--- a/org_fedora_oscap/content_discovery.py
+++ b/org_fedora_oscap/content_discovery.py
@@ -193,16 +193,25 @@ def _verify_fingerprint(self, dest_filename, fingerprint=""):
raise content_handling.ContentCheckError(msg)
log.info(f"Integrity check passed using {hash_obj.name} hash")
+ def allow_one_expected_tailoring_or_no_tailoring(self, labelled_files):
+ expected_tailoring = self._addon_data.tailoring_path
+ tailoring_label = CONTENT_TYPES["TAILORING"]
+ if expected_tailoring:
+ labelled_files = self.reduce_files(labelled_files, expected_tailoring, [tailoring_label])
+ else:
+ labelled_files = {
+ path: label for path, label in labelled_files.items()
+ if label != tailoring_label
+ }
+ return labelled_files
+
def filter_discovered_content(self, labelled_files):
expected_path = self._addon_data.content_path
categories = (CONTENT_TYPES["DATASTREAM"], CONTENT_TYPES["XCCDF_CHECKLIST"])
if expected_path:
labelled_files = self.reduce_files(labelled_files, expected_path, categories)
- expected_path = self._addon_data.tailoring_path
- categories = (CONTENT_TYPES["TAILORING"], )
- if expected_path:
- labelled_files = self.reduce_files(labelled_files, expected_path, categories)
+ labelled_files = self.allow_one_expected_tailoring_or_no_tailoring(labelled_files)
expected_path = self._addon_data.cpe_path
categories = (CONTENT_TYPES["CPE_DICT"], )
From 56d8e497e0a4c394784b1c950bd1a148a6dc42ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= <matyc@redhat.com>
Date: Thu, 10 Nov 2022 12:46:46 +0100
Subject: [PATCH 5/5] Make the content RPM installation robust
If a package manager fails to install the package,
use the rpm command directly and skip deps.
---
org_fedora_oscap/service/installation.py | 48 +++++++++++++++++-------
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/org_fedora_oscap/service/installation.py b/org_fedora_oscap/service/installation.py
index 255b992..f667479 100644
--- a/org_fedora_oscap/service/installation.py
+++ b/org_fedora_oscap/service/installation.py
@@ -18,6 +18,7 @@
import logging
import os
import shutil
+import io
from pyanaconda.core import util
from pyanaconda.modules.common.task import Task
@@ -198,21 +199,11 @@ def run(self):
elif self._policy_data.content_type == "datastream":
shutil.copy2(self._content_path, target_content_dir)
elif self._policy_data.content_type == "rpm":
- # copy the RPM to the target system
- shutil.copy2(self._file_path, target_content_dir)
+ try:
+ self._copy_rpm_to_target_and_install(target_content_dir)
- # get the path of the RPM
- content_name = common.get_content_name(self._policy_data)
- package_path = utils.join_paths(self._target_directory, content_name)
-
- # and install it with yum
- ret = util.execInSysroot(
- "yum", ["-y", "--nogpg", "install", package_path]
- )
-
- if ret != 0:
- msg = _(f"Failed to install content RPM to the target system.")
- terminate(msg)
+ except Exception as exc:
+ terminate(str(exc))
return
else:
pattern = utils.join_paths(common.INSTALLATION_CONTENT_DIR, "*")
@@ -221,6 +212,35 @@ def run(self):
if os.path.exists(self._tailoring_path):
shutil.copy2(self._tailoring_path, target_content_dir)
+ def _attempt_rpm_installation(self, chroot_package_path):
+ log.info("OSCAP addon: Installing the security content RPM to the installed system.")
+ stdout = io.StringIO()
+ ret = util.execWithRedirect(
+ "dnf", ["-y", "--nogpg", "install", chroot_package_path],
+ stdout=stdout, root=self._sysroot)
+ stdout.seek(0)
+ if ret != 0:
+ log.error(
+ "OSCAP addon: Error installing security content RPM using yum: {0}",
+ stdout.read())
+
+ stdout = io.StringIO()
+ ret = util.execWithRedirect(
+ "rpm", ["--install", "--nodeps", chroot_package_path],
+ stdout=stdout, root=self._sysroot)
+ if ret != 0:
+ log.error(
+ "OSCAP addon: Error installing security content RPM using rpm: {0}",
+ stdout.read())
+ msg = _(f"Failed to install content RPM to the target system.")
+ raise RuntimeError(msg)
+
+ def _copy_rpm_to_target_and_install(self, target_content_dir):
+ shutil.copy2(self._file_path, target_content_dir)
+ content_name = common.get_content_name(self._policy_data)
+ chroot_package_path = utils.join_paths(self._target_directory, content_name)
+ self._attempt_rpm_installation(chroot_package_path)
+
class RemediateSystemTask(Task):
"""The installation task for running the remediation."""

View File

@ -0,0 +1,74 @@
From 99fc53d3691b24c6724c1cf3e7281c181b31cf45 Mon Sep 17 00:00:00 2001
From: Matej Tyc <matyc@redhat.com>
Date: Tue, 11 Oct 2022 17:07:28 +0200
Subject: [PATCH 1/2] Remove redundant message
The send_ready already performs what the removed call
could aim to accomplish.
---
org_fedora_oscap/gui/spokes/oscap.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py
index 6d0aa5c..37b9681 100644
--- a/org_fedora_oscap/gui/spokes/oscap.py
+++ b/org_fedora_oscap/gui/spokes/oscap.py
@@ -151,7 +151,6 @@ def decorated(self, *args, **kwargs):
self._ready = True
# pylint: disable-msg=E1101
hubQ.send_ready(self.__class__.__name__)
- hubQ.send_message(self.__class__.__name__, self.status)
return ret
From 24787f02e80162129256dc57dc3d491f00080370 Mon Sep 17 00:00:00 2001
From: Matej Tyc <matyc@redhat.com>
Date: Thu, 13 Oct 2022 17:19:17 +0200
Subject: [PATCH 2/2] Increase robustness of fetching state detection
It is not completely practical to rely on locks alone,
and we can elliminate some corner cases by looking
whether well-known UI threads exist.
---
org_fedora_oscap/gui/spokes/oscap.py | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/org_fedora_oscap/gui/spokes/oscap.py b/org_fedora_oscap/gui/spokes/oscap.py
index 37b9681..97c4553 100644
--- a/org_fedora_oscap/gui/spokes/oscap.py
+++ b/org_fedora_oscap/gui/spokes/oscap.py
@@ -389,11 +389,14 @@ def _render_selected(self, column, renderer, model, itr, user_data=None):
else:
renderer.set_property("stock-id", None)
+ def _still_fetching(self):
+ return self._fetching or threadMgr.get('OSCAPguiWaitForDataFetchThread')
+
def _fetch_data_and_initialize(self):
"""Fetch data from a specified URL and initialize everything."""
with self._fetch_flag_lock:
- if self._fetching:
+ if self._still_fetching():
# prevent multiple fetches running simultaneously
return
self._fetching = True
@@ -940,7 +943,7 @@ def _refresh_ui(self):
# hide the progress box, no progress now
with self._fetch_flag_lock:
- if not self._fetching:
+ if not self._still_fetching():
really_hide(self._progress_box)
self._content_url_entry.set_sensitive(True)
@@ -1165,7 +1168,7 @@ def on_fetch_button_clicked(self, *args):
"""Handler for the Fetch button"""
with self._fetch_flag_lock:
- if self._fetching:
+ if self._still_fetching():
# some other fetching/pre-processing running, give up
log.warn(
"OSCAP Addon: "

View File

@ -10,7 +10,7 @@
Name: oscap-anaconda-addon Name: oscap-anaconda-addon
Version: 2.0.0 Version: 2.0.0
Release: 12%{?dist} Release: 13%{?dist}
Summary: Anaconda addon integrating OpenSCAP to the installation process Summary: Anaconda addon integrating OpenSCAP to the installation process
License: GPLv2+ License: GPLv2+
@ -30,6 +30,8 @@ Patch7: oscap-anaconda-addon-1.2.2-dbus_show_integration-PR_182.patch
Patch8: oscap-anaconda-addon-2.1.0-unified_help-PR_192.patch Patch8: oscap-anaconda-addon-2.1.0-unified_help-PR_192.patch
Patch9: oscap-anaconda-addon-2.0.1-absent_appstream-PR_185.patch Patch9: oscap-anaconda-addon-2.0.1-absent_appstream-PR_185.patch
Patch10: oscap-anaconda-addon-2.0.1-fix_strings-PR_207.patch Patch10: oscap-anaconda-addon-2.0.1-fix_strings-PR_207.patch
Patch11: oscap-anaconda-addon-2.1.0-clicking_fix-PR_223.patch
Patch12: oscap-anaconda-addon-2.1.0-archive_handling-PR_224.patch
BuildArch: noarch BuildArch: noarch
BuildRequires: make BuildRequires: make
@ -69,6 +71,12 @@ make install DESTDIR=%{buildroot}
%doc COPYING ChangeLog README.md %doc COPYING ChangeLog README.md
%changelog %changelog
* Fri Nov 11 2022 Matej Tyc <matyc@redhat.com> - 2.0.0-13
- Fix problems with handling multi-datastream archives
Resolves: rhbz#2129846
- Fix a crash when compulsively clicking in the GUI
Resolves: rhbz#2127502
* Fri Jun 10 2022 Matej Tyc <matyc@redhat.com> - 2.0.0-12 * Fri Jun 10 2022 Matej Tyc <matyc@redhat.com> - 2.0.0-12
- Remove the firstboot remediation feature completely. - Remove the firstboot remediation feature completely.
We can't have it, while maintaining the standard UX. We can't have it, while maintaining the standard UX.