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

@@ -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")
 

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"

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"

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):

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()
 

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")