7 files changed, 158 insertions, 74 deletions

diff --git a/clientloop.c b/clientloop.c
index f0a08f234..9820455c4 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.283 2016/02/01 21:18:17 millert Exp $ */
+/* $OpenBSD: clientloop.c,v 1.284 2016/02/08 10:57:07 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1502,7 +1502,7 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
 {
         fd_set *readset = NULL, *writeset = NULL;
         double start_time, total_time;
-        int r, max_fd = 0, max_fd2 = 0, len, rekeying = 0;
+        int r, max_fd = 0, max_fd2 = 0, len;
         u_int64_t ibytes, obytes;
         u_int nalloc = 0;
         char buf[100];
@@ -1617,10 +1617,15 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
                 if (compat20 && session_closed && !channel_still_open())
                         break;
 
-                rekeying = (active_state->kex != NULL && !active_state->kex->done);
-
-                if (rekeying) {
+                if (ssh_packet_is_rekeying(active_state)) {
                         debug("rekeying in progress");
+                } else if (need_rekeying) {
+                        /* manual rekey request */
+                        debug("need rekeying");
+                        if ((r = kex_start_rekex(active_state)) != 0)
+                                fatal("%s: kex_start_rekex: %s", __func__,
+                                    ssh_err(r));
+                        need_rekeying = 0;
                 } else {
                         /*
                          * Make packets of buffered stdin data, and buffer
@@ -1651,23 +1656,14 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
                  */
                 max_fd2 = max_fd;
                 client_wait_until_can_do_something(&readset, &writeset,
-                    &max_fd2, &nalloc, rekeying);
+                    &max_fd2, &nalloc, ssh_packet_is_rekeying(active_state));
 
                 if (quit_pending)
                         break;
 
                 /* Do channel operations unless rekeying in progress. */
-                if (!rekeying) {
+                if (!ssh_packet_is_rekeying(active_state))
                         channel_after_select(readset, writeset);
-                        if (need_rekeying || packet_need_rekeying()) {
-                                debug("need rekeying");
-                                active_state->kex->done = 0;
-                                if ((r = kex_send_kexinit(active_state)) != 0)
-                                        fatal("%s: kex_send_kexinit: %s",
-                                            __func__, ssh_err(r));
-                                need_rekeying = 0;
-                        }
-                }
 
                 /* Buffer input from the connection.  */
                 client_process_net_input(readset);
