Fix info leakage in Netmiko connection errors

Sanitize error messages generated by Netmiko on the event of a
connection failure and redact sensitive information before it's
propagated.

Currenly, when a connection to a network device fails, sensitive
information such as IP address, username, etc, are logged alongise the
error message.

Closes-Bug: #2100566
Change-Id: I124d44f161e3116ec2588617ed98d559f6fe3b1f

Author: Afonne-CID

networking_generic_switch/devices/netmiko_devices/__init__.py

@@ -26,6 +26,7 @@
 import tenacity
 from tooz import coordination
 
+from networking_generic_switch._i18n import _
 from networking_generic_switch import batching
 from networking_generic_switch import devices
 from networking_generic_switch.devices import utils as device_utils
@@ -153,10 +154,13 @@ def __init__(self, device_cfg, *args, **kwargs):
         self.batch_cmds = None
         if self._batch_requests():
             if not CONF.ngs_coordination.backend_url:
-                raise exc.GenericSwitchNetmikoConfigError(
-                    config=device_utils.sanitise_config(self.config),
-                    error="ngs_batch_requests is true but [ngs_coordination] "
-                          "backend_url is not provided")
+                error = ("ngs_batch_requests is true but [ngs_coordination] "
+                         "backend_url is not provided")
+                LOG.error(
+                    _("%(device)s, error: %(error)s"),
+                    {'device': device_utils.sanitise_config(self.config),
+                     'error': error})
+                raise exc.GenericSwitchNetmikoConfigError()
             # NOTE: we skip the lock if we are batching requests
             self.locker = None
             switch_name = self.lock_kwargs['locks_prefix']
@@ -221,13 +225,19 @@ def _create_connection():
         try:
             net_connect = _create_connection()
         except tenacity.RetryError as e:
-            LOG.error("Reached maximum SSH connection attempts, not retrying")
-            raise exc.GenericSwitchNetmikoConnectError(
-                config=device_utils.sanitise_config(self.config), error=e)
+            LOG.error(
+                _("Reached maximum SSH connection attempts, not retrying "
+                  "for device: %(device)s, error: %(error)s"), {
+                      'device': device_utils.sanitise_config(self.config),
+                      'error': e})
+            raise exc.GenericSwitchNetmikoConnectError()
         except Exception as e:
-            LOG.error("Unexpected exception during SSH connection")
-            raise exc.GenericSwitchNetmikoConnectError(
-                config=device_utils.sanitise_config(self.config), error=e)
+            LOG.error(
+                _("Unexpected exception during SSH connection "
+                  "to device: %(device)s, error: %(error)s"), {
+                      'device': device_utils.sanitise_config(self.config),
+                      'error': e})
+            raise exc.GenericSwitchNetmikoConnectError()
 
         # Now yield the connection to the caller.
         with net_connect:
@@ -257,8 +267,12 @@ def _send_commands_to_device(self, cmd_set):
             # module.
             raise
         except Exception as e:
-            raise exc.GenericSwitchNetmikoConnectError(
-                config=device_utils.sanitise_config(self.config), error=e)
+            LOG.error(
+                _("Error sending commands to device: %(device)s, "
+                  "error: %(error)s"), {
+                      'device': device_utils.sanitise_config(self.config),
+                      'error': e})
+            raise exc.GenericSwitchNetmikoConnectError()
 
         LOG.debug(output)
         return output
@@ -488,12 +502,14 @@ def check_output(self, output, operation):
 
         for pattern in self.ERROR_MSG_PATTERNS:
             if pattern.search(output):
-                msg = ("Found invalid configuration in device response. "
-                       "Operation: %(operation)s. Output: %(output)s" %
-                       {'operation': operation, 'output': output})
-                raise exc.GenericSwitchNetmikoConfigError(
-                    config=device_utils.sanitise_config(self.config),
-                    error=msg)
+                LOG.error(
+                    _("Found invalid configuration in device response. "
+                      "Operation: %(operation)s. Output: %(output)s. "
+                      "Device: %(device)s"), {
+                          'operation': operation, 'output': output,
+                          'device': device_utils.sanitise_config(self.config)
+                    })
+                raise exc.GenericSwitchNetmikoConfigError()
 
     def add_subports_on_trunk(self, binding_profile, port_id, subports):
         """Allow subports on trunk

networking_generic_switch/devices/netmiko_devices/juniper.py

@@ -15,6 +15,7 @@
 from oslo_log import log as logging
 import tenacity
 
+from networking_generic_switch._i18n import _
 from networking_generic_switch.devices import netmiko_devices
 from networking_generic_switch.devices import utils as device_utils
 from networking_generic_switch import exceptions as exc
@@ -172,20 +173,26 @@ def commit():
         try:
             commit()
         except DBLocked as e:
-            msg = ("Reached timeout waiting for switch configuration DB lock. "
-                   "Configuration might not be committed. Error: %s" % str(e))
+            msg = _("Reached timeout waiting for switch configuration "
+                    "DB lock. Configuration might not be committed. "
+                    "Device: %(device)s, error: %(error)s") % {
+                        'device': device_utils.sanitise_config(self.config),
+                        'error': e}
             LOG.error(msg)
-            raise exc.GenericSwitchNetmikoConfigError(
-                config=device_utils.sanitise_config(self.config), error=msg)
+            raise exc.GenericSwitchNetmikoConfigError()
         except (WarningStmtNotExist, WarningStmtExists) as e:
-            msg = ("Reached timeout while attempting to apply configuration. "
-                   "This is likely to be caused by multiple sessions "
-                   "configuring the device concurrently. Error: %s" % str(e))
+            msg = _("Reached timeout while attempting to apply "
+                    "configuration. This is likely to be caused by multiple "
+                    "sessions configuring the device concurrently. "
+                    "Device: %(device)s, error: %(error)s") % {
+                        'device': device_utils.sanitise_config(self.config),
+                        'error': e}
             LOG.error(msg)
-            raise exc.GenericSwitchNetmikoConfigError(
-                config=device_utils.sanitise_config(self.config), error=msg)
+            raise exc.GenericSwitchNetmikoConfigError()
         except ValueError as e:
-            msg = "Failed to commit configuration: %s" % e
+            msg = _("Failed to commit configuration: Device: %(device)s, "
+                    "error: %(error)s") % {
+                        'device': device_utils.sanitise_config(self.config),
+                        'error': e}
             LOG.error(msg)
-            raise exc.GenericSwitchNetmikoConfigError(
-                config=device_utils.sanitise_config(self.config), error=msg)
+            raise exc.GenericSwitchNetmikoConfigError()

networking_generic_switch/devices/netmiko_devices/nokia.py

@@ -14,6 +14,7 @@
 
 from oslo_log import log as logging
 
+from networking_generic_switch._i18n import _
 from networking_generic_switch.devices import netmiko_devices
 from networking_generic_switch.devices import utils as device_utils
 from networking_generic_switch import exceptions as exc
@@ -81,9 +82,10 @@ def send_commands_to_device(self, cmd_set):
             # module.
             raise
         except Exception as e:
-            raise exc.GenericSwitchNetmikoConnectError(
-                config=device_utils.sanitise_config(self.config), error=e
-            )
+            LOG.error(_("Device: %(device)s, error: %(error)s"), {
+                'device': device_utils.sanitise_config(self.config),
+                'error': e})
+            raise exc.GenericSwitchNetmikoConnectError()
 
         LOG.debug(output)
         return output

networking_generic_switch/devices/utils.py

@@ -46,7 +46,8 @@ def sanitise_config(config):
     :param config: a configuration dict to sanitise.
     :returns: a copy of the configuration, with sensitive fields removed.
     """
-    sanitised_fields = {"password"}
+    sanitised_fields = {"password", "ip", "device_type", "username",
+                        "session_log"}
     return {
         key: "******" if key in sanitised_fields else value
         for key, value in config.items()

networking_generic_switch/exceptions.py

@@ -44,11 +44,13 @@ class GenericSwitchNetmikoNotSupported(GenericSwitchException):
 
 
 class GenericSwitchNetmikoConnectError(GenericSwitchException):
-    message = _("Netmiko connection error: %(config)s, error: %(error)s")
+    message = _("Failed to connect to Netmiko switch. "
+                "Please contact your administrator.")
 
 
 class GenericSwitchNetmikoConfigError(GenericSwitchException):
-    message = _("Netmiko configuration error: %(config)s, error: %(error)s")
+    message = _("Netmiko switch configuration operation failed. "
+                "Please contact your administrator.")
 
 
 class GenericSwitchBatchError(GenericSwitchException):

networking_generic_switch/tests/unit/devices/test_utils.py

@@ -60,7 +60,9 @@ def test_get_switch_device_fallback_to_switch_info(self):
             ngs_mac_address='11:22:33:44:55:77'))
 
     def test_sanitise_config(self):
