CVE-2011-3211

@@ -1,5 +1,6 @@
 import getopt
 from subprocess import Popen, PIPE
+import pipes
 import Bcfg2.Server.Admin
 
 class Viz(Bcfg2.Server.Admin.MetadataCore):
@@ -62,7 +63,8 @@ def __call__(self, args):
 
         data = self.Visualize(self.get_repo_path(), hset, bset,
                               kset, outputfile)
-        print data
+        if data:
+            print(data)
         raise SystemExit, 0
 
     def Visualize(self, repopath, hosts=False,
@@ -73,11 +75,21 @@ def Visualize(self, repopath, hosts=False,
         else:
             format = 'png'
 
-        cmd = "dot -T%s" % (format)
+        cmd = ["dot", "-T", format]
         if output:
-            cmd += " -o %s" % output
-        dotpipe = Popen(cmd, shell=True, stdin=PIPE,
-                        stdout=PIPE, close_fds=True)
+            cmd.extend(["-o", output])
+        try:
+            dotpipe = Popen(cmd, stdin=PIPE, stdout=PIPE, close_fds=True)
+        except OSError:
+            # on some systems (RHEL 6), you cannot run dot with
+            # shell=True.  on others (Gentoo with Python 2.7), you
+            # must.  In yet others (RHEL 5), either way works.  I have
+            # no idea what the difference is, but it's kind of a PITA.
+            cmd = ["dot", "-T", pipes.quote(format)]
+            if output:
+                cmd.extend(["-o", pipes.quote(output)])
+            dotpipe = Popen(cmd, shell=True,
+                            stdin=PIPE, stdout=PIPE, close_fds=True)
         try:
             dotpipe.stdin.write("digraph groups {\n")
         except:

@@ -7,6 +7,7 @@
 import os
 import re
 import tempfile
+from subprocess import Popen, PIPE
 
 import Bcfg2.Server.Plugin
 
@@ -32,17 +33,16 @@ def process_delta(data, delta):
         basefile.write(data)
         basefile.close()
         os.close(basehandle)
-        dhandle, dname = tempfile.mkstemp()
-        dfile = open(dname, 'w')
-        dfile.write(delta.data)
-        dfile.close()
-        os.close(dhandle)
-        ret = os.system("patch -uf %s < %s > /dev/null 2>&1" \
-                        % (basefile.name, dfile.name))
+        
+        cmd = ["patch", "-u", "-f", basefile.name]
+        patch = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+        stderr = patch.communicate(input=delta.data)[1]
+        ret = patch.wait()
         output = open(basefile.name, 'r').read()
-        [os.unlink(fname) for fname in [basefile.name, dfile.name]]
+        os.unlink(basefile.name)
         if ret >> 8 != 0:
-            raise Bcfg2.Server.Plugin.PluginExecutionError, ('delta', delta)
+            logger.error("Error applying diff %s: %s" % (delta.name, stderr))
+            raise Bcfg2.Server.Plugin.PluginExecutionError('delta', delta)
         return output
 
 class CfgMatcher:

@@ -1,6 +1,5 @@
 import os
 from mercurial import ui, hg
-from subprocess import Popen, PIPE
 import Bcfg2.Server.Plugin
 
 # for debugging output only

@@ -3,6 +3,7 @@
 
 import binascii
 import os
+import sys
 import socket
 import shutil
 import tempfile
@@ -162,8 +163,7 @@ def get_ipcache_entry(self, client):
                 self.ipcache[client] = (ipaddr, client)
                 return (ipaddr, client)
             except socket.gaierror:
-                cmd = "getent hosts %s" % client
-                ipaddr = Popen(cmd, shell=True, \
+                ipaddr = Popen(["getent", "hosts", client],
                                stdout=PIPE).stdout.read().strip().split()
                 if ipaddr:
                     self.ipcache[client] = (ipaddr, client)
@@ -252,19 +252,26 @@ def GenerateHostKeys(self, client):
                                                      "H_%s" % client])
                 tempdir = tempfile.mkdtemp()
                 temploc = "%s/%s" % (tempdir, hostkey)
-                cmd = 'ssh-keygen -q -f %s -N "" -t %s -C root@%s < /dev/null'
-                os.system(cmd % (temploc, keytype, client))
-                shutil.copy(temploc, fileloc)
-                shutil.copy("%s.pub" % temploc, publoc)
-                self.AddEntry(hostkey)
-                self.AddEntry(".".join([hostkey.split('.')[0]]+['pub', "H_%s" \
-                                                                % client]))
+                cmd = ["ssh-keygen", "-q", "-f", temploc, "-N", "",
+                       "-t", keytype, "-C", "root@%s" % client]
+                proc = Popen(cmd, stdout=PIPE, stdin=PIPE)
+                proc.communicate()
+                proc.wait()
+
+                try:
+                    shutil.copy(temploc, fileloc)
+                    shutil.copy("%s.pub" % temploc, publoc)
+                except IOError:
+                    err = sys.exc_info()[1]
+                    self.logger.error("Temporary SSH keys not found: %s" % err)
                 try:
                     os.unlink(temploc)
                     os.unlink("%s.pub" % temploc)
                     os.rmdir(tempdir)
                 except OSError:
-                    self.logger.error("Failed to unlink temporary ssh keys")
+                    err = sys.exc_info()[1]
+                    self.logger.error("Failed to unlink temporary ssh keys: %s"
+                                      % err)
 
     def AcceptChoices(self, _, metadata):
         return [Bcfg2.Server.Plugin.Specificity(hostname=metadata.hostname)]

@@ -1,4 +1,5 @@
 import os
+import pipes
 from subprocess import Popen, PIPE
 import Bcfg2.Server.Plugin
 
@@ -35,7 +36,7 @@ def get_revision(self):
         """Read svn revision information for the Bcfg2 repository."""
         try:
             data = Popen(("env LC_ALL=C svn info %s" %
-                         (self.datastore)), shell=True,
+                         pipes.quote(self.datastore)), shell=True,
                          stdout=PIPE).communicate()[0].split('\n')
             return [line.split(': ')[1] for line in data \
                     if line[:9] == 'Revision:'][-1]

