doesn't fall back on ipv4 route when there is no ipv6 route #22

Closed
driesdesmet opened this Issue Jun 24, 2011 · 27 comments

Projects

None yet
@driesdesmet

I can't connect to a ssh server that has an AAAA record from a client that doesn't have an ipv6 route:

Python 2.6.1 (r261:67515, Jun 24 2010, 21:47:49)
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

import paramiko
client=paramiko.SSHClient()
client.connect(hostname='ssh.alwaysdata.com')
Traceback (most recent call last):
File "", line 1, in
File "/Library/Python/2.6/site-packages/paramiko/client.py", line 296, in connect
sock.connect(addr)
File "", line 1, in connect
socket.error: [Errno 65] No route to host

I would expect paramiko to use IPv4 in this case. Connecting with os x's default ssh client works as expected.

@Bluehorn Bluehorn pushed a commit to Bluehorn/paramiko that referenced this issue Aug 12, 2011
Torsten Landschoff Issue #22: Try IPv4 as well as IPv6 when port is not open on IPv6.
This only tries the next address family when the connection gets refused which is my
use case. The destination SSH server had IPv6 enabled but SSH running only on IPv4.

Making paramiko try the next route is just a matter of modifying an except clause.
014bf26
@Bluehorn
Contributor

I ran into almost the same problem when trying to connect to a server which had SSH running only on IPv4 but was reachable via IPv6. I just committed a patch to this problem.
Just noticed that the error code is listed in the issue above and will update the patch to catch that error as well.

@Bluehorn Bluehorn pushed a commit to Bluehorn/paramiko that referenced this issue Aug 12, 2011
Torsten Landschoff Issue #22: Try another address family on EHOSTUNREACH as well. e3a8ba4
@ulope
Contributor
ulope commented Oct 19, 2012

Is there anything happening with this issue?
With more and more servers gaining AAAA records while most home/work connections for the time being remain IPv4 only this is becoming a major PITA.

Is the patch as it stands not acceptable? If so why and what should be improved?

@flavioamieiro

I decided to bump this issue since I've run into this problem today and it took me about two hours of debugging to realize that this was the issue and changing "localhost" to "127.0.0.1" would solve it (I had a port bound to 0.0.0.0 but not ::1).

Please let me know if there's anything I could do to help in the process of merging this patch (as @ulope asked, is it not acceptable? Could we do anything about it?).

@bitprophet
Member

I suspect this has been biting Fabric users upstream, too, e.g. Fabric #372. Will review @Bluehorn's code in a bit. (The original submission predates me taking over the project -- apologies :))

@bitprophet
Member

