Skip to content

Commit b9bfb5d

Browse files
committed
Still trying to figure out how to maintain a minimal memory profile
while maintaining performance. When the memory is huge (by setting rlen to a huge size in sshbuf_allocate) we get higher perfomance but the memory usage is stupid. This commit also change the rekey frequency for CC20. That was taken from a post made by Damien Miller
1 parent d1ccfb0 commit b9bfb5d

File tree

4 files changed

+81
-39
lines changed

4 files changed

+81
-39
lines changed

cipher.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,47 @@ cipher_blocksize(const struct sshcipher *c)
178178
return (c->block_size);
179179
}
180180

181+
uint64_t
182+
cipher_rekey_blocks(const struct sshcipher *c)
183+
{
184+
/*
185+
* Chacha20-Poly1305 does not benefit from data-based rekeying,
186+
* per "The Security of ChaCha20-Poly1305 in the Multi-user Setting",
187+
* Degabriele, J. P., Govinden, J, Gunther, F. and Paterson K.
188+
* ACM CCS 2021; https://eprint.iacr.org/2023/085.pdf
189+
*
190+
* Cryptanalysis aside, we do still want do need to prevent the SSH
191+
* sequence number wrapping and also to rekey to provide some
192+
* protection for long lived sessions against key disclosure at the
193+
* endpoints, so arrange for rekeying every 2**32 blocks as the
194+
* 128-bit block ciphers do (i.e. every 32GB data).
195+
*/
196+
if ((c->flags & CFLAG_CHACHAPOLY) != 0)
197+
return (uint64_t)1 << 32;
198+
199+
/* there is no actual need to rekey the NULL cipher but
200+
* rekeying is a necessary step. In part, as mentioned above,
201+
* to keep the seqnr from wrapping. So we set it to the
202+
* maximum possible -cjr 4/10/23 */
203+
if ((c->flags & CFLAG_NONE) != 0)
204+
return (uint64_t)1 << 32;
205+
206+
/*
207+
* The 2^(blocksize*2) limit is too expensive for 3DES,
208+
* so enforce a 1GB data limit for small blocksizes.
209+
* See discussion in RFC4344 section 3.2.
210+
*/
211+
if (c->block_size < 16)
212+
return ((uint64_t)1 << 30) / c->block_size;
213+
/*
214+
* Otherwise, use the RFC4344 s3.2 recommendation of 2**(L/4) blocks
215+
* before rekeying where L is the blocksize in bits.
216+
* Most other ciphers have a 128 bit blocksize, so this equates to
217+
* 2**32 blocks / 64GB data.
218+
*/
219+
return (uint64_t)1 << (c->block_size * 2);
220+
}
221+
181222
u_int
182223
cipher_keylen(const struct sshcipher *c)
183224
{

cipher.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ int cipher_get_length(struct sshcipher_ctx *, u_int *, u_int,
6363
const u_char *, u_int);
6464
void cipher_free(struct sshcipher_ctx *);
6565
u_int cipher_blocksize(const struct sshcipher *);
66+
uint64_t cipher_rekey_blocks(const struct sshcipher *);
6667
u_int cipher_keylen(const struct sshcipher *);
6768
u_int cipher_seclen(const struct sshcipher *);
6869
u_int cipher_authlen(const struct sshcipher *);

packet.c

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@
6363
#endif
6464
#include <signal.h>
6565
#include <time.h>
66+
#ifdef HAVE_UTIL_H
67+
# include <util.h>
68+
#endif
6669

6770
/*
6871
* Explicitly include OpenSSL before zlib as some versions of OpenSSL have
@@ -103,12 +106,16 @@
103106
#define DBG(x)
104107
#endif
105108

106-
/* SSH_IOBUFSZ + 1k of head room */
107109
/* OpenSSH usings 256KB packet size max but that consumes a
108-
* lot of memory wiht the buffers we are using. This keeps it
109-
* in check. Doesn't seem to have an impact on performance or
110-
* functionality cjr 04/06/2023 */
111-
#define PACKET_MAX_SIZE (SSH_IOBUFSZ + 1024)
110+
* lot of memory with the buffers we are using. However, we need
111+
* a large packet size if the banner that's being sent is large.
112+
* So we need a 256KB packet pre authentication and a smaller one
113+
* in this case SSH_IOBUFSZ + 1KB, afterwards. So we change
114+
* PACKET_MAX_SIZE from a #define to a global. Then, in the function
115+
* ssh_packet_set_authentcated we reduce the size to something
116+
* more memory efficient. -cjr 04/07/23
117+
*/
118+
u_int packet_max_size = 256 * 1024;
112119

113120
struct packet_state {
114121
u_int32_t seqnr;
@@ -397,7 +404,7 @@ ssh_packet_stop_discard(struct ssh *ssh)
397404

398405
if (state->packet_discard_mac) {
399406
char buf[1024];
400-
size_t dlen = PACKET_MAX_SIZE;
407+
size_t dlen = packet_max_size;
401408

402409
if (dlen > state->packet_discard_mac_already)
403410
dlen -= state->packet_discard_mac_already;
@@ -884,6 +891,7 @@ ssh_set_newkeys(struct ssh *ssh, int mode)
884891
const char *wmsg;
885892
int r, crypt_type;
886893
const char *dir = mode == MODE_OUT ? "out" : "in";
894+
char blocks_s[FMT_SCALED_STRSIZE], bytes_s[FMT_SCALED_STRSIZE];
887895

888896
debug2_f("mode %d", mode);
889897

@@ -952,29 +960,20 @@ ssh_set_newkeys(struct ssh *ssh, int mode)
952960
}
953961
comp->enabled = 1;
954962
}
955-
/*
956-
* The 2^(blocksize*2) limit is too expensive for 3DES,
957-
* so enforce a 1GB limit for small blocksizes.
958-
* See RFC4344 section 3.2.
959-
*/
960963

961-
/* we really don't need to rekey if we are using the none cipher
962-
* but there isn't a good way to disable it entirely that I can find
963-
* and using a blocksize larger that 16 doesn't work (dunno why)
964-
* so this seems to be a good limit for now - CJR 10/16/2020*/
965-
if (ssh->none == 1) {
966-
*max_blocks = (u_int64_t)1 << (31*2);
967-
} else {
968-
if (enc->block_size >= 16)
969-
*max_blocks = (u_int64_t)1 << (enc->block_size*2);
970-
else
971-
*max_blocks = ((u_int64_t)1 << 30) / enc->block_size;
972-
}
973-
if (state->rekey_limit)
974-
*max_blocks = MINIMUM(*max_blocks,
975-
state->rekey_limit / enc->block_size);
976-
debug("rekey %s after %llu blocks", dir,
977-
(unsigned long long)*max_blocks);
964+
/* get the maximum number of blocks the cipher can
965+
* handle safely */
966+
*max_blocks = cipher_rekey_blocks(enc->cipher);
967+
968+
/* these lines support the debug */
969+
strlcpy(blocks_s, "?", sizeof(blocks_s));
970+
strlcpy(bytes_s, "?", sizeof(bytes_s));
971+
if (*max_blocks * enc->block_size < LLONG_MAX) {
972+
fmt_scaled((long long)*max_blocks, blocks_s);
973+
fmt_scaled((long long)*max_blocks * enc->block_size, bytes_s);
974+
}
975+
debug("rekey %s after %s blocks / %sB data", dir, blocks_s, bytes_s);
976+
978977
return 0;
979978
}
980979

