Subject: Bug#141: Fwd: [X2Go-Dev] autologin with x2goclient in broker-mode: analysis and fix for "enter passphrase"-bug

From: Mike Gabriel <[email-masked]>
Date: Sat, 20 Apr 2013 20:52:39 +0200

tag #141 - moreinfo
thanks

Detailed analysis from Anders below...

----- Weitergeleitete Nachricht von [email-masked] -----
      Datum: Fri, 19 Apr 2013 16:16:47 +0200
        Von: Anders Bruun Olsen <[email-masked]>
Antwort an: [email-masked]
    Betreff: [X2Go-Dev] autologin with x2goclient in broker-mode:  
analysis and fix for "enter passphrase"-bug
         An: x2go-dev <[email-masked]>

Hi guys,

I just spent most of the day digging through source code for x2goclient
(reminds my why I code Python rather than C++ :) ), trying to understand
why the "enter passphrase" dialog box appears when the broker is set to do
autologin.

Summary of the bug:
x2gobroker can be setup to do autologin of users, to avoid users having to
enter their credentials twice. This is accomplished by the broker placing a
temporary SSH public key in $HOME/.x2go/authorized_keys and handing the
matching private key to the client. This temporary key is then removed
after a short while. Unfortunately, on all machines I have tested with,
including thinclients, x2goclient pops up a dialog box with the text "Enter
passphrase to decrypt a key" after authenticating against the broker and
choosing a session with autologin enabled. Pressing cancel on this dialog
box will on my desktop machine result in the autologin completing and
getting logged in. However on the x2gothinclient I tested with, the dialog
box would just pop up again and again and login would never occur.

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.

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.

--
Anders Bruun Olsen
It-ansvarlig
Det Danske Sprog- og Litteraturselskab
(Society for Danish Language and Literature)

----- Ende der weitergeleiteten Nachricht -----

-- 

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

GnuPG Key ID 0x25771B31
mail: [email-masked], http://das-netzwerkteam.de

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

---

From: Mike Gabriel <[email-masked]>
Date: Mon, 22 Apr 2013 01:07:03 +0200

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,

> 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.

> 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

GnuPG Key ID 0x25771B31

freeBusy: