From 406685c276b460858010bc9128ab95ecb5635dc1 Mon Sep 17 00:00:00 2001 From: Fabian Vogt Date: Thu, 23 Dec 2021 12:34:00 +0100 Subject: [PATCH] Soften behaviour of the Compression=no/yes option Currently Compression=no (the default) force-disables zlib algos, while Compression=yes force-enables it. This means that mismatching options between client and server lead to connection failure. This can easily happen if the server has default settings but the client specifies Compression=yes. OpenSSH treats the option as a "prefer compression" setting: Compression=no -> none,zlib@openssh.com,zlib (default) Compression=yes -> zlib@openssh.com,zlib,none This commit changes the libssh behaviour to the same as OpenSSH. Signed-off-by: Fabian Vogt --- src/kex.c | 6 +++--- src/options.c | 8 ++++---- tests/unittests/torture_config.c | 10 ++++++---- tests/unittests/torture_options.c | 29 +++++++++++++++++++++++++---- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/kex.c b/src/kex.c index 21641cf0d..236d34a48 100644 --- a/src/kex.c +++ b/src/kex.c @@ -88,7 +88,7 @@ #endif /* HAVE_LIBCRYPTO */ #ifdef WITH_ZLIB -#define ZLIB "none,zlib,zlib@openssh.com" +#define ZLIB "none,zlib@openssh.com,zlib" #else #define ZLIB "none" #endif @@ -229,8 +229,8 @@ static const char *default_methods[] = { CHACHA20 AES, "hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-sha2-256,hmac-sha2-512", "hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-sha2-256,hmac-sha2-512", - "none", - "none", + ZLIB, + ZLIB, "", "", NULL diff --git a/src/options.c b/src/options.c index 264eacb7c..2c5a86b4d 100644 --- a/src/options.c +++ b/src/options.c @@ -844,10 +844,10 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type, return -1; } else { if (strcasecmp(value,"yes")==0){ - if(ssh_options_set_algo(session,SSH_COMP_C_S,"zlib@openssh.com,zlib") < 0) + if(ssh_options_set_algo(session,SSH_COMP_C_S,"zlib@openssh.com,zlib,none") < 0) return -1; } else if (strcasecmp(value,"no")==0){ - if(ssh_options_set_algo(session,SSH_COMP_C_S,"none") < 0) + if(ssh_options_set_algo(session,SSH_COMP_C_S,"none,zlib@openssh.com,zlib") < 0) return -1; } else { if (ssh_options_set_algo(session, SSH_COMP_C_S, v) < 0) @@ -862,10 +862,10 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type, return -1; } else { if (strcasecmp(value,"yes")==0){ - if(ssh_options_set_algo(session,SSH_COMP_S_C,"zlib@openssh.com,zlib") < 0) + if(ssh_options_set_algo(session,SSH_COMP_S_C,"zlib@openssh.com,zlib,none") < 0) return -1; } else if (strcasecmp(value,"no")==0){ - if(ssh_options_set_algo(session,SSH_COMP_S_C,"none") < 0) + if(ssh_options_set_algo(session,SSH_COMP_S_C,"none,zlib@openssh.com,zlib") < 0) return -1; } else { if (ssh_options_set_algo(session, SSH_COMP_S_C, v) < 0) diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c index a23acc9c1..ee070f62a 100644 --- a/tests/unittests/torture_config.c +++ b/tests/unittests/torture_config.c @@ -507,12 +507,14 @@ static void torture_config_new(void ** state, assert_string_equal(session->opts.bindaddr, BIND_ADDRESS); #ifdef WITH_ZLIB assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], - "zlib@openssh.com,zlib"); + "zlib@openssh.com,zlib,none"); assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], - "zlib@openssh.com,zlib"); + "zlib@openssh.com,zlib,none"); #else - assert_null(session->opts.wanted_methods[SSH_COMP_C_S]); - assert_null(session->opts.wanted_methods[SSH_COMP_S_C]); + assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], + "none"); + assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], + "none"); #endif /* WITH_ZLIB */ assert_int_equal(session->opts.StrictHostKeyChecking, 0); assert_int_equal(session->opts.gss_delegate_creds, 1); diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c index d2ceedd57..922b2aaf0 100644 --- a/tests/unittests/torture_options.c +++ b/tests/unittests/torture_options.c @@ -950,10 +950,17 @@ static void torture_options_getopt(void **state) assert_string_equal(session->opts.wanted_methods[SSH_CRYPT_S_C], "aes128-ctr"); assert_string_equal(session->opts.identity->root->data, "id_rsa"); +#ifdef WITH_ZLIB assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], - "zlib@openssh.com,zlib"); + "zlib@openssh.com,zlib,none"); assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], - "zlib@openssh.com,zlib"); + "zlib@openssh.com,zlib,none"); +#else + assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], + "none"); + assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], + "none"); +#endif /* -1 and -2 are noop */ @@ -1024,19 +1031,33 @@ static void torture_options_getopt(void **state) argc = 2; rc = ssh_options_set(session, SSH_OPTIONS_COMPRESSION, "no"); assert_ssh_return_code(session, rc); +#ifdef WITH_ZLIB + assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], + "none,zlib@openssh.com,zlib"); + assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], + "none,zlib@openssh.com,zlib"); +#else assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], "none"); assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], "none"); +#endif rc = ssh_options_getopt(session, &argc, (char **)argv); assert_ssh_return_code(session, rc); assert_int_equal(argc, 1); assert_string_equal(argv[0], EXECUTABLE_NAME); +#ifdef WITH_ZLIB + assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], + "zlib@openssh.com,zlib,none"); + assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], + "zlib@openssh.com,zlib,none"); +#else assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S], - "zlib@openssh.com,zlib"); + "none"); assert_string_equal(session->opts.wanted_methods[SSH_COMP_S_C], - "zlib@openssh.com,zlib"); + "none"); +#endif /* Corner case: only hostname is not parsed */ argv[1] = "example.com"; -- GitLab