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

transport: _key_info for ecdsa-sha2-nistp384 and -nistp521 #981

Merged
merged 3 commits into from Jun 6, 2017

Conversation

@ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Jun 5, 2017

To support host keys of these key types, which are already in _preferred_keys! This fixes the bug pointed out in #900 where the security options setter would fail with the list of key types the getter returned. So you couldn't get key_types, re-order or trim the list, and then set that list, because it would contain an un-supported type (even though you didn't add any).

I'm splitting this out from #911 because I think this fix is appropriate for release branches of paramiko. (If this is approved, I'll backport it to 1.8 as well).

ensures all defaults key/cipher/digest etc types are supported
@ploxiln ploxiln force-pushed the ploxiln:ecdsa_key_info branch from 53223f1 to d510b1a Jun 5, 2017
ploxiln added 2 commits Jun 5, 2017
To support host keys of these key types,
which are already in _preferred_keys!
@bitprophet bitprophet added this to the 2.2 milestone Jun 6, 2017
@bitprophet bitprophet merged commit 5f454de into paramiko:2.0 Jun 6, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
bitprophet added a commit that referenced this pull request Jun 6, 2017
@ploxiln
Copy link
Contributor Author

@ploxiln ploxiln commented Jun 6, 2017

I confirmed that there's no point in backporting this to 1.18 - it seems to have multiple other problems with ecdsa-sha2-nistp384/521 (but does work with ecdsa-sha2-nistp256 as-is) (if that's the only key type offered by the remote host)

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 6, 2017

@ploxiln yea I'm largely over the idea of backporting anything that's not a) super high priority or b) super trivial, patchwise. The 1.x-2.x gap is only going to grow and make it harder over time, and given the security implications of PyCrypto, 1.x would inevitably become increasingly insecure even with frequent backports. Also, simply, prioritization is a thing. I'd like to prioritize fixing longstanding pain points over having a 100% solid backport story, when the latter is difficult.

@ploxiln
Copy link
Contributor Author

@ploxiln ploxiln commented Jun 6, 2017

Agreed, I thought this was a trivial bugfix, but it turns out not to be for 1.18.

The real "bug" is key types in _preferred_keys but not in _key_info, and 1.18 actually only has "ecdsa-sha2-nistp256" in _preferred_keys, so it doesn't have the "bug" anyway. So we're all good :)

@ploxiln ploxiln deleted the ploxiln:ecdsa_key_info branch Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants