On 2/11/11 12:06 PM, Eric Stadtherr wrote:
> The port conflict arises when you start two vncviewers with the "-via"
> flag back-to-back pointed to two different servers. In lockstep, both
> viewers will:
>
> 1. look for a free TCP port on the local host within a pre-determined
> sequence (by binding and then closing a socket)
> 2. start an SSH connection to the specified host, with local port
> forwarding specified on the chosen port
> 3. connect the viewer's RFB connection to the local SSH port
>
> If the SSH connection takes long enough (a few seconds, perhaps), the
> port found by viewer #1 in step 1 will still free when viewer #2 goes
> through the same process, and viewer #2 will try to start SSH with the
> same local port. The second SSH will fail to bind to the local port, but
> nobody notices that. The second viewer then connects to the first
> viewer's local SSH process in step 3, and voila - the second viewer is
> now pointing to the first server. This is especially bad if these
> viewers are started by different users, and the second user doesn't have
> the credentials to connect to the first viewer's SSH server on their own.
>
> I use VNC in an environment where multiple users are all starting
> multiple VNC sessions in the morning (one for each screen / server
> combination). They've seen this bug a few times, but fortunately it has
> heretofore only resulted in cross-connects of the same user's viewers.
Yeah, I think the problem is that we're trying to iterate through a
specific port range ourselves to find a free port rather than letting
the system pick one for us. That leads to the race condition you
describe. I think that if we replace our port picking code as follows,
it should fix the issue. Of course, this means we're no longer
confining our local port to the range of 5500-5599. Does anyone see a
problem with that? I don't.
Index: common/network/TcpSocket.cxx
===================================================================
--- common/network/TcpSocket.cxx (revision 4280)
+++ common/network/TcpSocket.cxx (working copy)
@@ -71,7 +71,7 @@
/* Tunnelling support. */
int network::findFreeTcpPort (void)
{
- int sock, port;
+ int sock;
struct sockaddr_in addr;
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
@@ -80,15 +80,16 @@
if ((sock = socket (AF_INET, SOCK_STREAM, 0)) < 0)
throw SocketException ("unable to create socket", errorNumber);
- for (port = TUNNEL_PORT_OFFSET + 99; port > TUNNEL_PORT_OFFSET; port--) {
- addr.sin_port = htons ((unsigned short) port);
- if (bind (sock, (struct sockaddr *)&addr, sizeof (addr)) == 0) {
- closesocket (sock);
- return port;
- }
- }
- throw SocketException ("no free port in range", 0);
- return 0;
+ addr.sin_port = 0;
+ if (bind (sock, (struct sockaddr *)&addr, sizeof (addr)) < 0)
+ throw SocketException ("unable to find free port", errorNumber);
+
+ socklen_t n = sizeof(addr);
+ if (getsockname (sock, (struct sockaddr *)&addr, &n) < 0)
+ throw SocketException ("unable to get port number", errorNumber);
+
+ closesocket (sock);
+ return ntohs(addr.sin_port);
}
> No insult taken - this is a good discussion to have about a relatively
> significant implementation change.
>
> I sympathize with the pain introduced by GnuTLS, but this SSH change is
> different in several respects:
>
> 1. The SSH change fixes an existing capability, and doesn't introduce
> a new capability
Try out the patch above and see if it fixes the problem.
> 2. The existing SSH tunnel capability already had the same level of
> external dependency, albeit a runtime dependency on /usr/bin/ssh.
> Granted, the new implementation makes it a build-time dependency
> or a dynamic library dependency instead of a child process
> dependency. I would go so far as asserting that libssh is a more
> portable dependency than a hardcoded reference to "/usr/bin/ssh".
There is nothing preventing a -via option on Windows. It just hasn't
been implemented yet. We could use, for instance, PLink as the
tunneling process rather than /usr/bin/ssh. Also, on Unix, the VIA
command line can be configured at run time to use something other than
/usr/bin/ssh.
> 3. vncviewer is hardly unwieldy - have you tried building The GIMP?
> Wink Seriously, though, the existing "-via" logic would never
> have worked on Windows, so libssh might actually be a step towards
> better portability. Windows distros can be happily configured and
> built without libssh support, which leaves Windows users no better
> or worse off than they are today.
> 5. Only users that require/desire the SSH tunneling capability would
> be subject to any constraints imposed by the new implementation.
> This is different from the GnuTLS/VeNCrypt situation where the
> normal "vncviewer <server>:<screen>" case is affected.
Yeah, but one of the key things I want to avoid is a million different
configurations of TigerVNC. If everyone has their own "special sauce",
then a lot of features don't get tested -- particularly the features
that are difficult to build, like GnuTLS or (hypothetically) libssh.
TigerVNC may have different statistics, but with TurboVNC, the Windows
viewer is about 10x more popular than the Unix viewer, so if I want
something tested on the viewer side, I need it in the Windows viewer first.
> These are good topics to discuss, and I'd be happy to participate in the
> discussion.
>
> However, I'd still like to know how you and the rest of the community
> feel about this particular SSH change, and the implementation questions
> I raised. As I said, I don't think it has the same mainstream
> consequences as GnuTLS, and might actually be a step towards supporting
> the SSH tunneling capability on non-Unix platforms.
>
> What do you think?
If we can fix the existing solution, that would be my preference.
------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
Tigervnc-devel mailing list
Tigervnc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