@@ -1504,7 +1503,7 @@ ssh_packet_read_poll2_mux(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
15041503
return 0; /* packet is incomplete */
15051504
state->packlen = PEEK_U32(cp);
15061505
if (state->packlen < 4 + 1 ||
1507-
state->packlen > PACKET_MAX_SIZE)
1506+
state->packlen > packet_max_size)
15081507
return SSH_ERR_MESSAGE_INCOMPLETE;
15091508
}
15101509
need = state->packlen + 4;
@@ -1563,7 +1562,7 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
15631562
sshbuf_ptr(state->input), sshbuf_len(state->input)) != 0)
15641563
return 0;
15651564
if (state->packlen < 1 + 4 ||
1566-
state->packlen > PACKET_MAX_SIZE) {
1565+
state->packlen > packet_max_size) {
15671566
#ifdef PACKET_DEBUG
15681567
sshbuf_dump(state->input, stderr);
15691568
#endif
@@ -1590,7 +1589,7 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
15901589
goto out;
15911590
state->packlen = PEEK_U32(sshbuf_ptr(state->incoming_packet));
15921591
if (state->packlen < 1 + 4 ||
1593-
state->packlen > PACKET_MAX_SIZE) {
1592+
state->packlen > packet_max_size) {
15941593
#ifdef PACKET_DEBUG
15951594
fprintf(stderr, "input: \n");
15961595
sshbuf_dump(state->input, stderr);
@@ -1599,7 +1598,7 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
15991598
#endif
16001599
logit("Bad packet length %u.", state->packlen);
16011600
return ssh_packet_start_discard(ssh, enc, mac, 0,
1602-
PACKET_MAX_SIZE);
1601+
packet_max_size);
16031602
}
16041603
if ((r = sshbuf_consume(state->input, block_size)) != 0)
16051604
goto out;
@@ -1622,7 +1621,7 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
16221621
logit("padding error: need %d block %d mod %d",
16231622
need, block_size, need % block_size);
16241623
return ssh_packet_start_discard(ssh, enc, mac, 0,
1625-
PACKET_MAX_SIZE - block_size);
1624+
packet_max_size - block_size);
16261625
}
16271626
/*
16281627
* check if the entire packet has been received and
@@ -1666,11 +1665,11 @@ ssh_packet_read_poll2(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
16661665
if (r != SSH_ERR_MAC_INVALID)
16671666
goto out;
16681667
logit("Corrupted MAC on input.");
1669-
if (need + block_size > PACKET_MAX_SIZE)
1668+
if (need + block_size > packet_max_size)
16701669
return SSH_ERR_INTERNAL_ERROR;
16711670
return ssh_packet_start_discard(ssh, enc, mac,
16721671
sshbuf_len(state->incoming_packet),
1673-
PACKET_MAX_SIZE - need - block_size);
1672+
packet_max_size - need - block_size);
16741673
}
16751674
/* Remove MAC from input buffer */
16761675
DBG(debug("MAC #%d ok", state->p_read.seqnr));
@@ -1842,7 +1841,7 @@ ssh_packet_process_read(struct ssh *ssh, int fd)
18421841
int r;
18431842
size_t rlen;
18441843

1845-
if ((r = sshbuf_read(fd, state->input, PACKET_MAX_SIZE, &rlen)) != 0)
1844+
if ((r = sshbuf_read(fd, state->input, packet_max_size, &rlen)) != 0)
18461845
return r;
18471846

18481847
if (state->packet_discard) {
@@ -2241,6 +2240,7 @@ void
22412240
ssh_packet_set_authenticated(struct ssh *ssh)
22422241
{
22432242
ssh->state->after_authentication = 1;
2243+
packet_max_size = 256 * 1024;
22442244
}
22452245

22462246
void *

sshbuf.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
#define SSHBUF_INTERNAL
2828
#include "sshbuf.h"
2929
#include "misc.h"
30-
/* #include "log.h" */
30+
#include "log.h"
3131

3232
#define BUF_WATERSHED 256*1024
3333

@@ -425,8 +425,8 @@ sshbuf_allocate(struct sshbuf *buf, size_t len)
425425
* up being somewhat overallocated but works quickly */
426426
need = (4*1024*1024);
427427
rlen = ROUNDUP(buf->alloc + need, SSHBUF_SIZE_INC);
428-
/* debug_f ("Post: label: %s, %p, rlen is %zu need is %zu win_max is %zu max_size is %zu",
429-
buf->label, buf, rlen, need, buf->window_max, buf->max_size);*/
428+
debug_f ("Post: label: %s, %p, rlen is %zu need is %zu win_max is %zu max_size is %zu",
429+
buf->label, buf, rlen, need, buf->window_max, buf->max_size);
430430
}
431431
SSHBUF_DBG(("need %zu initial rlen %zu", need, rlen));
432432

0 commit comments

Comments
 (0)