diff --git a/ssh2bpp.c b/ssh2bpp.c index 4563034a..241a4818 100644 --- a/ssh2bpp.c +++ b/ssh2bpp.c @@ -514,7 +514,7 @@ static void ssh2_bpp_handle_input(BinaryPacketProtocol *bpp) s->length = s->payload + 5; - DTS_CONSUME(s->stats, in, s->packetlen); + dts_consume(&s->stats->in, s->packetlen); s->pktin->sequence = s->in.sequence++; @@ -762,8 +762,7 @@ static void ssh2_bpp_format_packet_inner(struct ssh2_bpp_state *s, PktOut *pkt) s->out.sequence++; /* whether or not we MACed */ - DTS_CONSUME(s->stats, out, origlen + padding); - + dts_consume(&s->stats->out, origlen + padding); } static void ssh2_bpp_format_packet(struct ssh2_bpp_state *s, PktOut *pkt) diff --git a/ssh2transport.c b/ssh2transport.c index 98c5a892..6a90ec78 100644 --- a/ssh2transport.c +++ b/ssh2transport.c @@ -1261,8 +1261,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) pktout = ssh_bpp_new_pktout(s->ppl.bpp, SSH2_MSG_NEWKEYS); pq_push(s->ppl.out_pq, pktout); /* Start counting down the outgoing-data limit for these cipher keys. */ - s->stats->out.running = true; - s->stats->out.remaining = s->max_data_size; + dts_reset(&s->stats->out, s->max_data_size); /* * Force the BPP to synchronously marshal all packets up to and @@ -1324,8 +1323,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) return; } /* Start counting down the incoming-data limit for these cipher keys. */ - s->stats->in.running = true; - s->stats->in.remaining = s->max_data_size; + dts_reset(&s->stats->in, s->max_data_size); /* * We've seen incoming NEWKEYS, so create and initialise @@ -1490,10 +1488,10 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) if (!s->rekey_class) { /* If we don't yet have any other reason to rekey, check * if we've hit our data limit in either direction. */ - if (!s->stats->in.running) { + if (s->stats->in.expired) { s->rekey_reason = "too much data received"; s->rekey_class = RK_NORMAL; - } else if (!s->stats->out.running) { + } else if (s->stats->out.expired) { s->rekey_reason = "too much data sent"; s->rekey_class = RK_NORMAL; } @@ -1511,9 +1509,8 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) s->rekey_reason); /* Reset the counters, so that at least this message doesn't * hit the event log _too_ often. */ - s->stats->in.running = s->stats->out.running = true; - s->stats->in.remaining = s->stats->out.remaining = - s->max_data_size; + dts_reset(&s->stats->in, s->max_data_size); + dts_reset(&s->stats->out, s->max_data_size); (void) ssh2_transport_timer_update(s, 0); s->rekey_class = RK_NONE; } else { @@ -1910,11 +1907,9 @@ static void ssh2_transport_reconfigure(PacketProtocolLayer *ppl, Conf *conf) if (s->max_data_size < old_max_data_size) { unsigned long diff = old_max_data_size - s->max_data_size; - /* We must decrement both counters, so avoid short-circuit - * evaluation skipping one */ - bool out_expired = DTS_CONSUME(s->stats, out, diff); - bool in_expired = DTS_CONSUME(s->stats, in, diff); - if (out_expired || in_expired) + dts_consume(&s->stats->out, diff); + dts_consume(&s->stats->in, diff); + if (s->stats->out.expired || s->stats->in.expired) rekey_reason = "data limit lowered"; } else { unsigned long diff = s->max_data_size - old_max_data_size; diff --git a/sshbpp.h b/sshbpp.h index d97d3299..6bf1c3ad 100644 --- a/sshbpp.h +++ b/sshbpp.h @@ -80,25 +80,47 @@ bool ssh2_bpp_check_unimplemented(BinaryPacketProtocol *bpp, PktIn *pktin); * purposes of triggering an SSH-2 rekey when either one gets over a * configured limit. In each direction, the flag 'running' indicates * that we haven't hit the limit yet, and 'remaining' tracks how much - * longer until we do. The macro DTS_CONSUME subtracts a given amount - * from the counter in a particular direction, and evaluates to a - * boolean indicating whether the limit has been hit. + * longer until we do. The function dts_consume() subtracts a given + * amount from the counter in a particular direction, and sets + * 'expired' if the limit has been hit. * * The limit is sticky: once 'running' has flipped to false, * 'remaining' is no longer decremented, so it shouldn't dangerously * wrap round. */ -struct DataTransferStats { - struct { - bool running; - unsigned long remaining; - } in, out; +struct DataTransferStatsDirection { + bool running, expired; + unsigned long remaining; }; -#define DTS_CONSUME(stats, direction, size) \ - ((stats)->direction.running && \ - (stats)->direction.remaining <= (size) ? \ - ((stats)->direction.running = false, true) : \ - ((stats)->direction.remaining -= (size), false)) +struct DataTransferStats { + struct DataTransferStatsDirection in, out; +}; +static inline void dts_consume(struct DataTransferStatsDirection *s, + unsigned long size_consumed) +{ + if (s->running) { + if (s->remaining <= size_consumed) { + s->running = false; + s->expired = true; + } else { + s->remaining -= size_consumed; + } + } +} +static inline void dts_reset(struct DataTransferStatsDirection *s, + unsigned long starting_size) +{ + s->expired = false; + s->remaining = starting_size; + /* + * The semantics of setting CONF_ssh_rekey_data to zero are to + * disable data-volume based rekeying completely. So if the + * starting size is actually zero, we don't set 'running' to true + * in the first place, which means we won't ever set the expired + * flag. + */ + s->running = (starting_size != 0); +} BinaryPacketProtocol *ssh2_bpp_new( LogContext *logctx, struct DataTransferStats *stats, bool is_server);