X2Go Bug report logs - #1345
x2goclient overrides user-specified SSH port with wrong default

Full log

Message #5 received at submit@bugs.x2go.org (full text, mbox, reply):

Received: (at submit) by bugs.x2go.org; 22 Nov 2018 02:52:37 +0000
From m.r.nemoinis@antichef.net  Thu Nov 22 03:51:52 2018
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on
	ymir.das-netzwerkteam.de
X-Spam-Level: 
X-Spam-Status: No, score=0.8 required=3.0 tests=BAYES_50 autolearn=ham
	autolearn_force=no version=3.4.2
Received: from localhost (localhost [127.0.0.1])
	by ymir.das-netzwerkteam.de (Postfix) with ESMTP id D11BD5DAED
	for <submit@bugs.x2go.org>; Thu, 22 Nov 2018 03:51:51 +0100 (CET)
X-Virus-Scanned: Debian amavisd-new at ymir.das-netzwerkteam.de
Received: from ymir.das-netzwerkteam.de ([127.0.0.1])
	by localhost (ymir.das-netzwerkteam.de [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id 1POdq2TMOGr6 for <submit@bugs.x2go.org>;
	Thu, 22 Nov 2018 03:51:45 +0100 (CET)
X-Greylist: delayed 490 seconds by postgrey-1.35 at ymir.das-netzwerkteam.de; Thu, 22 Nov 2018 03:51:45 CET
Received: from cmta18.telus.net (cmta18.telus.net [209.171.16.91])
	by ymir.das-netzwerkteam.de (Postfix) with ESMTPS id 5DAE15DACB
	for <submit@bugs.x2go.org>; Thu, 22 Nov 2018 03:51:45 +0100 (CET)
Received: from [192.168.0.88] ([99.199.50.138])
	by cmsmtp with SMTP
	id Pexng4RVLiBO5Pexogch5X; Wed, 21 Nov 2018 19:43:34 -0700
X-Authority-Analysis: v=2.3 cv=d5AkNirE c=1 sm=1 tr=0
 a=hiUt1CcxB3hVsh94y/8scw==:117 a=hiUt1CcxB3hVsh94y/8scw==:17
 a=IkcTkHD0fZMA:10 a=A1X0JdhQAAAA:8 a=lQECdqTcPlSpqJiwT_EA:9 a=QEXdDO2ut3YA:10
 a=Df3jFdWbhGDLdZNm0fyq:22
From: m.r.nemoinis@antichef.net
Subject: x2goclient overrides user-specified SSH port with wrong default
To: submit@bugs.x2go.org
Message-ID: <93eb64a6-848b-4ead-f3bf-abf648cd143b@antichef.net>
Date: Wed, 21 Nov 2018 18:43:30 -0800
User-Agent:
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-CMAE-Envelope: MS4wfKmRshVBqEAr/B6Sl9QDnQcCTQfGe6RdeZKLIa9etrGvoInDYdQebbrCZjaGrWzSkZsGV8wRt3mZw4qToaWS+HdNW4XPEMtHWKI6eGVjfJrVfhr5tyb/
 Kx5S2AyWOJjFraxUuGxw0adXqx921NDd6fAaqL9UhHxofNY5+avMgtNP2r5AZrQSwGSDtH8gSumVkQ==

Package: x2goclient
Version: 4.1.2.2
Severity: Important

OS: Linux Debian Buster
x2goclient:  4.1.2.2-0x2go1~git20181113.1828+10.heuler.1


There is a bug in x2goclient's port handling that was introduced when adding support for libssh
Config file (bug #1121):

sshmasterconnection.cpp uses ssh_options_parse_config in an attempt to obtain config-based
values for hostname and port, in case the values supplied by the user are short-hands or
defaults.  The problem is that sshmasterconnection takes the values returned by
ssh_options_parse_config even when the x2goclient user has given explicit hostnames and ports.

So let's say I want to connect to "frobisher.example.com" which has a ssh server on port 3322,
and I explicitly give these parameters in the x2goclient session.

All works well if "Host *" in /etc/ssh/ssh_config (and ~/.ssh/config) does not have a default port.
But if it does, like this:

Host *
  Port 6789

Then that port is taken and sshmasterconnection tries (and fails) to connect to
frobisher.example.com on port 6789.
The code should not override a port that is specified by the interactive user.
This is the way ssh works: command line options take precedence over config values, and so
system and user files are setup accordingly.

Second, what if the default port in /etc/ssh/ssh_config or ~/.ssh/config is actually 6789 (or
22, or any explicit value)? That fails too, because the following code in
SshMasterConnection::channelLoop() will override x2go's own channel forwarding ports:


/* The host and port might be a shorthand and zero, so fetch the actual data. */
if (ssh_options_parse_config (tmp_session, NULL) < 0) {
x2goDebug << "Warning: unable to parse the SSH config file.";
}

unsigned int inferred_port = 0;
ssh_options_get_port (tmp_session, &inferred_port);
x2goDebug << "Temporary session port after config file parse: " << inferred_port;

char *inferred_host = NULL;
ssh_options_get (tmp_session, SSH_OPTIONS_HOST, &inferred_host);
x2goDebug << "Temporary session host after config file parse: " << inferred_host;



Here is the debugging output of the code above when it works and when it doesn't:

GOOD (default Port commented out in /etc/ssh/ssh_config for "Host *"):

x2go-DEBUG-../src/sshmasterconnection.cpp:2111> Forwarding parameters: from remote
("localhost":49471) to local ("localhost":34508)
x2go-DEBUG-../src/sshmasterconnection.cpp:2148> Temporary session port after config file parse:
49471
x2go-DEBUG-../src/sshmasterconnection.cpp:2152> Temporary session host after config file parse:
localhost
x2go-DEBUG-../src/sshmasterconnection.cpp:2182> New channel forwarded.


BAD (/etc/ssh/ssh_config has an explicit "Port 6789"  for "Host *" ):

x2go-DEBUG-../src/sshmasterconnection.cpp:2111> Forwarding parameters: from remote
("localhost":40052) to local ("localhost":54592)
x2go-DEBUG-../src/sshmasterconnection.cpp:2148> Temporary session port after config file parse:
6789
x2go-DEBUG-../src/sshmasterconnection.cpp:2152> Temporary session host after config file parse:
localhost
x2go-DEBUG-../src/sshmasterconnection.cpp:2182> New channel forwarded.

Send a report that this bug log contains spam.