Solved issue #2910 - Added support for IPv6 with fallback to IPv4 for ssh

Solved issue #2910 (closed)

Description

Added support for IPv6 with fallback to IPv4 for ssh As ssh_options_set with SSH_OPTIONS_HOST allows for hostnames and IPs, it's just a matter of resolving the hostname before calling ssh_connect. I added a call to getaddrinfo and when IPv4 and IPv6 are returned, it uses IPv6 by default, then if the connection fails, it falls back to IPv4.

Related Issue

Issue #2910 (closed) "dual-stack IPv4/6 host can't be connected via domain name if the remote port is only opened for IPv4"

Motivation and Context

How Has This Been Tested?

I tested it on a dual stack machine of course, that tries to connect to another dual stack machine. OS used: Devuan Chimaera and Debian Bullseye

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

changed the description

Overall, do you think your approach is more preferable than passing a custom socket?

IMO I guess so, because usually the deeper you hack into the layers, the more problems you get !

I'm not so confident here because passing a socket might take less code still..
Somebody else should see.

bot, build

Hmm, doesn't seem to work:
https://github.com/flathub/flathub/wiki/App-Maintenance#test-builds-and-pull-requests

@myheroyuki any comments?

I guess to start it must be posted by a repo collaborator or owner.

Right, @myheroyuki could we make sure it works, would you post the magic phrase?

Hi have managed to rerun the pipeline (it failed because I did not give my credit card number ....). You can download the flatpak package above in the pipeline section.

What pipeline do you mean, I do not see any.

Hmm, I don't see it, I refreshed the page several times:

Seems buggy ... Perhaps a credentials problem. I guess only @myheroyuki could solve this...

I relaunched a pipeline on my fork, should be public: https://gitlab.com/scoulondre/Remmina/-/pipelines/907459034

Hmmm, I can see the pipeline, so it might be a permissions issue where only maintainers on the project or the creator can see the result of the pipeline, but I'm leaning towards it being buggy as I've had pipeline statuses that have taken multiple refreshes for me to load. I'll take a look and see if I can figure it out, and I'll look into the bot, build command as well.

In the meantime, @bam80, have you had a chance to test the code @scoulondre has posted? It looks good to me, so if it works for both of you I will merge it in.

I mentioned here I can't reproduce the original problem right now so can't say if the patch works for me:
#2910 (comment 1440541112)
Maybe some other test servers could help.

I also have some concerns about approach chosen here:
!2507 (comment 1438570235)

About "bot, build" command, maybe it works on Github only? Don't know.

I did see your concerns about the approach taken, but I think I agree with @scoulondre's assessment of the design.

IIUC, that approach implies we use resolving function twice - first time on our side when we resolve hostname to IP address, and then on the library side when it's not necessary already. Not a big deal but still..
On the other hand, passing socket of the right type should be trivial and without that quirks.

Does the library try to resolve it again? It seems to me if the SSH_OPTIONS_HOST is set to an already resolved ip and not a hostname it should not try to resolve it again, but I haven't gone through all of their source code to confirm that yet.

It's resolved by getaddrinfo, then the IP v6 or v4 is passed to the libssh library, so there is no additional resolution. BTW by opening a socket, you can not know if the server will respond. So there is no fallback possible.

Did you check it from the source code? I see both hostname and IP address can be passed:

SSH_OPTIONS_HOST: The hostname or ip address to connect to (const char *).

That implies the resolutions still happens inside as the library can't know what you supplied to it beforehand..

BTW by opening a socket, you can not know if the server will respond. So there is no fallback possible.

Could you elaborate on that?

If the hostname resolution returns a valid IPv6, then you want to open an IPv6 socket and pass it to ssh_connect. But if the firewall blocks IPv6 or if there is no SSH server on the IPv6 stack, we should be able to fall back to IPv4. It means calling again ssh_connect with an IPv4 socket. So adding more code too.

Yes, but that way we just implement on top what's missing in the library, rather than reimplementing what it already can. It seems to me more straightforward approach. But I'm not insisting, though.

The place where libssh detects that an IP is passed and not a hostname is here: https://github.com/libssh/libssh-mirror/blob/ac6d2fad4a8bf07277127736367e90387646363f/src/connect.c#L139

It passed the flag AI_NUMERICHOST to getaddrinfo, which basically tells it not to resolve the host against the DNS system. So there is no additional resolution.

OK, I just dislike we solving the problem implicitly, reimplementing resolution for the library, whereas the resolution itself is not a problem.

"Yes, but that way we just implement on top what's missing in the library, rather than reimplementing what it already can." That's not clear to me. How do you want to do it ?

I put my findings here:
#2910 (comment 1426033396)

Maybe you would agree to make the alternative patch and then we will decide? But see yourself, thank you anyway.

I already read extensively your findings, but I can not see another way to do it except opening an IPv6 socket and pass it to ssh_connect, handle connection error, close the socket, and retry with IPv4. This is not easier. Please be more precise, not just referencing the ssh_get_fd documentation and the socket solution.

Right, that's what I mean, could be still easier. It would be better if we had two patches to compare (at least for me).

I personally agree with @scoulondre's assessment of the situation.

@bam80 if you would like to create a separate patch you are welcome to do so to more clearly demonstrate your point, but I think the current solution is quite good.

Unfortunately, I can't do that ATM.

The point is the patch could express our intent more explicitly by setting the socket type, rather than reimplementing the resolve functionality.

From a design point of view, libssh provides the layer 7 of the OSI model, and sockets are at layer 5. Accessing layer 5 to handle a problem in layer 7 is more a hack than serious development.

We will just open and pass it, no more hacks there?

As I see, ping does it too with using just the hints.ai_family needed and different sockets:

