AMBARI-9689. Vulnerability issue: possible to make code injection with hosts bootstrap request (aonishuk)

ambari-common/src/main/python/resource_management/core/shell.py

@@ -191,7 +191,6 @@ def _call(command, logoutput=None, throw_on_failure=True,
       raise ExecuteTimeoutException()
    
   code = proc.returncode
-  print code
   
   if throw_on_failure and code:
     err_msg = Logger.filter_text(("Execution of '%s' returned %d. %s") % (command_alias, code, out))

ambari-server/pom.xml

@@ -1756,6 +1756,11 @@
       <artifactId>snmp4j</artifactId>
       <version>1.10.1</version>
     </dependency>
+	<dependency>
+		<groupId>org.apache.commons</groupId>
+		<artifactId>commons-io</artifactId>
+		<version>1.3.2</version>
+	</dependency>
   </dependencies>
 
   <pluginRepositories>

ambari-server/src/main/java/org/apache/ambari/server/bootstrap/BSRunner.java

@@ -19,6 +19,8 @@ package org.apache.ambari.server.bootstrap;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
@@ -27,6 +29,7 @@ import java.util.concurrent.TimeUnit;
 
 import org.apache.ambari.server.bootstrap.BootStrapStatus.BSStat;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -154,8 +157,7 @@ class BSRunner extends Thread {
     if (user == null || user.isEmpty()) {
       user = DEFAULT_USER;
     }
-    String commands[] = new String[12];
-    String shellCommand[] = new String[3];
+    String command[] = new String[12];
     BSStat stat = BSStat.RUNNING;
     String scriptlog = "";
     try {
@@ -181,23 +183,18 @@ class BSRunner extends Thread {
       /* Running command:
        * script hostlist bsdir user sshkeyfile
        */
-      shellCommand[0] = System.getProperty("os.name").contains("Windows") ? "cmd" : "sh";
-      shellCommand[1] = System.getProperty("os.name").contains("Windows") ? "/C" : "-c";
-
-      commands[0] = this.bsScript;
-      commands[1] = hostString;
-      commands[2] = this.requestIdDir.toString();
-      commands[3] = user;
-      commands[4] = this.sshKeyFile.toString();
-      commands[5] = this.agentSetupScript.toString();
-      commands[6] = this.ambariHostname;
-      commands[7] = this.clusterOsFamily;
-      commands[8] = this.projectVersion;
-      commands[9] = this.serverPort+"";
-      commands[10] = userRunAs;
-      if (this.passwordFile != null) {
-        commands[11] = this.passwordFile.toString();
-      }
+      command[0] = this.bsScript;
+      command[1] = hostString;
+      command[2] = this.requestIdDir.toString();
+      command[3] = user;
+      command[4] = this.sshKeyFile.toString();
+      command[5] = this.agentSetupScript.toString();
+      command[6] = this.ambariHostname;
+      command[7] = this.clusterOsFamily;
+      command[8] = this.projectVersion;
+      command[9] = this.serverPort+"";
+      command[10] = userRunAs;
+      command[11] = (this.passwordFile==null) ? "null" : this.passwordFile.toString();
       LOG.info("Host= " + hostString + " bs=" + this.bsScript + " requestDir=" +
           requestIdDir + " user=" + user + " keyfile=" + this.sshKeyFile +
           " passwordFile " + this.passwordFile + " server=" + this.ambariHostname +
@@ -207,22 +204,31 @@ class BSRunner extends Thread {
       if (this.verbose)
         env = new String[] { env[0], " BS_VERBOSE=\"-vvv\" " };
 
-      StringBuilder commandString = new StringBuilder();
-      for (String comm : commands) {
-        commandString.append(" " + comm);
-      }
-
       if (LOG.isDebugEnabled()) {
-        LOG.debug(commandString);
+        LOG.debug(Arrays.toString(command));
       }
 
-      String bootStrapOutputFile = requestIdDir + File.separator + "bootstrap.out";
-      String bootStrapErrorFile = requestIdDir + File.separator + "bootstrap.err";
-      commandString.append(
-          " 1> " + bootStrapOutputFile + " 2>" + bootStrapErrorFile);
+      String bootStrapOutputFilePath = requestIdDir + File.separator + "bootstrap.out";
+      String bootStrapErrorFilePath = requestIdDir + File.separator + "bootstrap.err";
+
+      Process process = Runtime.getRuntime().exec(command, env);
+      
+      PrintWriter stdOutWriter = null;
+      PrintWriter stdErrWriter = null;
+      
+      try {
+        stdOutWriter = new PrintWriter(bootStrapOutputFilePath);
+        stdErrWriter = new PrintWriter(bootStrapErrorFilePath);
+        IOUtils.copy(process.getInputStream(), stdOutWriter);
+        IOUtils.copy(process.getErrorStream(), stdErrWriter);
+      } finally {
+        if(stdOutWriter != null)
+          stdOutWriter.close();
+        
+        if(stdErrWriter != null)
+          stdErrWriter.close();
+      }
 
-      shellCommand[2] = commandString.toString();
-      Process process = Runtime.getRuntime().exec(shellCommand, env);
 
       // Startup a scheduled executor service to look through the logs
       ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
@@ -234,13 +240,13 @@ class BSRunner extends Thread {
       try {
 
         LOG.info("Bootstrap output, log="
-              + bootStrapErrorFile + " " + bootStrapOutputFile);
+              + bootStrapErrorFilePath + " " + bootStrapOutputFilePath);
         int exitCode = process.waitFor();
         String outMesg = "";
         String errMesg = "";
         try {
-          outMesg = FileUtils.readFileToString(new File(bootStrapOutputFile));
-          errMesg = FileUtils.readFileToString(new File(bootStrapErrorFile));
+          outMesg = FileUtils.readFileToString(new File(bootStrapOutputFilePath));
+          errMesg = FileUtils.readFileToString(new File(bootStrapErrorFilePath));
         } catch(IOException io) {
           LOG.info("Error in reading files ", io);
         }

ambari-server/src/main/python/bootstrap.py

@@ -32,6 +32,7 @@ import threading
 import traceback
 import re
 from datetime import datetime
+from resource_management.core.shell import quote_bash_args
 
 AMBARI_PASSPHRASE_VAR_NAME = "AMBARI_PASSPHRASE"
 HOST_BOOTSTRAP_TIMEOUT = 300
@@ -244,7 +245,7 @@ class Bootstrap(threading.Thread):
     params = self.shared_state
     user = params.user
 
-    command = "sudo mkdir -p {0} ; sudo chown -R {1} {0}".format(self.TEMP_FOLDER,params.user)
+    command = "sudo mkdir -p {0} ; sudo chown -R {1} {0}".format(self.TEMP_FOLDER,quote_bash_args(params.user))
 
     ssh = SSH(params.user, params.sshkey_file, self.host, command,
               params.bootdir, self.host_log)
@@ -368,7 +369,7 @@ class Bootstrap(threading.Thread):
     port = self.getAmbariPort()
     passwordFile = self.getPasswordFile()
     return "sudo -S python " + str(setupFile) + " " + str(expected_hostname) + \
-           " " + str(passphrase) + " " + str(server)+ " " + str(user_run_as) + " " + str(version) + \
+           " " + str(passphrase) + " " + str(server)+ " " + quote_bash_args(str(user_run_as)) + " " + str(version) + \
            " " + str(port) + " < " + str(passwordFile)
 
 
@@ -380,7 +381,7 @@ class Bootstrap(threading.Thread):
     version=self.getAmbariVersion()
     port=self.getAmbariPort()
     return "sudo python " + str(setupFile) + " " + str(expected_hostname) + \
-           " " + str(passphrase) + " " + str(server)+ " " + str(user_run_as) + " " + str(version) + \
+           " " + str(passphrase) + " " + str(server)+ " " + quote_bash_args(str(user_run_as)) + " " + str(version) + \
            " " + str(port)
 
 

ambari-server/src/main/python/setupAgent.py

@@ -76,9 +76,9 @@ def runAgent(passPhrase, expected_hostname, user_run_as, verbose):
   vo = ""
   if verbose:
     vo = " -v"
-  cmd = "su - %1s -c '/usr/sbin/ambari-agent restart --expected-hostname=%2s %3s'" % (user_run_as, expected_hostname, vo)
+  cmd = ['su', user_run_as, '-l', '-c', '/usr/sbin/ambari-agent restart --expected-hostname=%1s %2s' % (expected_hostname, vo)]
   log = ""
-  p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True)
+  p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
   p.communicate()
   agent_retcode = p.returncode
   for i in range(3):

ambari-server/src/test/java/org/apache/ambari/server/bootstrap/BootStrapTest.java

@@ -87,6 +87,7 @@ public class BootStrapTest extends TestCase {
     ArrayList<String> hosts = new ArrayList<String>();
     hosts.add("host1");
     hosts.add("host2");
+    info.setUserRunAs("root");
     info.setHosts(hosts);
     info.setUser("user");
     info.setPassword("passwd");
@@ -144,6 +145,7 @@ public class BootStrapTest extends TestCase {
     hosts.add("host2");
     info.setHosts(hosts);
     info.setUser("user");
+    info.setUserRunAs("root");
     info.setPassword("passwd");
     BSResponse response = impl.runBootStrap(info);
     long requestId = response.getRequestId();