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

ensure ed25519 password is bytes #1051

Merged
merged 2 commits into from
Nov 29, 2017
Merged

Conversation

ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Sep 4, 2017

fixes #1039

Side Note: I just noticed that this file imports "six", even though that's not explicitly a requirement in setup.py (but it's a requirement of cryptography)

@bitprophet
Copy link
Member

That snuck past me too (the six import). Since it's now in our dependency tree it might be nice to nuke a bunch of py3compat.py in favor of Just Using Six™, sometime. Side thought.

Actual change discussion / notes to future self: seems ok, I verified your statement from #1039 re: other key types implicitly performing b(); it happens in util.generate_key_bytes, which is called in PKey._read_private_key, which is called by all subclasses.

The reason that generate_key_bytes is not utilized by Ed25519Key seems to be that its newer key format always omits the 'headers' used by the older formats, including the 'is encrypted' header which is what triggers seeking/using of password. Ed25519Key thus always runs _read_private_key as if the key is unencrypted, then performs its own decryption in a subsequent step - the one @ploxiln is modifying here.

@bitprophet bitprophet added this to the 2.2.1 milestone Sep 5, 2017
@ploxiln ploxiln changed the base branch from master to 2.2 September 5, 2017 19:14
@ploxiln
Copy link
Contributor Author

ploxiln commented Nov 27, 2017

ping.
should I rebase?

@bitprophet
Copy link
Member

No idea why I did all that sleuthing and then didn't merge. Doing it now. Includes adding...dun dun dun...a TEST! Which I have proven yields the error from #1039 when unpatched, under Python 3.

@bitprophet bitprophet merged commit eec27bd into paramiko:2.2 Nov 29, 2017
bitprophet added a commit that referenced this pull request Nov 29, 2017
@bsholdice
Copy link

sorry to dig up an old issue, but i've arrived here after too many hours of troubleshooting my failing ansible play. this fix solved my paramiko ("Unicode-objects must be encoded before hashing") issue. Im not good at git but it looks like this fix was in v2.2, yet I dont see the fixes in the version 2.4 that I am using. what am I missing?

@ploxiln
Copy link
Contributor Author

ploxiln commented Feb 27, 2018

This was merged into the branches for v2.2 v2.3 v2.4, but there have been no releases made since then (v2.4.0 was released before this was merged).

@bitprophet
Copy link
Member

Yea that's my bad, was expecting to shovel some more bugfixes and then never did. Gonna pop something out this week/weekend hopefully though.

@ploxiln ploxiln deleted the ed25519_pass_b branch March 2, 2019 22:46
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.

3 participants