Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Transport cipher/algorithm choice lists fully user-configurable #2054

Open
bitprophet opened this issue May 13, 2022 · 6 comments
Open

Comments

@bitprophet
Copy link
Member

bitprophet commented May 13, 2022

This is an age old problem: users need ability to shuffle the order of, or flat out remove, certain options for kex, hostkey checks, pubkey auth offering, and so forth. This isn't currently possible without editing your Paramiko or doing monkeypatching on your Transport class or instance. A weak bone was thrown to this with disabled_algorithms recently but it's just a mediocre band-aid by all accounts.

Transport needs updating so that these lists are easy to override/modify/etc:

  • Minimum: straight up overwriting w/o monkeypatching, which allows most of the below, if not super user friendly (outside of the cases where one truly wants a very small list of supported algos)
  • Nicer: easy/ier API for removing problematic algorithms, eg "drop Blowfish so Cryptography shuts up (n.b. this is actually going to be moot soon but a good example of the issue)" or "drop SHA1-based stuff to force use of SHA2". disabled_algorithms allows this but is a bit clunky due to use of dicts. Maybe we want stuff like Transport.remove_algorithms() or w/e.
  • Maybe: allow shuffling-without-removal, eg shift undesirable algorithms lower down the list. I don't see when one would want this over straight removal or overwriting though.
  • Probably smart: remove SecurityOptions which is an old and crummy attempt at this same problem class. It's actually made things worse by being copypasta that then rots differently from the rest of the code! If we're putting this out as a backwards incompat fashion then such a deletion would go well. Otherwise, deprecate.
  • Extra credit: build in some easy way to bridge SSHConfig settings for these, though that's a higher level problem yet unsolved. Probably best to start with one of the other bullet points + Fabric consuming the new API instead, since that already works.

Related:

@bskinn
Copy link
Contributor

bskinn commented May 13, 2022

Believe this also covers #2049?

@nemesifier
Copy link

Believe this also covers #2049?

Sounds to me like it does.

@last-partizan
Copy link

I've came here after reading #1961, and instead of creating new issues i'm adding comment here.

If anyone can identify legit issues with the new feature that are not fixable by using disabled_algorithms (or pinning to 2.8.x until one's servers are upgraded - absolutely an acceptable solution!)

We having many junos devices in our network, some of them new, some of them old. Not all can be upgraded. And we're using py-junos-eznc with ssh transport to comminicate with those devices. It uses netconf which uses paramiko.

After updating to latest paramiko we can't connect to some junos devices. For now we're pinned paramiko to 2.8, but please, consider using some fallback algorithm to allow seamlessly connecting to old hardware.

paramiko.transport.transport._log: Server did not send a server-sig-algs list; defaulting to our first preferred algo ('rsa-sha2-512')

I think paramiko should not default to using first algorithm and fail, but try some other until it works.

chiefnoah pushed a commit to chiefnoah/paramiko that referenced this issue Oct 7, 2022
Adds a transport_factory argument to `SSHClient.connect` that allows you
to dynamically generate a Transport instance without, and therefore
modify inner connection parameters before a connection gets established.

This should address some of the issues in paramiko#2054 with minial changes to
the API and no changes to Transport while allowing for arbitrary control
over Transports API.
@chet-manley
Copy link

Until this issue is resolved, I have a question related to #2049
Is it not possible to fallback through each of the client's preferred pubkeys, and only raise if they all fail? I have a lot of servers I need to connect to, none of which I have the ability to modify in any meaningful way (like upgrading old OpenSSH packages). My problem right now, is some of the older servers use versions of OpenSSH that do not send server-sig-algs, and only support ssh-rsa. Blanket disabling pubkeys rsa-sha2-256 & rsa-sha2-512 is not an option, since the newer servers do return server-sig-algs, and only support SHA2 algos.

@bskinn
Copy link
Contributor

bskinn commented Oct 14, 2022

Is it not possible to fallback through each of the client's preferred pubkeys, and only raise if they all fail?

This seems like something that would be good to consider as part of the key/auth overhaul of #387 &c., @bitprophet.

From what I understand of the current key/auth implementation, @chet-manley, I suspect this isn't possible right now. Hopefully in the future.

@chet-manley
Copy link

I'm currently working around it by try..excepting AuthenticationException on connect, and retrying with SHA2 disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants