From 8eacfad08b3c27aa9510f2c3337356581bd9bebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Mon, 3 Jan 2022 17:31:49 +0100 Subject: [PATCH 1/3] Add oscap sanity check before attempting remediation If something is obviously wrong with the scanner, then don't attempt to remediate and try to show relevant information in a dialog window. --- org_fedora_oscap/common.py | 39 ++++++++++++++++++++++++++++-------- org_fedora_oscap/ks/oscap.py | 11 ++++++++++ tests/test_common.py | 8 ++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/org_fedora_oscap/common.py b/org_fedora_oscap/common.py index 884bbc8..05829ce 100644 --- a/org_fedora_oscap/common.py +++ b/org_fedora_oscap/common.py @@ -139,7 +139,8 @@ def execute(self, ** kwargs): proc = subprocess.Popen(self.args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, ** kwargs) except OSError as oserr: - msg = "Failed to run the oscap tool: %s" % oserr + msg = ("Failed to execute command '{command_string}': {oserr}" + .format(command_string=command_string, oserr=oserr)) raise OSCAPaddonError(msg) (stdout, stderr) = proc.communicate() @@ -215,6 +216,34 @@ def _run_oscap_gen_fix(profile, fpath, template, ds_id="", xccdf_id="", return proc.stdout +def do_chroot(chroot): + """Helper function doing the chroot if requested.""" + if chroot and chroot != "/": + os.chroot(chroot) + os.chdir("/") + + +def assert_scanner_works(chroot, executable="oscap"): + args = [executable, "--version"] + command = " ".join(args) + + try: + proc = subprocess.Popen( + args, preexec_fn=lambda: do_chroot(chroot), + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + (stdout, stderr) = proc.communicate() + stderr = stderr.decode(errors="replace") + except OSError as exc: + msg = _(f"Basic invocation '{command}' fails: {str(exc)}") + raise OSCAPaddonError(msg) + if proc.returncode != 0: + msg = _( + f"Basic scanner invocation '{command}' exited " + "with non-zero error code {proc.returncode}: {stderr}") + raise OSCAPaddonError(msg) + return True + + def run_oscap_remediate(profile, fpath, ds_id="", xccdf_id="", tailoring="", chroot=""): """ @@ -244,12 +273,6 @@ def run_oscap_remediate(profile, fpath, ds_id="", xccdf_id="", tailoring="", if not profile: return "" - def do_chroot(): - """Helper function doing the chroot if requested.""" - if chroot and chroot != "/": - os.chroot(chroot) - os.chdir("/") - # make sure the directory for the results exists results_dir = os.path.dirname(RESULTS_PATH) if chroot: @@ -274,7 +297,7 @@ def do_chroot(): args.append(fpath) proc = SubprocessLauncher(args) - proc.execute(preexec_fn=do_chroot) + proc.execute(preexec_fn=lambda: do_chroot(chroot)) proc.log_messages() if proc.returncode not in (0, 2): diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index 65d74cf..da1600f 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -488,6 +488,17 @@ def execute(self, storage, ksdata, users, payload): # selected return + try: + common.assert_scanner_works( + chroot=conf.target.system_root, executable="oscap") + except Exception as exc: + msg_lines = [_( + "The 'oscap' scanner doesn't work in the installed system: {error}" + .format(error=str(exc)))] + msg_lines.append(_("As a result, the installed system can't be hardened.")) + self._terminate("\n".join(msg_lines)) + return + target_content_dir = utils.join_paths(conf.target.system_root, common.TARGET_CONTENT_DIR) utils.ensure_dir_exists(target_content_dir) diff --git a/tests/test_common.py b/tests/test_common.py index 9f7a16a..4f25379 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -77,6 +77,14 @@ def _run_oscap(mock_subprocess, additional_args): return expected_args, kwargs +def test_oscap_works(): + assert common.assert_scanner_works(chroot="/") + with pytest.raises(common.OSCAPaddonError, match="No such file"): + common.assert_scanner_works(chroot="/", executable="i_dont_exist") + with pytest.raises(common.OSCAPaddonError, match="non-zero"): + common.assert_scanner_works(chroot="/", executable="false") + + def test_run_oscap_remediate_profile_only(mock_subprocess, monkeypatch): return run_oscap_remediate_profile( mock_subprocess, monkeypatch, From b54cf2bddba56e5b776fb60514a5e29d47c74cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Mon, 3 Jan 2022 17:42:31 +0100 Subject: [PATCH 2/3] Don't raise exceptions in execute() Those result in tracebacks during the installation, while a dialog window presents a more useful form of user interaction. --- org_fedora_oscap/ks/oscap.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index da1600f..d3f0dbe 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -513,8 +513,9 @@ def execute(self, storage, ksdata, users, payload): ret = util.execInSysroot("yum", ["-y", "--nogpg", "install", self.raw_postinst_content_path]) if ret != 0: - raise common.ExtractionError("Failed to install content " - "RPM to the target system") + msg = _(f"Failed to install content RPM to the target system.") + self._terminate(msg) + return elif self.content_type == "scap-security-guide": # nothing needed pass @@ -525,10 +526,15 @@ def execute(self, storage, ksdata, users, payload): if os.path.exists(self.preinst_tailoring_path): shutil.copy2(self.preinst_tailoring_path, target_content_dir) - common.run_oscap_remediate(self.profile_id, self.postinst_content_path, - self.datastream_id, self.xccdf_id, - self.postinst_tailoring_path, - chroot=conf.target.system_root) + try: + common.run_oscap_remediate(self.profile_id, self.postinst_content_path, + self.datastream_id, self.xccdf_id, + self.postinst_tailoring_path, + chroot=conf.target.system_root) + except Exception as exc: + msg = _(f"Something went wrong during the final hardening: {str(exc)}.") + self._terminate(msg) + return def clear_all(self): """Clear all the stored values.""" From 00d770d1b7f8e1f0734e93da227f1c3e445033c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Mon, 3 Jan 2022 17:44:12 +0100 Subject: [PATCH 3/3] Change the error feedback based on the installation mode The original approach was confusing, because non-interactive installs run without any user input, and the message assumed that the user is able to answer installer's questions. --- org_fedora_oscap/ks/oscap.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/org_fedora_oscap/ks/oscap.py b/org_fedora_oscap/ks/oscap.py index d3f0dbe..ef34448 100644 --- a/org_fedora_oscap/ks/oscap.py +++ b/org_fedora_oscap/ks/oscap.py @@ -372,13 +372,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 + message += "\n" + _("Aborting the installation.") raise errors.CmdlineError(message) + message += "\n" + _("The installation should be aborted.") + message += " " + _("Do you wish to continue anyway?") answ = errors.errorHandler.ui.showYesNoQuestion(message) if answ == errors.ERROR_CONTINUE: # prevent any futher actions here by switching to the dry