When IPv6 loopback is disabled on a host, X11 forwarding fails. tomas@tkuthan-cz:~$ ipadm show-addr lo0 ADDROBJ TYPE STATE ADDR lo0/v4 static ok 127.0.0.1/8 tomas@tkuthan-cz:~$ ssh -XY localhost xterm X11 forwarding request failed on channel 0 xterm: Xt error: Can't open display: xterm: DISPLAY is not set tomas@tkuthan-cz:~$ Syslog shows the following error: Nov 9 15:36:39 tkuthan-cz sshd[5046]: [ID 800047 auth.error] error: Failed to allocate internet-domain X11 display socket This is caused by bind() failing with EADDRNOTAVAIL on ::1 in x11_create_display_inet. When this particular error is returned by bind, it is safe to continue with the next address returned by getaddrinfo(), because in that case there is no risk of forwarded X11 connections being hijacked (CVE-2008-1483). Binding to 127.0.0.1 succeeds and makes X11 forwarding work again.
Created attachment 2888 [details] When bind returns AEDDRNOTAVAIL, continue with next address
To reproduce - remove IPv6 loopback interface: ipadm delete-addr lo0/v6
(In reply to Tomas Kuthan from comment #0) [...] > When this particular error is returned by bind, it is safe to > continue with the next address returned by getaddrinfo(), because in > that case there is no risk of forwarded X11 connections being > hijacked (CVE-2008-1483). No, there is still a risk, eg if the IPv6 address loopback is added after a connection is made. getaddrinfo w/AI_PASSIVE should not return non-existent addresses. Quoting RFC3493: If the AI_PASSIVE flag is specified, the returned address information shall be suitable for use in binding a socket for accepting incoming connections for the specified service (i.e., a call to bind()). In this case the returned address is not suitable to bind because it'll never work (unless you race bring up the interface).
(In reply to Darren Tucker from comment #3) ... > getaddrinfo w/AI_PASSIVE should not return non-existent addresses. > Quoting RFC3493: > > If the AI_PASSIVE flag is specified, the returned address > information > shall be suitable for use in binding a socket for accepting > incoming > connections for the specified service (i.e., a call to bind()). > > In this case the returned address is not suitable to bind because > it'll never work (unless you race bring up the interface). Hi Darren, I don't think RFC 3493 says, that bind() on address returned by getaddrinfo() has to succeed. I think it specifies what address should be returned for NULL hostname based on AI_PASSIVE flag. If the AI_PASSIVE flag is specified, the returned address information shall be suitable for use in binding a socket for accepting incoming connections for the specified service (i.e., a call to bind()). In this case, if the nodename argument is null, then the IP address portion of the socket address structure shall be set to INADDR_ANY for an IPv4 address or IN6ADDR_ANY_INIT for an IPv6 address. If the AI_PASSIVE flag is not specified, the returned address information shall be suitable for a call to connect() (for a connection-mode protocol) or for a call to connect(), sendto() or sendmsg() (for a connectionless protocol). In this case, if the nodename argument is null, then the IP address portion of the socket address structure shall be set to the loopback address. This flag is ignored if the nodename argument is not null. But I agree, the possibility of an IPv6 interface added after the IPv4 port was bound is troubling. In that case some user could hijack the traffic. I don't think it is directly exploitable; adding an IP interface can only be done by a privileged user, who already has all sorts of powers. But still, there is a a theoretical chance that an unprivileged attacker could exploit a condition that they did not cause. If we choose not to fix it this way, we should at least set hints.ai_family = options.address_family, so that users can make X11 forwarding work w/o IPv6 interface by setting AddressFamily=inet. Would you accept a patch for that? Thanks, Tomas
(In reply to Tomas Kuthan from comment #4) [...] > If we choose not to fix it this way, we should at least set > hints.ai_family = options.address_family, so that users can make X11 > forwarding work w/o IPv6 interface by setting AddressFamily=inet. > > Would you accept a patch for that? I thought that happened already. sshd calls channel_set_af(options.address_family); fairly early, which stores it in channel.c's "static int IPv4or6", which gets passed to getaddrinfo via hints.ai_family. Does that not work?
(In reply to Darren Tucker from comment #5) ... > I thought that happened already. sshd calls > > channel_set_af(options.address_family); > > fairly early, which stores it in channel.c's "static int IPv4or6", > which gets passed to getaddrinfo via hints.ai_family. Does that not > work? You are right, it does work. I must have done something wrong when testing that earlier, my bad. In that case there is nothing to be done, I am closing the bug. Sorry for the noise.
closing resolved bugs as of 8.6p1 release