X2Go Bug report logs - #184
missing public SSH key file should throw an error

Full log

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

Received: (at control) by bugs.x2go.org; 21 Apr 2013 23:07:13 +0000
From mike.gabriel@das-netzwerkteam.de  Mon Apr 22 01:07:04 2013
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on
	ymir.das-netzwerkteam.de
X-Spam-Level: 
X-Spam-Status: No, score=0.0 required=5.0 tests=URIBL_BLOCKED
	autolearn=unavailable version=3.3.2
Received: from freya.das-netzwerkteam.de (freya.das-netzwerkteam.de [88.198.48.199])
	by ymir (Postfix) with ESMTPS id 713FE5DB20;
	Mon, 22 Apr 2013 01:07:04 +0200 (CEST)
Received: from grimnir.das-netzwerkteam.de (grimnir.das-netzwerkteam.de [78.46.204.98])
	by freya.das-netzwerkteam.de (Postfix) with ESMTPS id 25ABFDE4;
	Mon, 22 Apr 2013 01:07:04 +0200 (CEST)
Received: from localhost (localhost [127.0.0.1])
	by grimnir.das-netzwerkteam.de (Postfix) with ESMTP id 0B1E53B954;
	Mon, 22 Apr 2013 01:07:04 +0200 (CEST)
X-Virus-Scanned: Debian amavisd-new at grimnir.das-netzwerkteam.de
Received: from grimnir.das-netzwerkteam.de ([127.0.0.1])
	by localhost (grimnir.das-netzwerkteam.de [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id I8ya1NeauB+2; Mon, 22 Apr 2013 01:07:03 +0200 (CEST)
Received: from localhost (localhost [127.0.0.1])
	by grimnir.das-netzwerkteam.de (Postfix) with ESMTP id DEFAD3B957;
	Mon, 22 Apr 2013 01:07:03 +0200 (CEST)
Received: from localhost (localhost [127.0.0.1])
	by grimnir.das-netzwerkteam.de (Postfix) with ESMTP id BA8CA3B954;
	Mon, 22 Apr 2013 01:07:03 +0200 (CEST)
Received: by grimnir.das-netzwerkteam.de (Postfix, from userid 33)
	id 6550E3B957; Mon, 22 Apr 2013 01:07:03 +0200 (CEST)
Received: from 146-176-142-46.pool.kielnet.net
 (146-176-142-46.pool.kielnet.net [46.142.176.146]) by
 mail.das-netzwerkteam.de (Horde Framework) with HTTP; Mon, 22 Apr 2013
 01:07:03 +0200
Message-ID: <20130422010703.19292xwil9lzpfdj@mail.das-netzwerkteam.de>
X-Priority: 3 (Normal)
Date: Mon, 22 Apr 2013 01:07:03 +0200
From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>
To: 141@bugs.x2go.org
Cc: control@bugs.x2go.org, 141-submitter@bugs.x2go.org
Subject: Re: [X2Go-Dev] Bug#141: Fwd: autologin with x2goclient in
 broker-mode: analysis and fix for "enter passphrase"-bug
References: <20130420205239.87507fsoftcfnqbb@mail.das-netzwerkteam.de>
In-Reply-To: <20130420205239.87507fsoftcfnqbb@mail.das-netzwerkteam.de>
MIME-Version: 1.0
Content-Type: multipart/signed;
 boundary="=_2v6u8siuil1z";
 protocol="application/pgp-signature";
 micalg="pgp-sha1"
Content-Transfer-Encoding: 7bit
User-Agent: Internet Messaging Program (IMP) H3 (4.3.4)

[Message part 1 (text/plain, inline)]

clone #141 -1
retitle -1 missing public SSH key file should throw an error
severity -1 minor
tag #141 pending
fixed #141 4.0.1.1
thanks

Hi Anders,

On Sa 20 Apr 2013 20:52:39 CEST Mike Gabriel wrote:

> Analysis of the bug:
> When autologin is enabled, SshMasterConnection::userAuth() will react by
> calling userAuthAuto(), which will look for ssh keys and if you, like me,
> have an ssh key with a passphrase, it will want to try out this key by
> asking for the passphrase (despite having ssh-agent running). If it does
> not find a key, it also asks for a passphrase, at least on my system. The
> reasons for this aren't really important here, in my oppinion. The
> important question here is why it even looks for other keys when the nice
> broker has provided a key. Further analysis and testing showed me that
> after userAuthAuto() exists without having gotten a proper key loaded (by
> pressing Cancel on the dialog box), userAuth() will then test if a key is
> loaded. And because httpbrokerclient has recieved a key and put it into the
> config-variable, a key IS available. This key is then used for login and
> all is good. Looking closer at the code revealed that setting
> config->autologin to true was actually not needed at all, and is the
> culprit here. If autologin is false, then userAuth() will still see that
> there is a key loaded, and happily log in the user.

Thanks for this detailled analysis. It indeed put me on some trail  
that worked.

> My naive fix for this bug:
> In ONMainWindow::startSession(), make setting the autologin variable
> dependent upon not being in brokerMode:
>
> diff --git a/onmainwindow.cpp b/onmainwindow.cpp
> index 31dbc17..bc2b70f 100644
> --- a/onmainwindow.cpp
> +++ b/onmainwindow.cpp
> @@ -3249,8 +3249,9 @@ bool ONMainWindow::startSession ( const QString& sid )
>
>      QString cmd=st->setting()->value ( sid+"/command",
>                                         ( QVariant ) QString::null
> ).toString();
> -    autologin=st->setting()->value ( sid+"/autologin",
> -                                     ( QVariant ) false ).toBool();
> +    if (!brokerMode)
> +        autologin=st->setting()->value ( sid+"/autologin",
> +                                         ( QVariant ) false ).toBool();
>      krblogin=st->setting()->value ( sid+"/krblogin",
>                                      ( QVariant ) false ).toBool();
>  #ifdef Q_OS_LINUX
>
> I can't say what other consequences this might have, not knowing the code
> well enough, but initial tests on my system shows that it works. This patch
> is against git/master btw.

The above fix is not appropriate as it will disable the autologin  
feature completely when x2goclient is in broker mode. That works for  
your setup, but is not a generic solution.

My approach is here:
http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=fe4408b12c982b81c56c52f37230865f4e9f41ea

@Alex: please cross-check. Thanks!

Greets,
Mike


-- 

DAS-NETZWERKTEAM
mike gabriel, rothenstein 5, 24214 neudorf-bornstein
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

[Message part 2 (application/pgp-signature, inline)]

Send a report that this bug log contains spam.