kexinit - use same kex algorithms as in security_options - #2083
kexinit - use same kex algorithms as in security_options#2083manfred-kaiser wants to merge 1 commit into
Conversation
|
Thanks for the PR! Really appreciate the work on this.
For my reference, is part of your argument here that the sending of the unmodified algo list to the client is a bug in all cases? |
|
Also, are there any reasonably simple tests that could be added around this? Definitely recognize that mocking/mimicking specific external hardware/software is not trivial. |
|
Sorry for the late response 😞
I have not found any information, if the modified or unmodified list should be sent to the clients. During my tests I had no problems with the modified list. I have tested following clients:
|
Implementing tests might be not possible, because there are a lot of ssh implementations and most of them work different. The only way to test this is by using the client. With OpenSSH, Dropbear, PuTTY this might be possible, because they are open source and you can reproduce the key exchange part. The source of MobyXterms secureblackbox ssh implementation is not available and it's not documented why this implementation only works with the modified list. If you have any suggestions how to implement such tests, I will try to write some. |
No, I had figured that substantial automated tests would be quite hard to write for this. I just wanted to raise the question to make sure it'd been considered. Thanks! |
|
I have tested 15 ssh clients (11 different ssh implementations) with the modified algorithm list and they worked without problems. 😄 You can find a list of tested clients in my project: https://github.com/ssh-mitm/ssh-mitm/blob/develop/sshmitm/data/client_info.yml Note: Some clients are based on another client. The list is not complete and I have to test more clients, but I have the problem that some windows clients does not run with Wine (Linux) or are commercial 😞 |
|
I have updated the list of working clients. SSH-MITM uses this list for client fingerprinting and vulnerability detection, but this is also a good compatibility test for paramiko 😉 Now I have tested 17 different ssh implementations There are more than 20 tested clients, but a lot of them are based on PuTTY. Tested SSH implementations:
PuTTY based:
Note: |
|
Thanks for all that work, and for the detailed report, @manfred-kaiser!! |
|
Will you merge this pull request or should I change some things? |
|
I don't have write access to the repo, only triage permissions, so I can't merge. bitprophet is in the middle of a sustained modernization campaign on all of his OSS projects, focused on dropping Python 2 and old 3.x, and pretty much all of them are on a feature/bugfix freeze until that's done. AFAIK, a rework of the key/auth internals (with #387 as kind of the focal point) is the next big thing for paramiko after that modernization, and I would expect this PR to get picked up as part of that. Sorry for the delay, we really do appreciate all the work you've put into this! |
|
This pull request fixes some issues with Dropbear SSH and some proprietary ssh libraries like "Secure Blackbox" (a ssh component for Delphi). The changes in this pull request are well tested against the listed ssh clients (see post above) |
|
This pull request has some code changes in the same place as the fix for terrapin-attack and might be incompatible with the latest paramiko version. I need to update my tool SSH-MITM to the latest paramiko version (ssh-mitm/ssh-mitm#163) and add some workarounds for the mentioned clients in a previous post: #2083 (comment) @bskinn @bitprophet Are you interested in a patch, which makes paramiko compatible with those clients and servers? |
Two related bugs in _send_kex_init when running in server mode: 1. When group-exchange algorithms are filtered out due to missing moduli, the filtered list was stored in a local variable `pkex` but `kex_algos` (which is what gets sent to the client) was never updated. This caused the server to advertise algorithms it cannot actually use, breaking compatibility with clients like Dropbear and MobaXterm (SecureBlackbox). 2. `ext-info-c` was appended unconditionally, potentially duplicating it if it was already present in preferred_kex.
|
I've rebased this PR on the current The previously mentioned incompatibility with the Terrapin fix is not an issue — the Summary of the two fixes:
Both fixes have been running as a monkey-patch workaround in SSH-MITM for several years and were tested against 17+ SSH implementations (see the client list in my previous comment above). @bitprophet @bskinn Would you be able to take another look at this? Note: The CI failure is pre-existing on |
Send modified kex algorithm list to client:
I had problems with some ssh implementations like MobaXterm (ssh-mitm/ssh-mitm#95) wich is based on SecureBlackbox.
When testing MobaXterm with paramiko versions < 2.9 it works without any problems. Due to the changes introduced in paramiko 2.10 which adds compatibility for OpenSSH 8.9 and the deprecated ssh-rsa algorithms, connections from MobaXterm wont work anymore.
After some tests with the kex algorithms I found out, that the algorithm list is modified and set for the security options. The unmodified list is sent to the client.
The list sent to the client includes "diffie-hellman-group-exchange-sha1" and "diffie-hellman-group-exchange-sha256" but not the list which is stored in the securiry options. (see https://datatracker.ietf.org/doc/html/rfc4419)
If MobaXterm wants to connect, the server raises an error because of an unsupported algorithm.
I have tested the changes with against the OpenSSH 8.9 client. Using this client the "diffie-hellman-group-exchange-sha*" algorithms are used.
Using the modified list with MobaXterm does not include the "diffie-hellman-group-exchange-sha*" algorithms and the client can connect without any problems.
Only add "ext-info-c" if not present
The second change checks if "ext-info-c" is already in the kex_algos list and only adds it, if it is not present: https://datatracker.ietf.org/doc/html/draft-ssh-ext-info-04#section-2.1