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

Reorder cipher and key preferences to make more sense #984

Merged
merged 1 commit into from Jun 9, 2017

Conversation

alex
Copy link
Member

@alex alex commented Jun 6, 2017

No description provided.

'ecdsa-sha2-nistp256',
'ecdsa-sha2-nistp384',
'ecdsa-sha2-nistp521',
'ssh-ed25519',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine with me, but this order was chosen because it looks like openssh puts ecdsa first

debug2: host key algorithms: ecdsa-sha2-nistp256-cert-v01@openssh.com,ecdsa-sha2-nistp384-cert-v01@openssh.com,ecdsa-sha2-nistp521-cert-v01@openssh.com,ssh-ed25519-cert-v01@openssh.com,ssh-rsa-cert-v01@openssh.com,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What SSH are you using?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ubuntu-16.04 : OpenSSH_7.2p2 Ubuntu-4ubuntu2.2, OpenSSL 1.0.2g 1 Mar 2016

OS X 10.11 : OpenSSH_6.9p1, LibreSSL 2.1.8

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match what I'm seeing with OpenSSH_7.4p1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, even newer openssh - fair enough

@@ -109,9 +109,9 @@ class Transport(threading.Thread, ClosingContextManager):
'aes192-ctr',
'aes256-ctr',
'aes128-cbc',
'blowfish-cbc',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is another case like md5 - older openssh puts blowfish-cbc here, newer openssh omits it (but includes aes192-cbc, aes256-cbc, and 3des-cbc ... maybe blowfish should go last)

debug2: kex_parse_kexinit: chacha20-poly1305@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com,arcfour256,arcfour128,aes128-cbc,3des-cbc,blowfish-cbc,cast128-cbc,aes192-cbc,aes256-cbc,arcfour,rijndael-cbc@lysator.liu.se

debug2: ciphers stoc: chacha20-poly1305@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com,aes128-cbc,aes192-cbc,aes256-cbc,3des-cbc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AES-192 and AES-256 are definitely preferable to blowfish. Blowfish vs 3des isn't something I care enough to think about.

@bitprophet bitprophet modified the milestones: Next 1.x+2.x bugfix release, Next 2.x-only bugfix release Jun 8, 2017
@bitprophet bitprophet changed the base branch from master to 2.0 June 9, 2017 20:57
@bitprophet bitprophet changed the base branch from 2.0 to master June 9, 2017 20:57
@bitprophet
Copy link
Member

Really gotta stop trying that bloody useless "change bases" feature...will cherry-pick to 2.0 in a sec.

@bitprophet
Copy link
Member

OK, change made to 2.0 and up, re: blowfish. Then just merged into master to get the Ed25519 tweak. Changelogged the former, latter no point since Ed25519 is new in master.

bitprophet added a commit that referenced this pull request Jun 9, 2017
@bitprophet bitprophet merged commit b395444 into paramiko:master Jun 9, 2017
dkhapun pushed a commit to cyberx-labs/paramiko that referenced this pull request Jun 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants