[SSH] Upgrade ssh proxy version and fix bug in which execution crashes if reading ssh log throws an exception

src/ssh/HISTORY.md

@@ -1,5 +1,10 @@
 Release History
 ===============
+2.0.7
+-----
+* Upgrade Azure Arc SSH Proxy Version to latest (1.3.033291)
+* [Bug Fix] Ensure connection will not crash if logs contain non utf-8 characters
+
 2.0.6
 -----
 * Remove msrestazure dependency

src/ssh/azext_ssh/constants.py

@@ -7,7 +7,7 @@
 
 AGENT_MINIMUM_VERSION_MAJOR = 1
 AGENT_MINIMUM_VERSION_MINOR = 31
-CLIENT_PROXY_VERSION = "1.3.026973"
+CLIENT_PROXY_VERSION = "1.3.033291"
 CLIENT_PROXY_MCR_TARGET = "mcr.microsoft.com/azureconnectivity/proxy"
 CLEANUP_TOTAL_TIME_LIMIT_IN_SECONDS = 120
 CLEANUP_TIME_INTERVAL_IN_SECONDS = 10

src/ssh/azext_ssh/ssh_utils.py

@@ -117,7 +117,7 @@ def _check_ssh_logs_for_common_errors(ssh_sub, op_info, delete_cert, delete_keys
     connection_established = False
     t0 = time.time()
     service_config_delay_error = False
-    next_line = ssh_sub.stderr.readline()
+    next_line = _read_ssh_log_lines(ssh_sub)
     while next_line:
         log_list.append(next_line)
         if not next_line.startswith("debug1:") and \
@@ -139,12 +139,25 @@ def _check_ssh_logs_for_common_errors(ssh_sub, op_info, delete_cert, delete_keys
             do_cleanup(delete_keys, delete_cert, op_info.delete_credentials,
                        op_info.cert_file, op_info.private_key_file, op_info.public_key_file)
 
-        next_line = ssh_sub.stderr.readline()
+        next_line = _read_ssh_log_lines(ssh_sub)
 
     ssh_sub.wait()
     return service_config_delay_error
 
 
+def _read_ssh_log_lines(ssh_sub):
+    retries = 0
+    max_retries = 5
+
+    while retries < max_retries:
+        try:
+            return ssh_sub.stderr.readline()
+        except UnicodeDecodeError:
+            retries += 1
+
+    return None
+
+
 def _wait_to_delete_credentials(ssh_sub, op_info, delete_cert, delete_keys):
     # wait for 2 minutes. If the process isn't closed until then, delete credentials.
     if delete_cert or op_info.delete_credentials:

src/ssh/azext_ssh/tests/latest/test_connectivity_utils.py

@@ -55,19 +55,19 @@ def test_get_client_os_unsupported(self, mock_plat):
 
     def test_get_proxy_filename_amd_windows(self):
         name = connectivity_utils._get_proxy_filename('Windows', 'amd64')
-        self.assertEqual(name, 'sshProxy_windows_amd64_1_3_026973.exe')
+        self.assertEqual(name, 'sshProxy_windows_amd64_1_3_033291.exe')
 
     def test_get_proxy_filename_arm_linux(self):
         name = connectivity_utils._get_proxy_filename('Linux', 'arm64')
-        self.assertEqual(name, 'sshProxy_linux_arm64_1_3_026973')
+        self.assertEqual(name, 'sshProxy_linux_arm64_1_3_033291')
 
     def test_get_proxy_filename_arm_Darwin(self):
         name = connectivity_utils._get_proxy_filename('Darwin', 'arm64')
-        self.assertEqual(name, 'sshProxy_darwin_arm64_1_3_026973')
+        self.assertEqual(name, 'sshProxy_darwin_arm64_1_3_033291')
 
     def test_get_proxy_filename_386_linuux(self):
         name = connectivity_utils._get_proxy_filename('Linux', '386')
-        self.assertEqual(name, 'sshProxy_linux_386_1_3_026973')
+        self.assertEqual(name, 'sshProxy_linux_386_1_3_033291')
 
     def test_get_proxy_filename_386_darwin(self):
         with self.assertRaises(azclierror.BadRequestError):
@@ -136,5 +136,5 @@ def test_install_proxy_create_dir(self, mock_check, mock_download, mock_dir, moc
         connectivity_utils.install_client_side_proxy(None)
 
         mock_dir.assert_called_once_with("/dir/proxy", "Failed to create client proxy directory \'/dir/proxy\'.")
-        mock_download.assert_called_once_with("/dir/proxy", "sshProxy_linux_arm64_1_3_026973", "linux", "arm64")
-        mock_check.assert_called_once_with("/dir/proxy", "sshProxy_linux_arm64_1_3_026973")
+        mock_download.assert_called_once_with("/dir/proxy", "sshProxy_linux_arm64_1_3_033291", "linux", "arm64")
+        mock_check.assert_called_once_with("/dir/proxy", "sshProxy_linux_arm64_1_3_033291")

src/ssh/azext_ssh/tests/latest/test_ssh_utils.py

@@ -305,3 +305,95 @@ def test_get_ssh_path_windows_ssh_preinstalled_not_found(self, mock_isfile, mock
         mock_isfile.return_value = False
 
         self.assertRaises(azclierror.UnclassifiedUserFault, ssh_utils.get_ssh_client_path)
+
+    @mock.patch.object(ssh_utils, 'do_cleanup')
+    def test_check_ssh_logs_no_errors(self, mock_cleanup):
+        lines = [
+            "debug1: OpenSSH_8.9, OpenSSL 1.1.1 11 Sep 2018\n",
+            "debug1: some debug info\n",
+            "debug2: more debug info\n",
+            "Authenticated to host\n",
+            "debug1: Entering interactive session.\n",
+        ]
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.side_effect = lines + ['']
+
+        op_info = mock.Mock()
+        op_info.delete_credentials = False
+        op_info.cert_file = "cert"
+        op_info.private_key_file = "priv"
+        op_info.public_key_file = "pub"
+
+        result = ssh_utils._check_ssh_logs_for_common_errors(ssh_sub, op_info, delete_cert=False, delete_keys=False)
+
+        self.assertFalse(result)
+        # Cleanup should be called once when "Entering interactive session." is found
+        mock_cleanup.assert_called_once_with(False, False, False, "cert", "priv", "pub")
+        ssh_sub.wait.assert_called_once()
+
+    @mock.patch.object(ssh_utils, 'do_cleanup')
+    def test_check_ssh_logs_service_config_delay_error(self, mock_cleanup):
+        error_line = ('{"level":"fatal","msg":"sshproxy: error connecting to the address: '
+                      '404 Endpoint does not exist","time":"2024-01-01T00:00:00Z"}\n')
+        lines = [
+            "debug1: OpenSSH_8.9, OpenSSL 1.1.1 11 Sep 2018\n",
+            error_line,
+        ]
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.side_effect = lines + ['']
+
+        op_info = mock.Mock()
+        op_info.delete_credentials = False
+        op_info.cert_file = "cert"
+        op_info.private_key_file = "priv"
+        op_info.public_key_file = "pub"
+
+        result = ssh_utils._check_ssh_logs_for_common_errors(ssh_sub, op_info, delete_cert=False, delete_keys=False)
+
+        self.assertTrue(result)
+        ssh_sub.wait.assert_called_once()
+
+    def test_readline_ssh_logs_returns_line(self):
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.return_value = "some log line\n"
+        result = ssh_utils._read_ssh_log_lines(ssh_sub)
+        self.assertEqual(result, "some log line\n")
+
+    def test_readline_ssh_logs_skips_errors_and_returns_next_line(self):
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.side_effect = [
+            UnicodeDecodeError('utf-8', b'\x80invalid', 0, 1, 'invalid start byte'),
+            UnicodeDecodeError('utf-8', b'\x81invalid', 0, 1, 'invalid start byte'),
+            "valid line\n",
+        ]
+        result = ssh_utils._read_ssh_log_lines(ssh_sub)
+        self.assertEqual(result, "valid line\n")
+        self.assertEqual(ssh_sub.stderr.readline.call_count, 3)
+
+    def test_readline_ssh_logs_returns_empty_string_when_stream_ends(self):
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.return_value = ''
+        result = ssh_utils._read_ssh_log_lines(ssh_sub)
+        self.assertEqual(result, '')
+
+    def test_readline_ssh_logs_returns_none(self):
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.return_value = None
+        result = ssh_utils._read_ssh_log_lines(ssh_sub)
+        self.assertIsNone(result)
+
+    def test_readline_ssh_logs_skips_errors_and_returns_next_line(self):
+        ssh_sub = mock.Mock()
+        ssh_sub.stderr.readline.side_effect = [
+            UnicodeDecodeError('utf-8', b'\x80invalid', 0, 1, 'invalid start byte'),
+            UnicodeDecodeError('utf-8', b'\x81invalid', 0, 1, 'invalid start byte'),
+            UnicodeDecodeError('utf-8', b'\x81invalid', 0, 1, 'invalid start byte'),
+            UnicodeDecodeError('utf-8', b'\x81invalid', 0, 1, 'invalid start byte'),
+            UnicodeDecodeError('utf-8', b'\x81invalid', 0, 1, 'invalid start byte'),
+            "valid line\n",
+        ]
+        result = ssh_utils._read_ssh_log_lines(ssh_sub)
+        self.assertIsNone(result)
+        self.assertEqual(ssh_sub.stderr.readline.call_count, 5)
+
+