From 42547ed95b693ac08d0770755f87ae123351daec Mon Sep 17 00:00:00 2001 From: jacob Date: Tue, 24 Feb 2009 01:01:23 +0000 Subject: [PATCH] Since r8305, Unix PuTTY has always "upgraded" an X11 display like "localhost:0" to a Unix-domain socket. This typically works fine when PuTTY is run on the same machine as the X server, but it's broken multi-hop X forwarding through OpenSSH; when OpenSSH creates a proxy X server "localhost:10", it only listens on TCP, not on a Unix-domain socket. Instead, when deciding on the details of the display, we actively probe to see if there's a Unix-domain socket we can use instead, and only use it if it's there, falling back to the specified IP "localhost" if not. Independently, when looking for local auth details in Xauthority for a "localhost" TCP display, we prefer a matching Unix-domain entry, but will fall back to an IP "localhost" entry (which would be unusual, but we don't trust a Windows X server not to do it) -- this is a generalisation of the special case added in r2538 (but removed in r8305, as the automatic upgrade masked the need for it). (This is now done in platform-independent code, so a side-effect is that get_hostname() is now part of the networking abstraction on all platforms.) git-svn-id: svn://svn.tartarus.org/sgt/putty@8462 cda61777-01e9-0310-a592-d414129be87e --- network.h | 6 +++ unix/ux_x11.c | 34 -------------- unix/uxnet.c | 17 +++++++ windows/winnet.c | 18 ++++++++ x11fwd.c | 138 ++++++++++++++++++++++++++++++++++++++++++++----------- 5 files changed, 153 insertions(+), 60 deletions(-) diff --git a/network.h b/network.h index af83e653..6ee7081e 100644 --- a/network.h +++ b/network.h @@ -202,6 +202,12 @@ void net_pending_errors(void); */ int net_service_lookup(char *service); +/* + * Look up the local hostname; return value needs freeing. + * May return NULL. + */ +char *get_hostname(void); + /********** SSL stuff **********/ /* diff --git a/unix/ux_x11.c b/unix/ux_x11.c index 5794d315..78a56150 100644 --- a/unix/ux_x11.c +++ b/unix/ux_x11.c @@ -18,40 +18,6 @@ void platform_get_x11_auth(struct X11Display *disp, const Config *cfg) int needs_free; /* - * Upgrade an IP-style localhost display to a Unix-socket - * display. - */ - if (!disp->unixdomain && sk_address_is_local(disp->addr)) { - sk_addr_free(disp->addr); - disp->unixdomain = TRUE; - disp->addr = platform_get_x11_unix_address(NULL, disp->displaynum); - disp->realhost = dupprintf("unix:%d", disp->displaynum); - disp->port = 0; - } - - /* - * Set the hostname for Unix-socket displays, so that we'll - * look it up correctly in the X authority file. - */ - if (disp->unixdomain) { - int len; - - sfree(disp->hostname); - disp->hostname = NULL; - len = 128; - do { - len *= 2; - disp->hostname = sresize(disp->hostname, len, char); - if ((gethostname(disp->hostname, len) < 0) && - (errno != ENAMETOOLONG)) { - sfree(disp->hostname); - disp->hostname = NULL; - return; - } - } while (strlen(disp->hostname) >= len-1); - } - - /* * Find the .Xauthority file. */ needs_free = FALSE; diff --git a/unix/uxnet.c b/unix/uxnet.c index 8e95a65e..4467d082 100644 --- a/unix/uxnet.c +++ b/unix/uxnet.c @@ -1395,6 +1395,23 @@ int net_service_lookup(char *service) return 0; } +char *get_hostname(void) +{ + int len = 128; + char *hostname = NULL; + do { + len *= 2; + hostname = sresize(hostname, len, char); + if ((gethostname(hostname, len) < 0) && + (errno != ENAMETOOLONG)) { + sfree(hostname); + hostname = NULL; + break; + } + } while (strlen(hostname) >= len-1); + return hostname; +} + SockAddr platform_get_x11_unix_address(const char *sockpath, int displaynum) { SockAddr ret = snew(struct SockAddr_tag); diff --git a/windows/winnet.c b/windows/winnet.c index c549b087..bcfc004d 100644 --- a/windows/winnet.c +++ b/windows/winnet.c @@ -167,6 +167,7 @@ DECL_WINSOCK_FUNCTION(static, u_long, ntohl, (u_long)); DECL_WINSOCK_FUNCTION(static, u_long, htonl, (u_long)); DECL_WINSOCK_FUNCTION(static, u_short, htons, (u_short)); DECL_WINSOCK_FUNCTION(static, u_short, ntohs, (u_short)); +DECL_WINSOCK_FUNCTION(static, int, gethostname, (char *, int)); DECL_WINSOCK_FUNCTION(static, struct hostent FAR *, gethostbyname, (const char FAR *)); DECL_WINSOCK_FUNCTION(static, struct servent FAR *, getservbyname, @@ -295,6 +296,7 @@ void sk_init(void) GET_WINSOCK_FUNCTION(winsock_module, htonl); GET_WINSOCK_FUNCTION(winsock_module, htons); GET_WINSOCK_FUNCTION(winsock_module, ntohs); + GET_WINSOCK_FUNCTION(winsock_module, gethostname); GET_WINSOCK_FUNCTION(winsock_module, gethostbyname); GET_WINSOCK_FUNCTION(winsock_module, getservbyname); GET_WINSOCK_FUNCTION(winsock_module, inet_addr); @@ -1684,6 +1686,22 @@ int net_service_lookup(char *service) return 0; } +char *get_hostname(void) +{ + int len = 128; + char *hostname = NULL; + do { + len *= 2; + hostname = sresize(hostname, len, char); + if (p_gethostname(hostname, len) < 0) { + sfree(hostname); + hostname = NULL; + break; + } + } while (strlen(hostname) >= len-1); + return hostname; +} + SockAddr platform_get_x11_unix_address(const char *display, int displaynum, char **canonicalname) { diff --git a/x11fwd.c b/x11fwd.c index 1fd4e190..ba76f782 100644 --- a/x11fwd.c +++ b/x11fwd.c @@ -50,6 +50,24 @@ static int xdmseen_cmp(void *a, void *b) memcmp(sa->clientid, sb->clientid, sizeof(sa->clientid)); } +/* Do-nothing "plug" implementation, used by x11_setup_display() when it + * creates a trial connection (and then immediately closes it). + * XXX: bit out of place here, could in principle live in a platform- + * independent network.c or something */ +static void dummy_plug_log(Plug p, int type, SockAddr addr, int port, + const char *error_msg, int error_code) { } +static int dummy_plug_closing + (Plug p, const char *error_msg, int error_code, int calling_back) +{ return 1; } +static int dummy_plug_receive(Plug p, int urgent, char *data, int len) +{ return 1; } +static void dummy_plug_sent(Plug p, int bufsize) { } +static int dummy_plug_accepting(Plug p, OSSocket sock) { return 1; } +static const struct plug_function_table dummy_plug = { + dummy_plug_log, dummy_plug_closing, dummy_plug_receive, + dummy_plug_sent, dummy_plug_accepting +}; + struct X11Display *x11_setup_display(char *display, int authtype, const Config *cfg) { @@ -87,6 +105,7 @@ struct X11Display *x11_setup_display(char *display, int authtype, disp->hostname = NULL; disp->displaynum = -1; disp->screennum = 0; + disp->addr = NULL; } else { char *colon, *dot, *slash; char *protocol, *hostname; @@ -134,6 +153,7 @@ struct X11Display *x11_setup_display(char *display, int authtype, disp->hostname = dupstr("localhost"); disp->unixsocketpath = NULL; + disp->addr = NULL; sfree(localcopy); } @@ -141,15 +161,7 @@ struct X11Display *x11_setup_display(char *display, int authtype, /* * Look up the display hostname, if we need to. */ - if (disp->unixdomain) { - disp->addr = platform_get_x11_unix_address(disp->unixsocketpath, - disp->displaynum); - if (disp->unixsocketpath) - disp->realhost = dupstr(disp->unixsocketpath); - else - disp->realhost = dupprintf("unix:%d", disp->displaynum); - disp->port = 0; - } else { + if (!disp->unixdomain) { const char *err; disp->port = 6000 + disp->displaynum; @@ -165,6 +177,42 @@ struct X11Display *x11_setup_display(char *display, int authtype, } /* + * Try upgrading an IP-style localhost display to a Unix-socket + * display (as the standard X connection libraries do). + */ + if (!disp->unixdomain && sk_address_is_local(disp->addr)) { + SockAddr ux = platform_get_x11_unix_address(NULL, disp->displaynum); + const char *err = sk_addr_error(ux); + if (!err) { + /* Create trial connection to see if there is a useful Unix-domain + * socket */ + const struct plug_function_table *dummy = &dummy_plug; + Socket s = sk_new(sk_addr_dup(ux), 0, 0, 0, 0, 0, (Plug)&dummy); + err = sk_socket_error(s); + sk_close(s); + } + if (err) { + sk_addr_free(ux); + } else { + sk_addr_free(disp->addr); + disp->unixdomain = TRUE; + disp->addr = ux; + /* Fill in the rest in a moment */ + } + } + + if (disp->unixdomain) { + if (!disp->addr) + disp->addr = platform_get_x11_unix_address(disp->unixsocketpath, + disp->displaynum); + if (disp->unixsocketpath) + disp->realhost = dupstr(disp->unixsocketpath); + else + disp->realhost = dupprintf("unix:%d", disp->displaynum); + disp->port = 0; + } + + /* * Invent the remote authorisation details. */ if (authtype == X11_MIT) { @@ -295,6 +343,31 @@ void x11_get_auth_from_authfile(struct X11Display *disp, char *buf, *ptr, *str[4]; int len[4]; int family, protocol; + int ideal_match = FALSE; + char *ourhostname = get_hostname(); + + /* + * Normally we should look for precisely the details specified in + * `disp'. However, there's an oddity when the display is local: + * displays like "localhost:0" usually have their details stored + * in a Unix-domain-socket record (even if there isn't actually a + * real Unix-domain socket available, as with OpenSSH's proxy X11 + * server). + * + * This is apparently a fudge to get round the meaninglessness of + * "localhost" in a shared-home-directory context -- xauth entries + * for Unix-domain sockets already disambiguate this by storing + * the *local* hostname in the conveniently-blank hostname field, + * but IP "localhost" records couldn't do this. So, typically, an + * IP "localhost" entry in the auth database isn't present and if + * it were it would be ignored. + * + * However, we don't entirely trust that (say) Windows X servers + * won't rely on a straight "localhost" entry, bad idea though + * that is; so if we can't find a Unix-domain-socket entry we'll + * fall back to an IP-based entry if we can find one. + */ + int localhost = !disp->unixdomain && sk_address_is_local(disp->addr); authfp = fopen(authfilename, "rb"); if (!authfp) @@ -303,8 +376,8 @@ void x11_get_auth_from_authfile(struct X11Display *disp, /* Records in .Xauthority contain four strings of up to 64K each */ buf = snewn(65537 * 4, char); - while (1) { - int c, i, j; + while (!ideal_match) { + int c, i, j, match = FALSE; #define GET do { c = fgetc(authfp); if (c == EOF) goto done; c = (unsigned char)c; } while (0) /* Expect a big-endian 2-byte number giving address family */ @@ -370,41 +443,54 @@ void x11_get_auth_from_authfile(struct X11Display *disp, continue; /* don't recognise this protocol, look for another */ switch (family) { - case 0: + case 0: /* IPv4 */ if (!disp->unixdomain && sk_addrtype(disp->addr) == ADDRTYPE_IPV4) { char buf[4]; sk_addrcopy(disp->addr, buf); - if (len[0] == 4 && !memcmp(str[0], buf, 4)) - goto found; + if (len[0] == 4 && !memcmp(str[0], buf, 4)) { + match = TRUE; + /* If this is a "localhost" entry, note it down + * but carry on looking for a Unix-domain entry. */ + ideal_match = !localhost; + } } break; - case 6: + case 6: /* IPv6 */ if (!disp->unixdomain && sk_addrtype(disp->addr) == ADDRTYPE_IPV6) { char buf[16]; sk_addrcopy(disp->addr, buf); - if (len[0] == 16 && !memcmp(str[0], buf, 16)) - goto found; + if (len[0] == 16 && !memcmp(str[0], buf, 16)) { + match = TRUE; + ideal_match = !localhost; + } } break; - case 256: - if (disp->unixdomain && !strcmp(disp->hostname, str[0])) - goto found; + case 256: /* Unix-domain / localhost */ + if ((disp->unixdomain || localhost) + && ourhostname && !strcmp(ourhostname, str[0])) + /* A matching Unix-domain socket is always the best + * match. */ + match = ideal_match = TRUE; break; } - } - found: - disp->localauthproto = protocol; - disp->localauthdata = snewn(len[3], unsigned char); - memcpy(disp->localauthdata, str[3], len[3]); - disp->localauthdatalen = len[3]; + if (match) { + /* Current best guess -- may be overridden if !ideal_match */ + disp->localauthproto = protocol; + sfree(disp->localauthdata); /* free previous guess, if any */ + disp->localauthdata = snewn(len[3], unsigned char); + memcpy(disp->localauthdata, str[3], len[3]); + disp->localauthdatalen = len[3]; + } + } done: fclose(authfp); memset(buf, 0, 65537 * 4); sfree(buf); + sfree(ourhostname); } static void x11_log(Plug p, int type, SockAddr addr, int port, -- 2.11.0