Reorganise ssh.py to cleanly separate responsibilities

The main exec_command/put_file/fetch_file methods now _build_command and
call _run to handle input from/output to the ssh process. The purpose is
to bring connection handling together in one place so that the locking
doesn't have to be split across functions.

Note that this doesn't change the privilege escalation and connection IO
code at all—just puts it all into one function.

Most of the changes are just moving code from one place to another (e.g.
from _connect to _build_command, from _exec_command and _communicate to
_run), but there are some other notable changes:

1. We test for the existence of sshpass the first time we need to use
   password authentication, and remember the result.
2. We set _persistent in _build_command if we're using ControlPersist,
   for later use in close(). (The detection could be smarter.)
3. Some apparently inadvertent inconsistencies between put_file and
   fetch_file (e.g. argument quoting, sftp -b use) have been removed.

Also reorders functions into a logical sequence, removes unused imports
and functions, etc.

Aside: the high-level EXEC/PUT/FETCH description should really be logged
from ConnectionBase, while individual subclasses log transport-specific
details.

lib/ansible/plugins/connection/__init__.py

@@ -159,13 +159,13 @@ class ConnectionBase(with_metaclass(ABCMeta, object)):
         if incorrect_password in output:
             raise AnsibleError('Incorrect %s password' % self._play_context.become_method)
 
-    def lock_connection(self):
+    def connection_lock(self):
         f = self._play_context.connection_lockfd
         self._display.vvvv('CONNECTION: pid %d waiting for lock on %d' % (os.getpid(), f))
         fcntl.lockf(f, fcntl.LOCK_EX)
         self._display.vvvv('CONNECTION: pid %d acquired lock on %d' % (os.getpid(), f))
 
-    def unlock_connection(self):
+    def connection_unlock(self):
         f = self._play_context.connection_lockfd
         fcntl.lockf(f, fcntl.LOCK_UN)
         self._display.vvvv('CONNECTION: pid %d released lock on %d' % (os.getpid(), f))

lib/ansible/plugins/connection/paramiko_ssh.py

@@ -81,7 +81,7 @@ class MyAddPolicy(object):
 
         if C.HOST_KEY_CHECKING:
 
-            self.connection.lock_connection()
+            self.connection.connection_lock()
 
             old_stdin = sys.stdin
             sys.stdin = self._new_stdin
@@ -95,7 +95,7 @@ class MyAddPolicy(object):
             inp = raw_input(AUTHENTICITY_MSG % (hostname, ktype, fingerprint))
             sys.stdin = old_stdin
 
-            self.connection.unlock_connection()
+            self.connection.connection_unlock()
 
             if inp not in ['yes','y','']:
                 raise AnsibleError("host connection rejected by user")

lib/ansible/plugins/connection/ssh.py

@@ -18,38 +18,31 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import gettext
 import fcntl
-import hmac
 import os
 import pipes
 import pty
 import pwd
-import random
-import re
 import select
 import shlex
 import subprocess
 import time
 
-from hashlib import sha1
-
 from ansible import constants as C
 from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNotFound
 from ansible.plugins.connection import ConnectionBase
 from ansible.utils.path import unfrackpath, makedirs_safe
 
+SSHPASS_AVAILABLE = None
+
 class Connection(ConnectionBase):
     ''' ssh based connections '''
 
+    transport = 'ssh'
     has_pipelining = True
     become_methods = frozenset(C.BECOME_METHODS).difference(['runas'])
 
     def __init__(self, *args, **kwargs):
-        # SSH connection specific init stuff
-        self._common_args = []
-        self.HASHED_KEY_MAGIC = "|1|"
-
         super(Connection, self).__init__(*args, **kwargs)
 
         self.host = self._play_context.remote_addr
@@ -63,38 +56,81 @@ class Connection(ConnectionBase):
         if 'ansible_ssh_args' in v:
             self.ssh_args = v['ansible_ssh_args']
 
-    @property
+    # The connection is created by running ssh/scp/sftp from the exec_command,
-    def transport(self):
+    # put_file, and fetch_file methods, so we don't need to do any connection
-        ''' used to identify this connection object from other classes '''
+    # management here.
-        return 'ssh'
-
-    def _split_args(self, argstring):
-        """
-        Takes a string like '-o Foo=1 -o Bar="foo bar"' and returns a
-        list ['-o', 'Foo=1', '-o', 'Bar=foo bar'] that can be added to
-        the argument list. The list will not contain any empty elements.
-        """
-        return [x.strip() for x in shlex.split(argstring) if x.strip()]
-
-    def add_args(self, explanation, args):
-        """
-        Adds the given args to _common_args and displays a
-        caller-supplied explanation of why they were added.
-        """
-        self._common_args += args
-        self._display.vvvvv('SSH: ' + explanation + ': (%s)' % ')('.join(args), host=self._play_context.remote_addr)
 
     def _connect(self):
-        ''' connect to the remote host '''
+        self._connected = True
+        return self
 
-        self._display.vvv("ESTABLISH SSH CONNECTION FOR USER: {0}".format(self._play_context.remote_user), host=self._play_context.remote_addr)
+    def close(self):
+        # If we have a persistent ssh connection (ControlPersist), we can ask it
+        # to stop listening. Otherwise, there's nothing to do here.
 
-        if self._connected:
+        # TODO: reenable once winrm issues are fixed
-            return self
+        # temporarily disabled as we are forced to currently close connections after every task because of winrm
+        # if self._connected and self._persistent:
+        #     cmd = self._build_command('ssh', '-O', 'stop', self.host)
+        #     p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        #     stdout, stderr = p.communicate()
 
-        # We start with ansible_ssh_args from the inventory if it's set,
+        self._connected = False
-        # or [ssh_connection]ssh_args from ansible.cfg, or the default
+
-        # Control* settings.
+    def _build_command(self, binary, *other_args):
+        '''
+        Takes a binary (ssh, scp, sftp) and optional extra arguments and returns
+        a command line as an array that can be passed to subprocess.Popen after
+        appending any extra commands to it.
+        '''
+
+        self._command = []
+
+        ## First, the command name.
+
+        # If we want to use password authentication, we have to set up a pipe to
+        # write the password to sshpass.
+
+        if self._play_context.password:
+            global SSHPASS_AVAILABLE
+
+            # We test once if sshpass is available, and remember the result. It
+            # would be nice to use distutils.spawn.find_executable for this, but
+            # distutils isn't always available; shutils.which() is Python3-only.
+
+            if SSHPASS_AVAILABLE is None:
+                try:
+                    p = subprocess.Popen(["sshpass"], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                    p.communicate()
+                    SSHPASS_AVAILABLE = True
+                except OSError:
+                    SSHPASS_AVAILABLE = False
+
+            if not SSHPASS_AVAILABLE:
+                raise AnsibleError("to use the 'ssh' connection type with passwords, you must install the sshpass program")
+
+            self.sshpass_pipe = os.pipe()
+            self._command += ['sshpass', '-d{0}'.format(self.sshpass_pipe[0])]
+
+        self._command += [binary]
+
+        ## Next, additional arguments based on the configuration.
+
+        # sftp batch mode allows us to correctly catch failed transfers, but can
+        # be disabled if the client side doesn't support the option. FIXME: is
+        # this still a real concern?
+
+        if binary == 'sftp' and C.DEFAULT_SFTP_BATCH_MODE:
+            self._command += ['-b', '-']
+
+        elif binary == 'ssh':
+            self._command += ['-C']
+
+        self._command += ['-vvv']
+
+        # Next, we add ansible_ssh_args from the inventory if it's set, or
+        # [ssh_connection]ssh_args from ansible.cfg, or the default Control*
+        # settings.
 
         if self.ssh_args:
             args = self._split_args(self.ssh_args)
@@ -109,29 +145,9 @@ class Connection(ConnectionBase):
             )
             self.add_args("default arguments", args)
 
-        # If any of the above have set ControlPersist but not a
+        # Now we add various arguments controlled by configuration file settings
-        # ControlPath, add one ourselves.
+        # (e.g. host_key_checking) or inventory variables (ansible_ssh_port) or
-
+        # a combination thereof.
-        cp_in_use = False
-        cp_path_set = False
-        for arg in self._common_args:
-            if "ControlPersist" in arg:
-                cp_in_use = True
-            if "ControlPath" in arg:
-                cp_path_set = True
-
-        if cp_in_use and not cp_path_set:
-            self._cp_dir = unfrackpath('$HOME/.ansible/cp')
-
-            args = ("-o", "ControlPath=\"{0}\"".format(
-                C.ANSIBLE_SSH_CONTROL_PATH % dict(directory=self._cp_dir))
-            )
-            self.add_args("found only ControlPersist; added ControlPath", args)
-
-            # The directory must exist and be writable.
-            makedirs_safe(self._cp_dir, 0o700)
-            if not os.access(self._cp_dir, os.W_OK):
-                raise AnsibleError("Cannot write to ControlPath %s" % self._cp_dir)
 
         if not C.HOST_KEY_CHECKING:
             self.add_args(
@@ -184,110 +200,43 @@ class Connection(ConnectionBase):
             args = self._split_args(self.ssh_extra_args)
             self.add_args("inventory added ansible_ssh_extra_args", args)
 
-        self._connected = True
+        # If ssh_args or ssh_extra_args set ControlPersist but not a
+        # ControlPath, add one ourselves.
 
-        return self
+        cp_in_use = False
+        cp_path_set = False
+        for arg in self._command:
+            if "ControlPersist" in arg:
+                cp_in_use = True
+            if "ControlPath" in arg:
+                cp_path_set = True
 
-    def _run(self, cmd, indata):
+        if cp_in_use and not cp_path_set:
-        if indata:
+            self._cp_dir = unfrackpath('$HOME/.ansible/cp')
-            # do not use pseudo-pty
-            p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            stdin = p.stdin
-        else:
-            # try to use upseudo-pty
-            try:
-                # Make sure stdin is a proper (pseudo) pty to avoid: tcgetattr errors
-                master, slave = pty.openpty()
-                p = subprocess.Popen(cmd, stdin=slave, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-                stdin = os.fdopen(master, 'w', 0)
-                os.close(slave)
-            except:
-                p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-                stdin = p.stdin
 
-        return (p, stdin)
+            args = ("-o", "ControlPath={0}".format(
+                C.ANSIBLE_SSH_CONTROL_PATH % dict(directory=self._cp_dir))
+            )
+            self.add_args("found only ControlPersist; added ControlPath", args)
 
-    def _password_cmd(self):
+            # The directory must exist and be writable.
-        if self._play_context.password:
+            makedirs_safe(self._cp_dir, 0o700)
-            try:
+            if not os.access(self._cp_dir, os.W_OK):
-                p = subprocess.Popen(["sshpass"], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                raise AnsibleError("Cannot write to ControlPath %s" % self._cp_dir)
-                p.communicate()
-            except OSError:
-                raise AnsibleError("to use the 'ssh' connection type with passwords, you must install the sshpass program")
-            (self.rfd, self.wfd) = os.pipe()
-            return ["sshpass", "-d{0}".format(self.rfd)]
-        return []
 
-    def _send_password(self):
+        # If the configuration dictates that we use a persistent connection,
-        if self._play_context.password:
+        # then we remember that for later. (We could be more thorough about
-            os.close(self.rfd)
+        # detecting this, though.)
-            os.write(self.wfd, "{0}\n".format(self._play_context.password))
-            os.close(self.wfd)
 
-    def _communicate(self, p, stdin, indata, sudoable=True):
+        if cp_in_use:
-        fcntl.fcntl(p.stdout, fcntl.F_SETFL, fcntl.fcntl(p.stdout, fcntl.F_GETFL) & ~os.O_NONBLOCK)
+            self._persistent = True
-        fcntl.fcntl(p.stderr, fcntl.F_SETFL, fcntl.fcntl(p.stderr, fcntl.F_GETFL) & ~os.O_NONBLOCK)
-        # We can't use p.communicate here because the ControlMaster may have stdout open as well
-        stdout = ''
-        stderr = ''
-        rpipes = [p.stdout, p.stderr]
-        if indata:
-            try:
-                stdin.write(indata)
-                stdin.close()
-            except:
-                raise AnsibleConnectionFailure('SSH Error: data could not be sent to the remote host. Make sure this host can be reached over ssh')
-        # Read stdout/stderr from process
-        while True:
-            rfd, wfd, efd = select.select(rpipes, [], rpipes, 1)
 
-            # fail early if the become password is wrong
+        ## Finally, we add any caller-supplied extras.
-            if self._play_context.become and sudoable:
-                if self._play_context.become_pass:
-                    self.check_incorrect_password(stdout)
-                elif self.check_password_prompt(stdout):
-                    raise AnsibleError('Missing %s password' % self._play_context.become_method)
 
-            if p.stderr in rfd:
+        if other_args:
-                dat = os.read(p.stderr.fileno(), 9000)
+            self._command += other_args
-                stderr += dat
-                if dat == '':
-                    rpipes.remove(p.stderr)
-            elif p.stdout in rfd:
-                dat = os.read(p.stdout.fileno(), 9000)
-                stdout += dat
-                if dat == '':
-                    rpipes.remove(p.stdout)
 
-            # only break out if no pipes are left to read or
+        return self._command
-            # the pipes are completely read and
-            # the process is terminated
-            if (not rpipes or not rfd) and p.poll() is not None:
-                break
-            # No pipes are left to read but process is not yet terminated
-            # Only then it is safe to wait for the process to be finished
-            # NOTE: Actually p.poll() is always None here if rpipes is empty
-            elif not rpipes and p.poll() == None:
-                p.wait()
-                # The process is terminated. Since no pipes to read from are
-                # left, there is no need to call select() again.
-                break
-        # close stdin after process is terminated and stdout/stderr are read
-        # completely (see also issue #848)
-        stdin.close()
-        return (p.returncode, stdout, stderr)
-
-    def lock_host_keys(self, lock):
-
-        # lock around the initial SSH connectivity so the user prompt about
-        # whether to add the host to known hosts is not intermingled with
-        # multiprocess output.
-        #
-        # This is a noop for now, pending further investigation. The lock file
-        # should be opened in TaskQueueManager and passed down through the
-        # PlayContext.
-
-        pass
 
     def exec_command(self, *args, **kwargs):
         """
@@ -331,7 +280,6 @@ class Connection(ConnectionBase):
                     time.sleep(pause)
                     continue
 
-
         return return_tuple
 
     def _exec_command(self, cmd, tmp_path, in_data=None, sudoable=True):
@@ -339,52 +287,125 @@ class Connection(ConnectionBase):
 
         super(Connection, self).exec_command(cmd, tmp_path, in_data=in_data, sudoable=sudoable)
 
-        ssh_cmd = self._password_cmd()
+        self._display.vvv("ESTABLISH SSH CONNECTION FOR USER: {0}".format(self._play_context.remote_user), host=self._play_context.remote_addr)
-        ssh_cmd += ("ssh", "-C")
+
-        if not in_data:
+        # we can only use tty when we are not pipelining the modules. piping
-            # we can only use tty when we are not pipelining the modules. piping data into /usr/bin/python
+        # data into /usr/bin/python inside a tty automatically invokes the
-            # inside a tty automatically invokes the python interactive-mode but the modules are not
+        # python interactive-mode but the modules are not compatible with the
-            # compatible with the interactive-mode ("unexpected indent" mainly because of empty lines)
+        # interactive-mode ("unexpected indent" mainly because of empty lines)
-            ssh_cmd.append("-tt")
+
-        if self._play_context.verbosity > 3:
+        if in_data:
-            ssh_cmd.append("-vvv")
+            cmd = self._build_command('ssh', self.host, cmd)
         else:
-            ssh_cmd.append("-q")
+            cmd = self._build_command('ssh', '-tt', self.host, cmd)
-        ssh_cmd += self._common_args
 
-        ssh_cmd.append(self.host)
+        (returncode, stdout, stderr) = self._run(cmd, in_data, sudoable=sudoable)
 
-        ssh_cmd.append(cmd)
+        return (returncode, '', stdout, stderr)
-        self._display.vvv("EXEC {0}".format(' '.join(ssh_cmd)), host=self.host)
 
-        self.lock_host_keys(True)
+    def put_file(self, in_path, out_path):
+        ''' transfer a file from local to remote '''
 
-        # create process
+        super(Connection, self).put_file(in_path, out_path)
-        (p, stdin) = self._run(ssh_cmd, in_data)
 
-        self._send_password()
+        self._display.vvv("PUT {0} TO {1}".format(in_path, out_path), host=self.host)
+        if not os.path.exists(in_path):
+            raise AnsibleFileNotFound("file or module does not exist: {0}".format(in_path))
+
+        # scp and sftp require square brackets for IPv6 addresses, but
+        # accept them for hostnames and IPv4 addresses too.
+        host = '[%s]' % self.host
+
+        if C.DEFAULT_SCP_IF_SSH:
+            cmd = self._build_command('scp', in_path, '{0}:{1}'.format(host, pipes.quote(out_path)))
+            in_data = None
+        else:
+            cmd = self._build_command('sftp', host)
+            in_data = "put {0} {1}\n".format(pipes.quote(in_path), pipes.quote(out_path))
+
+        (returncode, stdout, stderr) = self._run(cmd, in_data)
+
+        if returncode != 0:
+            raise AnsibleError("failed to transfer file to {0}:\n{1}\n{2}".format(out_path, stdout, stderr))
+
+    def fetch_file(self, in_path, out_path):
+        ''' fetch a file from remote to local '''
+
+        super(Connection, self).fetch_file(in_path, out_path)
+
+        self._display.vvv("FETCH {0} TO {1}".format(in_path, out_path), host=self.host)
+
+        # scp and sftp require square brackets for IPv6 addresses, but
+        # accept them for hostnames and IPv4 addresses too.
+        host = '[%s]' % self.host
+
+        if C.DEFAULT_SCP_IF_SSH:
+            cmd = self._build_command('scp', '{0}:{1}'.format(host, pipes.quote(in_path)), out_path)
+            in_data = None
+        else:
+            cmd = self._build_command('sftp', host)
+            in_data = "get {0} {1}\n".format(pipes.quote(in_path), pipes.quote(out_path))
+
+        (returncode, stdout, stderr) = self._run(cmd, in_data)
+
+        if returncode != 0:
+            raise AnsibleError("failed to transfer file from {0}:\n{1}\n{2}".format(in_path, stdout, stderr))
+
+    def _run(self, cmd, in_data, sudoable=True):
+        '''
+        Starts the command and communicates with it until it ends.
+        '''
+
+        display_cmd = map(pipes.quote, cmd[:-1]) + [cmd[-1]]
+        self._display.vvv('SSH: EXEC {0}'.format(' '.join(display_cmd)), host=self.host)
+
+        # Start the given command. If we don't need to pipeline data, we can try
+        # to use a pseudo-tty. If we are pipelining data, or can't create a pty,
+        # we fall back to using plain old pipes.
+
+        p = None
+        if not in_data:
+            try:
+                # Make sure stdin is a proper pty to avoid tcgetattr errors
+                master, slave = pty.openpty()
+                p = subprocess.Popen(cmd, stdin=slave, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                stdin = os.fdopen(master, 'w', 0)
+                os.close(slave)
+            except:
+                p = None
+
+        if not p:
+            p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            stdin = p.stdin
+
+        # If we are using SSH password authentication, write the password to the
+        # pipe we opened in _build_command.
+
+        if self._play_context.password:
+            os.close(self.sshpass_pipe[0])
+            os.write(self.sshpass_pipe[1], "{0}\n".format(self._play_context.password))
+            os.close(self.sshpass_pipe[1])
+
+        # This section is specific to ssh:
+        #
+        # If we have a privilege escalation prompt, we need to look for the
+        # prompt and send the password (but we won't be prompted if sudo has
+        # NOPASSWD configured), then detect successful escalation (or handle
+        # errors and timeouts).
 
         no_prompt_out = ''
         no_prompt_err = ''
 
         if self._play_context.prompt:
-            '''
-                Several cases are handled for privileges with password
-                * NOPASSWD (tty & no-tty): detect success_key on stdout
-                * without NOPASSWD:
-                  * detect prompt on stdout (tty)
-                  * detect prompt on stderr (no-tty)
-            '''
-
             self._display.debug("Handling privilege escalation password prompt.")
 
-
             fcntl.fcntl(p.stdout, fcntl.F_SETFL, fcntl.fcntl(p.stdout, fcntl.F_GETFL) | os.O_NONBLOCK)
             fcntl.fcntl(p.stderr, fcntl.F_SETFL, fcntl.fcntl(p.stderr, fcntl.F_GETFL) | os.O_NONBLOCK)
 
             become_output = ''
             become_errput = ''
             passprompt = False
+
             while True:
                 self._display.debug('Waiting for Privilege Escalation input')
 
@@ -414,7 +435,6 @@ class Connection(ConnectionBase):
                     become_output += chunk
                     self._display.debug('stdout chunk is: %s' % chunk)
 
-
                 if not chunk:
                     break
                     #raise AnsibleError('Connection closed waiting for privilege escalation password prompt: %s ' % become_output)
@@ -426,15 +446,69 @@ class Connection(ConnectionBase):
                 no_prompt_out = become_output
                 no_prompt_err = become_errput
 
+        # Now we're back to common handling for ssh/scp/sftp. If we have any
+        # data to write into the connection, we do it now. (But we can't use
+        # p.communicate because the ControlMaster may have stdout open too.)
 
-        (returncode, stdout, stderr) = self._communicate(p, stdin, in_data, sudoable=sudoable)
+        fcntl.fcntl(p.stdout, fcntl.F_SETFL, fcntl.fcntl(p.stdout, fcntl.F_GETFL) & ~os.O_NONBLOCK)
+        fcntl.fcntl(p.stderr, fcntl.F_SETFL, fcntl.fcntl(p.stderr, fcntl.F_GETFL) & ~os.O_NONBLOCK)
 
-        self.lock_host_keys(False)
+        if in_data:
+            try:
+                stdin.write(in_data)
+                stdin.close()
+            except:
+                raise AnsibleConnectionFailure('SSH Error: data could not be sent to the remote host. Make sure this host can be reached over ssh')
+
+        # Now we just loop reading stdout/stderr from the process until it
+        # terminates.
+
+        stdout = stderr = ''
+        rpipes = [p.stdout, p.stderr]
+
+        while True:
+            rfd, wfd, efd = select.select(rpipes, [], rpipes, 1)
+
+            # fail early if the become password is wrong
+            if self._play_context.become and sudoable:
+                if self._play_context.become_pass:
+                    self.check_incorrect_password(stdout)
+                elif self.check_password_prompt(stdout):
+                    raise AnsibleError('Missing %s password' % self._play_context.become_method)
+
+            if p.stderr in rfd:
+                dat = os.read(p.stderr.fileno(), 9000)
+                stderr += dat
+                if dat == '':
+                    rpipes.remove(p.stderr)
+            elif p.stdout in rfd:
+                dat = os.read(p.stdout.fileno(), 9000)
+                stdout += dat
+                if dat == '':
+                    rpipes.remove(p.stdout)
+
+            # only break out if no pipes are left to read or
+            # the pipes are completely read and
+            # the process is terminated
+            if (not rpipes or not rfd) and p.poll() is not None:
+                break
+            # No pipes are left to read but process is not yet terminated
+            # Only then it is safe to wait for the process to be finished
+            # NOTE: Actually p.poll() is always None here if rpipes is empty
+            elif not rpipes and p.poll() == None:
+                p.wait()
+                # The process is terminated. Since no pipes to read from are
+                # left, there is no need to call select() again.
+                break
+
+        # close stdin after process is terminated and stdout/stderr are read
+        # completely (see also issue #848)
+        stdin.close()
 
         controlpersisterror = 'Bad configuration option: ControlPersist' in stderr or 'unknown configuration option: ControlPersist' in stderr
 
         if C.HOST_KEY_CHECKING:
-            if ssh_cmd[0] == "sshpass" and p.returncode == 6:
+            if cmd[0] == "sshpass" and p.returncode == 6:
                 raise AnsibleError('Using a SSH password instead of a key is not possible because Host Key checking is enabled and sshpass does not support this.  Please add this host\'s fingerprint to your known_hosts file to manage this host.')
 
         if p.returncode != 0 and controlpersisterror:
@@ -444,88 +518,22 @@ class Connection(ConnectionBase):
         if p.returncode == 255 and in_data:
             raise AnsibleConnectionFailure('SSH Error: data could not be sent to the remote host. Make sure this host can be reached over ssh')
 
-        return (p.returncode, '', no_prompt_out + stdout, no_prompt_err + stderr)
+        return (p.returncode, no_prompt_out+stdout, no_prompt_err+stderr)
 
-    def put_file(self, in_path, out_path):
+    # Utility functions
-        ''' transfer a file from local to remote '''
 
-        super(Connection, self).put_file(in_path, out_path)
+    def _split_args(self, argstring):
-
+        """
-        self._display.vvv("PUT {0} TO {1}".format(in_path, out_path), host=self.host)
+        Takes a string like '-o Foo=1 -o Bar="foo bar"' and returns a
-        if not os.path.exists(in_path):
+        list ['-o', 'Foo=1', '-o', 'Bar=foo bar'] that can be added to
-            raise AnsibleFileNotFound("file or module does not exist: {0}".format(in_path))
+        the argument list. The list will not contain any empty elements.
-        cmd = self._password_cmd()
+        """
-
+        return [x.strip() for x in shlex.split(argstring) if x.strip()]
-        # scp and sftp require square brackets for IPv6 addresses, but
-        # accept them for hostnames and IPv4 addresses too.
-        host = '[%s]' % self.host
-
-        if C.DEFAULT_SCP_IF_SSH:
-            cmd.append('scp')
-            cmd.extend(self._common_args)
-            cmd.extend([in_path, '{0}:{1}'.format(host, pipes.quote(out_path))])
-            indata = None
-        else:
-            cmd.append('sftp')
-            cmd.extend(self._common_args)
-            cmd.append(host)
-            indata = "put {0} {1}\n".format(pipes.quote(in_path), pipes.quote(out_path))
-
-        (p, stdin) = self._run(cmd, indata)
-
-        self._send_password()
-
-        (returncode, stdout, stderr) = self._communicate(p, stdin, indata)
-
-        if returncode != 0:
-            raise AnsibleError("failed to transfer file to {0}:\n{1}\n{2}".format(out_path, stdout, stderr))
-
-    def fetch_file(self, in_path, out_path):
-        ''' fetch a file from remote to local '''
-
-        super(Connection, self).fetch_file(in_path, out_path)
-
-        self._display.vvv("FETCH {0} TO {1}".format(in_path, out_path), host=self.host)
-        cmd = self._password_cmd()
-
-
-        if C.DEFAULT_SCP_IF_SSH:
-            cmd.append('scp')
-            cmd.extend(self._common_args)
-            cmd.extend(['{0}:{1}'.format(self.host, in_path), out_path])
-            indata = None
-        else:
-            cmd.append('sftp')
-            # sftp batch mode allows us to correctly catch failed transfers,
-            # but can be disabled if for some reason the client side doesn't
-            # support the option
-            if C.DEFAULT_SFTP_BATCH_MODE:
-                cmd.append('-b')
-                cmd.append('-')
-            cmd.extend(self._common_args)
-            cmd.append(self.host)
-            indata = "get {0} {1}\n".format(in_path, out_path)
-
-        p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        self._send_password()
-        stdout, stderr = p.communicate(indata)
-
-        if p.returncode != 0:
-            raise AnsibleError("failed to transfer file from {0}:\n{1}\n{2}".format(in_path, stdout, stderr))
-
-    def close(self):
-
-        if self._connected:
-
-            # TODO: reenable once winrm issues are fixed
-            # temporarily disabled as we are forced to currently close connections after every task because of winrm
-            #if and 'ControlMaster' in self._common_args:
-            #    cmd = ['ssh','-O','stop']
-            #    cmd.extend(self._common_args)
-            #    cmd.append(self._play_context.remote_addr)
-
-            #    p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            #    stdout, stderr = p.communicate()
-
-            self._connected = False
 
+    def add_args(self, explanation, args):
+        """
+        Adds the given args to self._command and displays a caller-supplied
+        explanation of why they were added.
+        """
+        self._command += args
+        self._display.vvvvv('SSH: ' + explanation + ': (%s)' % ')('.join(args), host=self._play_context.remote_addr)