@@ -86,11 +86,10 @@ def Visualize(self, repopath, hosts=False,
         else:
             format = 'png'
 
-        cmd = "dot -T%s" % (format)
+        cmd = ["dot", "-T", format]
         if output:
-            cmd += " -o %s" % output
-        dotpipe = Popen(cmd, shell=True, stdin=PIPE,
-                        stdout=PIPE, close_fds=True)
+            cmd.extend(["-o", output])
+        dotpipe = Popen(cmd, stdin=PIPE, stdout=PIPE, close_fds=True)
         try:
             dotpipe.stdin.write("digraph groups {\n")
         except:

@@ -1,6 +1,5 @@
 import os
 from mercurial import ui, hg
-from subprocess import Popen, PIPE
 import Bcfg2.Server.Plugin
 
 # for debugging output only

@@ -169,8 +169,7 @@ def get_ipcache_entry(self, client):
                 self.ipcache[client] = (ipaddr, client)
                 return (ipaddr, client)
             except socket.gaierror:
-                cmd = "getent hosts %s" % client
-                ipaddr = Popen(cmd, shell=True, \
+                ipaddr = Popen(["getent", "hosts", client],
                                stdout=PIPE).stdout.read().strip().split()
                 if ipaddr:
                     self.ipcache[client] = (ipaddr, client)

@@ -3,6 +3,7 @@
 import lxml.etree
 import posixpath
 import tempfile
+import pipes
 import os
 from subprocess import Popen, PIPE, STDOUT
 # Compatibility import
@@ -119,10 +120,10 @@ def build_key(self, filename, entry, metadata):
         type = self.key_specs[entry.get('name')]['type']
         bits = self.key_specs[entry.get('name')]['bits']
         if type == 'rsa':
-            cmd = "openssl genrsa %s " % bits
+            cmd = ["openssl", "genrsa", bits]
         elif type == 'dsa':
-            cmd = "openssl dsaparam -noout -genkey %s" % bits
-        key = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
+            cmd = ["openssl", "dsaparam", "-noout", "-genkey", bits]
+        key = Popen(cmd, stdout=PIPE).stdout.read()
         return key
 
     def get_cert(self, entry, metadata):
@@ -176,8 +177,8 @@ def verify_cert_against_ca(self, filename, entry):
         """
         chaincert = self.CAs[self.cert_specs[entry.get('name')]['ca']].get('chaincert')
         cert = self.data + filename
-        cmd = "openssl verify -CAfile %s %s" % (chaincert, cert)
-        res = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
+        res = Popen(["openssl", "verify", "-CAfile", chaincert, cert],
+                    stdout=PIPE, stderr=STDOUT).stdout.read()
         if res == cert + ": OK\n":
             return True
         return False
@@ -188,9 +189,11 @@ def verify_cert_against_key(self, filename, key_filename):
         """
         cert = self.data + filename
         key = self.data + key_filename
-        cmd = "openssl x509 -noout -modulus -in %s | openssl md5" % cert
+        cmd = ("openssl x509 -noout -modulus -in %s | openssl md5" %
+               pipes.quote(cert))
         cert_md5 = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
-        cmd = "openssl rsa -noout -modulus -in %s | openssl md5" % key
+        cmd = ("openssl rsa -noout -modulus -in %s | openssl md5" %
+               pipes.quote(key))
         key_md5 = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
         if cert_md5 == key_md5:
             return True
@@ -206,16 +209,11 @@ def build_cert(self, key_filename, entry, metadata):
         ca_config = self.CAs[ca]['config']
         days = self.cert_specs[entry.get('name')]['days']
         passphrase = self.CAs[ca].get('passphrase')
+        cmd = ["openssl", "ca", "-config", ca_config, "-in", req,
+               "-days", days, "-batch"]
         if passphrase:
-            cmd = "openssl ca -config %s -in %s -days %s -batch -passin pass:%s" % (ca_config,
-                                                                                    req,
-                                                                                    days,
-                                                                                    passphrase)
-        else:
-            cmd = "openssl ca -config %s -in %s -days %s -batch" % (ca_config,
-                                                                    req,
-                                                                    days)
-        cert = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
+            cmd.extend(["-passin", "pass:%s" % passphrase])
+        cert = Popen(cmd, stdout=PIPE).stdout.read()
         try:
             os.unlink(req_config)
             os.unlink(req)
@@ -271,9 +269,7 @@ def build_request(self, key_filename, req_config, entry):
         req = tempfile.mkstemp()[1]
         days = self.cert_specs[entry.get('name')]['days']
         key = self.data + key_filename
-        cmd = "openssl req -new -config %s -days %s -key %s -text -out %s" % (req_config,
-                                                                              days,
-                                                                              key,
-                                                                              req)
+        cmd = ["openssl", "req", "-new", "-config", req_config,
+               "-days", days, "-key", key, "-text", "-out", req]
         res = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
         return req

@@ -35,7 +35,7 @@ def get_revision(self):
         """Read svn revision information for the Bcfg2 repository."""
         try:
             data = Popen(("env LC_ALL=C svn info %s" %
-                         (self.datastore)), shell=True,
+                         pipes.quote(self.datastore)), shell=True,
                          stdout=PIPE).communicate()[0].split('\n')
             return [line.split(': ')[1] for line in data \
                     if line[:9] == 'Revision:'][-1]