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

AttributeError when loading known_hosts (was: known_hosts storage bug) #176

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@scowcron
Contributor

scowcron commented Jun 13, 2013

The SSHClient.known_hosts attribute exists only in this location and an exception is raised when trying to store host keys. It looks like it's trying to read the _host_keys_filename attribute instead.

Nathan Scowcroft added some commits Jun 13, 2013

Nathan Scowcroft
@bitprophet

This comment has been minimized.

Member

bitprophet commented Oct 16, 2013

See #214 for another possible solution; I have to check to see what the right action really is in this situation.

@Janzert

This comment has been minimized.

Janzert commented Dec 23, 2013

There is no other place that the known_hosts attribute is referenced in any of the paramiko source. So using the other fix of always setting the attribute to None provides no value whatsoever and would probably be better to just remove the 2 lines. Using this fix appears to fulfill the original intent of the code.

kanzure added a commit to kanzure/boto that referenced this pull request Dec 27, 2013

prevent a paramiko AttributeError in SSHClient
There's a bug in the paramiko library that causes an AttributeError when
paramiko is looking for a known_hosts attribute on SSHClient.

paramiko/paramiko#176
paramiko/paramiko#214
paramiko/paramiko#220
@xdarklight

This comment has been minimized.

Contributor

xdarklight commented Jan 7, 2014

Thanks @scowcron for this fix - I added a test-case to confirm that save_host_keys works now (see #248)

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jan 8, 2014

Just checked things out and yup, @Janzert is correct. No idea what I was smoking earlier :(

Thanks for the test case, @xdarklight! Have minor concerns about it but will address in a merger branch & hopefully get you to yea/nay before it hits master.

@ludoo - thanks for the point about the other still-unhandled edge case, will also take a stab at this ASAP.

bitprophet added a commit that referenced this pull request Jan 8, 2014

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jan 8, 2014

Merging this momentarily.

I checked @ludoo's assertion and it looks like the underlying _host_keys_filename value is always initialized to None and is only overwritten upon an explicit request to load or save.

Therefore, when calling save_host_keys(filename='a new file that does not exist'), the new (and buggy, now fixed) attempt to 'reload' will not fire because it tests for not None. Thus, we only need to worry about IOErrors when client code calls load_host_keys(filename='nope doesnt exist'), which IMO is out of scope here and can be punted on.

Thanks all!

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jan 8, 2014

This ought to be fixed now - @xdarklight's test failed for me prior to merging the fix, and now passes. Backporting it to 1.10 on up; will release today.

@bitprophet bitprophet closed this Jan 8, 2014

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jan 9, 2014

Releases out (1.10.5, 1.11.3, 1.12.1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment