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

Corrected the length of the salt for private key encryption #346

Merged
merged 1 commit into from Sep 8, 2014

Conversation

@eXenon
Copy link
Contributor

@eXenon eXenon commented Jun 18, 2014

This would corrupt the private key file whenever the DES cipher would get selected.
Apparently, the list is not ordered in the same way on every system. It could also be a good idea to buffer the changes and write them all at once after to avoid corrupting the file whenever there is a bug.

This would corrupt the private key file whenever the DES cipher would get selected.
@coveralls
Copy link

@coveralls coveralls commented Jun 18, 2014

Coverage Status

Coverage decreased (-0.16%) when pulling 5641750 on eXenon:master into e811e71 on paramiko:master.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Aug 8, 2014

Can you provide a concrete test case for this (in the test suite, or something easily reproducible by hand)?

@eXenon
Copy link
Contributor Author

@eXenon eXenon commented Aug 11, 2014

For me, executing these two lines results in an error and deletes the private key:

 key = paramiko.RSAKey(filename=keyfile, password=oldpassword)
 key.write_private_key_file(keyfile, password=newpassword)

The error is the following:

ValueError: IV must be 8 bytes long

In the example, the filename is a path to the ssh private key file, generated with ssh-keygen, encrypted with AES-128-CBC. The oldpassword contains the old key for the ssh key, newpassword the new one.
The most annoying is that there is not simply an error, but the private key file only contains the first header line after the error, the actual key is lost.

@bitprophet bitprophet added this to the 1.15 milestone Sep 6, 2014
@bitprophet bitprophet merged commit 5641750 into paramiko:master Sep 8, 2014
1 check passed
1 check passed
@bitprophet
continuous-integration/travis-ci The Travis CI build passed
Details
bitprophet added a commit that referenced this pull request Sep 8, 2014
bitprophet added a commit that referenced this pull request Sep 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants