From mike.gabriel@das-netzwerkteam.de Mon Apr 22 01:07:04 2013 Received: (at control) by bugs.x2go.org; 21 Apr 2013 23:07:13 +0000 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 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) This message is in MIME format and has been PGP signed. --=_2v6u8siuil1z Content-Type: text/plain; charset=UTF-8; DelSp="Yes"; format="flowed" Content-Disposition: inline Content-Transfer-Encoding: 7bit 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 --=_2v6u8siuil1z Content-Type: application/pgp-signature Content-Description: Digitale PGP-Unterschrift Content-Disposition: inline Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iQIcBAABAgAGBQJRdHEXAAoJEJr0azAldxsxPfUP/A75z6Y5OtfeuYdFAZ2tEoQp XO4z6JIxU2oeyUcW9TKCNUM97yat70ErAwaZDnRntr6DCu8SLg9S5t9YM/U/+2zm 8qL/QRy30qXeUkgjfLso5MwII9wpA7EYY+Hl6M4guNQW4DiXiYcWlxtj+jNH3uX8 Hex87D+fob6bX6YKGDhAXLJIslPUmiat9mK8jBvphwm+6ZCccexi07rcPORsRtzM UcQpfKY2WIdakw0o2iO4Kgn5Y/VeM6MgMhhBHl35b/GNhC+Ui/djZj94UqIXmsQ+ Y6WT2yANcH5GJ9xawwPd6IpmkYSG61d3+QjQXL5i2zMGjCaVmULN18Ev9SDWD9qG H/KwhDbd/xf40ejAENZ9HLCb12gD8NGNycF9onbC8uiH8mxtxLJlcoFWKcgUjOfZ fRs3cwSF459tagTJIMfrlN/mHNc/XsSSJuof11iKsKoRRUA7sGNB52Hsxp1AeNrB XjXtbwgz7TsThdpWY+Boo7L8Th/XhqC6CW/55b8Cg6d+3FAx6G0hERAhoVuiDvxU rTdSG0X0NUekOHItNsLK3DhFIP5+/WS/gGiGC0/2vtMhIzSlzGgFbbl6XJOXrZna G1buYsDhTuWEt6F9f95nQl/PQPXm8DkOSc37hTFrxq7Jr8Tr9U+xTYBGrEFA/9fI 1u6peQeXME0k4T0h94Hs =GqkW -----END PGP SIGNATURE----- --=_2v6u8siuil1z--