Subject: Bug#241: Changed host key cannot be updated

From: Heinrich Schuchardt <[email-masked]>
Date: Sun, 16 Jun 2013 14:36:32 +0200

Package: x2goclient
Version: 4.0.0.3
Severity: normal

Dear maintainer,

from time to time the SSH key used for identification by a X2GO server 
may change.

When trying to connect the server a pop up is shown:

"Anmeldung fehlgeschlagen"
"Host-Key des Servers hat sich geändert Er lautet jetzt:
00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
Aus Sicherheitsgründen wird die Verbindung abgebrochen"

The user is left puzzled with what he should do next.

There is no indication in which file there is a problem, e.g.
~/.ssh/known_hosts
or
%APPDATA%\ssh\known_hosts

There is no indication which entry in this file is corrupted.

Deleting file known_hosts is a bad idea because it may contain the keys 
for dozens of validated servers.

There are examples of more informative output, e.g. from command line 
program ssh:

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@    WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!     @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
IT IS POSSIBLE THAT SOMEONE IS DOING SOMETHING NASTY!
Someone could be eavesdropping on you right now (man-in-the-middle attack)!
It is also possible that a host key has just been changed.
The fingerprint for the RSA key sent by the remote host is
00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00.
Please contact your system administrator.
Add correct host key in /home/user/.ssh/known_hosts to get rid of this 
message.
Offending RSA key in /home/user/.ssh/known_hosts:1
RSA host key for 10.0.0.5 has changed and you have requested strict 
checking.
Host key verification failed.

Here I can identify the filename: /home/user/.ssh/known_hosts
and the line of the the entry: 1

Manual editing of known_hosts is now possible but not too good an idea 
because it is error prone.

A good solution is what you see in PuTTY. A warning pop up is shown and 
you get the choice to update file known_hosts.

Best regards

Heinrich Schuchardt

---

From: Mike Gabriel <[email-masked]>
Date: Fri, 21 Jun 2013 10:20:49 +0200

Hi Heinrich,

On So 16 Jun 2013 14:36:32 CEST Heinrich Schuchardt wrote:

> Dear maintainer,
>
> from time to time the SSH key used for identification by a X2GO  
> server may change.
>
> Deleting file known_hosts is a bad idea because it may contain the  
> keys for dozens of validated servers.
>
> There are examples of more informative output, e.g. from command  
> line program ssh:
>
> Add correct host key in /home/user/.ssh/known_hosts to get rid of  
> this message.
> RSA host key for 10.0.0.5 has changed and you have requested strict checking.
> Manual editing of known_hosts is now possible but not too good an  
> idea because it is error prone.
>
> A good solution is what you see in PuTTY. A warning pop up is shown  
> and you get the choice to update file known_hosts.
>
> Best regards

The above surely is a good point to discuss first before implementing.

Obviously, such a replace-host-key button would improve usability in  
case host key changes occur.

However, if someone captured DNS and replaced my X2Go server by an  
agressive X2Go server, I (as developer) surely want to protect the  
user from simply klicking ,,Yeah, ok man... replace that host key...  
and can we go on then please...''.

The SSH-unexperienced user (i.e. probably nearly everyone in the  
windows world) will then just simply click ,,replace host key''.

So, for me this kind of replace-host-key dialog should at least have a  
double confirmation check dialog: Are you sure to replace... -> Are  
you really sure???. That kind of thing.

Heinrich: if you could come up with a patch for this issue, it would  
surely speed up an inclusion of your requested feature.

@all: comments, opinions on such a new feature?

Mike

-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
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: Fri, 21 Jun 2013 10:53:24 +0200

Hi Stefan, hi Heinrich,

On Fr 21 Jun 2013 10:42:12 CEST Stefan Baur wrote:

> Am 21.06.2013 10:20, schrieb Mike Gabriel:
>
> This has been discussed previously on the list - I believe it was  
> before the introduction of X2Go-BTS, which is why there's no  
> bug/ticket regarding it.
> Subject was: "Feature Request: update ssh public key fingerprint  
> from within x2goclient"
> Date: 2012-02-17 14:41
> Message-ID: <[email-masked]>
>
> We need to find a middle ground between the current "No soup for  
> you!" and the way-too-easy "Update host key [Y]/n" prompt.
>
> My suggestion back then was:
> "I would like to suggest adding an option to remove/update the key  
> from within the X2Go-Client. However, to avoid "user click-through",  
> it should be somewhere in the menu, and the popup message should be  
> amended with a note pointing to that menu."

This sounds like a nice solution. Heinrich, are you willing to work on  
this new feature? As you have also worked on the https proxy support  
code, I guess you are capable to perform this task ;-).

Otherwise, we will keep this wishlist request on the list, but do not  
expect a prompt solution for this.

Thanks+Greets,
Mike

-- 

DAS-NETZWERKTEAM

GnuPG Key ID 0x25771B31

freeBusy:

---

From: Stefan Baur <[email-masked]>
Date: Fri, 21 Jun 2013 10:42:12 +0200

> @all: comments, opinions on such a new feature?

This has been discussed previously on the list - I believe it was before 
the introduction of X2Go-BTS, which is why there's no bug/ticket 
regarding it.
Subject was: "Feature Request: update ssh public key fingerprint from 
within x2goclient"
Date: 2012-02-17 14:41
Message-ID: <[email-masked]>

