generic-library/ssh2
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

handshake: Compression enabled at the wrong time

Browse Source 
In KEXINIT messages, the client and server agree on, among other
things, whether to use compression. This method agreement occurs
in src/kex.c's kex_agree_methods() function. However, if
compression is enabled (either client->server, server->client, or
both), then the compression layer is initialized in
kex_agree_methods() -- before NEWKEYS has been received.

Instead, the initialization of the compression layer should
happen after NEWKEYS has been received. This looks to occur
insrc/kex.c's diffie_hellman_sha1(), which even has the comment:

    /* The first key exchange has been performed,

        switch to active crypt/comp/mac mode */

There, after NEWKEYS is received, the cipher and mac algorithms
are initialized, and that is where the compression should be
initialized as well.

The current implementation fails if server->client compression is
enabled because most server implementations follow OpenSSH's
lead, where compression is initialized after NEWKEYS. Since the
server initializes compression after NEWKEYS, but libssh2
initializes compression after KEXINIT (i.e. before NEWKEYS), they
are out of sync.

Reported in bug report #180
This commit is contained in:
TJ Saunders 2010-06-23 00:03:31 +02:00 committed by Daniel Stenberg
parent 04f90b2265
commit 2cc4a629ac
2 changed files with 43 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/comp.c
View File

@ -170,6 +170,15 @@ comp_method_zlib_comp(LIBSSH2_SESSION * session,
int out_maxlen = compress ? (src_len + 4) : (2 * src_len); int out_maxlen = compress ? (src_len + 4) : (2 * src_len);
int limiter = 0; int limiter = 0;
/* If strm is null, then we have not yet been initialized. */
if (strm == NULL) {
*dest = (unsigned char *) src;
*dest_len = src_len;
*free_dest = 0;
return 0;
}
/* In practice they never come smaller than this */ /* In practice they never come smaller than this */
if (out_maxlen < 25) { if (out_maxlen < 25) {
out_maxlen = 25; out_maxlen = 25;

46
src/kex.c
View File

@ -600,6 +600,40 @@ static int diffie_hellman_sha1(LIBSSH2_SESSION *session,
} }
_libssh2_debug(session, LIBSSH2_TRACE_KEX, _libssh2_debug(session, LIBSSH2_TRACE_KEX,
"Server to Client HMAC Key calculated"); "Server to Client HMAC Key calculated");
/* Initialize compression for each direction */
/* Cleanup any existing compression */
if (session->local.comp && session->local.comp->dtor) {
session->local.comp->dtor(session, 1,
&session->local.comp_abstract);
}
if (session->local.comp && session->local.comp->init) {
if (session->local.comp->init(session, 1,
&session->local.comp_abstract)) {
ret = LIBSSH2_ERROR_KEX_FAILURE;
goto clean_exit;
}
}
_libssh2_debug(session, LIBSSH2_TRACE_KEX,
"Client to Server compression initialized");
if (session->remote.comp && session->remote.comp->dtor) {
session->remote.comp->dtor(session, 0,
&session->remote.comp_abstract);
}
if (session->remote.comp && session->remote.comp->init) {
if (session->remote.comp->init(session, 0,
&session->remote.comp_abstract)) {
ret = LIBSSH2_ERROR_KEX_FAILURE;
goto clean_exit;
}
}
_libssh2_debug(session, LIBSSH2_TRACE_KEX,
"Server to Client compression initialized");
} }
clean_exit: clean_exit:
@ -1609,18 +1643,6 @@ static int kex_agree_methods(LIBSSH2_SESSION * session, unsigned char *data,
_libssh2_debug(session, LIBSSH2_TRACE_KEX, "Agreed on COMP_SC method: %s", _libssh2_debug(session, LIBSSH2_TRACE_KEX, "Agreed on COMP_SC method: %s",
session->remote.comp->name); session->remote.comp->name);
/* Initialize compression layer */
if (session->local.comp && session->local.comp->init &&
session->local.comp->init(session, 1, &session->local.comp_abstract)) {
return -1;
}
if (session->remote.comp && session->remote.comp->init &&
session->remote.comp->init(session, 0,
&session->remote.comp_abstract)) {
return -1;
}
return 0; return 0;
} }