rusackas commented on code in PR #40673:
URL: https://github.com/apache/superset/pull/40673#discussion_r3343677277
##########
superset/extensions/ssh.py:
##########
@@ -48,6 +78,68 @@ def build_sqla_url(
port=server.local_bind_port,
)
+ def _verify_host_key(self, ssh_tunnel: "SSHTunnel") -> None:
+ """
+ Opt-in defense-in-depth: verify the SSH server's host key before
opening the
+ tunnel, to resist man-in-the-middle attacks (paramiko's ``Transport``
does no
+ known-hosts checking by default).
+
+ Behavior:
+
+ - If the tunnel declares an expected ``server_host_key``, connect to
the SSH
+ server, read the host key it presents, and compare. On mismatch (or
if the
+ expected key cannot be parsed) raise
+ :class:`SSHTunnelHostKeyVerificationError`.
+ - If no expected key is set and
``SSH_TUNNEL_STRICT_HOST_KEY_CHECKING`` is
+ enabled, fail closed and raise.
+ - If no expected key is set and strict checking is disabled, do
nothing,
+ preserving existing (unverified) behavior.
+ """
+ expected_raw = ssh_tunnel.server_host_key
+
+ if not expected_raw or not expected_raw.strip():
+ if self.strict_host_key_checking:
+ raise SSHTunnelHostKeyVerificationError(
+ message=(
+ "SSH_TUNNEL_STRICT_HOST_KEY_CHECKING is enabled but no
"
+ "expected server host key is configured for this
tunnel."
+ )
+ )
+ return
+
+ try:
+ expected_key = _parse_authorized_key(expected_raw)
+ except ValueError as ex:
+ raise SSHTunnelHostKeyVerificationError(
+ message=f"The configured expected server host key is invalid:
{ex}"
+ ) from ex
+
+ transport = paramiko.Transport(
+ (ssh_tunnel.server_address, ssh_tunnel.server_port)
+ )
Review Comment:
Done — the verification path now builds the socket via
`socket.create_connection((host, port), timeout=sshtunnel.SSH_TIMEOUT)` and
hands that to `paramiko.Transport`, so the TCP connect phase is bounded by the
same timeout rather than the OS-level default. A connect failure surfaces as
`SSHTunnelHostKeyVerificationError`. Pushed in the latest commit.
##########
superset/extensions/ssh.py:
##########
@@ -60,6 +152,10 @@ def create_tunnel(
port = url.port or get_default_port(backend)
if not port:
raise SSHTunnelDatabasePortError()
+
+ # Opt-in host-key verification runs before the tunnel is opened.
+ self._verify_host_key(ssh_tunnel)
Review Comment:
Done — `SSHTunnelHostKeyVerificationError` is now propagated explicitly in
the test-connection command and the create path, and returned as a 400 with its
own message in the test_connection/create/update API handlers, instead of being
flattened into the generic connection-failure text. Pushed in the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
