From 624d49a0e8305e6613a9637437f7cdfb8a7852e9 Mon Sep 17 00:00:00 2001 From: Derek Higgins Date: Fri, 1 Feb 2013 06:57:55 -0500 Subject: [PATCH] Securely create temp directorys / files The temp directory packstack was using was being created in an insecure manor with world readable permissions. This commit ensures temp directories are created securly on both the local and remote hosts o Create var directory with tempfile.mkdtemp o remove other places where var directory was created o change permissions of all files that do (or may) contain sensitive data to 600 o No longer append data to mainifest file, it is now created and writen out in once o Attempts to remove data on remote hosts after the packstach run CVE-2013-0261 https://bugzilla.redhat.com/show_bug.cgi?id=908101 Change-Id: Ie7105207d3da128d630628c1df037ffafc94beb8 --- packstack/installer/basedefs.py | 11 ++++++++- packstack/installer/output_messages.py | 2 ++ packstack/installer/run_setup.py | 32 +++++++++++++++++++++++--- packstack/modules/ospluginutils.py | 22 ++++++++++++------ packstack/plugins/puppet_950.py | 11 ++++++--- packstack/plugins/serverprep_901.py | 7 +++++- 6 files changed, 70 insertions(+), 15 deletions(-) diff --git a/packstack/installer/basedefs.py b/packstack/installer/basedefs.py index f29e6eb9e..45454e1c7 100644 --- a/packstack/installer/basedefs.py +++ b/packstack/installer/basedefs.py @@ -4,12 +4,21 @@ provides all the predefined variables for engine-setup import os import sys import datetime +import tempfile APP_NAME = "Installer" FILE_YUM_VERSION_LOCK = "/etc/yum/pluginconf.d/versionlock.list" -VAR_DIR = os.path.join("/var/tmp/packstack", datetime.datetime.now().strftime('%Y%m%d-%H%M')) +_tmpdirprefix = datetime.datetime.now().strftime('%Y%m%d-%H%M%S-') + +PACKSTACK_VAR_DIR = "/var/tmp/packstack" +try: + os.mkdir(PACKSTACK_VAR_DIR, 0700) +except: + pass + +VAR_DIR = tempfile.mkdtemp(prefix = _tmpdirprefix, dir = PACKSTACK_VAR_DIR) DIR_LOG = VAR_DIR PUPPET_MANIFEST_DIR = os.path.join(VAR_DIR, "manifests") diff --git a/packstack/installer/output_messages.py b/packstack/installer/output_messages.py index c9136ed8d..1f791d996 100644 --- a/packstack/installer/output_messages.py +++ b/packstack/installer/output_messages.py @@ -51,6 +51,7 @@ INFO_STRING_LEN_LESS_THAN_MIN="String length is less than the minimum allowed: % INFO_STRING_EXCEEDS_MAX_LENGTH="String length exceeds the maximum length allowed: %s" INFO_STRING_CONTAINS_ILLEGAL_CHARS="String contains illegal characters" INFO_CINDER_VOLUMES_EXISTS="Did not create a cinder volume group, one already existed" +INFO_REMOVE_REMOTE_VAR="Removing %s on %s (if it is a remote host)" WARN_WEAK_PASS="Warning: Weak Password." @@ -80,6 +81,7 @@ ERR_RC_CODE="Return Code is not zero" ERR_FAILURE="General failure" ERR_NO_ANSWER_FILE="Error: Could not find file %s" ERR_ONLY_1_FLAG="Error: The %s flag is mutually exclusive to all other command line options" +ERR_REMOVE_REMOTE_VAR="Error: Failed to remove directory %s on %s, it contains sensitive data and should be removed" # INFO_KEYSTONERC="To use the command line tools source the file /root/keystonerc_admin created on %s" diff --git a/packstack/installer/run_setup.py b/packstack/installer/run_setup.py index cc39cede6..47581d2dc 100644 --- a/packstack/installer/run_setup.py +++ b/packstack/installer/run_setup.py @@ -19,10 +19,10 @@ import engine_validators as validate import output_messages from .exceptions import FlagValidationError, ParamValidationError +from packstack.modules.ospluginutils import gethostlist from setup_controller import Controller controller = Controller() -logFile = os.path.join(basedefs.DIR_LOG, basedefs.FILE_INSTALLER_LOG) commandLineValues = {} # List to hold all values to be masked in logging (i.e. passwords and sensitive data) @@ -37,8 +37,10 @@ def initLogging (debug): #in order to use UTC date for the log file, send True to getCurrentDateTime(True) logFilename = "openstack-setup.log" logFile = os.path.join(basedefs.DIR_LOG, logFilename) - if not os.path.isdir(os.path.dirname(logFile)): - os.makedirs(os.path.dirname(logFile)) + + # Create the log file with specific permissions, puppet has a habbit of putting + # passwords in logs + os.close(os.open(logFile, os.O_CREAT | os.O_EXCL, 0600)) hdlr = logging.FileHandler (filename=logFile, mode='w') if (debug): @@ -596,11 +598,35 @@ def _main(configFile=None): controller.MESSAGES.append(output_messages.ERR_CHECK_LOG_FILE_FOR_MORE_INFO%(logFile)) logging.exception(e) finally: + + remove_remote_var_dirs() + # Always print user params to log _printAdditionalMessages() _summaryParamsToLog() +def remove_remote_var_dirs(): + """ + Removes the temp directories on remote hosts, + doesn't remove data on localhost + """ + for host in gethostlist(controller.CONF): + logging.info(output_messages.INFO_REMOVE_REMOTE_VAR%(basedefs.VAR_DIR, host)) + server = utils.ScriptRunner(host) + # We don't want to remove the tmp dir on the host that was used to + # run packstack, this host has the logfile, if it doesn't have a + # logfile its a remote host, so we remove the temp files + server.append('ls -l %s || rm -rf %s'%(logFile, basedefs.VAR_DIR)) + try: + server.execute() + except Exception, e: + msg = output_messages.ERR_REMOVE_REMOTE_VAR%(basedefs.VAR_DIR, host) + logging.error(msg) + logging.exception(e) + controller.MESSAGES.append(utils.getColoredText(msg, basedefs.RED)) + + def generateAnswerFile(outputFile): sep = os.linesep fmt = ("%(comment)s%(separator)s%(conf_name)s=%(default_value)s" diff --git a/packstack/modules/ospluginutils.py b/packstack/modules/ospluginutils.py index a409cd819..47c95b2cb 100644 --- a/packstack/modules/ospluginutils.py +++ b/packstack/modules/ospluginutils.py @@ -38,10 +38,12 @@ class NovaConfig(object): class ManifestFiles(object): def __init__(self): self.filelist = [] + self.data = {} # continuous manifest file that have the same marker can be # installed in parallel, if on different servers - def addFile(self, filename, marker): + def addFile(self, filename, marker, data=''): + self.data[filename] = self.data.get(filename, '') + '\n' + data for f, p in self.filelist: if f == filename: return @@ -49,6 +51,17 @@ class ManifestFiles(object): def getFiles(self): return [f for f in self.filelist] + + def writeManifests(self): + """ + Write out the manifest data to disk, this should only be called once + write before the puppet manifests are copied to the various servers + """ + os.mkdir(basedefs.PUPPET_MANIFEST_DIR, 0700) + for file, data in self.data.items(): + fd = os.open(file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0600) + with os.fdopen(fd, 'w') as fp: + fp.write(data) manifestfiles = ManifestFiles() @@ -58,13 +71,8 @@ def getManifestTemplate(template_name): def appendManifestFile(manifest_name, data, marker=''): - if not os.path.exists(basedefs.PUPPET_MANIFEST_DIR): - os.mkdir(basedefs.PUPPET_MANIFEST_DIR) manifestfile = os.path.join(basedefs.PUPPET_MANIFEST_DIR, manifest_name) - manifestfiles.addFile(manifestfile, marker) - with open(manifestfile, 'a') as fp: - fp.write("\n") - fp.write(data) + manifestfiles.addFile(manifestfile, marker, data) def gethostlist(CONF): diff --git a/packstack/plugins/puppet_950.py b/packstack/plugins/puppet_950.py index 38a6ee3a5..5071d66c0 100644 --- a/packstack/plugins/puppet_950.py +++ b/packstack/plugins/puppet_950.py @@ -73,15 +73,18 @@ def installdeps(): def copyPuppetModules(): + # write puppet manifest to disk + manifestfiles.writeManifests() + server = utils.ScriptRunner() tar_opts = "" if platform.linux_distribution()[0] == "Fedora": tar_opts += "--exclude create_resources " for hostname in gethostlist(controller.CONF): server.append("cd %s/puppet" % basedefs.DIR_PROJECT_DIR) - server.append("tar %s --dereference -czf - modules facts | ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s tar -C %s -xzf -" % (tar_opts, hostname, basedefs.VAR_DIR)) + server.append("tar %s --dereference -cpzf - modules facts | ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s tar -C %s -xpzf -" % (tar_opts, hostname, basedefs.VAR_DIR)) server.append("cd %s" % basedefs.PUPPET_MANIFEST_DIR) - server.append("tar %s --dereference -czf - ../manifests | ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s tar -C %s -xzf -" % (tar_opts, hostname, basedefs.VAR_DIR)) + server.append("tar %s --dereference -cpzf - ../manifests | ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s tar -C %s -xpzf -" % (tar_opts, hostname, basedefs.VAR_DIR)) server.execute() @@ -134,9 +137,11 @@ def applyPuppetManifest(): running_logfile = "%s.running" % manifest finished_logfile = "%s.finished" % manifest currently_running.append((hostname, finished_logfile)) - command = "( flock %s/ps.lock puppet apply --modulepath %s/modules %s > %s 2>&1 < /dev/null ; mv %s %s ) > /dev/null 2>&1 < /dev/null &" % (basedefs.VAR_DIR, basedefs.VAR_DIR, manifest, running_logfile, running_logfile, finished_logfile) if not manifest.endswith('_horizon.pp'): server.append("export FACTERLIB=$FACTERLIB:%s/facts" % basedefs.VAR_DIR) + server.append("touch %s"%running_logfile) + server.append("chmod 600 %s"%running_logfile) + command = "( flock %s/ps.lock puppet apply --modulepath %s/modules %s > %s 2>&1 < /dev/null ; mv %s %s ) > /dev/null 2>&1 < /dev/null &" % (basedefs.VAR_DIR, basedefs.VAR_DIR, manifest, running_logfile, running_logfile, finished_logfile) server.append(command) server.execute() diff --git a/packstack/plugins/serverprep_901.py b/packstack/plugins/serverprep_901.py index 5b3564031..3a001852f 100644 --- a/packstack/plugins/serverprep_901.py +++ b/packstack/plugins/serverprep_901.py @@ -4,6 +4,7 @@ prepare server import os import logging +import os from packstack.installer import basedefs from packstack.installer import common_utils as utils @@ -376,7 +377,11 @@ def serverprep(): "( rpm -q epel-release || yum install -y --nogpg -c $REPOFILE epel-release ) || echo -n ''") server.append("rm -rf $REPOFILE") - server.append("mkdir -p %s" % basedefs.PUPPET_MANIFEST_DIR) + # Create the packstack tmp directory + server.append("mkdir -p %s" % basedefs.PACKSTACK_VAR_DIR) + # Separately create the tmp directory for this packstack run, this will fail if + # the directory already exists + server.append("mkdir --mode 0700 %s" % basedefs.VAR_DIR) # set highest priority of RHOS repository if EPEL is installed server.append("rpm -q epel-release && yum install -y yum-plugin-priorities || true")