upstream commit

refactor activation of rekeying

This makes automatic rekeying internal to the packet code (previously
the server and client loops needed to assist). In doing to it makes
application of rekey limits more accurate by accounting for packets
about to be sent as well as packets queued during rekeying events
themselves.

Based on a patch from dtucker@ which was in turn based on a patch
Aleksander Adamowski in bz#2521; ok markus@

Upstream-ID: a441227fd64f9739850ca97b4cf794202860fcd8

diff --git a/clientloop.c b/clientloop.c
index f0a08f234f7e67ee3ae989e0f74330ec2553803d..9820455c429393470c3a472ea934d7051b43bd4a 100644 (file)
--- 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 335b789fc07d4706efc79e75fde17dd5811c0e40..d371f47c48dd935e3c2251861fa5b27209083259 100644 (file)
--- 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 24d4aa15f9c00fefda493afddf93831cc0bd3826..1c589660505941fad416b89fb3e2e356de4d3185 100644 (file)
--- 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 b14b6769a4cdccf1d2370b4e22a9dbccfe9063ea..c26ade44c7f25ef6bbec89c5bd23d80dfa7b7ea7 100644 (file)
--- 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 f7e5bc78f1324a17de0c865272f9d7a5d23e180c..7ddebeb716102ca86c46fd2b456d290bb69821e4 100644 (file)
--- 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 62302747d9b9a39f0a52671219c82b66651a351a..28516a553a2054ad227f2f71aff2eaa08f82c17d 100644 (file)
--- 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 47bc168b27fddf1ed55c22e0a193ab9db217acc6..80d1db5490bcc0b9392824c020291f8ee37cfa8f 100644 (file)
--- 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;