On 02/11/2011 12:15 PM, DRC wrote:
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.
I see a couple problems with this solution:
1. This method still closes the socket after identifying a port,
which would allow the port to be allocated again before
/usr/bin/ssh finally gets around to opening the forwarding channel
and binding to the local port. A second vncviewer process started
in close proximity is admittedly *less* likely to choose that same
port, but the possibility still exists. The likelihood would be a
function of how the operating system assigns ephemeral ports,
which is certainly platform dependent and hard to predict.
2. The port number you'll end up with is an ephemeral port. Although
explicitly binding to and listening on an ephemeral port is
certainly allowed by ssh, it is also a little unconventional.
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.
Understood. Although I still think that libssh would get us one step
closer to Windows tunneling support than the existing system() call
method. libssh keeps vncviewer from having to encode the
platform-specific semantics of child process invocation and management.
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.
I guess I haven't seen any pain associated with building libssh. It
sounds like you had a bad experience with GnuTLS (and you're not the
only one!), but libssh doesn't depend on GnuTLS. I honestly can't speak
to the ease of building libssh on Windows, but only because I don't have
any Windows machines or VM's on which to play.
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
------------------------------------------------------------------------------
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