diff --git a/kex.c b/kex.c
index 335b789fc..d371f47c4 100644
--- a/kex.c
+++ b/kex.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.c,v 1.116 2016/01/14 16:17:39 markus Exp $ */
+/* $OpenBSD: kex.c,v 1.117 2016/02/08 10:57:07 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  *
@@ -606,6 +606,25 @@ kex_setup(struct ssh *ssh, char *proposal[PROPOSAL_MAX])
         return 0;
 }
 
+/*
+ * Request key re-exchange, returns 0 on success or a ssherr.h error
+ * code otherwise. Must not be called if KEX is incomplete or in-progress.
+ */
+int
+kex_start_rekex(struct ssh *ssh)
+{
+        if (ssh->kex == NULL) {
+                error("%s: no kex", __func__);
+                return SSH_ERR_INTERNAL_ERROR;
+        }
+        if (ssh->kex->done == 0) {
+                error("%s: requested twice", __func__);
+                return SSH_ERR_INTERNAL_ERROR;
+        }
+        ssh->kex->done = 0;
+        return kex_send_kexinit(ssh);
+}
+
 static int
 choose_enc(struct sshenc *enc, char *client, char *server)
 {
diff --git a/kex.h b/kex.h
index 24d4aa15f..1c5896605 100644
--- a/kex.h
+++ b/kex.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.h,v 1.75 2016/01/14 16:17:39 markus Exp $ */
+/* $OpenBSD: kex.h,v 1.76 2016/02/08 10:57:07 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -179,6 +179,7 @@ int	 kex_input_ext_info(int, u_int32_t, void *);
 int      kex_derive_keys(struct ssh *, u_char *, u_int, const struct sshbuf *);
 int      kex_derive_keys_bn(struct ssh *, u_char *, u_int, const BIGNUM *);
 int      kex_send_newkeys(struct ssh *);
+int      kex_start_rekex(struct ssh *);
 
 int      kexdh_client(struct ssh *);
 int      kexdh_server(struct ssh *);
diff --git a/opacket.h b/opacket.h
index b14b6769a..c26ade44c 100644
--- a/opacket.h
+++ b/opacket.h
@@ -125,8 +125,6 @@ void	packet_disconnect(const char *, ...)
         sshpkt_add_padding(active_state, (pad))
 #define packet_send_ignore(nbytes) \
         ssh_packet_send_ignore(active_state, (nbytes))
-#define packet_need_rekeying() \
-        ssh_packet_need_rekeying(active_state)
 #define packet_set_server() \
         ssh_packet_set_server(active_state)
 #define packet_set_authenticated() \
diff --git a/packet.c b/packet.c
index f7e5bc78f..7ddebeb71 100644
--- a/packet.c
+++ b/packet.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.c,v 1.227 2016/02/04 23:43:48 djm Exp $ */
+/* $OpenBSD: packet.c,v 1.228 2016/02/08 10:57:07 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -259,6 +259,14 @@ ssh_alloc_session_state(void)
         return NULL;
 }
 
+/* Returns nonzero if rekeying is in progress */
+int
+ssh_packet_is_rekeying(struct ssh *ssh)
+{
+        return ssh->state->rekeying ||
+            (ssh->kex != NULL && ssh->kex->done == 0);
+}
+
 /*
  * Sets the descriptors used for communication.  Disables encryption until
  * packet_set_encryption_key is called.
@@ -1029,6 +1037,51 @@ ssh_set_newkeys(struct ssh *ssh, int mode)
         return 0;
 }
 
+#define MAX_PACKETS     (1U<<31)
+static int
+ssh_packet_need_rekeying(struct ssh *ssh, u_int outbound_packet_len)
+{
+        struct session_state *state = ssh->state;
+        u_int32_t out_blocks;
+
+        /* XXX client can't cope with rekeying pre-auth */
+        if (!state->after_authentication)
+                return 0;
+
+        /* Haven't keyed yet or KEX in progress. */
+        if (ssh->kex == NULL || ssh_packet_is_rekeying(ssh))
+                return 0;
+
+        /* Peer can't rekey */
+        if (ssh->compat & SSH_BUG_NOREKEY)
+                return 0;
+
+        /*
+         * Permit one packet in or out per rekey - this allows us to
+         * make progress when rekey limits are very small.
+         */
+        if (state->p_send.packets == 0 && state->p_read.packets == 0)
+                return 0;
+
+        /* Time-based rekeying */
+        if (state->rekey_interval != 0 &&
+            state->rekey_time + state->rekey_interval <= monotime())
+                return 1;
+
+        /* Always rekey when MAX_PACKETS sent in either direction */
+        if (state->p_send.packets > MAX_PACKETS ||
+            state->p_read.packets > MAX_PACKETS)
+                return 1;
+
+        /* Rekey after (cipher-specific) maxiumum blocks */
+        out_blocks = roundup(outbound_packet_len,
+            state->newkeys[MODE_OUT]->enc.block_size);
+        return (state->max_blocks_out &&
+            (state->p_send.blocks + out_blocks > state->max_blocks_out)) ||
+            (state->max_blocks_in &&
+            (state->p_read.blocks > state->max_blocks_in));
+}
+
 /*
  * Delayed compression for SSH2 is enabled after authentication:
  * This happens on the server side after a SSH2_MSG_USERAUTH_SUCCESS is sent,
@@ -1232,35 +1285,58 @@ ssh_packet_send2_wrapped(struct ssh *ssh)
         return r;
 }
 
+/* returns non-zero if the specified packet type is usec by KEX */
+static int
+ssh_packet_type_is_kex(u_char type)
+{
+        return
+            type >= SSH2_MSG_TRANSPORT_MIN &&
+            type <= SSH2_MSG_TRANSPORT_MAX &&
+            type != SSH2_MSG_SERVICE_REQUEST &&
+            type != SSH2_MSG_SERVICE_ACCEPT &&
+            type != SSH2_MSG_EXT_INFO;
+}
+
 int
 ssh_packet_send2(struct ssh *ssh)
 {
         struct session_state *state = ssh->state;
         struct packet *p;
         u_char type;
-        int r;
+        int r, need_rekey;
 
+        if (sshbuf_len(state->outgoing_packet) < 6)
+                return SSH_ERR_INTERNAL_ERROR;
         type = sshbuf_ptr(state->outgoing_packet)[5];
+        need_rekey = !ssh_packet_type_is_kex(type) &&
+            ssh_packet_need_rekeying(ssh, sshbuf_len(state->outgoing_packet));
 
-        /* during rekeying we can only send key exchange messages */
-        if (state->rekeying) {
-                if ((type < SSH2_MSG_TRANSPORT_MIN) ||
-                    (type > SSH2_MSG_TRANSPORT_MAX) ||
-                    (type == SSH2_MSG_SERVICE_REQUEST) ||
-                    (type == SSH2_MSG_SERVICE_ACCEPT) ||
-                    (type == SSH2_MSG_EXT_INFO)) {
-                        debug("enqueue packet: %u", type);
-                        p = calloc(1, sizeof(*p));
-                        if (p == NULL)
-                                return SSH_ERR_ALLOC_FAIL;
-                        p->type = type;
-                        p->payload = state->outgoing_packet;
-                        TAILQ_INSERT_TAIL(&state->outgoing, p, next);
-                        state->outgoing_packet = sshbuf_new();
-                        if (state->outgoing_packet == NULL)
-                                return SSH_ERR_ALLOC_FAIL;
-                        return 0;
+        /*
+         * During rekeying we can only send key exchange messages.
+         * Queue everything else.
+         */
+        if ((need_rekey || state->rekeying) && !ssh_packet_type_is_kex(type)) {
+                if (need_rekey)
+                        debug3("%s: rekex triggered", __func__);
+                debug("enqueue packet: %u", type);
+                p = calloc(1, sizeof(*p));
+                if (p == NULL)
+                        return SSH_ERR_ALLOC_FAIL;
+                p->type = type;
+                p->payload = state->outgoing_packet;
+                TAILQ_INSERT_TAIL(&state->outgoing, p, next);
+                state->outgoing_packet = sshbuf_new();
+                if (state->outgoing_packet == NULL)
+                        return SSH_ERR_ALLOC_FAIL;
+                if (need_rekey) {
+                        /*
+                         * This packet triggered a rekey, so send the
+                         * KEXINIT now.
+                         * NB. reenters this function via kex_start_rekex().
+                         */
+                        return kex_start_rekex(ssh);
                 }
+                return 0;
         }
 
         /* rekeying starts with sending KEXINIT */
@@ -1276,10 +1352,22 @@ ssh_packet_send2(struct ssh *ssh)
                 state->rekey_time = monotime();
                 while ((p = TAILQ_FIRST(&state->outgoing))) {
                         type = p->type;
+                        /*
+                         * If this packet triggers a rekex, then skip the
+                         * remaining packets in the queue for now.
+                         * NB. re-enters this function via kex_start_rekex.
+                         */
+                        if (ssh_packet_need_rekeying(ssh,
+                            sshbuf_len(p->payload))) {
+                                debug3("%s: queued packet triggered rekex",
+                                    __func__);
+                                return kex_start_rekex(ssh);
+                        }
                         debug("dequeue packet: %u", type);
                         sshbuf_free(state->outgoing_packet);
                         state->outgoing_packet = p->payload;
                         TAILQ_REMOVE(&state->outgoing, p, next);
+                        memset(p, 0, sizeof(*p));
                         free(p);
                         if ((r = ssh_packet_send2_wrapped(ssh)) != 0)
                                 return r;
@@ -1784,6 +1872,13 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
 #endif
         /* reset for next packet */
         state->packlen = 0;
+
+        /* do we need to rekey? */
+        if (ssh_packet_need_rekeying(ssh, 0)) {
+                debug3("%s: rekex triggered", __func__);
+                if ((r = kex_start_rekex(ssh)) != 0)
+                        return r;
+        }
  out:
         return r;
 }
@@ -2268,25 +2363,6 @@ ssh_packet_send_ignore(struct ssh *ssh, int nbytes)
         }
 }
 
-#define MAX_PACKETS     (1U<<31)
-int
-ssh_packet_need_rekeying(struct ssh *ssh)
-{
-        struct session_state *state = ssh->state;
-
-        if (ssh->compat & SSH_BUG_NOREKEY)
-                return 0;
-        return
-            (state->p_send.packets > MAX_PACKETS) ||
-            (state->p_read.packets > MAX_PACKETS) ||
-            (state->max_blocks_out &&
-                (state->p_send.blocks > state->max_blocks_out)) ||
-            (state->max_blocks_in &&
-                (state->p_read.blocks > state->max_blocks_in)) ||
-            (state->rekey_interval != 0 && state->rekey_time +
-                 state->rekey_interval <= monotime());
-}
-
 void
 ssh_packet_set_rekey_limits(struct ssh *ssh, u_int64_t bytes, time_t seconds)
 {
diff --git a/packet.h b/packet.h
index 62302747d..28516a553 100644
--- a/packet.h
+++ b/packet.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.h,v 1.69 2016/01/29 02:54:45 dtucker Exp $ */
+/* $OpenBSD: packet.h,v 1.70 2016/02/08 10:57:07 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -86,6 +86,7 @@ int      ssh_packet_get_connection_in(struct ssh *);
 int      ssh_packet_get_connection_out(struct ssh *);
 void     ssh_packet_close(struct ssh *);
 void     ssh_packet_set_encryption_key(struct ssh *, const u_char *, u_int, int);
+int      ssh_packet_is_rekeying(struct ssh *);
 void     ssh_packet_set_protocol_flags(struct ssh *, u_int);
 u_int    ssh_packet_get_protocol_flags(struct ssh *);
 int      ssh_packet_start_compression(struct ssh *, int);
@@ -145,7 +146,6 @@ int	 ssh_packet_set_state(struct ssh *, struct sshbuf *);
 const char *ssh_remote_ipaddr(struct ssh *);
 int      ssh_remote_port(struct ssh *);
 
-int      ssh_packet_need_rekeying(struct ssh *);
 void     ssh_packet_set_rekey_limits(struct ssh *, u_int64_t, time_t);
 time_t   ssh_packet_get_rekey_timeout(struct ssh *);
 
diff --git a/serverloop.c b/serverloop.c
index 47bc168b2..80d1db549 100644
--- a/serverloop.c
+++ b/serverloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: serverloop.c,v 1.181 2016/01/14 16:17:40 markus Exp $ */
+/* $OpenBSD: serverloop.c,v 1.182 2016/02/08 10:57:07 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -820,7 +820,7 @@ void
 server_loop2(Authctxt *authctxt)
 {
         fd_set *readset = NULL, *writeset = NULL;
-        int rekeying = 0, max_fd;
+        int max_fd;
         u_int nalloc = 0;
         u_int64_t rekey_timeout_ms = 0;
 
@@ -847,11 +847,11 @@ server_loop2(Authctxt *authctxt)
         for (;;) {
                 process_buffered_input_packets();
 
-                rekeying = (active_state->kex != NULL && !active_state->kex->done);
-
-                if (!rekeying && packet_not_very_much_data_to_write())
+                if (!ssh_packet_is_rekeying(active_state) &&
+                    packet_not_very_much_data_to_write())
                         channel_output_poll();
-                if (options.rekey_interval > 0 && compat20 && !rekeying)
+                if (options.rekey_interval > 0 && compat20 &&
+                    !ssh_packet_is_rekeying(active_state))
                         rekey_timeout_ms = packet_get_rekey_timeout() * 1000;
                 else
                         rekey_timeout_ms = 0;
@@ -866,14 +866,8 @@ server_loop2(Authctxt *authctxt)
                 }
 
                 collect_children();
-                if (!rekeying) {
+                if (!ssh_packet_is_rekeying(active_state))
                         channel_after_select(readset, writeset);
-                        if (packet_need_rekeying()) {
-                                debug("need rekeying");
-                                active_state->kex->done = 0;
-                                kex_send_kexinit(active_state);
-                        }
-                }
                 process_input(readset);
                 if (connection_closed)
                         break;