$ ping whatever.com -v4
ping: sock4.fd: 3 (socktype: SOCK_RAW), sock6.fd: -1 (socktype: 0), hints.ai_family: AF_INET

ai->ai_family: AF_INET, ai->ai_canonname: 'whatever.com'
PING whatever.com (3.33.152.147) 56(84) bytes of data.
64 bytes from a4ec4c6ea1c92e2e6.awsglobalaccelerator.com (3.33.152.147): icmp_seq=1 ident=40345 ttl=117 time=15.7 ms
64 bytes from a4ec4c6ea1c92e2e6.awsglobalaccelerator.com (3.33.152.147): icmp_seq=2 ident=40345 ttl=117 time=14.9 ms
64 bytes from a4ec4c6ea1c92e2e6.awsglobalaccelerator.com (3.33.152.147): icmp_seq=3 ident=40345 ttl=117 time=14.9 ms
64 bytes from a4ec4c6ea1c92e2e6.awsglobalaccelerator.com (3.33.152.147): icmp_seq=4 ident=40345 ttl=117 time=14.7 ms
^C
--- whatever.com ping statistics ---
4 packets transmitted, 4 received, 0% packet loss, time 3003ms
rtt min/avg/max/mdev = 14.658/15.039/15.749/0.419 ms


bam@Feedme:~$ ping whatever.com -v6
ping: sock4.fd: -1 (socktype: 0), sock6.fd: 3 (socktype: SOCK_RAW), hints.ai_family: AF_INET6

ai->ai_family: AF_INET, ai->ai_canonname: 'whatever.com'
ai->ai_family: AF_INET, ai->ai_canonname: ''
ping: whatever.com: Address family for hostname not supported
$

I'm not in position to block this, but now I even more confident we should at least try to implement it the other way. Obviously it's made such way in all the other tools.

Having that, I can confirm the patch works. Thank you.

Of course Ping uses the sockets, because it's at OSI layer 5... It does not rely on SSH, not even TCP but on SOCK_RAW (a socket type that does not support the concept of services). Even libssh uses sockets. Every protocol uses sockets. The problem is not the sockets, it's the fact that it's bad practice to mix different layers. Stating that it's made such way in all the other tools is like comparing oranges and bananas.

We can compare with ssh util then.

Ok here is an analogy. Your car GPS does not know where your parents live. So instead of typing the address, you plug in your mobile phone with google maps that knows where they live and you follow your phone indications. Imagine your phone runs out of battery, or the sound stops working, or anything related to the phone, you can't reach your destination. If you had mentioned the address to the car GPS, as it is designed for your car, you won't have any bad surprise. That's good practices.

Still, I would like to see how it's done in ssh. If you prefer, I can look myself.

I am afraid you can't make it easier.

OK I'll look myself.

@scoulondre thanks for all your work making this fix, and for taking the time to explain it in detail. Now that it's been confirmed it works fine for both of you I'm merging this in.

@bam80, if you are still unhappy with this solution after all the explanation you have received please feel free to create an additional patch and I will review it, but as this does fix the problem, and does so in a way I approve of, I am closing the original issue.

I had a look how it's done in ssh and first of all can say it does it in similar way suggested by Stephane, meaning it resolves the host first and then for each address gathered tries to create corresponding socket in loop.
Here is the function:
https://github.com/openssh/openssh-portable/blob/8a6cd08850f576e7527c52a1b086cae82fab290e/sshconnect.c#L447

So my concerns here didn't justified, which is good.

Having that, I found some differences how it's handled in ssh, will comment in the code.

Overall, I have a feeling it actually should have been implemented in the library itself indeed, as there is actually no need to reimplement it in every client.
I'll try to create the issue upstream, maybe Sthephane could even patch the library itself as the way is clear now.

This issue in libssh has been opened in ... 2013, and still open.... libssh/libssh-mirror#27

@myheroyuki this fix is quite important, maybe we could make a beta version and publish it at least on flathub-beta channel?
I made a PR recently:
https://github.com/flathub/org.remmina.Remmina/pull/190

I'll try to create the issue upstream, maybe Sthephane could even patch the library itself as the way is clear now.

Apparently it was fixed upstream just 3 weeks ago, need to create test platpak with master libssh:
libssh/libssh-mirror#27 (comment 1445551642)

UPDATE:
The upstream fix didn't succeed:
libssh/libssh-mirror@1c0b8f62 (comment 1448520297)

mentioned in commit ee7b77a7

merged

That way we end up with 1 last IPv4 address and 1 IPv6 address, maximum.

Whereas ssh handles all the resolved addresses:

  /*
   * Loop through addresses for this host, and try each one in
   * sequence until the connection succeeds.
   */

https://github.com/openssh/openssh-portable/blob/8a6cd08850f576e7527c52a1b086cae82fab290e/sshconnect.c#L470

The way SSH handles addresses is an improvement. But is it really useful ? Here is a except from the getaddrinfo documentation https://linux.die.net/man/3/getaddrinfo

"There are several reasons why the linked list may have more than one addrinfo structure, including:

• the network host is multihomed while having one DNS CNAME record for all its interfaces -> very rare
• accessible over multiple protocols (e.g., both AF_INET and AF_INET6); -> our case
• the same service is available from multiple socket types (one SOCK_STREAM address and another SOCK_DGRAM address, for example). -> not our case because SOCK_DGRAM is for UDP"

Basically, in practice you never get multiple IPv4 or multiple IPv6 addresses as the result of a DNS resolution.

Good to know, thanks.

FYI: This loop itself is wrapped in other loop in ssh which makes finite number of attempts which goes probably from ssh config:
https://github.com/openssh/openssh-portable/blob/8a6cd08850f576e7527c52a1b086cae82fab290e/sshconnect.c#L460

mentioned in issue libssh/libssh-mirror#27