Hi, Over the last couple of weeks (as well as earlier) we all have observed a number of problem reports with X11 forwarding. Apart from the cases when xauth can't be located all the problems spin around a design flaw (part of which I've already pointed out back in April 1997) present in all versions of SSH (including gaining popularity OpenSSH). My point is that sshd derives DISPLAY and list of cookies under wrong assumptions. Or should I say "not always correct assumptions"? First DISPLAY is derived as "`uname -n`:XY.0" (provided that sshd listens at 60XY) which implies that `uname -n` is resolved to some IP address (hopefully of one of interfaces). Latter is not always true and it's not even a requirement for proper opertion of other programs (e.g. X-server). Yes, it's true in 90% cases, but not *always*. However, choosing the IP-address of the interface ssh client has connected through gonna *always* work, i.e. in 100% cases, regardless how the server is set up. List of cookies is also generated under assuption that `uname -n` is resolved to and IP address (hopefully of one of interfaces). Moreover when it comes to multihomed hosts another assumption is made, namely `uname -n` is resolved to *all* interfaces (i.e. gethostbyname returns the list of all interfaces). Latter is not always true either and definitely in way less than 90% cases. However, you can *list* interfaces at the login time and compose the list of cookies accordingly covering (again) 100% cases. Therefore attached patch. It's relative to 2.2.0 (note that I didn't bother to patch ./configure.in). Multihome stuff for 1.2.27 can be extracted from the patch available at http://fy.chalmers.se/~appro/ssh_beyond.html (in case you're disgusted by the whole patch:-). As for better DISPLAY calculation for 1.2.x I gonna put it out shortly (along with update for 1.2.30). Special comment about "I could have used ssh_tcp_get_host_by_addr_sync() instead..." comment below. Apart from the real reason (it does reverse lookup which I considered as overkill in this case) I found it poorly implemented. It reads: ... struct in_addr in_addr; unsigned char outbuf[16]; ... memmove (&in_addr.s_addr, outbuf, outbuflen); ... Now the catch is that sizeof(struct in_addr) is 4 (at the very least on Solaris up to 7 and Linux). Meaning that *if* compiler chooses to allocate in_addr last in stack, you gonna smash the stack... Cheers. Andy. ---------------------------------------------------------------- *** ./apps/ssh/sshchsession.c.orig Mon Jun 12 18:38:59 2000 --- ./apps/ssh/sshchsession.c Sun Jul 16 17:40:28 2000 *************** *** 71,76 **** --- 71,83 ---- #ifdef HAVE_SYS_SELECT_H #include <sys/select.h> #endif /* HAVE_SYS_SELECT_H */ + #ifdef HAVE_STROPTS_H + #include <stropts.h> + #endif /* HAVE_STROPTS_H */ + #include <net/if.h> + #ifdef HAVE_SYS_SOCKIO_H + #include <sys/sockio.h> + #endif /* HAVE_SYS_SOCKIO_H */ #endif /* XAUTH_PATH || HPSUX_NONSTANDARD_X11_KLUDGE */ #ifdef HAVE_ULIMIT_H *************** *** 652,662 **** --- 659,730 ---- f = popen(XAUTH_PATH " -q -", "w"); if (f) { + #ifndef SIOCGIFCONF fprintf(f, "add %s %s %s\n", display, auth_protocol, auth_cookie); + #endif cp = strchr(display, ':'); if (cp) { + #ifdef SIOCGIFCONF + /* + * List all IP interfaces and pass 'em down to xauth. + * <[EMAIL PROTECTED]> + */ + int s; + struct sockaddr_in *sin; + struct ifconf ifc; + struct ifreq *ifr; + char *ifreqs; + int ifrn; + + if ((s=socket (AF_INET,SOCK_DGRAM,0)) < 0) + ssh_fatal ("Unable to create socket: %s\n", + strerror(errno)); + #ifdef SIOCGIFNUM + if (ioctl (s,SIOCGIFNUM,&ifrn) < 0) + ssh_fatal ("Unable to SIOCGIFNUM: %s\n", + strerror(errno)); + #else + ifrn = 256; + #endif + ifc.ifc_len = sizeof(struct ifreq)*ifrn; + ifc.ifc_buf = ifreqs = ssh_xmalloc (ifc.ifc_len); + if (ioctl (s,SIOCGIFCONF,&ifc) < 0) + ssh_fatal ("Unable to SIOCGIFCONF: %s\n", + strerror(errno)); + ifr = ifc.ifc_req; + ifrn = ifc.ifc_len/sizeof(struct ifreq); + for (; ifrn--; ifr++) + { + if (ioctl (s,SIOCGIFFLAGS,ifr) < 0) continue; + if (!(ifr->ifr_flags&IFF_UP)) continue; + #ifdef IFF_UNNUMBERED + if (ifr->ifr_flags&IFF_UNNUMBERED) continue; + #endif + if (ioctl (s,SIOCGIFADDR, ifr) < 0) continue; + sin = (struct sockaddr_in *)&ifr->ifr_addr; + if (sin->sin_family != AF_INET) continue; + if (sin->sin_addr.s_addr == INADDR_ANY) + continue; + if (sin->sin_addr.s_addr == INADDR_LOOPBACK) + continue; + ssh_debug("\tadd %s%s %s %s\n", + inet_ntoa(sin->sin_addr), + cp,auth_protocol,auth_cookie); + fprintf (f, "add %s%s %s %s\n", + inet_ntoa(sin->sin_addr), + cp,auth_protocol,auth_cookie); + } + ssh_xfree (ifreqs); + close (s); + #else /* SIOCGIFCONF */ + #if 0 + /* + * It's redundant as sshd never listens on a unix + * socket in either case... + * <[EMAIL PROTECTED]> + */ #ifndef CRAY /* Cray xauth cannot take host/unix:0 as displayname */ fprintf(f, "add %.*s/unix%s %s %s\n", *************** *** 663,668 **** --- 731,737 ---- (int)(cp - display), display, cp, auth_protocol, auth_cookie); #endif + #endif hp = gethostbyname(name); if (hp) { *************** *** 679,684 **** --- 748,754 ---- cp, auth_protocol, auth_cookie); } } + #endif /* SIOCGIFCONF */ } pclose(f); } *** ./apps/ssh/sshchx11.c.orig Mon Jun 12 18:38:59 2000 --- ./apps/ssh/sshchx11.c Sun Jul 16 17:27:05 2000 *************** *** 557,562 **** --- 557,568 ---- } /* Set up a suitable value for the DISPLAY variable. */ + #if 0 + /* + * Following is no good. As it depends on the fact that `uname -n` + * is resolved to some IP address. It's not always the case... + * <[EMAIL PROTECTED]> + */ #ifdef HPSUX_NONSTANDARD_X11_KLUDGE /* HPSUX has some special shared memory stuff in their X server, which appears to be enabled if the host name matches that of the local machine. *************** *** 587,592 **** --- 593,621 ---- snprintf(buf, sizeof(buf), "%s:%d.%lu", hostname, display_number, session->screen_number); #endif /* HPSUX_NONSTANDARD_X11_KLUDGE */ + #else + /* + * Instead we should point at interface the client has connected at! + * And to be nice let's resolve corresponding IP to the canonical + * name:-) + * <[EMAIL PROTECTED]> + */ + { + #ifndef HPSUX_NONSTANDARD_X11_KLUDGE + /* I could have used ssh_tcp_get_host_by_addr_sync() instead... */ + struct hostent *hp; + struct in_addr addr; + addr.s_addr = inet_addr(session->common->local_ip); + if (addr.s_addr && (hp=gethostbyaddr((void*)&addr,sizeof(addr),AF_INET))) + snprintf(buf, sizeof(buf), "%s:%d.%lu", + hp->h_name, display_number, session->screen_number); + else + #endif + snprintf(buf, sizeof(buf), "%s:%d.%lu", + session->common->local_ip, + display_number, session->screen_number); + } + #endif /* Save the display name. */ session->display = ssh_xstrdup(buf); *** ./configure.orig Mon Jun 12 18:39:26 2000 --- ./configure Sun Jul 9 22:51:33 2000 *************** *** 2746,2752 **** done fi ! for ac_hdr in sys/ioctl.h do ac_safe=`echo "$ac_hdr" | sed 'y%./+-%__p_%'` echo $ac_n "checking for $ac_hdr""... $ac_c" 1>&6 --- 2746,2752 ---- done fi ! for ac_hdr in sys/ioctl.h sys/sockio.h do ac_safe=`echo "$ac_hdr" | sed 'y%./+-%__p_%'` echo $ac_n "checking for $ac_hdr""... $ac_c" 1>&6 *** ./sshconf.h.in.orig Mon Jun 12 18:39:24 2000 --- ./sshconf.h.in Sun Jul 9 22:48:55 2000 *************** *** 666,671 **** --- 666,674 ---- /* Define if you have the <sys/select.h> header file. */ #undef HAVE_SYS_SELECT_H + /* Define if you have the <sys/sockio.h> header file. */ + #undef HAVE_SYS_SOCKIO_H + /* Define if you have the <sys/stream.h> header file. */ #undef HAVE_SYS_STREAM_H
