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

only rsa host key checked #865

Closed
ploxiln opened this issue Dec 16, 2016 · 4 comments
Closed

only rsa host key checked #865

ploxiln opened this issue Dec 16, 2016 · 4 comments

Comments

@ploxiln
Copy link
Contributor

ploxiln commented Dec 16, 2016

I'm testing with Fabric==1.13.1, paramiko==2.1.1, cryptography==1.7.1

The default ssh server setup on ubuntu 16.04 (and probably most recent versions of ubuntu and debian) is to generate one of each kind of host key. The default sshd_config contains this:

Protocol 2
# HostKeys for protocol version 2
HostKey /etc/ssh/ssh_host_rsa_key
HostKey /etc/ssh/ssh_host_dsa_key
HostKey /etc/ssh/ssh_host_ecdsa_key
HostKey /etc/ssh/ssh_host_ed25519_key

When using the openssh client (6.9p1 on MacOS 10.11), it chooses the ecdsa key to put in known_hosts, and on later invocations it checks all the host keys the server presents to find the one of the appropriate type to check.

But paramiko seems to only look at the rsa one, sees that it's not the same type as what's in known_hosts, and so ignores it. Thus paramiko never notices when the host key changes.

$ ssh testdeploy03.ec2.st-av.net
...
@    WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!     @
...
Someone could be eavesdropping on you right now (man-in-the-middle attack)!
It is also possible that a host key has just been changed.
The fingerprint for the ECDSA key sent by the remote host is
SHA256:aB6l8o7WhWdJ02tPavyPBGTFgqe1VaqwuLVfJttiQ/c.
...
Offending ECDSA key in /Users/pierce/.ssh/known_hosts:19
ECDSA host key for testdeploy03.ec2.st-av.net has changed and you have requested strict checking.
Host key verification failed.
$ fab -H testdeploy03.ec2.st-av.net basictask
[testdeploy03.ec2.st-av.net] Executing task 'basictask'
[testdeploy03.ec2.st-av.net] run: hostname && echo hi there
[testdeploy03.ec2.st-av.net] out: testdeploy03.ec2.st-av.net
[testdeploy03.ec2.st-av.net] out: hi there
...
Disconnecting from testdeploy03.ec2.st-av.net... done.

But if I put an incorrect rsa key in known_hosts, paramiko correctly notices that the host key is wrong. Or, if I put an incorrect ecdsa key in known_hosts, and change sshd_config to only offer the ecdsa key, paramiko correctly notices that the host key is wrong.

$ fab -H testdeploy03.ec2.st-av.net basictask
[testdeploy03.ec2.st-av.net] Executing task 'basictask'
[testdeploy03.ec2.st-av.net] run: hostname && echo hi there

Fatal error: Host key for testdeploy03.ec2.st-av.net did not match pre-existing key! Server's key was changed recently, or possible man-in-the-middle attack.

Underlying exception:
    ('testdeploy03.ec2.st-av.net', <paramiko.ecdsakey.ECDSAKey object at 0x10c805310>, <paramiko.ecdsakey.ECDSAKey object at 0x10c7fb7d0>)

Aborting.

This also works to hide the problem pointed out in #794 - if the ecdsa-sha2-nistp384/521 key is not the only host key, if there is also a dsa or rsa host key in sshd_config, then paramiko will use the rsa or dsa host key and will seem to work. (But if you had the ecdsa key in known_hosts, it's not being checked.)

(As a final detail, recent openssh server and client will refuse to actually use the dsa key, I think it's obsolete.)

@bitprophet
Copy link
Member

May fall under #387 kinda-sorta, deserves a link at least. I also feel like we've seen this reported before but I assume you already searched :) thanks!

@ploxiln
Copy link
Contributor Author

ploxiln commented Dec 16, 2016

I admittedly only searched "ecdsa", and also didn't fully understand the situation until halfway through writing the report (I initially thought ecdsa keys were never checked).

By the way, it looks like you may have forgotten about #794, which it looks like you intended to apply the change from (in a linked gist, only two lines). That's very understandable, as there are many open issues and pull requests that have a accumulated over a long time. I would humbly suggest leaning more towards closing issues, and especially pull requests, when in doubt. Even this one, if you think #387 and some other issue covers it ;)

@bitprophet
Copy link
Member

Rule of thumb is if it's got a milestone assigned, I haven't forgotten, even if I have had to punt it a few times :) and #794 is in the "next feature release" milestone. The previous release cycle was me trying to make up for lost time so I only got through about half of what was assigned; the rest I'll be trying to knock out in the next month or so (holiday travel notwithstanding).

Overall, I am way overdue for a mass culling of tickets, for sure...something I want to do before I put out a much ballyhooed 2.0 of another project :)

@ploxiln
Copy link
Contributor Author

ploxiln commented Aug 19, 2019

fixed long ago

@ploxiln ploxiln closed this as completed Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants