diff -up sos-3.2/sos/policies/__init__.py.orig sos-3.2/sos/policies/__init__.py --- sos-3.2/sos/policies/__init__.py.orig 2015-12-16 13:06:08.730706383 +0000 +++ sos-3.2/sos/policies/__init__.py 2015-12-16 13:07:09.456189257 +0000 @@ -10,11 +10,9 @@ from os import environ from sos.utilities import (ImporterHelper, import_module, - get_hash_name, shell_out) from sos.plugins import IndependentPlugin from sos import _sos as _ -import hashlib from textwrap import fill from six import print_ from six.moves import input @@ -284,43 +282,29 @@ No changes will be made to system config considered to be a superuser""" return (os.getuid() == 0) - def _create_checksum(self, final_filename=None): - if not final_filename: - return False - - archive_fp = open(final_filename, 'rb') - digest = hashlib.new(get_hash_name()) - digest.update(archive_fp.read()) - archive_fp.close() - return digest.hexdigest() - - def get_preferred_hash_algorithm(self): + def get_preferred_hash_name(self): """Returns the string name of the hashlib-supported checksum algorithm to use""" return "md5" - def display_results(self, final_filename=None, build=False): + def display_results(self, archive, directory, checksum): + # Display results is called from the tail of SoSReport.final_work() + # + # Logging is already shutdown and all terminal output must use the + # print() call. # make sure a report exists - if not final_filename: + if not archive and not directory: return False self._print() - if not build: - # store checksum into file - fp = open(final_filename + "." + get_hash_name(), "w") - checksum = self._create_checksum(final_filename) - if checksum: - fp.write(checksum + "\n") - fp.close() - + if archive: self._print(_("Your sosreport has been generated and saved " - "in:\n %s") % final_filename) + "in:\n %s") % archive) else: - checksum = None self._print(_("sosreport build tree is located at : %s" % - final_filename)) + directory)) self._print() if checksum: @@ -371,20 +355,28 @@ class LinuxPolicy(Policy): vendor = "None" PATH = "/bin:/sbin:/usr/bin:/usr/sbin" + _preferred_hash_name = None + def __init__(self, sysroot=None): super(LinuxPolicy, self).__init__(sysroot=sysroot) - def get_preferred_hash_algorithm(self): + def get_preferred_hash_name(self): + + if self._preferred_hash_name: + return self._preferred_hash_name + checksum = "md5" try: fp = open("/proc/sys/crypto/fips_enabled", "r") except: + self._preferred_hash_name = checksum return checksum fips_enabled = fp.read() if fips_enabled.find("1") >= 0: checksum = "sha256" fp.close() + self._preferred_hash_name = checksum return checksum def default_runlevel(self): diff -up sos-3.2/sos/sosreport.py.orig sos-3.2/sos/sosreport.py --- sos-3.2/sos/sosreport.py.orig 2015-12-16 13:06:08.748706527 +0000 +++ sos-3.2/sos/sosreport.py 2015-12-16 13:07:09.458189272 +0000 @@ -32,7 +32,9 @@ from sos.utilities import ImporterHelper from stat import ST_UID, ST_GID, ST_MODE, ST_CTIME, ST_ATIME, ST_MTIME, S_IMODE from time import strftime, localtime from collections import deque +from shutil import rmtree import tempfile +import hashlib from sos import _sos as _ from sos import __version__ @@ -658,6 +660,7 @@ class SoSReport(object): self.tempfile_util = None self._args = args self.sysroot = "/" + self.sys_tmp = None try: import signal @@ -676,15 +679,22 @@ class SoSReport(object): self._is_root = self.policy.is_root() - self.tmpdir = os.path.abspath( - self.policy.get_tmp_dir(self.opts.tmp_dir)) - if not os.path.isdir(self.tmpdir) \ - or not os.access(self.tmpdir, os.W_OK): + # system temporary directory to use + tmp = os.path.abspath(self.policy.get_tmp_dir(self.opts.tmp_dir)) + + if not os.path.isdir(tmp) \ + or not os.access(tmp, os.W_OK): # write directly to stderr as logging is not initialised yet - sys.stderr.write("temporary directory %s " % self.tmpdir + sys.stderr.write("temporary directory %s " % tmp + "does not exist or is not writable\n") self._exit(1) + + self.sys_tmp = tmp + + # our (private) temporary directory + self.tmpdir = tempfile.mkdtemp(prefix="sos.", dir=self.sys_tmp) self.tempfile_util = TempFileUtil(self.tmpdir) + self._set_directories() self._setup_logging() @@ -1406,25 +1416,47 @@ class SoSReport(object): raise self._log_plugin_exception(plugname, "postproc") + def _create_checksum(self, archive, hash_name): + if not archive: + return False + + archive_fp = open(archive, 'rb') + digest = hashlib.new(hash_name) + digest.update(archive_fp.read()) + archive_fp.close() + return digest.hexdigest() + + def _write_checksum(self, archive, hash_name, checksum): + # store checksum into file + fp = open(archive + "." + hash_name, "w") + if checksum: + fp.write(checksum + "\n") + fp.close() + def final_work(self): - # this must come before archive creation to ensure that log + # This must come before archive creation to ensure that log # files are closed and cleaned up at exit. + # + # All subsequent terminal output must use print(). self._finish_logging() - # package up the results for the support organization + + archive = None # archive path + directory = None # report directory path (--build) + + # package up and compress the results if not self.opts.build: old_umask = os.umask(0o077) if not self.opts.quiet: print(_("Creating compressed archive...")) # compression could fail for a number of reasons try: - final_filename = self.archive.finalize( + archive = self.archive.finalize( self.opts.compression_type) except (OSError, IOError) as e: if e.errno in fatal_fs_errors: - self.ui_log.error("") - self.ui_log.error(" %s while finalizing archive" - % e.strerror) - self.ui_log.error("") + print("") + print(_(" %s while finalizing archive" % e.strerror)) + print("") self._exit(1) except: if self.opts.debug: @@ -1434,8 +1466,58 @@ class SoSReport(object): finally: os.umask(old_umask) else: - final_filename = self.archive.get_archive_path() - self.policy.display_results(final_filename, build=self.opts.build) + # move the archive root out of the private tmp directory. + directory = self.archive.get_archive_path() + dir_name = os.path.basename(directory) + try: + os.rename(directory, os.path.join(self.sys_tmp, dir_name)) + except (OSError, IOError): + print(_("Error moving directory: %s" % directory)) + return False + + checksum = None + + if not self.opts.build: + # compute and store the archive checksum + hash_name = self.policy.get_preferred_hash_name() + checksum = self._create_checksum(archive, hash_name) + self._write_checksum(archive, hash_name, checksum) + + # output filename is in the private tmpdir - move it to the + # containing directory. + final_name = os.path.join(self.sys_tmp, os.path.basename(archive)) + + archive_hash = archive + "." + hash_name + final_hash = final_name + "." + hash_name + + # move the archive and checksum file + try: + os.rename(archive, final_name) + archive = final_name + except (OSError, IOError): + print(_("Error moving archive file: %s" % archive)) + return False + + # There is a race in the creation of the final checksum file: + # since the archive has already been published and the checksum + # file name is predictable once the archive name is known a + # malicious user could attempt to create a symbolic link in order + # to misdirect writes to a file of the attacker's choosing. + # + # To mitigate this we write the checksum inside the private tmp + # directory and use an atomic rename that is guaranteed to either + # succeed or fail: at worst the move will fail and be reported to + # the user. The correct checksum value is still written to the + # terminal and nothing is written to a location under the control + # of the user creating the link. + try: + os.rename(archive_hash, final_hash) + except (OSError, IOError): + print(_("Error moving checksum file: %s" % archive_hash)) + return False + + self.policy.display_results(archive, directory, checksum) + self.tempfile_util.clean() return True @@ -1490,8 +1572,10 @@ class SoSReport(object): self.archive.cleanup() if self.tempfile_util: self.tempfile_util.clean() + if self.tmpdir: + rmtree(self.tmpdir) except: - pass + raise return False diff -up sos-3.2/sos/utilities.py.orig sos-3.2/sos/utilities.py --- sos-3.2/sos/utilities.py.orig 2015-12-16 13:06:08.732706400 +0000 +++ sos-3.2/sos/utilities.py 2015-12-16 13:07:09.455189249 +0000 @@ -53,18 +53,6 @@ def fileobj(path_or_file, mode='r'): return closing(path_or_file) -def get_hash_name(): - """Returns the algorithm used when computing a hash""" - import sos.policies - policy = sos.policies.load() - try: - name = policy.get_preferred_hash_algorithm() - hashlib.new(name) - return name - except: - return 'sha256' - - def convert_bytes(bytes_, K=1 << 10, M=1 << 20, G=1 << 30, T=1 << 40): """Converts a number of bytes to a shorter, more human friendly format""" fn = float(bytes_) diff -up sos-3.2/tests/utilities_tests.py.orig sos-3.2/tests/utilities_tests.py --- sos-3.2/tests/utilities_tests.py.orig 2015-12-16 13:06:57.512094280 +0000 +++ sos-3.2/tests/utilities_tests.py 2015-12-16 13:07:09.455189249 +0000 @@ -5,7 +5,7 @@ import unittest import six from six import StringIO -from sos.utilities import grep, get_hash_name, is_executable, sos_get_command_output, find, tail, shell_out +from sos.utilities import grep, is_executable, sos_get_command_output, find, tail, shell_out import sos TEST_DIR = os.path.dirname(__file__)