-        config = {'username': 'fake-user', 'password': 'fake-password'}
+        config = {'username': 'fake-user', 'password': 'fake-password',
+                  'ip': '123.456.789', 'session_log': '/some/path/here',
+                  "device_type": "my_device"}
         result = device_utils.sanitise_config(config)
-        expected = {'username': 'fake-user', 'password': '******'}
+        expected = {k: '******' for k in config}
         self.assertEqual(expected, result)

networking_generic_switch/tests/unit/netmiko/test_dell.py

@@ -565,9 +565,8 @@ def test_check_output(self):
         self.switch.check_output('fake output', 'fake op')
 
     def _test_check_output_error(self, output):
-        msg = ("Found invalid configuration in device response. Operation: "
-               "fake op. Output: %s" % output)
-        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg,
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
+                               "switch configuration operation failed",
                                self.switch.check_output, output, 'fake op')
 
     def test_check_output_incomplete_command(self):

networking_generic_switch/tests/unit/netmiko/test_juniper.py

@@ -177,7 +177,7 @@ def test_save_configuration_db_locked_timeout(self, m_stop, m_wait):
             "Commit failed with the following errors:\n\n{0}".format(output))
 
         self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
-                               "Reached timeout waiting for",
+                               "switch configuration operation failed",
                                self.switch.save_configuration,
                                mock_connection)
         self.assertGreater(mock_connection.commit.call_count, 1)
@@ -227,7 +227,7 @@ def test_save_configuration_warn_already_exists_timeout(
             "Commit failed with the following errors:\n\n{0}".format(output))
 
         self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
-                               "Reached timeout while attempting",
+                               "switch configuration operation failed",
                                self.switch.save_configuration,
                                mock_connection)
         self.assertGreater(mock_connection.commit.call_count, 1)
@@ -277,7 +277,7 @@ def test_save_configuration_warn_does_not_exist_timeout(
             "Commit failed with the following errors:\n\n{0}".format(output))
 
         self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
-                               "Reached timeout while attempting",
+                               "switch configuration operation failed",
                                self.switch.save_configuration,
                                mock_connection)
         self.assertGreater(mock_connection.commit.call_count, 1)
@@ -299,7 +299,7 @@ def test_save_configuration_error(self):
             "Commit failed with the following errors:\n\n{0}".format(output))
 
         self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
-                               "Failed to commit configuration",
+                               "switch configuration operation failed",
                                self.switch.save_configuration,
                                mock_connection)
         mock_connection.commit.assert_called_once_with()

networking_generic_switch/tests/unit/netmiko/test_netmiko_base.py

@@ -54,7 +54,7 @@ def test_batch(self):
 
     def test_batch_missing_backend_url(self):
         self.assertRaisesRegex(
-            Exception, "backend_url",
+            Exception, "switch configuration operation failed",
             self._make_switch_device, {'ngs_batch_requests': True})
 
     @mock.patch('networking_generic_switch.devices.netmiko_devices.'
@@ -445,7 +445,6 @@ def test_check_output_error(self):
 fake switch command
 fake error message
 """
-        msg = ("Found invalid configuration in device response. Operation: "
-               "fake op. Output: %s" % output)
-        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError, msg,
+        self.assertRaisesRegex(exc.GenericSwitchNetmikoConfigError,
+                               "switch configuration operation failed",
                                self.switch.check_output, output, 'fake op')

releasenotes/notes/fix-netmiko-info-leakage-423c4c59b924c06f.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes a security vulnerability where sensitive information (IP addresses,
+    usernames, hostname and file paths) were leaked in error messages when a
+    connection to a network device fails. Error messages are now properly
+    sanitized to redact these sensitive details while preserving meaningful
+    information for troubleshooting.