from irc: <no_where> I believe I have found a bug in libxcb: src/xcb_auth.c. Line 190 reads: <no_where> APPEND(info->data, j, si6->sin6_addr.s6_addr[12]); <no_where> As far as I can see, this will copy exactly one byte, because sizeof(si6->sin6_addr.s6_addr[12]) is 1. But it should copy four bytes, namely the IPv6-mapped IPv4-address from bytes 12 through 15. <no_where> Can some knowledgable person confirm this, and if this analysis is correct, file a bug report? <christoph4> no_where: hmm, your statement makes sense <no_where> christoph4: at least in the old libX11/.../ConnDis.c, 4 bytes were copied explicitly i've checked the xcb_auth.c code of current git head, and it matches the description
Created attachment 23866 [details] [review] patch This patch should fix it. Can someone test it ?
I have implemented exactly the same patch and can confirm that it works. Regards, no where
commit fcb433db80315a44154248a9229c9cfcbab63f04 Author: Julien Danjou <julien@danjou.info> Date: Sun Mar 15 10:18:50 2009 +0100 Copy full IPv4 mapping (Bug #20665) Signed-off-by: Julien Danjou <julien@danjou.info>
xcb_auth.c: In function 'compute_auth': xcb_auth.c:190: warning: passing argument 2 of 'do_append' makes pointer from integer without a cast -do_append(info->data, j, &si6->sin6_addr.s6_addr[12], 4); +do_append(info->data, &j, &si6->sin6_addr.s6_addr[12], 4); Thanks for taking care of this, Julien.
blush... yes, this is the line I have: do_append(info->data, &j, &(si6->sin6_addr.s6_addr[12]), 4); actually, I do this for all IPv6 addresses, so that the patch looks like this: --- ./src/xcb_auth.c.ORIG 2008-08-28 13:49:21.000000000 +0200 +++ ./src/xcb_auth.c 2009-03-14 18:04:22.000000000 +0100 @@ -185,21 +185,13 @@ case AF_INET6: /*block*/ { struct sockaddr_in6 *si6 = (struct sockaddr_in6 *) sockname; - if(IN6_IS_ADDR_V4MAPPED(SIN6_ADDR(sockname))) - { - APPEND(info->data, j, si6->sin6_addr.s6_addr[12]); - APPEND(info->data, j, si6->sin6_port); - } - else - { - /* XDM-AUTHORIZATION-1 does not handle IPv6 correctly. Do the - same thing Xlib does: use all zeroes for the 4-byte address - and 2-byte port number. */ - uint32_t fakeaddr = 0; - uint16_t fakeport = 0; - APPEND(info->data, j, fakeaddr); - APPEND(info->data, j, fakeport); - } + /* XXX + copy low-order address values only (not enough + space for a full IPv6 address) + note that the original code is false as it copies only one + byte of the address instead of four */ + do_append(info->data, &j, &(si6->sin6_addr.s6_addr[12]), 4); + APPEND(info->data, j, si6->sin6_port); } break; #endif and also for libX11 (although it's now unused): --- ./src/ConnDis.c.ORIG 2009-02-17 15:24:40.000000000 +0100 +++ ./src/ConnDis.c 2009-03-14 17:35:11.000000000 +0100 @@ -1111,27 +1111,14 @@ #endif /* AF_INET */ #if defined(IPv6) && defined(AF_INET6) case AF_INET6: - /* XXX This should probably never happen */ { - unsigned char ipv4mappedprefix[] = { - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff }; - - /* In the case of v4 mapped addresses send the v4 - part of the address - addr is already in network byte order */ - if (memcmp((char*)addr+8, ipv4mappedprefix, 12) == 0) { - for (i = 20 ; i < 24; i++) - xdmcp_data[j++] = ((char *)addr)[i]; - - /* Port number */ - for (i=2; i<4; i++) - xdmcp_data[j++] = ((char *)addr)[i]; - } else { - /* Fake data to keep the data aligned. Otherwise the - the server will bail about incorrect timing data */ - for (i = 0; i < 6; i++) { - xdmcp_data[j++] = 0; - } - } + /* XXX + copy low-order address values only (not enough + space for a full IPv6 address) */ + for (i=20; i<24; i++) /* do sin6_addr */ + xdmcp_data[j++] = ((char *)addr)[i]; + for (i=2; i<4; i++) /* do sin6_port */ + xdmcp_data[j++] = ((char *)addr)[i]; break; } #endif /* AF_INET6 */ and then I can get XDM-AUTHORIZATION-1 working for IPv6 with both kdm and xdm: --- ./kdm/backend/auth.c.ORIG 2009-03-14 16:45:56.000000000 +0100 +++ ./kdm/backend/auth.c 2009-03-14 16:59:53.000000000 +0100 @@ -628,12 +628,15 @@ debug( "Skipping IPv6 localhost address\n" ); continue; } +/* XXX */ +#if 0 /* Also skip XDM-AUTHORIZATION-1 */ if (auth->name_length == 19 && !memcmp( auth->name, "XDM-AUTHORIZATION-1", 19 )) { debug( "Skipping IPv6 XDM-AUTHORIZATION-1\n" ); continue; } +#endif } # endif writeAddr( family, len, addr, file, auth, ok ); @@ -884,12 +887,15 @@ debug( "Skipping IPv6 localhost address\n" ); continue; } +/* XXX */ +#if 0 /* Also skip XDM-AUTHORIZATION-1 */ if (auth->name_length == 19 && !memcmp( auth->name, "XDM-AUTHORIZATION-1", 19 )) { debug( "Skipping IPv6 XDM-AUTHORIZATION-1\n" ); continue; } +#endif } #endif } --- ./auth.c.ORIG 2008-05-21 20:08:45.000000000 +0200 +++ ./auth.c 2009-03-14 18:09:11.000000000 +0100 @@ -756,12 +756,6 @@ Debug ("Skipping IPv6 localhost address\n"); continue; } - /* Also skip XDM-AUTHORIZATION-1 */ - if (auth->name_length == 19 && - strcmp(auth->name, "XDM-AUTHORIZATION-1") == 0) { - Debug ("Skipping IPv6 XDM-AUTHORIZATION-1\n"); - continue; - } } #endif writeAddr(family, len, addr, file, auth); @@ -1056,12 +1050,6 @@ Debug ("Skipping IPv6 localhost address\n"); continue; } - /* Also skip XDM-AUTHORIZATION-1 */ - if (auth->name_length == 19 && - strcmp(auth->name, "XDM-AUTHORIZATION-1") == 0) { - Debug ("Skipping IPv6 XDM-AUTHORIZATION-1\n"); - continue; - } } #endif } Even though it may not be fully kosher, it's working nicely. (I do a lot of remote X programs via IPv6 and with XDM-AUTHORIZATION-1.) Please forgive the mess I did to this bug report. :-)
Err, my bad. OTOH, Did you test your patch version for non mapped IPv6 address?
Yes, I am using them right now (all four patches, to libxcb, libX11, old school xdm, and kdm) on several machines. Would probably need another similar patch for gdm. It's not pretty, but XDM-AUTHORIZATION-1 cannot really handly v6 addresses, and I thought that simply using only to least significant part of the address is good enough in this case. The nicer solution would have been to implement XDM-AUTHORIZATION-2 et al.
XDM-AUTHORIZATION-2 got stuck in the "sounds like a nice standard, but what's the point if no one cares enough to write code?" trap. The draft is still around, just waiting for someone to implement it and verify it works (and hopefully get someone with crypto background to verify it's a secure usage of the specified cryptography).
Yes, I know.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/lib/libxcb/issues/31.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.