Subject: Bug#1183: Pass broker creds to RDP client as plaintext

From: Walid MOGHRABI <[email-masked]>
Date: Wed, 31 May 2017 16:40:49 +0200 (CEST)

package: x2goclient
priority: wishlist
tags: patch

This is a proposal patch in order to let the RDP client use the credentials used at broker auth login so that users can enter them only once in broker mode.

This patch also add support for --close-disconnect in broker mode + RDP

Regards,
Walid Moghrabi

TRAVAUX.COM
BAT I - PARC CEZANNE 2 290 AVENUE GALILEE - CS 80403
13591 AIX EN PROVENCE CEDEX 3

---
DISCLAIMER: This e-mail is private and confidential and may contain proprietary or legally privileged information. It is for the intended recipient only. If you have received this email in error, please notify the author by replying to it and then destroy it. If you are not the intended recipient you must not use, disclose, distribute, copy, print or rely on this e-mail or any attachment. Thank you

--- /home/walid/Bureau/x2goclient-current/src/help.cpp
+++ /home/walid/Bureau/x2goclient/src/help.cpp
@@ -158,6 +158,7 @@
   ADD_OPT ("--broker-ssh-key=<path to key>", QT_TRANSLATE_NOOP ("Help", "Sets the path to an SSH key to use for authentication against an SSH session broker. The client's behavior is undefined if this flag is used for non-SSH session brokers."));
   ADD_OPT ("--broker-autologin", QT_TRANSLATE_NOOP ("Help", "Enables the use of the default SSH key or SSH agent for authentication against an SSH session broker. The client's behavior is undefined if this flag is used for non-SSH session brokers."));
   ADD_OPT ("--broker-noauth", QT_TRANSLATE_NOOP ("Help", "Does not ask for user credentials during session broker authentication. This can be useful if you are using an HTTP(S) session broker without authentication. If you run an HTTP(S) server without authentication, but with user-specific profiles, then put the user name into the broker URL (refer to --broker-url.) The user name then will be extracted from the broker URL and be sent to the session broker. The client's behavior is undefined if this flag is used for non-HTTP(S) session brokers."));
+  ADD_OPT ("--use-broker-creds-for-rdp-session", QT_TRANSLATE_NOOP ("Help", "Pass broker credentials as plain text to directRDP sessions when using broker mode with broker authentication."));
   ADD_OPT ("--background=<svg-file>", QT_TRANSLATE_NOOP ("Help", "Use a custom/branded background image (SVG format) for X2Go Client's main window."));
   ADD_OPT ("--branding=<svg-file>", QT_TRANSLATE_NOOP ("Help", "Use a custom icon (SVG format) for additional branding to replace the default in the lower left corner of X2Go Client's main window."));

--- /home/walid/Bureau/x2goclient-current/src/onmainwindow.h
+++ /home/walid/Bureau/x2goclient/src/onmainwindow.h
@@ -609,6 +609,7 @@
     bool keepTrayIcon;
     bool hideFolderSharing;
     bool brokerNoauthWithSessionUsername;
+    bool brokerCredsForRDPSession;
     bool defaultUseSound;
     bool defaultXinerama;
     bool cardStarted;

--- /home/walid/Bureau/x2goclient-current/src/onmainwindow.cpp
+++ /home/walid/Bureau/x2goclient/src/onmainwindow.cpp
@@ -74,6 +74,7 @@
     keepTrayIcon=false;
     hideFolderSharing=false;
     brokerNoauthWithSessionUsername=false;
+    brokerCredsForRDPSession=false;
     thinMode=false;
     closeDisconnect=false;
     showHaltBtn=false;
@@ -3482,8 +3483,16 @@
     }
     else
     {
-        user=st->setting()->value ( sid+"/user",
-                                       ( QVariant ) "").toString();
+        if (brokerCredsForRDPSession)
+        {
+            user=login->text();
+            password=pass->text();
+        }
+        else
+        {
+            user=st->setting()->value ( sid+"/user",
+                                           ( QVariant ) "").toString();
+        }
     }
 
     nxproxy=new QProcess;
@@ -6230,6 +6239,10 @@
 #ifdef Q_OS_LINUX
     if (directRDP)
     {
+        if (closeDisconnect)
+        {
+            close();
+        }
         pass->setText ( "" );
         QTimer::singleShot ( 2000,this,
                              SLOT ( slotShowPassForm() ) );
@@ -7425,6 +7438,12 @@
         return true;
     }
 
