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 f0a08f2..9820455 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 @@
{
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 @@
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 @@
*/
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 335b789..d371f47 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 @@
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 24d4aa1..1c58966 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_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 b14b676..c26ade4 100644
--- a/opacket.h
+++ b/opacket.h
@@ -125,8 +125,6 @@
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 f7e5bc7..7ddebeb 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 @@
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 @@
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 @@
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 @@
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 @@
#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 @@
}
}
-#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 6230274..28516a5 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_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 @@
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 47bc168..80d1db5 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 @@
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 @@
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 @@
}
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;