Skip to content

Commit

Permalink
Split compression s2c/c2s, disable server c2s
Browse files Browse the repository at this point in the history
Disable zlib entirely for server in the client to server direction.
This avoids attack surface for zlib, and also saves 35kB runtime RAM
for the decompression context. Interactive sessions don't have much
traffic in the client->server direction so performance difference
should not be noticable.
  • Loading branch information
mkj committed Dec 3, 2024
1 parent 556064e commit 305bf38
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 31 deletions.
4 changes: 2 additions & 2 deletions src/cli-runopts.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ void cli_getopts(int argc, char ** argv) {
cli_opts.bind_port = NULL;
cli_opts.keepalive_arg = NULL;
#ifndef DISABLE_ZLIB
opts.compress_mode = DROPBEAR_COMPRESS_ON;
opts.allow_compress = 1;
#endif
#if DROPBEAR_USER_ALGO_LIST
opts.cipher_list = NULL;
Expand Down Expand Up @@ -681,7 +681,7 @@ static void parse_multihop_hostname(const char* orighostarg, const char* argv0)
passthrough_args, remainder);
#ifndef DISABLE_ZLIB
/* The stream will be incompressible since it's encrypted. */
opts.compress_mode = DROPBEAR_COMPRESS_OFF;
opts.allow_compress = 0;
#endif
m_free(passthrough_args);
}
Expand Down
45 changes: 26 additions & 19 deletions src/common-kex.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ void send_msg_kexinit() {


/* compression_algorithms_client_to_server */
buf_put_algolist(ses.writepayload, ses.compress_algos);
buf_put_algolist(ses.writepayload, ses.compress_algos_c2s);

/* compression_algorithms_server_to_client */
buf_put_algolist(ses.writepayload, ses.compress_algos);
buf_put_algolist(ses.writepayload, ses.compress_algos_s2c);

/* languages_client_to_server */
buf_putstring(ses.writepayload, "", 0);
Expand Down Expand Up @@ -205,27 +205,34 @@ void recv_msg_newkeys() {
TRACE(("leave recv_msg_newkeys"))
}


/* Set up the kex for the first time */
void kexfirstinitialise() {
static void kex_setup_compress(void) {
#ifdef DISABLE_ZLIB
ses.compress_algos = ssh_nocompress;
ses.compress_algos_c2s = ssh_nocompress;
ses.compress_algos_s2c = ssh_nocompress;
#else
switch (opts.compress_mode)
{
case DROPBEAR_COMPRESS_DELAYED:
ses.compress_algos = ssh_delaycompress;
break;

case DROPBEAR_COMPRESS_ON:
ses.compress_algos = ssh_compress;
break;
if (!opts.allow_compress) {
ses.compress_algos_c2s = ssh_nocompress;
ses.compress_algos_s2c = ssh_nocompress;
return;
}

case DROPBEAR_COMPRESS_OFF:
ses.compress_algos = ssh_nocompress;
break;
if (IS_DROPBEAR_CLIENT) {
/* TODO: should c2s in dbclient be disabled?
* Current Dropbear server disables it. Disabling it also
* lets DROPBEAR_CLI_IMMEDIATE_AUTH work (see comment there) */
ses.compress_algos_c2s = ssh_compress;
ses.compress_algos_s2c = ssh_compress;
} else {
ses.compress_algos_c2s = ssh_nocompress;
ses.compress_algos_s2c = ssh_compress;
}
#endif
}

/* Set up the kex for the first time */
void kexfirstinitialise() {
kex_setup_compress();
kexinitialise();
}

Expand Down Expand Up @@ -948,15 +955,15 @@ static void read_kex_algos() {
DEBUG2(("hmac s2c is %s", s2c_hash_algo ? s2c_hash_algo->name : "<implicit>"))

/* compression_algorithms_client_to_server */
c2s_comp_algo = buf_match_algo(ses.payload, ses.compress_algos, 0, NULL);
c2s_comp_algo = buf_match_algo(ses.payload, ses.compress_algos_c2s, 0, NULL);
if (c2s_comp_algo == NULL) {
erralgo = "comp c->s";
goto error;
}
DEBUG2(("comp c2s is %s", c2s_comp_algo->name))

/* compression_algorithms_server_to_client */
s2c_comp_algo = buf_match_algo(ses.payload, ses.compress_algos, 0, NULL);
s2c_comp_algo = buf_match_algo(ses.payload, ses.compress_algos_s2c, 0, NULL);
if (s2c_comp_algo == NULL) {
erralgo = "comp s->c";
goto error;
Expand Down
11 changes: 3 additions & 8 deletions src/runopts.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,9 @@ typedef struct runopts {
int usingsyslog;

#ifndef DISABLE_ZLIB
/* TODO: add a commandline flag. Currently this is on by default if compression
* is compiled in, but disabled for a client's non-final multihop stages. (The
* intermediate stages are compressed streams, so are uncompressible. */
enum {
DROPBEAR_COMPRESS_DELAYED, /* Server only */
DROPBEAR_COMPRESS_ON,
DROPBEAR_COMPRESS_OFF,
} compress_mode;
/* Whether any compression is allowed. The specific method used
* varies between client and server, it will be set up by kex_setup_compress() */
int allow_compress;
#endif

#if DROPBEAR_USER_ALGO_LIST
Expand Down
3 changes: 2 additions & 1 deletion src/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ struct sshsession {
can add it to the hash when generating keys */

/* Enables/disables compression */
algo_type *compress_algos;
algo_type *compress_algos_c2s;
algo_type *compress_algos_s2c;

/* Other side allows SSH_MSG_EXT_INFO. Currently only set for server */
int allow_ext_info;
Expand Down
2 changes: 1 addition & 1 deletion src/svr-runopts.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void svr_getopts(int argc, char ** argv) {
svr_opts.reexec_childpipe = -1;

#ifndef DISABLE_ZLIB
opts.compress_mode = DROPBEAR_COMPRESS_DELAYED;
opts.allow_compress = 1;
#endif

/* not yet
Expand Down

0 comments on commit 305bf38

Please sign in to comment.