+    if ( param=="--use-broker-creds-for-rdp-session" )
+    {
+        brokerCredsForRDPSession=true;
+        return true;
+    }
+
     //force to show trayicon
     if (param == "--tray-icon")
     {

---

From: Mike Gabriel <[email-masked]>
Date: Wed, 31 May 2017 19:58:28 +0000

Hi Walid,

On  Mi 31 Mai 2017 16:40:49 CEST, Walid MOGHRABI wrote:

> package: x2goclient
> priority: wishlist
> tags: patch
>
> This is a proposal patch in order to let the RDP client use the  
> credentials used at broker auth login so that users can enter them  
> only once in broker mode.
>

I just looked at your patch.

Requests before patch can get accepted:

   1. Please split up the RDP broker creds as session creds from the  
--close-disconnect change.
   2. Please let the cmdline option start with --broker-...

            --broker-use-creds-for-session

   3. Don't limit this functionality to RDP sessions only. It is  
useful for all sorts of session
      types (X2Go, DirectRDP, DirectXDMCP if already in (there were  
rumours about such a new feature)).

Thanks for your work on X2Go,
Mike

-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
mobile: +49 (1520) 1976 148
landline: +49 (4354) 8390 139

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22  0782 9AF4 6B30 2577 1B31
mail: [email-masked], http://das-netzwerkteam.de

---

From: Walid MOGHRABI <[email-masked]>
Date: Thu, 1 Jun 2017 10:46:55 +0200 (CEST)

> --close-disconnect change. 

This little fix is related to this support since, in that particular case which is broker mode + RDP session + --close-disconnect activated, you couldn't have a one time authentication (at broker auth).
For that "one time auth" to work, I need a way to pass broker credentials to the session and to close the client at the end of the session in order to force a re-auth at broker login.
Without the --close-disconnect fix, I can pass my credentials to the RDP session but when finishing the session, I'm still on the broker page with my session list and I don't re-auth which is what I wanted.
I can easily split these patches since they are quite clearly separated but I thought they were related to the same need that's why I kept them together.

> --broker-use-creds-for-session 

ok

> rumours about such a new feature)). 

Well, I'm not aware of XDMCP and have nothing under my hand to test it.
This patch affect RDP sessions only in fact because X2Go sessions have heir own way to pass credentials from broker to x2go server with the intermediate key auth so using this method for this kind of session is purely useless.
On the other hand, RDP sessions have no such key authentication available so it is necessary to pass credentials as plaintext to xfreerdp/rdesktop because in the case of the broker mode only, when clicking on the session profile, the client is waiting for the credentials but you are not prompted for them so the client stay stuck in an unusable situation.
So really, this is a "broker + RDP only" method that's why I precised this was for RDP only in order not to confuse users who might think this could be used for any type of connection.

I'll modify the cmdline option name and wait for your comments on my precisions. 

Regards,
Walid Moghrabi

TRAVAUX.COM

----- Mail original ----- 

De: "Mike Gabriel" <[email-masked]> 
À: "Walid MOGHRABI" <[email-masked]>, [email-masked] 
Envoyé: Mercredi 31 Mai 2017 21:58:28 
Objet: Re: [X2Go-Dev] Bug#1183: Pass broker creds to RDP client as plaintext 

Hi Walid, 

-- 

DAS-NETZWERKTEAM

---

From: Mike Gabriel <[email-masked]>
Date: Sat, 03 Jun 2017 21:48:36 +0000

HI Walid,

On  Do 01 Jun 2017 10:46:55 CEST, Walid MOGHRABI wrote:

> This little fix is related to this support since, in that particular  
> case which is broker mode + RDP session + --close-disconnect  
> activated, you couldn't have a one time authentication (at broker  
> auth).
> For that "one time auth" to work, I need a way to pass broker  
> credentials to the session and to close the client at the end of the  
> session in order to force a re-auth at broker login.
> Without the --close-disconnect fix, I can pass my credentials to the  
> RDP session but when finishing the session, I'm still on the broker  
> page with my session list and I don't re-auth which is what I wanted.
> I can easily split these patches since they are quite clearly  
> separated but I thought they were related to the same need that's  
> why I kept them together.

Please split off the change for --close-disconnect into a separate commit.

> This patch affect RDP sessions only in fact because X2Go sessions  
> have heir own way to pass credentials from broker to x2go server  
> with the intermediate key auth so using this method for this kind of  
> session is purely useless.
> On the other hand, RDP sessions have no such key authentication  
> available so it is necessary to pass credentials as plaintext to  
> xfreerdp/rdesktop because in the case of the broker mode only, when  
> clicking on the session profile, the client is waiting for the  
> credentials but you are not prompted for them so the client stay  
> stuck in an unusable situation.
> So really, this is a "broker + RDP only" method that's why I  
> precised this was for RDP only in order not to confuse users who  
> might think this could be used for any type of connection.
>

IMHO, the --broker-use-creds-for-session could be a nice and cheap  
alternative to setting up x2gobroker-agent based authentication. So,  
it would be nice to have it working for X2Go and RDP sessions.

Thanks,
Mike
-- 

DAS-NETZWERKTEAM