This looks good overall (haven't tried it out yet though). I have one nitpick/question, hopefully I'm just blind, but the above patches from @Bluehorn would seem to swallow all connection refused / host unreachable errors -- including valid ones.

In other words, it doesn't seem to handle the case where the given host is truly unreachable or not speaking on the port in question. We need a test after the loop ends to determine whether the loop connected OK or if it failed, which implies storing the exception in a variable.

Given there seems no reliable socket.connected test (unless I'm missing something) we can't e.g. if not socket.connected and exception is not None:, so I think we need to detect if the final iteration of the loop generated an exception. (Which means turning the generator output into a list so we know its length, but oh well, it's not like it needs to be a lazy iteration with so few members, eh?)

If @Bluehorn is no longer actively following this I'll get to it myself at some point; otherwise a proper pull request with his patches + the above tweak would be awesome :) Thanks!

@Bluehorn
Contributor
Bluehorn commented Feb 4, 2013

@bitprophet You are right, we stumbled over that omission as well. I think my fork on github is updated. I'll clean it up a bit and send a new pull request if that is okay for you.

@jeffrizzo

Just another vote for "please fix this!" :)

@TheBits
TheBits commented Aug 2, 2013

+1

@zertrin
zertrin commented Oct 17, 2013

👍 too.
It seems it's properly fixed in your fork @Bluehorn (https://github.com/Bluehorn/paramiko/blob/master/paramiko/client.py#L306)

@Bluehorn
Contributor

Whoops, sorry, forgot about this. I promised to clean it up and send a pull request.
Thanks for reminding me.

@Bluehorn Bluehorn added a commit to Bluehorn/paramiko that referenced this issue Oct 23, 2013
@Bluehorn Torsten Landschoff + Bluehorn Issue #22: Try IPv4 as well as IPv6 when port is not open on IPv6.
With this change, paramiko tries the next address family when the connection
gets refused or the target port is unreachable.
27b800b
@Bluehorn
Contributor

I just updated the patch for this issue. See this gist for my test methodology:

https://gist.github.com/Bluehorn/7128643

@tgoodlet
tgoodlet commented Feb 6, 2015

Oh man. This still isn't fixed hehe?
ipv6 is like trying to push craft beer on people :D

@bitprophet
Member

Whoa there, I'm actually drinking a craft beer right now! (Proof)

Interestingly I could swear I remembered merging a fix related to this, but maybe it's just my memory of the comments I left above. Gonna poke & see.

@bitprophet
Member

Confirmed that the current state of the code has the problem this is fixing (tries only 1st resolved address/family).

The latest state of https://github.com/Bluehorn/paramiko/compare/issue22 looks good. My only worry is if some platforms not under test raise socket exception types not included in (ECONNREFUSED, EHOSTUNREACH) (e.g. I remember other issues discovering fun subtle differences between Mac/BSD and Linux, where the same scenario yielded different exception numbers.) However that's not worth blocking a merge.

@bitprophet
Member

As I merge I notice that my earlier concern still isn't fully addressed, if we encounter errors which are conn-refused/host-unreachable, and they persist for all address families, they will be hidden and then we'll just die farther down the file when socket is unset/None.

Will attempt to recreate a few scenarios to confirm that this is an issue & to verify a fix.

@yaccz
yaccz commented Feb 6, 2015

I think paramiko should also accept the connection hints (addrinfo) in a user controlled way so the user can for example say he wants just ipv4 addresses

@bitprophet
Member

I agree, @yaccz, though I'd prefer to take things one step at a time :)

@bitprophet
Member

Confirmed that the "wholly unreachable address == no bueno" scenario does raise a totally inscrutable error (an EOFError from attempts to .start_client() in Transport, at least when run via Fabric). Which also happened to trigger fabric/fabric#1263!

@bitprophet
Member

Confirmed I can recreate the real problem here by disabling IPv6 on my Mac's sshd config, reloading sshd, and then trying to use paramiko to talk to localhost. (For whatever reason, getaddrinfo on this platform is returning IPv6 first and IPv4 second - presumably the opposite scenario from @Bluehorn's setup (but same as @driesdesmet).

@tgoodlet
tgoodlet commented Feb 7, 2015

@bitprophet Hah!
Nice to see a fellow beer advocate at work. I've been meaning to sign up to untappd. Should really do that...

I was lucky enough to snag a case of these this week.
Maybe I can start with that as my first rate.

Glad to see the issue get sorted out.
Thanks a ton!

@bitprophet
Member

I need to rename the exception I'm using here (Feels too general in hindsight), then make sure any pertinent docstrings & the changelog are updated, then merge.

@domenkozar

I've just hit an ipv6 bug, it's really annoying since it's hard to debug.

Make sure you have a public ipv6 address and create foo.py:

import paramiko as ssh

client = ssh.SSHClient()
client.connect(
    hostname='dogbert.kiberpipa.org',
    port=22,
)

Running python foo.py

Traceback (most recent call last):
  File "foo.py", line 6, in <module>
    port=22,
  File "/nix/store/dd1rf5bfds81wm588jagra109gbswas9-python2.7-paramiko-1.15.1/lib/python2.7/site-packages/paramiko/client.py", line 251, in connect
    retry_on_signal(lambda: sock.connect(addr))
  File "/nix/store/dd1rf5bfds81wm588jagra109gbswas9-python2.7-paramiko-1.15.1/lib/python2.7/site-packages/paramiko/util.py", line 278, in retry_on_signal
    return function()
  File "/nix/store/dd1rf5bfds81wm588jagra109gbswas9-python2.7-paramiko-1.15.1/lib/python2.7/site-packages/paramiko/client.py", line 251, in <lambda>
    retry_on_signal(lambda: sock.connect(addr))
  File "/nix/store/nk1hw5c6jzin073difisgi0jfbbrqj7g-python-2.7.9/lib/python2.7/socket.py", line 224, in meth
    return getattr(self._sock,name)(*args)
socket.error: [Errno 13] Permission denied
@domenkozar

Main problem is that the ipv6 it returns is not accessible.

@bitprophet
Member

I think I ran into that while testing this, @iElectric - I'm gonna merge it soon because it at least fixes the original case, but see https://github.com/paramiko/paramiko/compare/try-multiple-address-families-22 which is the branch I've been working in, curious if it (or post-merge, master) solves your case too.

@domenkozar

I don't have IPv6 address right now, but try connecting to dogbert.kiberpipa.org, it doesn't have ipv6 setup on the interface.

@bitprophet
Member

Tried it, both master and my branch work, but that's because my system is ending up with the IPv4 address first in the list returned by socket.getaddrinfo (this was a thing that complicated earlier troubleshooting as well).

The changes in the new branch should work for most permutations of the issue, and when I manually jigger it to e.g. reverse the results from getaddrinfo, things still work fine w/ your host. (If I do the same on master, I get No route to host - not Permission denied - but as far as I've seen there's lots of small differences between OSes and instances thereof which mean the specific error differs widely.)

@bitprophet bitprophet added a commit that closed this issue Feb 27, 2015
@bitprophet bitprophet Changelog closes #22 4ca8d68
@bitprophet
Member

Ran into a few minor issues with this after the fact:

  • Forgot to rename the exception class as I'd been planning to, ConnectionError is too generic-sounding after all, my mistake.

  • Code downstream in Fabric which catches socket.error assumes one can do exception_obj[1] to obtain the exception message string. This implies that most socket errors in the past were of the latter form as per Python core docs (and if Fabric is making this assumption I'm sure other Paramiko using code might too):

    The accompanying value is either a string telling what went wrong or a pair (errno, string) representing an error returned by a system call

    Our subclass initializes using the superclass' __init__ but only passes in the single message string. I should see whether it makes sense to initialize with an errno field (either a real error number that seems to map, or more likely, just None, because it is not a real OS level socket error.)

@bitprophet bitprophet added a commit that referenced this issue Mar 5, 2015
@bitprophet bitprophet Add null errno to socket.error subclass.
Makes downstream code less likely to break when they expect
errno+msg style socket error objects.

Re #22
136e0de
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment