cros shell: Disable known_hosts for USB connections.

known_hosts isn't necessary for the USB link since there's no real risk
of a man-in-the-middle attack. Additionally, our USB connection reuses
the same IP address for each device so it triggers a false warning
whenever switching devices.

This CL adds support to remote_access for indicating when a device is
attached over USB, and disables known_hosts automatically in cros shell
in this case.

BUG=brillo:1170
TEST=cbuildbot/run_tests
TEST=`brillo shell --debug` to confirm known_hosts isn't used
TEST=cros shell <ip>
     # modify ~/.ssh/known_hosts to corrupt identity
     cros shell <ip>   # still gives known_hosts warning as expected

Change-Id: If8c95ce2a25d829954bc4a68788d2d4b1511a4d1
Reviewed-on: https://chromium-review.googlesource.com/275355
Trybot-Ready: David Pursell <dpursell@chromium.org>
Reviewed-by: Yiming Chen <yimingc@chromium.org>
Commit-Queue: David Pursell <dpursell@chromium.org>
Tested-by: David Pursell <dpursell@chromium.org>

2 files changed, 19 insertions, 8 deletions

diff --git a/cli/cros/cros_shell.py b/cli/cros/cros_shell.py
index ed2a3f0d5..b5c29f22e 100644
--- a/cli/cros/cros_shell.py
+++ b/cli/cros/cros_shell.py
@@ -81,7 +81,8 @@ Quoting can be tricky; the rules are the same as with ssh:
         help='SSH identify file (private key).')
     parser.add_argument(
         '--no-known-hosts', action='store_false', dest='known_hosts',
-        default=True, help='Do not use a known_hosts file.')
+        default=True, help='Do not use a known_hosts file; always set'
+        ' for USB connections.')
     parser.add_argument(
         'command', nargs=argparse.REMAINDER,
         help='(optional) Command to execute on the device.')
@@ -105,7 +106,10 @@ Quoting can be tricky; the rules are the same as with ssh:
   def _ConnectSettings(self):
     """Generates the correct SSH connect settings based on our state."""
     kwargs = {'NumberOfPasswordPrompts': 2}
-    if self.known_hosts:
+    # USB has no risk of a man-in-the-middle attack so we can turn off
+    # known_hosts for any USB connection.
+    if (self.known_hosts and
+        self.device.connection_type != remote_access.CONNECTION_TYPE_USB):
       # Use the default known_hosts and our current key check setting.
       kwargs['UserKnownHostsFile'] = None
       kwargs['StrictHostKeyChecking'] = self.host_key_checking
diff --git a/cli/cros/cros_shell_unittest.py b/cli/cros/cros_shell_unittest.py
index b9aee7e15..c6b77b0e5 100644
--- a/cli/cros/cros_shell_unittest.py
+++ b/cli/cros/cros_shell_unittest.py
@@ -25,12 +25,6 @@ class MockShellCommand(command_unittest.MockCommand):
   TARGET_CLASS = cros_shell.ShellCommand
   COMMAND = 'shell'
 
-  def __init__(self, *args, **kwargs):
-    command_unittest.MockCommand.__init__(self, *args, **kwargs)
-
-  def Run(self, inst):
-    return command_unittest.MockCommand.Run(self, inst)
-
 
 class ShellTest(cros_test_lib.MockTempDirTestCase,
                 cros_test_lib.OutputTestCase):
@@ -57,6 +51,7 @@ class ShellTest(cros_test_lib.MockTempDirTestCase,
     self.mock_device = self.PatchObject(
         remote_access, 'ChromiumOSDevice', autospec=True).return_value
     self.mock_device.hostname = self.DEVICE_IP
+    self.mock_device.connection_type = None
     self.mock_base_run_command = self.mock_device.BaseRunCommand
     self.mock_base_run_command.return_value = cros_build_lib.CommandResult()
 
@@ -156,3 +151,15 @@ class ShellTest(cros_test_lib.MockTempDirTestCase,
     self.assertFalse(self.mock_prompt.called)
     self.assertEqual(self.mock_base_run_command.call_count, 1)
     self.assertFalse(self.mock_remove_known_host.called)
+
+  def testUsbConnectionDoesNotUseKnownHosts(self):
+    """Tests that known_hosts is disabled by default for USB connections."""
+    self.SetupCommandMock([self.DEVICE_IP])
+    self.mock_device.connection_type = remote_access.CONNECTION_TYPE_USB
+    mock_compile_ssh_connect_settings = self.PatchObject(
+        remote_access, 'CompileSSHConnectSettings', autospec=True)
+
+    self.cmd_mock.inst.Run()
+
+    self.assertNotIn('UserKnownHostsFile',
+                     mock_compile_ssh_connect_settings.call_args[-1])