Reduce the number of round-trips involved in opening an SSH-2 session

by sending most of the initial SSH_MSG_CHANNEL_REQUEST messages before
waiting for any replies.  The initial version of this code was a clever
thing with a two-pass loop, but that got hairy so I went for the simpler
approach of separating the request and reply code and having flags to
keep track of which requests have been sent.

git-svn-id: svn://svn.tartarus.org/sgt/putty@9599 cda61777-01e9-0310-a592-d414129be87e

diff --git a/ssh.c b/ssh.c
index 351003f..f39894a 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -7512,6 +7512,9 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
        int siglen, retlen, len;
        char *q, *agentreq, *ret;
        int try_send;
+       int requested_x11;
+       int requested_agent;
+       int requested_tty;
        int num_env, env_left, env_ok;
        struct Packet *pktout;
        Filename *keyfile;
@@ -8999,6 +9002,17 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
     }
 
     /*
+     * Enable port forwardings.
+     */
+    ssh_setup_portfwd(ssh, ssh->conf);
+
+    /*
+     * Send the CHANNEL_REQUESTS for the main channel.  We send them all
+     * and then start looking for responses, so it's important that the
+     * sending and receiving code below it is kept in sync.
+     */
+
+    /*
      * Potentially enable X11 forwarding.
      */
     if (ssh->mainchan && !ssh->ncmode && conf_get_int(ssh->conf, CONF_x11_forward) &&
@@ -9023,26 +9037,9 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
        end_log_omission(ssh, s->pktout);
        ssh2_pkt_adduint32(s->pktout, ssh->x11disp->screennum);
        ssh2_pkt_send(ssh, s->pktout);
-
-       crWaitUntilV(pktin);
-
-       if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
-           if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
-               bombout(("Unexpected response to X11 forwarding request:"
-                        " packet type %d", pktin->type));
-               crStopV;
-           }
-           logevent("X11 forwarding refused");
-       } else {
-           logevent("X11 forwarding enabled");
-           ssh->X11_fwd_enabled = TRUE;
-       }
-    }
-
-    /*
-     * Enable port forwardings.
-     */
-    ssh_setup_portfwd(ssh, ssh->conf);
+       s->requested_x11 = TRUE;
+    } else
+       s->requested_x11 = FALSE;
 
     /*
      * Potentially enable agent forwarding.
@@ -9054,21 +9051,9 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
        ssh2_pkt_addstring(s->pktout, "auth-agent-req@openssh.com");
        ssh2_pkt_addbool(s->pktout, 1);        /* want reply */
        ssh2_pkt_send(ssh, s->pktout);
-
-       crWaitUntilV(pktin);
-
-       if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
-           if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
-               bombout(("Unexpected response to agent forwarding request:"
-                        " packet type %d", pktin->type));
-               crStopV;
-           }
-           logevent("Agent forwarding refused");
-       } else {
-           logevent("Agent forwarding enabled");
-           ssh->agentfwd_enabled = TRUE;
-       }
-    }
+       s->requested_agent = TRUE;
+    } else
+       s->requested_agent = FALSE;
 
     /*
      * Now allocate a pty for the session.
@@ -9097,25 +9082,9 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
        ssh2_pkt_addstring_data(s->pktout, "\0", 1); /* TTY_OP_END */
        ssh2_pkt_send(ssh, s->pktout);
        ssh->state = SSH_STATE_INTERMED;
-
-       crWaitUntilV(pktin);
-
-       if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
-           if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
-               bombout(("Unexpected response to pty request:"
-                        " packet type %d", pktin->type));
-               crStopV;
-           }
-           c_write_str(ssh, "Server refused to allocate pty\r\n");
-           ssh->editing = ssh->echoing = 1;
-       } else {
-           logeventf(ssh, "Allocated pty (ospeed %dbps, ispeed %dbps)",
-                     ssh->ospeed, ssh->ispeed);
-            ssh->got_pty = TRUE;
-       }
-    } else {
-       ssh->editing = ssh->echoing = 1;
-    }
+       s->requested_tty = TRUE;
+    } else
+       s->requested_tty = FALSE;
 
     /*
      * Send environment variables.
@@ -9123,11 +9092,10 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
      * Simplest thing here is to send all the requests at once, and
      * then wait for a whole bunch of successes or failures.
      */
+    s->num_env = 0;
     if (ssh->mainchan && !ssh->ncmode) {
        char *key, *val;
 
-       s->num_env = 0;
-
        for (val = conf_get_str_strs(ssh->conf, CONF_environmt, NULL, &key);
             val != NULL;
             val = conf_get_str_strs(ssh->conf, CONF_environmt, key, &key)) {
@@ -9141,39 +9109,96 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
 
            s->num_env++;
        }
-
-       if (s->num_env) {
+       if (s->num_env)
            logeventf(ssh, "Sent %d environment variables", s->num_env);
+    }
 
-           s->env_ok = 0;
-           s->env_left = s->num_env;
+    /*
+     * All CHANNEL_REQUESTs sent.  Now collect up the replies.  These
+     * must be in precisely the same order as the requests.
+     */
 
-           while (s->env_left > 0) {
-               crWaitUntilV(pktin);
+    if (s->requested_x11) {
+       crWaitUntilV(pktin);
 
-               if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
-                   if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
-                       bombout(("Unexpected response to environment request:"
-                                " packet type %d", pktin->type));
-                       crStopV;
-                   }
-               } else {
-                   s->env_ok++;
-               }
+       if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
+           if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
+               bombout(("Unexpected response to X11 forwarding request:"
+                        " packet type %d", pktin->type));
+               crStopV;
+           }
+           logevent("X11 forwarding refused");
+       } else {
+           logevent("X11 forwarding enabled");
+           ssh->X11_fwd_enabled = TRUE;
+       }
+    }
+
+    if (s->requested_agent) {
+       crWaitUntilV(pktin);
+
+       if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
+           if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
+               bombout(("Unexpected response to agent forwarding request:"
+                        " packet type %d", pktin->type));
+               crStopV;
+           }
+           logevent("Agent forwarding refused");
+       } else {
+           logevent("Agent forwarding enabled");
+           ssh->agentfwd_enabled = TRUE;
+       }
+    }
+
+    if (s->requested_tty) {
+       crWaitUntilV(pktin);
 
-               s->env_left--;
+       if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
+           if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
+               bombout(("Unexpected response to pty request:"
+                        " packet type %d", pktin->type));
+               crStopV;
            }
+           c_write_str(ssh, "Server refused to allocate pty\r\n");
+           ssh->editing = ssh->echoing = 1;
+       } else {
+           logeventf(ssh, "Allocated pty (ospeed %dbps, ispeed %dbps)",
+                     ssh->ospeed, ssh->ispeed);
+            ssh->got_pty = TRUE;
+       }
+    } else {
+       ssh->editing = ssh->echoing = 1;
+    }
 
-           if (s->env_ok == s->num_env) {
-               logevent("All environment variables successfully set");
-           } else if (s->env_ok == 0) {
-               logevent("All environment variables refused");
-               c_write_str(ssh, "Server refused to set environment variables\r\n");
+    if (s->num_env) {
+       s->env_ok = 0;
+       s->env_left = s->num_env;
+
+       while (s->env_left > 0) {
+           crWaitUntilV(pktin);
+
+           if (pktin->type != SSH2_MSG_CHANNEL_SUCCESS) {
+               if (pktin->type != SSH2_MSG_CHANNEL_FAILURE) {
+                   bombout(("Unexpected response to environment request:"
+                            " packet type %d", pktin->type));
+                   crStopV;
+               }
            } else {
-               logeventf(ssh, "%d environment variables refused",
-                         s->num_env - s->env_ok);
-               c_write_str(ssh, "Server refused to set all environment variables\r\n");
+               s->env_ok++;
            }
+
+           s->env_left--;
+       }
+
+       if (s->env_ok == s->num_env) {
+           logevent("All environment variables successfully set");
+       } else if (s->env_ok == 0) {
+           logevent("All environment variables refused");
+           c_write_str(ssh, "Server refused to set environment variables\r\n");
+       } else {
+           logeventf(ssh, "%d environment variables refused",
+                     s->num_env - s->env_ok);
+           c_write_str(ssh, "Server refused to set all environment variables\r\n");
        }
     }