rusackas opened a new pull request, #40673:
URL: https://github.com/apache/superset/pull/40673

   ### SUMMARY
   
   This is an opt-in, defense-in-depth hardening feature for SSH tunnels. It is 
**not** an undisclosed security fix — it adds a capability operators can choose 
to enable. Per the project's `SECURITY.md`, host-key verification / 
MITM-resistance is treated as a public hardening topic, so this is described 
plainly.
   
   **Problem:** `SSHManager.create_tunnel` (`superset/extensions/ssh.py`) calls 
`sshtunnel.open_tunnel(**params)` with no server host-key verification. 
paramiko's `Transport` performs no known-hosts checking by default, so the SSH 
server's identity is not verified when a tunnel is opened. An attacker 
positioned between Superset and the SSH server could present their own host key 
(a man-in-the-middle).
   
   **Change:** Add opt-in host-key pinning so operators can verify the SSH 
server's host key and reject mismatches. Default behavior is unchanged — 
existing tunnels keep working with no configuration.
   
   What's included:
   
   - **Model + migration:** new nullable `server_host_key` (`Text`) column on 
the `SSHTunnel` model storing the expected host key in authorized-key form 
(e.g. `ssh-ed25519 AAAA...`). It is a public key, so it is stored in 
**plaintext** (not `EncryptedType`). Alembic migration `a1b2c3d4e5f6` is off 
the current head `33d7e0e21daa`, nullable, using 
`superset.migrations.shared.utils` helpers.
   - **Config flag:** `SSH_TUNNEL_STRICT_HOST_KEY_CHECKING: bool = False`. When 
`True`, a tunnel without `server_host_key` is rejected (fail-closed). When 
`False` (default), an unset key preserves current behavior; a set key is still 
verified.
   - **Verification:** `SSHManager._verify_host_key(ssh_tunnel)` runs 
**before** `sshtunnel.open_tunnel`. When `server_host_key` is set it opens a 
short `paramiko.Transport((server_address, server_port))`, calls 
`.start_client()`, reads `transport.get_remote_server_key()`, parses the stored 
expected key via `paramiko.PKey.from_type_string(key_type, base64_decode(key))` 
(robust to whitespace and the optional trailing comment field), compares with 
`==`, and closes the transport. On mismatch (or a strict-flag violation, or an 
unparseable expected key) it raises the new `SSHTunnelHostKeyVerificationError`.
   - **Schema/API/DAO:** `server_host_key` is exposed in the SSH tunnel 
POST/PUT schema (`DatabaseSSHTunnel`) and threads through create/update via 
`daos/database.py` `SSHTunnel(**attrs)`. It is not sensitive, so it is not 
masked, and it is returned in the tunnel `data` payload in cleartext.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A (backend-only).
   
   ### TESTING INSTRUCTIONS
   
   Unit tests (mocking `paramiko.Transport`) cover: matching key passes; 
mismatch raises; unset + non-strict skips (back-compat); unset + strict raises; 
whitespace/comment-tolerant match; invalid expected key raises; schema 
round-trip of `server_host_key`.
   
   ```bash
   pytest tests/unit_tests/extensions/ssh_test.py
   ```
   
   Manual runbook:
   
   1. Capture the SSH server's host key out-of-band, e.g. `ssh-keyscan -t 
ed25519 ssh.example.com`.
   2. Set that value on the tunnel's `server_host_key` (via the database / SSH 
tunnel API payload `ssh_tunnel.server_host_key`).
   3. Open a connection through the tunnel — verification runs before the 
tunnel opens; a mismatch is rejected with `SSHTunnelHostKeyVerificationError`.
   4. Optionally set `SSH_TUNNEL_STRICT_HOST_KEY_CHECKING = True` in 
`superset_config.py` to require an expected host key on every tunnel.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [x] Required feature flags: none (opt-in via 
`SSH_TUNNEL_STRICT_HOST_KEY_CHECKING` config flag, default off)
   - [ ] Changes UI
   - [x] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [x] Migration is atomic, supports rollback & is backwards-compatible
     - [x] Confirm DB migration upgrade and downgrade tested
     - [x] Runtime estimates and downtime expectations provided: single 
nullable `ADD COLUMN` on `ssh_tunnels` (a small table); effectively 
instantaneous, no downtime.
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   Back-compat notes: the column is nullable and the flag defaults off, so 
existing tunnels are unaffected. Verification only happens when a tunnel 
declares `server_host_key`, or when strict checking is explicitly enabled.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to