SSH Only Accepts RSA Keys

metropolis-ameer commented 1 year ago

SSH only accepts RSA keys. Using ed25519 key fails with Error unpack requires 4 bytes.

How to reproduce the bug

Generate ed25519 key. ssh-keygen -t ed25519 -C "your_email@example.com"

Create new database connection in Superset UI.

Use the key with an SSH connection.

Expected results

Connection Success

Actual results

Error: unpack requires 4 bytes.

Screenshots

Environment

browser type and version: Chrome
superset version: 2.1.0
python version: python 3.8
any feature flags active: SSH_TUNNELING

Checklist

Make sure to follow these steps before submitting your issue - thank you!

[x ] I have checked the superset logs for python stacktraces and included it here as text if there are any.
[ x] I have reproduced the issue with at least the latest released version of superset.
[ x] I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

Looking at like 64 of ssh.py I see I referencing RSAKey.from_private_key.

        if ssh_tunnel.password:
            params["ssh_password"] = ssh_tunnel.password
        elif ssh_tunnel.private_key:
            private_key_file = StringIO(ssh_tunnel.private_key)
            private_key = RSAKey.from_private_key(
                private_key_file, ssh_tunnel.private_key_password
            )

I don't know enough about python or SSH to fix this myself unfortunately as how do you identify a key type from a pasted value? Is this something where you try each type and hope one succeeds or provide a dropdown to identify the key type?

bertrand-pledge-io commented 1 year ago

Can confirm the issue.

Screenshot 2023-09-05 at 19 47 04

rusackas commented 5 months ago

A couple of questions: • Is this still an issue in Superset 3.x or newer? • Is there documentation saying what forms of key Superset does or doesn't take (i.e. could a docs change clear this up)? • Is this a bug, or a feature request? I'm on the fence here :)

CC @eschutho

metropolis-ameer commented 5 months ago

Answers:

It looks like it's still an issue based on what I see in the code, but don't know for certain as I haven't upgraded.
I wasn't aware of it in the documentation, I just ran into the issue and jumped into the code to find out why.
To me it is a bug because from the front end perspective, it accepts an SSH key, but doesn't state the RSA variety (again, based on when I last looked at this in SuperSet 2)

Answers aside, it looks like it could be a relatively easy fix based on what I see here: https://github.com/apache/superset/blob/master/superset/extensions/ssh.py#L68

Plus the fact that paramiko.RSAKey.from_private_key_file reads RSA keys paramiko.Ed25519Key.from_private_key_file reads ED25519 keys So many another elif based on the key type?

arrowcircle commented 4 months ago

This thing still exists in 4.0.0 and annoys a lot. Adding rsa key is working, but modern keys don't work

apache / superset