We need to find a middle ground between the current "No soup for you!" 
and the way-too-easy "Update host key [Y]/n" prompt.

My suggestion back then was:
"I would like to suggest adding an option to remove/update the key from 
within the X2Go-Client. However, to avoid "user click-through", it 
should be somewhere in the menu, and the popup message should be amended 
with a note pointing to that menu."

-Stefan

---

From: [email-masked]
Date: Sat, 22 Jun 2013 16:34:46 +0200

From: Heinrich Schuchardt <[email-masked]>

The appended patch allows to updated changed host keys.

It does not include the necessary changes for the translations.

Best regards

Heinrich Schuchardt

Signed-off-by: Heinrich Schuchardt <[email-masked]>
---
 onmainwindow.cpp |   67 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/onmainwindow.cpp b/onmainwindow.cpp
index b707d84..d0993f2 100644
--- a/onmainwindow.cpp
+++ b/onmainwindow.cpp
@@ -2953,33 +2953,58 @@ void ONMainWindow::slotSshServerAuthError ( int error, QString sshMessage, SshMa
     {
     case SSH_SERVER_KNOWN_CHANGED:
         errMsg=tr ( "Host key for server changed.\nIt is now: " ) +sshMessage+"\n"+
-               tr ( "For security reasons, connection will be stopped" );
-        connection->writeKnownHosts(false);
-        connection->wait();
-        if(sshConnection && sshConnection !=connection)
+               tr ( "This can be an indication of a man-in-the-middle attack.\n"
+                    "Somebody might be eavesdropping on you.\n"
+                    "For security reasons, it is recommended to stop the connection.\n"
+                    "Do you want to terminate the connection?\n" );
+        if ( !QMessageBox::warning( 0, tr( "Host key verification failed" ),
+                errMsg, tr( "Yes" ), tr( "No" ) ) != 0)
+            {
+            connection->writeKnownHosts(false);
+            connection->wait();
+            if(sshConnection && sshConnection !=connection)
+            {
+                sshConnection->wait();
+                delete sshConnection;
+            }
+            slotSshUserAuthError ( tr ( "Host key verification failed" ) );
+            sshConnection=0;
+            return;
+        }
+        else
         {
-            sshConnection->wait();
-            delete sshConnection;
+            errMsg = tr( "If you accept the new host key the security of your "
+                         "connection may be compromised.\n"
+                         "Do you want to update the host key?" );
         }
-        sshConnection=0;
-        slotSshUserAuthError ( errMsg );
-        return;
-
+        break;
     case SSH_SERVER_FOUND_OTHER:
         errMsg=tr ( "The host key for this server was not found but an other"
-                    "type of key exists.An attacker might change the default server key to"
-                    "confuse your client into thinking the key does not exist" );
-        connection->writeKnownHosts(false);
-        connection->wait();
-        if(sshConnection && sshConnection !=connection)
+                    "type of key exists. An attacker might change the default server key to "
+                    "confuse your client into thinking the key does not exist. \n"
+                    "For security reasons, it is recommended to stop the connection.\n"
+                    "Do you want to terminate the connection?\n");
+        if ( !QMessageBox::warning( 0, tr( "Host key verification failed" ),
+                errMsg, tr( "Yes" ), tr( "No" ) ) != 0)
+            {
+            connection->writeKnownHosts(false);
+            connection->wait();
+            if(sshConnection && sshConnection !=connection)
+            {
+                sshConnection->wait();
+                delete sshConnection;
+            }
+            slotSshUserAuthError ( tr ( "Host key verification failed" ) );
+            sshConnection=0;
+            return;
+        }
+        else
         {
-            sshConnection->wait();
-            delete sshConnection;
+            errMsg = tr( "If you accept the new host key the security of your "
+                         "connection may be compromised.\n"
+                         "Do you want to update the host key?" );
         }
-        sshConnection=0;
-        slotSshUserAuthError ( errMsg );
-        return ;
-
+        break;
     case SSH_SERVER_ERROR:
         connection->writeKnownHosts(false);
         connection->wait();
-- 
1.7.10.4

---

From: Nable 80 <[email-masked]>
Date: Sat, 22 Jun 2013 21:09:11 +0400

> +                errMsg, tr( "Yes" ), tr( "No" ) ) != 0)
I think that using `!' in the beginning and `!=' in the end is a great
idea to confuse everybody, including those who have written this piece
of code.
Or did I understood these lines in a wrong way (here is some irony) ?

---

From: Mike Gabriel <[email-masked]>
Date: Fri, 19 Jul 2013 18:31:24 +0200

Hi Heinrich,

On Sa 22 Jun 2013 16:34:46 CEST  wrote:

>  onmainwindow.cpp |   67  
> +++++++++++++++++++++++++++++++++++++-----------------
> [...]

I just realized that I have not reacted to this one yet.

As the patch introduces changes on translatable strings, this patch  
will only be applied after release 4.0.1.1.

Thanks for this piece of work.

Mike

-- 

DAS-NETZWERKTEAM

GnuPG Key ID 0x25771B31

freeBusy: