From bfbb2f3d22b687df4d7e073b42d3ce1409f49574 Mon Sep 17 00:00:00 2001
From: Melroy van den Berg <melroy@melroy.org>
Date: Tue, 19 May 2020 22:57:19 +0200
Subject: [PATCH 05/11] Refactor and fix channel/session closes

---
 src/sshmasterconnection.cpp | 272 ++++++++++++++++++++----------------
 src/sshmasterconnection.h   |  13 +-
 2 files changed, 156 insertions(+), 129 deletions(-)

diff --git a/src/sshmasterconnection.cpp b/src/sshmasterconnection.cpp
index aa75ec1..c068c89 100644
--- a/src/sshmasterconnection.cpp
+++ b/src/sshmasterconnection.cpp
@@ -544,6 +544,7 @@ void SshMasterConnection::run()
 #endif
     }
 
+    // Set time-out to 1 min.
     long timeout = 60;
 
     my_ssh_session = ssh_new();
@@ -694,7 +695,7 @@ void SshMasterConnection::run()
             else
             {
                 x2goDebug << "Login Check - Failed" << endl;
-                //          if(!interactionInterrupt)
+                // if(!interactionInterrupt)
                 {
                     emit finishInteraction(this);
                 }
@@ -787,7 +788,7 @@ bool SshMasterConnection::sshConnect()
     /* Parse ~/.ssh/config. */
     if (ssh_options_parse_config(my_ssh_session, NULL) < 0)
     {
-        x2goDebug << "Warning: unable to parse the SSH config file.";
+        x2goDebug << "Warning: unable to parse the SSH config file." << endl;
     }
 
 #if LIBSSH_VERSION_INT >= SSH_VERSION_INT(0, 6, 0)
@@ -849,6 +850,7 @@ bool SshMasterConnection::sshConnect()
 
                 work_port = inferred_port & 0xFFFF;
 
+                ssh_disconnect(tmp_session);
                 ssh_free(tmp_session);
             }
         }
@@ -1974,129 +1976,16 @@ void SshMasterConnection::channelLoop()
 
         for (int i = 0; i < channelConnections.size(); ++i)
         {
-            int tcpSocket = channelConnections.at(i).sock;
-            if (tcpSocket > 0)
-                FD_SET(tcpSocket, &rfds);
-            if (channelConnections.at(i).channel == 0l)
+            // Try to make a channel connection
+            // TODO: Mabye a retry?
+            bool res = createChannelConnection(i, maxsock, rfds, read_chan);
+            if (!res)
             {
-                x2goDebug << "Creating new channel." << endl;
-                ssh_channel channel = ssh_channel_new(my_ssh_session);
-
-                if (!channel)
-                {
-                    QString err = ssh_get_error(my_ssh_session);
-                    /*: Argument in this context will be a function name. */
-                    QString error_msg = tr("%1 failed.").arg("ssh_channel_new");
-                    emit ioErr(channelConnections[i].creator, error_msg, err);
-
-                    x2goDebug << error_msg.left(error_msg.size() - 1) << ": " << err << endl;
-
-                    continue;
-                }
-                x2goDebug << "New channel:" << channel << endl;
-                channelConnections[i].channel = channel;
-                if (tcpSocket > 0)
-                {
-                    x2goDebug << "Forwarding parameters: from remote (" << channelConnections.at(i).forwardHost << ":"
-                              << channelConnections.at(i).forwardPort << ") to local ("
-                              << channelConnections.at(i).localHost << ":" << channelConnections.at(i).localPort
-                              << ")" << endl;
-
-                    /*
-                     * Cannot support config file parsing here with pre-0.6.0 libssh versions.
-                     * There's just no way to get the inferred host and port values.
-                     */
-#if LIBSSH_VERSION_INT >= SSH_VERSION_INT(0, 6, 0)
-                    ssh_session tmp_session = ssh_new();
-
-                    if (!tmp_session)
-                    {
-                        QString error_msg = tr("Cannot create SSH session.");
-                        x2goDebug << error_msg << endl;
-                        emit ioErr(channelConnections[i].creator, error_msg, "");
-                    }
-                    else
-                    {
-                        QByteArray tmp_BA = channelConnections.at(i).forwardHost.toLocal8Bit();
-                        const int tmp_port = channelConnections.at(i).forwardPort;
-
-                        ssh_options_set(tmp_session, SSH_OPTIONS_HOST, tmp_BA.data());
-
-                        if (tmp_port)
-                        {
-                            ssh_options_set(tmp_session, SSH_OPTIONS_PORT, &tmp_port);
-                        }
-
-                        /* The host and port might be a shorthand and zero, so fetch the actual data. */
-                        if (ssh_options_parse_config(tmp_session, NULL) < 0)
-                        {
-                            x2goDebug << "Warning: unable to parse the SSH config file." << endl;
-                        }
-
-                        unsigned int inferred_port = 0;
-                        ssh_options_get_port(tmp_session, &inferred_port);
-                        x2goDebug << "Temporary session port after config file parse: " << inferred_port << endl;
-
-                        char *inferred_host = NULL;
-                        ssh_options_get(tmp_session, SSH_OPTIONS_HOST, &inferred_host);
-                        x2goDebug << "Temporary session host after config file parse: " << inferred_host << endl;
-
-                        channelConnections[i].forwardHost = QString(inferred_host);
-                        channelConnections[i].forwardPort = static_cast<int>(inferred_port);
-
-                        ssh_string_free_char(inferred_host);
-                        ssh_free(tmp_session);
-                    }
-#endif
-
-                    {
-                        QByteArray tmp_BA_forward = channelConnections.at(i).forwardHost.toLocal8Bit();
-                        QByteArray tmp_BA_local = channelConnections.at(i).localHost.toLocal8Bit();
-
-                        if (ssh_channel_open_forward(channel,
-                                                     tmp_BA_forward.data(),
-                                                     channelConnections.at(i).forwardPort,
-                                                     tmp_BA_local.data(),
-                                                     channelConnections.at(i).localPort) != SSH_OK)
-                        {
-                            QString err = ssh_get_error(my_ssh_session);
-                            QString errorMsg = tr("%1 failed.").arg("ssh_channel_open_forward");
-                            emit ioErr(channelConnections[i].creator, errorMsg, err);
-                            x2goDebug << errorMsg.left(errorMsg.size() - 1) << ": " << err << endl;
-                        }
-                        else
-                        {
-                            x2goDebug << "New channel forwarded." << endl;
-                        }
-                    }
-                }
-                else
-                {
-                    x2goDebug << "Executing remote: " << channelConnections.at(i).command << endl;
-                    if (ssh_channel_open_session(channel) != SSH_OK)
-                    {
-                        QString err = ssh_get_error(my_ssh_session);
-                        QString errorMsg = tr("%1 failed.").arg("ssh_channel_open_session");
-                        emit ioErr(channelConnections[i].creator, errorMsg, err);
-                        x2goDebug << errorMsg.left(errorMsg.size() - 1) << ": " << err << endl;
-                    }
-                    else if (ssh_channel_request_exec(channel, channelConnections[i].command.toLatin1()) != SSH_OK)
-                    {
-                        QString err = ssh_get_error(my_ssh_session);
-                        QString errorMsg = tr("%1 failed.").arg("ssh_channel_request_exec");
-                        emit ioErr(channelConnections[i].creator, errorMsg, err);
-                        x2goDebug << errorMsg.left(errorMsg.size() - 1) << ": " << err << endl;
-                    }
-                    else
-                    {
-                        x2goDebug << "New exec channel created." << endl;
-                    }
-                }
+                x2goDebug << "Connection to channel went wrong.";
+                continue;
             }
-            read_chan[i] = channelConnections.at(i).channel;
-            if (tcpSocket > maxsock)
-                maxsock = tcpSocket;
         }
+
         channelConnectionsMutex.unlock();
         retval = ssh_select(read_chan, out_chan, maxsock + 1, &rfds, &tv);
         delete[] read_chan;
@@ -2219,6 +2108,145 @@ void SshMasterConnection::channelLoop()
     }
 }
 
+/**
+ * Create SSH channel connection (used within a loop of channels)
+ */
+bool SshMasterConnection::createChannelConnection(int i, int &maxsock, fd_set &rfds, ssh_channel read_chan[])
+{
+    int tcpSocket = channelConnections.at(i).sock;
+    if (tcpSocket > 0)
+        FD_SET(tcpSocket, &rfds);
+    if (channelConnections.at(i).channel == 0l)
+    {
+        x2goDebug << "Creating new channel." << endl;
+
+        ssh_channel channel = ssh_channel_new(my_ssh_session);
+        if (!channel)
+        {
+            QString err = ssh_get_error(my_ssh_session);
+            /*: Argument in this context will be a function name. */
+            QString error_msg = tr("%1 failed.").arg("ssh_channel_new");
+            emit ioErr(channelConnections[i].creator, error_msg, err);
+
+            x2goDebug << error_msg.left(error_msg.size() - 1) << ": " << err << endl;
+            return false;
+        }
+        x2goDebug << "New channel:" << channel << endl;
+        channelConnections[i].channel = channel;
+        if (tcpSocket > 0)
+        {
+            x2goDebug << "Forwarding parameters: from remote (" << channelConnections.at(i).forwardHost << ":"
+                      << channelConnections.at(i).forwardPort << ") to local ("
+                      << channelConnections.at(i).localHost << ":" << channelConnections.at(i).localPort
+                      << ")";
+
+            /*
+                * Cannot support config file parsing here with pre-0.6.0 libssh versions.
+                * There's just no way to get the inferred host and port values.
+                */
+#if LIBSSH_VERSION_INT >= SSH_VERSION_INT(0, 6, 0)
+            ssh_session tmp_session = ssh_new();
+
+            if (!tmp_session)
+            {
+                QString error_msg = tr("Cannot create SSH session.");
+                x2goDebug << error_msg << endl;
+                emit ioErr(channelConnections[i].creator, error_msg, "");
+            }
+            else
+            {
+                QByteArray tmp_BA = channelConnections.at(i).forwardHost.toLocal8Bit();
+                const int tmp_port = channelConnections.at(i).forwardPort;
+
+                ssh_options_set(tmp_session, SSH_OPTIONS_HOST, tmp_BA.data());
+
+                if (tmp_port)
+                {
+                    ssh_options_set(tmp_session, SSH_OPTIONS_PORT, &tmp_port);
+                }
+
+                /* The host and port might be a shorthand and zero, so fetch the actual data. */
+                if (ssh_options_parse_config(tmp_session, NULL) < 0)
+                {
+                    x2goDebug << "Warning: unable to parse the SSH config file." << endl;
+                }
+
+                unsigned int inferred_port = 0;
+                ssh_options_get_port(tmp_session, &inferred_port);
+                x2goDebug << "Temporary session port after config file parse: " << inferred_port << endl;
+
+                char *inferred_host = NULL;
+                ssh_options_get(tmp_session, SSH_OPTIONS_HOST, &inferred_host);
+                x2goDebug << "Temporary session host after config file parse: " << inferred_host << endl;
+
+                channelConnections[i].forwardHost = QString(inferred_host);
+                channelConnections[i].forwardPort = static_cast<int>(inferred_port);
+
+                ssh_string_free_char(inferred_host);
+                ssh_free(tmp_session);
+            }
+#endif
+
+            {
+                QByteArray tmp_BA_forward = channelConnections.at(i).forwardHost.toLocal8Bit();
+                QByteArray tmp_BA_local = channelConnections.at(i).localHost.toLocal8Bit();
+
+                if (ssh_channel_open_forward(channel,
+                                             tmp_BA_forward.data(),
+                                             channelConnections.at(i).forwardPort,
+                                             tmp_BA_local.data(),
+                                             channelConnections.at(i).localPort) != SSH_OK)
+                {
+                    QString err = ssh_get_error(my_ssh_session);
+                    QString errorMsg = tr("%1 failed.").arg("ssh_channel_open_forward");
+                    emit ioErr(channelConnections[i].creator, errorMsg, err);
+                    x2goDebug << errorMsg.left(errorMsg.size() - 1) << ": " << err << endl;
+                }
+                else
+                {
+                    x2goDebug << "New channel forwarded." << endl;
+                }
+            }
+        }
+        else
+        {
+            x2goDebug << "Executing remote: " << channelConnections.at(i).command << endl;
+            if (ssh_channel_open_session(channel) != SSH_OK)
+            {
+                QString err = ssh_get_error(my_ssh_session);
+                QString errorMsg = tr("%1 failed.").arg("ssh_channel_open_session");
+
+                // Free channel
+                ssh_channel_free(channel);
+                emit ioErr(channelConnections[i].creator, errorMsg, err);
+                x2goDebug << errorMsg.left(errorMsg.size() - 1) << ": " << err << endl;
+                return false;
+            }
+            else if (ssh_channel_request_exec(channel, channelConnections[i].command.toLatin1()) != SSH_OK)
+            {
+                QString err = ssh_get_error(my_ssh_session);
+                QString errorMsg = tr("%1 failed.").arg("ssh_channel_request_exec");
+
+                // Free close connection and free channel
+                ssh_channel_close(channel);
+                ssh_channel_free(channel);
+                emit ioErr(channelConnections[i].creator, errorMsg, err);
+                x2goDebug << errorMsg.left(errorMsg.size() - 1) << ": " << err << endl;
+                return false;
+            }
+            else
+            {
+                // Everything is OK
+                x2goDebug << "New exec channel created." << endl;
+            }
+        }
+    }
+    read_chan[i] = channelConnections.at(i).channel;
+    if (tcpSocket > maxsock)
+        maxsock = tcpSocket;
+    return true;
+}
+
 void SshMasterConnection::finalize(int item)
 {
     int tcpSocket = channelConnections.at(item).sock;
diff --git a/src/sshmasterconnection.h b/src/sshmasterconnection.h
index c7883be..effae16 100644
--- a/src/sshmasterconnection.h
+++ b/src/sshmasterconnection.h
@@ -27,13 +27,11 @@
 #include <QTcpSocket>
 #include <QNetworkProxy>
 
-#define PROPERTY(TYPE, NAME)           \
-private:                               \
-    TYPE NAME;                         \
-                                       \
-public:                                \
-    TYPE get_##NAME() { return NAME; } \
-    void set_##NAME(TYPE VAL) { NAME = VAL; }
+
+#define PROPERTY(TYPE,NAME)  private: TYPE NAME; \
+public: TYPE get_##NAME(){return NAME;} \
+void set_##NAME(TYPE VAL){NAME=VAL;}
+
 
 class ONMainWindow;
 class SshProcess;
@@ -141,6 +139,7 @@ private:
     bool userAuthKrb();
     bool userAuthKeyboardInteractive(QString prompt);
     void channelLoop();
+    bool createChannelConnection(int i, int& maxsock, fd_set& rfds, ssh_channel read_chan[]);
     void finalize(int arg1);
     void copy();
     int serverAuth(QString &errorMsg);
-- 
2.17.1

