1
1

Fix handshake bug with AEAD ciphers and no HMAC overlap

There's currently a bug in libssh that a handshake doesn't complete if
there is no overlap between HMAC methods, but when an AEAD cipher is
used.

In case of an AEAD cipher such as chacha20-poly1305 or aes256-gcm, the
HMAC algorithm that is being picked is not relevant. But the problem
here is that the HMAC still needs to have an overlap in the handshake,
even if it is not used afterwards.

This was found with a very strict server side configuration with libssh
where only AEAD ciphers and EtM HMAC modes are accepted. The client
tested against was dropbear.

Dropbear does have support for chacha20-poly1305 and AES GCM modes, but
no support for EtM HMAC modes. This meant that the libssh server in this
case rejected the dropbear client, even though it is perfectly able to
serve it since dropbear supports AEAD algorithms.

The fix implemented here updates the HMAC phase of the handshake to
handle this case. If it detects an AEAD cipher is used, it uses the HMAC
abbreviations for the method instead. This is the same name that is used
in other places as well. It matches the client to server and server to
client values, but it does depend on the order of things in the
ssh_kex_types_e enum, which I'm assuming here is ok since it's explicit.

I've looked at how to add a test for this, but I couldn't really find a
suitable place for it. I would love some tips if this is easily
possible, or if it's easier for someone else to contribute, that's of
course welcome too.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
Этот коммит содержится в:
Dirkjan Bussink 2020-09-29 15:14:09 +02:00 коммит произвёл Jakub Jelen
родитель 39cbe8178e
Коммит 42741b1883

Просмотреть файл

@ -755,13 +755,29 @@ int ssh_set_client_kex(ssh_session session)
return SSH_OK;
}
static const char *ssh_find_aead_hmac(const char *cipher)
{
if (cipher == NULL) {
return NULL;
} else if (strcmp(cipher, "chacha20-poly1305@openssh.com") == 0) {
return "aead-poly1305";
} else if (strcmp(cipher, "aes256-gcm@openssh.com") == 0) {
return "aead-gcm";
} else if (strcmp(cipher, "aes128-gcm@openssh.com") == 0) {
return "aead-gcm";
}
return NULL;
}
/** @brief Select the different methods on basis of client's and
* server's kex messages, and watches out if a match is possible.
*/
int ssh_kex_select_methods (ssh_session session){
int ssh_kex_select_methods (ssh_session session)
{
struct ssh_kex_struct *server = &session->next_crypto->server_kex;
struct ssh_kex_struct *client = &session->next_crypto->client_kex;
char *ext_start = NULL;
const char *aead_hmac = NULL;
int i;
/* Here we should drop the ext-info-c from the list so we avoid matching.
@ -773,7 +789,15 @@ int ssh_kex_select_methods (ssh_session session){
for (i = 0; i < SSH_KEX_METHODS; i++) {
session->next_crypto->kex_methods[i]=ssh_find_matching(server->methods[i],client->methods[i]);
if(session->next_crypto->kex_methods[i] == NULL && i < SSH_LANG_C_S){
if (i == SSH_MAC_C_S || i == SSH_MAC_S_C) {
aead_hmac = ssh_find_aead_hmac(session->next_crypto->kex_methods[i-2]);
if (aead_hmac) {
free(session->next_crypto->kex_methods[i]);
session->next_crypto->kex_methods[i] = strdup(aead_hmac);
}
}
if (session->next_crypto->kex_methods[i] == NULL && i < SSH_LANG_C_S){
ssh_set_error(session,SSH_FATAL,"kex error : no match for method %s: server [%s], client [%s]",
ssh_kex_descriptions[i],server->methods[i],client->methods[i]);
return SSH_ERROR;
@ -782,31 +806,31 @@ int ssh_kex_select_methods (ssh_session session){
session->next_crypto->kex_methods[i] = strdup("");
}
}
if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group1-sha1") == 0){
if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group1-sha1") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP1_SHA1;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha1") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha1") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP14_SHA1;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha256") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group14-sha256") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP14_SHA256;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group16-sha512") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group16-sha512") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP16_SHA512;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group18-sha512") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group18-sha512") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GROUP18_SHA512;
#ifdef WITH_GEX
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha1") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha1") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GEX_SHA1;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha256") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "diffie-hellman-group-exchange-sha256") == 0){
session->next_crypto->kex_type=SSH_KEX_DH_GEX_SHA256;
#endif /* WITH_GEX */
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp256") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp256") == 0){
session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP256;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp384") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp384") == 0){
session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP384;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp521") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp521") == 0){
session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP521;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256@libssh.org") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256@libssh.org") == 0){
session->next_crypto->kex_type=SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG;
} else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256") == 0){
} else if (strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256") == 0){
session->next_crypto->kex_type=SSH_KEX_CURVE25519_SHA256;
}
SSH_LOG(SSH_LOG_INFO, "Negotiated %s,%s,%s,%s,%s,%s,%s,%s,%s,%s",
@ -826,7 +850,8 @@ int ssh_kex_select_methods (ssh_session session){
/* this function only sends the predefined set of kex methods */
int ssh_send_kex(ssh_session session, int server_kex) {
int ssh_send_kex(ssh_session session, int server_kex)
{
struct ssh_kex_struct *kex = (server_kex ? &session->next_crypto->server_kex :
&session->next_crypto->client_kex);
ssh_string str = NULL;
@ -1043,7 +1068,7 @@ int ssh_make_sessionid(ssh_session session)
ssh_buffer_get(server_hash),
server_pubkey_blob);
SSH_STRING_FREE(server_pubkey_blob);
if(rc != SSH_OK){
if (rc != SSH_OK){
goto error;
}